[pylint] Implement unnecessary-dict-index-lookup (PLR1733)#8036
[pylint] Implement unnecessary-dict-index-lookup (PLR1733)#8036charliermarsh merged 13 commits intoastral-sh:mainfrom
pylint] Implement unnecessary-dict-index-lookup (PLR1733)#8036Conversation
|
This PR is practically just #7999 in a trenchcoat |
This comment was marked as outdated.
This comment was marked as outdated.
unnecessary_dict_index_lookup (R1733) + autofixunnecessary_dict_index_lookup (PLR1733) + autofix
|
Does this pylint rule cover dict comprehensions? or would that be covered by another rule? Or is it just not covered sadly? |
ooh, that was an oversight, I'll add all comprehensions later, thanks! |
0b2c21d to
a0cd6b1
Compare
|
@charliermarsh Anything blocking these two PRs btw? |
|
@Skylion007 - Just me finding time to review it, there's a lot of logic so I just need to read it closely. |
|
I would add a test case for this bug: #7999 (comment) . I suspect you would hit it with dict comprehensions as well. |
| FRUITS[fruit_name]: int = FRUITS[fruit_name] # Ok | ||
| FRUITS[fruit_name] = FRUITS[fruit_name] # Ok | ||
| FRUITS[fruit_name] += FRUITS[fruit_name] # Ok | ||
| FRUITS[fruit_name] = "d" # Ok |
There was a problem hiding this comment.
What about accessing _ after FRUITS[fruit_name] is edited? that's the main thing missing here.
CodSpeed Performance ReportMerging #8036 will not alter performanceComparing Summary
|
|
| code | total | + violation | - violation | + fix | - fix |
|---|---|---|---|---|---|
| PLR1733 | 3 | 3 | 0 | 0 | 0 |
crates/ruff_linter/src/codes.rs
Outdated
| (Pylint, "R1711") => (RuleGroup::Stable, rules::pylint::rules::UselessReturn), | ||
| (Pylint, "R1714") => (RuleGroup::Stable, rules::pylint::rules::RepeatedEqualityComparison), | ||
| (Pylint, "R1706") => (RuleGroup::Preview, rules::pylint::rules::AndOrTernary), | ||
| (Pylint, "R1733") => (RuleGroup::Preview, rules::pylint::rules::UnnecessaryDictIndexLookup), |
There was a problem hiding this comment.
Shouldn't this be 1 row down to maintain sorted order?
There was a problem hiding this comment.
rebased and fixed!
29e0e45 to
e9dfe6b
Compare
e9dfe6b to
a6a286e
Compare
charliermarsh
left a comment
There was a problem hiding this comment.
@diceroll123 -- Thank you! Do you mind taking a look at some of the tweaks I made in #8932 and the comments I left to explain them, and applying those changes here?
|
Will do, I see you're still adding comments so I'll check back later! |
unnecessary_dict_index_lookup (PLR1733) + autofixpylint] Implement unnecessary-dict-index-lookup (PLR1733)
|
Thank YOU! 😄 |
Summary
Add R1733 and autofix!
See #970
Test Plan
cargo testand manually