Skip to content

Commit 401b012

Browse files
author
Madda
committed
Check for duplicate dictionary keys
1 parent d721eaf commit 401b012

File tree

3 files changed

+329
-1
lines changed

3 files changed

+329
-1
lines changed

‎pyflakes/checker.py‎

Lines changed: 96 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,33 @@ def iter_child_nodes(node, omit=None, _fields_order=_FieldsOrder()):
9191
yield item
9292

9393

94+
def convert_to_value(item):
95+
if isinstance(item, ast.Str):
96+
return item.s
97+
elif hasattr(ast, 'Bytes') and isinstance(item, ast.Bytes):
98+
return item.s
99+
elif isinstance(item, ast.Tuple):
100+
return tuple(convert_to_value(i) for i in item.elts)
101+
elif isinstance(item, ast.Num):
102+
return item.n
103+
elif isinstance(item, ast.Name):
104+
result = VariableKey(item=item)
105+
constants_lookup = {
106+
'True': True,
107+
'False': False,
108+
'None': None,
109+
}
110+
return constants_lookup.get(
111+
result.name,
112+
result,
113+
)
114+
elif (not PY33) and isinstance(item, ast.NameConstant):
115+
# None, True, False are nameconstants in python3, but names in 2
116+
return item.value
117+
else:
118+
return UnhandledKeyType()
119+
120+
94121
class Binding(object):
95122
"""
96123
Represents the binding of a value to a name.
@@ -127,6 +154,31 @@ class Definition(Binding):
127154
"""
128155

129156

157+
class UnhandledKeyType(object):
158+
"""
159+
A dictionary key of a type that we cannot or do not check for duplicates.
160+
"""
161+
162+
163+
class VariableKey(object):
164+
"""
165+
A dictionary key which is a variable.
166+
167+
@ivar item: The variable AST object.
168+
"""
169+
def __init__(self, item):
170+
self.name = item.id
171+
172+
def __eq__(self, compare):
173+
return (
174+
compare.__class__ == self.__class__
175+
and compare.name == self.name
176+
)
177+
178+
def __hash__(self):
179+
return hash(self.name)
180+
181+
130182
class Importation(Definition):
131183
"""
132184
A binding created by an import statement.
@@ -849,7 +901,7 @@ def ignore(self, node):
849901
PASS = ignore
850902

851903
# "expr" type nodes
852-
BOOLOP = BINOP = UNARYOP = IFEXP = DICT = SET = \
904+
BOOLOP = BINOP = UNARYOP = IFEXP = SET = \
853905
COMPARE = CALL = REPR = ATTRIBUTE = SUBSCRIPT = \
854906
STARRED = NAMECONSTANT = handleChildren
855907

@@ -870,6 +922,49 @@ def ignore(self, node):
870922
# additional node types
871923
COMPREHENSION = KEYWORD = FORMATTEDVALUE = handleChildren
872924

925+
def DICT(self, node):
926+
# Complain if there are duplicate keys with different values
927+
# If they have the same value it's not going to cause potentially
928+
# unexpected behaviour so we'll not complain.
929+
keys = [
930+
convert_to_value(key) for key in node.keys
931+
]
932+
for key in set(keys):
933+
if keys.count(key) <= 1:
934+
continue
935+
candidate_values = []
936+
for candidate in range(len(keys)):
937+
if key == keys[candidate]:
938+
candidate_values.append(
939+
convert_to_value(node.values[candidate])
940+
)
941+
942+
# Determine whether we have different values for the repeats
943+
differing_values = False
944+
for value in set(candidate_values):
945+
if candidate_values.count(value) == 1:
946+
differing_values = True
947+
break
948+
949+
if differing_values:
950+
key_indices = []
951+
for index in range(len(keys)):
952+
if keys[index] == key:
953+
key_indices.append(index)
954+
for key_index in key_indices:
955+
key_node = node.keys[key_index]
956+
if isinstance(key, VariableKey):
957+
self.report(messages.MultiValueRepeatedKeyVariable,
958+
key_node,
959+
key.name)
960+
else:
961+
self.report(
962+
messages.MultiValueRepeatedKeyLiteral,
963+
key_node,
964+
key,
965+
)
966+
self.handleChildren(node)
967+
873968
def ASSERT(self, node):
874969
if isinstance(node.test, ast.Tuple) and node.test.elts != []:
875970
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)