Skip to content

Commit

Permalink
cmd: removes hidden dependency on diff command (#6452)
Browse files Browse the repository at this point in the history
Replace call to local diff command as a hidden dependency
with call to go-diff, which is already included in the repo.

Fixes #6284

Signed-off-by: Colin Lacy <colinjlacy@gmail.com>
  • Loading branch information
colinjlacy committed Dec 4, 2023
1 parent 26ceadb commit 2f43132
Show file tree
Hide file tree
Showing 2 changed files with 75 additions and 36 deletions.
44 changes: 8 additions & 36 deletions cmd/fmt.go
Expand Up @@ -9,9 +9,9 @@ import (
"fmt"
"io"
"os"
"os/exec"
"path/filepath"

"github.com/sergi/go-diff/diffmatchpatch"
"github.com/spf13/cobra"

"github.com/open-policy-agent/opa/format"
Expand Down Expand Up @@ -135,14 +135,10 @@ func formatFile(params *fmtCommandParams, out io.Writer, filename string, info o

if params.diff {
if changed {
stdout, stderr, err := doDiff(contents, formatted)
if err != nil && stdout.Len() == 0 {
fmt.Fprintln(os.Stderr, stderr.String())
return newError("failed to diff formatting: %v", err)
diffString := doDiff(contents, formatted)
if _, err := fmt.Fprintln(out, diffString); err != nil {
return newError("failed to print contents: %v", err)
}

fmt.Fprintln(out, stdout.String())

if params.fail {
return newError("unexpected diff")
}
Expand Down Expand Up @@ -183,34 +179,10 @@ func formatStdin(params *fmtCommandParams, r io.Reader, w io.Writer) error {
return err
}

func doDiff(old, new []byte) (stdout, stderr bytes.Buffer, err error) {
o, err := os.CreateTemp("", ".opafmt")
if err != nil {
return stdout, stderr, err
}
defer os.Remove(o.Name())
defer o.Close()

n, err := os.CreateTemp("", ".opafmt")
if err != nil {
return stdout, stderr, err
}
defer os.Remove(n.Name())
defer n.Close()

_, err = o.Write(old)
if err != nil {
return stdout, stderr, err
}
_, err = n.Write(new)
if err != nil {
return stdout, stderr, err
}

cmd := exec.Command("diff", "-u", o.Name(), n.Name())
cmd.Stdout = &stdout
cmd.Stderr = &stderr
return stdout, stderr, cmd.Run()
func doDiff(old, new []byte) (diffString string) {
dmp := diffmatchpatch.New()
diffs := dmp.DiffMain(string(old), string(new), false)
return dmp.DiffPrettyText(diffs)
}

type fmtError struct {
Expand Down
67 changes: 67 additions & 0 deletions cmd/fmt_test.go
Expand Up @@ -54,6 +54,14 @@ p {
}
`

type errorWriter struct {
ErrMsg string
}

func (ew errorWriter) Write(p []byte) (n int, err error) {
return 0, fmt.Errorf(ew.ErrMsg)
}

func TestFmtFormatFile(t *testing.T) {
params := fmtCommandParams{}
var stdout bytes.Buffer
Expand All @@ -77,6 +85,36 @@ func TestFmtFormatFile(t *testing.T) {
})
}

func TestFmtFormatFileFailToReadFile(t *testing.T) {

params := fmtCommandParams{
diff: true,
}

var stdout = bytes.Buffer{}

files := map[string]string{
"policy.rego": unformatted,
}

notThere := "notThere.rego"

test.WithTempFS(files, func(path string) {
policyFile := filepath.Join(path, "policy.rego")
info, err := os.Stat(policyFile)
err = formatFile(&params, &stdout, notThere, info, err)
if err == nil {
t.Fatalf("Expected error, found none")
}

actual := err.Error()

if !strings.Contains(actual, notThere) {
t.Fatalf("Expected error message to include %s, got:\n%s\n\n", notThere, actual)
}
})
}

func TestFmtFormatFileNoChanges(t *testing.T) {
params := fmtCommandParams{}
var stdout bytes.Buffer
Expand Down Expand Up @@ -152,6 +190,35 @@ func TestFmtFormatFileDiff(t *testing.T) {
})
}

func TestFmtFormatFileFailToPrintDiff(t *testing.T) {

params := fmtCommandParams{
diff: true,
}

errMsg := "io.Write error"
var stdout = errorWriter{ErrMsg: errMsg}

files := map[string]string{
"policy.rego": unformatted,
}

test.WithTempFS(files, func(path string) {
policyFile := filepath.Join(path, "policy.rego")
info, err := os.Stat(policyFile)
err = formatFile(&params, &stdout, policyFile, info, err)
if err == nil {
t.Fatalf("Expected error, found none")
}

actual := err.Error()

if !strings.Contains(actual, errMsg) {
t.Fatalf("Expected error message to include %s, got:\n%s\n\n", errMsg, actual)
}
})
}

func TestFmtFormatFileList(t *testing.T) {
params := fmtCommandParams{
list: true,
Expand Down

0 comments on commit 2f43132

Please sign in to comment.