[pygrep_hooks] Check blanket ignores via file-level pragmas (PGH004)
#11540
Conversation
|
| code | total | + violation | - violation | + fix | - fix |
|---|---|---|---|---|---|
| PGH004 | 5 | 5 | 0 | 0 | 0 |
|
The stable behavior should be unchanged, the added behavior should only be present in preview. |
|
Thanks for the swift feedback. I added a check for Would you like me to also update the docs for |
| @@ -85,7 +86,19 @@ pub(crate) fn blanket_noqa( | |||
| diagnostics: &mut Vec<Diagnostic>, | |||
| noqa_directives: &NoqaDirectives, | |||
| locator: &Locator, | |||
| exemption: &Option<FileExemption>, | |||
There was a problem hiding this comment.
One downside here is that this is only going to flag the first # ruff: noqa in the file. I assume if you add multiple such comments, only the first is flagged as a diagnostic?
There was a problem hiding this comment.
We might need to do something like NoqaDirectives, whereby we parse all of these upfront, then use the parsed exemptions in crates/ruff_linter/src/checkers/noqa.rs (and here).
There was a problem hiding this comment.
@ajesipow - Do you want to give that a try?
There was a problem hiding this comment.
Thanks for the ping @charliermarsh.
I haven't found the time yet after work this week, but today is a public holiday over here so I'll look into it today :)
There was a problem hiding this comment.
I just pushed the changes and adapted the tests again accordingly.
Looking forward to your feedback again.
charliermarsh
added
rule
| /// The file is exempt from all rules. | ||
| All, | ||
| /// The file is exempt from the given rules. | ||
| Codes(Vec<NoqaCode>), | ||
| Codes(Vec<&'a NoqaCode>), |
There was a problem hiding this comment.
I left this as a Vec, but we could consider using a HashSet instead as we're checking if certain NoqaCodes are contained in a few places (one being the loop on line 54 in checkers/noqa.rs).
It's probably fine though as I'd expect there will usually only be very few elements here and checking the small Vec is likely faster compared to the overhead of hashing every time (and building the HashSet in the first place).
We'd also have to derive Hash on NoqaCode.
|
Thanks @ajesipow, I will take a look. |
charliermarsh
changed the title
pygrep_hooks] Check blanket ignores via file-level pragmas (PGH004)
CodSpeed Performance ReportMerging #11540 will degrade performances by 5.63%Comparing Summary
Benchmarks breakdown
|
* main: CI: add job to run tests under minimum supported rust version (msrv) (#11737) Lexer should consider BOM for the start offset (#11732) Use cursor offset for lexer checkpoint (#11734) red-knot: Change `resolve_global_symbol` to take `Module` as an argument (#11723) red-knot: Use `parse_unchecked` to get all parse errors (#11725) Respect per-file ignores for blanket and redirected noqa rules (#11728) [`pygrep_hooks`] Check blanket ignores via file-level pragmas (`PGH004`) (#11540)
Summary
Should resolve #11454.
This is my first PR to
ruff, so I may have missed something.If I understood the suggestion in the issue correctly, rule
PGH004should be set toPreviewagain.Test Plan
Created two fixtures derived from the issue.