Skip to content

Commit 35996b7

Browse files
committed
Add check to enforce literal syntax for Python builtin types
This check requires authors to initialize empty or zero builtin types using the literal syntax (e.g., `{}` instead of `dict()`). Authors may ignore this requirement for certain builtins using the `--ignore` option. Authors may also forbid calling `dict()` with keyword arguments (`dict(a=1, b=2)`) using the `--no-allow-dict-kwargs` flag.
1 parent e718847 commit 35996b7

File tree

8 files changed

+231
-0
lines changed

8 files changed

+231
-0
lines changed

.pre-commit-hooks.yaml

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,15 @@
3434
# for backward compatibility
3535
files: ''
3636
minimum_pre_commit_version: 0.15.0
37+
- id: check-builtin-literals
38+
name: Check builtin type constructor use
39+
description: Require literal syntax when initializing empty or zero Python builtin types.
40+
entry: check-builtin-literals
41+
language: python
42+
types: [python]
43+
# for backward compatibility
44+
files: ''
45+
minimum_pre_commit_version: 0.15.0
3746
- id: check-case-conflict
3847
name: Check for case conflicts
3948
description: Check for files that would conflict in case-insensitive filesystems

README.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,10 @@ Add this to your `.pre-commit-config.yaml`
3030
- `check-added-large-files` - Prevent giant files from being committed.
3131
- Specify what is "too large" with `args: ['--maxkb=123']` (default=500kB).
3232
- `check-ast` - Simply check whether files parse as valid python.
33+
- `check-builtin-literals` - Require literal syntax when initializing empty or zero Python builtin types.
34+
- Allows calling constructors with positional arguments (e.g., `list('abc')`).
35+
- Ignore this requirement for specific builtin types with `--ignore=type1,type2,…`.
36+
- Forbid `dict` keyword syntax with `--no-allow-dict-kwargs`.
3337
- `check-byte-order-marker` - Forbid files which have a UTF-8 byte-order marker
3438
- `check-case-conflict` - Check for files with names that would conflict on a
3539
case-insensitive filesystem like MacOS HFS+ or Windows FAT.

hooks.yaml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,12 @@
1616
entry: upgrade-your-pre-commit-version
1717
files: ''
1818
minimum_pre_commit_version: 0.15.0
19+
- id: check-builtin-literals
20+
language: system
21+
name: upgrade-your-pre-commit-version
22+
entry: upgrade-your-pre-commit-version
23+
files: ''
24+
minimum_pre_commit_version: 0.15.0
1925
- id: check-byte-order-marker
2026
language: system
2127
name: upgrade-your-pre-commit-version
Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
from __future__ import unicode_literals
2+
3+
import argparse
4+
import ast
5+
import collections
6+
import sys
7+
8+
9+
BUILTIN_TYPES = {
10+
'complex': '0j',
11+
'dict': '{}',
12+
'float': '0.0',
13+
'int': '0',
14+
'list': '[]',
15+
'str': "''",
16+
'tuple': '()',
17+
}
18+
19+
20+
BuiltinTypeCall = collections.namedtuple('BuiltinTypeCall', ['name', 'line', 'column'])
21+
22+
23+
class BuiltinTypeVisitor(ast.NodeVisitor):
24+
def __init__(self, ignore=None, allow_dict_kwargs=True):
25+
self.builtin_type_calls = []
26+
self.ignore = set(ignore) if ignore else set()
27+
self.allow_dict_kwargs = allow_dict_kwargs
28+
29+
def _check_dict_call(self, node):
30+
return self.allow_dict_kwargs and (getattr(node, 'kwargs', None) or getattr(node, 'keywords', None))
31+
32+
def visit_Call(self, node):
33+
if node.func.id not in set(BUILTIN_TYPES).difference(self.ignore):
34+
return
35+
if node.func.id == 'dict' and self._check_dict_call(node):
36+
return
37+
elif node.args:
38+
return
39+
self.builtin_type_calls.append(
40+
BuiltinTypeCall(node.func.id, node.lineno, node.col_offset),
41+
)
42+
43+
44+
def check_file_for_builtin_type_constructors(filename, ignore=None, allow_dict_kwargs=True):
45+
tree = ast.parse(open(filename, 'rb').read(), filename=filename)
46+
visitor = BuiltinTypeVisitor(ignore=ignore, allow_dict_kwargs=allow_dict_kwargs)
47+
visitor.visit(tree)
48+
return visitor.builtin_type_calls
49+
50+
51+
def parse_args(argv):
52+
def parse_ignore(value):
53+
return set(value.split(','))
54+
55+
parser = argparse.ArgumentParser()
56+
parser.add_argument('filenames', nargs='*')
57+
parser.add_argument('--ignore', type=parse_ignore, default=set())
58+
59+
allow_dict_kwargs = parser.add_mutually_exclusive_group(required=False)
60+
allow_dict_kwargs.add_argument('--allow-dict-kwargs', action='store_true')
61+
allow_dict_kwargs.add_argument('--no-allow-dict-kwargs', dest='allow_dict_kwargs', action='store_false')
62+
allow_dict_kwargs.set_defaults(allow_dict_kwargs=True)
63+
64+
return parser.parse_args(argv)
65+
66+
67+
def main(argv=None):
68+
args = parse_args(argv)
69+
rc = 0
70+
for filename in args.filenames:
71+
calls = check_file_for_builtin_type_constructors(
72+
filename,
73+
ignore=args.ignore,
74+
allow_dict_kwargs=args.allow_dict_kwargs,
75+
)
76+
if calls:
77+
rc = rc or 1
78+
for call in calls:
79+
print(
80+
'{filename}:{call.line}:{call.column} - Replace {call.name}() with {replacement}'.format(
81+
filename=filename,
82+
call=call,
83+
replacement=BUILTIN_TYPES[call.name],
84+
),
85+
)
86+
return rc
87+
88+
89+
if __name__ == '__main__':
90+
sys.exit(main())

setup.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
'autopep8-wrapper = pre_commit_hooks.autopep8_wrapper:main',
3737
'check-added-large-files = pre_commit_hooks.check_added_large_files:main',
3838
'check-ast = pre_commit_hooks.check_ast:check_ast',
39+
'check-builtin-literals = pre_commit_hooks.check_builtin_literals:main',
3940
'check-byte-order-marker = pre_commit_hooks.check_byte_order_marker:main',
4041
'check-case-conflict = pre_commit_hooks.check_case_conflict:main',
4142
'check-docstring-first = pre_commit_hooks.check_docstring_first:main',
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
c1 = complex()
2+
d1 = dict()
3+
f1 = float()
4+
i1 = int()
5+
l1 = list()
6+
s1 = str()
7+
t1 = tuple()
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
c1 = 0j
2+
d1 = {}
3+
f1 = 0.0
4+
i1 = 0
5+
l1 = []
6+
s1 = ''
7+
t1 = ()
Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
import ast
2+
3+
import pytest
4+
5+
from pre_commit_hooks.check_builtin_literals import BuiltinTypeCall
6+
from pre_commit_hooks.check_builtin_literals import BuiltinTypeVisitor
7+
from pre_commit_hooks.check_builtin_literals import main
8+
from testing.util import get_resource_path
9+
10+
11+
@pytest.fixture
12+
def visitor():
13+
return BuiltinTypeVisitor()
14+
15+
16+
@pytest.mark.parametrize(
17+
('expression', 'calls'),
18+
[
19+
# complex
20+
("0j", []),
21+
("complex()", [BuiltinTypeCall('complex', 1, 0)]),
22+
("complex(0, 0)", []),
23+
("complex('0+0j')", []),
24+
# float
25+
("0.0", []),
26+
("float()", [BuiltinTypeCall('float', 1, 0)]),
27+
("float('0.0')", []),
28+
# int
29+
("0", []),
30+
("int()", [BuiltinTypeCall('int', 1, 0)]),
31+
("int('0')", []),
32+
# list
33+
("[]", []),
34+
("list()", [BuiltinTypeCall('list', 1, 0)]),
35+
("list('abc')", []),
36+
("list([c for c in 'abc'])", []),
37+
("list(c for c in 'abc')", []),
38+
# str
39+
("''", []),
40+
("str()", [BuiltinTypeCall('str', 1, 0)]),
41+
("str('0')", []),
42+
("[]", []),
43+
# tuple
44+
("()", []),
45+
("tuple()", [BuiltinTypeCall('tuple', 1, 0)]),
46+
("tuple('abc')", []),
47+
("tuple([c for c in 'abc'])", []),
48+
("tuple(c for c in 'abc')", []),
49+
],
50+
)
51+
def test_non_dict_exprs(visitor, expression, calls):
52+
visitor.visit(ast.parse(expression))
53+
assert visitor.builtin_type_calls == calls
54+
55+
56+
@pytest.mark.parametrize(
57+
('expression', 'calls'),
58+
[
59+
("{}", []),
60+
("dict()", [BuiltinTypeCall('dict', 1, 0)]),
61+
("dict(a=1, b=2, c=3)", []),
62+
("dict(**{'a': 1, 'b': 2, 'c': 3})", []),
63+
("dict([(k, v) for k, v in [('a', 1), ('b', 2), ('c', 3)]])", []),
64+
("dict((k, v) for k, v in [('a', 1), ('b', 2), ('c', 3)])", []),
65+
],
66+
)
67+
def test_dict_allow_kwargs_exprs(visitor, expression, calls):
68+
visitor.visit(ast.parse(expression))
69+
assert visitor.builtin_type_calls == calls
70+
71+
72+
@pytest.mark.parametrize(
73+
('expression', 'calls'),
74+
[
75+
("dict()", [BuiltinTypeCall('dict', 1, 0)]),
76+
("dict(a=1, b=2, c=3)", [BuiltinTypeCall('dict', 1, 0)]),
77+
("dict(**{'a': 1, 'b': 2, 'c': 3})", [BuiltinTypeCall('dict', 1, 0)]),
78+
],
79+
)
80+
def test_dict_no_allow_kwargs_exprs(expression, calls):
81+
visitor = BuiltinTypeVisitor(allow_dict_kwargs=False)
82+
visitor.visit(ast.parse(expression))
83+
assert visitor.builtin_type_calls == calls
84+
85+
86+
def test_ignore_constructors():
87+
visitor = BuiltinTypeVisitor(ignore=('complex', 'dict', 'float', 'int', 'list', 'str', 'tuple'))
88+
visitor.visit(ast.parse(open(get_resource_path('builtin_constructors.py'), 'rb').read(), 'builtin_constructors.py'))
89+
assert visitor.builtin_type_calls == []
90+
91+
92+
def test_failing_file():
93+
rc = main([get_resource_path('builtin_constructors.py')])
94+
assert rc == 1
95+
96+
97+
def test_passing_file():
98+
rc = main([get_resource_path('builtin_literals.py')])
99+
assert rc == 0
100+
101+
102+
def test_failing_file_ignore_all():
103+
rc = main([
104+
'--ignore=complex,dict,float,int,list,str,tuple',
105+
get_resource_path('builtin_constructors.py'),
106+
])
107+
assert rc == 0

0 commit comments

Comments
 (0)