Skip to content

B020: don't flag rebinding an attribute of the loop's base object - #568

Open
Eljees wants to merge 1 commit into
PyCQA:mainfrom
Eljees:fix/248-b020-attribute-target
Open

B020: don't flag rebinding an attribute of the loop's base object#568
Eljees wants to merge 1 commit into
PyCQA:mainfrom
Eljees:fix/248-b020-attribute-target

Conversation

@Eljees

@Eljees Eljees commented Aug 15, 2026

Copy link
Copy Markdown

Fixes #248.

The false positive

for self.a in self.b rebinds the attribute a — it does not rebind the name self.
check_for_b020 compared the bare base name of the loop target against the names in the
iterable, so self matched self, and every loop over a sibling attribute of the same
object was reported:

class A:
    test_suite = [1, 2, 3]

    def method(self):
        for self.model_instance.value in self.test_suite:  # B020 (false positive)
            print(self.model_instance.value)

self.model_instance.value and self.test_suite are two different bindings, so this loop
cannot reassign the thing it is iterating.

The fix

Two changes in check_for_b020:

  • compare the whole dotted path (self.model_instance.value against self.test_suite)
    rather than the base name, on both the target and the iterable side;
  • ignore names that only ever appear in load context — those are the base of an
    attribute or subscript target, not something the loop rebinds.

What still errors

The case the check exists for is unchanged, and there is an eval case pinning it:

for self.test_suite in self.test_suite:  # B020: 12, "self.test_suite"

Checks

  • tests/eval_files/b020.py gains three cases — two that must now be silent and one that
    must still error. Reverting only bugbear.py makes the file fail with exactly the two
    extra B020s, so the new cases do pin this fix rather than passing incidentally.
  • Full suite on 2155484: 79 passed, 2 skipped — same as main.
  • pre-commit run --all-files: isort, black, flake8, rstcheck all pass.
  • README.rst UNRELEASED updated.

@cooperlees — you wrote on the issue that you weren't sure this one could be fixed.
Comparing full dotted paths turned out to be enough; happy to adjust the approach if you'd
rather this stayed a known limitation.

`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 PyCQA#248

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes B020 false positives when loop targets and iterables use different attributes of the same object.

Changes:

  • Compares complete dotted attribute paths.
  • Ignores load-only target names.
  • Adds regression cases and changelog documentation.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
bugbear.py Updates B020 attribute-target analysis.
tests/eval_files/b020.py Adds attribute-target regression cases.
README.rst Documents the B020 fix.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread bugbear.py
Comment on lines +986 to +990
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)
Comment thread tests/eval_files/b020.py
print(self.value)

def still_an_error(self):
for self.test_suite in self.test_suite: # B020: 12, "self.test_suite"

@cooperlees cooperlees left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - Thanks for this.

I think copilot has found some nice performance things especially to polish up here - As always, feel free to state why it's wrong tho if it is.

@Eljees

Eljees commented Aug 16, 2026

Copy link
Copy Markdown
Author

Thanks. Both are right.

The scope point is the substantive one: I collected dotted paths with ast.walk, so a comprehension- or lambda-local binding is treated as the same object as the outer one, and for obj.value in [obj.value for obj in objects] regresses. I will collect the paths with a visitor that reuses the comprehension and lambda exclusions B020NameFinder already applies, and add both scopes as regression cases.

The stale Should emit header is my oversight — line 58 will be listed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Possible B020 false positive with instance attribute

3 participants