From 90f82c1008cd4d4573a4e278aaa52d93e199cc6b Mon Sep 17 00:00:00 2001 From: Amethyst Reese Date: Wed, 10 Dec 2025 12:53:40 -0800 Subject: [PATCH 01/28] Define rule codes RUF103/104 for invalid/unmatched suppression comments --- crates/ruff_linter/src/codes.rs | 2 + .../ruff/rules/invalid_suppression_comment.rs | 39 +++++++++++++++++++ .../ruff_linter/src/rules/ruff/rules/mod.rs | 4 ++ .../rules/unmatched_suppression_comment.rs | 38 ++++++++++++++++++ ruff.schema.json | 2 + 5 files changed, 85 insertions(+) create mode 100644 crates/ruff_linter/src/rules/ruff/rules/invalid_suppression_comment.rs create mode 100644 crates/ruff_linter/src/rules/ruff/rules/unmatched_suppression_comment.rs diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index ebec5f4acc09d..04fb7eee11644 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/rules/invalid_suppression_comment.rs b/crates/ruff_linter/src/rules/ruff/rules/invalid_suppression_comment.rs new file mode 100644 index 0000000000000..8e6ac88dd3f61 --- /dev/null +++ b/crates/ruff_linter/src/rules/ruff/rules/invalid_suppression_comment.rs @@ -0,0 +1,39 @@ +use ruff_macros::{ViolationMetadata, derive_message_formats}; + +use crate::AlwaysFixableViolation; + +/// ## 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; + +impl AlwaysFixableViolation for InvalidSuppressionComment { + #[derive_message_formats] + fn message(&self) -> String { + "Invalid suppression comment".to_string() + } + + fn fix_title(&self) -> String { + "Remove invalid suppression comment".to_string() + } +} diff --git a/crates/ruff_linter/src/rules/ruff/rules/mod.rs b/crates/ruff_linter/src/rules/ruff/rules/mod.rs index 206a504e740b4..70363d3f8be5e 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 0000000000000..2dae90a8c1dab --- /dev/null +++ b/crates/ruff_linter/src/rules/ruff/rules/unmatched_suppression_comment.rs @@ -0,0 +1,38 @@ +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 +/// ... +/// ``` +/// +/// Use instead: +/// ```python +/// def foo(): +/// # ruff: disable[E501] +/// ... +/// # ruff: enable[E501] +/// ``` +/// +/// ## 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 { + "Unmatched suppression comment".to_string() + } +} diff --git a/ruff.schema.json b/ruff.schema.json index 1c8a092042a57..7ff15dbe1487e 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -4050,6 +4050,8 @@ "RUF100", "RUF101", "RUF102", + "RUF103", + "RUF104", "RUF2", "RUF20", "RUF200", From 976a52c3f4c20aeda2cc37e5967ace8a7f343b74 Mon Sep 17 00:00:00 2001 From: Amethyst Reese Date: Wed, 10 Dec 2025 15:17:11 -0800 Subject: [PATCH 02/28] Report RUF104 unmatched suppression diagnostics --- ...ules__ruff__tests__range_suppressions.snap | 80 ++++++++++++++++++- crates/ruff_linter/src/suppression.rs | 18 ++++- 2 files changed, 96 insertions(+), 2 deletions(-) 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 24a43ade5ef1a..908cca2cbd2c5 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 @@ -7,7 +7,7 @@ source: crates/ruff_linter/src/rules/ruff/mod.rs --- Summary --- Removed: 14 -Added: 11 +Added: 18 --- Removed --- E741 Ambiguous variable name: `I` @@ -240,6 +240,17 @@ note: This is an unsafe fix and may change runtime behavior --- Added --- +RUF104 Unmatched suppression 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 + | + + RUF100 [*] Unused suppression (non-enabled: `E501`) --> suppressions.py:46:5 | @@ -298,6 +309,17 @@ help: Remove unused `noqa` directive 58 | +RUF104 Unmatched suppression 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 +340,18 @@ help: Remove unused suppression 64 | +RUF104 Unmatched suppression 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 +371,28 @@ help: Remove unused suppression 71 | +RUF104 Unmatched suppression 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 + | + + +RUF104 Unmatched suppression 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 +433,17 @@ help: Remove unused suppression 78 | +RUF104 Unmatched suppression 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 +464,17 @@ help: Remove unused suppression 84 | +RUF104 Unmatched suppression 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 | diff --git a/crates/ruff_linter/src/suppression.rs b/crates/ruff_linter/src/suppression.rs index 18c6ad50cea41..7b4e57a67ab8a 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,9 @@ 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::{ + UnmatchedSuppressionComment, UnusedCodes, UnusedNOQA, UnusedNOQAKind, +}; use crate::settings::LinterSettings; #[derive(Clone, Debug, Eq, PartialEq)] @@ -220,6 +223,19 @@ impl Suppressions { } } + let unmatched = self + .valid + .iter() + .filter(|suppression| { + suppression.comments.len() == 1 + && suppression.comments[0].action == SuppressionAction::Disable + }) + .map(|suppression| suppression.comments[0].range) + .collect::>(); + for range in unmatched { + context.report_diagnostic(UnmatchedSuppressionComment {}, range); + } + for error in self .errors .iter() From 05f63c91b9670744dba704e09fbd30f163dbd4a8 Mon Sep 17 00:00:00 2001 From: Amethyst Reese Date: Wed, 10 Dec 2025 17:10:48 -0800 Subject: [PATCH 03/28] Report invalid suppression diagnostics --- .../ruff/rules/invalid_suppression_comment.rs | 32 ++++++++-- crates/ruff_linter/src/suppression.rs | 63 +++++++++++++------ 2 files changed, 70 insertions(+), 25 deletions(-) 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 8e6ac88dd3f61..54e2da0434207 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 7b4e57a67ab8a..f7778d1403a22 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, From aa913534fd1ad4729bfa53ef2e6a0612529bc8f8 Mon Sep 17 00:00:00 2001 From: Amethyst Reese Date: Wed, 10 Dec 2025 17:12:04 -0800 Subject: [PATCH 04/28] Todos --- crates/ruff_linter/src/suppression.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/ruff_linter/src/suppression.rs b/crates/ruff_linter/src/suppression.rs index f7778d1403a22..fd5e2064bae34 100644 --- a/crates/ruff_linter/src/suppression.rs +++ b/crates/ruff_linter/src/suppression.rs @@ -163,6 +163,7 @@ impl Suppressions { } pub(crate) fn check_suppressions(&self, context: &LintContext, locator: &Locator) { + // TODO: wrap these around relevant bits below if !context.any_rule_enabled(&[ Rule::UnusedNOQA, Rule::InvalidRuleCode, @@ -250,6 +251,7 @@ impl Suppressions { diagnostic.set_fix(Fix::safe_edit(delete_comment(error.range, locator))); } + // TODO: sometimes these don't get fixes attached? for invalid in &self.invalid { let mut diagnostic = context.report_diagnostic( InvalidSuppressionComment { From ebf012170de76667c0330157699893267bf5fd53 Mon Sep 17 00:00:00 2001 From: Amethyst Reese Date: Thu, 11 Dec 2025 15:05:06 -0800 Subject: [PATCH 05/28] Fix test cases causing panics --- crates/ruff_linter/src/rules/ruff/mod.rs | 6 ++++++ .../ruff/rules/invalid_suppression_comment.rs | 6 +++--- ...ules__ruff__tests__range_suppressions.snap | 20 ++++++++++++++++++- crates/ruff_linter/src/suppression.rs | 1 - 4 files changed, 28 insertions(+), 5 deletions(-) diff --git a/crates/ruff_linter/src/rules/ruff/mod.rs b/crates/ruff_linter/src/rules/ruff/mod.rs index c2d03fb1ae6be..ad18e24d59d86 100644 --- a/crates/ruff_linter/src/rules/ruff/mod.rs +++ b/crates/ruff_linter/src/rules/ruff/mod.rs @@ -313,11 +313,17 @@ mod tests { Rule::UnusedVariable, Rule::AmbiguousVariableName, Rule::UnusedNOQA, + Rule::InvalidRuleCode, + Rule::InvalidSuppressionComment, + Rule::UnmatchedSuppressionComment, ]), &settings::LinterSettings::for_rules(vec![ Rule::UnusedVariable, Rule::AmbiguousVariableName, Rule::UnusedNOQA, + Rule::InvalidRuleCode, + Rule::InvalidSuppressionComment, + Rule::UnmatchedSuppressionComment, ]) .with_preview_mode(), ); 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 54e2da0434207..25b9eb2ebb118 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 @@ -30,7 +30,7 @@ pub(crate) struct InvalidSuppressionComment { pub kind: InvalidSuppressionCommentKind, } -impl Violation for InvalidSuppressionComment { +impl AlwaysFixableViolation for InvalidSuppressionComment { #[derive_message_formats] fn message(&self) -> String { let msg = match self.kind { @@ -48,8 +48,8 @@ impl Violation for InvalidSuppressionComment { format!("Invalid suppression comment: {msg}") } - fn fix_title(&self) -> Option { - Some("Remove invalid suppression comment".to_string()) + fn fix_title(&self) -> String { + "Remove invalid suppression 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 908cca2cbd2c5..0094d8474ad9d 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 @@ -7,7 +7,7 @@ source: crates/ruff_linter/src/rules/ruff/mod.rs --- Summary --- Removed: 14 -Added: 18 +Added: 19 --- Removed --- E741 Ambiguous variable name: `I` @@ -251,6 +251,24 @@ RUF104 Unmatched suppression comment | +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(): + + RUF100 [*] Unused suppression (non-enabled: `E501`) --> suppressions.py:46:5 | diff --git a/crates/ruff_linter/src/suppression.rs b/crates/ruff_linter/src/suppression.rs index fd5e2064bae34..83acaffe4f8dc 100644 --- a/crates/ruff_linter/src/suppression.rs +++ b/crates/ruff_linter/src/suppression.rs @@ -251,7 +251,6 @@ impl Suppressions { diagnostic.set_fix(Fix::safe_edit(delete_comment(error.range, locator))); } - // TODO: sometimes these don't get fixes attached? for invalid in &self.invalid { let mut diagnostic = context.report_diagnostic( InvalidSuppressionComment { From 2b6dc036377ee791ef9c178d1f381023ade5f15a Mon Sep 17 00:00:00 2001 From: Amethyst Reese Date: Thu, 11 Dec 2025 16:30:55 -0800 Subject: [PATCH 06/28] Move rule-enabled checks to local logic, extract some bits --- .../ruff/rules/invalid_suppression_comment.rs | 2 +- crates/ruff_linter/src/suppression.rs | 199 ++++++++++-------- 2 files changed, 108 insertions(+), 93 deletions(-) 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 25b9eb2ebb118..7c73578b9c2c6 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,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 diff --git a/crates/ruff_linter/src/suppression.rs b/crates/ruff_linter/src/suppression.rs index 83acaffe4f8dc..87e382f32297f 100644 --- a/crates/ruff_linter/src/suppression.rs +++ b/crates/ruff_linter/src/suppression.rs @@ -163,120 +163,135 @@ impl Suppressions { } pub(crate) fn check_suppressions(&self, context: &LintContext, locator: &Locator) { - // TODO: wrap these around relevant bits below - if !context.any_rule_enabled(&[ - Rule::UnusedNOQA, - Rule::InvalidRuleCode, - Rule::InvalidSuppressionComment, - Rule::UnmatchedSuppressionComment, - ]) { - return; - } - - let unused = self - .valid - .iter() - .filter(|suppression| !suppression.used.get()); + if context.is_rule_enabled(Rule::UnusedNOQA) { + 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(), - ) + 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); + + 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() - } - }; + 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) + { let mut diagnostic = context.report_diagnostic( UnusedNOQA { - codes: Some(codes), + codes: Some(UnusedCodes::default()), kind: UnusedNOQAKind::Suppression, }, - range, + error.range, ); - diagnostic.set_fix(Fix::safe_edit(edit)); + diagnostic.set_fix(Fix::safe_edit(delete_comment(error.range, locator))); } } - 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( + 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))); - } + ); + 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, - ))); + 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::>(); - for range in unmatched { - context.report_diagnostic(UnmatchedSuppressionComment {}, range); + 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) + } } #[derive(Default)] From bcd9374005e0e7ff415e43bec5acba0d6a1c5954 Mon Sep 17 00:00:00 2001 From: Amethyst Reese Date: Thu, 11 Dec 2025 16:59:18 -0800 Subject: [PATCH 07/28] Generate invalid rule code diagnostics --- .../test/fixtures/ruff/suppressions.py | 9 ++ ...ules__ruff__tests__range_suppressions.snap | 125 ++++++++++++++++-- crates/ruff_linter/src/suppression.rs | 27 +++- 3 files changed, 146 insertions(+), 15 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/ruff/suppressions.py b/crates/ruff_linter/resources/test/fixtures/ruff/suppressions.py index f8a3c882aab8e..2eabfcde897f9 100644 --- a/crates/ruff_linter/resources/test/fixtures/ruff/suppressions.py +++ b/crates/ruff_linter/resources/test/fixtures/ruff/suppressions.py @@ -86,3 +86,12 @@ 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] 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 0094d8474ad9d..26aa8f1384941 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: 19 +Removed: 15 +Added: 23 --- Removed --- E741 Ambiguous variable name: `I` @@ -238,6 +238,27 @@ 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] +note: This is an unsafe fix and may change runtime behavior + + --- Added --- RUF104 Unmatched suppression comment @@ -370,7 +391,7 @@ RUF104 Unmatched suppression comment | -RUF100 [*] Unused suppression (unused: `F841`) +RUF104 Unmatched suppression comment --> suppressions.py:69:5 | 67 | # and the other should trigger an unused suppression diagnostic @@ -379,17 +400,9 @@ RUF100 [*] Unused suppression (unused: `F841`) | ^^^^^^^^^^^^^^^^^^^^^ 70 | foo = 0 | -help: Remove unused suppression -66 | # Overlapping range suppressions, one should be marked as used, -67 | # and the other should trigger an unused suppression diagnostic -68 | # ruff: disable[F841] - - # ruff: disable[F841] -69 | foo = 0 -70 | -71 | -RUF104 Unmatched suppression comment +RUF100 [*] Unused suppression (unused: `F841`) --> suppressions.py:69:5 | 67 | # and the other should trigger an unused suppression diagnostic @@ -398,6 +411,14 @@ RUF104 Unmatched suppression comment | ^^^^^^^^^^^^^^^^^^^^^ 70 | foo = 0 | +help: Remove unused suppression +66 | # Overlapping range suppressions, one should be marked as used, +67 | # and the other should trigger an unused suppression diagnostic +68 | # ruff: disable[F841] + - # ruff: disable[F841] +69 | foo = 0 +70 | +71 | RUF104 Unmatched suppression comment @@ -509,6 +530,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`) @@ -527,6 +550,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`) @@ -545,3 +570,79 @@ help: Remove unused suppression - # ruff: disable[E741, F401, F841] 87 + # ruff: disable[E741, F401] 88 | print("hello") +89 | +90 | + + +RUF102 [*] Invalid rule code in `# noqa`: 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 `# noqa`: 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 `# noqa`: 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] + + +RUF102 [*] Invalid rule code in `# noqa`: 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] diff --git a/crates/ruff_linter/src/suppression.rs b/crates/ruff_linter/src/suppression.rs index 87e382f32297f..e9e9106d112c7 100644 --- a/crates/ruff_linter/src/suppression.rs +++ b/crates/ruff_linter/src/suppression.rs @@ -1,6 +1,6 @@ use compact_str::CompactString; use core::fmt; -use ruff_db::diagnostic::Diagnostic; +use ruff_db::diagnostic::{self, Diagnostic}; use ruff_diagnostics::{Edit, Fix}; use ruff_python_ast::token::{TokenKind, Tokens}; use ruff_python_ast::whitespace::indentation; @@ -19,8 +19,8 @@ use crate::codes::Rule; use crate::fix::edits::delete_comment; use crate::preview::is_range_suppressions_enabled; use crate::rules::ruff::rules::{ - InvalidSuppressionComment, InvalidSuppressionCommentKind, UnmatchedSuppressionComment, - UnusedCodes, UnusedNOQA, UnusedNOQAKind, + InvalidRuleCode, InvalidSuppressionComment, InvalidSuppressionCommentKind, + UnmatchedSuppressionComment, UnusedCodes, UnusedNOQA, UnusedNOQAKind, }; use crate::settings::LinterSettings; @@ -217,6 +217,26 @@ impl Suppressions { } } + 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); + let mut diagnostic = context.report_diagnostic( + InvalidRuleCode { + rule_code: suppression.code.to_string(), + }, + range, + ); + diagnostic.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 @@ -262,6 +282,7 @@ impl Suppressions { } } } + fn delete_code_or_comment( locator: &Locator<'_>, suppression: &Suppression, From 58f57348a0b26a51466fd49849cdbac8e1de8eb0 Mon Sep 17 00:00:00 2001 From: Amethyst Reese Date: Thu, 11 Dec 2025 17:06:05 -0800 Subject: [PATCH 08/28] Target the diagnostic at the code even if only one code in comment --- ...ules__ruff__tests__range_suppressions.snap | 20 +++++++++---------- crates/ruff_linter/src/suppression.rs | 3 ++- 2 files changed, 12 insertions(+), 11 deletions(-) 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 26aa8f1384941..21e633d1894a5 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 @@ -291,12 +291,12 @@ help: Remove invalid suppression comment RUF100 [*] Unused suppression (non-enabled: `E501`) - --> suppressions.py:46:5 + --> suppressions.py:46:21 | 44 | # Neither of these are ignored and warnings are 45 | # logged to user 46 | # ruff: disable[E501] - | ^^^^^^^^^^^^^^^^^^^^^ + | ^^^^ 47 | I = 1 48 | # ruff: enable[E501] | @@ -311,12 +311,12 @@ help: Remove unused suppression RUF100 [*] Unused suppression (non-enabled: `E501`) - --> suppressions.py:48:5 + --> suppressions.py:48:20 | 46 | # ruff: disable[E501] 47 | I = 1 48 | # ruff: enable[E501] - | ^^^^^^^^^^^^^^^^^^^^ + | ^^^^ | help: Remove unused suppression 45 | # logged to user @@ -403,12 +403,12 @@ RUF104 Unmatched suppression comment RUF100 [*] Unused suppression (unused: `F841`) - --> suppressions.py:69:5 + --> suppressions.py:69:21 | 67 | # and the other should trigger an unused suppression diagnostic 68 | # ruff: disable[F841] 69 | # ruff: disable[F841] - | ^^^^^^^^^^^^^^^^^^^^^ + | ^^^^ 70 | foo = 0 | help: Remove unused suppression @@ -575,12 +575,12 @@ help: Remove unused suppression RUF102 [*] Invalid rule code in `# noqa`: YF829 - --> suppressions.py:93:5 + --> suppressions.py:93:21 | 91 | def f(): 92 | # Unknown rule codes 93 | # ruff: disable[YF829] - | ^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^ 94 | # ruff: disable[F841, RQW320] 95 | value = 0 | @@ -634,12 +634,12 @@ help: Remove the rule code RUF102 [*] Invalid rule code in `# noqa`: YF829 - --> suppressions.py:97:5 + --> 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] diff --git a/crates/ruff_linter/src/suppression.rs b/crates/ruff_linter/src/suppression.rs index e9e9106d112c7..87d082409e753 100644 --- a/crates/ruff_linter/src/suppression.rs +++ b/crates/ruff_linter/src/suppression.rs @@ -288,8 +288,9 @@ impl Suppressions { suppression: &Suppression, comment: &SuppressionComment, ) -> (TextRange, Edit) { - let mut range = comment.range; + let range: TextRange; let edit = if comment.codes.len() == 1 { + range = comment.codes[0]; delete_comment(comment.range, locator) } else { let code_index = comment From 4793f52b84ebf6fda83478bbec1d30a3001a19da Mon Sep 17 00:00:00 2001 From: Amethyst Reese Date: Thu, 11 Dec 2025 17:06:28 -0800 Subject: [PATCH 09/28] clippy --- crates/ruff_linter/src/suppression.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/ruff_linter/src/suppression.rs b/crates/ruff_linter/src/suppression.rs index 87d082409e753..6db42580b34c2 100644 --- a/crates/ruff_linter/src/suppression.rs +++ b/crates/ruff_linter/src/suppression.rs @@ -1,6 +1,6 @@ use compact_str::CompactString; use core::fmt; -use ruff_db::diagnostic::{self, Diagnostic}; +use ruff_db::diagnostic::Diagnostic; use ruff_diagnostics::{Edit, Fix}; use ruff_python_ast::token::{TokenKind, Tokens}; use ruff_python_ast::whitespace::indentation; From c50026e3ff7f8e775d7a45357b7227701acffc46 Mon Sep 17 00:00:00 2001 From: Amethyst Reese Date: Thu, 11 Dec 2025 17:25:45 -0800 Subject: [PATCH 10/28] Document that implicit ranges will produce RUF104 --- docs/linter.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/linter.md b/docs/linter.md index 6644d54f34921..6d009e243d4f3 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 From a6e9ca858a79e2fb143bf4089649a832eb27ace5 Mon Sep 17 00:00:00 2001 From: Amethyst Reese Date: Tue, 16 Dec 2025 15:20:08 -0800 Subject: [PATCH 11/28] Add test case covering external rules --- .../resources/test/fixtures/ruff/suppressions.py | 7 +++++++ crates/ruff_linter/src/rules/ruff/mod.rs | 4 +++- crates/ruff_linter/src/settings/mod.rs | 7 +++++++ 3 files changed, 17 insertions(+), 1 deletion(-) diff --git a/crates/ruff_linter/resources/test/fixtures/ruff/suppressions.py b/crates/ruff_linter/resources/test/fixtures/ruff/suppressions.py index 2eabfcde897f9..2a4b06898aea4 100644 --- a/crates/ruff_linter/resources/test/fixtures/ruff/suppressions.py +++ b/crates/ruff_linter/resources/test/fixtures/ruff/suppressions.py @@ -95,3 +95,10 @@ def f(): 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/rules/ruff/mod.rs b/crates/ruff_linter/src/rules/ruff/mod.rs index ad18e24d59d86..9bc636373135d 100644 --- a/crates/ruff_linter/src/rules/ruff/mod.rs +++ b/crates/ruff_linter/src/rules/ruff/mod.rs @@ -316,7 +316,8 @@ mod tests { Rule::InvalidRuleCode, Rule::InvalidSuppressionComment, Rule::UnmatchedSuppressionComment, - ]), + ]) + .with_external_rules(&["TK421"]), &settings::LinterSettings::for_rules(vec![ Rule::UnusedVariable, Rule::AmbiguousVariableName, @@ -325,6 +326,7 @@ mod tests { Rule::InvalidSuppressionComment, Rule::UnmatchedSuppressionComment, ]) + .with_external_rules(&["TK421"]) .with_preview_mode(), ); Ok(()) diff --git a/crates/ruff_linter/src/settings/mod.rs b/crates/ruff_linter/src/settings/mod.rs index 5d5e35aa8d0a0..2f22f91b6bf86 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 From 6cee82d206846902b05ec451e23a65c309f747f1 Mon Sep 17 00:00:00 2001 From: Amethyst Reese Date: Tue, 16 Dec 2025 15:20:36 -0800 Subject: [PATCH 12/28] Correct pub usage --- .../src/rules/ruff/rules/invalid_suppression_comment.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 7c73578b9c2c6..56e09ad936fb2 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 @@ -27,7 +27,7 @@ use crate::suppression::{InvalidSuppressionKind, ParseErrorKind}; #[derive(ViolationMetadata)] #[violation_metadata(preview_since = "0.14.9")] pub(crate) struct InvalidSuppressionComment { - pub kind: InvalidSuppressionCommentKind, + pub(crate) kind: InvalidSuppressionCommentKind, } impl AlwaysFixableViolation for InvalidSuppressionComment { From 5b7d82baed77a78e22f3cc452727093f52fb1bfc Mon Sep 17 00:00:00 2001 From: Amethyst Reese Date: Tue, 16 Dec 2025 16:30:55 -0800 Subject: [PATCH 13/28] Improve documentation for unmatched diagnostic --- .../ruff/rules/unmatched_suppression_comment.rs | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) 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 index 2dae90a8c1dab..818cfd2150fae 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/unmatched_suppression_comment.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/unmatched_suppression_comment.rs @@ -13,15 +13,24 @@ use crate::Violation; /// ```python /// def foo(): /// ruff: disable[E501] # unmatched -/// ... +/// REALLY_LONG_VALUES = [ +/// ... +/// ] +/// +/// print(REALLY_LONG_VALUE) /// ``` /// /// Use instead: /// ```python /// def foo(): -/// # ruff: disable[E501] /// ... +/// # ruff: disable[E501] +/// REALLY_LONG_VALUES = [ +/// ... +/// ] /// # ruff: enable[E501] +/// +/// print(REALLY_LONG_VALUE) /// ``` /// /// ## References @@ -33,6 +42,6 @@ pub(crate) struct UnmatchedSuppressionComment; impl Violation for UnmatchedSuppressionComment { #[derive_message_formats] fn message(&self) -> String { - "Unmatched suppression comment".to_string() + "Suppression comment without matching `#ruff:enable` comment".to_string() } } From b659b5b7f96f1bae7352057f3bdd7cde59ba67e1 Mon Sep 17 00:00:00 2001 From: Amethyst Reese Date: Tue, 16 Dec 2025 16:35:03 -0800 Subject: [PATCH 14/28] Mark invalid comment diagnostics as unsafe fixes --- crates/ruff_linter/src/suppression.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/ruff_linter/src/suppression.rs b/crates/ruff_linter/src/suppression.rs index 6db42580b34c2..46bfe5489ad88 100644 --- a/crates/ruff_linter/src/suppression.rs +++ b/crates/ruff_linter/src/suppression.rs @@ -250,7 +250,7 @@ impl Suppressions { }, error.range, ); - diagnostic.set_fix(Fix::safe_edit(delete_comment(error.range, locator))); + diagnostic.set_fix(Fix::unsafe_edit(delete_comment(error.range, locator))); } for invalid in &self.invalid { @@ -260,7 +260,7 @@ impl Suppressions { }, invalid.comment.range, ); - diagnostic.set_fix(Fix::safe_edit(delete_comment( + diagnostic.set_fix(Fix::unsafe_edit(delete_comment( invalid.comment.range, locator, ))); From 83706a90b0d7975e00a44cecd52e5e5077f7fa01 Mon Sep 17 00:00:00 2001 From: Amethyst Reese Date: Tue, 16 Dec 2025 16:36:50 -0800 Subject: [PATCH 15/28] Update test snapshots --- ...ules__ruff__tests__range_suppressions.snap | 57 ++++++++++++++++--- 1 file changed, 49 insertions(+), 8 deletions(-) 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 21e633d1894a5..5dc7b3bb415b3 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 @@ -7,7 +7,7 @@ source: crates/ruff_linter/src/rules/ruff/mod.rs --- Summary --- Removed: 15 -Added: 23 +Added: 25 --- Removed --- E741 Ambiguous variable name: `I` @@ -256,12 +256,13 @@ help: Remove assignment to unused variable `value` 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 Unmatched suppression comment +RUF104 Suppression comment without matching `#ruff:enable` comment --> suppressions.py:11:5 | 9 | # These should both be ignored by the implicit range suppression. @@ -288,6 +289,7 @@ help: Remove invalid suppression comment 19 | 20 | 21 | def f(): +note: This is an unsafe fix and may change runtime behavior RUF100 [*] Unused suppression (non-enabled: `E501`) @@ -348,7 +350,7 @@ help: Remove unused `noqa` directive 58 | -RUF104 Unmatched suppression comment +RUF104 Suppression comment without matching `#ruff:enable` comment --> suppressions.py:61:5 | 59 | def f(): @@ -379,7 +381,7 @@ help: Remove unused suppression 64 | -RUF104 Unmatched suppression comment +RUF104 Suppression comment without matching `#ruff:enable` comment --> suppressions.py:68:5 | 66 | # Overlapping range suppressions, one should be marked as used, @@ -391,7 +393,7 @@ RUF104 Unmatched suppression comment | -RUF104 Unmatched suppression comment +RUF104 Suppression comment without matching `#ruff:enable` comment --> suppressions.py:69:5 | 67 | # and the other should trigger an unused suppression diagnostic @@ -421,7 +423,7 @@ help: Remove unused suppression 71 | -RUF104 Unmatched suppression comment +RUF104 Suppression comment without matching `#ruff:enable` comment --> suppressions.py:75:5 | 73 | def f(): @@ -472,7 +474,7 @@ help: Remove unused suppression 78 | -RUF104 Unmatched suppression comment +RUF104 Suppression comment without matching `#ruff:enable` comment --> suppressions.py:81:5 | 79 | def f(): @@ -503,7 +505,7 @@ help: Remove unused suppression 84 | -RUF104 Unmatched suppression comment +RUF104 Suppression comment without matching `#ruff:enable` comment --> suppressions.py:87:5 | 85 | def f(): @@ -631,6 +633,8 @@ help: Remove the rule code - # ruff: enable[F841, RQW320] 96 + # ruff: enable[F841] 97 | # ruff: enable[YF829] +98 | +99 | RUF102 [*] Invalid rule code in `# noqa`: YF829 @@ -646,3 +650,40 @@ help: Remove the rule code 95 | value = 0 96 | # ruff: enable[F841, RQW320] - # ruff: enable[YF829] +97 | +98 | +99 | def f(): + + +RUF102 [*] Invalid rule code in `# noqa`: TK421 + --> suppressions.py:102:21 + | +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 `# noqa`: TK421 + --> suppressions.py:104:20 + | +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] From 5c112af02df30e98cfd6971eae67bc43e2e22df3 Mon Sep 17 00:00:00 2001 From: Amethyst Reese Date: Tue, 16 Dec 2025 18:17:17 -0800 Subject: [PATCH 16/28] update wording in invalid rule code diagnostics --- .../src/rules/ruff/rules/invalid_rule_code.rs | 24 ++++++++++++++++++- ...ules__ruff__tests__range_suppressions.snap | 12 +++++----- crates/ruff_linter/src/suppression.rs | 3 ++- 3 files changed, 31 insertions(+), 8 deletions(-) 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 3c9cde312e851..56cf0585096f9 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/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 5dc7b3bb415b3..da3bd62f4b3a1 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 @@ -576,7 +576,7 @@ help: Remove unused suppression 90 | -RUF102 [*] Invalid rule code in `# noqa`: YF829 +RUF102 [*] Invalid rule code in suppression: YF829 --> suppressions.py:93:21 | 91 | def f(): @@ -596,7 +596,7 @@ help: Remove the rule code 95 | # ruff: enable[F841, RQW320] -RUF102 [*] Invalid rule code in `# noqa`: RQW320 +RUF102 [*] Invalid rule code in suppression: RQW320 --> suppressions.py:94:27 | 92 | # Unknown rule codes @@ -617,7 +617,7 @@ help: Remove the rule code 97 | # ruff: enable[YF829] -RUF102 [*] Invalid rule code in `# noqa`: RQW320 +RUF102 [*] Invalid rule code in suppression: RQW320 --> suppressions.py:96:26 | 94 | # ruff: disable[F841, RQW320] @@ -637,7 +637,7 @@ help: Remove the rule code 99 | -RUF102 [*] Invalid rule code in `# noqa`: YF829 +RUF102 [*] Invalid rule code in suppression: YF829 --> suppressions.py:97:20 | 95 | value = 0 @@ -655,7 +655,7 @@ help: Remove the rule code 99 | def f(): -RUF102 [*] Invalid rule code in `# noqa`: TK421 +RUF102 [*] Invalid rule code in suppression: TK421 --> suppressions.py:102:21 | 100 | def f(): @@ -674,7 +674,7 @@ help: Remove the rule code 103 | # ruff: enable[TK421] -RUF102 [*] Invalid rule code in `# noqa`: TK421 +RUF102 [*] Invalid rule code in suppression: TK421 --> suppressions.py:104:20 | 102 | # ruff: disable[TK421] diff --git a/crates/ruff_linter/src/suppression.rs b/crates/ruff_linter/src/suppression.rs index 46bfe5489ad88..535b44c0a12ad 100644 --- a/crates/ruff_linter/src/suppression.rs +++ b/crates/ruff_linter/src/suppression.rs @@ -19,7 +19,7 @@ use crate::codes::Rule; use crate::fix::edits::delete_comment; use crate::preview::is_range_suppressions_enabled; use crate::rules::ruff::rules::{ - InvalidRuleCode, InvalidSuppressionComment, InvalidSuppressionCommentKind, + InvalidRuleCode, InvalidRuleCodeKind, InvalidSuppressionComment, InvalidSuppressionCommentKind, UnmatchedSuppressionComment, UnusedCodes, UnusedNOQA, UnusedNOQAKind, }; use crate::settings::LinterSettings; @@ -229,6 +229,7 @@ impl Suppressions { let mut diagnostic = context.report_diagnostic( InvalidRuleCode { rule_code: suppression.code.to_string(), + kind: InvalidRuleCodeKind::Suppression, }, range, ); From 0f3c74f6df925df228c11d680ee4d997cf69c989 Mon Sep 17 00:00:00 2001 From: Amethyst Reese Date: Tue, 16 Dec 2025 18:20:38 -0800 Subject: [PATCH 17/28] Revert "Target the diagnostic at the code even if only one code in comment" This reverts commit 3c8cd30a351dc1dacba86440db86dad1d44c1ef5. --- ...ules__ruff__tests__range_suppressions.snap | 28 +++++++++---------- crates/ruff_linter/src/suppression.rs | 3 +- 2 files changed, 15 insertions(+), 16 deletions(-) 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 da3bd62f4b3a1..a6eb782f31a71 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 @@ -293,12 +293,12 @@ note: This is an unsafe fix and may change runtime behavior RUF100 [*] Unused suppression (non-enabled: `E501`) - --> suppressions.py:46:21 + --> suppressions.py:46:5 | 44 | # Neither of these are ignored and warnings are 45 | # logged to user 46 | # ruff: disable[E501] - | ^^^^ + | ^^^^^^^^^^^^^^^^^^^^^ 47 | I = 1 48 | # ruff: enable[E501] | @@ -313,12 +313,12 @@ help: Remove unused suppression RUF100 [*] Unused suppression (non-enabled: `E501`) - --> suppressions.py:48:20 + --> suppressions.py:48:5 | 46 | # ruff: disable[E501] 47 | I = 1 48 | # ruff: enable[E501] - | ^^^^ + | ^^^^^^^^^^^^^^^^^^^^ | help: Remove unused suppression 45 | # logged to user @@ -405,12 +405,12 @@ RUF104 Suppression comment without matching `#ruff:enable` comment RUF100 [*] Unused suppression (unused: `F841`) - --> suppressions.py:69:21 + --> 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 | help: Remove unused suppression @@ -577,12 +577,12 @@ help: Remove unused suppression RUF102 [*] Invalid rule code in suppression: YF829 - --> suppressions.py:93:21 + --> suppressions.py:93:5 | 91 | def f(): 92 | # Unknown rule codes 93 | # ruff: disable[YF829] - | ^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^ 94 | # ruff: disable[F841, RQW320] 95 | value = 0 | @@ -638,12 +638,12 @@ help: Remove the rule code RUF102 [*] Invalid rule code in suppression: YF829 - --> suppressions.py:97:20 + --> 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] @@ -656,12 +656,12 @@ help: Remove the rule code RUF102 [*] Invalid rule code in suppression: TK421 - --> suppressions.py:102:21 + --> 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] | @@ -675,12 +675,12 @@ help: Remove the rule code RUF102 [*] Invalid rule code in suppression: TK421 - --> suppressions.py:104:20 + --> 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 diff --git a/crates/ruff_linter/src/suppression.rs b/crates/ruff_linter/src/suppression.rs index 535b44c0a12ad..a6195f560b65a 100644 --- a/crates/ruff_linter/src/suppression.rs +++ b/crates/ruff_linter/src/suppression.rs @@ -289,9 +289,8 @@ impl Suppressions { suppression: &Suppression, comment: &SuppressionComment, ) -> (TextRange, Edit) { - let range: TextRange; + let mut range = comment.range; let edit = if comment.codes.len() == 1 { - range = comment.codes[0]; delete_comment(comment.range, locator) } else { let code_index = comment From f5532fffe3958f63aecc4e0b4a1cfd162b784f25 Mon Sep 17 00:00:00 2001 From: Amethyst Reese Date: Tue, 16 Dec 2025 18:23:16 -0800 Subject: [PATCH 18/28] Avoid unnecessary assignments --- crates/ruff_linter/src/suppression.rs | 87 ++++++++++++++------------- 1 file changed, 46 insertions(+), 41 deletions(-) diff --git a/crates/ruff_linter/src/suppression.rs b/crates/ruff_linter/src/suppression.rs index a6195f560b65a..bf982c0ccd072 100644 --- a/crates/ruff_linter/src/suppression.rs +++ b/crates/ruff_linter/src/suppression.rs @@ -189,14 +189,15 @@ impl Suppressions { } }; - 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)); } } @@ -206,14 +207,15 @@ impl Suppressions { .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))); + context + .report_diagnostic( + UnusedNOQA { + codes: Some(UnusedCodes::default()), + kind: UnusedNOQAKind::Suppression, + }, + error.range, + ) + .set_fix(Fix::safe_edit(delete_comment(error.range, locator))); } } @@ -226,14 +228,15 @@ impl Suppressions { for comment in &suppression.comments { let (range, edit) = Suppressions::delete_code_or_comment(locator, suppression, comment); - let mut diagnostic = context.report_diagnostic( - InvalidRuleCode { - rule_code: suppression.code.to_string(), - kind: InvalidRuleCodeKind::Suppression, - }, - range, - ); - diagnostic.set_fix(Fix::safe_edit(edit)); + context + .report_diagnostic( + InvalidRuleCode { + rule_code: suppression.code.to_string(), + kind: InvalidRuleCodeKind::Suppression, + }, + range, + ) + .set_fix(Fix::safe_edit(edit)); } } } @@ -245,26 +248,28 @@ impl Suppressions { .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::unsafe_edit(delete_comment(error.range, locator))); + 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 { - let mut diagnostic = context.report_diagnostic( - InvalidSuppressionComment { - kind: InvalidSuppressionCommentKind::Invalid(invalid.kind), - }, - invalid.comment.range, - ); - diagnostic.set_fix(Fix::unsafe_edit(delete_comment( - invalid.comment.range, - locator, - ))); + context + .report_diagnostic( + InvalidSuppressionComment { + kind: InvalidSuppressionCommentKind::Invalid(invalid.kind), + }, + invalid.comment.range, + ) + .set_fix(Fix::unsafe_edit(delete_comment( + invalid.comment.range, + locator, + ))); } } From ea17270df8d972c452d01f34b47f0c1d48583933 Mon Sep 17 00:00:00 2001 From: Amethyst Reese Date: Wed, 17 Dec 2025 10:56:29 -0800 Subject: [PATCH 19/28] Use itertools unique --- crates/ruff_linter/src/suppression.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/ruff_linter/src/suppression.rs b/crates/ruff_linter/src/suppression.rs index bf982c0ccd072..14305d6019167 100644 --- a/crates/ruff_linter/src/suppression.rs +++ b/crates/ruff_linter/src/suppression.rs @@ -1,10 +1,10 @@ use compact_str::CompactString; use core::fmt; +use itertools::Itertools; 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; @@ -282,7 +282,7 @@ impl Suppressions { && suppression.comments[0].action == SuppressionAction::Disable }) .map(|suppression| suppression.comments[0].range) - .collect::>() + .unique() { context.report_diagnostic(UnmatchedSuppressionComment {}, range); } From b90f418ef40d322c33043da10b56750f8c8faf01 Mon Sep 17 00:00:00 2001 From: Amethyst Reese Date: Wed, 17 Dec 2025 11:24:35 -0800 Subject: [PATCH 20/28] Handle external codes and redirects --- .../src/rules/ruff/rules/invalid_rule_code.rs | 13 +++---- ...ules__ruff__tests__range_suppressions.snap | 36 +------------------ crates/ruff_linter/src/suppression.rs | 15 ++++---- 3 files changed, 16 insertions(+), 48 deletions(-) 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 56cf0585096f9..f8b24d6d862af 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 @@ -81,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; @@ -89,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); @@ -101,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( 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 a6eb782f31a71..7a23a5ed1c7bf 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 @@ -7,7 +7,7 @@ source: crates/ruff_linter/src/rules/ruff/mod.rs --- Summary --- Removed: 15 -Added: 25 +Added: 23 --- Removed --- E741 Ambiguous variable name: `I` @@ -653,37 +653,3 @@ help: Remove the rule code 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/suppression.rs b/crates/ruff_linter/src/suppression.rs index 14305d6019167..6db06d9437d3a 100644 --- a/crates/ruff_linter/src/suppression.rs +++ b/crates/ruff_linter/src/suppression.rs @@ -18,9 +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::rule_redirects::get_redirect_target; use crate::rules::ruff::rules::{ InvalidRuleCode, InvalidRuleCodeKind, InvalidSuppressionComment, InvalidSuppressionCommentKind, - UnmatchedSuppressionComment, UnusedCodes, UnusedNOQA, UnusedNOQAKind, + UnmatchedSuppressionComment, UnusedCodes, UnusedNOQA, UnusedNOQAKind, code_is_valid, }; use crate::settings::LinterSettings; @@ -154,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; } @@ -220,11 +223,9 @@ impl Suppressions { } if context.is_rule_enabled(Rule::InvalidRuleCode) { - for suppression in self - .valid - .iter() - .filter(|suppression| Rule::from_code(&suppression.code).is_err()) - { + for suppression in self.valid.iter().filter(|suppression| { + !code_is_valid(&suppression.code, &context.settings().external) + }) { for comment in &suppression.comments { let (range, edit) = Suppressions::delete_code_or_comment(locator, suppression, comment); From 5bdf0497ad660b7647f54bfb058a7a8293c25592 Mon Sep 17 00:00:00 2001 From: Amethyst Reese Date: Wed, 17 Dec 2025 17:11:24 -0800 Subject: [PATCH 21/28] Reorganize check_suppressions to iterate over valid/errors only once --- ...ules__ruff__tests__range_suppressions.snap | 24 +-- crates/ruff_linter/src/suppression.rs | 177 ++++++++---------- 2 files changed, 80 insertions(+), 121 deletions(-) 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 7a23a5ed1c7bf..1c76dcf14082d 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 @@ -7,7 +7,7 @@ source: crates/ruff_linter/src/rules/ruff/mod.rs --- Summary --- Removed: 15 -Added: 23 +Added: 21 --- Removed --- E741 Ambiguous variable name: `I` @@ -393,17 +393,6 @@ RUF104 Suppression comment without matching `#ruff:enable` comment | -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 | @@ -505,17 +494,6 @@ 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 | diff --git a/crates/ruff_linter/src/suppression.rs b/crates/ruff_linter/src/suppression.rs index 6db06d9437d3a..6b99bde8d26b5 100644 --- a/crates/ruff_linter/src/suppression.rs +++ b/crates/ruff_linter/src/suppression.rs @@ -5,6 +5,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; @@ -166,99 +167,94 @@ impl Suppressions { } pub(crate) fn check_suppressions(&self, context: &LintContext, locator: &Locator) { - if context.is_rule_enabled(Rule::UnusedNOQA) { - 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); - - 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 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); + 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; // should trigger InvalidRuleCode above }; - - context - .report_diagnostic( - UnusedNOQA { - codes: Some(codes), - kind: UnusedNOQAKind::Suppression, - }, - range, - ) - .set_fix(Fix::safe_edit(edit)); + for comment in &suppression.comments { + let (range, edit) = + Suppressions::delete_code_or_comment(locator, suppression, comment); + + 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)); + } + } + } 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); } - } - - // 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))); } } - if context.is_rule_enabled(Rule::InvalidRuleCode) { - for suppression in self.valid.iter().filter(|suppression| { - !code_is_valid(&suppression.code, &context.settings().external) - }) { - 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)); + for error in &self.errors { + // treat comments with no codes as unused suppression + if error.kind == ParseErrorKind::MissingCodes { + if let Some(mut diagnostic) = context.report_diagnostic_if_enabled( + UnusedNOQA { + codes: Some(UnusedCodes::default()), + kind: UnusedNOQAKind::Suppression, + }, + error.range, + ) { + diagnostic.set_fix(Fix::safe_edit(delete_comment(error.range, locator))); + } + } else { + if let Some(mut diagnostic) = context.report_diagnostic_if_enabled( + InvalidSuppressionComment { + kind: InvalidSuppressionCommentKind::Error(error.kind), + }, + error.range, + ) { + diagnostic.set_fix(Fix::unsafe_edit(delete_comment(error.range, locator))); } } } 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( @@ -273,21 +269,6 @@ impl Suppressions { ))); } } - - 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) - .unique() - { - context.report_diagnostic(UnmatchedSuppressionComment {}, range); - } - } } fn delete_code_or_comment( From 0548374e5eb7f0199aea212429ffb4d886084581 Mon Sep 17 00:00:00 2001 From: Amethyst Reese Date: Wed, 17 Dec 2025 17:19:39 -0800 Subject: [PATCH 22/28] clippy --- crates/ruff_linter/src/suppression.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/ruff_linter/src/suppression.rs b/crates/ruff_linter/src/suppression.rs index 6b99bde8d26b5..cb663701894f2 100644 --- a/crates/ruff_linter/src/suppression.rs +++ b/crates/ruff_linter/src/suppression.rs @@ -1,6 +1,5 @@ use compact_str::CompactString; use core::fmt; -use itertools::Itertools; use ruff_db::diagnostic::Diagnostic; use ruff_diagnostics::{Edit, Fix}; use ruff_python_ast::token::{TokenKind, Tokens}; From bafc629c11e3f8979d2dbbe028f31cd3a194fe57 Mon Sep 17 00:00:00 2001 From: Amethyst Reese Date: Wed, 17 Dec 2025 18:04:26 -0800 Subject: [PATCH 23/28] Fix docs --- .../ruff/rules/unmatched_suppression_comment.rs | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) 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 index 818cfd2150fae..1b31882ae6f98 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/unmatched_suppression_comment.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/unmatched_suppression_comment.rs @@ -12,25 +12,20 @@ use crate::Violation; /// ## Example /// ```python /// def foo(): -/// ruff: disable[E501] # unmatched -/// REALLY_LONG_VALUES = [ -/// ... -/// ] +/// # ruff: disable[E501] # unmatched +/// REALLY_LONG_VALUES = [...] /// -/// print(REALLY_LONG_VALUE) +/// print(REALLY_LONG_VALUES) /// ``` /// /// Use instead: /// ```python /// def foo(): -/// ... /// # ruff: disable[E501] -/// REALLY_LONG_VALUES = [ -/// ... -/// ] +/// REALLY_LONG_VALUES = [...] /// # ruff: enable[E501] /// -/// print(REALLY_LONG_VALUE) +/// print(REALLY_LONG_VALUES) /// ``` /// /// ## References From a6c73ae10dbba3ca8b1255a5e25b123a75c65639 Mon Sep 17 00:00:00 2001 From: Amethyst Reese Date: Thu, 18 Dec 2025 10:30:12 -0800 Subject: [PATCH 24/28] Only target the invalid rule code --- crates/ruff_linter/src/suppression.rs | 54 +++++++++++++++------------ 1 file changed, 30 insertions(+), 24 deletions(-) diff --git a/crates/ruff_linter/src/suppression.rs b/crates/ruff_linter/src/suppression.rs index cb663701894f2..9a681b4c76704 100644 --- a/crates/ruff_linter/src/suppression.rs +++ b/crates/ruff_linter/src/suppression.rs @@ -173,7 +173,7 @@ impl Suppressions { if context.is_rule_enabled(Rule::InvalidRuleCode) { for comment in &suppression.comments { let (range, edit) = - Suppressions::delete_code_or_comment(locator, suppression, comment); + Suppressions::delete_code(locator, suppression, comment); context .report_diagnostic( InvalidRuleCode { @@ -270,35 +270,41 @@ impl Suppressions { } } - fn delete_code_or_comment( + fn delete_code( 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) + let code_index = comment + .codes + .iter() + .position(|range| locator.slice(range) == suppression.code) + .unwrap(); + let range = comment.codes[code_index]; + let edit_range = if code_index < (comment.codes.len() - 1) { + TextRange::new( + comment.codes[code_index].start(), + comment.codes[code_index + 1].start(), + ) } 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) + TextRange::new( + comment.codes[code_index - 1].end(), + comment.codes[code_index].end(), + ) }; - (range, edit) + (range, Edit::range_deletion(edit_range)) + } + + fn delete_code_or_comment( + locator: &Locator<'_>, + suppression: &Suppression, + comment: &SuppressionComment, + ) -> (TextRange, Edit) { + if comment.codes.len() == 1 { + (comment.range, delete_comment(comment.range, locator)) + } else { + Suppressions::delete_code(locator, suppression, comment) + } } } From 0e8b8aa84781703c15dcda19ce79b86ffda557d3 Mon Sep 17 00:00:00 2001 From: Amethyst Reese Date: Thu, 18 Dec 2025 10:31:39 -0800 Subject: [PATCH 25/28] Treat missing codes as invalid suppression comment, update error message --- crates/ruff_linter/src/suppression.rs | 33 +++++++++------------------ 1 file changed, 11 insertions(+), 22 deletions(-) diff --git a/crates/ruff_linter/src/suppression.rs b/crates/ruff_linter/src/suppression.rs index 9a681b4c76704..3fdddca086bea 100644 --- a/crates/ruff_linter/src/suppression.rs +++ b/crates/ruff_linter/src/suppression.rs @@ -229,27 +229,16 @@ impl Suppressions { } } - for error in &self.errors { - // treat comments with no codes as unused suppression - if error.kind == ParseErrorKind::MissingCodes { - if let Some(mut diagnostic) = context.report_diagnostic_if_enabled( - UnusedNOQA { - codes: Some(UnusedCodes::default()), - kind: UnusedNOQAKind::Suppression, - }, - error.range, - ) { - diagnostic.set_fix(Fix::safe_edit(delete_comment(error.range, locator))); - } - } else { - if let Some(mut diagnostic) = context.report_diagnostic_if_enabled( - InvalidSuppressionComment { - kind: InvalidSuppressionCommentKind::Error(error.kind), - }, - error.range, - ) { - diagnostic.set_fix(Fix::unsafe_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))); } } @@ -472,7 +461,7 @@ pub(crate) enum ParseErrorKind { #[error("unknown ruff directive")] UnknownAction, - #[error("missing suppression codes")] + #[error("missing suppression codes, add one or more codes like `[E501, ...]`")] MissingCodes, #[error("missing closing bracket")] From 5f5bd0563d2f796572fb2199a6bb568d8e4dcd8e Mon Sep 17 00:00:00 2001 From: Amethyst Reese Date: Thu, 18 Dec 2025 10:57:37 -0800 Subject: [PATCH 26/28] Better implementation of highlighting only code for invalid rule codes --- ...ules__ruff__tests__range_suppressions.snap | 8 +-- crates/ruff_linter/src/suppression.rs | 72 ++++++++++--------- 2 files changed, 43 insertions(+), 37 deletions(-) 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 1c76dcf14082d..4668d83ff7f08 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 @@ -555,12 +555,12 @@ help: Remove unused suppression RUF102 [*] Invalid rule code in suppression: YF829 - --> suppressions.py:93:5 + --> suppressions.py:93:21 | 91 | def f(): 92 | # Unknown rule codes 93 | # ruff: disable[YF829] - | ^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^ 94 | # ruff: disable[F841, RQW320] 95 | value = 0 | @@ -616,12 +616,12 @@ help: Remove the rule code RUF102 [*] Invalid rule code in suppression: YF829 - --> suppressions.py:97:5 + --> 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] diff --git a/crates/ruff_linter/src/suppression.rs b/crates/ruff_linter/src/suppression.rs index 3fdddca086bea..8bedc3db690be 100644 --- a/crates/ruff_linter/src/suppression.rs +++ b/crates/ruff_linter/src/suppression.rs @@ -172,8 +172,12 @@ impl Suppressions { // InvalidRuleCode if context.is_rule_enabled(Rule::InvalidRuleCode) { for comment in &suppression.comments { - let (range, edit) = - Suppressions::delete_code(locator, suppression, comment); + let (range, edit) = Suppressions::delete_code_or_comment( + locator, + suppression, + comment, + true, + ); context .report_diagnostic( InvalidRuleCode { @@ -194,8 +198,12 @@ impl Suppressions { continue; // should trigger InvalidRuleCode above }; for comment in &suppression.comments { - let (range, edit) = - Suppressions::delete_code_or_comment(locator, suppression, comment); + let (range, edit) = Suppressions::delete_code_or_comment( + locator, + suppression, + comment, + false, + ); let codes = if context.is_rule_enabled(rule) { UnusedCodes { @@ -259,41 +267,39 @@ impl Suppressions { } } - fn delete_code( - locator: &Locator<'_>, - suppression: &Suppression, - comment: &SuppressionComment, - ) -> (TextRange, Edit) { - let code_index = comment - .codes - .iter() - .position(|range| locator.slice(range) == suppression.code) - .unwrap(); - let range = comment.codes[code_index]; - let edit_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(), - ) - }; - (range, Edit::range_deletion(edit_range)) - } - fn delete_code_or_comment( locator: &Locator<'_>, suppression: &Suppression, comment: &SuppressionComment, + highlight_only_code: bool, ) -> (TextRange, Edit) { - if comment.codes.len() == 1 { - (comment.range, delete_comment(comment.range, locator)) + 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 { - Suppressions::delete_code(locator, suppression, comment) - } + 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) } } From b14a6ac1a3a0e496cc500cc50c8ad7c51b85111d Mon Sep 17 00:00:00 2001 From: Amethyst Reese Date: Thu, 18 Dec 2025 11:18:17 -0800 Subject: [PATCH 27/28] More test cases, improve error/fix wording --- .../test/fixtures/ruff/suppressions.py | 7 ++++ .../ruff/rules/invalid_suppression_comment.rs | 2 +- ...ules__ruff__tests__range_suppressions.snap | 42 ++++++++++++++++++- crates/ruff_linter/src/suppression.rs | 2 +- 4 files changed, 49 insertions(+), 4 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/ruff/suppressions.py b/crates/ruff_linter/resources/test/fixtures/ruff/suppressions.py index 2a4b06898aea4..4f4796a01a0ea 100644 --- a/crates/ruff_linter/resources/test/fixtures/ruff/suppressions.py +++ b/crates/ruff_linter/resources/test/fixtures/ruff/suppressions.py @@ -102,3 +102,10 @@ def f(): # 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/rules/ruff/rules/invalid_suppression_comment.rs b/crates/ruff_linter/src/rules/ruff/rules/invalid_suppression_comment.rs index 56e09ad936fb2..2ae52f26d1100 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 @@ -49,7 +49,7 @@ impl AlwaysFixableViolation for InvalidSuppressionComment { } fn fix_title(&self) -> String { - "Remove invalid suppression comment".to_string() + "Remove suppression 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 4668d83ff7f08..ddac917621b59 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 @@ -7,7 +7,7 @@ source: crates/ruff_linter/src/rules/ruff/mod.rs --- Summary --- Removed: 15 -Added: 21 +Added: 23 --- Removed --- E741 Ambiguous variable name: `I` @@ -281,7 +281,7 @@ RUF103 [*] Invalid suppression comment: no matching 'disable' comment 19 | # ruff: enable[E741, F841] | ^^^^^^^^^^^^^^^^^^^^^^^^^^ | -help: Remove invalid suppression comment +help: Remove suppression comment 16 | # Neither warning is ignored, and an "unmatched suppression" 17 | # should be generated. 18 | I = 1 @@ -631,3 +631,41 @@ help: Remove the rule code 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/suppression.rs b/crates/ruff_linter/src/suppression.rs index 8bedc3db690be..d78a1ebb9ab72 100644 --- a/crates/ruff_linter/src/suppression.rs +++ b/crates/ruff_linter/src/suppression.rs @@ -467,7 +467,7 @@ pub(crate) enum ParseErrorKind { #[error("unknown ruff directive")] UnknownAction, - #[error("missing suppression codes, add one or more codes like `[E501, ...]`")] + #[error("missing suppression codes like `[E501, ...]`")] MissingCodes, #[error("missing closing bracket")] From 78e8594189681bdba456f0f99deb63949daf7b20 Mon Sep 17 00:00:00 2001 From: Amethyst Reese Date: Thu, 18 Dec 2025 11:42:21 -0800 Subject: [PATCH 28/28] Update comment to clarify unused external codes --- crates/ruff_linter/src/suppression.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/ruff_linter/src/suppression.rs b/crates/ruff_linter/src/suppression.rs index d78a1ebb9ab72..4cb5deca69b52 100644 --- a/crates/ruff_linter/src/suppression.rs +++ b/crates/ruff_linter/src/suppression.rs @@ -195,7 +195,7 @@ impl Suppressions { let Ok(rule) = Rule::from_code( get_redirect_target(&suppression.code).unwrap_or(&suppression.code), ) else { - continue; // should trigger InvalidRuleCode above + continue; // "external" lint code, don't treat it as unused }; for comment in &suppression.comments { let (range, edit) = Suppressions::delete_code_or_comment(