Consolidate diagnostics for matched disable/enable suppression comments #22099
Conversation
amyreese
added
rule
|
MichaReiser
left a comment
MichaReiser
left a comment
There was a problem hiding this comment.
I've a few nit comments and a suggestion on how to encode the invariant that a suppression always has at least a disable comment.
I also think we should delete the _full test. It's not clear what value it adds over the existing test
| 97 | # ruff: enable[YF829] | ||
| | ----- | ||
| | | ||
| help: Remove the rule code |
There was a problem hiding this comment.
I think the help text here is a bit misleading. It should probably be: Remove the invalid suppression
| } | ||
| } | ||
|
|
||
| fn delete_code_or_comment( |
There was a problem hiding this comment.
Let's create an issue to track fixing delete_code_or_comment to correctly handle the cases where a suppression comment contains repeated codes (in which case code_index will always delete the first unused code but never the second).
The unwrap call in delete_code_or_comment also deserves a commetn why it is okay to call unwrap there OR, even better, use expect
amyreese
force-pushed
the
amy/group-diagnostics
branch
from
0f38751 to
e067aa0
Compare
Summary
Combines diagnostics for matched suppression comments, so that ranges and autofixes for both
the
#ruff:disableand#ruff:enablecomments will be reported as a single diagnostic.Test Plan
Snapshot changes, added new snapshot for full output from preview mode rather than just a diff.
Issue #3711