From b4986aafd409f9a3cd0e9c9c9ec1dea86a67f703 Mon Sep 17 00:00:00 2001 From: Martin Imre Date: Fri, 26 Apr 2024 16:47:05 +0200 Subject: [PATCH] fix(b909): Fix false positive affecting containers of mutables (#469) * 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 ``` * 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) ``` * 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 | 53 ++++++++++++++++++++++++++++++++++++++++++++++++--- tests/b909.py | 23 ++++++++++++++++++++++ 2 files changed, 73 insertions(+), 3 deletions(-) diff --git a/bugbear.py b/bugbear.py index befb300..5c0aac7 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() @@ -1603,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 = ( @@ -1624,14 +1666,19 @@ 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 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 + and _to_name_str(_tansform_slice_to_py39(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 00ee343..1601ee9 100644 --- a/tests/b909.py +++ b/tests/b909.py @@ -127,3 +127,26 @@ def __init__(self, ls): bar.remove(1) break break + +lst: list[dict] = [{}, {}, {}] +for dic in lst: + 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