Enable autofix for M001 (#255)

astral-sh · Sep 22, 2022 · e5b1697 · e5b1697
1 parent de9ceb2
commit e5b1697
Show file tree

Hide file tree

Showing 6 changed files with 154 additions and 24 deletions.
diff --git a/resources/test/fixtures/M001.py b/resources/test/fixtures/M001.py
@@ -16,16 +16,42 @@ def f() -> None:
     d = 1  # noqa: F841, E501
 
 
+# Valid
 _ = """Lorem ipsum dolor sit amet.
 
     https://github.com/PyCQA/pycodestyle/pull/258/files#diff-841c622497a8033d10152bfdfb15b20b92437ecdea21a260944ea86b77b51533
 
 Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.
 """  # noqa: E501
 
+# Valid
+_ = """Lorem ipsum dolor sit amet.
+
+    https://github.com/PyCQA/pycodestyle/pull/258/files#diff-841c622497a8033d10152bfdfb15b20b92437ecdea21a260944ea86b77b51533
+
+Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.
+"""  # noqa
+
+# Invalid
 _ = """Lorem ipsum dolor sit amet.
 
     https://github.com/PyCQA/pycodestyle/pull/258/files#diff-841c622497a8033d10152bfdfb15b20b92437ecdea21a260944ea86b77b51533
 
 Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.
 """  # noqa: E501, F841
+
+# Invalid
+_ = """Lorem ipsum dolor sit amet.
+
+    https://github.com/PyCQA/pycodestyle/pull/258/files#diff-841c622497a8033d10152bfdfb15b20b92437ecdea21a260944ea86b77b51533
+
+Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor.
+"""  # noqa: E501
+
+# Invalid
+_ = """Lorem ipsum dolor sit amet.
+
+    https://github.com/PyCQA/pycodestyle/pull/258/files#diff-841c622497a8033d10152bfdfb15b20b92437ecdea21a260944ea86b77b51533
+
+Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor.
+"""  # noqa
diff --git a/src/check_lines.rs b/src/check_lines.rs
@@ -2,7 +2,8 @@ use std::collections::BTreeMap;
 
 use rustpython_parser::ast::Location;
 
-use crate::checks::{Check, CheckCode, CheckKind};
+use crate::autofix::fixer;
+use crate::checks::{Check, CheckCode, CheckKind, Fix};
 use crate::noqa;
 use crate::noqa::Directive;
 use crate::settings::Settings;
@@ -28,6 +29,7 @@ pub fn check_lines(
     contents: &str,
     noqa_line_for: &[usize],
     settings: &Settings,
+    autofix: &fixer::Mode,
 ) {
     let enforce_line_too_long = settings.select.contains(&CheckCode::E501);
     let enforce_noqa = settings.select.contains(&CheckCode::M001);
@@ -113,21 +115,56 @@ pub fn check_lines(
             match directive {
                 Directive::All(column) => {
                     if matches.is_empty() {
-                        line_checks.push(Check::new(
+                        let mut check = Check::new(
                             CheckKind::UnusedNOQA(None),
                             Location::new(row + 1, column + 1),
-                        ));
+                        );
+                        if matches!(autofix, fixer::Mode::Generate | fixer::Mode::Apply) {
+                            check.amend(Fix {
+                                content: "".to_string(),
+                                start: Location::new(row + 1, column + 1),
+                                end: Location::new(row + 1, lines[row].chars().count() + 1),
+                                applied: false,
+                            });
+                        }
+                        line_checks.push(check);
                     }
                 }
                 Directive::Codes(column, codes) => {
-                    for code in &codes {
-                        if !matches.contains(code) {
-                            line_checks.push(Check::new(
-                                CheckKind::UnusedNOQA(Some(code.to_string())),
-                                Location::new(row + 1, column + 1),
-                            ));
+                    let mut invalid_codes = vec![];
+                    let mut valid_codes = vec![];
+                    for code in codes {
+                        if !matches.contains(&code) {
+                            invalid_codes.push(code);
+                        } else {
+                            valid_codes.push(code);
                         }
                     }
+
+                    if !invalid_codes.is_empty() {
+                        let mut check = Check::new(
+                            CheckKind::UnusedNOQA(Some(invalid_codes.join(", "))),
+                            Location::new(row + 1, column + 1),
+                        );
+                        if matches!(autofix, fixer::Mode::Generate | fixer::Mode::Apply) {
+                            if valid_codes.is_empty() {
+                                check.amend(Fix {
+                                    content: "".to_string(),
+                                    start: Location::new(row + 1, column + 1),
+                                    end: Location::new(row + 1, lines[row].chars().count() + 1),
+                                    applied: false,
+                                });
+                            } else {
+                                check.amend(Fix {
+                                    content: format!("  # noqa: {}", valid_codes.join(", ")),
+                                    start: Location::new(row + 1, column + 1),
+                                    end: Location::new(row + 1, lines[row].chars().count() + 1),
+                                    applied: false,
+                                });
+                            }
+                        }
+                        line_checks.push(check);
+                    }
                 }
                 Directive::None => {}
             }
@@ -162,8 +199,13 @@ mod tests {
                 extend_exclude: vec![],
                 select: BTreeSet::from_iter(vec![CheckCode::E501]),
             };
-
-            check_lines(&mut checks, line, &noqa_line_for, &settings);
+            check_lines(
+                &mut checks,
+                line,
+                &noqa_line_for,
+                &settings,
+                &fixer::Mode::Generate,
+            );
             return checks;
         };
         assert!(!check_with_max_line_length(6).is_empty());

diff --git a/src/checks.rs b/src/checks.rs
@@ -609,7 +609,7 @@ impl CheckKind {
             }
             CheckKind::UnusedNOQA(code) => match code {
                 None => "Unused `noqa` directive".to_string(),
-                Some(code) => format!("Unused `noqa` directive for {code}"),
+                Some(code) => format!("Unused `noqa` directive for: {code}"),
             },
         }
     }
@@ -618,7 +618,9 @@ impl CheckKind {
     pub fn fixable(&self) -> bool {
         matches!(
             self,
-            CheckKind::NoAssertEquals | CheckKind::UselessObjectInheritance(_)
+            CheckKind::NoAssertEquals
+                | CheckKind::UselessObjectInheritance(_)
+                | CheckKind::UnusedNOQA(_)
         )
     }
 }

diff --git a/src/linter.rs b/src/linter.rs
@@ -51,7 +51,7 @@ fn check_path(
     }
 
     // Run the lines-based checks.
-    check_lines(&mut checks, contents, &noqa_line_for, settings);
+    check_lines(&mut checks, contents, &noqa_line_for, settings, autofix);
 
     checks
 }

diff --git a/src/noqa.rs b/src/noqa.rs
@@ -5,7 +5,7 @@ use regex::Regex;
 use rustpython_parser::lexer::{LexResult, Tok};
 
 static NO_QA_REGEX: Lazy<Regex> = Lazy::new(|| {
-    Regex::new(r"(?i)(?P<noqa># noqa(?::\s?(?P<codes>([A-Z]+[0-9]+(?:[,\s]+)?)+))?)")
+    Regex::new(r"(?i)(?P<noqa>\s*# noqa(?::\s?(?P<codes>([A-Z]+[0-9]+(?:[,\s]+)?)+))?)")
         .expect("Invalid regex")
 });
 static SPLIT_COMMA_REGEX: Lazy<Regex> = Lazy::new(|| Regex::new(r"[,\s]").expect("Invalid regex"));

diff --git a/src/snapshots/ruff__linter__tests__m001.snap b/src/snapshots/ruff__linter__tests__m001.snap
@@ -6,24 +6,84 @@ expression: checks
     UnusedNOQA: ~
   location:
     row: 9
-    column: 12
-  fix: ~
+    column: 10
+  fix:
+    content: ""
+    start:
+      row: 9
+      column: 10
+    end:
+      row: 9
+      column: 18
+    applied: false
 - kind:
     UnusedNOQA: E501
   location:
     row: 13
-    column: 12
-  fix: ~
+    column: 10
+  fix:
+    content: ""
+    start:
+      row: 13
+      column: 10
+    end:
+      row: 13
+      column: 24
+    applied: false
 - kind:
     UnusedNOQA: E501
   location:
     row: 16
-    column: 12
-  fix: ~
+    column: 10
+  fix:
+    content: "  # noqa: F841"
+    start:
+      row: 16
+      column: 10
+    end:
+      row: 16
+      column: 30
+    applied: false
 - kind:
     UnusedNOQA: F841
   location:
-    row: 31
-    column: 6
-  fix: ~
+    row: 41
+    column: 4
+  fix:
+    content: "  # noqa: E501"
+    start:
+      row: 41
+      column: 4
+    end:
+      row: 41
+      column: 24
+    applied: false
+- kind:
+    UnusedNOQA: E501
+  location:
+    row: 49
+    column: 4
+  fix:
+    content: ""
+    start:
+      row: 49
+      column: 4
+    end:
+      row: 49
+      column: 18
+    applied: false
+- kind:
+    UnusedNOQA: ~
+  location:
+    row: 57
+    column: 4
+  fix:
+    content: ""
+    start:
+      row: 57
+      column: 4
+    end:
+      row: 57
+      column: 12
+    applied: false
-Original file line number
+Diff line change
@@ Expand Up / @@ -51,7 +51,7 @@ fn check_path( @@
         }
         // Run the lines-based checks.
-        check_lines(&mut checks, contents, &noqa_line_for, settings);
+        check_lines(&mut checks, contents, &noqa_line_for, settings, autofix);
         checks
     }
@@ Expand Down @@