From af80f3126e7011c96925906cca1bc7466863d40d Mon Sep 17 00:00:00 2001 From: Amethyst Reese Date: Wed, 10 Dec 2025 12:53:40 -0800 Subject: [PATCH 01/18] 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 ebec5f4acc..04fb7eee11 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -1063,6 +1063,8 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Ruff, "100") => rules::ruff::rules::UnusedNOQA, (Ruff, "101") => rules::ruff::rules::RedirectedNOQA, (Ruff, "102") => rules::ruff::rules::InvalidRuleCode, + (Ruff, "103") => rules::ruff::rules::InvalidSuppressionComment, + (Ruff, "104") => rules::ruff::rules::UnmatchedSuppressionComment, (Ruff, "200") => rules::ruff::rules::InvalidPyprojectToml, #[cfg(any(feature = "test-rules", test))] diff --git a/crates/ruff_linter/src/rules/ruff/rules/invalid_suppression_comment.rs b/crates/ruff_linter/src/rules/ruff/rules/invalid_suppression_comment.rs new file mode 100644 index 0000000000..8e6ac88dd3 --- /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 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..2dae90a8c1 --- /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 1c8a092042..7ff15dbe14 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -4050,6 +4050,8 @@ "RUF100", "RUF101", "RUF102", + "RUF103", + "RUF104", "RUF2", "RUF20", "RUF200", From 5de905120e1d723cc57755b5c24e345dbd3950d1 Mon Sep 17 00:00:00 2001 From: Amethyst Reese Date: Wed, 10 Dec 2025 15:17:11 -0800 Subject: [PATCH 02/18] 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 24a43ade5e..908cca2cbd 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 18c6ad50ce..7b4e57a67a 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 45c2ede4932225a1ff726afd0decb2e0c5e54ac4 Mon Sep 17 00:00:00 2001 From: Amethyst Reese Date: Wed, 10 Dec 2025 17:10:48 -0800 Subject: [PATCH 03/18] 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 8e6ac88dd3..54e2da0434 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/invalid_suppression_comment.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/invalid_suppression_comment.rs @@ -1,6 +1,7 @@ use ruff_macros::{ViolationMetadata, derive_message_formats}; -use crate::AlwaysFixableViolation; +use crate::suppression::{InvalidSuppressionKind, ParseErrorKind}; +use crate::{AlwaysFixableViolation, Violation}; /// ## What it does /// Checks for invalid suppression comments @@ -25,15 +26,34 @@ use crate::AlwaysFixableViolation; /// - [Ruff error suppression](https://docs.astral.sh/ruff/linter/#error-suppression) #[derive(ViolationMetadata)] #[violation_metadata(preview_since = "0.14.9")] -pub(crate) struct InvalidSuppressionComment; +pub(crate) struct InvalidSuppressionComment { + pub kind: InvalidSuppressionCommentKind, +} -impl AlwaysFixableViolation for InvalidSuppressionComment { +impl Violation for InvalidSuppressionComment { #[derive_message_formats] fn message(&self) -> String { - "Invalid suppression comment".to_string() + let msg = match self.kind { + InvalidSuppressionCommentKind::Invalid(InvalidSuppressionKind::Indentation) => { + "unexpected indentation".to_string() + } + InvalidSuppressionCommentKind::Invalid(InvalidSuppressionKind::Trailing) => { + "trailing comments are not supported".to_string() + } + InvalidSuppressionCommentKind::Invalid(InvalidSuppressionKind::Unmatched) => { + "no matching 'disable' comment".to_string() + } + InvalidSuppressionCommentKind::Error(error) => format!("{error}"), + }; + format!("Invalid suppression comment: {msg}") } - fn fix_title(&self) -> String { - "Remove invalid suppression comment".to_string() + fn fix_title(&self) -> Option { + Some("Remove invalid suppression comment".to_string()) } } + +pub(crate) enum InvalidSuppressionCommentKind { + Invalid(InvalidSuppressionKind), + Error(ParseErrorKind), +} diff --git a/crates/ruff_linter/src/suppression.rs b/crates/ruff_linter/src/suppression.rs index 7b4e57a67a..f7778d1403 100644 --- a/crates/ruff_linter/src/suppression.rs +++ b/crates/ruff_linter/src/suppression.rs @@ -19,7 +19,8 @@ use crate::codes::Rule; use crate::fix::edits::delete_comment; use crate::preview::is_range_suppressions_enabled; use crate::rules::ruff::rules::{ - UnmatchedSuppressionComment, UnusedCodes, UnusedNOQA, UnusedNOQAKind, + InvalidSuppressionComment, InvalidSuppressionCommentKind, UnmatchedSuppressionComment, + UnusedCodes, UnusedNOQA, UnusedNOQAKind, }; use crate::settings::LinterSettings; @@ -133,7 +134,7 @@ impl Suppressions { } pub(crate) fn is_empty(&self) -> bool { - self.valid.is_empty() + self.valid.is_empty() && self.invalid.is_empty() && self.errors.is_empty() } /// Check if a diagnostic is suppressed by any known range suppressions @@ -162,7 +163,12 @@ impl Suppressions { } pub(crate) fn check_suppressions(&self, context: &LintContext, locator: &Locator) { - if !context.any_rule_enabled(&[Rule::UnusedNOQA, Rule::InvalidRuleCode]) { + if !context.any_rule_enabled(&[ + Rule::UnusedNOQA, + Rule::InvalidRuleCode, + Rule::InvalidSuppressionComment, + Rule::UnmatchedSuppressionComment, + ]) { return; } @@ -223,6 +229,40 @@ impl Suppressions { } } + for error in &self.errors { + // treat comments with no codes as unused suppression + let mut diagnostic = if error.kind == ParseErrorKind::MissingCodes { + context.report_diagnostic( + UnusedNOQA { + codes: Some(UnusedCodes::default()), + kind: UnusedNOQAKind::Suppression, + }, + error.range, + ) + } else { + context.report_diagnostic( + InvalidSuppressionComment { + kind: InvalidSuppressionCommentKind::Error(error.kind), + }, + error.range, + ) + }; + diagnostic.set_fix(Fix::safe_edit(delete_comment(error.range, locator))); + } + + for invalid in &self.invalid { + let mut diagnostic = context.report_diagnostic( + InvalidSuppressionComment { + kind: InvalidSuppressionCommentKind::Invalid(invalid.kind), + }, + invalid.comment.range, + ); + diagnostic.set_fix(Fix::safe_edit(delete_comment( + invalid.comment.range, + locator, + ))); + } + let unmatched = self .valid .iter() @@ -235,21 +275,6 @@ impl Suppressions { for range in unmatched { context.report_diagnostic(UnmatchedSuppressionComment {}, range); } - - for error in self - .errors - .iter() - .filter(|error| error.kind == ParseErrorKind::MissingCodes) - { - let mut diagnostic = context.report_diagnostic( - UnusedNOQA { - codes: Some(UnusedCodes::default()), - kind: UnusedNOQAKind::Suppression, - }, - error.range, - ); - diagnostic.set_fix(Fix::safe_edit(delete_comment(error.range, locator))); - } } } @@ -407,7 +432,7 @@ impl<'a> SuppressionsBuilder<'a> { } #[derive(Copy, Clone, Debug, Eq, Error, PartialEq)] -enum ParseErrorKind { +pub(crate) enum ParseErrorKind { #[error("not a suppression comment")] NotASuppression, From 61b48fbcc41188ca62596c850e624bdf2e1bcb37 Mon Sep 17 00:00:00 2001 From: Amethyst Reese Date: Wed, 10 Dec 2025 17:12:04 -0800 Subject: [PATCH 04/18] 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 f7778d1403..fd5e2064ba 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 e19ddb6e1563ec4b39b0c2a4194f7ab23428b578 Mon Sep 17 00:00:00 2001 From: Amethyst Reese Date: Thu, 11 Dec 2025 15:05:06 -0800 Subject: [PATCH 05/18] 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 c2d03fb1ae..ad18e24d59 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 54e2da0434..25b9eb2ebb 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 908cca2cbd..0094d8474a 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 fd5e2064ba..83acaffe4f 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 f48d4a437b495d3da43157f3b528bedff9f59113 Mon Sep 17 00:00:00 2001 From: Amethyst Reese Date: Thu, 11 Dec 2025 16:30:55 -0800 Subject: [PATCH 06/18] Move rule-enabled checks to local logic, extract some bits --- .../ruff/rules/invalid_suppression_comment.rs | 2 +- crates/ruff_linter/src/suppression.rs | 203 ++++++++++-------- 2 files changed, 110 insertions(+), 95 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 25b9eb2ebb..7c73578b9c 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 83acaffe4f..87e382f322 100644 --- a/crates/ruff_linter/src/suppression.rs +++ b/crates/ruff_linter/src/suppression.rs @@ -163,119 +163,134 @@ impl Suppressions { } pub(crate) fn check_suppressions(&self, context: &LintContext, locator: &Locator) { - // TODO: wrap these around relevant bits below - if !context.any_rule_enabled(&[ - Rule::UnusedNOQA, - Rule::InvalidRuleCode, - Rule::InvalidSuppressionComment, - Rule::UnmatchedSuppressionComment, - ]) { - return; - } + if context.is_rule_enabled(Rule::UnusedNOQA) { + let unused = self + .valid + .iter() + .filter(|suppression| !suppression.used.get()); - let unused = self - .valid - .iter() - .filter(|suppression| !suppression.used.get()); + for suppression in unused { + let Ok(rule) = Rule::from_code(&suppression.code) else { + continue; // TODO: invalid code + }; + for comment in &suppression.comments { + let (range, edit) = + Suppressions::delete_code_or_comment(locator, suppression, comment); - for suppression in unused { - let Ok(rule) = Rule::from_code(&suppression.code) else { - continue; // TODO: invalid code - }; - for comment in &suppression.comments { - let mut range = comment.range; - let edit = if comment.codes.len() == 1 { - delete_comment(comment.range, locator) - } else { - let code_index = comment - .codes - .iter() - .position(|range| locator.slice(range) == suppression.code) - .unwrap(); - range = comment.codes[code_index]; - let code_range = if code_index < (comment.codes.len() - 1) { - TextRange::new( - comment.codes[code_index].start(), - comment.codes[code_index + 1].start(), - ) + let codes = if context.is_rule_enabled(rule) { + UnusedCodes { + unmatched: vec![suppression.code.to_string()], + ..Default::default() + } } else { - TextRange::new( - comment.codes[code_index - 1].end(), - comment.codes[code_index].end(), - ) + UnusedCodes { + disabled: vec![suppression.code.to_string()], + ..Default::default() + } }; - Edit::range_deletion(code_range) - }; - let codes = if context.is_rule_enabled(rule) { - UnusedCodes { - unmatched: vec![suppression.code.to_string()], - ..Default::default() - } - } else { - UnusedCodes { - disabled: vec![suppression.code.to_string()], - ..Default::default() - } - }; - - let mut diagnostic = context.report_diagnostic( - UnusedNOQA { - codes: Some(codes), - kind: UnusedNOQAKind::Suppression, - }, - range, - ); - diagnostic.set_fix(Fix::safe_edit(edit)); + let mut diagnostic = context.report_diagnostic( + UnusedNOQA { + codes: Some(codes), + kind: UnusedNOQAKind::Suppression, + }, + range, + ); + diagnostic.set_fix(Fix::safe_edit(edit)); + } } - } - for error in &self.errors { // treat comments with no codes as unused suppression - let mut diagnostic = if error.kind == ParseErrorKind::MissingCodes { - context.report_diagnostic( + 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, - ) - } else { - context.report_diagnostic( + ); + diagnostic.set_fix(Fix::safe_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) + { + let mut diagnostic = context.report_diagnostic( InvalidSuppressionComment { kind: InvalidSuppressionCommentKind::Error(error.kind), }, error.range, + ); + diagnostic.set_fix(Fix::safe_edit(delete_comment(error.range, locator))); + } + + for invalid in &self.invalid { + let mut diagnostic = context.report_diagnostic( + InvalidSuppressionComment { + kind: InvalidSuppressionCommentKind::Invalid(invalid.kind), + }, + invalid.comment.range, + ); + diagnostic.set_fix(Fix::safe_edit(delete_comment( + invalid.comment.range, + locator, + ))); + } + } + + if context.is_rule_enabled(Rule::UnmatchedSuppressionComment) { + for range in self + .valid + .iter() + .filter(|suppression| { + suppression.comments.len() == 1 + && suppression.comments[0].action == SuppressionAction::Disable + }) + .map(|suppression| suppression.comments[0].range) + .collect::>() + { + context.report_diagnostic(UnmatchedSuppressionComment {}, range); + } + } + } + fn delete_code_or_comment( + locator: &Locator<'_>, + suppression: &Suppression, + comment: &SuppressionComment, + ) -> (TextRange, Edit) { + let mut range = comment.range; + let edit = if comment.codes.len() == 1 { + delete_comment(comment.range, locator) + } else { + let code_index = comment + .codes + .iter() + .position(|range| locator.slice(range) == suppression.code) + .unwrap(); + range = comment.codes[code_index]; + let code_range = if code_index < (comment.codes.len() - 1) { + TextRange::new( + comment.codes[code_index].start(), + comment.codes[code_index + 1].start(), + ) + } else { + TextRange::new( + comment.codes[code_index - 1].end(), + comment.codes[code_index].end(), ) }; - diagnostic.set_fix(Fix::safe_edit(delete_comment(error.range, locator))); - } - - 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); - } + Edit::range_deletion(code_range) + }; + (range, edit) } } From 25174307383613f0a544f1181da203cdd8d0a623 Mon Sep 17 00:00:00 2001 From: Amethyst Reese Date: Thu, 11 Dec 2025 16:59:18 -0800 Subject: [PATCH 07/18] Generate invalid rule code diagnostics --- .../test/fixtures/ruff/suppressions.py | 9 ++ ...ules__ruff__tests__range_suppressions.snap | 127 ++++++++++++++++-- crates/ruff_linter/src/suppression.rs | 27 +++- 3 files changed, 147 insertions(+), 16 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/ruff/suppressions.py b/crates/ruff_linter/resources/test/fixtures/ruff/suppressions.py index f8a3c882aa..2eabfcde89 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 0094d8474a..26aa8f1384 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,6 +391,17 @@ RUF104 Unmatched suppression comment | +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 + | + + RUF100 [*] Unused suppression (unused: `F841`) --> suppressions.py:69:5 | @@ -389,17 +421,6 @@ 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 | @@ -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 87e382f322..e9e9106d11 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 3c8cd30a351dc1dacba86440db86dad1d44c1ef5 Mon Sep 17 00:00:00 2001 From: Amethyst Reese Date: Thu, 11 Dec 2025 17:06:05 -0800 Subject: [PATCH 08/18] 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 26aa8f1384..21e633d189 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 e9e9106d11..87d082409e 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 cb211a85804beeed1d75e7cd3fcb1679008c6a7e Mon Sep 17 00:00:00 2001 From: Amethyst Reese Date: Thu, 11 Dec 2025 17:06:28 -0800 Subject: [PATCH 09/18] 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 87d082409e..6db42580b3 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 999a481e7449dd02b321883194478fac162e1f4d Mon Sep 17 00:00:00 2001 From: Amethyst Reese Date: Thu, 11 Dec 2025 17:25:45 -0800 Subject: [PATCH 10/18] 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 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 From 43429e11ec1162dbc2639d6bcb5b0021ec95dd53 Mon Sep 17 00:00:00 2001 From: Amethyst Reese Date: Tue, 16 Dec 2025 15:20:08 -0800 Subject: [PATCH 11/18] 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 2eabfcde89..2a4b06898a 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 ad18e24d59..9bc6363731 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 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 From 005fa7f7f0a7b39742b31bf10334a1cd7ba7cce8 Mon Sep 17 00:00:00 2001 From: Amethyst Reese Date: Tue, 16 Dec 2025 15:20:36 -0800 Subject: [PATCH 12/18] 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 7c73578b9c..56e09ad936 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 9929cf416cf457c7f4742242cd6dc9500e149b5e Mon Sep 17 00:00:00 2001 From: Amethyst Reese Date: Tue, 16 Dec 2025 16:30:55 -0800 Subject: [PATCH 13/18] 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 2dae90a8c1..818cfd2150 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 32af34141f6164711603ddde98d78475742a0ef0 Mon Sep 17 00:00:00 2001 From: Amethyst Reese Date: Tue, 16 Dec 2025 16:35:03 -0800 Subject: [PATCH 14/18] 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 6db42580b3..46bfe5489a 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 13b3143c29d8cd697b62ca7ceeee704d6f31a48f Mon Sep 17 00:00:00 2001 From: Amethyst Reese Date: Tue, 16 Dec 2025 16:36:50 -0800 Subject: [PATCH 15/18] 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 21e633d189..5dc7b3bb41 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 9735f0af19ecd29a368ff4c43cffce9a4c20cca0 Mon Sep 17 00:00:00 2001 From: Amethyst Reese Date: Tue, 16 Dec 2025 18:17:17 -0800 Subject: [PATCH 16/18] 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 3c9cde312e..56cf058509 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/invalid_rule_code.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/invalid_rule_code.rs @@ -9,6 +9,21 @@ use crate::registry::Rule; use crate::rule_redirects::get_redirect_target; use crate::{AlwaysFixableViolation, Edit, Fix}; +#[derive(Debug, PartialEq, Eq)] +pub(crate) enum InvalidRuleCodeKind { + Noqa, + Suppression, +} + +impl InvalidRuleCodeKind { + fn as_str(&self) -> &str { + match self { + InvalidRuleCodeKind::Noqa => "`# noqa`", + InvalidRuleCodeKind::Suppression => "suppression", + } + } +} + /// ## What it does /// Checks for `noqa` codes that are invalid. /// @@ -36,12 +51,17 @@ use crate::{AlwaysFixableViolation, Edit, Fix}; #[violation_metadata(preview_since = "0.11.4")] pub(crate) struct InvalidRuleCode { pub(crate) rule_code: String, + pub(crate) kind: InvalidRuleCodeKind, } impl AlwaysFixableViolation for InvalidRuleCode { #[derive_message_formats] fn message(&self) -> String { - format!("Invalid rule code in `# noqa`: {}", self.rule_code) + format!( + "Invalid rule code in {}: {}", + self.kind.as_str(), + self.rule_code + ) } fn fix_title(&self) -> String { @@ -100,6 +120,7 @@ fn all_codes_invalid_diagnostic( .map(Code::as_str) .collect::>() .join(", "), + kind: InvalidRuleCodeKind::Noqa, }, directive.range(), ) @@ -116,6 +137,7 @@ fn some_codes_are_invalid_diagnostic( .report_diagnostic( InvalidRuleCode { rule_code: invalid_code.to_string(), + kind: InvalidRuleCodeKind::Noqa, }, invalid_code.range(), ) diff --git a/crates/ruff_linter/src/rules/ruff/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 5dc7b3bb41..da3bd62f4b 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 46bfe5489a..535b44c0a1 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 c1101cb04cc82d5f330e3f12e60a2649b091b783 Mon Sep 17 00:00:00 2001 From: Amethyst Reese Date: Tue, 16 Dec 2025 18:20:38 -0800 Subject: [PATCH 17/18] 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 da3bd62f4b..a6eb782f31 100644 --- a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__range_suppressions.snap +++ b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__range_suppressions.snap @@ -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 535b44c0a1..a6195f560b 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 9ebae7aa6bfc4c2c1b44b5c929b821462410be31 Mon Sep 17 00:00:00 2001 From: Amethyst Reese Date: Tue, 16 Dec 2025 18:23:16 -0800 Subject: [PATCH 18/18] 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 a6195f560b..bf982c0ccd 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, + ))); } }