Skip to content

Commit

Permalink
[ruff] - extend comment deletions for unused-noqa (RUF100) (#13105)
Browse files Browse the repository at this point in the history
## 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`
  • Loading branch information
diceroll123 authored Aug 29, 2024
1 parent 770ef2a commit a998320
Show file tree
Hide file tree
Showing 5 changed files with 72 additions and 24 deletions.
10 changes: 10 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/ruff/RUF100_5.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,13 @@


#import os # noqa: E501

def f():
data = 1
# line below should autofix to `return data # fmt: skip`
return data # noqa: RET504 # fmt: skip

def f():
data = 1
# line below should autofix to `return data`
return data # noqa: RET504 - intentional incorrect noqa, will be removed
24 changes: 11 additions & 13 deletions crates/ruff_linter/src/checkers/noqa.rs
Original file line number Diff line number Diff line change
Expand Up @@ -118,10 +118,10 @@ pub(crate) fn check_noqa(
match &line.directive {
Directive::All(directive) => {
if line.matches.is_empty() {
let edit = delete_comment(directive.range(), locator);
let mut diagnostic =
Diagnostic::new(UnusedNOQA { codes: None }, directive.range());
diagnostic
.set_fix(Fix::safe_edit(delete_comment(directive.range(), locator)));
diagnostic.set_fix(Fix::safe_edit(edit));

diagnostics.push(diagnostic);
}
Expand Down Expand Up @@ -172,6 +172,14 @@ pub(crate) fn check_noqa(
&& unknown_codes.is_empty()
&& unmatched_codes.is_empty())
{
let edit = if valid_codes.is_empty() {
delete_comment(directive.range(), locator)
} else {
Edit::range_replacement(
format!("# noqa: {}", valid_codes.join(", ")),
directive.range(),
)
};
let mut diagnostic = Diagnostic::new(
UnusedNOQA {
codes: Some(UnusedCodes {
Expand All @@ -195,17 +203,7 @@ pub(crate) fn check_noqa(
},
directive.range(),
);
if valid_codes.is_empty() {
diagnostic.set_fix(Fix::safe_edit(delete_comment(
directive.range(),
locator,
)));
} else {
diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement(
format!("# noqa: {}", valid_codes.join(", ")),
directive.range(),
)));
}
diagnostic.set_fix(Fix::safe_edit(edit));
diagnostics.push(diagnostic);
}
}
Expand Down
7 changes: 2 additions & 5 deletions crates/ruff_linter/src/fix/edits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,11 +99,8 @@ pub(crate) fn delete_comment(range: TextRange, locator: &Locator) -> Edit {
}
// Ex) `x = 1 # noqa here`
else {
// Replace `# noqa here` with `# here`.
Edit::range_replacement(
"# ".to_string(),
TextRange::new(range.start(), range.end() + trailing_space_len),
)
// Remove `# noqa here` and whitespace
Edit::deletion(range.start() - leading_space_len, line_range.end())
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ RUF100_3.py:6:10: RUF100 [*] Unused blanket `noqa` directive
4 4 | print() # noqa # comment
5 5 | print() # noqa # comment
6 |-print() # noqa comment
6 |+print() # comment
6 |+print()
7 7 | print() # noqa comment
8 8 | print(a) # noqa
9 9 | print(a) # noqa # comment
Expand All @@ -133,7 +133,7 @@ RUF100_3.py:7:10: RUF100 [*] Unused blanket `noqa` directive
5 5 | print() # noqa # comment
6 6 | print() # noqa comment
7 |-print() # noqa comment
7 |+print() # comment
7 |+print()
8 8 | print(a) # noqa
9 9 | print(a) # noqa # comment
10 10 | print(a) # noqa # comment
Expand Down Expand Up @@ -257,7 +257,7 @@ RUF100_3.py:19:10: RUF100 [*] Unused `noqa` directive (unused: `E501`, `F821`)
17 17 | print() # noqa: E501, F821 # comment
18 18 | print() # noqa: E501, F821 # comment
19 |-print() # noqa: E501, F821 comment
19 |+print() # comment
19 |+print()
20 20 | print() # noqa: E501, F821 comment
21 21 | print(a) # noqa: E501, F821
22 22 | print(a) # noqa: E501, F821 # comment
Expand All @@ -278,7 +278,7 @@ RUF100_3.py:20:10: RUF100 [*] Unused `noqa` directive (unused: `E501`, `F821`)
18 18 | print() # noqa: E501, F821 # comment
19 19 | print() # noqa: E501, F821 comment
20 |-print() # noqa: E501, F821 comment
20 |+print() # comment
20 |+print()
21 21 | print(a) # noqa: E501, F821
22 22 | print(a) # noqa: E501, F821 # comment
23 23 | print(a) # noqa: E501, F821 # comment
Expand Down Expand Up @@ -428,5 +428,3 @@ RUF100_3.py:28:39: RUF100 [*] Unused `noqa` directive (unused: `E501`)
27 27 | print(a) # comment with unicode µ # noqa: E501
28 |-print(a) # comment with unicode µ # noqa: E501, F821
28 |+print(a) # comment with unicode µ # noqa: F821


Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ RUF100_5.py:11:1: ERA001 Found commented-out code
|
11 | #import os # noqa: E501
| ^^^^^^^^^^^^^^^^^^^^^^^^ ERA001
12 |
13 | def f():
|
= help: Remove commented-out code

Expand All @@ -32,11 +34,16 @@ RUF100_5.py:11:1: ERA001 Found commented-out code
9 9 |
10 10 |
11 |-#import os # noqa: E501
12 11 |
13 12 | def f():
14 13 | data = 1

RUF100_5.py:11:13: RUF100 [*] Unused `noqa` directive (unused: `E501`)
|
11 | #import os # noqa: E501
| ^^^^^^^^^^^^ RUF100
12 |
13 | def f():
|
= help: Remove unused `noqa` directive

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

RUF100_5.py:16:18: RUF100 [*] Unused `noqa` directive (non-enabled: `RET504`)
|
14 | data = 1
15 | # line below should autofix to `return data # fmt: skip`
16 | return data # noqa: RET504 # fmt: skip
| ^^^^^^^^^^^^^^ RUF100
17 |
18 | def f():
|
= help: Remove unused `noqa` directive

Safe fix
13 13 | def f():
14 14 | data = 1
15 15 | # line below should autofix to `return data # fmt: skip`
16 |- return data # noqa: RET504 # fmt: skip
16 |+ return data # fmt: skip
17 17 |
18 18 | def f():
19 19 | data = 1

RUF100_5.py:21:18: RUF100 [*] Unused `noqa` directive (non-enabled: `RET504`)
|
19 | data = 1
20 | # line below should autofix to `return data`
21 | return data # noqa: RET504 - intentional incorrect noqa, will be removed
| ^^^^^^^^^^^^^^ RUF100
|
= help: Remove unused `noqa` directive

Safe fix
18 18 | def f():
19 19 | data = 1
20 20 | # line below should autofix to `return data`
21 |- return data # noqa: RET504 - intentional incorrect noqa, will be removed
21 |+ return data

0 comments on commit a998320

Please sign in to comment.