From 57324c54d55eba5d81fc01a2542f1714efb7a100 Mon Sep 17 00:00:00 2001 From: augustelalande Date: Thu, 28 Mar 2024 14:24:30 -0400 Subject: [PATCH 1/2] merge NQA002 and NQA003 into PGH004 --- .../test/fixtures/pygrep_hooks/PGH004_0.py | 21 +++++ crates/ruff_linter/src/noqa.rs | 2 +- .../rules/pygrep_hooks/rules/blanket_noqa.rs | 85 ++++++++++++++++- ...grep_hooks__tests__PGH004_PGH004_0.py.snap | 93 +++++++++++++++++++ 4 files changed, 195 insertions(+), 6 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/pygrep_hooks/PGH004_0.py b/crates/ruff_linter/resources/test/fixtures/pygrep_hooks/PGH004_0.py index c35f8e8824b68..bbba166f2847e 100644 --- a/crates/ruff_linter/resources/test/fixtures/pygrep_hooks/PGH004_0.py +++ b/crates/ruff_linter/resources/test/fixtures/pygrep_hooks/PGH004_0.py @@ -9,3 +9,24 @@ x = 1 # noqa: F401, W203 # noqa: F401 # noqa: F401, W203 + + + +# Ok +x = 2 # noqa: X100 +x = 2 # noqa:X100 + +# PGH004 +x = 2 # noqa X100 + +# PGH004 +x = 2 # noqa X100, X200 + +# PGH004 +x = 2 # noqa : X300 + +# PGH004 +x = 2 # noqa : X400 + +# PGH004 +x = 2 # noqa :X500 diff --git a/crates/ruff_linter/src/noqa.rs b/crates/ruff_linter/src/noqa.rs index d50c3b1757038..3af740e819193 100644 --- a/crates/ruff_linter/src/noqa.rs +++ b/crates/ruff_linter/src/noqa.rs @@ -147,7 +147,7 @@ impl<'a> Directive<'a> { /// Lex an individual rule code (e.g., `F401`). #[inline] - fn lex_code(line: &str) -> Option<&str> { + pub(crate) fn lex_code(line: &str) -> Option<&str> { // Extract, e.g., the `F` in `F401`. let prefix = line.chars().take_while(char::is_ascii_uppercase).count(); // Extract, e.g., the `401` in `F401`. diff --git a/crates/ruff_linter/src/rules/pygrep_hooks/rules/blanket_noqa.rs b/crates/ruff_linter/src/rules/pygrep_hooks/rules/blanket_noqa.rs index 7bbf4fef771d9..961f774113fde 100644 --- a/crates/ruff_linter/src/rules/pygrep_hooks/rules/blanket_noqa.rs +++ b/crates/ruff_linter/src/rules/pygrep_hooks/rules/blanket_noqa.rs @@ -1,8 +1,8 @@ -use ruff_diagnostics::{Diagnostic, Violation}; +use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation}; use ruff_macros::{derive_message_formats, violation}; use ruff_python_index::Indexer; use ruff_source_file::Locator; -use ruff_text_size::Ranged; +use ruff_text_size::{Ranged, TextRange, TextSize}; use crate::noqa::Directive; @@ -30,13 +30,42 @@ use crate::noqa::Directive; /// ## References /// - [Ruff documentation](https://docs.astral.sh/ruff/configuration/#error-suppression) #[violation] -pub struct BlanketNOQA; +pub struct BlanketNOQA { + missing_colon: bool, + space_before_colon: bool, +} impl Violation for BlanketNOQA { + const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes; + #[derive_message_formats] fn message(&self) -> String { + let BlanketNOQA { + missing_colon, + space_before_colon, + } = self; + if *missing_colon { + return format!("Use a colon when specifying `noqa` rule codes"); + } + if *space_before_colon { + return format!("Do not add spaces between `noqa` and its colon"); + } format!("Use specific rule codes when using `noqa`") } + + fn fix_title(&self) -> Option { + let BlanketNOQA { + missing_colon, + space_before_colon, + } = self; + if *missing_colon { + return Some("Add missing colon".to_string()); + } + if *space_before_colon { + return Some("Remove space(s) before colon".to_string()); + } + None + } } /// PGH004 @@ -47,8 +76,54 @@ pub(crate) fn blanket_noqa( ) { for range in indexer.comment_ranges() { let line = locator.slice(*range); - if let Ok(Some(Directive::All(all))) = Directive::try_extract(line, range.start()) { - diagnostics.push(Diagnostic::new(BlanketNOQA, all.range())); + let offset = range.start(); + if let Ok(Some(Directive::All(all))) = Directive::try_extract(line, TextSize::new(0)) { + let noqa_start = offset + all.range().start(); + let noqa_end = offset + all.range().end(); + let post_noqa_start = all.range().end().to_usize(); + let mut cursor = post_noqa_start; + cursor += leading_whitespace_len(&line[cursor..]); + + // Check for extraneous space before colon + if matches!(line[cursor..].chars().next(), Some(':')) { + let start = offset + all.range().end(); + let end = offset + TextSize::new(u32::try_from(cursor).unwrap()); + let mut diagnostic = Diagnostic::new( + BlanketNOQA { + missing_colon: false, + space_before_colon: true, + }, + TextRange::new(noqa_start, end), + ); + diagnostic.set_fix(Fix::unsafe_edit(Edit::deletion(start, end))); + diagnostics.push(diagnostic); + } + // Check for missing colon + else if Directive::lex_code(&line[cursor..]).is_some() { + let start = offset + all.range().end(); + let end = start + TextSize::new(1); + let mut diagnostic = Diagnostic::new( + BlanketNOQA { + missing_colon: true, + space_before_colon: false, + }, + TextRange::new(noqa_start, end), + ); + diagnostic.set_fix(Fix::unsafe_edit(Edit::insertion(':'.to_string(), start))); + diagnostics.push(diagnostic); + } else { + diagnostics.push(Diagnostic::new( + BlanketNOQA { + missing_colon: false, + space_before_colon: false, + }, + TextRange::new(noqa_start, noqa_end), + )); + } } } } + +fn leading_whitespace_len(text: &str) -> usize { + text.find(|c: char| !c.is_whitespace()).unwrap_or(0) +} diff --git a/crates/ruff_linter/src/rules/pygrep_hooks/snapshots/ruff_linter__rules__pygrep_hooks__tests__PGH004_PGH004_0.py.snap b/crates/ruff_linter/src/rules/pygrep_hooks/snapshots/ruff_linter__rules__pygrep_hooks__tests__PGH004_PGH004_0.py.snap index 45a79a3e85d34..ad8d5e7fe3778 100644 --- a/crates/ruff_linter/src/rules/pygrep_hooks/snapshots/ruff_linter__rules__pygrep_hooks__tests__PGH004_PGH004_0.py.snap +++ b/crates/ruff_linter/src/rules/pygrep_hooks/snapshots/ruff_linter__rules__pygrep_hooks__tests__PGH004_PGH004_0.py.snap @@ -29,4 +29,97 @@ PGH004_0.py:4:1: PGH004 Use specific rule codes when using `noqa` 6 | # noqa:F401,W203 | +PGH004_0.py:20:8: PGH004 [*] Use a colon when specifying `noqa` rule codes + | +19 | # PGH004 +20 | x = 2 # noqa X100 + | ^^^^^^^ PGH004 +21 | +22 | # PGH004 + | + = help: Add missing colon +ℹ Unsafe fix +17 17 | x = 2 # noqa:X100 +18 18 | +19 19 | # PGH004 +20 |-x = 2 # noqa X100 + 20 |+x = 2 # noqa: X100 +21 21 | +22 22 | # PGH004 +23 23 | x = 2 # noqa X100, X200 + +PGH004_0.py:23:8: PGH004 [*] Use a colon when specifying `noqa` rule codes + | +22 | # PGH004 +23 | x = 2 # noqa X100, X200 + | ^^^^^^^ PGH004 +24 | +25 | # PGH004 + | + = help: Add missing colon + +ℹ Unsafe fix +20 20 | x = 2 # noqa X100 +21 21 | +22 22 | # PGH004 +23 |-x = 2 # noqa X100, X200 + 23 |+x = 2 # noqa: X100, X200 +24 24 | +25 25 | # PGH004 +26 26 | x = 2 # noqa : X300 + +PGH004_0.py:26:8: PGH004 [*] Do not add spaces between `noqa` and its colon + | +25 | # PGH004 +26 | x = 2 # noqa : X300 + | ^^^^^^^ PGH004 +27 | +28 | # PGH004 + | + = help: Remove space(s) before colon + +ℹ Unsafe fix +23 23 | x = 2 # noqa X100, X200 +24 24 | +25 25 | # PGH004 +26 |-x = 2 # noqa : X300 + 26 |+x = 2 # noqa: X300 +27 27 | +28 28 | # PGH004 +29 29 | x = 2 # noqa : X400 + +PGH004_0.py:29:8: PGH004 [*] Do not add spaces between `noqa` and its colon + | +28 | # PGH004 +29 | x = 2 # noqa : X400 + | ^^^^^^^^ PGH004 +30 | +31 | # PGH004 + | + = help: Remove space(s) before colon + +ℹ Unsafe fix +26 26 | x = 2 # noqa : X300 +27 27 | +28 28 | # PGH004 +29 |-x = 2 # noqa : X400 + 29 |+x = 2 # noqa: X400 +30 30 | +31 31 | # PGH004 +32 32 | x = 2 # noqa :X500 + +PGH004_0.py:32:8: PGH004 [*] Do not add spaces between `noqa` and its colon + | +31 | # PGH004 +32 | x = 2 # noqa :X500 + | ^^^^^^^ PGH004 + | + = help: Remove space(s) before colon + +ℹ Unsafe fix +29 29 | x = 2 # noqa : X400 +30 30 | +31 31 | # PGH004 +32 |-x = 2 # noqa :X500 + 32 |+x = 2 # noqa:X500 From fcc872c167e4b267a4c59a1750e3768cc24f1727 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Wed, 10 Apr 2024 00:19:58 -0400 Subject: [PATCH 2/2] Use Cursor --- .../test/fixtures/pygrep_hooks/PGH004_0.py | 4 +- .../rules/pygrep_hooks/rules/blanket_noqa.rs | 69 ++++++---- ...grep_hooks__tests__PGH004_PGH004_0.py.snap | 120 +++++++++--------- 3 files changed, 102 insertions(+), 91 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/pygrep_hooks/PGH004_0.py b/crates/ruff_linter/resources/test/fixtures/pygrep_hooks/PGH004_0.py index bbba166f2847e..2e7ae46fba766 100644 --- a/crates/ruff_linter/resources/test/fixtures/pygrep_hooks/PGH004_0.py +++ b/crates/ruff_linter/resources/test/fixtures/pygrep_hooks/PGH004_0.py @@ -10,9 +10,7 @@ # noqa: F401 # noqa: F401, W203 - - -# Ok +# OK x = 2 # noqa: X100 x = 2 # noqa:X100 diff --git a/crates/ruff_linter/src/rules/pygrep_hooks/rules/blanket_noqa.rs b/crates/ruff_linter/src/rules/pygrep_hooks/rules/blanket_noqa.rs index 961f774113fde..5d6fc4ff0eeb0 100644 --- a/crates/ruff_linter/src/rules/pygrep_hooks/rules/blanket_noqa.rs +++ b/crates/ruff_linter/src/rules/pygrep_hooks/rules/blanket_noqa.rs @@ -1,6 +1,7 @@ use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation}; use ruff_macros::{derive_message_formats, violation}; use ruff_python_index::Indexer; +use ruff_python_trivia::Cursor; use ruff_source_file::Locator; use ruff_text_size::{Ranged, TextRange, TextSize}; @@ -27,6 +28,14 @@ use crate::noqa::Directive; /// from .base import * # noqa: F403 /// ``` /// +/// ## Fix safety +/// This rule will attempt to fix blanket `noqa` annotations that appear to +/// be unintentional. For example, given `# noqa F401`, the rule will suggest +/// inserting a colon, as in `# noqa: F401`. +/// +/// While modifying `noqa` comments is generally safe, doing so may introduce +/// additional diagnostics. +/// /// ## References /// - [Ruff documentation](https://docs.astral.sh/ruff/configuration/#error-suppression) #[violation] @@ -44,13 +53,16 @@ impl Violation for BlanketNOQA { missing_colon, space_before_colon, } = self; - if *missing_colon { - return format!("Use a colon when specifying `noqa` rule codes"); - } - if *space_before_colon { - return format!("Do not add spaces between `noqa` and its colon"); + + // This awkward branching is necessary to ensure that the generic message is picked up by + // `derive_message_formats`. + if !missing_colon && !space_before_colon { + format!("Use specific rule codes when using `noqa`") + } else if *missing_colon { + format!("Use a colon when specifying `noqa` rule codes") + } else { + format!("Do not add spaces between `noqa` and its colon") } - format!("Use specific rule codes when using `noqa`") } fn fix_title(&self) -> Option { @@ -58,13 +70,14 @@ impl Violation for BlanketNOQA { missing_colon, space_before_colon, } = self; + if *missing_colon { - return Some("Add missing colon".to_string()); + Some("Add missing colon".to_string()) + } else if *space_before_colon { + Some("Remove space(s) before colon".to_string()) + } else { + None } - if *space_before_colon { - return Some("Remove space(s) before colon".to_string()); - } - None } } @@ -78,16 +91,19 @@ pub(crate) fn blanket_noqa( let line = locator.slice(*range); let offset = range.start(); if let Ok(Some(Directive::All(all))) = Directive::try_extract(line, TextSize::new(0)) { - let noqa_start = offset + all.range().start(); - let noqa_end = offset + all.range().end(); - let post_noqa_start = all.range().end().to_usize(); - let mut cursor = post_noqa_start; - cursor += leading_whitespace_len(&line[cursor..]); + // The `all` range is that of the `noqa` directive in, e.g., `# noqa` or `# noqa F401`. + let noqa_start = offset + all.start(); + let noqa_end = offset + all.end(); + + // Skip the `# noqa`, plus any trailing whitespace. + let mut cursor = Cursor::new(&line[all.end().to_usize()..]); + cursor.eat_while(char::is_whitespace); - // Check for extraneous space before colon - if matches!(line[cursor..].chars().next(), Some(':')) { - let start = offset + all.range().end(); - let end = offset + TextSize::new(u32::try_from(cursor).unwrap()); + // Check for extraneous spaces before the colon. + // Ex) `# noqa : F401` + if cursor.first() == ':' { + let start = offset + all.end(); + let end = start + cursor.token_len(); let mut diagnostic = Diagnostic::new( BlanketNOQA { missing_colon: false, @@ -97,10 +113,10 @@ pub(crate) fn blanket_noqa( ); diagnostic.set_fix(Fix::unsafe_edit(Edit::deletion(start, end))); diagnostics.push(diagnostic); - } - // Check for missing colon - else if Directive::lex_code(&line[cursor..]).is_some() { - let start = offset + all.range().end(); + } else if Directive::lex_code(cursor.chars().as_str()).is_some() { + // Check for a missing colon. + // Ex) `# noqa F401` + let start = offset + all.end(); let end = start + TextSize::new(1); let mut diagnostic = Diagnostic::new( BlanketNOQA { @@ -112,6 +128,7 @@ pub(crate) fn blanket_noqa( diagnostic.set_fix(Fix::unsafe_edit(Edit::insertion(':'.to_string(), start))); diagnostics.push(diagnostic); } else { + // Otherwise, it looks like an intentional blanket `noqa` annotation. diagnostics.push(Diagnostic::new( BlanketNOQA { missing_colon: false, @@ -123,7 +140,3 @@ pub(crate) fn blanket_noqa( } } } - -fn leading_whitespace_len(text: &str) -> usize { - text.find(|c: char| !c.is_whitespace()).unwrap_or(0) -} diff --git a/crates/ruff_linter/src/rules/pygrep_hooks/snapshots/ruff_linter__rules__pygrep_hooks__tests__PGH004_PGH004_0.py.snap b/crates/ruff_linter/src/rules/pygrep_hooks/snapshots/ruff_linter__rules__pygrep_hooks__tests__PGH004_PGH004_0.py.snap index ad8d5e7fe3778..5c4e0772479d1 100644 --- a/crates/ruff_linter/src/rules/pygrep_hooks/snapshots/ruff_linter__rules__pygrep_hooks__tests__PGH004_PGH004_0.py.snap +++ b/crates/ruff_linter/src/rules/pygrep_hooks/snapshots/ruff_linter__rules__pygrep_hooks__tests__PGH004_PGH004_0.py.snap @@ -29,97 +29,97 @@ PGH004_0.py:4:1: PGH004 Use specific rule codes when using `noqa` 6 | # noqa:F401,W203 | -PGH004_0.py:20:8: PGH004 [*] Use a colon when specifying `noqa` rule codes +PGH004_0.py:18:8: PGH004 [*] Use a colon when specifying `noqa` rule codes | -19 | # PGH004 -20 | x = 2 # noqa X100 +17 | # PGH004 +18 | x = 2 # noqa X100 | ^^^^^^^ PGH004 -21 | -22 | # PGH004 +19 | +20 | # PGH004 | = help: Add missing colon ℹ Unsafe fix -17 17 | x = 2 # noqa:X100 -18 18 | -19 19 | # PGH004 -20 |-x = 2 # noqa X100 - 20 |+x = 2 # noqa: X100 -21 21 | -22 22 | # PGH004 -23 23 | x = 2 # noqa X100, X200 +15 15 | x = 2 # noqa:X100 +16 16 | +17 17 | # PGH004 +18 |-x = 2 # noqa X100 + 18 |+x = 2 # noqa: X100 +19 19 | +20 20 | # PGH004 +21 21 | x = 2 # noqa X100, X200 -PGH004_0.py:23:8: PGH004 [*] Use a colon when specifying `noqa` rule codes +PGH004_0.py:21:8: PGH004 [*] Use a colon when specifying `noqa` rule codes | -22 | # PGH004 -23 | x = 2 # noqa X100, X200 +20 | # PGH004 +21 | x = 2 # noqa X100, X200 | ^^^^^^^ PGH004 -24 | -25 | # PGH004 +22 | +23 | # PGH004 | = help: Add missing colon ℹ Unsafe fix -20 20 | x = 2 # noqa X100 -21 21 | -22 22 | # PGH004 -23 |-x = 2 # noqa X100, X200 - 23 |+x = 2 # noqa: X100, X200 -24 24 | -25 25 | # PGH004 -26 26 | x = 2 # noqa : X300 +18 18 | x = 2 # noqa X100 +19 19 | +20 20 | # PGH004 +21 |-x = 2 # noqa X100, X200 + 21 |+x = 2 # noqa: X100, X200 +22 22 | +23 23 | # PGH004 +24 24 | x = 2 # noqa : X300 -PGH004_0.py:26:8: PGH004 [*] Do not add spaces between `noqa` and its colon +PGH004_0.py:24:8: PGH004 [*] Do not add spaces between `noqa` and its colon | -25 | # PGH004 -26 | x = 2 # noqa : X300 +23 | # PGH004 +24 | x = 2 # noqa : X300 | ^^^^^^^ PGH004 -27 | -28 | # PGH004 +25 | +26 | # PGH004 | = help: Remove space(s) before colon ℹ Unsafe fix -23 23 | x = 2 # noqa X100, X200 -24 24 | -25 25 | # PGH004 -26 |-x = 2 # noqa : X300 - 26 |+x = 2 # noqa: X300 -27 27 | -28 28 | # PGH004 -29 29 | x = 2 # noqa : X400 +21 21 | x = 2 # noqa X100, X200 +22 22 | +23 23 | # PGH004 +24 |-x = 2 # noqa : X300 + 24 |+x = 2 # noqa: X300 +25 25 | +26 26 | # PGH004 +27 27 | x = 2 # noqa : X400 -PGH004_0.py:29:8: PGH004 [*] Do not add spaces between `noqa` and its colon +PGH004_0.py:27:8: PGH004 [*] Do not add spaces between `noqa` and its colon | -28 | # PGH004 -29 | x = 2 # noqa : X400 +26 | # PGH004 +27 | x = 2 # noqa : X400 | ^^^^^^^^ PGH004 -30 | -31 | # PGH004 +28 | +29 | # PGH004 | = help: Remove space(s) before colon ℹ Unsafe fix -26 26 | x = 2 # noqa : X300 -27 27 | -28 28 | # PGH004 -29 |-x = 2 # noqa : X400 - 29 |+x = 2 # noqa: X400 -30 30 | -31 31 | # PGH004 -32 32 | x = 2 # noqa :X500 +24 24 | x = 2 # noqa : X300 +25 25 | +26 26 | # PGH004 +27 |-x = 2 # noqa : X400 + 27 |+x = 2 # noqa: X400 +28 28 | +29 29 | # PGH004 +30 30 | x = 2 # noqa :X500 -PGH004_0.py:32:8: PGH004 [*] Do not add spaces between `noqa` and its colon +PGH004_0.py:30:8: PGH004 [*] Do not add spaces between `noqa` and its colon | -31 | # PGH004 -32 | x = 2 # noqa :X500 +29 | # PGH004 +30 | x = 2 # noqa :X500 | ^^^^^^^ PGH004 | = help: Remove space(s) before colon ℹ Unsafe fix -29 29 | x = 2 # noqa : X400 -30 30 | -31 31 | # PGH004 -32 |-x = 2 # noqa :X500 - 32 |+x = 2 # noqa:X500 +27 27 | x = 2 # noqa : X400 +28 28 | +29 29 | # PGH004 +30 |-x = 2 # noqa :X500 + 30 |+x = 2 # noqa:X500