[flake8-bugbear] Flag useless subscript access in B018#23966
[flake8-bugbear] Flag useless subscript access in B018#23966shivamtiwari3 wants to merge 2 commits intoastral-sh:mainfrom
flake8-bugbear] Flag useless subscript access in B018#23966Conversation
B018flake8-bugbear] Flag useless subscript access in B018
|
| code | total | + violation | - violation | + fix | - fix |
|---|---|---|---|---|---|
| B018 | 125 | 125 | 0 | 0 | 0 |
| PLR0917 | 2 | 1 | 1 | 0 | 0 |
|
Thanks! I think we should make this a preview change since this is a significant expansion to a stable rule (as shown by the ecosystem check). |
From a quick look, most ecosystem violations happen in voluntary places, either inside a |
Root cause: `contains_effect` returns `true` for `Expr::Subscript` (because `__getitem__` may have side effects), causing `B018` to skip subscript expressions entirely — just as it used to do for attributes. Fix: mirror the attribute-access special case that was added in astral-sh#3455. When `contains_effect` returns `true` but the top-level expression is a subscript, report `UselessExpression` with a new `Subscript` kind and the message "Found useless subscript access. Either assign it to a variable or remove it." Fixes astral-sh#23932
The subscript flagging is a significant expansion to a stable rule (123 new violations across the ecosystem), so it should only run in preview mode. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
ca5aa11 to
2ee4802
Compare
|
This PR will benefit from the new |
Summary
Fixes #23932.
B018already flags useless attribute accesses (e.g.obj.attr) even whencontains_effectreturnstruefor them, because attribute access can trigger__getattribute__. This PR applies the same treatment to subscript expressions(e.g.
x["item"]), whichcontains_effectalso marks as potentiallyside-effectful (due to
__getitem__), but which are equally "useless" whenthe result is discarded.
Root Cause
contains_effectinruff_python_ast/src/helpers.rs(line 123) unconditionallyreturns
trueforExpr::Subscript. InB018'suseless_expressionfunction(
flake8_bugbear/rules/useless_expression.rs), whencontains_effectreturnstruethe function exits early — unless the top-level expression is anAttribute, in which case it still reports a diagnostic. Subscripts received nosuch special-case, so
x["item"]was silently skipped.Solution
Mirror the existing attribute special-case for subscript expressions:
A new
Kind::Subscriptvariant is added with the message:Testing
Added
foo6()toB018.pycovering:x["item"]— plain name subscript → flaggedobj.attr["key"]— subscript on attribute → flaggedget_dict()["key"]— subscript on call result → flaggedx[0]— integer key → flagged_ = x["item"]— intentional ignore (assignment) → not flaggedAll existing
B018tests (B018.pyandB018.ipynb) still pass. Snapshotupdated accordingly.
Checklist
cargo test -p ruff_linter)uvx prek run -apasses (rustfmt, clippy, ruff format/check, etc.)