diff --git a/crates/ruff_linter/src/rules/ruff/rules/invalid_suppression_comment.rs b/crates/ruff_linter/src/rules/ruff/rules/invalid_suppression_comment.rs index 8e6ac88dd3..54e2da0434 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/invalid_suppression_comment.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/invalid_suppression_comment.rs @@ -1,6 +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 @@ -25,15 +26,34 @@ use crate::AlwaysFixableViolation; /// - [Ruff error suppression](https://docs.astral.sh/ruff/linter/#error-suppression) #[derive(ViolationMetadata)] #[violation_metadata(preview_since = "0.14.9")] -pub(crate) struct InvalidSuppressionComment; +pub(crate) struct InvalidSuppressionComment { + pub kind: InvalidSuppressionCommentKind, +} -impl AlwaysFixableViolation for InvalidSuppressionComment { +impl Violation for InvalidSuppressionComment { #[derive_message_formats] fn message(&self) -> String { - "Invalid suppression comment".to_string() + let msg = match self.kind { + InvalidSuppressionCommentKind::Invalid(InvalidSuppressionKind::Indentation) => { + "unexpected indentation".to_string() + } + InvalidSuppressionCommentKind::Invalid(InvalidSuppressionKind::Trailing) => { + "trailing comments are not supported".to_string() + } + InvalidSuppressionCommentKind::Invalid(InvalidSuppressionKind::Unmatched) => { + "no matching 'disable' comment".to_string() + } + InvalidSuppressionCommentKind::Error(error) => format!("{error}"), + }; + format!("Invalid suppression comment: {msg}") } - fn fix_title(&self) -> String { - "Remove invalid suppression comment".to_string() + fn fix_title(&self) -> Option { + Some("Remove invalid suppression comment".to_string()) } } + +pub(crate) enum InvalidSuppressionCommentKind { + Invalid(InvalidSuppressionKind), + Error(ParseErrorKind), +} diff --git a/crates/ruff_linter/src/suppression.rs b/crates/ruff_linter/src/suppression.rs index 7b4e57a67a..f7778d1403 100644 --- a/crates/ruff_linter/src/suppression.rs +++ b/crates/ruff_linter/src/suppression.rs @@ -19,7 +19,8 @@ use crate::codes::Rule; use crate::fix::edits::delete_comment; use crate::preview::is_range_suppressions_enabled; use crate::rules::ruff::rules::{ - UnmatchedSuppressionComment, UnusedCodes, UnusedNOQA, UnusedNOQAKind, + InvalidSuppressionComment, InvalidSuppressionCommentKind, UnmatchedSuppressionComment, + UnusedCodes, UnusedNOQA, UnusedNOQAKind, }; use crate::settings::LinterSettings; @@ -133,7 +134,7 @@ impl Suppressions { } pub(crate) fn is_empty(&self) -> bool { - self.valid.is_empty() + self.valid.is_empty() && self.invalid.is_empty() && self.errors.is_empty() } /// Check if a diagnostic is suppressed by any known range suppressions @@ -162,7 +163,12 @@ impl Suppressions { } pub(crate) fn check_suppressions(&self, context: &LintContext, locator: &Locator) { - if !context.any_rule_enabled(&[Rule::UnusedNOQA, Rule::InvalidRuleCode]) { + if !context.any_rule_enabled(&[ + Rule::UnusedNOQA, + Rule::InvalidRuleCode, + Rule::InvalidSuppressionComment, + Rule::UnmatchedSuppressionComment, + ]) { return; } @@ -223,6 +229,40 @@ impl Suppressions { } } + for error in &self.errors { + // treat comments with no codes as unused suppression + let mut diagnostic = if error.kind == ParseErrorKind::MissingCodes { + context.report_diagnostic( + UnusedNOQA { + codes: Some(UnusedCodes::default()), + kind: UnusedNOQAKind::Suppression, + }, + error.range, + ) + } else { + 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, + ))); + } + let unmatched = self .valid .iter() @@ -235,21 +275,6 @@ impl Suppressions { for range in unmatched { context.report_diagnostic(UnmatchedSuppressionComment {}, range); } - - for error in self - .errors - .iter() - .filter(|error| error.kind == ParseErrorKind::MissingCodes) - { - let mut diagnostic = context.report_diagnostic( - UnusedNOQA { - codes: Some(UnusedCodes::default()), - kind: UnusedNOQAKind::Suppression, - }, - error.range, - ); - diagnostic.set_fix(Fix::safe_edit(delete_comment(error.range, locator))); - } } } @@ -407,7 +432,7 @@ impl<'a> SuppressionsBuilder<'a> { } #[derive(Copy, Clone, Debug, Eq, Error, PartialEq)] -enum ParseErrorKind { +pub(crate) enum ParseErrorKind { #[error("not a suppression comment")] NotASuppression,