Skip to content

Add B913 for unused trailing zip values#564

Draft
sapunyangkut wants to merge 1 commit into
PyCQA:mainfrom
sapunyangkut:codex/issue-545-unused-zip-item
Draft

Add B913 for unused trailing zip values#564
sapunyangkut wants to merge 1 commit into
PyCQA:mainfrom
sapunyangkut:codex/issue-545-unused-zip-item

Conversation

@sapunyangkut

Copy link
Copy Markdown

Summary

  • add the optional B913 diagnostic for direct zip() loops with unused or repeated trailing _ targets
  • distinguish real _ loads from stores and comprehension-local bindings
  • document the rule and cover positive, conservative, starred, arity, strict=, nested-target, and loop-length cases

Closes #545.

Behavior and safety

B913 remains disabled by default. It only checks direct zip() calls with a statically known one-to-one top-level target mapping. Calls with keywords (including strict=), starred targets or arguments, arity mismatches, non-trailing or nested _ targets, and all-_ targets are skipped.

Removing a zip() iterable can change the loop length. The diagnostic therefore recommends keeping a descriptive variable when that iterable intentionally controls iteration, and only suggests removal otherwise. Complex nested rebinding is treated conservatively as usage, favoring missed diagnostics over false positives.

Validation

  • python -m coverage run -m pytest tests/test_bugbear.py: 81 passed, 1 skipped
  • python -m coverage report -m: 98% for bugbear.py
  • python -m mypy bugbear.py: success
  • pre-commit run --files bugbear.py README.rst tests/eval_files/b913.py: isort, Black, flake8, and rstcheck passed
  • git diff --check: passed

AI assistance

This pull request was fully AI-assisted and automatically prepared under the GitHub account owner's authorization. Separate AI review passes checked diagnostic correctness, prior review concerns, tests, and security boundaries; their actionable findings were addressed before publication. No issue or pull-request claim comment was posted.

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

Adds a new optional flake8-bugbear diagnostic (B913) to detect for ... in zip(...) loops where one or more trailing zip() values are discarded via _ targets, plus accompanying documentation and eval coverage. This fits the codebase’s pattern of providing conservative, opt-in correctness/style checks in the B9xx range.

Changes:

  • Implement B913 in bugbear.py, including _-usage detection that avoids counting stores and comprehension-local bindings.
  • Document B913 in README.rst and add it to the UNRELEASED notes.
  • Add tests/eval_files/b913.py covering positive and “no error” cases (strict=, starred, arity, nested targets, loop-length semantics, etc.).

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
tests/eval_files/b913.py Adds eval cases for B913 hits and non-hits across a range of zip/target patterns.
README.rst Documents the new B913 rule and notes it in the changelog.
bugbear.py Implements B913 detection logic, usage finder, and registers the new error code/message.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread bugbear.py
Comment on lines +1673 to +1681
trailing_underscores = 0
for target in reversed(targets):
if isinstance(target, ast.Name) and target.id == "_":
trailing_underscores += 1
else:
break
if trailing_underscores == 0 or trailing_underscores == len(targets):
return

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.

Proposed rule: match zip() arguments and unpacked variables

2 participants