Skip to content

Commit 152ca18

Browse files
geokalajayvdb
geokala
authored and
jayvdb
committed
Check for duplicate dictionary keys (#72)
1 parent 2ab47d7 commit 152ca18

File tree

3 files changed

+333
-1
lines changed

3 files changed

+333
-1
lines changed

‎pyflakes/checker.py

Lines changed: 100 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,17 @@ def __missing__(self, node_class):
8181
return fields
8282

8383

84+
def counter(items):
85+
"""
86+
Simplest required implementation of collections.Counter. Required as 2.6
87+
does not have Counter in collections.
88+
"""
89+
results = {}
90+
for item in items:
91+
results[item] = results.get(item, 0) + 1
92+
return results
93+
94+
8495
def iter_child_nodes(node, omit=None, _fields_order=_FieldsOrder()):
8596
"""
8697
Yield all direct child nodes of *node*, that is, all fields that
@@ -97,6 +108,33 @@ def iter_child_nodes(node, omit=None, _fields_order=_FieldsOrder()):
97108
yield item
98109

99110

111+
def convert_to_value(item):
112+
if isinstance(item, ast.Str):
113+
return item.s
114+
elif hasattr(ast, 'Bytes') and isinstance(item, ast.Bytes):
115+
return item.s
116+
elif isinstance(item, ast.Tuple):
117+
return tuple(convert_to_value(i) for i in item.elts)
118+
elif isinstance(item, ast.Num):
119+
return item.n
120+
elif isinstance(item, ast.Name):
121+
result = VariableKey(item=item)
122+
constants_lookup = {
123+
'True': True,
124+
'False': False,
125+
'None': None,
126+
}
127+
return constants_lookup.get(
128+
result.name,
129+
result,
130+
)
131+
elif (not PY33) and isinstance(item, ast.NameConstant):
132+
# None, True, False are nameconstants in python3, but names in 2
133+
return item.value
134+
else:
135+
return UnhandledKeyType()
136+
137+
100138
class Binding(object):
101139
"""
102140
Represents the binding of a value to a name.
@@ -133,6 +171,31 @@ class Definition(Binding):
133171
"""
134172

135173

174+
class UnhandledKeyType(object):
175+
"""
176+
A dictionary key of a type that we cannot or do not check for duplicates.
177+
"""
178+
179+
180+
class VariableKey(object):
181+
"""
182+
A dictionary key which is a variable.
183+
184+
@ivar item: The variable AST object.
185+
"""
186+
def __init__(self, item):
187+
self.name = item.id
188+
189+
def __eq__(self, compare):
190+
return (
191+
compare.__class__ == self.__class__
192+
and compare.name == self.name
193+
)
194+
195+
def __hash__(self):
196+
return hash(self.name)
197+
198+
136199
class Importation(Definition):
137200
"""
138201
A binding created by an import statement.
@@ -855,7 +918,7 @@ def ignore(self, node):
855918
PASS = ignore
856919

857920
# "expr" type nodes
858-
BOOLOP = BINOP = UNARYOP = IFEXP = DICT = SET = \
921+
BOOLOP = BINOP = UNARYOP = IFEXP = SET = \
859922
COMPARE = CALL = REPR = ATTRIBUTE = SUBSCRIPT = \
860923
STARRED = NAMECONSTANT = handleChildren
861924

@@ -876,6 +939,42 @@ def ignore(self, node):
876939
# additional node types
877940
COMPREHENSION = KEYWORD = FORMATTEDVALUE = handleChildren
878941

942+
def DICT(self, node):
943+
# Complain if there are duplicate keys with different values
944+
# If they have the same value it's not going to cause potentially
945+
# unexpected behaviour so we'll not complain.
946+
keys = [
947+
convert_to_value(key) for key in node.keys
948+
]
949+
950+
key_counts = counter(keys)
951+
duplicate_keys = [
952+
key for key, count in key_counts.items()
953+
if count > 1
954+
]
955+
956+
for key in duplicate_keys:
957+
key_indices = [i for i, i_key in enumerate(keys) if i_key == key]
958+
959+
values = counter(
960+
convert_to_value(node.values[index])
961+
for index in key_indices
962+
)
963+
if any(count == 1 for value, count in values.items()):
964+
for key_index in key_indices:
965+
key_node = node.keys[key_index]
966+
if isinstance(key, VariableKey):
967+
self.report(messages.MultiValueRepeatedKeyVariable,
968+
key_node,
969+
key.name)
970+
else:
971+
self.report(
972+
messages.MultiValueRepeatedKeyLiteral,
973+
key_node,
974+
key,
975+
)
976+
self.handleChildren(node)
977+
879978
def ASSERT(self, node):
880979
if isinstance(node.test, ast.Tuple) and node.test.elts != []:
881980
self.report(messages.AssertTuple, node)

‎pyflakes/messages.py

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,22 @@ def __init__(self, filename, loc, name):
116116
self.message_args = (name,)
117117

118118

119+
class MultiValueRepeatedKeyLiteral(Message):
120+
message = 'dictionary key %r repeated with different values'
121+
122+
def __init__(self, filename, loc, key):
123+
Message.__init__(self, filename, loc)
124+
self.message_args = (key,)
125+
126+
127+
class MultiValueRepeatedKeyVariable(Message):
128+
message = 'dictionary key variable %s repeated with different values'
129+
130+
def __init__(self, filename, loc, key):
131+
Message.__init__(self, filename, loc)
132+
self.message_args = (key,)
133+
134+
119135
class LateFutureImport(Message):
120136
message = 'from __future__ imports must occur at the beginning of the file'
121137

‎pyflakes/test/test_dict.py

Lines changed: 217 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,217 @@
1+
"""
2+
Tests for dict duplicate keys Pyflakes behavior.
3+
"""
4+
5+
from sys import version_info
6+
7+
from pyflakes import messages as m
8+
from pyflakes.test.harness import TestCase, skipIf
9+
10+
11+
class Test(TestCase):
12+
13+
def test_duplicate_keys(self):
14+
self.flakes(
15+
"{'yes': 1, 'yes': 2}",
16+
m.MultiValueRepeatedKeyLiteral,
17+
m.MultiValueRepeatedKeyLiteral,
18+
)
19+
20+
@skipIf(version_info < (3,),
21+
"bytes and strings with same 'value' are not equal in python3")
22+
@skipIf(version_info[0:2] == (3, 2),
23+
"python3.2 does not allow u"" literal string definition")
24+
def test_duplicate_keys_bytes_vs_unicode_py3(self):
25+
self.flakes("{b'a': 1, u'a': 2}")
26+
27+
@skipIf(version_info < (3,),
28+
"bytes and strings with same 'value' are not equal in python3")
29+
@skipIf(version_info[0:2] == (3, 2),
30+
"python3.2 does not allow u"" literal string definition")
31+
def test_duplicate_values_bytes_vs_unicode_py3(self):
32+
self.flakes(
33+
"{1: b'a', 1: u'a'}",
34+
m.MultiValueRepeatedKeyLiteral,
35+
m.MultiValueRepeatedKeyLiteral,
36+
)
37+
38+
@skipIf(version_info >= (3,),
39+
"bytes and strings with same 'value' are equal in python2")
40+
def test_duplicate_keys_bytes_vs_unicode_py2(self):
41+
self.flakes(
42+
"{b'a': 1, u'a': 2}",
43+
m.MultiValueRepeatedKeyLiteral,
44+
m.MultiValueRepeatedKeyLiteral,
45+
)
46+
47+
@skipIf(version_info >= (3,),
48+
"bytes and strings with same 'value' are equal in python2")
49+
def test_duplicate_values_bytes_vs_unicode_py2(self):
50+
self.flakes("{1: b'a', 1: u'a'}")
51+
52+
def test_multiple_duplicate_keys(self):
53+
self.flakes(
54+
"{'yes': 1, 'yes': 2, 'no': 2, 'no': 3}",
55+
m.MultiValueRepeatedKeyLiteral,
56+
m.MultiValueRepeatedKeyLiteral,
57+
m.MultiValueRepeatedKeyLiteral,
58+
m.MultiValueRepeatedKeyLiteral,
59+
)
60+
61+
def test_duplicate_keys_in_function(self):
62+
self.flakes(
63+
'''
64+
def f(thing):
65+
pass
66+
f({'yes': 1, 'yes': 2})
67+
''',
68+
m.MultiValueRepeatedKeyLiteral,
69+
m.MultiValueRepeatedKeyLiteral,
70+
)
71+
72+
def test_duplicate_keys_in_lambda(self):
73+
self.flakes(
74+
"lambda x: {(0,1): 1, (0,1): 2}",
75+
m.MultiValueRepeatedKeyLiteral,
76+
m.MultiValueRepeatedKeyLiteral,
77+
)
78+
79+
def test_duplicate_keys_tuples(self):
80+
self.flakes(
81+
"{(0,1): 1, (0,1): 2}",
82+
m.MultiValueRepeatedKeyLiteral,
83+
m.MultiValueRepeatedKeyLiteral,
84+
)
85+
86+
def test_duplicate_keys_tuples_int_and_float(self):
87+
self.flakes(
88+
"{(0,1): 1, (0,1.0): 2}",
89+
m.MultiValueRepeatedKeyLiteral,
90+
m.MultiValueRepeatedKeyLiteral,
91+
)
92+
93+
def test_duplicate_keys_ints(self):
94+
self.flakes(
95+
"{1: 1, 1: 2}",
96+
m.MultiValueRepeatedKeyLiteral,
97+
m.MultiValueRepeatedKeyLiteral,
98+
)
99+
100+
def test_duplicate_keys_bools(self):
101+
self.flakes(
102+
"{True: 1, True: 2}",
103+
m.MultiValueRepeatedKeyLiteral,
104+
m.MultiValueRepeatedKeyLiteral,
105+
)
106+
107+
def test_duplicate_keys_bools_false(self):
108+
# Needed to ensure 2.x correctly coerces these from variables
109+
self.flakes(
110+
"{False: 1, False: 2}",
111+
m.MultiValueRepeatedKeyLiteral,
112+
m.MultiValueRepeatedKeyLiteral,
113+
)
114+
115+
def test_duplicate_keys_none(self):
116+
self.flakes(
117+
"{None: 1, None: 2}",
118+
m.MultiValueRepeatedKeyLiteral,
119+
m.MultiValueRepeatedKeyLiteral,
120+
)
121+
122+
def test_duplicate_variable_keys(self):
123+
self.flakes(
124+
'''
125+
a = 1
126+
{a: 1, a: 2}
127+
''',
128+
m.MultiValueRepeatedKeyVariable,
129+
m.MultiValueRepeatedKeyVariable,
130+
)
131+
132+
def test_duplicate_variable_values(self):
133+
self.flakes(
134+
'''
135+
a = 1
136+
b = 2
137+
{1: a, 1: b}
138+
''',
139+
m.MultiValueRepeatedKeyLiteral,
140+
m.MultiValueRepeatedKeyLiteral,
141+
)
142+
143+
def test_duplicate_variable_values_same_value(self):
144+
# Current behaviour is not to look up variable values. This is to
145+
# confirm that.
146+
self.flakes(
147+
'''
148+
a = 1
149+
b = 1
150+
{1: a, 1: b}
151+
''',
152+
m.MultiValueRepeatedKeyLiteral,
153+
m.MultiValueRepeatedKeyLiteral,
154+
)
155+
156+
def test_duplicate_key_float_and_int(self):
157+
"""
158+
These do look like different values, but when it comes to their use as
159+
keys, they compare as equal and so are actually duplicates.
160+
The literal dict {1: 1, 1.0: 1} actually becomes {1.0: 1}.
161+
"""
162+
self.flakes(
163+
'''
164+
{1: 1, 1.0: 2}
165+
''',
166+
m.MultiValueRepeatedKeyLiteral,
167+
m.MultiValueRepeatedKeyLiteral,
168+
)
169+
170+
def test_no_duplicate_key_error_same_value(self):
171+
self.flakes('''
172+
{'yes': 1, 'yes': 1}
173+
''')
174+
175+
def test_no_duplicate_key_errors(self):
176+
self.flakes('''
177+
{'yes': 1, 'no': 2}
178+
''')
179+
180+
def test_no_duplicate_keys_tuples_same_first_element(self):
181+
self.flakes("{(0,1): 1, (0,2): 1}")
182+
183+
def test_no_duplicate_key_errors_func_call(self):
184+
self.flakes('''
185+
def test(thing):
186+
pass
187+
test({True: 1, None: 2, False: 1})
188+
''')
189+
190+
def test_no_duplicate_key_errors_bool_or_none(self):
191+
self.flakes("{True: 1, None: 2, False: 1}")
192+
193+
def test_no_duplicate_key_errors_ints(self):
194+
self.flakes('''
195+
{1: 1, 2: 1}
196+
''')
197+
198+
def test_no_duplicate_key_errors_vars(self):
199+
self.flakes('''
200+
test = 'yes'
201+
rest = 'yes'
202+
{test: 1, rest: 2}
203+
''')
204+
205+
def test_no_duplicate_key_errors_tuples(self):
206+
self.flakes('''
207+
{(0,1): 1, (0,2): 1}
208+
''')
209+
210+
def test_no_duplicate_key_errors_instance_attributes(self):
211+
self.flakes('''
212+
class Test():
213+
pass
214+
f = Test()
215+
f.a = 1
216+
{f.a: 1, f.a: 1}
217+
''')

0 commit comments

Comments
 (0)