Skip to content

Commit c54fb35

Browse files
committed
Collect the iterable's attribute paths with a scope-aware visitor
Addresses the two review points. Walking node.iter with ast.walk ignored lexical scope, which the name side of the check does not: B020NameFinder skips names bound by a comprehension or a lambda. So in for obj.value in [obj.value for obj in objects]: the comprehension-local obj.value was matched against the loop target and the loop was reported. B020AttributeFinder now collects the paths, inheriting those exclusions, and drops paths rooted in a lambda argument the same way the base class drops the argument names. The eval file's 'Should emit' header is refreshed. It was stale by more than the new line: line 32 was missing from it as well.
1 parent ccf030f commit c54fb35

2 files changed

Lines changed: 45 additions & 7 deletions

File tree

bugbear.py

Lines changed: 27 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -975,7 +975,7 @@ def check_for_b019(self, node: ast.FunctionDef | ast.AsyncFunctionDef) -> None:
975975
return
976976

977977
def check_for_b020(self, node: ast.For) -> None:
978-
iterset = B020NameFinder()
978+
iterset = B020AttributeFinder()
979979
iterset.visit(node.iter)
980980
iterset_names = set(iterset.names)
981981

@@ -984,11 +984,7 @@ def check_for_b020(self, node: ast.For) -> None:
984984
# attribute of the same object. Compare the whole dotted path instead.
985985
candidates: dict[str, ast.expr] = dict(_dotted_targets(node.target))
986986
if candidates:
987-
for sub in ast.walk(node.iter):
988-
if isinstance(sub, ast.Attribute):
989-
path = _dotted_name(sub)
990-
if path is not None:
991-
iterset_names.add(path)
987+
iterset_names |= iterset.paths
992988

993989
# a name that only ever appears in load context is the *base* of an
994990
# attribute or subscript target, not something the loop rebinds
@@ -2272,6 +2268,31 @@ def visit_Lambda(self, node) -> None:
22722268
self.names.pop(lambda_arg.arg, None)
22732269

22742270

2271+
@attr.s
2272+
class B020AttributeFinder(B020NameFinder):
2273+
"""Dotted attribute paths, under the scope rules B020NameFinder uses for names.
2274+
2275+
Collecting the paths with a plain `ast.walk` would ignore lexical scope: in
2276+
`for obj.value in [obj.value for obj in objects]` the two `obj` bindings are
2277+
different objects, and the comprehension-local one must not be matched
2278+
against the loop target.
2279+
"""
2280+
2281+
paths: set[str] = attr.ib(factory=set)
2282+
2283+
def visit_Attribute(self, node: ast.Attribute) -> None:
2284+
path = _dotted_name(node)
2285+
if path is not None:
2286+
self.paths.add(path)
2287+
self.generic_visit(node)
2288+
2289+
def visit_Lambda(self, node: ast.Lambda) -> None:
2290+
super().visit_Lambda(node)
2291+
for lambda_arg in node.args.args:
2292+
prefix = f"{lambda_arg.arg}."
2293+
self.paths = {path for path in self.paths if not path.startswith(prefix)}
2294+
2295+
22752296
B005_METHODS = {"lstrip", "rstrip", "strip"}
22762297

22772298
# Note: these are also used by B039

tests/eval_files/b020.py

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
"""
22
Should emit:
3-
B020 - on lines 8, 21, and 36
3+
B020 - on lines 8, 21, 32, 36, 58, and 75
44
"""
55

66
items = [1, 2, 3]
@@ -57,3 +57,20 @@ def ok_plain_attribute(self):
5757
def still_an_error(self):
5858
for self.test_suite in self.test_suite: # B020: 12, "self.test_suite"
5959
print(self.test_suite)
60+
61+
# the `obj` a comprehension binds is not the `obj` the loop rebinds
62+
def ok_comprehension_scope(obj, objects):
63+
for obj.value in [obj.value for obj in objects]:
64+
print(obj.value)
65+
66+
67+
# nor is the `obj` a lambda binds
68+
def ok_lambda_scope(obj, objects):
69+
for obj.value in map(lambda obj: obj.value, objects):
70+
print(obj.value)
71+
72+
73+
# the same path on both sides is still an error
74+
def still_an_error_at_module_level(obj):
75+
for obj.value in obj.value: # B020: 8, "obj.value"
76+
print(obj.value)

0 commit comments

Comments
 (0)