Skip to content

B031: allow reusing groups after materialization - #576

Merged
cooperlees merged 2 commits into
PyCQA:mainfrom
Sonike:fix/b031-materialized-group-reassignment
Sep 9, 2026
Merged

B031: allow reusing groups after materialization#576
cooperlees merged 2 commits into
PyCQA:mainfrom
Sonike:fix/b031-materialized-group-reassignment

Conversation

@Sonike

@Sonike Sonike commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

After group = list(group), B031 currently flags later reads of group even though it now holds a reusable list. This recognizes explicit list(group) and tuple(group) assignments back to the same name, including annotated and chained assignments.

The assignment's right-hand side is checked before the name becomes reusable. Conditional paths are merged conservatively, so a conversion on only one branch or inside a potentially empty nested loop does not suppress genuine repeated-use warnings. Arbitrary calls such as iter(group) retain the existing behavior; this does not add alias analysis or infer the return types of helper functions.

Fixes #395.

Validation:

  • Confirmed that the initial regression cases fail on the current main branch, reporting three spurious B031 diagnostics, and pass with this change.
  • tox -e py313,py314: Python 3.13 — 80 passed, 1 version-specific skip; Python 3.14 — 81 passed. Both report 98% coverage.
  • pre-commit run --all-files: isort, black, flake8, and rstcheck passed.
  • Regression coverage includes direct/annotated/chained assignments, both and partial conditional branches, previous consumption, assignment to another name, iter(), nested loops, and deferred function bodies.

Recognize list and tuple assignments back to the group name after checking their right-hand side. Preserve diagnostics on paths where the original generator can still be used, including conditional branches and potentially empty nested loops.

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.

🟢 Approval recommended

The implementation and new eval coverage align with the stated behavior change and appear to preserve conservative warnings in ambiguous control-flow cases.

Pull request overview

This PR updates the B031 check (groupby-group reuse) so that reassigning group = list(group) or group = tuple(group) back to the same variable is recognized as “materialized and now reusable,” avoiding false positives like #395 while still being conservative in conditionals and potentially-empty nested loops.

Changes:

  • Teach B031’s AST walk to detect same-name list(...) / tuple(...) materialization assignments (including annotated and chained assignments) and treat the name as reusable afterward.
  • Adjust B031 path-merging for nested loops and other constructs to avoid assuming assignments in potentially-optional/deferred execution definitely ran.
  • Add eval coverage for direct/annotated/chained materialization, conditional branches, nested loops, iter(), and deferred function bodies; document the change in the changelog.
File summaries
File Description
bugbear.py Adds materialization detection for B031 and refines control-flow handling to avoid spurious repeated-use warnings after list(group) / tuple(group) reassignment.
tests/eval_files/b031.py Adds regression/eval cases covering materialization and conservative merging scenarios (conditionals, nested loops, deferred bodies).
README.rst Notes the B031 behavior change in the UNRELEASED changelog.
Review details
  • Files reviewed: 3/3 changed files
  • Comments generated: 0
  • Review effort level: Lite

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

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

Agree with copilot. Many thanks for adding this!

@cooperlees

Copy link
Copy Markdown
Collaborator

@copilot resolve the merge conflicts on this branch.

- Keep both the B031 materialization and B028 skip_file_prefixes entries.
- Preserve the approved B031 implementation and regression coverage.
- Include the upstream B028 fix without rewriting the PR branch.

Validation: tox -e py313,py314 (80 passed/1 skipped; 81 passed), 98% coverage; pre-commit run --all-files (isort, black, flake8, rstcheck).
@Sonike

Sonike commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

Resolved the changelog conflict by merging the latest main in a5db492, keeping both the B031 and B028 entries. The approved B031 implementation and regression cases are unchanged.

Revalidated the combined result:

  • tox -e py313,py314: Python 3.13 — 80 passed, 1 version-specific skip; Python 3.14 — 81 passed. Both report 98% coverage.
  • pre-commit run --all-files: isort, black, flake8, and rstcheck passed.
  • Rechecked direct list/tuple, annotated, and chained materialization assignments: four false positives on current main, none on this branch; the iter(group) reuse warning is retained.

@cooperlees
cooperlees merged commit 6358c02 into PyCQA:main Sep 9, 2026
6 checks passed
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.

B031 false positive when reassigning the returned generator

3 participants