format: Output list and diff changes with --fail flag (#4508) (#4710)

The change enables using --diff and --list together with --fail as discussed in #4508, for example: $ opa fmt [path [...]] --list --fail; exit $? path/to/file-1.rego path/to/file-2.rego unexpected diff 2 Previously, the same command returned only the error: $ opa fmt [path [...]] --list --fail; exit $? unexpected diff 2 Signed-off-by: David Kuridža <david@kuridza.si>
open-policy-agent · Jun 5, 2022 · 6e5c0fc · 6e5c0fc
1 parent 51e1a9a
commit 6e5c0fc
Show file tree

Hide file tree

Showing 2 changed files with 122 additions and 20 deletions.
diff --git a/cmd/fmt.go b/cmd/fmt.go
@@ -116,7 +116,7 @@ func formatFile(params *fmtCommandParams, out io.Writer, filename string, info o
 
 	changed := !bytes.Equal(contents, formatted)
 
-	if params.fail {
+	if params.fail && !params.list && !params.diff {
 		if changed {
 			return newError("unexpected diff")
 		}
@@ -125,6 +125,10 @@ func formatFile(params *fmtCommandParams, out io.Writer, filename string, info o
 	if params.list {
 		if changed {
 			fmt.Fprintln(out, filename)
+
+			if params.fail {
+				return newError("unexpected diff")
+			}
 		}
 		return nil
 	}
@@ -138,6 +142,10 @@ func formatFile(params *fmtCommandParams, out io.Writer, filename string, info o
 			}
 
 			fmt.Fprintln(out, stdout.String())
+
+			if params.fail {
+				return newError("unexpected diff")
+			}
 		}
 		return nil
 	}

diff --git a/cmd/fmt_test.go b/cmd/fmt_test.go
@@ -20,20 +20,20 @@ p {
 }
 `
 
+const unformatted = `
+        package test
+
+        p { a == 1; true
+                1 +    3
+        }
+
+
+`
+
 func TestFmtFormatFile(t *testing.T) {
 	params := fmtCommandParams{}
 	var stdout bytes.Buffer
 
-	unformatted := `
-	package test
-	
-	p { a == 1; true
-		1 +    3
-	}
-
-	
-	`
-
 	files := map[string]string{
 		"policy.rego": unformatted,
 	}
@@ -76,6 +76,32 @@ func TestFmtFormatFileNoChanges(t *testing.T) {
 	})
 }
 
+func TestFmtFailFormatFileNoChanges(t *testing.T) {
+	params := fmtCommandParams{
+		fail: true,
+		diff: true,
+	}
+	var stdout bytes.Buffer
+
+	files := map[string]string{
+		"policy.rego": formatted,
+	}
+
+	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("Unexpected error: %v", err)
+		}
+
+		actual := stdout.String()
+		if len(actual) > 0 {
+			t.Fatalf("Expected no output, got:\n%v\n\n", actual)
+		}
+	})
+}
+
 func TestFmtFormatFileDiff(t *testing.T) {
 	params := fmtCommandParams{
 		diff: true,
@@ -128,6 +154,58 @@ func TestFmtFormatFileList(t *testing.T) {
 	})
 }
 
+func TestFmtFailFormatFileList(t *testing.T) {
+	params := fmtCommandParams{
+		fail: true,
+		list: true,
+	}
+	var stdout bytes.Buffer
+
+	files := map[string]string{
+		"policy.rego": formatted,
+	}
+
+	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("Unexpected error: %v", err)
+		}
+
+		actual := strings.TrimSpace(stdout.String())
+		if len(actual) > 0 {
+			t.Fatalf("Expected no output, got:\n%v\n\n", actual)
+		}
+	})
+}
+
+func TestFmtFailFormatFileChangesList(t *testing.T) {
+	params := fmtCommandParams{
+		fail: true,
+		list: true,
+	}
+	var stdout bytes.Buffer
+
+	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("Unexpected error: %v", err)
+		}
+
+		actual := strings.TrimSpace(stdout.String())
+		if len(actual) == 0 {
+			t.Fatalf("Expected output, got:\n%v\n\n", actual)
+		}
+	})
+}
+
 func TestFmtFailFileNoChanges(t *testing.T) {
 	params := fmtCommandParams{
 		fail: true,
@@ -152,15 +230,26 @@ func TestFmtFailFileChanges(t *testing.T) {
 		fail: true,
 	}
 
-	unformatted := `
-	package test
-	
-	p { a == 1; true
-		1 +    3
+	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, ioutil.Discard, policyFile, info, err)
+		if err == nil {
+			t.Fatalf("Unexpected error: %s", err)
+		}
+	})
+}
+
+func TestFmtFailFileChangesDiff(t *testing.T) {
+	params := fmtCommandParams{
+		diff: true,
+		fail: true,
+	}
+	var stdout bytes.Buffer
 
 	files := map[string]string{
 		"policy.rego": unformatted,
@@ -169,9 +258,14 @@ func TestFmtFailFileChanges(t *testing.T) {
 	test.WithTempFS(files, func(path string) {
 		policyFile := filepath.Join(path, "policy.rego")
 		info, err := os.Stat(policyFile)
-		err = formatFile(&params, ioutil.Discard, policyFile, info, err)
+		err = formatFile(&params, &stdout, policyFile, info, err)
 		if err == nil {
-			t.Fatalf("Unexpected error: %s", err)
+			t.Fatalf("Unexpected error: %v", err)
+		}
+
+		actual := strings.TrimSpace(stdout.String())
+		if len(actual) == 0 {
+			t.Fatalf("Expected output, got:\n%v\n\n", actual)
 		}
 	})
 }