Skip to content

[pylint] Fix false positives in PLR1733 and PLR1736#25233

Draft
ntBre wants to merge 1 commit into
mainfrom
brent/pylint-unnecessary-lookups
Draft

[pylint] Fix false positives in PLR1733 and PLR1736#25233
ntBre wants to merge 1 commit into
mainfrom
brent/pylint-unnecessary-lookups

Conversation

@ntBre
Copy link
Copy Markdown
Contributor

@ntBre ntBre commented May 18, 2026

Summary

This PR fixes #25182, as well as some related issues I didn't notice there. In short, both of these
rules flag issues like:

FRUITS = {"apple": 1, "orange": 10, "berry": 22}

for fruit_name, fruit_count in FRUITS.items():
    print(FRUITS[fruit_name])

where the FRUITS[fruit_name] can be replaced by fruit_count from the loop. However, both rules
were missing some ways of shadowing these variables, so I added more tracking to SequenceIndexVisitor.

Initially I tried comparing Bindings instead of the str names currently stored on the visitor
before realizing that the rules were tracking this information via the is_modified field, which
also handles other modifications. I'm still not sure I'm entirely happy with this, but I wanted to
check the ecosystem report and checkpoint the progress so far.

Test Plan

New mdtests based on the issue

Summary
--

This PR fixes #25182, as well as some related issues I didn't notice there. In short, both of these
rules flag issues like:

```py
FRUITS = {"apple": 1, "orange": 10, "berry": 22}

for fruit_name, fruit_count in FRUITS.items():
    print(FRUITS[fruit_name])
```

where the `FRUITS[fruit_name]` can be replaced by `fruit_count` from the loop. However, both rules
were missing some ways of shadowing these variables, so I added that to `SequenceIndexVisitor`.

Initially I tried comparing `Binding`s instead of the `str` names currently stored on the visitor
before realizing that the rules were tracking this information via the `is_modified` field, which
also handles other modifications. I'm still not sure I'm entirely happy with this, but I wanted to
check the ecosystem report and checkpoint the progress so far.

Test Plan
--

New mdtests based on the issue
@ntBre ntBre added bug Something isn't working rule Implementing or modifying a lint rule labels May 18, 2026
@astral-sh-bot
Copy link
Copy Markdown

astral-sh-bot Bot commented May 18, 2026

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working rule Implementing or modifying a lint rule

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Additional false positives in PLR1733 and PLR1736

1 participant