B008: resolve imported immutable calls - #574
Conversation
There was a problem hiding this comment.
🟡 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.
| if self._b008_in_module_scope() and isinstance(node.ctx, (ast.Store, ast.Del)): | ||
| self._b008_shadow_imports((node.id,)) |
| if self._b008_in_module_scope() and isinstance(node.ctx, (ast.Store, ast.Del)): | ||
| self._b008_shadow_imports((node.id,)) |
cooperlees
left a comment
There was a problem hiding this comment.
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)
a362dec to
4588df1
Compare
There was a problem hiding this comment.
🟡 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
contextscontains 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 requiringlen == 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 exemptsDepends(). 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
| self.check_for_b903(node) | ||
| if self._b008_in_direct_module_child(): | ||
| self._b008_shadow_named_expr_targets(node.decorator_list) |
| if ( | ||
| self.b008_b039_extend_immutable_calls | ||
| and self._b008_in_direct_module_child() | ||
| ): | ||
| imported_names = self._b008_imports |
Fixes #252.
Summary
extend-immutable-callsValidation
tox -e py313— 81 passed, 1 skipped; 97% coveragepre-commit run --all-files— isort, Black, flake8, and rstcheck passedgit diff --checkScope
This intentionally resolves only direct absolute module-level imports. Nested/local, relative, and star-import resolution remain out of scope, and B039 behavior is unchanged.