Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[ruff] - extend comment deletions for unused-noqa (RUF100) #13105

Merged
merged 2 commits into from
Aug 29, 2024
Merged

[ruff] - extend comment deletions for unused-noqa (RUF100) #13105

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6c005d8
[`ruff`] - extend comment deletions for unused-noqa (`RUF100`)
diceroll123 Aug 26, 2024
e534e29
address PR comments
diceroll123 Aug 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/ruff/RUF100_5.py
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
10 changes: 4 additions & 6 deletions crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__ruf100_3.snap
View file
Open in desktop
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


45 changes: 45 additions & 0 deletions crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__ruf100_5.snap
View file
Open in desktop
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
Loading