Skip to content

Commit

Permalink
fix(b909): Fix false positive affecting containers of mutables (#469)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
mimre25 authored Apr 26, 2024
1 parent b9f9dce commit b4986aa
Show file tree
Hide file tree
Showing 2 changed files with 73 additions and 3 deletions.
53 changes: 50 additions & 3 deletions bugbear.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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 = (
Expand All @@ -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)

Expand Down
23 changes: 23 additions & 0 deletions tests/b909.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit b4986aa

Please sign in to comment.