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 c35f8e8824b68b..2e7ae46fba7663 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,22 @@ 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 d50c3b17570380..3af740e819193a 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 7bbf4fef771d92..5d6fc4ff0eeb0f 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,9 @@ -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_python_trivia::Cursor; use ruff_source_file::Locator; -use ruff_text_size::Ranged; +use ruff_text_size::{Ranged, TextRange, TextSize}; use crate::noqa::Directive; @@ -27,15 +28,56 @@ 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] -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 { - format!("Use specific rule codes when using `noqa`") + let BlanketNOQA { + missing_colon, + space_before_colon, + } = self; + + // 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") + } + } + + fn fix_title(&self) -> Option { + let BlanketNOQA { + missing_colon, + space_before_colon, + } = self; + + if *missing_colon { + Some("Add missing colon".to_string()) + } else if *space_before_colon { + Some("Remove space(s) before colon".to_string()) + } else { + None + } } } @@ -47,8 +89,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)) { + // 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 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, + space_before_colon: true, + }, + TextRange::new(noqa_start, end), + ); + diagnostic.set_fix(Fix::unsafe_edit(Edit::deletion(start, end))); + diagnostics.push(diagnostic); + } 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 { + 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 { + // Otherwise, it looks like an intentional blanket `noqa` annotation. + diagnostics.push(Diagnostic::new( + BlanketNOQA { + missing_colon: false, + space_before_colon: false, + }, + TextRange::new(noqa_start, noqa_end), + )); + } } } } 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 45a79a3e85d345..5c4e0772479d1d 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:18:8: PGH004 [*] Use a colon when specifying `noqa` rule codes + | +17 | # PGH004 +18 | x = 2 # noqa X100 + | ^^^^^^^ PGH004 +19 | +20 | # PGH004 + | + = help: Add missing colon +ℹ Unsafe fix +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:21:8: PGH004 [*] Use a colon when specifying `noqa` rule codes + | +20 | # PGH004 +21 | x = 2 # noqa X100, X200 + | ^^^^^^^ PGH004 +22 | +23 | # PGH004 + | + = help: Add missing colon + +ℹ Unsafe fix +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:24:8: PGH004 [*] Do not add spaces between `noqa` and its colon + | +23 | # PGH004 +24 | x = 2 # noqa : X300 + | ^^^^^^^ PGH004 +25 | +26 | # PGH004 + | + = help: Remove space(s) before colon + +ℹ Unsafe fix +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:27:8: PGH004 [*] Do not add spaces between `noqa` and its colon + | +26 | # PGH004 +27 | x = 2 # noqa : X400 + | ^^^^^^^^ PGH004 +28 | +29 | # PGH004 + | + = help: Remove space(s) before colon + +ℹ Unsafe fix +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:30:8: PGH004 [*] Do not add spaces between `noqa` and its colon + | +29 | # PGH004 +30 | x = 2 # noqa :X500 + | ^^^^^^^ PGH004 + | + = help: Remove space(s) before colon + +ℹ Unsafe fix +27 27 | x = 2 # noqa : X400 +28 28 | +29 29 | # PGH004 +30 |-x = 2 # noqa :X500 + 30 |+x = 2 # noqa:X500