[pygrep-hooks] Improve blanket-noqa error message (PGH004)

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

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`.

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<String> {
+        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),
+                ));
+            }
         }
     }
 }

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