Skip to content

Commit de46a36

Browse files
augustelalandeaugustelalande
authored
[pygrep-hooks] Improve blanket-noqa error message (PGH004) (#10851)
## Summary Improve `blanket-noqa` error message in cases where codes are provided but not detected due to formatting issues. Namely `# noqa X100` (missing colon) or `noqa : X100` (space before colon). The behavior is similar to `NQA002` and `NQA003` from `flake8-noqa` mentioned in #850. The idea to merge the rules into `PGH004` was suggested by @MichaReiser #10325 (comment). ## Test Plan Test cases added to fixture.
1 parent dbf8d0c commit de46a36

File tree

4 files changed

+207
-7
lines changed

4 files changed

+207
-7
lines changed

‎crates/ruff_linter/resources/test/fixtures/pygrep_hooks/PGH004_0.py

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,3 +9,22 @@
99
x = 1 # noqa: F401, W203
1010
# noqa: F401
1111
# noqa: F401, W203
12+
13+
# OK
14+
x = 2 # noqa: X100
15+
x = 2 # noqa:X100
16+
17+
# PGH004
18+
x = 2 # noqa X100
19+
20+
# PGH004
21+
x = 2 # noqa X100, X200
22+
23+
# PGH004
24+
x = 2 # noqa : X300
25+
26+
# PGH004
27+
x = 2 # noqa : X400
28+
29+
# PGH004
30+
x = 2 # noqa :X500

‎crates/ruff_linter/src/noqa.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ impl<'a> Directive<'a> {
147147

148148
/// Lex an individual rule code (e.g., `F401`).
149149
#[inline]
150-
fn lex_code(line: &str) -> Option<&str> {
150+
pub(crate) fn lex_code(line: &str) -> Option<&str> {
151151
// Extract, e.g., the `F` in `F401`.
152152
let prefix = line.chars().take_while(char::is_ascii_uppercase).count();
153153
// Extract, e.g., the `401` in `F401`.

‎crates/ruff_linter/src/rules/pygrep_hooks/rules/blanket_noqa.rs

Lines changed: 94 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
1-
use ruff_diagnostics::{Diagnostic, Violation};
1+
use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation};
22
use ruff_macros::{derive_message_formats, violation};
33
use ruff_python_index::Indexer;
4+
use ruff_python_trivia::Cursor;
45
use ruff_source_file::Locator;
5-
use ruff_text_size::Ranged;
6+
use ruff_text_size::{Ranged, TextRange, TextSize};
67

78
use crate::noqa::Directive;
89

@@ -27,15 +28,56 @@ use crate::noqa::Directive;
2728
/// from .base import * # noqa: F403
2829
/// ```
2930
///
31+
/// ## Fix safety
32+
/// This rule will attempt to fix blanket `noqa` annotations that appear to
33+
/// be unintentional. For example, given `# noqa F401`, the rule will suggest
34+
/// inserting a colon, as in `# noqa: F401`.
35+
///
36+
/// While modifying `noqa` comments is generally safe, doing so may introduce
37+
/// additional diagnostics.
38+
///
3039
/// ## References
3140
/// - [Ruff documentation](https://docs.astral.sh/ruff/configuration/#error-suppression)
3241
#[violation]
33-
pub struct BlanketNOQA;
42+
pub struct BlanketNOQA {
43+
missing_colon: bool,
44+
space_before_colon: bool,
45+
}
3446

3547
impl Violation for BlanketNOQA {
48+
const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes;
49+
3650
#[derive_message_formats]
3751
fn message(&self) -> String {
38-
format!("Use specific rule codes when using `noqa`")
52+
let BlanketNOQA {
53+
missing_colon,
54+
space_before_colon,
55+
} = self;
56+
57+
// This awkward branching is necessary to ensure that the generic message is picked up by
58+
// `derive_message_formats`.
59+
if !missing_colon && !space_before_colon {
60+
format!("Use specific rule codes when using `noqa`")
61+
} else if *missing_colon {
62+
format!("Use a colon when specifying `noqa` rule codes")
63+
} else {
64+
format!("Do not add spaces between `noqa` and its colon")
65+
}
66+
}
67+
68+
fn fix_title(&self) -> Option<String> {
69+
let BlanketNOQA {
70+
missing_colon,
71+
space_before_colon,
72+
} = self;
73+
74+
if *missing_colon {
75+
Some("Add missing colon".to_string())
76+
} else if *space_before_colon {
77+
Some("Remove space(s) before colon".to_string())
78+
} else {
79+
None
80+
}
3981
}
4082
}
4183

@@ -47,8 +89,54 @@ pub(crate) fn blanket_noqa(
4789
) {
4890
for range in indexer.comment_ranges() {
4991
let line = locator.slice(*range);
50-
if let Ok(Some(Directive::All(all))) = Directive::try_extract(line, range.start()) {
51-
diagnostics.push(Diagnostic::new(BlanketNOQA, all.range()));
92+
let offset = range.start();
93+
if let Ok(Some(Directive::All(all))) = Directive::try_extract(line, TextSize::new(0)) {
94+
// The `all` range is that of the `noqa` directive in, e.g., `# noqa` or `# noqa F401`.
95+
let noqa_start = offset + all.start();
96+
let noqa_end = offset + all.end();
97+
98+
// Skip the `# noqa`, plus any trailing whitespace.
99+
let mut cursor = Cursor::new(&line[all.end().to_usize()..]);
100+
cursor.eat_while(char::is_whitespace);
101+
102+
// Check for extraneous spaces before the colon.
103+
// Ex) `# noqa : F401`
104+
if cursor.first() == ':' {
105+
let start = offset + all.end();
106+
let end = start + cursor.token_len();
107+
let mut diagnostic = Diagnostic::new(
108+
BlanketNOQA {
109+
missing_colon: false,
110+
space_before_colon: true,
111+
},
112+
TextRange::new(noqa_start, end),
113+
);
114+
diagnostic.set_fix(Fix::unsafe_edit(Edit::deletion(start, end)));
115+
diagnostics.push(diagnostic);
116+
} else if Directive::lex_code(cursor.chars().as_str()).is_some() {
117+
// Check for a missing colon.
118+
// Ex) `# noqa F401`
119+
let start = offset + all.end();
120+
let end = start + TextSize::new(1);
121+
let mut diagnostic = Diagnostic::new(
122+
BlanketNOQA {
123+
missing_colon: true,
124+
space_before_colon: false,
125+
},
126+
TextRange::new(noqa_start, end),
127+
);
128+
diagnostic.set_fix(Fix::unsafe_edit(Edit::insertion(':'.to_string(), start)));
129+
diagnostics.push(diagnostic);
130+
} else {
131+
// Otherwise, it looks like an intentional blanket `noqa` annotation.
132+
diagnostics.push(Diagnostic::new(
133+
BlanketNOQA {
134+
missing_colon: false,
135+
space_before_colon: false,
136+
},
137+
TextRange::new(noqa_start, noqa_end),
138+
));
139+
}
52140
}
53141
}
54142
}

‎crates/ruff_linter/src/rules/pygrep_hooks/snapshots/ruff_linter__rules__pygrep_hooks__tests__PGH004_PGH004_0.py.snap

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,4 +29,97 @@ PGH004_0.py:4:1: PGH004 Use specific rule codes when using `noqa`
2929
6 | # noqa:F401,W203
3030
|
3131

32+
PGH004_0.py:18:8: PGH004 [*] Use a colon when specifying `noqa` rule codes
33+
|
34+
17 | # PGH004
35+
18 | x = 2 # noqa X100
36+
| ^^^^^^^ PGH004
37+
19 |
38+
20 | # PGH004
39+
|
40+
= help: Add missing colon
3241

42+
ℹ Unsafe fix
43+
15 15 | x = 2 # noqa:X100
44+
16 16 |
45+
17 17 | # PGH004
46+
18 |-x = 2 # noqa X100
47+
18 |+x = 2 # noqa: X100
48+
19 19 |
49+
20 20 | # PGH004
50+
21 21 | x = 2 # noqa X100, X200
51+
52+
PGH004_0.py:21:8: PGH004 [*] Use a colon when specifying `noqa` rule codes
53+
|
54+
20 | # PGH004
55+
21 | x = 2 # noqa X100, X200
56+
| ^^^^^^^ PGH004
57+
22 |
58+
23 | # PGH004
59+
|
60+
= help: Add missing colon
61+
62+
ℹ Unsafe fix
63+
18 18 | x = 2 # noqa X100
64+
19 19 |
65+
20 20 | # PGH004
66+
21 |-x = 2 # noqa X100, X200
67+
21 |+x = 2 # noqa: X100, X200
68+
22 22 |
69+
23 23 | # PGH004
70+
24 24 | x = 2 # noqa : X300
71+
72+
PGH004_0.py:24:8: PGH004 [*] Do not add spaces between `noqa` and its colon
73+
|
74+
23 | # PGH004
75+
24 | x = 2 # noqa : X300
76+
| ^^^^^^^ PGH004
77+
25 |
78+
26 | # PGH004
79+
|
80+
= help: Remove space(s) before colon
81+
82+
ℹ Unsafe fix
83+
21 21 | x = 2 # noqa X100, X200
84+
22 22 |
85+
23 23 | # PGH004
86+
24 |-x = 2 # noqa : X300
87+
24 |+x = 2 # noqa: X300
88+
25 25 |
89+
26 26 | # PGH004
90+
27 27 | x = 2 # noqa : X400
91+
92+
PGH004_0.py:27:8: PGH004 [*] Do not add spaces between `noqa` and its colon
93+
|
94+
26 | # PGH004
95+
27 | x = 2 # noqa : X400
96+
| ^^^^^^^^ PGH004
97+
28 |
98+
29 | # PGH004
99+
|
100+
= help: Remove space(s) before colon
101+
102+
ℹ Unsafe fix
103+
24 24 | x = 2 # noqa : X300
104+
25 25 |
105+
26 26 | # PGH004
106+
27 |-x = 2 # noqa : X400
107+
27 |+x = 2 # noqa: X400
108+
28 28 |
109+
29 29 | # PGH004
110+
30 30 | x = 2 # noqa :X500
111+
112+
PGH004_0.py:30:8: PGH004 [*] Do not add spaces between `noqa` and its colon
113+
|
114+
29 | # PGH004
115+
30 | x = 2 # noqa :X500
116+
| ^^^^^^^ PGH004
117+
|
118+
= help: Remove space(s) before colon
119+
120+
ℹ Unsafe fix
121+
27 27 | x = 2 # noqa : X400
122+
28 28 |
123+
29 29 | # PGH004
124+
30 |-x = 2 # noqa :X500
125+
30 |+x = 2 # noqa:X500

0 commit comments

Comments
 (0)