From 7a6dd877f9735ea9b4821fc9b1f138e7c25f94cf Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Tue, 26 Nov 2024 16:03:36 -0500 Subject: [PATCH 01/11] add test case from #14531 --- .../fixtures/ruff/{RUF101.py => RUF101_0.py} | 0 .../resources/test/fixtures/ruff/RUF101_1.py | 13 ++++++++++ crates/ruff_linter/src/rules/ruff/mod.rs | 3 ++- ...les__ruff__tests__RUF101_RUF101_0.py.snap} | 14 +++++------ ...ules__ruff__tests__RUF101_RUF101_1.py.snap | 24 +++++++++++++++++++ 5 files changed, 46 insertions(+), 8 deletions(-) rename crates/ruff_linter/resources/test/fixtures/ruff/{RUF101.py => RUF101_0.py} (100%) create mode 100644 crates/ruff_linter/resources/test/fixtures/ruff/RUF101_1.py rename crates/ruff_linter/src/rules/ruff/snapshots/{ruff_linter__rules__ruff__tests__RUF101_RUF101.py.snap => ruff_linter__rules__ruff__tests__RUF101_RUF101_0.py.snap} (89%) create mode 100644 crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF101_RUF101_1.py.snap diff --git a/crates/ruff_linter/resources/test/fixtures/ruff/RUF101.py b/crates/ruff_linter/resources/test/fixtures/ruff/RUF101_0.py similarity index 100% rename from crates/ruff_linter/resources/test/fixtures/ruff/RUF101.py rename to crates/ruff_linter/resources/test/fixtures/ruff/RUF101_0.py diff --git a/crates/ruff_linter/resources/test/fixtures/ruff/RUF101_1.py b/crates/ruff_linter/resources/test/fixtures/ruff/RUF101_1.py new file mode 100644 index 0000000000000..7d620f9c6e8a2 --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/ruff/RUF101_1.py @@ -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) diff --git a/crates/ruff_linter/src/rules/ruff/mod.rs b/crates/ruff_linter/src/rules/ruff/mod.rs index aad87d1d336c6..d8f171d244f5a 100644 --- a/crates/ruff_linter/src/rules/ruff/mod.rs +++ b/crates/ruff_linter/src/rules/ruff/mod.rs @@ -60,7 +60,8 @@ mod tests { #[test_case(Rule::IncorrectlyParenthesizedTupleInSubscript, Path::new("RUF031.py"))] #[test_case(Rule::DecimalFromFloatLiteral, Path::new("RUF032.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::NoneNotAtEndOfUnion, Path::new("RUF036.py"))] #[test_case(Rule::NoneNotAtEndOfUnion, Path::new("RUF036.pyi"))] diff --git a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF101_RUF101.py.snap b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF101_RUF101_0.py.snap similarity index 89% rename from crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF101_RUF101.py.snap rename to crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF101_RUF101_0.py.snap index 92fdfe631756b..3a43790129b8c 100644 --- a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF101_RUF101.py.snap +++ b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF101_RUF101_0.py.snap @@ -2,7 +2,7 @@ source: crates/ruff_linter/src/rules/ruff/mod.rs 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 | ^^^^^^ 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 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 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 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 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 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 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 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 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 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 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: 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 5 | x = 2 # noqa: RUF940, RUF950, RUF940 diff --git a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF101_RUF101_1.py.snap b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF101_RUF101_1.py.snap new file mode 100644 index 0000000000000..101987e969d02 --- /dev/null +++ b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF101_RUF101_1.py.snap @@ -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 | From 7953e8f371c42e2d4d84ddea20401d28a16411c3 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Wed, 27 Nov 2024 08:58:29 -0500 Subject: [PATCH 02/11] detect noqa redirections at file level --- crates/ruff_linter/src/checkers/noqa.rs | 1 + crates/ruff_linter/src/noqa.rs | 73 ++++++++++++++----- .../src/rules/ruff/rules/redirected_noqa.rs | 33 ++++++++- 3 files changed, 88 insertions(+), 19 deletions(-) diff --git a/crates/ruff_linter/src/checkers/noqa.rs b/crates/ruff_linter/src/checkers/noqa.rs index a2ecc7a34161f..c48ab99ad57e1 100644 --- a/crates/ruff_linter/src/checkers/noqa.rs +++ b/crates/ruff_linter/src/checkers/noqa.rs @@ -211,6 +211,7 @@ pub(crate) fn check_noqa( && !exemption.includes(Rule::RedirectedNOQA) { ruff::rules::redirected_noqa(diagnostics, &noqa_directives); + ruff::rules::redirected_file_noqa(diagnostics, &file_noqa_directives); } if settings.rules.enabled(Rule::BlanketNOQA) diff --git a/crates/ruff_linter/src/noqa.rs b/crates/ruff_linter/src/noqa.rs index 0b51ac3008df4..6e43fecd5a253 100644 --- a/crates/ruff_linter/src/noqa.rs +++ b/crates/ruff_linter/src/noqa.rs @@ -319,7 +319,7 @@ impl<'a> From<&'a FileNoqaDirectives<'a>> for FileExemption<'a> { if directives .lines() .iter() - .any(|line| ParsedFileExemption::All == line.parsed_file_exemption) + .any(|line| matches!(line.parsed_file_exemption, ParsedFileExemption::All)) { FileExemption::All(codes) } else { @@ -362,7 +362,7 @@ impl<'a> FileNoqaDirectives<'a> { let mut lines = vec![]; for range in comment_ranges { - match ParsedFileExemption::try_extract(&locator.contents()[range]) { + match ParsedFileExemption::try_extract(&locator.contents()[range], range.start()) { Err(err) => { #[allow(deprecated)] let line = locator.compute_line_index(range.start()); @@ -385,11 +385,11 @@ impl<'a> FileNoqaDirectives<'a> { ParsedFileExemption::Codes(codes) => { codes.iter().filter_map(|code| { // Ignore externally-defined rules. - if external.iter().any(|external| code.starts_with(external)) { + if external.iter().any(|external| code.as_str().starts_with(external)) { return None; } - if let Ok(rule) = Rule::from_code(get_redirect_target(code).unwrap_or(code)) + if let Ok(rule) = Rule::from_code(get_redirect_target(code.as_str()).unwrap_or(code.as_str())) { Some(rule.noqa_code()) } else { @@ -424,21 +424,23 @@ impl<'a> FileNoqaDirectives<'a> { /// 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 /// across a source file. -#[derive(Debug, PartialEq, Eq)] +#[derive(Debug)] pub(crate) enum ParsedFileExemption<'a> { /// The file-level exemption ignores all rules (e.g., `# ruff: noqa`). All, /// The file-level exemption ignores specific rules (e.g., `# ruff: noqa: F401, F841`). - Codes(Vec<&'a str>), + Codes(Codes<'a>), } impl<'a> ParsedFileExemption<'a> { /// Return a [`ParsedFileExemption`] for a given comment line. - fn try_extract(line: &'a str) -> Result, ParseError> { + fn try_extract(line: &'a str, offset: TextSize) -> Result, ParseError> { + let init_line_len = line.len(); let line = Self::lex_whitespace(line); let Some(line) = Self::lex_char(line, '#') else { return Ok(None); }; + let comment_start = init_line_len - line.len(); let line = Self::lex_whitespace(line); let Some(line) = Self::lex_flake8(line).or_else(|| Self::lex_ruff(line)) else { @@ -469,7 +471,12 @@ impl<'a> ParsedFileExemption<'a> { let mut codes = vec![]; let mut line = line; while let Some(code) = Self::lex_code(line) { - codes.push(code); + let codes_end = init_line_len - line.len(); + codes.push(Code { + code, + range: TextRange::at(TextSize::try_from(codes_end).unwrap(), code.text_len()) + .add(offset), + }); line = &line[code.len()..]; // Codes can be comma- or whitespace-delimited. @@ -485,7 +492,15 @@ impl<'a> ParsedFileExemption<'a> { return Err(ParseError::MissingCodes); } - Self::Codes(codes) + let codes_end = init_line_len - line.len(); + let range = TextRange::new( + TextSize::try_from(comment_start).unwrap(), + TextSize::try_from(codes_end).unwrap(), + ); + Self::Codes(Codes { + range: range.add(offset), + codes, + }) })) } @@ -1226,50 +1241,74 @@ mod tests { #[test] fn flake8_exemption_all() { let source = "# flake8: noqa"; - assert_debug_snapshot!(ParsedFileExemption::try_extract(source)); + assert_debug_snapshot!(ParsedFileExemption::try_extract( + source, + TextSize::default() + )); } #[test] fn ruff_exemption_all() { let source = "# ruff: noqa"; - assert_debug_snapshot!(ParsedFileExemption::try_extract(source)); + assert_debug_snapshot!(ParsedFileExemption::try_extract( + source, + TextSize::default() + )); } #[test] fn flake8_exemption_all_no_space() { let source = "#flake8:noqa"; - assert_debug_snapshot!(ParsedFileExemption::try_extract(source)); + assert_debug_snapshot!(ParsedFileExemption::try_extract( + source, + TextSize::default() + )); } #[test] fn ruff_exemption_all_no_space() { let source = "#ruff:noqa"; - assert_debug_snapshot!(ParsedFileExemption::try_extract(source)); + assert_debug_snapshot!(ParsedFileExemption::try_extract( + source, + TextSize::default() + )); } #[test] fn flake8_exemption_codes() { // Note: Flake8 doesn't support this; it's treated as a blanket exemption. let source = "# flake8: noqa: F401, F841"; - assert_debug_snapshot!(ParsedFileExemption::try_extract(source)); + assert_debug_snapshot!(ParsedFileExemption::try_extract( + source, + TextSize::default() + )); } #[test] fn ruff_exemption_codes() { let source = "# ruff: noqa: F401, F841"; - assert_debug_snapshot!(ParsedFileExemption::try_extract(source)); + assert_debug_snapshot!(ParsedFileExemption::try_extract( + source, + TextSize::default() + )); } #[test] fn flake8_exemption_all_case_insensitive() { let source = "# flake8: NoQa"; - assert_debug_snapshot!(ParsedFileExemption::try_extract(source)); + assert_debug_snapshot!(ParsedFileExemption::try_extract( + source, + TextSize::default() + )); } #[test] fn ruff_exemption_all_case_insensitive() { let source = "# ruff: NoQa"; - assert_debug_snapshot!(ParsedFileExemption::try_extract(source)); + assert_debug_snapshot!(ParsedFileExemption::try_extract( + source, + TextSize::default() + )); } #[test] diff --git a/crates/ruff_linter/src/rules/ruff/rules/redirected_noqa.rs b/crates/ruff_linter/src/rules/ruff/rules/redirected_noqa.rs index 455aa7775e843..974a74184d1ce 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/redirected_noqa.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/redirected_noqa.rs @@ -2,7 +2,7 @@ use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix}; use ruff_macros::{derive_message_formats, violation}; use ruff_text_size::Ranged; -use crate::noqa::{Directive, NoqaDirectives}; +use crate::noqa::{Directive, FileNoqaDirectives, NoqaDirectives, ParsedFileExemption}; use crate::rule_redirects::get_redirect_target; /// ## What it does @@ -42,7 +42,7 @@ impl AlwaysFixableViolation for RedirectedNOQA { } } -/// RUF101 +/// RUF101 for in-line noqa directives pub(crate) fn redirected_noqa(diagnostics: &mut Vec, noqa_directives: &NoqaDirectives) { for line in noqa_directives.lines() { let Directive::Codes(directive) = &line.directive else { @@ -67,3 +67,32 @@ pub(crate) fn redirected_noqa(diagnostics: &mut Vec, noqa_directives } } } + +/// RUF101 for file noqa directives +pub(crate) fn redirected_file_noqa( + diagnostics: &mut Vec, + noqa_directives: &FileNoqaDirectives, +) { + for line in noqa_directives.lines() { + let ParsedFileExemption::Codes(codes) = &line.parsed_file_exemption else { + continue; + }; + + 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); + } + } + } +} From 7676a23aed3ed82fe5092c5d62a45019f3de12c1 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Wed, 27 Nov 2024 09:09:50 -0500 Subject: [PATCH 03/11] include comment character in noqa range --- crates/ruff_linter/src/noqa.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/ruff_linter/src/noqa.rs b/crates/ruff_linter/src/noqa.rs index 6e43fecd5a253..c46a2dbd0409e 100644 --- a/crates/ruff_linter/src/noqa.rs +++ b/crates/ruff_linter/src/noqa.rs @@ -440,7 +440,8 @@ impl<'a> ParsedFileExemption<'a> { let Some(line) = Self::lex_char(line, '#') else { return Ok(None); }; - let comment_start = init_line_len - line.len(); + // -1 to include the comment character + let comment_start = init_line_len - line.len() - 1; let line = Self::lex_whitespace(line); let Some(line) = Self::lex_flake8(line).or_else(|| Self::lex_ruff(line)) else { From 28a14260f440d309eb815b10631fb2aeb4388c9d Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Wed, 27 Nov 2024 09:10:05 -0500 Subject: [PATCH 04/11] update code representation --- ...__noqa__tests__flake8_exemption_codes.snap | 19 ++++++++++++++----- ...er__noqa__tests__ruff_exemption_codes.snap | 19 ++++++++++++++----- 2 files changed, 28 insertions(+), 10 deletions(-) diff --git a/crates/ruff_linter/src/snapshots/ruff_linter__noqa__tests__flake8_exemption_codes.snap b/crates/ruff_linter/src/snapshots/ruff_linter__noqa__tests__flake8_exemption_codes.snap index bdd8114fba2c1..1984fa86c63ed 100644 --- a/crates/ruff_linter/src/snapshots/ruff_linter__noqa__tests__flake8_exemption_codes.snap +++ b/crates/ruff_linter/src/snapshots/ruff_linter__noqa__tests__flake8_exemption_codes.snap @@ -1,15 +1,24 @@ --- source: crates/ruff_linter/src/noqa.rs -expression: "ParsedFileExemption::try_extract(source)" +expression: "ParsedFileExemption::try_extract(source, TextSize::default())" snapshot_kind: text --- Ok( Some( Codes( - [ - "F401", - "F841", - ], + Codes { + range: 0..26, + codes: [ + Code { + code: "F401", + range: 16..20, + }, + Code { + code: "F841", + range: 22..26, + }, + ], + }, ), ), ) diff --git a/crates/ruff_linter/src/snapshots/ruff_linter__noqa__tests__ruff_exemption_codes.snap b/crates/ruff_linter/src/snapshots/ruff_linter__noqa__tests__ruff_exemption_codes.snap index bdd8114fba2c1..807bd078f10eb 100644 --- a/crates/ruff_linter/src/snapshots/ruff_linter__noqa__tests__ruff_exemption_codes.snap +++ b/crates/ruff_linter/src/snapshots/ruff_linter__noqa__tests__ruff_exemption_codes.snap @@ -1,15 +1,24 @@ --- source: crates/ruff_linter/src/noqa.rs -expression: "ParsedFileExemption::try_extract(source)" +expression: "ParsedFileExemption::try_extract(source, TextSize::default())" snapshot_kind: text --- Ok( Some( Codes( - [ - "F401", - "F841", - ], + Codes { + range: 0..24, + codes: [ + Code { + code: "F401", + range: 14..18, + }, + Code { + code: "F841", + range: 20..24, + }, + ], + }, ), ), ) From 24829c5388ead44060398c40f32bf7d5197d6bb6 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Wed, 27 Nov 2024 09:24:31 -0500 Subject: [PATCH 05/11] call as_str once --- crates/ruff_linter/src/noqa.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/crates/ruff_linter/src/noqa.rs b/crates/ruff_linter/src/noqa.rs index c46a2dbd0409e..03e835b6ba6a6 100644 --- a/crates/ruff_linter/src/noqa.rs +++ b/crates/ruff_linter/src/noqa.rs @@ -384,12 +384,13 @@ impl<'a> FileNoqaDirectives<'a> { } ParsedFileExemption::Codes(codes) => { codes.iter().filter_map(|code| { + let code = code.as_str(); // Ignore externally-defined rules. - if external.iter().any(|external| code.as_str().starts_with(external)) { + if external.iter().any(|external| code.starts_with(external)) { return None; } - if let Ok(rule) = Rule::from_code(get_redirect_target(code.as_str()).unwrap_or(code.as_str())) + if let Ok(rule) = Rule::from_code(get_redirect_target(code).unwrap_or(code)) { Some(rule.noqa_code()) } else { From e0afd4a6ea2997b64091fa773a8b1d0c9994dd1c Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Wed, 27 Nov 2024 09:26:14 -0500 Subject: [PATCH 06/11] use len_utf8 like Directive --- crates/ruff_linter/src/noqa.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/crates/ruff_linter/src/noqa.rs b/crates/ruff_linter/src/noqa.rs index 03e835b6ba6a6..7e3d62e393fb8 100644 --- a/crates/ruff_linter/src/noqa.rs +++ b/crates/ruff_linter/src/noqa.rs @@ -441,8 +441,7 @@ impl<'a> ParsedFileExemption<'a> { let Some(line) = Self::lex_char(line, '#') else { return Ok(None); }; - // -1 to include the comment character - let comment_start = init_line_len - line.len() - 1; + let comment_start = init_line_len - line.len() - '#'.len_utf8(); let line = Self::lex_whitespace(line); let Some(line) = Self::lex_flake8(line).or_else(|| Self::lex_ruff(line)) else { From 3da4bba66f25d446270338c1e1e7287aac0533e7 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Wed, 27 Nov 2024 10:20:40 -0500 Subject: [PATCH 07/11] factor out build_diagnostics --- .../src/rules/ruff/rules/redirected_noqa.rs | 54 ++++++++----------- 1 file changed, 22 insertions(+), 32 deletions(-) diff --git a/crates/ruff_linter/src/rules/ruff/rules/redirected_noqa.rs b/crates/ruff_linter/src/rules/ruff/rules/redirected_noqa.rs index 974a74184d1ce..000ce0301b2bf 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/redirected_noqa.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/redirected_noqa.rs @@ -2,7 +2,7 @@ use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix}; use ruff_macros::{derive_message_formats, violation}; use ruff_text_size::Ranged; -use crate::noqa::{Directive, FileNoqaDirectives, NoqaDirectives, ParsedFileExemption}; +use crate::noqa::{Codes, Directive, FileNoqaDirectives, NoqaDirectives, ParsedFileExemption}; use crate::rule_redirects::get_redirect_target; /// ## What it does @@ -49,22 +49,7 @@ pub(crate) fn redirected_noqa(diagnostics: &mut Vec, noqa_directives continue; }; - for code in directive.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); - } - } + build_diagnostics(diagnostics, directive); } } @@ -78,21 +63,26 @@ pub(crate) fn redirected_file_noqa( continue; }; - 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); - } + build_diagnostics(diagnostics, codes); + } +} + +/// Convert a sequence of [Codes] into [Diagnostic]s and append them to `diagnostics`. +fn build_diagnostics(diagnostics: &mut Vec, 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); } } } From 6e55a7526a7798fca406061162919179db556313 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Wed, 27 Nov 2024 10:47:10 -0500 Subject: [PATCH 08/11] update ParsedFileExemption::try_extract signature --- crates/ruff_linter/src/noqa.rs | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/crates/ruff_linter/src/noqa.rs b/crates/ruff_linter/src/noqa.rs index 7e3d62e393fb8..306b965a2d4b6 100644 --- a/crates/ruff_linter/src/noqa.rs +++ b/crates/ruff_linter/src/noqa.rs @@ -362,7 +362,7 @@ impl<'a> FileNoqaDirectives<'a> { let mut lines = vec![]; for range in comment_ranges { - match ParsedFileExemption::try_extract(&locator.contents()[range], range.start()) { + match ParsedFileExemption::try_extract(range.start(), &locator.contents()[range]) { Err(err) => { #[allow(deprecated)] let line = locator.compute_line_index(range.start()); @@ -435,9 +435,9 @@ pub(crate) enum ParsedFileExemption<'a> { impl<'a> ParsedFileExemption<'a> { /// Return a [`ParsedFileExemption`] for a given comment line. - fn try_extract(line: &'a str, offset: TextSize) -> Result, ParseError> { - let init_line_len = line.len(); - let line = Self::lex_whitespace(line); + fn try_extract(comment_range: TextSize, source: &'a str) -> Result, ParseError> { + let init_line_len = source.len(); + let line = Self::lex_whitespace(source); let Some(line) = Self::lex_char(line, '#') else { return Ok(None); }; @@ -476,7 +476,7 @@ impl<'a> ParsedFileExemption<'a> { codes.push(Code { code, range: TextRange::at(TextSize::try_from(codes_end).unwrap(), code.text_len()) - .add(offset), + .add(comment_range), }); line = &line[code.len()..]; @@ -499,7 +499,7 @@ impl<'a> ParsedFileExemption<'a> { TextSize::try_from(codes_end).unwrap(), ); Self::Codes(Codes { - range: range.add(offset), + range: range.add(comment_range), codes, }) })) @@ -1243,8 +1243,8 @@ mod tests { fn flake8_exemption_all() { let source = "# flake8: noqa"; assert_debug_snapshot!(ParsedFileExemption::try_extract( + TextSize::default(), source, - TextSize::default() )); } @@ -1252,8 +1252,8 @@ mod tests { fn ruff_exemption_all() { let source = "# ruff: noqa"; assert_debug_snapshot!(ParsedFileExemption::try_extract( + TextSize::default(), source, - TextSize::default() )); } @@ -1261,8 +1261,8 @@ mod tests { fn flake8_exemption_all_no_space() { let source = "#flake8:noqa"; assert_debug_snapshot!(ParsedFileExemption::try_extract( + TextSize::default(), source, - TextSize::default() )); } @@ -1270,8 +1270,8 @@ mod tests { fn ruff_exemption_all_no_space() { let source = "#ruff:noqa"; assert_debug_snapshot!(ParsedFileExemption::try_extract( + TextSize::default(), source, - TextSize::default() )); } @@ -1280,8 +1280,8 @@ mod tests { // Note: Flake8 doesn't support this; it's treated as a blanket exemption. let source = "# flake8: noqa: F401, F841"; assert_debug_snapshot!(ParsedFileExemption::try_extract( + TextSize::default(), source, - TextSize::default() )); } @@ -1289,8 +1289,8 @@ mod tests { fn ruff_exemption_codes() { let source = "# ruff: noqa: F401, F841"; assert_debug_snapshot!(ParsedFileExemption::try_extract( + TextSize::default(), source, - TextSize::default() )); } @@ -1298,8 +1298,8 @@ mod tests { fn flake8_exemption_all_case_insensitive() { let source = "# flake8: NoQa"; assert_debug_snapshot!(ParsedFileExemption::try_extract( + TextSize::default(), source, - TextSize::default() )); } @@ -1307,8 +1307,8 @@ mod tests { fn ruff_exemption_all_case_insensitive() { let source = "# ruff: NoQa"; assert_debug_snapshot!(ParsedFileExemption::try_extract( + TextSize::default(), source, - TextSize::default() )); } From 212b9a39e0bfbf802ed7840f23fbbe4b0c858711 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Wed, 27 Nov 2024 10:52:49 -0500 Subject: [PATCH 09/11] use TextLen throughout --- crates/ruff_linter/src/noqa.rs | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/crates/ruff_linter/src/noqa.rs b/crates/ruff_linter/src/noqa.rs index 306b965a2d4b6..972b4ebd3f02f 100644 --- a/crates/ruff_linter/src/noqa.rs +++ b/crates/ruff_linter/src/noqa.rs @@ -436,12 +436,12 @@ pub(crate) enum ParsedFileExemption<'a> { impl<'a> ParsedFileExemption<'a> { /// Return a [`ParsedFileExemption`] for a given comment line. fn try_extract(comment_range: TextSize, source: &'a str) -> Result, ParseError> { - let init_line_len = source.len(); + let init_line_len = source.text_len(); let line = Self::lex_whitespace(source); let Some(line) = Self::lex_char(line, '#') else { return Ok(None); }; - let comment_start = init_line_len - line.len() - '#'.len_utf8(); + let comment_start = init_line_len - line.text_len() - '#'.text_len(); let line = Self::lex_whitespace(line); let Some(line) = Self::lex_flake8(line).or_else(|| Self::lex_ruff(line)) else { @@ -472,11 +472,10 @@ impl<'a> ParsedFileExemption<'a> { let mut codes = vec![]; let mut line = line; while let Some(code) = Self::lex_code(line) { - let codes_end = init_line_len - line.len(); + let codes_end = init_line_len - line.text_len(); codes.push(Code { code, - range: TextRange::at(TextSize::try_from(codes_end).unwrap(), code.text_len()) - .add(comment_range), + range: TextRange::at(codes_end, code.text_len()).add(comment_range), }); line = &line[code.len()..]; @@ -493,11 +492,8 @@ impl<'a> ParsedFileExemption<'a> { return Err(ParseError::MissingCodes); } - let codes_end = init_line_len - line.len(); - let range = TextRange::new( - TextSize::try_from(comment_start).unwrap(), - TextSize::try_from(codes_end).unwrap(), - ); + let codes_end = init_line_len - line.text_len(); + let range = TextRange::new(comment_start, codes_end); Self::Codes(Codes { range: range.add(comment_range), codes, From 2af107d548ab54650f2f9ca2c2781915fbabb3ef Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Wed, 27 Nov 2024 11:32:18 -0500 Subject: [PATCH 10/11] actually fix try_extract signature --- crates/ruff_linter/src/noqa.rs | 35 ++++++++++++++++++---------------- 1 file changed, 19 insertions(+), 16 deletions(-) diff --git a/crates/ruff_linter/src/noqa.rs b/crates/ruff_linter/src/noqa.rs index 972b4ebd3f02f..ce670225b89c9 100644 --- a/crates/ruff_linter/src/noqa.rs +++ b/crates/ruff_linter/src/noqa.rs @@ -362,7 +362,7 @@ impl<'a> FileNoqaDirectives<'a> { let mut lines = vec![]; for range in comment_ranges { - match ParsedFileExemption::try_extract(range.start(), &locator.contents()[range]) { + match ParsedFileExemption::try_extract(range, locator.contents()) { Err(err) => { #[allow(deprecated)] let line = locator.compute_line_index(range.start()); @@ -434,10 +434,13 @@ pub(crate) enum ParsedFileExemption<'a> { } impl<'a> ParsedFileExemption<'a> { - /// Return a [`ParsedFileExemption`] for a given comment line. - fn try_extract(comment_range: TextSize, source: &'a str) -> Result, ParseError> { - let init_line_len = source.text_len(); - let line = Self::lex_whitespace(source); + /// Return a [`ParsedFileExemption`] for a given `comment_range` in `source`. + fn try_extract(comment_range: TextRange, source: &'a str) -> Result, 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 Some(line) = Self::lex_char(line, '#') else { return Ok(None); }; @@ -475,7 +478,7 @@ impl<'a> ParsedFileExemption<'a> { let codes_end = init_line_len - line.text_len(); codes.push(Code { code, - range: TextRange::at(codes_end, code.text_len()).add(comment_range), + range: TextRange::at(codes_end, code.text_len()).add(offset), }); line = &line[code.len()..]; @@ -495,7 +498,7 @@ impl<'a> ParsedFileExemption<'a> { let codes_end = init_line_len - line.text_len(); let range = TextRange::new(comment_start, codes_end); Self::Codes(Codes { - range: range.add(comment_range), + range: range.add(offset), codes, }) })) @@ -1071,7 +1074,7 @@ mod tests { use ruff_diagnostics::{Diagnostic, Edit}; use ruff_python_trivia::CommentRanges; 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::rules::pycodestyle::rules::{AmbiguousVariableName, UselessSemicolon}; @@ -1239,7 +1242,7 @@ mod tests { fn flake8_exemption_all() { let source = "# flake8: noqa"; assert_debug_snapshot!(ParsedFileExemption::try_extract( - TextSize::default(), + TextRange::up_to(source.text_len()), source, )); } @@ -1248,7 +1251,7 @@ mod tests { fn ruff_exemption_all() { let source = "# ruff: noqa"; assert_debug_snapshot!(ParsedFileExemption::try_extract( - TextSize::default(), + TextRange::up_to(source.text_len()), source, )); } @@ -1257,7 +1260,7 @@ mod tests { fn flake8_exemption_all_no_space() { let source = "#flake8:noqa"; assert_debug_snapshot!(ParsedFileExemption::try_extract( - TextSize::default(), + TextRange::up_to(source.text_len()), source, )); } @@ -1266,7 +1269,7 @@ mod tests { fn ruff_exemption_all_no_space() { let source = "#ruff:noqa"; assert_debug_snapshot!(ParsedFileExemption::try_extract( - TextSize::default(), + TextRange::up_to(source.text_len()), source, )); } @@ -1276,7 +1279,7 @@ mod tests { // Note: Flake8 doesn't support this; it's treated as a blanket exemption. let source = "# flake8: noqa: F401, F841"; assert_debug_snapshot!(ParsedFileExemption::try_extract( - TextSize::default(), + TextRange::up_to(source.text_len()), source, )); } @@ -1285,7 +1288,7 @@ mod tests { fn ruff_exemption_codes() { let source = "# ruff: noqa: F401, F841"; assert_debug_snapshot!(ParsedFileExemption::try_extract( - TextSize::default(), + TextRange::up_to(source.text_len()), source, )); } @@ -1294,7 +1297,7 @@ mod tests { fn flake8_exemption_all_case_insensitive() { let source = "# flake8: NoQa"; assert_debug_snapshot!(ParsedFileExemption::try_extract( - TextSize::default(), + TextRange::up_to(source.text_len()), source, )); } @@ -1303,7 +1306,7 @@ mod tests { fn ruff_exemption_all_case_insensitive() { let source = "# ruff: NoQa"; assert_debug_snapshot!(ParsedFileExemption::try_extract( - TextSize::default(), + TextRange::up_to(source.text_len()), source, )); } From eb5bb0bba96106f31756e1a83da0680f5c6fa782 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Wed, 27 Nov 2024 11:56:27 -0500 Subject: [PATCH 11/11] update snapshot metadata to match try_extract signature --- .../ruff_linter__noqa__tests__flake8_exemption_all.snap | 2 +- ...ter__noqa__tests__flake8_exemption_all_case_insensitive.snap | 2 +- ...ruff_linter__noqa__tests__flake8_exemption_all_no_space.snap | 2 +- .../ruff_linter__noqa__tests__flake8_exemption_codes.snap | 2 +- .../snapshots/ruff_linter__noqa__tests__ruff_exemption_all.snap | 2 +- ...inter__noqa__tests__ruff_exemption_all_case_insensitive.snap | 2 +- .../ruff_linter__noqa__tests__ruff_exemption_all_no_space.snap | 2 +- .../ruff_linter__noqa__tests__ruff_exemption_codes.snap | 2 +- 8 files changed, 8 insertions(+), 8 deletions(-) diff --git a/crates/ruff_linter/src/snapshots/ruff_linter__noqa__tests__flake8_exemption_all.snap b/crates/ruff_linter/src/snapshots/ruff_linter__noqa__tests__flake8_exemption_all.snap index 82d71163d072f..d2833468243f3 100644 --- a/crates/ruff_linter/src/snapshots/ruff_linter__noqa__tests__flake8_exemption_all.snap +++ b/crates/ruff_linter/src/snapshots/ruff_linter__noqa__tests__flake8_exemption_all.snap @@ -1,6 +1,6 @@ --- 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 --- Ok( diff --git a/crates/ruff_linter/src/snapshots/ruff_linter__noqa__tests__flake8_exemption_all_case_insensitive.snap b/crates/ruff_linter/src/snapshots/ruff_linter__noqa__tests__flake8_exemption_all_case_insensitive.snap index 82d71163d072f..d2833468243f3 100644 --- a/crates/ruff_linter/src/snapshots/ruff_linter__noqa__tests__flake8_exemption_all_case_insensitive.snap +++ b/crates/ruff_linter/src/snapshots/ruff_linter__noqa__tests__flake8_exemption_all_case_insensitive.snap @@ -1,6 +1,6 @@ --- 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 --- Ok( diff --git a/crates/ruff_linter/src/snapshots/ruff_linter__noqa__tests__flake8_exemption_all_no_space.snap b/crates/ruff_linter/src/snapshots/ruff_linter__noqa__tests__flake8_exemption_all_no_space.snap index 82d71163d072f..d2833468243f3 100644 --- a/crates/ruff_linter/src/snapshots/ruff_linter__noqa__tests__flake8_exemption_all_no_space.snap +++ b/crates/ruff_linter/src/snapshots/ruff_linter__noqa__tests__flake8_exemption_all_no_space.snap @@ -1,6 +1,6 @@ --- 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 --- Ok( diff --git a/crates/ruff_linter/src/snapshots/ruff_linter__noqa__tests__flake8_exemption_codes.snap b/crates/ruff_linter/src/snapshots/ruff_linter__noqa__tests__flake8_exemption_codes.snap index 1984fa86c63ed..88c0df09bc840 100644 --- a/crates/ruff_linter/src/snapshots/ruff_linter__noqa__tests__flake8_exemption_codes.snap +++ b/crates/ruff_linter/src/snapshots/ruff_linter__noqa__tests__flake8_exemption_codes.snap @@ -1,6 +1,6 @@ --- source: crates/ruff_linter/src/noqa.rs -expression: "ParsedFileExemption::try_extract(source, TextSize::default())" +expression: "ParsedFileExemption::try_extract(TextRange::up_to(source.text_len()), source,)" snapshot_kind: text --- Ok( diff --git a/crates/ruff_linter/src/snapshots/ruff_linter__noqa__tests__ruff_exemption_all.snap b/crates/ruff_linter/src/snapshots/ruff_linter__noqa__tests__ruff_exemption_all.snap index 82d71163d072f..d2833468243f3 100644 --- a/crates/ruff_linter/src/snapshots/ruff_linter__noqa__tests__ruff_exemption_all.snap +++ b/crates/ruff_linter/src/snapshots/ruff_linter__noqa__tests__ruff_exemption_all.snap @@ -1,6 +1,6 @@ --- 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 --- Ok( diff --git a/crates/ruff_linter/src/snapshots/ruff_linter__noqa__tests__ruff_exemption_all_case_insensitive.snap b/crates/ruff_linter/src/snapshots/ruff_linter__noqa__tests__ruff_exemption_all_case_insensitive.snap index 82d71163d072f..d2833468243f3 100644 --- a/crates/ruff_linter/src/snapshots/ruff_linter__noqa__tests__ruff_exemption_all_case_insensitive.snap +++ b/crates/ruff_linter/src/snapshots/ruff_linter__noqa__tests__ruff_exemption_all_case_insensitive.snap @@ -1,6 +1,6 @@ --- 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 --- Ok( diff --git a/crates/ruff_linter/src/snapshots/ruff_linter__noqa__tests__ruff_exemption_all_no_space.snap b/crates/ruff_linter/src/snapshots/ruff_linter__noqa__tests__ruff_exemption_all_no_space.snap index 82d71163d072f..d2833468243f3 100644 --- a/crates/ruff_linter/src/snapshots/ruff_linter__noqa__tests__ruff_exemption_all_no_space.snap +++ b/crates/ruff_linter/src/snapshots/ruff_linter__noqa__tests__ruff_exemption_all_no_space.snap @@ -1,6 +1,6 @@ --- 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 --- Ok( diff --git a/crates/ruff_linter/src/snapshots/ruff_linter__noqa__tests__ruff_exemption_codes.snap b/crates/ruff_linter/src/snapshots/ruff_linter__noqa__tests__ruff_exemption_codes.snap index 807bd078f10eb..d9533e0f5f884 100644 --- a/crates/ruff_linter/src/snapshots/ruff_linter__noqa__tests__ruff_exemption_codes.snap +++ b/crates/ruff_linter/src/snapshots/ruff_linter__noqa__tests__ruff_exemption_codes.snap @@ -1,6 +1,6 @@ --- source: crates/ruff_linter/src/noqa.rs -expression: "ParsedFileExemption::try_extract(source, TextSize::default())" +expression: "ParsedFileExemption::try_extract(TextRange::up_to(source.text_len()), source,)" snapshot_kind: text --- Ok(