Repository: pre-commit/pre-commit-hooks

Issue:
`pretty-format-json` stops formatting JSONs when it encounters one with incorrect format (e.g. JSON5 comments)
Transferred from https://github.com/pre-commit/pre-commit/issues/3124.

`pretty-format-json` prints "Input File {json_file} is not a valid JSON, consider using check-json" when it encounters an incorrectly formatted JSON file (e.g. with JSON5 comments), but it's not clear from that message that it then stops further processing files.

As `pre-commit` can partition the processed files into batches, this makes it even harder to tell as multiple batches of files can fail midway through with a few "X is not a valid JSON" messages printed, giving the impression that all the JSONs were formatted (or attempted formatted) and only those files printed are incorrect. The reality is that some JSONs may not have been formatted. I think this hook should try to format every JSON and only fail at the end if any have an invalid JSON format.

Discussion:
amarvin: @asottile, any chance you could consider this change, please?
amarvin: > this needs a test which is why I didn't look at it

Great point! I added a test that failed in `main`, but passes in this branch.

Patch:
diff --git a/pre_commit_hooks/pretty_format_json.py b/pre_commit_hooks/pretty_format_json.py
index 627a11c..5c0292b 100644
--- a/pre_commit_hooks/pretty_format_json.py
+++ b/pre_commit_hooks/pretty_format_json.py
@@ -115,16 +115,20 @@ def main(argv: Sequence[str] | None = None) -> int:
                 f'Input File {json_file} is not a valid JSON, consider using '
                 f'check-json',
             )
-            return 1
-
-        if contents != pretty_contents:
-            if args.autofix:
-                _autofix(json_file, pretty_contents)
-            else:
-                diff_output = get_diff(contents, pretty_contents, json_file)
-                sys.stdout.buffer.write(diff_output.encode())
-
             status = 1
+        else:
+            if contents != pretty_contents:
+                if args.autofix:
+                    _autofix(json_file, pretty_contents)
+                else:
+                    diff_output = get_diff(
+                        contents,
+                        pretty_contents,
+                        json_file,
+                    )
+                    sys.stdout.buffer.write(diff_output.encode())
+
+                status = 1
 
     return status
 


Test patch:
diff --git a/tests/pretty_format_json_test.py b/tests/pretty_format_json_test.py
index 5ded724..68b6d7a 100644
--- a/tests/pretty_format_json_test.py
+++ b/tests/pretty_format_json_test.py
@@ -82,6 +82,24 @@ def test_autofix_main(tmpdir):
     assert ret == 0
 
 
+def test_invalid_main(tmpdir):
+    srcfile1 = tmpdir.join('not_valid_json.json')
+    srcfile1.write(
+        '{\n'
+        '  // not json\n'
+        '  "a": "b"\n'
+        '}',
+    )
+    srcfile2 = tmpdir.join('to_be_json_formatted.json')
+    srcfile2.write('{ "a": "b" }')
+
+    # it should have skipped the first file and formatted the second one
+    assert main(['--autofix', str(srcfile1), str(srcfile2)]) == 1
+
+    # confirm second file was formatted (shouldn't trigger linter again)
+    assert main([str(srcfile2)]) == 0
+
+
 def test_orderfile_get_pretty_format():
     ret = main((
         '--top-keys=alist', get_resource_path('pretty_formatted_json.json'),