Implement RUF028 to detect useless formatter suppression comments

crates/ruff_linter/resources/test/fixtures/ruff/RUF028.py

@@ -0,0 +1,50 @@
+def fmt_off_between_lists():
+    test_list = [
+        # fmt: off
+        1,
+        2,
+        3,
+    ]
+
+# note: the second `fmt: skip`` should be OK
+def fmt_skip_on_own_line():
+    # fmt: skip
+    pass # fmt: skip
+
+
+@fmt_skip_on_own_line
+# fmt: off
+@fmt_off_between_lists
+def fmt_off_between_decorators():
+    pass
+
+@fmt_off_between_decorators
+# fmt: off
+@fmt_off_between_lists
+class FmtOffBetweenClassDecorators:
+    ...
+
+def fmt_off_in_else():
+    x = [1, 2, 3]
+    for val in x:
+        print(x)
+    # fmt: off
+    else:
+        print("done")
+    while False:
+        print("while")
+        # fmt: off
+    # fmt: off
+    else:
+        print("done")
+    if len(x) > 3:
+        print("huh?")
+    # fmt: on
+    # fmt: off
+    else:
+        print("expected")
+
+def fmt_on_trailing():
+    # fmt: off
+    val = 5 # fmt: on
+    pass # fmt: on

crates/ruff_linter/src/checkers/ast/analyze/module.rs

@@ -2,11 +2,14 @@ use ruff_python_ast::Suite;
 
 use crate::checkers::ast::Checker;
 use crate::codes::Rule;
-use crate::rules::flake8_bugbear;
+use crate::rules::{flake8_bugbear, ruff};
 
 /// Run lint rules over a module.
 pub(crate) fn module(suite: &Suite, checker: &mut Checker) {
     if checker.enabled(Rule::FStringDocstring) {
         flake8_bugbear::rules::f_string_docstring(checker, suite);
     }
+    if checker.enabled(Rule::InvalidFormatterSuppressionComment) {
+        ruff::rules::ignored_formatter_suppression_comment(checker, suite);
+    }
 }

crates/ruff_linter/src/checkers/noqa.rs

@@ -3,12 +3,13 @@
 use std::path::Path;
 
 use itertools::Itertools;
-use ruff_text_size::{Ranged, TextLen, TextRange};
+use ruff_text_size::Ranged;
 
 use ruff_diagnostics::{Diagnostic, Edit, Fix};
-use ruff_python_trivia::{CommentRanges, PythonWhitespace};
+use ruff_python_trivia::CommentRanges;
 use ruff_source_file::Locator;
 
+use crate::fix::edits::delete_comment;
 use crate::noqa;
 use crate::noqa::{Directive, FileExemption, NoqaDirectives, NoqaMapping};
 use crate::registry::{AsRule, Rule, RuleSet};
@@ -111,7 +112,8 @@ pub(crate) fn check_noqa(
                     if line.matches.is_empty() {
                         let mut diagnostic =
                             Diagnostic::new(UnusedNOQA { codes: None }, directive.range());
-                        diagnostic.set_fix(Fix::safe_edit(delete_noqa(directive.range(), locator)));
+                        diagnostic
+                            .set_fix(Fix::safe_edit(delete_comment(directive.range(), locator)));
 
                         diagnostics.push(diagnostic);
                     }
@@ -177,8 +179,10 @@ pub(crate) fn check_noqa(
                             directive.range(),
                         );
                         if valid_codes.is_empty() {
-                            diagnostic
-                                .set_fix(Fix::safe_edit(delete_noqa(directive.range(), locator)));
+                            diagnostic.set_fix(Fix::safe_edit(delete_comment(
+                                directive.range(),
+                                locator,
+                            )));
                         } else {
                             diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement(
                                 format!("# noqa: {}", valid_codes.join(", ")),
@@ -195,48 +199,3 @@ pub(crate) fn check_noqa(
     ignored_diagnostics.sort_unstable();
     ignored_diagnostics
 }
-
-/// Generate a [`Edit`] to delete a `noqa` directive.
-fn delete_noqa(range: TextRange, locator: &Locator) -> Edit {
-    let line_range = locator.line_range(range.start());
-
-    // Compute the leading space.
-    let prefix = locator.slice(TextRange::new(line_range.start(), range.start()));
-    let leading_space_len = prefix.text_len() - prefix.trim_whitespace_end().text_len();
-
-    // Compute the trailing space.
-    let suffix = locator.slice(TextRange::new(range.end(), line_range.end()));
-    let trailing_space_len = suffix.text_len() - suffix.trim_whitespace_start().text_len();
-
-    // Ex) `# noqa`
-    if line_range
-        == TextRange::new(
-            range.start() - leading_space_len,
-            range.end() + trailing_space_len,
-        )
-    {
-        let full_line_end = locator.full_line_end(line_range.end());
-        Edit::deletion(line_range.start(), full_line_end)
-    }
-    // Ex) `x = 1  # noqa`
-    else if range.end() + trailing_space_len == line_range.end() {
-        Edit::deletion(range.start() - leading_space_len, line_range.end())
-    }
-    // Ex) `x = 1  # noqa  # type: ignore`
-    else if locator
-        .slice(TextRange::new(
-            range.end() + trailing_space_len,
-            line_range.end(),
-        ))
-        .starts_with('#')
-    {
-        Edit::deletion(range.start(), range.end() + trailing_space_len)
-    }
-    // Ex) `x = 1  # noqa here`
-    else {
-        Edit::deletion(
-            range.start() + "# ".text_len(),
-            range.end() + trailing_space_len,
-        )
-    }
-}

crates/ruff_linter/src/codes.rs

@@ -945,6 +945,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
         (Ruff, "025") => (RuleGroup::Preview, rules::ruff::rules::UnnecessaryDictComprehensionForIterable),
         (Ruff, "026") => (RuleGroup::Preview, rules::ruff::rules::DefaultFactoryKwarg),
         (Ruff, "027") => (RuleGroup::Preview, rules::ruff::rules::MissingFStringSyntax),
+        (Ruff, "028") => (RuleGroup::Preview, rules::ruff::rules::InvalidFormatterSuppressionComment),
         (Ruff, "100") => (RuleGroup::Stable, rules::ruff::rules::UnusedNOQA),
         (Ruff, "200") => (RuleGroup::Stable, rules::ruff::rules::InvalidPyprojectToml),
         #[cfg(feature = "test-rules")]

crates/ruff_linter/src/fix/edits.rs

@@ -62,6 +62,51 @@ pub(crate) fn delete_stmt(
     }
 }
 
+/// Generate a [`Edit`] to delete a comment (for example: a `noqa` directive).
+pub(crate) fn delete_comment(range: TextRange, locator: &Locator) -> Edit {
+    let line_range = locator.line_range(range.start());
+
+    // Compute the leading space.
+    let prefix = locator.slice(TextRange::new(line_range.start(), range.start()));
+    let leading_space_len = prefix.text_len() - prefix.trim_whitespace_end().text_len();
+
+    // Compute the trailing space.
+    let suffix = locator.slice(TextRange::new(range.end(), line_range.end()));
+    let trailing_space_len = suffix.text_len() - suffix.trim_whitespace_start().text_len();
+
+    // Ex) `# noqa`
+    if line_range
+        == TextRange::new(
+            range.start() - leading_space_len,
+            range.end() + trailing_space_len,
+        )
+    {
+        let full_line_end = locator.full_line_end(line_range.end());
+        Edit::deletion(line_range.start(), full_line_end)
+    }
+    // Ex) `x = 1  # noqa`
+    else if range.end() + trailing_space_len == line_range.end() {
+        Edit::deletion(range.start() - leading_space_len, line_range.end())
+    }
+    // Ex) `x = 1  # noqa  # type: ignore`
+    else if locator
+        .slice(TextRange::new(
+            range.end() + trailing_space_len,
+            line_range.end(),
+        ))
+        .starts_with('#')
+    {
+        Edit::deletion(range.start(), range.end() + trailing_space_len)
+    }
+    // Ex) `x = 1  # noqa here`
+    else {
+        Edit::deletion(
+            range.start() + "# ".text_len(),
+            range.end() + trailing_space_len,
+        )
+    }
+}
+
 /// Generate a `Fix` to remove the specified imports from an `import` statement.
 pub(crate) fn remove_unused_imports<'a>(
     member_names: impl Iterator<Item = &'a str>,

crates/ruff_linter/src/rules/ruff/mod.rs

@@ -49,6 +49,7 @@ mod tests {
     #[test_case(Rule::MissingFStringSyntax, Path::new("RUF027_0.py"))]
     #[test_case(Rule::MissingFStringSyntax, Path::new("RUF027_1.py"))]
     #[test_case(Rule::MissingFStringSyntax, Path::new("RUF027_2.py"))]
+    #[test_case(Rule::InvalidFormatterSuppressionComment, Path::new("RUF028.py"))]
     fn rules(rule_code: Rule, path: &Path) -> Result<()> {
         let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy());
         let diagnostics = test_path(