-
Notifications
You must be signed in to change notification settings - Fork 1.3k
[flake8-pyi
] Implement PYI062
(duplicate-literal-member
)
#11269
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
||
x: Literal[True, False, True, False] # PYI062 twice here | ||
|
||
y: Literal[1, print("hello"), 3, Literal[4, 1]] # PYI062 on the last 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add test cases where the inner literal is entirely redundant? e.g.
Literal[1, Literal[1]]
Literal[1, 2, Literal[1, 2]]
Literal[1, Literal[1], Literal[1]]
Literal[1, Literal[2], Literal[2]]
Literal[1, Literal[2, Literal[1]]]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These will be more important for when we add a fix, I think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's also good to have coverage for cases like Literal[1, 1, 1]
where we should raise the diagnostic twice.
|
code | total | + violation | - violation | + fix | - fix |
---|---|---|---|---|---|
PYI062 | 15 | 15 | 0 | 0 | 0 |
Hm the typeshed ecosystem checks look concerning, do you know what's going on there? |
@zanieb Doesn't seem to reproduce locally at least with copypasting the concerned files :/ |
@tusharsadhwani can you try rebasing your PR on the latest |
I think something might be up with the ecosystem checks generally when it comes to typeshed. I've had several odd results on some of my other PRs that have touched |
Can confirm, merging main doesn't fix the issue. |
Oh, I see. @AlexWaygood Can you open a new issue to keep track of it? No need to go into detail there but if you're unable to look into it we're still aware of it. |
Done: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
crates/ruff_linter/src/rules/flake8_pyi/rules/duplicate_literal_member.rs
Outdated
Show resolved
Hide resolved
crates/ruff_linter/src/rules/flake8_pyi/rules/duplicate_literal_member.rs
Outdated
Show resolved
Hide resolved
crates/ruff_linter/src/rules/flake8_pyi/rules/duplicate_literal_member.rs
Outdated
Show resolved
Hide resolved
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
I double-checked again, and I cannot reproduce the ecosystem results locally on my clone of typeshed. I checked out this PR branch locally in my ruff clone, and then ran this command from the directory of my typeshed clone, with the following results
I also confirmed that the same command did highlight duplicate literal members if I purposefully introduced some to a random |
flake8-pyi
] Implement PYI062
(duplicate-literal-member
)
Summary
Implements
Y062
fromflake8-pyi
, mostly adopted fromRule::DuplicateUnionMember
(without the|
BinOp parts).Currently
DuplicateUnionMember
also doesn't have an autofix forUnion[]
cases, so implementing both together seems like a good idea.Test Plan
cargo test
/cargo insta review