From 65c86b09abe8a3d305b923b78159b54e1e2a629e Mon Sep 17 00:00:00 2001 From: Martin Imre Date: Thu, 25 Apr 2024 12:15:19 +0200 Subject: [PATCH 1/3] fix(b909): Fix false positive affecting containers of mutables The false positives occurred when trying to edit a dictionary while iterating over a list of dictionaries: ``` lst: list[dict] = [{}, {}, {}] for dic in lst: dic["key"] = False # was false positive - fixed now ``` --- bugbear.py | 5 ++++- tests/b909.py | 4 ++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/bugbear.py b/bugbear.py index befb300..9ae5cee 100644 --- a/bugbear.py +++ b/bugbear.py @@ -1631,7 +1631,10 @@ def __init__(self, name: str): def visit_Assign(self, node: ast.Assign): for target in node.targets: - if isinstance(target, ast.Subscript) and _to_name_str(target.value): + if ( + isinstance(target, ast.Subscript) + and _to_name_str(target.value) == self.name + ): self.mutations[self._conditional_block].append(node) self.generic_visit(node) diff --git a/tests/b909.py b/tests/b909.py index 00ee343..043d2e0 100644 --- a/tests/b909.py +++ b/tests/b909.py @@ -127,3 +127,7 @@ def __init__(self, ls): bar.remove(1) break break + +lst: list[dict] = [{}, {}, {}] +for dic in lst: + dic["key"] = False From 1b659f63b2cedf2b951cdcf106d3e2ee6283719e Mon Sep 17 00:00:00 2001 From: Martin Imre Date: Fri, 26 Apr 2024 08:51:31 +0200 Subject: [PATCH 2/3] fix(b909): Allow mutation of dict[key] form These changes allow the following: ``` some_dict = {"foo": "bar"} for key in some_dict: some_dict[key] = 3 # no error (previously error'd) ``` --- bugbear.py | 8 ++++++-- tests/b909.py | 21 ++++++++++++++++++++- 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/bugbear.py b/bugbear.py index 9ae5cee..a78ce09 100644 --- a/bugbear.py +++ b/bugbear.py @@ -1581,11 +1581,13 @@ def check(num_args, param_name): def check_for_b909(self, node: ast.For): if isinstance(node.iter, ast.Name): name = _to_name_str(node.iter) + key = _to_name_str(node.target) elif isinstance(node.iter, ast.Attribute): name = _to_name_str(node.iter) + key = _to_name_str(node.target) else: return - checker = B909Checker(name) + checker = B909Checker(name, key) checker.visit(node.body) for mutation in itertools.chain.from_iterable( m for m in checker.mutations.values() @@ -1624,8 +1626,9 @@ class B909Checker(ast.NodeVisitor): "discard", ) - def __init__(self, name: str): + def __init__(self, name: str, key: str): self.name = name + self.key = key self.mutations = defaultdict(list) self._conditional_block = 0 @@ -1634,6 +1637,7 @@ def visit_Assign(self, node: ast.Assign): if ( isinstance(target, ast.Subscript) and _to_name_str(target.value) == self.name + and _to_name_str(target.slice) != self.key ): self.mutations[self._conditional_block].append(node) self.generic_visit(node) diff --git a/tests/b909.py b/tests/b909.py index 043d2e0..1601ee9 100644 --- a/tests/b909.py +++ b/tests/b909.py @@ -130,4 +130,23 @@ def __init__(self, ls): lst: list[dict] = [{}, {}, {}] for dic in lst: - dic["key"] = False + dic["key"] = False # no error + + + +for grammar in grammars: + errors[grammar.version] = InvalidInput() # no error + + + +for key in self.hpo_params: + if key in nni_config: + nni_config[key] = self.hpo_params[key] # no error + + +some_dict = {"foo": "bar"} +for key in some_dict: + some_dict[key] = 3 # no error + +for key in some_dict.keys(): + some_dict[key] = 3 # no error From 81e029a50f816f589f043680341b45db052659d1 Mon Sep 17 00:00:00 2001 From: Martin Imre Date: Fri, 26 Apr 2024 09:56:25 +0200 Subject: [PATCH 3/3] fix(b909): Fix python 3.8 incompatibility Turns out, that the slice type was changed in python 3.9. > Changed in version 3.9: Simple indices are represented by their value, > extended slices are represented as tuples. from https://docs.python.org/3/library/ast.html#module-ast --- bugbear.py | 42 +++++++++++++++++++++++++++++++++++++++++- 1 file changed, 41 insertions(+), 1 deletion(-) diff --git a/bugbear.py b/bugbear.py index a78ce09..5c0aac7 100644 --- a/bugbear.py +++ b/bugbear.py @@ -1605,6 +1605,46 @@ def compose_call_path(node): yield node.id +def _tansform_slice_to_py39(slice: ast.Slice) -> ast.Slice | ast.Name: + """Transform a py38 style slice to a py39 style slice. + + In py39 the slice was changed to have simple names directly assigned: + ```py + # code: + some_dict[key] + # py38: + slice=Index( + value=Name( + lineno=152, + col_offset=14, + end_lineno=152, + end_col_offset=17, + id='key', + ctx=Load() + ), + ) + # py39 onwards: + slice=Name( + lineno=152, + col_offset=14, + end_lineno=152, + end_col_offset=17, + id='key', + ctx=Load() + ), + ``` + + > Changed in version 3.9: Simple indices are represented by their value, + > extended slices are represented as tuples. + from https://docs.python.org/3/library/ast.html#module-ast + """ + if sys.version_info >= (3, 9): + return slice + if isinstance(slice, ast.Index) and isinstance(slice.value, ast.Name): + slice = slice.value + return slice + + class B909Checker(ast.NodeVisitor): # https://docs.python.org/3/library/stdtypes.html#mutable-sequence-types MUTATING_FUNCTIONS = ( @@ -1637,7 +1677,7 @@ def visit_Assign(self, node: ast.Assign): if ( isinstance(target, ast.Subscript) and _to_name_str(target.value) == self.name - and _to_name_str(target.slice) != self.key + and _to_name_str(_tansform_slice_to_py39(target.slice)) != self.key ): self.mutations[self._conditional_block].append(node) self.generic_visit(node)