Skip to content

[pylint] Recurse into subscript subexpressions when searching for list/dict lookups (PLR1733, PLR1736)#13186

Merged
AlexWaygood merged 3 commits intomainfrom
alex/index-lookups
Sep 1, 2024
Merged

[pylint] Recurse into subscript subexpressions when searching for list/dict lookups (PLR1733, PLR1736)#13186
AlexWaygood merged 3 commits intomainfrom
alex/index-lookups

Conversation

@AlexWaygood
Copy link
Copy Markdown
Member

@AlexWaygood AlexWaygood commented Aug 31, 2024

Summary

The SequenceIndexVisitor currently does not recurse into subexpressions of subscripts when searching for subscript accesses that would trigger this rule. That means that we don't currently detect violations of the rule on snippets like this:

data = {"a": 1, "b": 2}
column_names = ["a", "b"]
for index, column_name in enumerate(column_names):
    _ = data[column_names[index]]

Fixes #13183

Test Plan

cargo test -p ruff_linter

@AlexWaygood AlexWaygood added bug Something isn't working rule Implementing or modifying a lint rule labels Aug 31, 2024
@AlexWaygood AlexWaygood changed the title [pylint] Recurse into subexpressions of subscripts when searching for unnecessary-list-index-lookup violations (PLR1736) [pylint] Recurse into subscript subexpressions when searching for unnecessary-list-index-lookup violations (PLR1736) Aug 31, 2024
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Aug 31, 2024 •
edited
Loading

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

ℹ️ ecosystem check detected linter changes. (+2 -0 violations, +0 -0 fixes in 1 projects; 53 projects unchanged)

zulip/zulip (+2 -0 violations, +0 -0 fixes) 

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview --select ALL

+ analytics/views/stats.py:623:51: PLR1733 [*] Unnecessary lookup of dictionary value by key
+ analytics/views/stats.py:625:44: PLR1733 [*] Unnecessary lookup of dictionary value by key

Changes by rule (1 rules affected)

code total + violation - violation + fix - fix
PLR1733 2 2 0 0 0

@AlexWaygood
Copy link
Copy Markdown
Member Author

AlexWaygood commented Aug 31, 2024

ℹ️ ecosystem check detected linter changes. (+2 -0 violations, +0 -0 fixes in 1 projects; 53 projects unchanged)
zulip/zulip (+2 -0 violations, +0 -0 fixes)

Nice, it looks like this also fixes false negatives in other rules that use this visitor

@AlexWaygood AlexWaygood changed the title [pylint] Recurse into subscript subexpressions when searching for unnecessary-list-index-lookup violations (PLR1736) [pylint] Recurse into subscript subexpressions when searching for listd/dict lookups (PLR1733, PLR1736) Aug 31, 2024
@AlexWaygood AlexWaygood changed the title [pylint] Recurse into subscript subexpressions when searching for listd/dict lookups (PLR1733, PLR1736) [pylint] Recurse into subscript subexpressions when searching for list/dict lookups (PLR1733, PLR1736) Aug 31, 2024
@sbrugman
Copy link
Copy Markdown
Contributor

sbrugman commented Aug 31, 2024

LGTM. Thanks for the quick PR :)

@AlexWaygood AlexWaygood merged commit 3abd5c0 into main Sep 1, 2024
@AlexWaygood AlexWaygood deleted the alex/index-lookups branch September 1, 2024 16:22
@BrewTestBot BrewTestBot mentioned this pull request Sep 5, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@charliermarsh charliermarsh charliermarsh approved these changes

Assignees

No one assigned

Labels

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

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Possible false negative on PLR1736

3 participants

@AlexWaygood @sbrugman @charliermarsh