Skip to content

B008: resolve imported immutable calls - #574

Open
1678092075 wants to merge 1 commit into
PyCQA:mainfrom
1678092075:fix/b008-imported-immutable-calls
Open

1678092075 wants to merge 1 commit into
PyCQA:mainfrom
1678092075:fix/b008-imported-immutable-calls

Conversation

@1678092075

@1678092075 1678092075 commented Sep 2, 2026 •

Copy link
Copy Markdown

Fixes #252.

Summary

  • resolve direct absolute module-level imports and aliases before matching B008's extend-immutable-calls, including uses in methods
  • preserve existing source-path matches while invalidating imported bindings after module or class-scope redefinitions
  • track definition-time rebinding through defaults, decorators, annotations, class bases/keywords, eager comprehensions, and class global declarations
  • add focused eval coverage for imports, aliases, negative matches, shadowing, deferred generator expressions, and unchanged B039 source-path behavior

Validation

  • tox -e py313 — 84 passed, 1 skipped; 97% coverage
  • tox -e py314 — 85 passed; 97% coverage
  • pre-commit run --all-files — isort, Black, flake8, and rstcheck passed
  • independent read-only review — no P0/P1 findings after fixes
  • git diff --check

tox -e py313-mypy reports the same four existing errors as upstream/main; this change adds no mypy error.

Scope

This intentionally resolves only direct absolute module-level imports. Nested function definitions and conditionally guarded definitions remain out of scope, as do local, relative, and star-import resolution. B039 keeps its existing source-path behavior and does not resolve imported aliases in this PR.

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.

🟡 Changes recommended

Module-scope tracking mishandles annotation-only assignments and misses some walrus rebindings.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds import-aware resolution for B008 immutable-call matching and shadowing detection.

Changes:

  • Resolves absolute module imports and aliases.
  • Tracks module-level rebinding.
  • Adds evaluation tests and changelog entry.
File summaries
File Description
bugbear.py Implements import resolution and shadow tracking.
tests/eval_files/b008_extended.py Tests imports, aliases, and rebinding.
tests/eval_files/b008_extended_shadowing.py Tests unqualified configuration shadowing.
README.rst Adds the unreleased changelog entry.
Review details
  • Files reviewed: 4/4 changed files
  • Comments generated: 2
  • Review effort level: Balanced

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

Comment thread bugbear.py Outdated
Comment on lines +643 to +644
if self._b008_in_module_scope() and isinstance(node.ctx, (ast.Store, ast.Del)):
self._b008_shadow_imports((node.id,))

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Fixed in 56eee1a. Annotation-only assignments now preserve the imported binding, with a regression case covering a subsequent configured call.

Comment thread bugbear.py Outdated
Comment on lines +643 to +644
if self._b008_in_module_scope() and isinstance(node.ctx, (ast.Store, ast.Del)):
self._b008_shadow_imports((node.id,))

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Fixed in 56eee1a. Definition-time walrus rebindings are tracked across defaults, decorators, eager nested comprehensions, annotations, and lambda defaults; deferred generator-expression bodies remain deferred.

@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.

Thanks for this. This seems mostly there, but maybe we can add a test case + handle the walrus operator too? (if I'm understanding correctly copilots finding)

@1678092075
1678092075 force-pushed the fix/b008-imported-immutable-calls branch from a362dec to 4588df1 Compare September 8, 2026 04:25
@cooperlees
cooperlees requested a balanced review from Copilot September 8, 2026 15:49

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.

🟡 Changes recommended

Import resolution misses class methods and several valid module-scope rebinding forms.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (2)

bugbear.py:804

  • This recognizes a walrus only when it is inside exactly one comprehension context. In a nested module-level comprehension, the assignment-expression target still binds at module scope, but contexts contains the module plus both comprehensions, so the imported name remains trusted and later calls are incorrectly exempted. Check that the first context is the module and every remaining context is a comprehension instead of requiring len == 2.
            or (
                len(self.contexts) == 2
                and isinstance(self.contexts[0].node, ast.Module)
                and isinstance(self.contexts[1].node, COMPREHENSION_NODES)
            )

bugbear.py:2504

  • Skipping the entire lambda also skips its default expressions, even though those expressions execute immediately when the lambda is created. For a decorator such as @decorate(lambda value=(Depends := Other): value), the module binding is changed before the decorated function's defaults are evaluated, but the stale import mapping still exempts Depends(). Visit the lambda defaults while continuing to skip its deferred body.
class B008NamedExprFinder(NamedExprFinder):
    def visit_Lambda(self, node: ast.Lambda) -> None:  # noqa: B906
        pass
  • Files reviewed: 4/4 changed files
  • Comments generated: 2
  • Review effort level: Balanced

Comment thread bugbear.py Outdated
Comment on lines +732 to +734
self.check_for_b903(node)
if self._b008_in_direct_module_child():
self._b008_shadow_named_expr_targets(node.decorator_list)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Fixed in 56eee1a. Class bases and keywords are scanned for definition-time rebindings, lambda defaults are visited without traversing deferred lambda bodies, and the regression suite covers both paths.

Comment thread bugbear.py Outdated
Comment on lines +910 to +914
if (
self.b008_b039_extend_immutable_calls
and self._b008_in_direct_module_child()
):
imported_names = self._b008_imports

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Fixed in 56eee1a. Methods in direct module-level classes inherit the module import mapping, while class-local assignments, imports, definitions, and global declarations invalidate the appropriate binding. Regression cases cover inherited and rebound names.

@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.

I think copilot is right - let's add in that fix.

@1678092075
1678092075 force-pushed the fix/b008-imported-immutable-calls branch 2 times, most recently from 02d48ec to 505e474 Compare September 21, 2026 13:51
@1678092075
1678092075 force-pushed the fix/b008-imported-immutable-calls branch from 505e474 to 56eee1a Compare September 21, 2026 13:52
@cooperlees
cooperlees requested a lite review from Copilot September 21, 2026 18:30

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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@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.

I read all previous requests on this PR and checked out 56eee1a locally — 85 passed, parse OK. This fixes #252 for the reported cases.

What looks good in 56eee1a:

  • Direct absolute import / from + aliases resolved to qualified extend-immutable-calls, e.g. Depends -> fastapi.Depends, fastapi_alias.Depends, and source-path matches (fastapi.Depends(...)) still work.
  • Previous threads addressed:
    • annotation-only Depends: Callable no longer shadows — correct, with regression test.
    • definition-time walrus tracked in defaults, decorators, eager nested comprehensions, class bases/keywords, lambda defaults; deferred GeneratorExp bodies stay deferred — matches runtime eval order (decorators before defaults).
    • methods in direct module-level classes inherit mapping, class-local Assign/Import/def/global correctly invalidate.
  • Shadowing coverage is thorough: if-guarded / loop / except as / match / global / conditional import all invalidate, as they should.

Non-blocking follow-ups for later (happy to merge as-is if you do not want to spin again — I will give it a day then merge):

  1. Scope — usage location: only direct module children + direct class children get imported_names. So this still flags B008 despite a valid module import:
from fastapi import Depends
def outer():
    def inner(db=Depends()): ...  # still B008
if cond:
    def f(db=Depends()): ...  # still B008

If intentional, worth one line in the PR description — "nested / guarded defs remain out of scope for now".
2. B039 inconsistency: check_for_b006_and_b008 passes imported_names, but check_for_b039 does not, so ContextVar("x", default=Depends()) with extend=["fastapi.Depends"] still flags while the B008 equivalent is exempt. If B039 import support is intentionally out of scope ("B039 source-path unchanged"), suggest stating that explicitly.
3. Minor — annotation walrus is dead code in real Python: def f(x: (D := other)) parses with ast but fails compile() with named expression cannot be used within an annotation. The _b008_function_annotations pre-scan + test therefore can never trigger at runtime. Harmless/defensive, but consider a comment or removal.

No P0/P1 from me — approving, thanks for iterating on all the Copilot + manual feedback!

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.

B008: extend-immutable-calls does not work with imported function

3 participants