diff --git a/crates/ruff_linter/resources/test/fixtures/ruff/suppressions.py b/crates/ruff_linter/resources/test/fixtures/ruff/suppressions.py index f8a3c882aa..4f4796a01a 100644 --- a/crates/ruff_linter/resources/test/fixtures/ruff/suppressions.py +++ b/crates/ruff_linter/resources/test/fixtures/ruff/suppressions.py @@ -86,3 +86,26 @@ 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] + + +def f(): + # Empty or missing rule codes + # ruff: disable + # ruff: disable[] + print("hello") diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index b005c6e914..74cb38bddb 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -1064,6 +1064,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..f8b24d6d86 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 { @@ -61,7 +81,9 @@ pub(crate) fn invalid_noqa_code( continue; }; - let all_valid = directive.iter().all(|code| code_is_valid(code, external)); + let all_valid = directive + .iter() + .all(|code| code_is_valid(code.as_str(), external)); if all_valid { continue; @@ -69,7 +91,7 @@ pub(crate) fn invalid_noqa_code( let (valid_codes, invalid_codes): (Vec<_>, Vec<_>) = directive .iter() - .partition(|&code| code_is_valid(code, external)); + .partition(|&code| code_is_valid(code.as_str(), external)); if valid_codes.is_empty() { all_codes_invalid_diagnostic(directive, invalid_codes, context); @@ -81,10 +103,9 @@ pub(crate) fn invalid_noqa_code( } } -fn code_is_valid(code: &Code, external: &[String]) -> bool { - let code_str = code.as_str(); - Rule::from_code(get_redirect_target(code_str).unwrap_or(code_str)).is_ok() - || external.iter().any(|ext| code_str.starts_with(ext)) +pub(crate) fn code_is_valid(code: &str, external: &[String]) -> bool { + Rule::from_code(get_redirect_target(code).unwrap_or(code)).is_ok() + || external.iter().any(|ext| code.starts_with(ext)) } fn all_codes_invalid_diagnostic( @@ -100,6 +121,7 @@ fn all_codes_invalid_diagnostic( .map(Code::as_str) .collect::>() .join(", "), + kind: InvalidRuleCodeKind::Noqa, }, directive.range(), ) @@ -116,6 +138,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..2ae52f26d1 --- /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 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..1b31882ae6 --- /dev/null +++ b/crates/ruff_linter/src/rules/ruff/rules/unmatched_suppression_comment.rs @@ -0,0 +1,42 @@ +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_VALUES) +/// ``` +/// +/// Use instead: +/// ```python +/// def foo(): +/// # ruff: disable[E501] +/// REALLY_LONG_VALUES = [...] +/// # ruff: enable[E501] +/// +/// print(REALLY_LONG_VALUES) +/// ``` +/// +/// ## 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..ddac917621 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: 23 --- 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 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,18 @@ 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 + | + + RUF100 [*] Unused suppression (unused: `F841`) --> suppressions.py:69:5 | @@ -337,6 +412,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 +463,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 | @@ -413,6 +510,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 +530,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 +550,122 @@ 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:21 + | +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:20 + | +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(): + + +RUF103 [*] Invalid suppression comment: missing suppression codes like `[E501, ...]` + --> suppressions.py:109:5 + | +107 | def f(): +108 | # Empty or missing rule codes +109 | # ruff: disable + | ^^^^^^^^^^^^^^^ +110 | # ruff: disable[] +111 | print("hello") + | +help: Remove suppression comment +106 | +107 | def f(): +108 | # Empty or missing rule codes + - # ruff: disable +109 | # ruff: disable[] +110 | print("hello") +note: This is an unsafe fix and may change runtime behavior + + +RUF103 [*] Invalid suppression comment: missing suppression codes like `[E501, ...]` + --> suppressions.py:110:5 + | +108 | # Empty or missing rule codes +109 | # ruff: disable +110 | # ruff: disable[] + | ^^^^^^^^^^^^^^^^^ +111 | print("hello") + | +help: Remove suppression comment +107 | def f(): +108 | # Empty or missing rule codes +109 | # ruff: disable + - # ruff: disable[] +110 | print("hello") +note: This is an unsafe fix and may change runtime behavior 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..4cb5deca69 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,11 @@ 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::rule_redirects::get_redirect_target; +use crate::rules::ruff::rules::{ + InvalidRuleCode, InvalidRuleCodeKind, InvalidSuppressionComment, InvalidSuppressionCommentKind, + UnmatchedSuppressionComment, UnusedCodes, UnusedNOQA, UnusedNOQAKind, code_is_valid, +}; use crate::settings::LinterSettings; #[derive(Clone, Debug, Eq, PartialEq)] @@ -130,7 +135,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 @@ -150,7 +155,9 @@ impl Suppressions { }; for suppression in &self.valid { - if *code == suppression.code.as_str() && suppression.range.contains_range(range) { + let suppression_code = + get_redirect_target(suppression.code.as_str()).unwrap_or(suppression.code.as_str()); + if *code == suppression_code && suppression.range.contains_range(range) { suppression.used.set(true); return true; } @@ -159,81 +166,140 @@ impl Suppressions { } pub(crate) fn check_suppressions(&self, context: &LintContext, locator: &Locator) { - if !context.any_rule_enabled(&[Rule::UnusedNOQA, Rule::InvalidRuleCode]) { - return; - } - - 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 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(), - ) + let mut unmatched_ranges = FxHashSet::default(); + for suppression in &self.valid { + if !code_is_valid(&suppression.code, &context.settings().external) { + // InvalidRuleCode + if context.is_rule_enabled(Rule::InvalidRuleCode) { + for comment in &suppression.comments { + let (range, edit) = Suppressions::delete_code_or_comment( + locator, + suppression, + comment, + true, + ); + context + .report_diagnostic( + InvalidRuleCode { + rule_code: suppression.code.to_string(), + kind: InvalidRuleCodeKind::Suppression, + }, + range, + ) + .set_fix(Fix::safe_edit(edit)); + } + } + } else if !suppression.used.get() { + // UnusedNOQA + if context.is_rule_enabled(Rule::UnusedNOQA) { + let Ok(rule) = Rule::from_code( + get_redirect_target(&suppression.code).unwrap_or(&suppression.code), + ) else { + continue; // "external" lint code, don't treat it as unused }; - Edit::range_deletion(code_range) - }; + for comment in &suppression.comments { + let (range, edit) = Suppressions::delete_code_or_comment( + locator, + suppression, + comment, + false, + ); - 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() - } - }; + 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() + } + }; - let mut diagnostic = context.report_diagnostic( - UnusedNOQA { - codes: Some(codes), - kind: UnusedNOQAKind::Suppression, - }, - range, - ); - diagnostic.set_fix(Fix::safe_edit(edit)); + context + .report_diagnostic( + UnusedNOQA { + codes: Some(codes), + kind: UnusedNOQAKind::Suppression, + }, + range, + ) + .set_fix(Fix::safe_edit(edit)); + } + } + } else if suppression.comments.len() == 1 { + // UnmatchedSuppressionComment + let range = suppression.comments[0].range; + if unmatched_ranges.insert(range) { + context.report_diagnostic_if_enabled(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))); + if context.is_rule_enabled(Rule::InvalidSuppressionComment) { + for error in &self.errors { + context + .report_diagnostic( + InvalidSuppressionComment { + kind: InvalidSuppressionCommentKind::Error(error.kind), + }, + error.range, + ) + .set_fix(Fix::unsafe_edit(delete_comment(error.range, locator))); + } } + + if context.is_rule_enabled(Rule::InvalidSuppressionComment) { + 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, + ))); + } + } + } + + fn delete_code_or_comment( + locator: &Locator<'_>, + suppression: &Suppression, + comment: &SuppressionComment, + highlight_only_code: bool, + ) -> (TextRange, Edit) { + let mut range = comment.range; + let edit = if comment.codes.len() == 1 { + if highlight_only_code { + range = comment.codes[0]; + } + 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 +457,7 @@ impl<'a> SuppressionsBuilder<'a> { } #[derive(Copy, Clone, Debug, Eq, Error, PartialEq)] -enum ParseErrorKind { +pub(crate) enum ParseErrorKind { #[error("not a suppression comment")] NotASuppression, @@ -401,7 +467,7 @@ enum ParseErrorKind { #[error("unknown ruff directive")] UnknownAction, - #[error("missing suppression codes")] + #[error("missing suppression codes like `[E501, ...]`")] MissingCodes, #[error("missing closing bracket")] 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 0dd54c0a07..5af8837779 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -4051,6 +4051,8 @@ "RUF100", "RUF101", "RUF102", + "RUF103", + "RUF104", "RUF2", "RUF20", "RUF200",