Repository: pre-commit/pre-commit-hooks
Pull request #558 (2021-02-16 20:45:37)

Report duplicate keys in check_json
Raise ValueError and return 1 if json contains duplicate keys

Fixes https://github.com/pre-commit/pre-commit-hooks/issues/554

asottile: there's a few things CI is pointing out that you'll want to fix:

from the tox output:

```
pre_commit_hooks/check_json.py      29      1      6      1    94%   15->16, 16
```

this indicates that the code on line 16 is not covered by an automated test -- there's a set of tests for this module in tests/check_json_test.py -- you can probably find some examples there to fix it

from the pre-commit output:

```
pre_commit_hooks/check_json.py:12: error: Missing type parameters for generic type "Dict"
```

this indicates that the `Dict` generic does not have its type parameters -- usually these are `Dict[str, Any]` when loading from json.  I'm also pretty sure that `Hashable` => `str` in the function signature should be changed too? not 100% sure on that though I'd have to clone and check with `pre-commit run mypy --all-files`
AdityaKhursale: > there's a few things CI is pointing out that you'll want to fix:
> 
> from the tox output:
> 
> ```
> pre_commit_hooks/check_json.py      29      1      6      1    94%   15->16, 16
> ```
> 
> this indicates that the code on line 16 is not covered by an automated test -- there's a set of tests for this module in tests/check_json_test.py -- you can probably find some examples there to fix it
> 
> from the pre-commit output:
> 
> ```
> pre_commit_hooks/check_json.py:12: error: Missing type parameters for generic type "Dict"
> ```
> 
> this indicates that the `Dict` generic does not have its type parameters -- usually these are `Dict[str, Any]` when loading from json. I'm also pretty sure that `Hashable` => `str` in the function signature should be changed too? not 100% sure on that though I'd have to clone and check with `pre-commit run mypy --all-files`

Thank you for the help!
Covered line no. 16 in automated tests. Will update fix for another issue. 
AdityaKhursale: Hi @asottile 

I have fixed the things reported by CI.
Can you please review and check if PR is good to merge?

Thanks,
Aditya

Patch:
diff --git a/pre_commit_hooks/check_json.py b/pre_commit_hooks/check_json.py
index 6026270..db589d0 100644
--- a/pre_commit_hooks/check_json.py
+++ b/pre_commit_hooks/check_json.py
@@ -1,7 +1,23 @@
 import argparse
 import json
+from typing import Any
+from typing import Dict
+from typing import List
 from typing import Optional
 from typing import Sequence
+from typing import Tuple
+
+
+def raise_duplicate_keys(
+        ordered_pairs: List[Tuple[str, Any]],
+) -> Dict[str, Any]:
+    d = {}
+    for key, val in ordered_pairs:
+        if key in d:
+            raise ValueError(f'Duplicate key: {key}')
+        else:
+            d[key] = val
+    return d
 
 
 def main(argv: Optional[Sequence[str]] = None) -> int:
@@ -13,7 +29,7 @@ def main(argv: Optional[Sequence[str]] = None) -> int:
     for filename in args.filenames:
         with open(filename, 'rb') as f:
             try:
-                json.load(f)
+                json.load(f, object_pairs_hook=raise_duplicate_keys)
             except ValueError as exc:
                 print(f'{filename}: Failed to json decode ({exc})')
                 retval = 1


Test patch:
diff --git a/testing/resources/duplicate_key_json.json b/testing/resources/duplicate_key_json.json
new file mode 100644
index 0000000..8a43262
--- /dev/null
+++ b/testing/resources/duplicate_key_json.json
@@ -0,0 +1,4 @@
+{
+    "hello": "world",
+    "hello": "planet"
+}
diff --git a/tests/check_json_test.py b/tests/check_json_test.py
index c63dc4c..e010faa 100644
--- a/tests/check_json_test.py
+++ b/tests/check_json_test.py
@@ -9,6 +9,7 @@ from testing.util import get_resource_path
         ('bad_json.notjson', 1),
         ('bad_json_latin1.nonjson', 1),
         ('ok_json.json', 0),
+        ('duplicate_key_json.json', 1),
     ),
 )
 def test_main(capsys, filename, expected_retval):