From ccf030f06585b906b5cfff6aa9c946fead48af00 Mon Sep 17 00:00:00 2001 From: Eljees <3.14hell@gmail.com> Date: Sat, 15 Aug 2026 17:49:17 +0300 Subject: [PATCH 1/2] B020: don't flag rebinding an attribute of the loop's base object `for self.a in self.b` rebinds the attribute `a`, not the name `self`, so comparing the bare base name reported every loop over a sibling attribute of the same object. Compare the whole dotted path instead, and ignore names that only ever appear in load context (they are the base of an attribute target, not something the loop rebinds). Fixes #248 --- README.rst | 2 ++ bugbear.py | 53 ++++++++++++++++++++++++++++++++++------ tests/eval_files/b020.py | 19 ++++++++++++++ 3 files changed, 67 insertions(+), 7 deletions(-) diff --git a/README.rst b/README.rst index 51cca44..8c4a76e 100644 --- a/README.rst +++ b/README.rst @@ -501,6 +501,8 @@ UNRELEASED ~~~~~~~~~~ * B019: also flag `async_lru.alru_cache` and check cache decorators on `async def` methods (#488) +* B020: don't flag `for self.a in self.b`: rebinding an attribute is not rebinding the + base name, so two different attributes of the same object are two bindings (#248) * B018: handle also useless calls such as `isinstance(x, int)` without assigning or using the result * B031: don't count a store-context reference (e.g. an annotation target like `group: T`) as a use of the `groupby` generator (#465) * B902: don't raise a false positive on a metaclass defined with a dotted base such as `abc.ABCMeta` or `enum.EnumMeta` (#411) diff --git a/bugbear.py b/bugbear.py index 1b49311..8f57064 100644 --- a/bugbear.py +++ b/bugbear.py @@ -975,18 +975,32 @@ def check_for_b019(self, node: ast.FunctionDef | ast.AsyncFunctionDef) -> None: return def check_for_b020(self, node: ast.For) -> None: - targets = NameFinder() - targets.visit(node.target) - ctrl_names = set(targets.names) - iterset = B020NameFinder() iterset.visit(node.iter) iterset_names = set(iterset.names) - for name in sorted(ctrl_names): + # `for self.a in self.b` rebinds an attribute, not the name `self`, so + # comparing the bare base name reports every loop over a sibling + # attribute of the same object. Compare the whole dotted path instead. + candidates: dict[str, ast.expr] = dict(_dotted_targets(node.target)) + if candidates: + for sub in ast.walk(node.iter): + if isinstance(sub, ast.Attribute): + path = _dotted_name(sub) + if path is not None: + iterset_names.add(path) + + # a name that only ever appears in load context is the *base* of an + # attribute or subscript target, not something the loop rebinds + targets = NameFinder() + targets.visit(node.target) + for name, names in targets.names.items(): + if any(isinstance(n.ctx, ast.Store) for n in names): + candidates[name] = names[0] + + for name in sorted(candidates): if name in iterset_names: - n = targets.names[name][0] - self.add_error("B020", n, name) + self.add_error("B020", candidates[name], name) def check_for_b023( # noqa: C901 self, @@ -2082,6 +2096,31 @@ def visit(self, node: ast.AST | list[ast.stmt]) -> ast.AST | list[ast.stmt] | No return node +def _dotted_name(node: ast.AST) -> str | None: + """Return `"self.a.b"` for an attribute chain rooted in a name, else None.""" + parts = [] + while isinstance(node, ast.Attribute): + parts.append(node.attr) + node = node.value + if not isinstance(node, ast.Name): + return None + parts.append(node.id) + return ".".join(reversed(parts)) + + +def _dotted_targets(target: ast.AST) -> Iterator[tuple[str, ast.Attribute]]: + """Yield the attribute paths a `for` statement rebinds on each iteration.""" + if isinstance(target, (ast.Tuple, ast.List)): + for element in target.elts: + yield from _dotted_targets(element) + elif isinstance(target, ast.Starred): + yield from _dotted_targets(target.value) + elif isinstance(target, ast.Attribute): + name = _dotted_name(target) + if name is not None: + yield name, target + + @attr.s class NameFinder(ast.NodeVisitor): """Finds a name within a tree of nodes. diff --git a/tests/eval_files/b020.py b/tests/eval_files/b020.py index e6b39ea..c28a075 100644 --- a/tests/eval_files/b020.py +++ b/tests/eval_files/b020.py @@ -38,3 +38,22 @@ for var in sorted(range(10), key=lambda var: var.real): print(var) + + +# `for self.a.b in self.c` rebinds an attribute, not the name `self`: two +# different attributes of the same object are two different bindings. +# https://github.com/PyCQA/flake8-bugbear/issues/248 +class AttributeTargets: + test_suite = [1, 2, 3] + + def ok_sibling_attributes(self): + for self.model_instance.value in self.test_suite: + print(self.model_instance.value) + + def ok_plain_attribute(self): + for self.value in self.test_suite: + print(self.value) + + def still_an_error(self): + for self.test_suite in self.test_suite: # B020: 12, "self.test_suite" + print(self.test_suite) From c54fb354b38fdade4cbe0d5efe38a1c835693d27 Mon Sep 17 00:00:00 2001 From: Eljees <3.14hell@gmail.com> Date: Fri, 21 Aug 2026 08:24:14 +0000 Subject: [PATCH 2/2] 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. --- bugbear.py | 33 +++++++++++++++++++++++++++------ tests/eval_files/b020.py | 19 ++++++++++++++++++- 2 files changed, 45 insertions(+), 7 deletions(-) diff --git a/bugbear.py b/bugbear.py index 8f57064..70b395f 100644 --- a/bugbear.py +++ b/bugbear.py @@ -975,7 +975,7 @@ def check_for_b019(self, node: ast.FunctionDef | ast.AsyncFunctionDef) -> None: return def check_for_b020(self, node: ast.For) -> None: - iterset = B020NameFinder() + iterset = B020AttributeFinder() iterset.visit(node.iter) iterset_names = set(iterset.names) @@ -984,11 +984,7 @@ def check_for_b020(self, node: ast.For) -> None: # attribute of the same object. Compare the whole dotted path instead. candidates: dict[str, ast.expr] = dict(_dotted_targets(node.target)) if candidates: - for sub in ast.walk(node.iter): - if isinstance(sub, ast.Attribute): - path = _dotted_name(sub) - if path is not None: - iterset_names.add(path) + iterset_names |= iterset.paths # a name that only ever appears in load context is the *base* of an # attribute or subscript target, not something the loop rebinds @@ -2272,6 +2268,31 @@ def visit_Lambda(self, node) -> None: self.names.pop(lambda_arg.arg, None) +@attr.s +class B020AttributeFinder(B020NameFinder): + """Dotted attribute paths, under the scope rules B020NameFinder uses for names. + + Collecting the paths with a plain `ast.walk` would ignore lexical scope: in + `for obj.value in [obj.value for obj in objects]` the two `obj` bindings are + different objects, and the comprehension-local one must not be matched + against the loop target. + """ + + paths: set[str] = attr.ib(factory=set) + + def visit_Attribute(self, node: ast.Attribute) -> None: + path = _dotted_name(node) + if path is not None: + self.paths.add(path) + self.generic_visit(node) + + def visit_Lambda(self, node: ast.Lambda) -> None: + super().visit_Lambda(node) + for lambda_arg in node.args.args: + prefix = f"{lambda_arg.arg}." + self.paths = {path for path in self.paths if not path.startswith(prefix)} + + B005_METHODS = {"lstrip", "rstrip", "strip"} # Note: these are also used by B039 diff --git a/tests/eval_files/b020.py b/tests/eval_files/b020.py index c28a075..7e5cac8 100644 --- a/tests/eval_files/b020.py +++ b/tests/eval_files/b020.py @@ -1,6 +1,6 @@ """ Should emit: -B020 - on lines 8, 21, and 36 +B020 - on lines 8, 21, 32, 36, 58, and 75 """ items = [1, 2, 3] @@ -57,3 +57,20 @@ def ok_plain_attribute(self): def still_an_error(self): for self.test_suite in self.test_suite: # B020: 12, "self.test_suite" print(self.test_suite) + +# the `obj` a comprehension binds is not the `obj` the loop rebinds +def ok_comprehension_scope(obj, objects): + for obj.value in [obj.value for obj in objects]: + print(obj.value) + + +# nor is the `obj` a lambda binds +def ok_lambda_scope(obj, objects): + for obj.value in map(lambda obj: obj.value, objects): + print(obj.value) + + +# the same path on both sides is still an error +def still_an_error_at_module_level(obj): + for obj.value in obj.value: # B020: 8, "obj.value" + print(obj.value)