[`ruff`] Detect redirected-noqa in file-level comments (`RUF101`) (#14635)

This commit is contained in:
Brent Westbrook 2024-11-27 12:25:47 -05:00 committed by GitHub
parent c40b37aa36
commit 6fcbe8efb4
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
16 changed files with 173 additions and 58 deletions

View File

@ -0,0 +1,13 @@
"""Regression test for #14531.
RUF101 should trigger here because the TCH rules have been recoded to TC.
"""
# ruff: noqa: TCH002
from __future__ import annotations
import local_module
def func(sized: local_module.Container) -> int:
return len(sized)

View File

@ -211,6 +211,7 @@ pub(crate) fn check_noqa(
&& !exemption.includes(Rule::RedirectedNOQA) && !exemption.includes(Rule::RedirectedNOQA)
{ {
ruff::rules::redirected_noqa(diagnostics, &noqa_directives); ruff::rules::redirected_noqa(diagnostics, &noqa_directives);
ruff::rules::redirected_file_noqa(diagnostics, &file_noqa_directives);
} }
if settings.rules.enabled(Rule::BlanketNOQA) if settings.rules.enabled(Rule::BlanketNOQA)

View File

@ -319,7 +319,7 @@ impl<'a> From<&'a FileNoqaDirectives<'a>> for FileExemption<'a> {
if directives if directives
.lines() .lines()
.iter() .iter()
.any(|line| ParsedFileExemption::All == line.parsed_file_exemption) .any(|line| matches!(line.parsed_file_exemption, ParsedFileExemption::All))
{ {
FileExemption::All(codes) FileExemption::All(codes)
} else { } else {
@ -362,7 +362,7 @@ impl<'a> FileNoqaDirectives<'a> {
let mut lines = vec![]; let mut lines = vec![];
for range in comment_ranges { for range in comment_ranges {
match ParsedFileExemption::try_extract(&locator.contents()[range]) { match ParsedFileExemption::try_extract(range, locator.contents()) {
Err(err) => { Err(err) => {
#[allow(deprecated)] #[allow(deprecated)]
let line = locator.compute_line_index(range.start()); let line = locator.compute_line_index(range.start());
@ -384,6 +384,7 @@ impl<'a> FileNoqaDirectives<'a> {
} }
ParsedFileExemption::Codes(codes) => { ParsedFileExemption::Codes(codes) => {
codes.iter().filter_map(|code| { codes.iter().filter_map(|code| {
let code = code.as_str();
// Ignore externally-defined rules. // Ignore externally-defined rules.
if external.iter().any(|external| code.starts_with(external)) { if external.iter().any(|external| code.starts_with(external)) {
return None; return None;
@ -424,21 +425,26 @@ impl<'a> FileNoqaDirectives<'a> {
/// An individual file-level exemption (e.g., `# ruff: noqa` or `# ruff: noqa: F401, F841`). Like /// An individual file-level exemption (e.g., `# ruff: noqa` or `# ruff: noqa: F401, F841`). Like
/// [`FileNoqaDirectives`], but only for a single line, as opposed to an aggregated set of exemptions /// [`FileNoqaDirectives`], but only for a single line, as opposed to an aggregated set of exemptions
/// across a source file. /// across a source file.
#[derive(Debug, PartialEq, Eq)] #[derive(Debug)]
pub(crate) enum ParsedFileExemption<'a> { pub(crate) enum ParsedFileExemption<'a> {
/// The file-level exemption ignores all rules (e.g., `# ruff: noqa`). /// The file-level exemption ignores all rules (e.g., `# ruff: noqa`).
All, All,
/// The file-level exemption ignores specific rules (e.g., `# ruff: noqa: F401, F841`). /// The file-level exemption ignores specific rules (e.g., `# ruff: noqa: F401, F841`).
Codes(Vec<&'a str>), Codes(Codes<'a>),
} }
impl<'a> ParsedFileExemption<'a> { impl<'a> ParsedFileExemption<'a> {
/// Return a [`ParsedFileExemption`] for a given comment line. /// Return a [`ParsedFileExemption`] for a given `comment_range` in `source`.
fn try_extract(line: &'a str) -> Result<Option<Self>, ParseError> { fn try_extract(comment_range: TextRange, source: &'a str) -> Result<Option<Self>, ParseError> {
let line = &source[comment_range];
let offset = comment_range.start();
let init_line_len = line.text_len();
let line = Self::lex_whitespace(line); let line = Self::lex_whitespace(line);
let Some(line) = Self::lex_char(line, '#') else { let Some(line) = Self::lex_char(line, '#') else {
return Ok(None); return Ok(None);
}; };
let comment_start = init_line_len - line.text_len() - '#'.text_len();
let line = Self::lex_whitespace(line); let line = Self::lex_whitespace(line);
let Some(line) = Self::lex_flake8(line).or_else(|| Self::lex_ruff(line)) else { let Some(line) = Self::lex_flake8(line).or_else(|| Self::lex_ruff(line)) else {
@ -469,7 +475,11 @@ impl<'a> ParsedFileExemption<'a> {
let mut codes = vec![]; let mut codes = vec![];
let mut line = line; let mut line = line;
while let Some(code) = Self::lex_code(line) { while let Some(code) = Self::lex_code(line) {
codes.push(code); let codes_end = init_line_len - line.text_len();
codes.push(Code {
code,
range: TextRange::at(codes_end, code.text_len()).add(offset),
});
line = &line[code.len()..]; line = &line[code.len()..];
// Codes can be comma- or whitespace-delimited. // Codes can be comma- or whitespace-delimited.
@ -485,7 +495,12 @@ impl<'a> ParsedFileExemption<'a> {
return Err(ParseError::MissingCodes); return Err(ParseError::MissingCodes);
} }
Self::Codes(codes) let codes_end = init_line_len - line.text_len();
let range = TextRange::new(comment_start, codes_end);
Self::Codes(Codes {
range: range.add(offset),
codes,
})
})) }))
} }
@ -1059,7 +1074,7 @@ mod tests {
use ruff_diagnostics::{Diagnostic, Edit}; use ruff_diagnostics::{Diagnostic, Edit};
use ruff_python_trivia::CommentRanges; use ruff_python_trivia::CommentRanges;
use ruff_source_file::LineEnding; use ruff_source_file::LineEnding;
use ruff_text_size::{TextRange, TextSize}; use ruff_text_size::{TextLen, TextRange, TextSize};
use crate::noqa::{add_noqa_inner, Directive, NoqaMapping, ParsedFileExemption}; use crate::noqa::{add_noqa_inner, Directive, NoqaMapping, ParsedFileExemption};
use crate::rules::pycodestyle::rules::{AmbiguousVariableName, UselessSemicolon}; use crate::rules::pycodestyle::rules::{AmbiguousVariableName, UselessSemicolon};
@ -1226,50 +1241,74 @@ mod tests {
#[test] #[test]
fn flake8_exemption_all() { fn flake8_exemption_all() {
let source = "# flake8: noqa"; let source = "# flake8: noqa";
assert_debug_snapshot!(ParsedFileExemption::try_extract(source)); assert_debug_snapshot!(ParsedFileExemption::try_extract(
TextRange::up_to(source.text_len()),
source,
));
} }
#[test] #[test]
fn ruff_exemption_all() { fn ruff_exemption_all() {
let source = "# ruff: noqa"; let source = "# ruff: noqa";
assert_debug_snapshot!(ParsedFileExemption::try_extract(source)); assert_debug_snapshot!(ParsedFileExemption::try_extract(
TextRange::up_to(source.text_len()),
source,
));
} }
#[test] #[test]
fn flake8_exemption_all_no_space() { fn flake8_exemption_all_no_space() {
let source = "#flake8:noqa"; let source = "#flake8:noqa";
assert_debug_snapshot!(ParsedFileExemption::try_extract(source)); assert_debug_snapshot!(ParsedFileExemption::try_extract(
TextRange::up_to(source.text_len()),
source,
));
} }
#[test] #[test]
fn ruff_exemption_all_no_space() { fn ruff_exemption_all_no_space() {
let source = "#ruff:noqa"; let source = "#ruff:noqa";
assert_debug_snapshot!(ParsedFileExemption::try_extract(source)); assert_debug_snapshot!(ParsedFileExemption::try_extract(
TextRange::up_to(source.text_len()),
source,
));
} }
#[test] #[test]
fn flake8_exemption_codes() { fn flake8_exemption_codes() {
// Note: Flake8 doesn't support this; it's treated as a blanket exemption. // Note: Flake8 doesn't support this; it's treated as a blanket exemption.
let source = "# flake8: noqa: F401, F841"; let source = "# flake8: noqa: F401, F841";
assert_debug_snapshot!(ParsedFileExemption::try_extract(source)); assert_debug_snapshot!(ParsedFileExemption::try_extract(
TextRange::up_to(source.text_len()),
source,
));
} }
#[test] #[test]
fn ruff_exemption_codes() { fn ruff_exemption_codes() {
let source = "# ruff: noqa: F401, F841"; let source = "# ruff: noqa: F401, F841";
assert_debug_snapshot!(ParsedFileExemption::try_extract(source)); assert_debug_snapshot!(ParsedFileExemption::try_extract(
TextRange::up_to(source.text_len()),
source,
));
} }
#[test] #[test]
fn flake8_exemption_all_case_insensitive() { fn flake8_exemption_all_case_insensitive() {
let source = "# flake8: NoQa"; let source = "# flake8: NoQa";
assert_debug_snapshot!(ParsedFileExemption::try_extract(source)); assert_debug_snapshot!(ParsedFileExemption::try_extract(
TextRange::up_to(source.text_len()),
source,
));
} }
#[test] #[test]
fn ruff_exemption_all_case_insensitive() { fn ruff_exemption_all_case_insensitive() {
let source = "# ruff: NoQa"; let source = "# ruff: NoQa";
assert_debug_snapshot!(ParsedFileExemption::try_extract(source)); assert_debug_snapshot!(ParsedFileExemption::try_extract(
TextRange::up_to(source.text_len()),
source,
));
} }
#[test] #[test]

View File

@ -60,7 +60,8 @@ mod tests {
#[test_case(Rule::IncorrectlyParenthesizedTupleInSubscript, Path::new("RUF031.py"))] #[test_case(Rule::IncorrectlyParenthesizedTupleInSubscript, Path::new("RUF031.py"))]
#[test_case(Rule::DecimalFromFloatLiteral, Path::new("RUF032.py"))] #[test_case(Rule::DecimalFromFloatLiteral, Path::new("RUF032.py"))]
#[test_case(Rule::UselessIfElse, Path::new("RUF034.py"))] #[test_case(Rule::UselessIfElse, Path::new("RUF034.py"))]
#[test_case(Rule::RedirectedNOQA, Path::new("RUF101.py"))] #[test_case(Rule::RedirectedNOQA, Path::new("RUF101_0.py"))]
#[test_case(Rule::RedirectedNOQA, Path::new("RUF101_1.py"))]
#[test_case(Rule::PostInitDefault, Path::new("RUF033.py"))] #[test_case(Rule::PostInitDefault, Path::new("RUF033.py"))]
#[test_case(Rule::NoneNotAtEndOfUnion, Path::new("RUF036.py"))] #[test_case(Rule::NoneNotAtEndOfUnion, Path::new("RUF036.py"))]
#[test_case(Rule::NoneNotAtEndOfUnion, Path::new("RUF036.pyi"))] #[test_case(Rule::NoneNotAtEndOfUnion, Path::new("RUF036.pyi"))]

View File

@ -2,7 +2,7 @@ use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix};
use ruff_macros::{derive_message_formats, ViolationMetadata}; use ruff_macros::{derive_message_formats, ViolationMetadata};
use ruff_text_size::Ranged; use ruff_text_size::Ranged;
use crate::noqa::{Directive, NoqaDirectives}; use crate::noqa::{Codes, Directive, FileNoqaDirectives, NoqaDirectives, ParsedFileExemption};
use crate::rule_redirects::get_redirect_target; use crate::rule_redirects::get_redirect_target;
/// ## What it does /// ## What it does
@ -42,28 +42,47 @@ impl AlwaysFixableViolation for RedirectedNOQA {
} }
} }
/// RUF101 /// RUF101 for in-line noqa directives
pub(crate) fn redirected_noqa(diagnostics: &mut Vec<Diagnostic>, noqa_directives: &NoqaDirectives) { pub(crate) fn redirected_noqa(diagnostics: &mut Vec<Diagnostic>, noqa_directives: &NoqaDirectives) {
for line in noqa_directives.lines() { for line in noqa_directives.lines() {
let Directive::Codes(directive) = &line.directive else { let Directive::Codes(directive) = &line.directive else {
continue; continue;
}; };
for code in directive.iter() { build_diagnostics(diagnostics, directive);
if let Some(redirected) = get_redirect_target(code.as_str()) { }
let mut diagnostic = Diagnostic::new( }
RedirectedNOQA {
original: code.to_string(), /// RUF101 for file noqa directives
target: redirected.to_string(), pub(crate) fn redirected_file_noqa(
}, diagnostics: &mut Vec<Diagnostic>,
code.range(), noqa_directives: &FileNoqaDirectives,
); ) {
diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement( for line in noqa_directives.lines() {
redirected.to_string(), let ParsedFileExemption::Codes(codes) = &line.parsed_file_exemption else {
code.range(), continue;
))); };
diagnostics.push(diagnostic);
} build_diagnostics(diagnostics, codes);
}
}
/// Convert a sequence of [Codes] into [Diagnostic]s and append them to `diagnostics`.
fn build_diagnostics(diagnostics: &mut Vec<Diagnostic>, codes: &Codes<'_>) {
for code in codes.iter() {
if let Some(redirected) = get_redirect_target(code.as_str()) {
let mut diagnostic = Diagnostic::new(
RedirectedNOQA {
original: code.to_string(),
target: redirected.to_string(),
},
code.range(),
);
diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement(
redirected.to_string(),
code.range(),
)));
diagnostics.push(diagnostic);
} }
} }
} }

View File

@ -2,7 +2,7 @@
source: crates/ruff_linter/src/rules/ruff/mod.rs source: crates/ruff_linter/src/rules/ruff/mod.rs
snapshot_kind: text snapshot_kind: text
--- ---
RUF101.py:1:15: RUF101 [*] `RUF940` is a redirect to `RUF950` RUF101_0.py:1:15: RUF101 [*] `RUF940` is a redirect to `RUF950`
| |
1 | x = 2 # noqa: RUF940 1 | x = 2 # noqa: RUF940
| ^^^^^^ RUF101 | ^^^^^^ RUF101
@ -19,7 +19,7 @@ RUF101.py:1:15: RUF101 [*] `RUF940` is a redirect to `RUF950`
4 4 | x = 2 # noqa: RUF950, RUF940, RUF950, RUF950, RUF950 4 4 | x = 2 # noqa: RUF950, RUF940, RUF950, RUF950, RUF950
5 5 | x = 2 # noqa: RUF940, RUF950, RUF940 5 5 | x = 2 # noqa: RUF940, RUF950, RUF940
RUF101.py:3:15: RUF101 [*] `RUF940` is a redirect to `RUF950` RUF101_0.py:3:15: RUF101 [*] `RUF940` is a redirect to `RUF950`
| |
1 | x = 2 # noqa: RUF940 1 | x = 2 # noqa: RUF940
2 | x = 2 # noqa: RUF950 2 | x = 2 # noqa: RUF950
@ -39,7 +39,7 @@ RUF101.py:3:15: RUF101 [*] `RUF940` is a redirect to `RUF950`
5 5 | x = 2 # noqa: RUF940, RUF950, RUF940 5 5 | x = 2 # noqa: RUF940, RUF950, RUF940
6 6 | x = 2 # noqa: RUF940, RUF950, RUF940, RUF950 6 6 | x = 2 # noqa: RUF940, RUF950, RUF940, RUF950
RUF101.py:4:23: RUF101 [*] `RUF940` is a redirect to `RUF950` RUF101_0.py:4:23: RUF101 [*] `RUF940` is a redirect to `RUF950`
| |
2 | x = 2 # noqa: RUF950 2 | x = 2 # noqa: RUF950
3 | x = 2 # noqa: RUF940, RUF950 3 | x = 2 # noqa: RUF940, RUF950
@ -59,7 +59,7 @@ RUF101.py:4:23: RUF101 [*] `RUF940` is a redirect to `RUF950`
5 5 | x = 2 # noqa: RUF940, RUF950, RUF940 5 5 | x = 2 # noqa: RUF940, RUF950, RUF940
6 6 | x = 2 # noqa: RUF940, RUF950, RUF940, RUF950 6 6 | x = 2 # noqa: RUF940, RUF950, RUF940, RUF950
RUF101.py:5:15: RUF101 [*] `RUF940` is a redirect to `RUF950` RUF101_0.py:5:15: RUF101 [*] `RUF940` is a redirect to `RUF950`
| |
3 | x = 2 # noqa: RUF940, RUF950 3 | x = 2 # noqa: RUF940, RUF950
4 | x = 2 # noqa: RUF950, RUF940, RUF950, RUF950, RUF950 4 | x = 2 # noqa: RUF950, RUF940, RUF950, RUF950, RUF950
@ -77,7 +77,7 @@ RUF101.py:5:15: RUF101 [*] `RUF940` is a redirect to `RUF950`
5 |+x = 2 # noqa: RUF950, RUF950, RUF940 5 |+x = 2 # noqa: RUF950, RUF950, RUF940
6 6 | x = 2 # noqa: RUF940, RUF950, RUF940, RUF950 6 6 | x = 2 # noqa: RUF940, RUF950, RUF940, RUF950
RUF101.py:5:31: RUF101 [*] `RUF940` is a redirect to `RUF950` RUF101_0.py:5:31: RUF101 [*] `RUF940` is a redirect to `RUF950`
| |
3 | x = 2 # noqa: RUF940, RUF950 3 | x = 2 # noqa: RUF940, RUF950
4 | x = 2 # noqa: RUF950, RUF940, RUF950, RUF950, RUF950 4 | x = 2 # noqa: RUF950, RUF940, RUF950, RUF950, RUF950
@ -95,7 +95,7 @@ RUF101.py:5:31: RUF101 [*] `RUF940` is a redirect to `RUF950`
5 |+x = 2 # noqa: RUF940, RUF950, RUF950 5 |+x = 2 # noqa: RUF940, RUF950, RUF950
6 6 | x = 2 # noqa: RUF940, RUF950, RUF940, RUF950 6 6 | x = 2 # noqa: RUF940, RUF950, RUF940, RUF950
RUF101.py:6:15: RUF101 [*] `RUF940` is a redirect to `RUF950` RUF101_0.py:6:15: RUF101 [*] `RUF940` is a redirect to `RUF950`
| |
4 | x = 2 # noqa: RUF950, RUF940, RUF950, RUF950, RUF950 4 | x = 2 # noqa: RUF950, RUF940, RUF950, RUF950, RUF950
5 | x = 2 # noqa: RUF940, RUF950, RUF940 5 | x = 2 # noqa: RUF940, RUF950, RUF940
@ -111,7 +111,7 @@ RUF101.py:6:15: RUF101 [*] `RUF940` is a redirect to `RUF950`
6 |-x = 2 # noqa: RUF940, RUF950, RUF940, RUF950 6 |-x = 2 # noqa: RUF940, RUF950, RUF940, RUF950
6 |+x = 2 # noqa: RUF950, RUF950, RUF940, RUF950 6 |+x = 2 # noqa: RUF950, RUF950, RUF940, RUF950
RUF101.py:6:31: RUF101 [*] `RUF940` is a redirect to `RUF950` RUF101_0.py:6:31: RUF101 [*] `RUF940` is a redirect to `RUF950`
| |
4 | x = 2 # noqa: RUF950, RUF940, RUF950, RUF950, RUF950 4 | x = 2 # noqa: RUF950, RUF940, RUF950, RUF950, RUF950
5 | x = 2 # noqa: RUF940, RUF950, RUF940 5 | x = 2 # noqa: RUF940, RUF950, RUF940

View File

@ -0,0 +1,24 @@
---
source: crates/ruff_linter/src/rules/ruff/mod.rs
snapshot_kind: text
---
RUF101_1.py:5:15: RUF101 [*] `TCH002` is a redirect to `TC002`
|
3 | RUF101 should trigger here because the TCH rules have been recoded to TC.
4 | """
5 | # ruff: noqa: TCH002
| ^^^^^^ RUF101
6 |
7 | from __future__ import annotations
|
= help: Replace with `TC002`
Safe fix
2 2 |
3 3 | RUF101 should trigger here because the TCH rules have been recoded to TC.
4 4 | """
5 |-# ruff: noqa: TCH002
5 |+# ruff: noqa: TC002
6 6 |
7 7 | from __future__ import annotations
8 8 |

View File

@ -1,6 +1,6 @@
--- ---
source: crates/ruff_linter/src/noqa.rs source: crates/ruff_linter/src/noqa.rs
expression: "ParsedFileExemption::try_extract(source)" expression: "ParsedFileExemption::try_extract(TextRange::up_to(source.text_len()), source,)"
snapshot_kind: text snapshot_kind: text
--- ---
Ok( Ok(

View File

@ -1,6 +1,6 @@
--- ---
source: crates/ruff_linter/src/noqa.rs source: crates/ruff_linter/src/noqa.rs
expression: "ParsedFileExemption::try_extract(source)" expression: "ParsedFileExemption::try_extract(TextRange::up_to(source.text_len()), source,)"
snapshot_kind: text snapshot_kind: text
--- ---
Ok( Ok(

View File

@ -1,6 +1,6 @@
--- ---
source: crates/ruff_linter/src/noqa.rs source: crates/ruff_linter/src/noqa.rs
expression: "ParsedFileExemption::try_extract(source)" expression: "ParsedFileExemption::try_extract(TextRange::up_to(source.text_len()), source,)"
snapshot_kind: text snapshot_kind: text
--- ---
Ok( Ok(

View File

@ -1,15 +1,24 @@
--- ---
source: crates/ruff_linter/src/noqa.rs source: crates/ruff_linter/src/noqa.rs
expression: "ParsedFileExemption::try_extract(source)" expression: "ParsedFileExemption::try_extract(TextRange::up_to(source.text_len()), source,)"
snapshot_kind: text snapshot_kind: text
--- ---
Ok( Ok(
Some( Some(
Codes( Codes(
[ Codes {
"F401", range: 0..26,
"F841", codes: [
], Code {
code: "F401",
range: 16..20,
},
Code {
code: "F841",
range: 22..26,
},
],
},
), ),
), ),
) )

View File

@ -1,6 +1,6 @@
--- ---
source: crates/ruff_linter/src/noqa.rs source: crates/ruff_linter/src/noqa.rs
expression: "ParsedFileExemption::try_extract(source)" expression: "ParsedFileExemption::try_extract(TextRange::up_to(source.text_len()), source,)"
snapshot_kind: text snapshot_kind: text
--- ---
Ok( Ok(

View File

@ -1,6 +1,6 @@
--- ---
source: crates/ruff_linter/src/noqa.rs source: crates/ruff_linter/src/noqa.rs
expression: "ParsedFileExemption::try_extract(source)" expression: "ParsedFileExemption::try_extract(TextRange::up_to(source.text_len()), source,)"
snapshot_kind: text snapshot_kind: text
--- ---
Ok( Ok(

View File

@ -1,6 +1,6 @@
--- ---
source: crates/ruff_linter/src/noqa.rs source: crates/ruff_linter/src/noqa.rs
expression: "ParsedFileExemption::try_extract(source)" expression: "ParsedFileExemption::try_extract(TextRange::up_to(source.text_len()), source,)"
snapshot_kind: text snapshot_kind: text
--- ---
Ok( Ok(

View File

@ -1,15 +1,24 @@
--- ---
source: crates/ruff_linter/src/noqa.rs source: crates/ruff_linter/src/noqa.rs
expression: "ParsedFileExemption::try_extract(source)" expression: "ParsedFileExemption::try_extract(TextRange::up_to(source.text_len()), source,)"
snapshot_kind: text snapshot_kind: text
--- ---
Ok( Ok(
Some( Some(
Codes( Codes(
[ Codes {
"F401", range: 0..24,
"F841", codes: [
], Code {
code: "F401",
range: 14..18,
},
Code {
code: "F841",
range: 20..24,
},
],
},
), ),
), ),
) )