From 2cfe2fc5cbcfed6c4a5ead4a500ef3cc6438eb87 Mon Sep 17 00:00:00 2001 From: yangdanny97 Date: Tue, 29 Oct 2024 21:45:45 -0400 Subject: [PATCH 1/3] b041 duplicate key in dictionary literal --- README.rst | 2 ++ bugbear.py | 48 ++++++++++++++++++++++++++++- tests/b041.py | 84 +++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 133 insertions(+), 1 deletion(-) create mode 100644 tests/b041.py diff --git a/README.rst b/README.rst index 54bdbc2..0d0c40f 100644 --- a/README.rst +++ b/README.rst @@ -205,6 +205,8 @@ second usage. Save the result to a list if the result is needed multiple times. **B040**: Caught exception with call to ``add_note`` not used. Did you forget to ``raise`` it? +**B041**: Repeated key in dictionary literal. The last value will override any previous values. + Opinionated warnings ~~~~~~~~~~~~~~~~~~~~ diff --git a/bugbear.py b/bugbear.py index 2519a24..21937f5 100644 --- a/bugbear.py +++ b/bugbear.py @@ -8,7 +8,7 @@ import re import sys import warnings -from collections import defaultdict, namedtuple +from collections import defaultdict, namedtuple, Counter from contextlib import suppress from functools import lru_cache, partial from keyword import iskeyword @@ -362,6 +362,17 @@ class B040CaughtException: has_note: bool +class B041UnhandledKeyType: + """ + A dictionary key of a type that we do not check for duplicates. + """ + + +@attr.define(frozen=True) +class B041VariableKeyType: + name: str + + @attr.s class BugBearVisitor(ast.NodeVisitor): filename = attr.ib() @@ -633,6 +644,34 @@ def visit_Set(self, node) -> None: self.check_for_b033(node) self.generic_visit(node) + def visit_Dict(self, node) -> None: + self.check_for_b041(node) + self.generic_visit(node) + + def check_for_b041(self, node) -> None: + # Complain if there are duplicate keys in a dictionary literal. + def convert_to_value(item): + if isinstance(item, ast.Constant): + return item.value + elif isinstance(item, ast.Tuple): + return tuple(convert_to_value(i) for i in item.elts) + elif isinstance(item, ast.Name): + return B041VariableKeyType(item.id) + else: + return B041UnhandledKeyType() + + keys = [convert_to_value(key) for key in node.keys] + key_counts = Counter(keys) + duplicate_keys = [ + key for key, count in key_counts.items() + if count > 1 + ] + for key in duplicate_keys: + key_indices = [i for i, i_key in enumerate(keys) if i_key == key] + for key_index in key_indices: + key_node = node.keys[key_index] + self.errors.append(B041(key_node.lineno, key_node.col_offset)) + def check_for_b005(self, node) -> None: if isinstance(node, ast.Import): for name in node.names: @@ -2327,6 +2366,13 @@ def visit_Lambda(self, node) -> None: message="B040 Exception with added note not used. Did you forget to raise it?" ) +B041 = Error( + message=( + "B041 Repeated key in dictionary literal. The last value will override " + "any previous values." + ) +) + # Warnings disabled by default. B901 = Error( message=( diff --git a/tests/b041.py b/tests/b041.py new file mode 100644 index 0000000..333440c --- /dev/null +++ b/tests/b041.py @@ -0,0 +1,84 @@ +# Simple case +{'yes': 1, 'yes': 2} # error + +# Duplicate keys with bytes vs unicode in Python 3 +{b'a': 1, u'a': 2} # error +{1: b'a', 1: u'a'} # error + +# Duplicate keys with bytes vs unicode in Python 2 +{b'a': 1, u'a': 2} # error + +# Duplicate values in Python 2 +{1: b'a', 1: u'a'} # error + +# Multiple duplicate keys +{'yes': 1, 'yes': 2, 'no': 2, 'no': 3} # error + +# Duplicate keys in function call +def f(thing): + pass +f({'yes': 1, 'yes': 2}) # error + +# Duplicate keys in lambda function +(lambda x: {(0, 1): 1, (0, 1): 2}) # error + +# Duplicate keys with tuples +{(0, 1): 1, (0, 1): 2} # error + +# Duplicate keys with int and float +{1: 1, 1.0: 2} # error + +# Duplicate keys with booleans +{True: 1, True: 2} # error + +# Duplicate keys with None +{None: 1, None: 2} # error + +# Duplicate keys with variables +a = 1 +{a: 1, a: 2} # error + +# Duplicate values with variables +a = 1 +b = 2 +{1: a, 1: b} # error + +# Duplicate values with same variable value +a = 1 +b = 1 +{1: a, 1: b} # error + +# Duplicate keys with same values +{'yes': 1, 'yes': 1} # error + +# Non-duplicate keys +{'yes': 1, 'no': 2} # safe + +# Non-duplicate keys with tuples having the same first element +{(0, 1): 1, (0, 2): 1} # safe + +# Non-duplicate keys in function call +def test_func(thing): + pass +test_func({True: 1, None: 2, False: 1}) # safe + +# Non-duplicate keys with bool and None +{True: 1, None: 2, False: 1} # safe + +# Non-duplicate keys with different ints +{1: 1, 2: 1} # safe + +# Duplicate keys with differently-named variables +test = 'yes' +rest = 'yes' +{test: 1, rest: 2} # safe + +# Non-duplicate tuple keys +{(0, 1): 1, (0, 2): 1} # safe + +# Duplicate keys with instance attributes +class TestClass: + pass +f = TestClass() +f.a = 1 +{f.a: 1, f.a: 1} # safe From 40bb66642ce4ac730d89c33de3ea8043c9cf31a4 Mon Sep 17 00:00:00 2001 From: yangdanny97 Date: Tue, 29 Oct 2024 22:28:41 -0400 Subject: [PATCH 2/3] only error if both keys and values are the same --- README.rst | 2 +- bugbear.py | 15 ++++--- tests/b041.py | 96 ++++++++----------------------------------- tests/test_bugbear.py | 18 ++++++++ 4 files changed, 46 insertions(+), 85 deletions(-) diff --git a/README.rst b/README.rst index 0d0c40f..188e4cd 100644 --- a/README.rst +++ b/README.rst @@ -205,7 +205,7 @@ second usage. Save the result to a list if the result is needed multiple times. **B040**: Caught exception with call to ``add_note`` not used. Did you forget to ``raise`` it? -**B041**: Repeated key in dictionary literal. The last value will override any previous values. +**B041**: Repeated key-value pair in dictionary literal. Opinionated warnings ~~~~~~~~~~~~~~~~~~~~ diff --git a/bugbear.py b/bugbear.py index 21937f5..c5ab062 100644 --- a/bugbear.py +++ b/bugbear.py @@ -649,7 +649,7 @@ def visit_Dict(self, node) -> None: self.generic_visit(node) def check_for_b041(self, node) -> None: - # Complain if there are duplicate keys in a dictionary literal. + # Complain if there are duplicate key-value pairs in a dictionary literal. def convert_to_value(item): if isinstance(item, ast.Constant): return item.value @@ -668,9 +668,13 @@ def convert_to_value(item): ] for key in duplicate_keys: key_indices = [i for i, i_key in enumerate(keys) if i_key == key] - for key_index in key_indices: - key_node = node.keys[key_index] - self.errors.append(B041(key_node.lineno, key_node.col_offset)) + seen = set() + for index in key_indices: + value = convert_to_value(node.values[index]) + if value in seen: + key_node = node.keys[index] + self.errors.append(B041(key_node.lineno, key_node.col_offset)) + seen.add(value) def check_for_b005(self, node) -> None: if isinstance(node, ast.Import): @@ -2368,8 +2372,7 @@ def visit_Lambda(self, node) -> None: B041 = Error( message=( - "B041 Repeated key in dictionary literal. The last value will override " - "any previous values." + "B041 Repeated key-value pair in dictionary literal." ) ) diff --git a/tests/b041.py b/tests/b041.py index 333440c..ae819f4 100644 --- a/tests/b041.py +++ b/tests/b041.py @@ -1,84 +1,24 @@ -# Simple case -{'yes': 1, 'yes': 2} # error - -# Duplicate keys with bytes vs unicode in Python 3 -{b'a': 1, u'a': 2} # error -{1: b'a', 1: u'a'} # error - -# Duplicate keys with bytes vs unicode in Python 2 -{b'a': 1, u'a': 2} # error - -# Duplicate values in Python 2 -{1: b'a', 1: u'a'} # error - -# Multiple duplicate keys -{'yes': 1, 'yes': 2, 'no': 2, 'no': 3} # error - -# Duplicate keys in function call -def f(thing): - pass -f({'yes': 1, 'yes': 2}) # error - -# Duplicate keys in lambda function -(lambda x: {(0, 1): 1, (0, 1): 2}) # error - -# Duplicate keys with tuples -{(0, 1): 1, (0, 1): 2} # error - -# Duplicate keys with int and float -{1: 1, 1.0: 2} # error - -# Duplicate keys with booleans -{True: 1, True: 2} # error - -# Duplicate keys with None -{None: 1, None: 2} # error - -# Duplicate keys with variables -a = 1 -{a: 1, a: 2} # error - -# Duplicate values with variables -a = 1 -b = 2 -{1: a, 1: b} # error - -# Duplicate values with same variable value a = 1 +test = {'yes': 1, 'yes': 1} +test = {'yes': 1, 'yes': 1, 'no': 2, 'no': 2} +test = {'yes': 1, 'yes': 1, 'yes': 1} +test = {1: 1, 1.0: 1} +test = {True: 1, True: 1} +test = {None: 1, None: 1} +test = {a: a, a: a} + +# no error if either keys or values are different +test = {'yes': 1, 'yes': 2} +test = {1: 1, 2: 1} +test = {(0, 1): 1, (0, 2): 1} +test = {(0, 1): 1, (0, 1): 2} b = 1 -{1: a, 1: b} # error - -# Duplicate keys with same values -{'yes': 1, 'yes': 1} # error - -# Non-duplicate keys -{'yes': 1, 'no': 2} # safe - -# Non-duplicate keys with tuples having the same first element -{(0, 1): 1, (0, 2): 1} # safe - -# Non-duplicate keys in function call -def test_func(thing): - pass -test_func({True: 1, None: 2, False: 1}) # safe - -# Non-duplicate keys with bool and None -{True: 1, None: 2, False: 1} # safe - -# Non-duplicate keys with different ints -{1: 1, 2: 1} # safe - -# Duplicate keys with differently-named variables -test = 'yes' -rest = 'yes' -{test: 1, rest: 2} # safe - -# Non-duplicate tuple keys -{(0, 1): 1, (0, 2): 1} # safe - -# Duplicate keys with instance attributes +test = {a: a, b: a} +test = {a: a, a: b} class TestClass: pass f = TestClass() f.a = 1 -{f.a: 1, f.a: 1} # safe +test = {f.a: 1, f.a: 1} + + diff --git a/tests/test_bugbear.py b/tests/test_bugbear.py index bdc2552..e702a2d 100644 --- a/tests/test_bugbear.py +++ b/tests/test_bugbear.py @@ -48,6 +48,7 @@ B037, B039, B040, + B041, B901, B902, B903, @@ -666,6 +667,23 @@ def test_b040(self) -> None: ) self.assertEqual(errors, expected) + def test_b041(self) -> None: + filename = Path(__file__).absolute().parent / "b041.py" + bbc = BugBearChecker(filename=str(filename)) + errors = list(bbc.run()) + expected = self.errors( + B041(2, 18), + B041(3, 18), + B041(3, 37), + B041(4, 18), + B041(4, 28), + B041(5, 14), + B041(6, 17), + B041(7, 17), + B041(8, 14), + ) + self.assertEqual(errors, expected) + def test_b908(self): filename = Path(__file__).absolute().parent / "b908.py" bbc = BugBearChecker(filename=str(filename)) From 1c5a73a8bc7dce5c1875957ae8611aee273f0e68 Mon Sep 17 00:00:00 2001 From: yangdanny97 Date: Tue, 29 Oct 2024 22:37:29 -0400 Subject: [PATCH 3/3] format --- bugbear.py | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/bugbear.py b/bugbear.py index c5ab062..25e160e 100644 --- a/bugbear.py +++ b/bugbear.py @@ -8,7 +8,7 @@ import re import sys import warnings -from collections import defaultdict, namedtuple, Counter +from collections import Counter, defaultdict, namedtuple from contextlib import suppress from functools import lru_cache, partial from keyword import iskeyword @@ -662,10 +662,7 @@ def convert_to_value(item): keys = [convert_to_value(key) for key in node.keys] key_counts = Counter(keys) - duplicate_keys = [ - key for key, count in key_counts.items() - if count > 1 - ] + duplicate_keys = [key for key, count in key_counts.items() if count > 1] for key in duplicate_keys: key_indices = [i for i, i_key in enumerate(keys) if i_key == key] seen = set() @@ -2370,11 +2367,7 @@ def visit_Lambda(self, node) -> None: message="B040 Exception with added note not used. Did you forget to raise it?" ) -B041 = Error( - message=( - "B041 Repeated key-value pair in dictionary literal." - ) -) +B041 = Error(message=("B041 Repeated key-value pair in dictionary literal.")) # Warnings disabled by default. B901 = Error(