[ruff] Detect duplicate codes as part of unused-noqa (RUF100)#10850
[ruff] Detect duplicate codes as part of unused-noqa (RUF100)#10850charliermarsh merged 12 commits intoastral-sh:mainfrom
ruff] Detect duplicate codes as part of unused-noqa (RUF100)#10850Conversation
|
|
Given the violations in the ecosystem check ( |
|
Hmm yeah... I think it would make sense to match based on rule, but it's confusing that the diagnostic lists the redirected code (and so, presumedly, the one that should stay). I'd say let's do the duplicate detection based on code for now. Seems simpler. |
|
Ok I modified it to use the original code. I also had to modify the list of valid codes, to also use the original code for the fix to work. This has the added effect that redirects will no longer be updated if another rule is violated. But this might be less confusing behavior for users since codes will no longer be updated to new codes without explanation. If we want to detect redirects and lint against them, then the user should probably be notified that they are using a redirect. |
|
Could we add a separate "reason" for redirected codes? |
|
Ya I was just thinking about that. Do you want me to do it in this PR, or a separate one? |
|
I'm cool with either, whichever is easier for you. |
|
Ok I will add it to this one |
|
Should redirects be a seperate rule? Technically |
dd18619 to
5c58c3a
Compare
5c58c3a to
d439da7
Compare
Co-authored-by: Micha Reiser <micha@reiser.io>
Co-authored-by: Micha Reiser <micha@reiser.io>
ruff] Detect duplicate codes as part of unused-noqa (RUF100)ruff] Implement redirected-noqa (RUF101), also detect duplicate codes as part of unused-noqa (RUF100)
|
I'm gonna split this to help merge it |
## Summary Based on discussion in #10850. As it stands today `RUF100` will attempt to replace code redirects with their target codes even though this is not the "goal" of `RUF100`. This behavior is confusing and inconsistent, since code redirects which don't otherwise violate `RUF100` will not be updated. The behavior is also undocumented. Additionally, users who want to use `RUF100` but do not want to update redirects have no way to opt out. This PR explicitly detects redirects with a new rule `RUF101` and patches `RUF100` to keep original codes in fixes and reporting. ## Test Plan Added fixture.
ruff] Implement redirected-noqa (RUF101), also detect duplicate codes as part of unused-noqa (RUF100)ruff] Detect duplicate codes as part of unused-noqa (RUF100)
|
@charliermarsh hopefully this should be an easy merge now 😉 |
Summary
Implement duplicate code detection as part of
RUF100, mirroring the behavior offlake8-noqa(NQA005) mentioned in #850. The idea to merge the rule intoRUF100was suggested by @MichaReiser #10325 (comment).Test Plan
Test cases were added to the fixture.