diff --git a/crates/ruff_linter/resources/test/fixtures/ruff/suppressions.py b/crates/ruff_linter/resources/test/fixtures/ruff/suppressions.py index f8a3c882aa..2a4b06898a 100644 --- a/crates/ruff_linter/resources/test/fixtures/ruff/suppressions.py +++ b/crates/ruff_linter/resources/test/fixtures/ruff/suppressions.py @@ -86,3 +86,19 @@ def f(): # Multiple codes but none are used # ruff: disable[E741, F401, F841] print("hello") + + +def f(): + # Unknown rule codes + # ruff: disable[YF829] + # ruff: disable[F841, RQW320] + value = 0 + # ruff: enable[F841, RQW320] + # ruff: enable[YF829] + + +def f(): + # External rule codes should be ignored + # ruff: disable[TK421] + print("hello") + # ruff: enable[TK421] diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index ebec5f4acc..04fb7eee11 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -1063,6 +1063,8 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Ruff, "100") => rules::ruff::rules::UnusedNOQA, (Ruff, "101") => rules::ruff::rules::RedirectedNOQA, (Ruff, "102") => rules::ruff::rules::InvalidRuleCode, + (Ruff, "103") => rules::ruff::rules::InvalidSuppressionComment, + (Ruff, "104") => rules::ruff::rules::UnmatchedSuppressionComment, (Ruff, "200") => rules::ruff::rules::InvalidPyprojectToml, #[cfg(any(feature = "test-rules", test))] diff --git a/crates/ruff_linter/src/rules/ruff/mod.rs b/crates/ruff_linter/src/rules/ruff/mod.rs index c2d03fb1ae..9bc6363731 100644 --- a/crates/ruff_linter/src/rules/ruff/mod.rs +++ b/crates/ruff_linter/src/rules/ruff/mod.rs @@ -313,12 +313,20 @@ mod tests { Rule::UnusedVariable, Rule::AmbiguousVariableName, Rule::UnusedNOQA, - ]), + Rule::InvalidRuleCode, + Rule::InvalidSuppressionComment, + Rule::UnmatchedSuppressionComment, + ]) + .with_external_rules(&["TK421"]), &settings::LinterSettings::for_rules(vec![ Rule::UnusedVariable, Rule::AmbiguousVariableName, Rule::UnusedNOQA, + Rule::InvalidRuleCode, + Rule::InvalidSuppressionComment, + Rule::UnmatchedSuppressionComment, ]) + .with_external_rules(&["TK421"]) .with_preview_mode(), ); Ok(()) diff --git a/crates/ruff_linter/src/rules/ruff/rules/invalid_rule_code.rs b/crates/ruff_linter/src/rules/ruff/rules/invalid_rule_code.rs index 3c9cde312e..56cf058509 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/invalid_rule_code.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/invalid_rule_code.rs @@ -9,6 +9,21 @@ use crate::registry::Rule; use crate::rule_redirects::get_redirect_target; use crate::{AlwaysFixableViolation, Edit, Fix}; +#[derive(Debug, PartialEq, Eq)] +pub(crate) enum InvalidRuleCodeKind { + Noqa, + Suppression, +} + +impl InvalidRuleCodeKind { + fn as_str(&self) -> &str { + match self { + InvalidRuleCodeKind::Noqa => "`# noqa`", + InvalidRuleCodeKind::Suppression => "suppression", + } + } +} + /// ## What it does /// Checks for `noqa` codes that are invalid. /// @@ -36,12 +51,17 @@ use crate::{AlwaysFixableViolation, Edit, Fix}; #[violation_metadata(preview_since = "0.11.4")] pub(crate) struct InvalidRuleCode { pub(crate) rule_code: String, + pub(crate) kind: InvalidRuleCodeKind, } impl AlwaysFixableViolation for InvalidRuleCode { #[derive_message_formats] fn message(&self) -> String { - format!("Invalid rule code in `# noqa`: {}", self.rule_code) + format!( + "Invalid rule code in {}: {}", + self.kind.as_str(), + self.rule_code + ) } fn fix_title(&self) -> String { @@ -100,6 +120,7 @@ fn all_codes_invalid_diagnostic( .map(Code::as_str) .collect::>() .join(", "), + kind: InvalidRuleCodeKind::Noqa, }, directive.range(), ) @@ -116,6 +137,7 @@ fn some_codes_are_invalid_diagnostic( .report_diagnostic( InvalidRuleCode { rule_code: invalid_code.to_string(), + kind: InvalidRuleCodeKind::Noqa, }, invalid_code.range(), ) 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 new file mode 100644 index 0000000000..56e09ad936 --- /dev/null +++ b/crates/ruff_linter/src/rules/ruff/rules/invalid_suppression_comment.rs @@ -0,0 +1,59 @@ +use ruff_macros::{ViolationMetadata, derive_message_formats}; + +use crate::AlwaysFixableViolation; +use crate::suppression::{InvalidSuppressionKind, ParseErrorKind}; + +/// ## What it does +/// Checks for invalid suppression comments +/// +/// ## Why is this bad? +/// Invalid suppression comments are ignored by Ruff, and should either +/// be fixed or removed to avoid confusion. +/// +/// ## Example +/// ```python +/// ruff: disable # missing codes +/// ``` +/// +/// Use instead: +/// ```python +/// # ruff: disable[E501] +/// ``` +/// +/// Or delete the invalid suppression comment. +/// +/// ## References +/// - [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) kind: InvalidSuppressionCommentKind, +} + +impl AlwaysFixableViolation for InvalidSuppressionComment { + #[derive_message_formats] + fn message(&self) -> 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() + } +} + +pub(crate) enum InvalidSuppressionCommentKind { + Invalid(InvalidSuppressionKind), + Error(ParseErrorKind), +} diff --git a/crates/ruff_linter/src/rules/ruff/rules/mod.rs b/crates/ruff_linter/src/rules/ruff/rules/mod.rs index 206a504e74..70363d3f8b 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/mod.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/mod.rs @@ -22,6 +22,7 @@ pub(crate) use invalid_formatter_suppression_comment::*; pub(crate) use invalid_index_type::*; pub(crate) use invalid_pyproject_toml::*; pub(crate) use invalid_rule_code::*; +pub(crate) use invalid_suppression_comment::*; pub(crate) use legacy_form_pytest_raises::*; pub(crate) use logging_eager_conversion::*; pub(crate) use map_int_version_parsing::*; @@ -46,6 +47,7 @@ pub(crate) use starmap_zip::*; pub(crate) use static_key_dict_comprehension::*; #[cfg(any(feature = "test-rules", test))] pub(crate) use test_rules::*; +pub(crate) use unmatched_suppression_comment::*; pub(crate) use unnecessary_cast_to_int::*; pub(crate) use unnecessary_iterable_allocation_for_first_element::*; pub(crate) use unnecessary_key_check::*; @@ -87,6 +89,7 @@ mod invalid_formatter_suppression_comment; mod invalid_index_type; mod invalid_pyproject_toml; mod invalid_rule_code; +mod invalid_suppression_comment; mod legacy_form_pytest_raises; mod logging_eager_conversion; mod map_int_version_parsing; @@ -113,6 +116,7 @@ mod static_key_dict_comprehension; mod suppression_comment_visitor; #[cfg(any(feature = "test-rules", test))] pub(crate) mod test_rules; +mod unmatched_suppression_comment; mod unnecessary_cast_to_int; mod unnecessary_iterable_allocation_for_first_element; mod unnecessary_key_check; diff --git a/crates/ruff_linter/src/rules/ruff/rules/unmatched_suppression_comment.rs b/crates/ruff_linter/src/rules/ruff/rules/unmatched_suppression_comment.rs new file mode 100644 index 0000000000..818cfd2150 --- /dev/null +++ b/crates/ruff_linter/src/rules/ruff/rules/unmatched_suppression_comment.rs @@ -0,0 +1,47 @@ +use ruff_macros::{ViolationMetadata, derive_message_formats}; + +use crate::Violation; + +/// ## What it does +/// Checks for unmatched range suppression comments +/// +/// ## Why is this bad? +/// Unmatched range suppression comments can inadvertently suppress violations +/// over larger sections of code than intended, particularly at module scope. +/// +/// ## Example +/// ```python +/// def foo(): +/// ruff: disable[E501] # unmatched +/// REALLY_LONG_VALUES = [ +/// ... +/// ] +/// +/// print(REALLY_LONG_VALUE) +/// ``` +/// +/// Use instead: +/// ```python +/// def foo(): +/// ... +/// # ruff: disable[E501] +/// REALLY_LONG_VALUES = [ +/// ... +/// ] +/// # ruff: enable[E501] +/// +/// print(REALLY_LONG_VALUE) +/// ``` +/// +/// ## References +/// - [Ruff error suppression](https://docs.astral.sh/ruff/linter/#error-suppression) +#[derive(ViolationMetadata)] +#[violation_metadata(preview_since = "0.14.9")] +pub(crate) struct UnmatchedSuppressionComment; + +impl Violation for UnmatchedSuppressionComment { + #[derive_message_formats] + fn message(&self) -> String { + "Suppression comment without matching `#ruff:enable` comment".to_string() + } +} diff --git a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__range_suppressions.snap b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__range_suppressions.snap index 24a43ade5e..a6eb782f31 100644 --- a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__range_suppressions.snap +++ b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__range_suppressions.snap @@ -6,8 +6,8 @@ source: crates/ruff_linter/src/rules/ruff/mod.rs +linter.preview = enabled --- Summary --- -Removed: 14 -Added: 11 +Removed: 15 +Added: 25 --- Removed --- E741 Ambiguous variable name: `I` @@ -238,8 +238,60 @@ help: Remove assignment to unused variable `I` note: This is an unsafe fix and may change runtime behavior +F841 [*] Local variable `value` is assigned to but never used + --> suppressions.py:95:5 + | +93 | # ruff: disable[YF829] +94 | # ruff: disable[F841, RQW320] +95 | value = 0 + | ^^^^^ +96 | # ruff: enable[F841, RQW320] +97 | # ruff: enable[YF829] + | +help: Remove assignment to unused variable `value` +92 | # Unknown rule codes +93 | # ruff: disable[YF829] +94 | # ruff: disable[F841, RQW320] + - value = 0 +95 + pass +96 | # ruff: enable[F841, RQW320] +97 | # ruff: enable[YF829] +98 | +note: This is an unsafe fix and may change runtime behavior + + --- Added --- +RUF104 Suppression comment without matching `#ruff:enable` comment + --> suppressions.py:11:5 + | + 9 | # These should both be ignored by the implicit range suppression. +10 | # Should also generate an "unmatched suppression" warning. +11 | # ruff:disable[E741,F841] + | ^^^^^^^^^^^^^^^^^^^^^^^^^ +12 | I = 1 + | + + +RUF103 [*] Invalid suppression comment: no matching 'disable' comment + --> suppressions.py:19:5 + | +17 | # should be generated. +18 | I = 1 +19 | # ruff: enable[E741, F841] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: Remove invalid suppression comment +16 | # Neither warning is ignored, and an "unmatched suppression" +17 | # should be generated. +18 | I = 1 + - # ruff: enable[E741, F841] +19 | +20 | +21 | def f(): +note: This is an unsafe fix and may change runtime behavior + + RUF100 [*] Unused suppression (non-enabled: `E501`) --> suppressions.py:46:5 | @@ -298,6 +350,17 @@ help: Remove unused `noqa` directive 58 | +RUF104 Suppression comment without matching `#ruff:enable` comment + --> suppressions.py:61:5 + | +59 | def f(): +60 | # TODO: Duplicate codes should be counted as duplicate, not unused +61 | # ruff: disable[F841, F841] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ +62 | foo = 0 + | + + RUF100 [*] Unused suppression (unused: `F841`) --> suppressions.py:61:21 | @@ -318,6 +381,29 @@ help: Remove unused suppression 64 | +RUF104 Suppression comment without matching `#ruff:enable` comment + --> suppressions.py:68:5 + | +66 | # Overlapping range suppressions, one should be marked as used, +67 | # and the other should trigger an unused suppression diagnostic +68 | # ruff: disable[F841] + | ^^^^^^^^^^^^^^^^^^^^^ +69 | # ruff: disable[F841] +70 | foo = 0 + | + + +RUF104 Suppression comment without matching `#ruff:enable` comment + --> suppressions.py:69:5 + | +67 | # and the other should trigger an unused suppression diagnostic +68 | # ruff: disable[F841] +69 | # ruff: disable[F841] + | ^^^^^^^^^^^^^^^^^^^^^ +70 | foo = 0 + | + + RUF100 [*] Unused suppression (unused: `F841`) --> suppressions.py:69:5 | @@ -337,6 +423,17 @@ help: Remove unused suppression 71 | +RUF104 Suppression comment without matching `#ruff:enable` comment + --> suppressions.py:75:5 + | +73 | def f(): +74 | # Multiple codes but only one is used +75 | # ruff: disable[E741, F401, F841] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +76 | foo = 0 + | + + RUF100 [*] Unused suppression (unused: `E741`) --> suppressions.py:75:21 | @@ -377,6 +474,17 @@ help: Remove unused suppression 78 | +RUF104 Suppression comment without matching `#ruff:enable` comment + --> suppressions.py:81:5 + | +79 | def f(): +80 | # Multiple codes but only two are used +81 | # ruff: disable[E741, F401, F841] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +82 | I = 0 + | + + RUF100 [*] Unused suppression (non-enabled: `F401`) --> suppressions.py:81:27 | @@ -397,6 +505,17 @@ help: Remove unused suppression 84 | +RUF104 Suppression comment without matching `#ruff:enable` comment + --> suppressions.py:87:5 + | +85 | def f(): +86 | # Multiple codes but none are used +87 | # ruff: disable[E741, F401, F841] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +88 | print("hello") + | + + RUF100 [*] Unused suppression (unused: `E741`) --> suppressions.py:87:21 | @@ -413,6 +532,8 @@ help: Remove unused suppression - # ruff: disable[E741, F401, F841] 87 + # ruff: disable[F401, F841] 88 | print("hello") +89 | +90 | RUF100 [*] Unused suppression (non-enabled: `F401`) @@ -431,6 +552,8 @@ help: Remove unused suppression - # ruff: disable[E741, F401, F841] 87 + # ruff: disable[E741, F841] 88 | print("hello") +89 | +90 | RUF100 [*] Unused suppression (unused: `F841`) @@ -449,3 +572,118 @@ help: Remove unused suppression - # ruff: disable[E741, F401, F841] 87 + # ruff: disable[E741, F401] 88 | print("hello") +89 | +90 | + + +RUF102 [*] Invalid rule code in suppression: YF829 + --> suppressions.py:93:5 + | +91 | def f(): +92 | # Unknown rule codes +93 | # ruff: disable[YF829] + | ^^^^^^^^^^^^^^^^^^^^^^ +94 | # ruff: disable[F841, RQW320] +95 | value = 0 + | +help: Remove the rule code +90 | +91 | def f(): +92 | # Unknown rule codes + - # ruff: disable[YF829] +93 | # ruff: disable[F841, RQW320] +94 | value = 0 +95 | # ruff: enable[F841, RQW320] + + +RUF102 [*] Invalid rule code in suppression: RQW320 + --> suppressions.py:94:27 + | +92 | # Unknown rule codes +93 | # ruff: disable[YF829] +94 | # ruff: disable[F841, RQW320] + | ^^^^^^ +95 | value = 0 +96 | # ruff: enable[F841, RQW320] + | +help: Remove the rule code +91 | def f(): +92 | # Unknown rule codes +93 | # ruff: disable[YF829] + - # ruff: disable[F841, RQW320] +94 + # ruff: disable[F841] +95 | value = 0 +96 | # ruff: enable[F841, RQW320] +97 | # ruff: enable[YF829] + + +RUF102 [*] Invalid rule code in suppression: RQW320 + --> suppressions.py:96:26 + | +94 | # ruff: disable[F841, RQW320] +95 | value = 0 +96 | # ruff: enable[F841, RQW320] + | ^^^^^^ +97 | # ruff: enable[YF829] + | +help: Remove the rule code +93 | # ruff: disable[YF829] +94 | # ruff: disable[F841, RQW320] +95 | value = 0 + - # ruff: enable[F841, RQW320] +96 + # ruff: enable[F841] +97 | # ruff: enable[YF829] +98 | +99 | + + +RUF102 [*] Invalid rule code in suppression: YF829 + --> suppressions.py:97:5 + | +95 | value = 0 +96 | # ruff: enable[F841, RQW320] +97 | # ruff: enable[YF829] + | ^^^^^^^^^^^^^^^^^^^^^ + | +help: Remove the rule code +94 | # ruff: disable[F841, RQW320] +95 | value = 0 +96 | # ruff: enable[F841, RQW320] + - # ruff: enable[YF829] +97 | +98 | +99 | def f(): + + +RUF102 [*] Invalid rule code in suppression: TK421 + --> suppressions.py:102:5 + | +100 | def f(): +101 | # External rule codes should be ignored +102 | # ruff: disable[TK421] + | ^^^^^^^^^^^^^^^^^^^^^^ +103 | print("hello") +104 | # ruff: enable[TK421] + | +help: Remove the rule code +99 | +100 | def f(): +101 | # External rule codes should be ignored + - # ruff: disable[TK421] +102 | print("hello") +103 | # ruff: enable[TK421] + + +RUF102 [*] Invalid rule code in suppression: TK421 + --> suppressions.py:104:5 + | +102 | # ruff: disable[TK421] +103 | print("hello") +104 | # ruff: enable[TK421] + | ^^^^^^^^^^^^^^^^^^^^^ + | +help: Remove the rule code +101 | # External rule codes should be ignored +102 | # ruff: disable[TK421] +103 | print("hello") + - # ruff: enable[TK421] diff --git a/crates/ruff_linter/src/settings/mod.rs b/crates/ruff_linter/src/settings/mod.rs index 5d5e35aa8d..2f22f91b6b 100644 --- a/crates/ruff_linter/src/settings/mod.rs +++ b/crates/ruff_linter/src/settings/mod.rs @@ -471,6 +471,13 @@ impl LinterSettings { self } + #[must_use] + pub fn with_external_rules(mut self, rules: &[&str]) -> Self { + self.external + .extend(rules.iter().map(std::string::ToString::to_string)); + self + } + /// Resolve the [`TargetVersion`] to use for linting. /// /// This method respects the per-file version overrides in diff --git a/crates/ruff_linter/src/suppression.rs b/crates/ruff_linter/src/suppression.rs index 18c6ad50ce..bf982c0ccd 100644 --- a/crates/ruff_linter/src/suppression.rs +++ b/crates/ruff_linter/src/suppression.rs @@ -4,6 +4,7 @@ use ruff_db::diagnostic::Diagnostic; use ruff_diagnostics::{Edit, Fix}; use ruff_python_ast::token::{TokenKind, Tokens}; use ruff_python_ast::whitespace::indentation; +use rustc_hash::FxHashSet; use std::cell::Cell; use std::{error::Error, fmt::Formatter}; use thiserror::Error; @@ -17,7 +18,10 @@ use crate::checkers::ast::LintContext; use crate::codes::Rule; use crate::fix::edits::delete_comment; use crate::preview::is_range_suppressions_enabled; -use crate::rules::ruff::rules::{UnusedCodes, UnusedNOQA, UnusedNOQAKind}; +use crate::rules::ruff::rules::{ + InvalidRuleCode, InvalidRuleCodeKind, InvalidSuppressionComment, InvalidSuppressionCommentKind, + UnmatchedSuppressionComment, UnusedCodes, UnusedNOQA, UnusedNOQAKind, +}; use crate::settings::LinterSettings; #[derive(Clone, Debug, Eq, PartialEq)] @@ -130,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 @@ -159,81 +163,161 @@ impl Suppressions { } pub(crate) fn check_suppressions(&self, context: &LintContext, locator: &Locator) { - if !context.any_rule_enabled(&[Rule::UnusedNOQA, Rule::InvalidRuleCode]) { - return; - } + if context.is_rule_enabled(Rule::UnusedNOQA) { + let unused = self + .valid + .iter() + .filter(|suppression| !suppression.used.get()); - let unused = self - .valid - .iter() - .filter(|suppression| !suppression.used.get()); + for suppression in unused { + let Ok(rule) = Rule::from_code(&suppression.code) else { + continue; // TODO: invalid code + }; + for comment in &suppression.comments { + let (range, edit) = + Suppressions::delete_code_or_comment(locator, suppression, comment); - for suppression in unused { - let Ok(rule) = Rule::from_code(&suppression.code) else { - continue; // TODO: invalid code - }; - 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(), - ) + let codes = if context.is_rule_enabled(rule) { + UnusedCodes { + unmatched: vec![suppression.code.to_string()], + ..Default::default() + } } else { - TextRange::new( - comment.codes[code_index - 1].end(), - comment.codes[code_index].end(), - ) + UnusedCodes { + disabled: vec![suppression.code.to_string()], + ..Default::default() + } }; - Edit::range_deletion(code_range) - }; - let codes = if context.is_rule_enabled(rule) { - UnusedCodes { - unmatched: vec![suppression.code.to_string()], - ..Default::default() - } - } else { - UnusedCodes { - disabled: vec![suppression.code.to_string()], - ..Default::default() - } - }; + context + .report_diagnostic( + UnusedNOQA { + codes: Some(codes), + kind: UnusedNOQAKind::Suppression, + }, + range, + ) + .set_fix(Fix::safe_edit(edit)); + } + } - let mut diagnostic = context.report_diagnostic( - UnusedNOQA { - codes: Some(codes), - kind: UnusedNOQAKind::Suppression, - }, - range, - ); - diagnostic.set_fix(Fix::safe_edit(edit)); + // treat comments with no codes as unused suppression + for error in self + .errors + .iter() + .filter(|error| error.kind == ParseErrorKind::MissingCodes) + { + context + .report_diagnostic( + UnusedNOQA { + codes: Some(UnusedCodes::default()), + kind: UnusedNOQAKind::Suppression, + }, + error.range, + ) + .set_fix(Fix::safe_edit(delete_comment(error.range, locator))); } } - 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))); + if context.is_rule_enabled(Rule::InvalidRuleCode) { + for suppression in self + .valid + .iter() + .filter(|suppression| Rule::from_code(&suppression.code).is_err()) + { + for comment in &suppression.comments { + let (range, edit) = + Suppressions::delete_code_or_comment(locator, suppression, comment); + context + .report_diagnostic( + InvalidRuleCode { + rule_code: suppression.code.to_string(), + kind: InvalidRuleCodeKind::Suppression, + }, + range, + ) + .set_fix(Fix::safe_edit(edit)); + } + } } + + 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) + { + context + .report_diagnostic( + InvalidSuppressionComment { + kind: InvalidSuppressionCommentKind::Error(error.kind), + }, + error.range, + ) + .set_fix(Fix::unsafe_edit(delete_comment(error.range, locator))); + } + + for invalid in &self.invalid { + context + .report_diagnostic( + InvalidSuppressionComment { + kind: InvalidSuppressionCommentKind::Invalid(invalid.kind), + }, + invalid.comment.range, + ) + .set_fix(Fix::unsafe_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::>() + { + 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(), + ) + }; + Edit::range_deletion(code_range) + }; + (range, edit) } } @@ -391,7 +475,7 @@ impl<'a> SuppressionsBuilder<'a> { } #[derive(Copy, Clone, Debug, Eq, Error, PartialEq)] -enum ParseErrorKind { +pub(crate) enum ParseErrorKind { #[error("not a suppression comment")] NotASuppression, diff --git a/docs/linter.md b/docs/linter.md index 6644d54f34..6d009e243d 100644 --- a/docs/linter.md +++ b/docs/linter.md @@ -384,6 +384,9 @@ foo() It is strongly suggested to use explicit range suppressions, in order to prevent accidental suppressions of violations, especially at global module scope. +For this reason, a `RUF104` diagnostic will also be produced for any implicit range. +If implicit range suppressions are desired, the `RUF104` rule can be disabled, +or an inline `noqa` suppression can be added to the end of the "disable" comment. Range suppressions cannot be used to enable or select rules that aren't already selected by the project configuration or runtime flags. An "enable" comment can only diff --git a/ruff.schema.json b/ruff.schema.json index 1c8a092042..7ff15dbe14 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -4050,6 +4050,8 @@ "RUF100", "RUF101", "RUF102", + "RUF103", + "RUF104", "RUF2", "RUF20", "RUF200",