Skip to content

Commit a998320

Browse files
diceroll123diceroll123
authored
[ruff] - extend comment deletions for unused-noqa (RUF100) (#13105)
## Summary Extends deletions for RUF100, deleting trailing text from noqa directives, while preserving upcoming comments on the same line if any. In cases where it deletes a comment up to another comment on the same line, the whitespace between them is now shown to be in the autofix in the diagnostic as well. Leading whitespace before the removed comment is not, though. Fixes #12251 ## Test Plan `cargo test`
1 parent 770ef2a commit a998320

5 files changed

Lines changed: 72 additions & 24 deletions

File tree

‎crates/ruff_linter/resources/test/fixtures/ruff/RUF100_5.py‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,3 +9,13 @@
99

1010

1111
#import os # noqa: E501
12+
13+
def f():
14+
data = 1
15+
# line below should autofix to `return data # fmt: skip`
16+
return data # noqa: RET504 # fmt: skip
17+
18+
def f():
19+
data = 1
20+
# line below should autofix to `return data`
21+
return data # noqa: RET504 - intentional incorrect noqa, will be removed

‎crates/ruff_linter/src/checkers/noqa.rs‎

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -118,10 +118,10 @@ pub(crate) fn check_noqa(
118118
match &line.directive {
119119
Directive::All(directive) => {
120120
if line.matches.is_empty() {
121+
let edit = delete_comment(directive.range(), locator);
121122
let mut diagnostic =
122123
Diagnostic::new(UnusedNOQA { codes: None }, directive.range());
123-
diagnostic
124-
.set_fix(Fix::safe_edit(delete_comment(directive.range(), locator)));
124+
diagnostic.set_fix(Fix::safe_edit(edit));
125125

126126
diagnostics.push(diagnostic);
127127
}
@@ -172,6 +172,14 @@ pub(crate) fn check_noqa(
172172
&& unknown_codes.is_empty()
173173
&& unmatched_codes.is_empty())
174174
{
175+
let edit = if valid_codes.is_empty() {
176+
delete_comment(directive.range(), locator)
177+
} else {
178+
Edit::range_replacement(
179+
format!("# noqa: {}", valid_codes.join(", ")),
180+
directive.range(),
181+
)
182+
};
175183
let mut diagnostic = Diagnostic::new(
176184
UnusedNOQA {
177185
codes: Some(UnusedCodes {
@@ -195,17 +203,7 @@ pub(crate) fn check_noqa(
195203
},
196204
directive.range(),
197205
);
198-
if valid_codes.is_empty() {
199-
diagnostic.set_fix(Fix::safe_edit(delete_comment(
200-
directive.range(),
201-
locator,
202-
)));
203-
} else {
204-
diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement(
205-
format!("# noqa: {}", valid_codes.join(", ")),
206-
directive.range(),
207-
)));
208-
}
206+
diagnostic.set_fix(Fix::safe_edit(edit));
209207
diagnostics.push(diagnostic);
210208
}
211209
}

‎crates/ruff_linter/src/fix/edits.rs‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -99,11 +99,8 @@ pub(crate) fn delete_comment(range: TextRange, locator: &Locator) -> Edit {
9999
}
100100
// Ex) `x = 1 # noqa here`
101101
else {
102-
// Replace `# noqa here` with `# here`.
103-
Edit::range_replacement(
104-
"# ".to_string(),
105-
TextRange::new(range.start(), range.end() + trailing_space_len),
106-
)
102+
// Remove `# noqa here` and whitespace
103+
Edit::deletion(range.start() - leading_space_len, line_range.end())
107104
}
108105
}
109106

‎crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__ruf100_3.snap‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ RUF100_3.py:6:10: RUF100 [*] Unused blanket `noqa` directive
112112
4 4 | print() # noqa # comment
113113
5 5 | print() # noqa # comment
114114
6 |-print() # noqa comment
115-
6 |+print() # comment
115+
6 |+print()
116116
7 7 | print() # noqa comment
117117
8 8 | print(a) # noqa
118118
9 9 | print(a) # noqa # comment
@@ -133,7 +133,7 @@ RUF100_3.py:7:10: RUF100 [*] Unused blanket `noqa` directive
133133
5 5 | print() # noqa # comment
134134
6 6 | print() # noqa comment
135135
7 |-print() # noqa comment
136-
7 |+print() # comment
136+
7 |+print()
137137
8 8 | print(a) # noqa
138138
9 9 | print(a) # noqa # comment
139139
10 10 | print(a) # noqa # comment
@@ -257,7 +257,7 @@ RUF100_3.py:19:10: RUF100 [*] Unused `noqa` directive (unused: `E501`, `F821`)
257257
17 17 | print() # noqa: E501, F821 # comment
258258
18 18 | print() # noqa: E501, F821 # comment
259259
19 |-print() # noqa: E501, F821 comment
260-
19 |+print() # comment
260+
19 |+print()
261261
20 20 | print() # noqa: E501, F821 comment
262262
21 21 | print(a) # noqa: E501, F821
263263
22 22 | print(a) # noqa: E501, F821 # comment
@@ -278,7 +278,7 @@ RUF100_3.py:20:10: RUF100 [*] Unused `noqa` directive (unused: `E501`, `F821`)
278278
18 18 | print() # noqa: E501, F821 # comment
279279
19 19 | print() # noqa: E501, F821 comment
280280
20 |-print() # noqa: E501, F821 comment
281-
20 |+print() # comment
281+
20 |+print()
282282
21 21 | print(a) # noqa: E501, F821
283283
22 22 | print(a) # noqa: E501, F821 # comment
284284
23 23 | print(a) # noqa: E501, F821 # comment
@@ -428,5 +428,3 @@ RUF100_3.py:28:39: RUF100 [*] Unused `noqa` directive (unused: `E501`)
428428
27 27 | print(a) # comment with unicode µ # noqa: E501
429429
28 |-print(a) # comment with unicode µ # noqa: E501, F821
430430
28 |+print(a) # comment with unicode µ # noqa: F821
431-
432-

‎crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__ruf100_5.snap‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@ RUF100_5.py:11:1: ERA001 Found commented-out code
2424
|
2525
11 | #import os # noqa: E501
2626
| ^^^^^^^^^^^^^^^^^^^^^^^^ ERA001
27+
12 |
28+
13 | def f():
2729
|
2830
= help: Remove commented-out code
2931

@@ -32,11 +34,16 @@ RUF100_5.py:11:1: ERA001 Found commented-out code
3234
9 9 |
3335
10 10 |
3436
11 |-#import os # noqa: E501
37+
12 11 |
38+
13 12 | def f():
39+
14 13 | data = 1
3540

3641
RUF100_5.py:11:13: RUF100 [*] Unused `noqa` directive (unused: `E501`)
3742
|
3843
11 | #import os # noqa: E501
3944
| ^^^^^^^^^^^^ RUF100
45+
12 |
46+
13 | def f():
4047
|
4148
= help: Remove unused `noqa` directive
4249

@@ -46,5 +53,43 @@ RUF100_5.py:11:13: RUF100 [*] Unused `noqa` directive (unused: `E501`)
4653
10 10 |
4754
11 |-#import os # noqa: E501
4855
11 |+#import os
56+
12 12 |
57+
13 13 | def f():
58+
14 14 | data = 1
4959

60+
RUF100_5.py:16:18: RUF100 [*] Unused `noqa` directive (non-enabled: `RET504`)
61+
|
62+
14 | data = 1
63+
15 | # line below should autofix to `return data # fmt: skip`
64+
16 | return data # noqa: RET504 # fmt: skip
65+
| ^^^^^^^^^^^^^^ RUF100
66+
17 |
67+
18 | def f():
68+
|
69+
= help: Remove unused `noqa` directive
70+
71+
ℹ Safe fix
72+
13 13 | def f():
73+
14 14 | data = 1
74+
15 15 | # line below should autofix to `return data # fmt: skip`
75+
16 |- return data # noqa: RET504 # fmt: skip
76+
16 |+ return data # fmt: skip
77+
17 17 |
78+
18 18 | def f():
79+
19 19 | data = 1
5080

81+
RUF100_5.py:21:18: RUF100 [*] Unused `noqa` directive (non-enabled: `RET504`)
82+
|
83+
19 | data = 1
84+
20 | # line below should autofix to `return data`
85+
21 | return data # noqa: RET504 - intentional incorrect noqa, will be removed
86+
| ^^^^^^^^^^^^^^ RUF100
87+
|
88+
= help: Remove unused `noqa` directive
89+
90+
ℹ Safe fix
91+
18 18 | def f():
92+
19 19 | data = 1
93+
20 20 | # line below should autofix to `return data`
94+
21 |- return data # noqa: RET504 - intentional incorrect noqa, will be removed
95+
21 |+ return data

0 commit comments

Comments
 (0)