Repository: pre-commit/pre-commit-hooks
Pull request #254 (2017-12-10 09:38:18)

Refactor/pretty format json
Two commits to fix pretty_format_json:


**Remove pretty_format_json simplejson dependency**

 * The simplejson module is only needed for <=py25 so replace with builtin json.
 * Replace six dependecy for simple Py2 check for convertion to unicode.
 * Cleanup quotes.

----
**Fix pretty_format_json to use int indent**

The indent parameter for json should be integer and under Python2 is
will raise an error if not. So switch from str to int and mention
default value in help text.

cas--: btw I thought I had run the tests before creating the PR, sorry about that!

Patch:
diff --git a/README.md b/README.md
index 12c86fe..3961204 100644
--- a/README.md
+++ b/README.md
@@ -34,7 +34,6 @@ Add this to your `.pre-commit-config.yaml`
 - `check-ast` - Simply check whether files parse as valid python.
 - `check-builtin-literals` - Require literal syntax when initializing empty or zero Python builtin types.
     - Allows calling constructors with positional arguments (e.g., `list('abc')`).
-    - Allows calling constructors from the `builtins` (`__builtin__`) namespace (`builtins.list()`).
     - Ignore this requirement for specific builtin types with `--ignore=type1,type2,…`.
     - Forbid `dict` keyword syntax with `--no-allow-dict-kwargs`.
 - `check-byte-order-marker` - Forbid files which have a UTF-8 byte-order marker
diff --git a/pre_commit_hooks/check_builtin_literals.py b/pre_commit_hooks/check_builtin_literals.py
index c4ac969..1213288 100644
--- a/pre_commit_hooks/check_builtin_literals.py
+++ b/pre_commit_hooks/check_builtin_literals.py
@@ -30,11 +30,6 @@ class BuiltinTypeVisitor(ast.NodeVisitor):
         return self.allow_dict_kwargs and (getattr(node, 'kwargs', None) or getattr(node, 'keywords', None))
 
     def visit_Call(self, node):
-        if isinstance(node.func, ast.Attribute):
-            # Ignore functions that are object attributes (`foo.bar()`).
-            # Assume that if the user calls `builtins.list()`, they know what
-            # they're doing.
-            return
         if node.func.id not in set(BUILTIN_TYPES).difference(self.ignore):
             return
         if node.func.id == 'dict' and self._check_dict_call(node):
diff --git a/pre_commit_hooks/pretty_format_json.py b/pre_commit_hooks/pretty_format_json.py
index bb7a3d0..fb1c487 100644
--- a/pre_commit_hooks/pretty_format_json.py
+++ b/pre_commit_hooks/pretty_format_json.py
@@ -2,11 +2,11 @@ from __future__ import print_function
 
 import argparse
 import io
+import json
 import sys
 from collections import OrderedDict
 
-import simplejson
-import six
+from six import text_type
 
 
 def _get_pretty_format(contents, indent, ensure_ascii=True, sort_keys=True, top_keys=[]):
@@ -17,40 +17,28 @@ def _get_pretty_format(contents, indent, ensure_ascii=True, sort_keys=True, top_
         if sort_keys:
             after = sorted(after, key=lambda x: x[0])
         return OrderedDict(before + after)
-    return six.text_type(simplejson.dumps(
-        simplejson.loads(
-            contents,
-            object_pairs_hook=pairs_first,
-        ),
+    json_pretty = json.dumps(
+        json.loads(contents, object_pairs_hook=pairs_first),
         indent=indent,
         ensure_ascii=ensure_ascii,
-    )) + "\n"  # dumps does not end with a newline
+        separators=(',', ': '),  # Workaround for https://bugs.python.org/issue16333
+    )
+    # Ensure unicode (Py2) and add the newline that dumps does not end with.
+    return text_type(json_pretty) + '\n'
 
 
 def _autofix(filename, new_contents):
-    print("Fixing file {}".format(filename))
+    print('Fixing file {}'.format(filename))
     with io.open(filename, 'w', encoding='UTF-8') as f:
         f.write(new_contents)
 
 
-def parse_indent(s):
-    # type: (str) -> str
+def parse_num_to_int(s):
+    """Convert string numbers to int, leaving strings as is."""
     try:
-        int_indentation_spec = int(s)
+        return int(s)
     except ValueError:
-        if not s.strip():
-            return s
-        else:
-            raise ValueError(
-                'Non-whitespace JSON indentation delimiter supplied. ',
-            )
-    else:
-        if int_indentation_spec >= 0:
-            return int_indentation_spec * ' '
-        else:
-            raise ValueError(
-                'Negative integer supplied to construct JSON indentation delimiter. ',
-            )
+        return s
 
 
 def parse_topkeys(s):
@@ -68,9 +56,12 @@ def pretty_format_json(argv=None):
     )
     parser.add_argument(
         '--indent',
-        type=parse_indent,
-        default='  ',
-        help='String used as delimiter for one indentation level',
+        type=parse_num_to_int,
+        default='2',
+        help=(
+            'The number of indent spaces or a string to be used as delimiter'
+            ' for indentation level e.g. 4 or "\t" (Default: 2)'
+        ),
     )
     parser.add_argument(
         '--no-ensure-ascii',
@@ -110,16 +101,15 @@ def pretty_format_json(argv=None):
             )
 
             if contents != pretty_contents:
-                print("File {} is not pretty-formatted".format(json_file))
+                print('File {} is not pretty-formatted'.format(json_file))
 
                 if args.autofix:
                     _autofix(json_file, pretty_contents)
 
                 status = 1
-
-        except simplejson.JSONDecodeError:
+        except ValueError:
             print(
-                "Input File {} is not a valid JSON, consider using check-json"
+                'Input File {} is not a valid JSON, consider using check-json'
                 .format(json_file),
             )
             return 1
diff --git a/setup.py b/setup.py
index ce8de14..8f9550d 100644
--- a/setup.py
+++ b/setup.py
@@ -28,7 +28,6 @@ setup(
         'flake8!=2.5.3',
         'autopep8>=1.3',
         'pyyaml',
-        'simplejson',
         'six',
     ],
     entry_points={


Test patch:
diff --git a/testing/resources/builtin_constructors.py b/testing/resources/builtin_constructors.py
index 174a9e8..3fab056 100644
--- a/testing/resources/builtin_constructors.py
+++ b/testing/resources/builtin_constructors.py
@@ -1,5 +1,3 @@
-from six.moves import builtins
-
 c1 = complex()
 d1 = dict()
 f1 = float()
@@ -7,11 +5,3 @@ i1 = int()
 l1 = list()
 s1 = str()
 t1 = tuple()
-
-c2 = builtins.complex()
-d2 = builtins.dict()
-f2 = builtins.float()
-i2 = builtins.int()
-l2 = builtins.list()
-s2 = builtins.str()
-t2 = builtins.tuple()
diff --git a/tests/check_builtin_literals_test.py b/tests/check_builtin_literals_test.py
index 13f896a..a38e522 100644
--- a/tests/check_builtin_literals_test.py
+++ b/tests/check_builtin_literals_test.py
@@ -21,36 +21,31 @@ def visitor():
         ("complex()", [BuiltinTypeCall('complex', 1, 0)]),
         ("complex(0, 0)", []),
         ("complex('0+0j')", []),
-        ('builtins.complex()', []),
         # float
         ("0.0", []),
         ("float()", [BuiltinTypeCall('float', 1, 0)]),
         ("float('0.0')", []),
-        ('builtins.float()', []),
         # int
         ("0", []),
         ("int()", [BuiltinTypeCall('int', 1, 0)]),
         ("int('0')", []),
-        ('builtins.int()', []),
         # list
         ("[]", []),
         ("list()", [BuiltinTypeCall('list', 1, 0)]),
         ("list('abc')", []),
         ("list([c for c in 'abc'])", []),
         ("list(c for c in 'abc')", []),
-        ('builtins.list()', []),
         # str
         ("''", []),
         ("str()", [BuiltinTypeCall('str', 1, 0)]),
         ("str('0')", []),
-        ('builtins.str()', []),
+        ("[]", []),
         # tuple
         ("()", []),
         ("tuple()", [BuiltinTypeCall('tuple', 1, 0)]),
         ("tuple('abc')", []),
         ("tuple([c for c in 'abc'])", []),
         ("tuple(c for c in 'abc')", []),
-        ('builtins.tuple()', []),
     ],
 )
 def test_non_dict_exprs(visitor, expression, calls):
@@ -67,7 +62,6 @@ def test_non_dict_exprs(visitor, expression, calls):
         ("dict(**{'a': 1, 'b': 2, 'c': 3})", []),
         ("dict([(k, v) for k, v in [('a', 1), ('b', 2), ('c', 3)]])", []),
         ("dict((k, v) for k, v in [('a', 1), ('b', 2), ('c', 3)])", []),
-        ('builtins.dict()', []),
     ],
 )
 def test_dict_allow_kwargs_exprs(visitor, expression, calls):
@@ -81,7 +75,6 @@ def test_dict_allow_kwargs_exprs(visitor, expression, calls):
         ("dict()", [BuiltinTypeCall('dict', 1, 0)]),
         ("dict(a=1, b=2, c=3)", [BuiltinTypeCall('dict', 1, 0)]),
         ("dict(**{'a': 1, 'b': 2, 'c': 3})", [BuiltinTypeCall('dict', 1, 0)]),
-        ('builtins.dict()', []),
     ],
 )
 def test_dict_no_allow_kwargs_exprs(expression, calls):
diff --git a/tests/pretty_format_json_test.py b/tests/pretty_format_json_test.py
index eeef65b..7ce7e16 100644
--- a/tests/pretty_format_json_test.py
+++ b/tests/pretty_format_json_test.py
@@ -1,20 +1,18 @@
 import shutil
 
 import pytest
+from six import PY2
 
-from pre_commit_hooks.pretty_format_json import parse_indent
+from pre_commit_hooks.pretty_format_json import parse_num_to_int
 from pre_commit_hooks.pretty_format_json import pretty_format_json
 from testing.util import get_resource_path
 
 
-def test_parse_indent():
-    assert parse_indent('0') == ''
-    assert parse_indent('2') == '  '
-    assert parse_indent('\t') == '\t'
-    with pytest.raises(ValueError):
-        parse_indent('a')
-    with pytest.raises(ValueError):
-        parse_indent('-2')
+def test_parse_num_to_int():
+    assert parse_num_to_int('0') == 0
+    assert parse_num_to_int('2') == 2
+    assert parse_num_to_int('\t') == '\t'
+    assert parse_num_to_int('  ') == '  '
 
 
 @pytest.mark.parametrize(
@@ -43,6 +41,7 @@ def test_unsorted_pretty_format_json(filename, expected_retval):
     assert ret == expected_retval
 
 
+@pytest.mark.skipif(PY2, reason="Requires Python3")
 @pytest.mark.parametrize(
     ('filename', 'expected_retval'), (
         ('not_pretty_formatted_json.json', 1),
@@ -52,7 +51,7 @@ def test_unsorted_pretty_format_json(filename, expected_retval):
         ('tab_pretty_formatted_json.json', 0),
     ),
 )
-def test_tab_pretty_format_json(filename, expected_retval):
+def test_tab_pretty_format_json(filename, expected_retval):  # pragma: no cover
     ret = pretty_format_json(['--indent', '\t', get_resource_path(filename)])
     assert ret == expected_retval