Move rule-enabled checks to local logic, extract some bits

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

@@ -1,7 +1,7 @@
 use ruff_macros::{ViolationMetadata, derive_message_formats};
 
+use crate::AlwaysFixableViolation;
 use crate::suppression::{InvalidSuppressionKind, ParseErrorKind};
-use crate::{AlwaysFixableViolation, Violation};
 
 /// ## What it does
 /// Checks for invalid suppression comments

crates/ruff_linter/src/suppression.rs

@@ -163,119 +163,134 @@ impl Suppressions {
     }
 
     pub(crate) fn check_suppressions(&self, context: &LintContext, locator: &Locator) {
-        // TODO: wrap these around relevant bits below
+        if context.is_rule_enabled(Rule::UnusedNOQA) {
-        if !context.any_rule_enabled(&[
+            let unused = self
-            Rule::UnusedNOQA,
+                .valid
-            Rule::InvalidRuleCode,
+                .iter()
-            Rule::InvalidSuppressionComment,
+                .filter(|suppression| !suppression.used.get());
-            Rule::UnmatchedSuppressionComment,
-        ]) {
-            return;
-        }
 
-        let unused = self
+            for suppression in unused {
-            .valid
+                let Ok(rule) = Rule::from_code(&suppression.code) else {
-            .iter()
+                    continue; // TODO: invalid code
-            .filter(|suppression| !suppression.used.get());
+                };
+                for comment in &suppression.comments {
+                    let (range, edit) =
+                        Suppressions::delete_code_or_comment(locator, suppression, comment);
 
-        for suppression in unused {
+                    let codes = if context.is_rule_enabled(rule) {
-            let Ok(rule) = Rule::from_code(&suppression.code) else {
+                        UnusedCodes {
-                continue; // TODO: invalid code
+                            unmatched: vec![suppression.code.to_string()],
-            };
+                            ..Default::default()
-            for comment in &suppression.comments {
+                        }
-                let mut range = comment.range;
-                let edit = if comment.codes.len() == 1 {
-                    delete_comment(comment.range, locator)
-                } else {
-                    let code_index = comment
-                        .codes
-                        .iter()
-                        .position(|range| locator.slice(range) == suppression.code)
-                        .unwrap();
-                    range = comment.codes[code_index];
-                    let code_range = if code_index < (comment.codes.len() - 1) {
-                        TextRange::new(
-                            comment.codes[code_index].start(),
-                            comment.codes[code_index + 1].start(),
-                        )
                     } else {
-                        TextRange::new(
+                        UnusedCodes {
-                            comment.codes[code_index - 1].end(),
+                            disabled: vec![suppression.code.to_string()],
-                            comment.codes[code_index].end(),
+                            ..Default::default()
-                        )
+                        }
                     };
-                    Edit::range_deletion(code_range)
-                };
 
-                let codes = if context.is_rule_enabled(rule) {
+                    let mut diagnostic = context.report_diagnostic(
-                    UnusedCodes {
+                        UnusedNOQA {
-                        unmatched: vec![suppression.code.to_string()],
+                            codes: Some(codes),
-                        ..Default::default()
+                            kind: UnusedNOQAKind::Suppression,
-                    }
+                        },
-                } else {
+                        range,
-                    UnusedCodes {
+                    );
-                        disabled: vec![suppression.code.to_string()],
+                    diagnostic.set_fix(Fix::safe_edit(edit));
-                        ..Default::default()
+                }
-                    }
-                };
-
-                let mut diagnostic = context.report_diagnostic(
-                    UnusedNOQA {
-                        codes: Some(codes),
-                        kind: UnusedNOQAKind::Suppression,
-                    },
-                    range,
-                );
-                diagnostic.set_fix(Fix::safe_edit(edit));
             }
-        }
 
-        for error in &self.errors {
             // treat comments with no codes as unused suppression
-            let mut diagnostic = if error.kind == ParseErrorKind::MissingCodes {
+            for error in self
-                context.report_diagnostic(
+                .errors
+                .iter()
+                .filter(|error| error.kind == ParseErrorKind::MissingCodes)
+            {
+                let mut diagnostic = context.report_diagnostic(
                     UnusedNOQA {
                         codes: Some(UnusedCodes::default()),
                         kind: UnusedNOQAKind::Suppression,
                     },
                     error.range,
-                )
+                );
-            } else {
+                diagnostic.set_fix(Fix::safe_edit(delete_comment(error.range, locator)));
-                context.report_diagnostic(
+            }
+        }
+
+        if context.is_rule_enabled(Rule::InvalidSuppressionComment) {
+            // missing codes already handled above, report the rest as invalid comments
+            for error in self
+                .errors
+                .iter()
+                .filter(|error| error.kind != ParseErrorKind::MissingCodes)
+            {
+                let mut diagnostic = context.report_diagnostic(
                     InvalidSuppressionComment {
                         kind: InvalidSuppressionCommentKind::Error(error.kind),
                     },
                     error.range,
+                );
+                diagnostic.set_fix(Fix::safe_edit(delete_comment(error.range, locator)));
+            }
+
+            for invalid in &self.invalid {
+                let mut diagnostic = context.report_diagnostic(
+                    InvalidSuppressionComment {
+                        kind: InvalidSuppressionCommentKind::Invalid(invalid.kind),
+                    },
+                    invalid.comment.range,
+                );
+                diagnostic.set_fix(Fix::safe_edit(delete_comment(
+                    invalid.comment.range,
+                    locator,
+                )));
+            }
+        }
+
+        if context.is_rule_enabled(Rule::UnmatchedSuppressionComment) {
+            for range in self
+                .valid
+                .iter()
+                .filter(|suppression| {
+                    suppression.comments.len() == 1
+                        && suppression.comments[0].action == SuppressionAction::Disable
+                })
+                .map(|suppression| suppression.comments[0].range)
+                .collect::<FxHashSet<TextRange>>()
+            {
+                context.report_diagnostic(UnmatchedSuppressionComment {}, range);
+            }
+        }
+    }
+    fn delete_code_or_comment(
+        locator: &Locator<'_>,
+        suppression: &Suppression,
+        comment: &SuppressionComment,
+    ) -> (TextRange, Edit) {
+        let mut range = comment.range;
+        let edit = if comment.codes.len() == 1 {
+            delete_comment(comment.range, locator)
+        } else {
+            let code_index = comment
+                .codes
+                .iter()
+                .position(|range| locator.slice(range) == suppression.code)
+                .unwrap();
+            range = comment.codes[code_index];
+            let code_range = if code_index < (comment.codes.len() - 1) {
+                TextRange::new(
+                    comment.codes[code_index].start(),
+                    comment.codes[code_index + 1].start(),
+                )
+            } else {
+                TextRange::new(
+                    comment.codes[code_index - 1].end(),
+                    comment.codes[code_index].end(),
                 )
             };
-            diagnostic.set_fix(Fix::safe_edit(delete_comment(error.range, locator)));
+            Edit::range_deletion(code_range)
-        }
+        };
-
+        (range, edit)
-        for invalid in &self.invalid {
-            let mut diagnostic = context.report_diagnostic(
-                InvalidSuppressionComment {
-                    kind: InvalidSuppressionCommentKind::Invalid(invalid.kind),
-                },
-                invalid.comment.range,
-            );
-            diagnostic.set_fix(Fix::safe_edit(delete_comment(
-                invalid.comment.range,
-                locator,
-            )));
-        }
-
-        let unmatched = self
-            .valid
-            .iter()
-            .filter(|suppression| {
-                suppression.comments.len() == 1
-                    && suppression.comments[0].action == SuppressionAction::Disable
-            })
-            .map(|suppression| suppression.comments[0].range)
-            .collect::<FxHashSet<TextRange>>();
-        for range in unmatched {
-            context.report_diagnostic(UnmatchedSuppressionComment {}, range);
-        }
     }
 }