From f69de03302ce653c24f9801884cd910c5f9a7edc Mon Sep 17 00:00:00 2001 From: Amethyst Reese Date: Wed, 3 Dec 2025 12:59:12 -0800 Subject: [PATCH 01/14] Track which suppressions get used when filtering diagnostics --- crates/ruff_linter/src/checkers/noqa.rs | 2 +- crates/ruff_linter/src/linter.rs | 2 +- crates/ruff_linter/src/noqa.rs | 14 +++++++------- crates/ruff_linter/src/suppression.rs | 10 ++++++++-- 4 files changed, 17 insertions(+), 11 deletions(-) diff --git a/crates/ruff_linter/src/checkers/noqa.rs b/crates/ruff_linter/src/checkers/noqa.rs index 2602adeeeef6a..760abfe62d1d6 100644 --- a/crates/ruff_linter/src/checkers/noqa.rs +++ b/crates/ruff_linter/src/checkers/noqa.rs @@ -34,7 +34,7 @@ pub(crate) fn check_noqa( noqa_line_for: &NoqaMapping, analyze_directives: bool, settings: &LinterSettings, - suppressions: &Suppressions, + suppressions: &mut Suppressions, ) -> Vec { // Identify any codes that are globally exempted (within the current file). let file_noqa_directives = diff --git a/crates/ruff_linter/src/linter.rs b/crates/ruff_linter/src/linter.rs index 08c0417020b02..d6d9389f3e7ac 100644 --- a/crates/ruff_linter/src/linter.rs +++ b/crates/ruff_linter/src/linter.rs @@ -434,7 +434,7 @@ pub fn add_noqa_to_path( &directives.noqa_line_for, stylist.line_ending(), reason, - &suppressions, + &mut suppressions, ) } diff --git a/crates/ruff_linter/src/noqa.rs b/crates/ruff_linter/src/noqa.rs index e8c3ada650de4..0ff6583068277 100644 --- a/crates/ruff_linter/src/noqa.rs +++ b/crates/ruff_linter/src/noqa.rs @@ -735,7 +735,7 @@ pub(crate) fn add_noqa( noqa_line_for: &NoqaMapping, line_ending: LineEnding, reason: Option<&str>, - suppressions: &Suppressions, + suppressions: &mut Suppressions, ) -> Result { let (count, output) = add_noqa_inner( path, @@ -763,7 +763,7 @@ fn add_noqa_inner( noqa_line_for: &NoqaMapping, line_ending: LineEnding, reason: Option<&str>, - suppressions: &Suppressions, + suppressions: &mut Suppressions, ) -> (usize, String) { let mut count = 0; @@ -879,7 +879,7 @@ fn find_noqa_comments<'a>( exemption: &'a FileExemption, directives: &'a NoqaDirectives, noqa_line_for: &NoqaMapping, - suppressions: &Suppressions, + suppressions: &'a mut Suppressions, ) -> Vec>> { // List of noqa comments, ordered to match up with `messages` let mut comments_by_line: Vec>> = vec![]; @@ -2876,7 +2876,7 @@ mod tests { &noqa_line_for, LineEnding::Lf, None, - &Suppressions::default(), + &mut Suppressions::default(), ); assert_eq!(count, 0); assert_eq!(output, format!("{contents}")); @@ -2901,7 +2901,7 @@ mod tests { &noqa_line_for, LineEnding::Lf, None, - &Suppressions::default(), + &mut Suppressions::default(), ); assert_eq!(count, 1); assert_eq!(output, "x = 1 # noqa: F841\n"); @@ -2933,7 +2933,7 @@ mod tests { &noqa_line_for, LineEnding::Lf, None, - &Suppressions::default(), + &mut Suppressions::default(), ); assert_eq!(count, 1); assert_eq!(output, "x = 1 # noqa: E741, F841\n"); @@ -2965,7 +2965,7 @@ mod tests { &noqa_line_for, LineEnding::Lf, None, - &Suppressions::default(), + &mut Suppressions::default(), ); assert_eq!(count, 0); assert_eq!(output, "x = 1 # noqa"); diff --git a/crates/ruff_linter/src/suppression.rs b/crates/ruff_linter/src/suppression.rs index 3c1a2f57abfb4..7f149c6e900d7 100644 --- a/crates/ruff_linter/src/suppression.rs +++ b/crates/ruff_linter/src/suppression.rs @@ -79,6 +79,9 @@ pub(crate) struct Suppression { /// Any comments associated with the suppression comments: SmallVec<[SuppressionComment; 2]>, + + /// Whether this suppression actually suppressed a diagnostic + used: bool, } #[allow(unused)] @@ -130,7 +133,7 @@ impl Suppressions { } /// Check if a diagnostic is suppressed by any known range suppressions - pub(crate) fn check_diagnostic(&self, diagnostic: &Diagnostic) -> bool { + pub(crate) fn check_diagnostic(&mut self, diagnostic: &Diagnostic) -> bool { if self.valid.is_empty() { return false; } @@ -145,8 +148,9 @@ impl Suppressions { return false; }; - for suppression in &self.valid { + for suppression in &mut self.valid { if *code == suppression.code.as_str() && suppression.range.contains_range(range) { + suppression.used = true; return true; } } @@ -276,6 +280,7 @@ impl<'a> SuppressionsBuilder<'a> { code: code.into(), range: combined_range, comments: smallvec![comment.comment.clone(), other.comment.clone()], + used: false, }); } @@ -292,6 +297,7 @@ impl<'a> SuppressionsBuilder<'a> { code: code.into(), range: implicit_range, comments: smallvec![comment.comment.clone()], + used: false, }); } self.pending.remove(comment_index); From 8c7578eba896891b71256a9e42b13adf83ec1e46 Mon Sep 17 00:00:00 2001 From: Amethyst Reese Date: Wed, 3 Dec 2025 13:50:16 -0800 Subject: [PATCH 02/14] Generate diagnostics for unused suppressions --- crates/ruff_linter/src/checkers/noqa.rs | 2 + crates/ruff_linter/src/codes.rs | 1 + .../ruff_linter/src/rules/ruff/rules/mod.rs | 2 + .../rules/ruff/rules/unused_suppression.rs | 48 +++++++++++++++++++ crates/ruff_linter/src/suppression.rs | 39 +++++++++++++++ 5 files changed, 92 insertions(+) create mode 100644 crates/ruff_linter/src/rules/ruff/rules/unused_suppression.rs diff --git a/crates/ruff_linter/src/checkers/noqa.rs b/crates/ruff_linter/src/checkers/noqa.rs index 760abfe62d1d6..eea41f6594e2c 100644 --- a/crates/ruff_linter/src/checkers/noqa.rs +++ b/crates/ruff_linter/src/checkers/noqa.rs @@ -119,6 +119,8 @@ pub(crate) fn check_noqa( } } + suppressions.generate_diagnostics(context, locator); + // Enforce that the noqa directive was actually used (RUF100), unless RUF100 was itself // suppressed. if context.is_rule_enabled(Rule::UnusedNOQA) diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index ebec5f4acc09d..4dfab63e08608 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -1063,6 +1063,7 @@ 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::UnusedSuppression, (Ruff, "200") => rules::ruff::rules::InvalidPyprojectToml, #[cfg(any(feature = "test-rules", test))] diff --git a/crates/ruff_linter/src/rules/ruff/rules/mod.rs b/crates/ruff_linter/src/rules/ruff/rules/mod.rs index 206a504e740b4..6cd4cbb51838e 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/mod.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/mod.rs @@ -57,6 +57,7 @@ pub(crate) use unraw_re_pattern::*; pub(crate) use unsafe_markup_use::*; pub(crate) use unused_async::*; pub(crate) use unused_noqa::*; +pub(crate) use unused_suppression::*; pub(crate) use unused_unpacked_variable::*; pub(crate) use used_dummy_variable::*; pub(crate) use useless_if_else::*; @@ -124,6 +125,7 @@ mod unraw_re_pattern; mod unsafe_markup_use; mod unused_async; mod unused_noqa; +mod unused_suppression; mod unused_unpacked_variable; mod used_dummy_variable; mod useless_if_else; diff --git a/crates/ruff_linter/src/rules/ruff/rules/unused_suppression.rs b/crates/ruff_linter/src/rules/ruff/rules/unused_suppression.rs new file mode 100644 index 0000000000000..85d38aa707153 --- /dev/null +++ b/crates/ruff_linter/src/rules/ruff/rules/unused_suppression.rs @@ -0,0 +1,48 @@ +use crate::AlwaysFixableViolation; +use ruff_macros::{ViolationMetadata, derive_message_formats}; + +/// ## What it does +/// Checks for range suppressions that are no longer used. +/// +/// ## Why is this bad? +/// A range suppression that no longer matches any diagnostic violations is +/// likely included by mistake, and should be removed to avoid confusion. +/// +/// ## Example +/// ```python +/// # ruff: disable[F401] +/// import foo +/// +/// +/// def bar(): +/// foo.bar() +/// ``` +/// +/// Use instead: +/// ```python +/// import foo +/// +/// +/// def bar(): +/// foo.bar() +/// ``` +/// +/// ## Options +/// - `lint.external` +/// +/// ## References +/// - [Ruff error suppression](https://docs.astral.sh/ruff/linter/#error-suppression) +#[derive(ViolationMetadata)] +#[violation_metadata(preview_since = "0.14.9")] +pub(crate) struct UnusedSuppression; + +impl AlwaysFixableViolation for UnusedSuppression { + #[derive_message_formats] + fn message(&self) -> String { + "Unused suppression".to_string() + } + + fn fix_title(&self) -> String { + "Remove unused suppression".to_string() + } +} diff --git a/crates/ruff_linter/src/suppression.rs b/crates/ruff_linter/src/suppression.rs index 7f149c6e900d7..ef17c9261ddb1 100644 --- a/crates/ruff_linter/src/suppression.rs +++ b/crates/ruff_linter/src/suppression.rs @@ -1,6 +1,7 @@ use compact_str::CompactString; use core::fmt; use ruff_db::diagnostic::Diagnostic; +use ruff_diagnostics::{Edit, Fix}; use ruff_python_ast::token::{TokenKind, Tokens}; use ruff_python_ast::whitespace::indentation; use std::{error::Error, fmt::Formatter}; @@ -10,7 +11,12 @@ use ruff_python_trivia::Cursor; use ruff_text_size::{Ranged, TextLen, TextRange, TextSize, TextSlice}; use smallvec::{SmallVec, smallvec}; +use crate::Locator; +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::UnusedSuppression; use crate::settings::LinterSettings; #[allow(unused)] @@ -156,6 +162,39 @@ impl Suppressions { } false } + + pub(crate) fn generate_diagnostics(&self, context: &LintContext, locator: &Locator) { + if context.is_rule_enabled(Rule::UnusedSuppression) { + for suppression in &self.valid { + if !suppression.used { + for comment in &suppression.comments { + let edit = if comment.codes.len() == 1 { + delete_comment(suppression.range, locator) + } else { + let code_index = comment + .codes + .iter() + .position(|range| locator.slice(range) == suppression.code) + .unwrap(); + let code_range = if code_index < (comment.codes.len() - 1) { + TextRange::new( + comment.codes[code_index].start(), + comment.codes[code_index + 1].start(), + ) + } else { + comment.codes[code_index] + }; + Edit::range_deletion(code_range) + }; + + let mut diagnostic = context + .report_diagnostic(UnusedSuppression, suppression.comments[0].range); + diagnostic.set_fix(Fix::safe_edit(edit)); + } + } + } + } + } } #[derive(Default)] From ea5618a11374a888354ac12d8535e1fafed86b48 Mon Sep 17 00:00:00 2001 From: Amethyst Reese Date: Wed, 3 Dec 2025 18:20:16 -0800 Subject: [PATCH 03/14] Fix panic with delete_comment --- crates/ruff_linter/src/checkers/noqa.rs | 1 + crates/ruff_linter/src/suppression.rs | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/ruff_linter/src/checkers/noqa.rs b/crates/ruff_linter/src/checkers/noqa.rs index eea41f6594e2c..53702bcbd475f 100644 --- a/crates/ruff_linter/src/checkers/noqa.rs +++ b/crates/ruff_linter/src/checkers/noqa.rs @@ -119,6 +119,7 @@ pub(crate) fn check_noqa( } } + // Diagnostics for unused/invalid range suppressions suppressions.generate_diagnostics(context, locator); // Enforce that the noqa directive was actually used (RUF100), unless RUF100 was itself diff --git a/crates/ruff_linter/src/suppression.rs b/crates/ruff_linter/src/suppression.rs index ef17c9261ddb1..0e1bb38b19094 100644 --- a/crates/ruff_linter/src/suppression.rs +++ b/crates/ruff_linter/src/suppression.rs @@ -169,7 +169,7 @@ impl Suppressions { if !suppression.used { for comment in &suppression.comments { let edit = if comment.codes.len() == 1 { - delete_comment(suppression.range, locator) + delete_comment(comment.range, locator) } else { let code_index = comment .codes From b35918d11e60904ccb05e2bece9f41d57f4ba3cf Mon Sep 17 00:00:00 2001 From: Amethyst Reese Date: Thu, 4 Dec 2025 17:24:55 -0800 Subject: [PATCH 04/14] Merge changes from base branch --- crates/ruff_linter/src/linter.rs | 20 +++++++++++--------- crates/ruff_linter/src/noqa.rs | 10 +++++----- crates/ruff_linter/src/rules/pyflakes/mod.rs | 4 ++-- crates/ruff_linter/src/test.rs | 8 ++++---- crates/ruff_server/src/lint.rs | 6 +++--- crates/ruff_wasm/src/lib.rs | 4 ++-- 6 files changed, 27 insertions(+), 25 deletions(-) diff --git a/crates/ruff_linter/src/linter.rs b/crates/ruff_linter/src/linter.rs index d6d9389f3e7ac..fc3aa3907fa21 100644 --- a/crates/ruff_linter/src/linter.rs +++ b/crates/ruff_linter/src/linter.rs @@ -129,7 +129,7 @@ pub fn check_path( source_type: PySourceType, parsed: &Parsed, target_version: TargetVersion, - suppressions: &Suppressions, + suppressions: &mut Suppressions, ) -> Vec { // Aggregate all diagnostics. let mut context = LintContext::new(path, locator.contents(), settings); @@ -404,7 +404,7 @@ pub fn add_noqa_to_path( ); // Parse range suppression comments - let suppressions = Suppressions::from_tokens(settings, locator.contents(), parsed.tokens()); + let mut suppressions = Suppressions::from_tokens(settings, locator.contents(), parsed.tokens()); // Generate diagnostics, ignoring any existing `noqa` directives. let diagnostics = check_path( @@ -420,7 +420,7 @@ pub fn add_noqa_to_path( source_type, &parsed, target_version, - &suppressions, + &mut suppressions, ); // Add any missing `# noqa` pragmas. @@ -470,7 +470,7 @@ pub fn lint_only( ); // Parse range suppression comments - let suppressions = Suppressions::from_tokens(settings, locator.contents(), parsed.tokens()); + let mut suppressions = Suppressions::from_tokens(settings, locator.contents(), parsed.tokens()); // Generate diagnostics. let diagnostics = check_path( @@ -486,7 +486,7 @@ pub fn lint_only( source_type, &parsed, target_version, - &suppressions, + &mut suppressions, ); LinterResult { @@ -579,7 +579,8 @@ pub fn lint_fix<'a>( ); // Parse range suppression comments - let suppressions = Suppressions::from_tokens(settings, locator.contents(), parsed.tokens()); + let mut suppressions = + Suppressions::from_tokens(settings, locator.contents(), parsed.tokens()); // Generate diagnostics. let diagnostics = check_path( @@ -595,7 +596,7 @@ pub fn lint_fix<'a>( source_type, &parsed, target_version, - &suppressions, + &mut suppressions, ); if iterations == 0 { @@ -961,7 +962,8 @@ mod tests { &locator, &indexer, ); - let suppressions = Suppressions::from_tokens(settings, locator.contents(), parsed.tokens()); + let mut suppressions = + Suppressions::from_tokens(settings, locator.contents(), parsed.tokens()); let mut diagnostics = check_path( path, None, @@ -975,7 +977,7 @@ mod tests { source_type, &parsed, target_version, - &suppressions, + &mut suppressions, ); diagnostics.sort_by(Diagnostic::ruff_start_ordering); diagnostics diff --git a/crates/ruff_linter/src/noqa.rs b/crates/ruff_linter/src/noqa.rs index 0ff6583068277..7775b7f235fcb 100644 --- a/crates/ruff_linter/src/noqa.rs +++ b/crates/ruff_linter/src/noqa.rs @@ -36,7 +36,7 @@ pub fn generate_noqa_edits( external: &[String], noqa_line_for: &NoqaMapping, line_ending: LineEnding, - suppressions: &Suppressions, + suppressions: &mut Suppressions, ) -> Vec> { let file_directives = FileNoqaDirectives::extract(locator, comment_ranges, external, path); let exemption = FileExemption::from(&file_directives); @@ -2988,7 +2988,7 @@ print( let messages = [PrintfStringFormatting .into_diagnostic(TextRange::new(12.into(), 79.into()), &source_file)]; let comment_ranges = CommentRanges::default(); - let suppressions = Suppressions::default(); + let mut suppressions = Suppressions::default(); let edits = generate_noqa_edits( path, &messages, @@ -2997,7 +2997,7 @@ print( &[], &noqa_line_for, LineEnding::Lf, - &suppressions, + &mut suppressions, ); assert_eq!( edits, @@ -3021,7 +3021,7 @@ bar = [UselessSemicolon.into_diagnostic(TextRange::new(4.into(), 5.into()), &source_file)]; let noqa_line_for = NoqaMapping::default(); let comment_ranges = CommentRanges::default(); - let suppressions = Suppressions::default(); + let mut suppressions = Suppressions::default(); let edits = generate_noqa_edits( path, &messages, @@ -3030,7 +3030,7 @@ bar = &[], &noqa_line_for, LineEnding::Lf, - &suppressions, + &mut suppressions, ); assert_eq!( edits, diff --git a/crates/ruff_linter/src/rules/pyflakes/mod.rs b/crates/ruff_linter/src/rules/pyflakes/mod.rs index 02cd5158a8ac4..60ddf10ee2a66 100644 --- a/crates/ruff_linter/src/rules/pyflakes/mod.rs +++ b/crates/ruff_linter/src/rules/pyflakes/mod.rs @@ -956,7 +956,7 @@ mod tests { &locator, &indexer, ); - let suppressions = + let mut suppressions = Suppressions::from_tokens(&settings, locator.contents(), parsed.tokens()); let mut messages = check_path( Path::new(""), @@ -971,7 +971,7 @@ mod tests { source_type, &parsed, target_version, - &suppressions, + &mut suppressions, ); messages.sort_by(Diagnostic::ruff_start_ordering); let actual = messages diff --git a/crates/ruff_linter/src/test.rs b/crates/ruff_linter/src/test.rs index 344c921890105..0eb9a5f416b01 100644 --- a/crates/ruff_linter/src/test.rs +++ b/crates/ruff_linter/src/test.rs @@ -235,7 +235,7 @@ pub(crate) fn test_contents<'a>( &locator, &indexer, ); - let suppressions = Suppressions::from_tokens(settings, locator.contents(), parsed.tokens()); + let mut suppressions = Suppressions::from_tokens(settings, locator.contents(), parsed.tokens()); let messages = check_path( path, path.parent() @@ -251,7 +251,7 @@ pub(crate) fn test_contents<'a>( source_type, &parsed, target_version, - &suppressions, + &mut suppressions, ); let source_has_errors = parsed.has_invalid_syntax(); @@ -302,7 +302,7 @@ pub(crate) fn test_contents<'a>( &indexer, ); - let suppressions = + let mut suppressions = Suppressions::from_tokens(settings, locator.contents(), parsed.tokens()); let fixed_messages = check_path( path, @@ -317,7 +317,7 @@ pub(crate) fn test_contents<'a>( source_type, &parsed, target_version, - &suppressions, + &mut suppressions, ); if parsed.has_invalid_syntax() && !source_has_errors { diff --git a/crates/ruff_server/src/lint.rs b/crates/ruff_server/src/lint.rs index db3f9ce4d85d7..edd566d14fec7 100644 --- a/crates/ruff_server/src/lint.rs +++ b/crates/ruff_server/src/lint.rs @@ -120,7 +120,7 @@ pub(crate) fn check( let directives = extract_directives(parsed.tokens(), Flags::all(), &locator, &indexer); // Parse range suppression comments - let suppressions = + let mut suppressions = Suppressions::from_tokens(&settings.linter, locator.contents(), parsed.tokens()); // Generate checks. @@ -137,7 +137,7 @@ pub(crate) fn check( source_type, &parsed, target_version, - &suppressions, + &mut suppressions, ); let noqa_edits = generate_noqa_edits( @@ -148,7 +148,7 @@ pub(crate) fn check( &settings.linter.external, &directives.noqa_line_for, stylist.line_ending(), - &suppressions, + &mut suppressions, ); let mut diagnostics_map = DiagnosticsMap::default(); diff --git a/crates/ruff_wasm/src/lib.rs b/crates/ruff_wasm/src/lib.rs index 6dd49a15bf44c..264897bca896b 100644 --- a/crates/ruff_wasm/src/lib.rs +++ b/crates/ruff_wasm/src/lib.rs @@ -213,7 +213,7 @@ impl Workspace { &indexer, ); - let suppressions = + let mut suppressions = Suppressions::from_tokens(&self.settings.linter, locator.contents(), parsed.tokens()); // Generate checks. @@ -230,7 +230,7 @@ impl Workspace { source_type, &parsed, target_version, - &suppressions, + &mut suppressions, ); let source_code = locator.to_source_code(); From 7a8adcce2b6475fa2e3841ab994ed9cbdc79efe9 Mon Sep 17 00:00:00 2001 From: Amethyst Reese Date: Thu, 4 Dec 2025 17:25:07 -0800 Subject: [PATCH 05/14] Update schema for new rule --- ruff.schema.json | 1 + 1 file changed, 1 insertion(+) diff --git a/ruff.schema.json b/ruff.schema.json index 1c8a092042a57..c096b8bcedc97 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -4050,6 +4050,7 @@ "RUF100", "RUF101", "RUF102", + "RUF103", "RUF2", "RUF20", "RUF200", From 7fc52f25ad9bde3e4f71c6aaef0dab71af6c6205 Mon Sep 17 00:00:00 2001 From: Amethyst Reese Date: Thu, 4 Dec 2025 18:17:49 -0800 Subject: [PATCH 06/14] Suggested changes, improve target ranges and fixes, handle disabled rules --- crates/ruff_linter/src/suppression.rs | 66 ++++++++++++++++----------- 1 file changed, 39 insertions(+), 27 deletions(-) diff --git a/crates/ruff_linter/src/suppression.rs b/crates/ruff_linter/src/suppression.rs index 0e1bb38b19094..a4215c4b2e945 100644 --- a/crates/ruff_linter/src/suppression.rs +++ b/crates/ruff_linter/src/suppression.rs @@ -164,34 +164,46 @@ impl Suppressions { } pub(crate) fn generate_diagnostics(&self, context: &LintContext, locator: &Locator) { - if context.is_rule_enabled(Rule::UnusedSuppression) { - for suppression in &self.valid { - if !suppression.used { - for comment in &suppression.comments { - 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(); - let code_range = if code_index < (comment.codes.len() - 1) { - TextRange::new( - comment.codes[code_index].start(), - comment.codes[code_index + 1].start(), - ) - } else { - comment.codes[code_index] - }; - Edit::range_deletion(code_range) - }; + if !context.is_rule_enabled(Rule::UnusedSuppression) { + return; + } - let mut diagnostic = context - .report_diagnostic(UnusedSuppression, suppression.comments[0].range); - diagnostic.set_fix(Fix::safe_edit(edit)); - } - } + let unused = self.valid.iter().filter(|suppression| !suppression.used); + + for suppression in unused { + let Ok(rule) = Rule::from_code(&suppression.code) else { + continue; // TODO: invalid code + }; + if !context.is_rule_enabled(rule) { + continue; // don't count disabled rules as unused + } + for comment in &suppression.comments { + let mut range = comment.range; + let edit = if comment.codes.len() == 1 { + delete_comment(comment.range, locator) + } else { + let code_index = comment + .codes + .iter() + .position(|range| locator.slice(range) == suppression.code) + .unwrap(); + range = comment.codes[code_index]; + let code_range = if code_index < (comment.codes.len() - 1) { + TextRange::new( + comment.codes[code_index].start(), + comment.codes[code_index + 1].start(), + ) + } else { + TextRange::new( + comment.codes[code_index - 1].end(), + comment.codes[code_index].end(), + ) + }; + Edit::range_deletion(code_range) + }; + + let mut diagnostic = context.report_diagnostic(UnusedSuppression, range); + diagnostic.set_fix(Fix::safe_edit(edit)); } } } From 3de50d083edb8d7c8d661a81b2b601cf00299383 Mon Sep 17 00:00:00 2001 From: Amethyst Reese Date: Fri, 5 Dec 2025 11:54:38 -0800 Subject: [PATCH 07/14] Use cell for tracking used suppressions --- crates/ruff_linter/src/checkers/noqa.rs | 2 +- crates/ruff_linter/src/linter.rs | 22 ++++++++---------- crates/ruff_linter/src/noqa.rs | 24 ++++++++++---------- crates/ruff_linter/src/rules/pyflakes/mod.rs | 4 ++-- crates/ruff_linter/src/suppression.rs | 20 +++++++++------- crates/ruff_linter/src/test.rs | 8 +++---- crates/ruff_server/src/lint.rs | 6 ++--- crates/ruff_wasm/src/lib.rs | 4 ++-- 8 files changed, 46 insertions(+), 44 deletions(-) diff --git a/crates/ruff_linter/src/checkers/noqa.rs b/crates/ruff_linter/src/checkers/noqa.rs index 53702bcbd475f..afac033f30995 100644 --- a/crates/ruff_linter/src/checkers/noqa.rs +++ b/crates/ruff_linter/src/checkers/noqa.rs @@ -34,7 +34,7 @@ pub(crate) fn check_noqa( noqa_line_for: &NoqaMapping, analyze_directives: bool, settings: &LinterSettings, - suppressions: &mut Suppressions, + suppressions: &Suppressions, ) -> Vec { // Identify any codes that are globally exempted (within the current file). let file_noqa_directives = diff --git a/crates/ruff_linter/src/linter.rs b/crates/ruff_linter/src/linter.rs index fc3aa3907fa21..08c0417020b02 100644 --- a/crates/ruff_linter/src/linter.rs +++ b/crates/ruff_linter/src/linter.rs @@ -129,7 +129,7 @@ pub fn check_path( source_type: PySourceType, parsed: &Parsed, target_version: TargetVersion, - suppressions: &mut Suppressions, + suppressions: &Suppressions, ) -> Vec { // Aggregate all diagnostics. let mut context = LintContext::new(path, locator.contents(), settings); @@ -404,7 +404,7 @@ pub fn add_noqa_to_path( ); // Parse range suppression comments - let mut suppressions = Suppressions::from_tokens(settings, locator.contents(), parsed.tokens()); + let suppressions = Suppressions::from_tokens(settings, locator.contents(), parsed.tokens()); // Generate diagnostics, ignoring any existing `noqa` directives. let diagnostics = check_path( @@ -420,7 +420,7 @@ pub fn add_noqa_to_path( source_type, &parsed, target_version, - &mut suppressions, + &suppressions, ); // Add any missing `# noqa` pragmas. @@ -434,7 +434,7 @@ pub fn add_noqa_to_path( &directives.noqa_line_for, stylist.line_ending(), reason, - &mut suppressions, + &suppressions, ) } @@ -470,7 +470,7 @@ pub fn lint_only( ); // Parse range suppression comments - let mut suppressions = Suppressions::from_tokens(settings, locator.contents(), parsed.tokens()); + let suppressions = Suppressions::from_tokens(settings, locator.contents(), parsed.tokens()); // Generate diagnostics. let diagnostics = check_path( @@ -486,7 +486,7 @@ pub fn lint_only( source_type, &parsed, target_version, - &mut suppressions, + &suppressions, ); LinterResult { @@ -579,8 +579,7 @@ pub fn lint_fix<'a>( ); // Parse range suppression comments - let mut suppressions = - Suppressions::from_tokens(settings, locator.contents(), parsed.tokens()); + let suppressions = Suppressions::from_tokens(settings, locator.contents(), parsed.tokens()); // Generate diagnostics. let diagnostics = check_path( @@ -596,7 +595,7 @@ pub fn lint_fix<'a>( source_type, &parsed, target_version, - &mut suppressions, + &suppressions, ); if iterations == 0 { @@ -962,8 +961,7 @@ mod tests { &locator, &indexer, ); - let mut suppressions = - Suppressions::from_tokens(settings, locator.contents(), parsed.tokens()); + let suppressions = Suppressions::from_tokens(settings, locator.contents(), parsed.tokens()); let mut diagnostics = check_path( path, None, @@ -977,7 +975,7 @@ mod tests { source_type, &parsed, target_version, - &mut suppressions, + &suppressions, ); diagnostics.sort_by(Diagnostic::ruff_start_ordering); diagnostics diff --git a/crates/ruff_linter/src/noqa.rs b/crates/ruff_linter/src/noqa.rs index 7775b7f235fcb..a3b5b6133d533 100644 --- a/crates/ruff_linter/src/noqa.rs +++ b/crates/ruff_linter/src/noqa.rs @@ -36,7 +36,7 @@ pub fn generate_noqa_edits( external: &[String], noqa_line_for: &NoqaMapping, line_ending: LineEnding, - suppressions: &mut Suppressions, + suppressions: &Suppressions, ) -> Vec> { let file_directives = FileNoqaDirectives::extract(locator, comment_ranges, external, path); let exemption = FileExemption::from(&file_directives); @@ -735,7 +735,7 @@ pub(crate) fn add_noqa( noqa_line_for: &NoqaMapping, line_ending: LineEnding, reason: Option<&str>, - suppressions: &mut Suppressions, + suppressions: &Suppressions, ) -> Result { let (count, output) = add_noqa_inner( path, @@ -763,7 +763,7 @@ fn add_noqa_inner( noqa_line_for: &NoqaMapping, line_ending: LineEnding, reason: Option<&str>, - suppressions: &mut Suppressions, + suppressions: &Suppressions, ) -> (usize, String) { let mut count = 0; @@ -879,7 +879,7 @@ fn find_noqa_comments<'a>( exemption: &'a FileExemption, directives: &'a NoqaDirectives, noqa_line_for: &NoqaMapping, - suppressions: &'a mut Suppressions, + suppressions: &'a Suppressions, ) -> Vec>> { // List of noqa comments, ordered to match up with `messages` let mut comments_by_line: Vec>> = vec![]; @@ -2876,7 +2876,7 @@ mod tests { &noqa_line_for, LineEnding::Lf, None, - &mut Suppressions::default(), + &Suppressions::default(), ); assert_eq!(count, 0); assert_eq!(output, format!("{contents}")); @@ -2901,7 +2901,7 @@ mod tests { &noqa_line_for, LineEnding::Lf, None, - &mut Suppressions::default(), + &Suppressions::default(), ); assert_eq!(count, 1); assert_eq!(output, "x = 1 # noqa: F841\n"); @@ -2933,7 +2933,7 @@ mod tests { &noqa_line_for, LineEnding::Lf, None, - &mut Suppressions::default(), + &Suppressions::default(), ); assert_eq!(count, 1); assert_eq!(output, "x = 1 # noqa: E741, F841\n"); @@ -2965,7 +2965,7 @@ mod tests { &noqa_line_for, LineEnding::Lf, None, - &mut Suppressions::default(), + &Suppressions::default(), ); assert_eq!(count, 0); assert_eq!(output, "x = 1 # noqa"); @@ -2988,7 +2988,7 @@ print( let messages = [PrintfStringFormatting .into_diagnostic(TextRange::new(12.into(), 79.into()), &source_file)]; let comment_ranges = CommentRanges::default(); - let mut suppressions = Suppressions::default(); + let suppressions = Suppressions::default(); let edits = generate_noqa_edits( path, &messages, @@ -2997,7 +2997,7 @@ print( &[], &noqa_line_for, LineEnding::Lf, - &mut suppressions, + &suppressions, ); assert_eq!( edits, @@ -3021,7 +3021,7 @@ bar = [UselessSemicolon.into_diagnostic(TextRange::new(4.into(), 5.into()), &source_file)]; let noqa_line_for = NoqaMapping::default(); let comment_ranges = CommentRanges::default(); - let mut suppressions = Suppressions::default(); + let suppressions = Suppressions::default(); let edits = generate_noqa_edits( path, &messages, @@ -3030,7 +3030,7 @@ bar = &[], &noqa_line_for, LineEnding::Lf, - &mut suppressions, + &suppressions, ); assert_eq!( edits, diff --git a/crates/ruff_linter/src/rules/pyflakes/mod.rs b/crates/ruff_linter/src/rules/pyflakes/mod.rs index 60ddf10ee2a66..02cd5158a8ac4 100644 --- a/crates/ruff_linter/src/rules/pyflakes/mod.rs +++ b/crates/ruff_linter/src/rules/pyflakes/mod.rs @@ -956,7 +956,7 @@ mod tests { &locator, &indexer, ); - let mut suppressions = + let suppressions = Suppressions::from_tokens(&settings, locator.contents(), parsed.tokens()); let mut messages = check_path( Path::new(""), @@ -971,7 +971,7 @@ mod tests { source_type, &parsed, target_version, - &mut suppressions, + &suppressions, ); messages.sort_by(Diagnostic::ruff_start_ordering); let actual = messages diff --git a/crates/ruff_linter/src/suppression.rs b/crates/ruff_linter/src/suppression.rs index a4215c4b2e945..f44874835d3f2 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 std::cell::Cell; use std::{error::Error, fmt::Formatter}; use thiserror::Error; @@ -75,7 +76,7 @@ impl PendingSuppressionComment<'_> { } #[allow(unused)] -#[derive(Clone, Debug)] +#[derive(Debug)] pub(crate) struct Suppression { /// The lint code being suppressed code: CompactString, @@ -87,7 +88,7 @@ pub(crate) struct Suppression { comments: SmallVec<[SuppressionComment; 2]>, /// Whether this suppression actually suppressed a diagnostic - used: bool, + used: Cell, } #[allow(unused)] @@ -139,7 +140,7 @@ impl Suppressions { } /// Check if a diagnostic is suppressed by any known range suppressions - pub(crate) fn check_diagnostic(&mut self, diagnostic: &Diagnostic) -> bool { + pub(crate) fn check_diagnostic(&self, diagnostic: &Diagnostic) -> bool { if self.valid.is_empty() { return false; } @@ -154,9 +155,9 @@ impl Suppressions { return false; }; - for suppression in &mut self.valid { + for suppression in &self.valid { if *code == suppression.code.as_str() && suppression.range.contains_range(range) { - suppression.used = true; + suppression.used.set(true); return true; } } @@ -168,7 +169,10 @@ impl Suppressions { return; } - let unused = self.valid.iter().filter(|suppression| !suppression.used); + let unused = self + .valid + .iter() + .filter(|suppression| !suppression.used.get()); for suppression in unused { let Ok(rule) = Rule::from_code(&suppression.code) else { @@ -331,7 +335,7 @@ impl<'a> SuppressionsBuilder<'a> { code: code.into(), range: combined_range, comments: smallvec![comment.comment.clone(), other.comment.clone()], - used: false, + used: false.into(), }); } @@ -348,7 +352,7 @@ impl<'a> SuppressionsBuilder<'a> { code: code.into(), range: implicit_range, comments: smallvec![comment.comment.clone()], - used: false, + used: false.into(), }); } self.pending.remove(comment_index); diff --git a/crates/ruff_linter/src/test.rs b/crates/ruff_linter/src/test.rs index 0eb9a5f416b01..344c921890105 100644 --- a/crates/ruff_linter/src/test.rs +++ b/crates/ruff_linter/src/test.rs @@ -235,7 +235,7 @@ pub(crate) fn test_contents<'a>( &locator, &indexer, ); - let mut suppressions = Suppressions::from_tokens(settings, locator.contents(), parsed.tokens()); + let suppressions = Suppressions::from_tokens(settings, locator.contents(), parsed.tokens()); let messages = check_path( path, path.parent() @@ -251,7 +251,7 @@ pub(crate) fn test_contents<'a>( source_type, &parsed, target_version, - &mut suppressions, + &suppressions, ); let source_has_errors = parsed.has_invalid_syntax(); @@ -302,7 +302,7 @@ pub(crate) fn test_contents<'a>( &indexer, ); - let mut suppressions = + let suppressions = Suppressions::from_tokens(settings, locator.contents(), parsed.tokens()); let fixed_messages = check_path( path, @@ -317,7 +317,7 @@ pub(crate) fn test_contents<'a>( source_type, &parsed, target_version, - &mut suppressions, + &suppressions, ); if parsed.has_invalid_syntax() && !source_has_errors { diff --git a/crates/ruff_server/src/lint.rs b/crates/ruff_server/src/lint.rs index edd566d14fec7..db3f9ce4d85d7 100644 --- a/crates/ruff_server/src/lint.rs +++ b/crates/ruff_server/src/lint.rs @@ -120,7 +120,7 @@ pub(crate) fn check( let directives = extract_directives(parsed.tokens(), Flags::all(), &locator, &indexer); // Parse range suppression comments - let mut suppressions = + let suppressions = Suppressions::from_tokens(&settings.linter, locator.contents(), parsed.tokens()); // Generate checks. @@ -137,7 +137,7 @@ pub(crate) fn check( source_type, &parsed, target_version, - &mut suppressions, + &suppressions, ); let noqa_edits = generate_noqa_edits( @@ -148,7 +148,7 @@ pub(crate) fn check( &settings.linter.external, &directives.noqa_line_for, stylist.line_ending(), - &mut suppressions, + &suppressions, ); let mut diagnostics_map = DiagnosticsMap::default(); diff --git a/crates/ruff_wasm/src/lib.rs b/crates/ruff_wasm/src/lib.rs index 264897bca896b..6dd49a15bf44c 100644 --- a/crates/ruff_wasm/src/lib.rs +++ b/crates/ruff_wasm/src/lib.rs @@ -213,7 +213,7 @@ impl Workspace { &indexer, ); - let mut suppressions = + let suppressions = Suppressions::from_tokens(&self.settings.linter, locator.contents(), parsed.tokens()); // Generate checks. @@ -230,7 +230,7 @@ impl Workspace { source_type, &parsed, target_version, - &mut suppressions, + &suppressions, ); let source_code = locator.to_source_code(); From 226eee2da0b51a8ad9c7e728d16babb5394ecbe2 Mon Sep 17 00:00:00 2001 From: Amethyst Reese Date: Fri, 5 Dec 2025 12:08:37 -0800 Subject: [PATCH 08/14] rename function --- crates/ruff_linter/src/checkers/noqa.rs | 2 +- crates/ruff_linter/src/suppression.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/ruff_linter/src/checkers/noqa.rs b/crates/ruff_linter/src/checkers/noqa.rs index afac033f30995..f87664b512a19 100644 --- a/crates/ruff_linter/src/checkers/noqa.rs +++ b/crates/ruff_linter/src/checkers/noqa.rs @@ -120,7 +120,7 @@ pub(crate) fn check_noqa( } // Diagnostics for unused/invalid range suppressions - suppressions.generate_diagnostics(context, locator); + suppressions.check_suppressions(context, locator); // Enforce that the noqa directive was actually used (RUF100), unless RUF100 was itself // suppressed. diff --git a/crates/ruff_linter/src/suppression.rs b/crates/ruff_linter/src/suppression.rs index f44874835d3f2..82602ea04d414 100644 --- a/crates/ruff_linter/src/suppression.rs +++ b/crates/ruff_linter/src/suppression.rs @@ -164,7 +164,7 @@ impl Suppressions { false } - pub(crate) fn generate_diagnostics(&self, context: &LintContext, locator: &Locator) { + pub(crate) fn check_suppressions(&self, context: &LintContext, locator: &Locator) { if !context.is_rule_enabled(Rule::UnusedSuppression) { return; } From 90ea030f8c8d423add3ccc811720d93e9feb657f Mon Sep 17 00:00:00 2001 From: Amethyst Reese Date: Fri, 5 Dec 2025 14:47:35 -0800 Subject: [PATCH 09/14] Resue existing UnusedNOQA rule --- crates/ruff_linter/src/checkers/noqa.rs | 10 +++- crates/ruff_linter/src/codes.rs | 1 - .../ruff_linter/src/rules/ruff/rules/mod.rs | 2 - .../src/rules/ruff/rules/unused_noqa.rs | 30 ++++++++++-- .../rules/ruff/rules/unused_suppression.rs | 48 ------------------- crates/ruff_linter/src/suppression.rs | 15 ++++-- ruff.schema.json | 1 - 7 files changed, 45 insertions(+), 62 deletions(-) delete mode 100644 crates/ruff_linter/src/rules/ruff/rules/unused_suppression.rs diff --git a/crates/ruff_linter/src/checkers/noqa.rs b/crates/ruff_linter/src/checkers/noqa.rs index f87664b512a19..e5da04d93f6b8 100644 --- a/crates/ruff_linter/src/checkers/noqa.rs +++ b/crates/ruff_linter/src/checkers/noqa.rs @@ -143,8 +143,13 @@ pub(crate) fn check_noqa( Directive::All(directive) => { if matches.is_empty() { let edit = delete_comment(directive.range(), locator); - let mut diagnostic = context - .report_diagnostic(UnusedNOQA { codes: None }, directive.range()); + let mut diagnostic = context.report_diagnostic( + UnusedNOQA { + codes: None, + kind: ruff::rules::UnusedNOQAKind::NOQA, + }, + directive.range(), + ); diagnostic.add_primary_tag(ruff_db::diagnostic::DiagnosticTag::Unnecessary); diagnostic.set_fix(Fix::safe_edit(edit)); } @@ -239,6 +244,7 @@ pub(crate) fn check_noqa( .map(|code| (*code).to_string()) .collect(), }), + kind: ruff::rules::UnusedNOQAKind::NOQA, }, directive.range(), ); diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index 4dfab63e08608..ebec5f4acc09d 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -1063,7 +1063,6 @@ 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::UnusedSuppression, (Ruff, "200") => rules::ruff::rules::InvalidPyprojectToml, #[cfg(any(feature = "test-rules", test))] diff --git a/crates/ruff_linter/src/rules/ruff/rules/mod.rs b/crates/ruff_linter/src/rules/ruff/rules/mod.rs index 6cd4cbb51838e..206a504e740b4 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/mod.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/mod.rs @@ -57,7 +57,6 @@ pub(crate) use unraw_re_pattern::*; pub(crate) use unsafe_markup_use::*; pub(crate) use unused_async::*; pub(crate) use unused_noqa::*; -pub(crate) use unused_suppression::*; pub(crate) use unused_unpacked_variable::*; pub(crate) use used_dummy_variable::*; pub(crate) use useless_if_else::*; @@ -125,7 +124,6 @@ mod unraw_re_pattern; mod unsafe_markup_use; mod unused_async; mod unused_noqa; -mod unused_suppression; mod unused_unpacked_variable; mod used_dummy_variable; mod useless_if_else; diff --git a/crates/ruff_linter/src/rules/ruff/rules/unused_noqa.rs b/crates/ruff_linter/src/rules/ruff/rules/unused_noqa.rs index e4645a5541a78..a97beaf8499a3 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/unused_noqa.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/unused_noqa.rs @@ -4,7 +4,7 @@ use ruff_macros::{ViolationMetadata, derive_message_formats}; use crate::AlwaysFixableViolation; -#[derive(Debug, PartialEq, Eq)] +#[derive(Debug, PartialEq, Eq, Default)] pub(crate) struct UnusedCodes { pub disabled: Vec, pub duplicated: Vec, @@ -12,6 +12,21 @@ pub(crate) struct UnusedCodes { pub unmatched: Vec, } +#[derive(Debug, PartialEq, Eq)] +pub(crate) enum UnusedNOQAKind { + NOQA, + Suppression, +} + +impl UnusedNOQAKind { + fn as_str(&self) -> &str { + match self { + UnusedNOQAKind::NOQA => "`noqa` directive", + UnusedNOQAKind::Suppression => "suppression", + } + } +} + /// ## What it does /// Checks for `noqa` directives that are no longer applicable. /// @@ -46,6 +61,7 @@ pub(crate) struct UnusedCodes { #[violation_metadata(stable_since = "v0.0.155")] pub(crate) struct UnusedNOQA { pub codes: Option, + pub kind: UnusedNOQAKind, } impl AlwaysFixableViolation for UnusedNOQA { @@ -95,16 +111,20 @@ impl AlwaysFixableViolation for UnusedNOQA { )); } if codes_by_reason.is_empty() { - "Unused `noqa` directive".to_string() + format!("Unused {}", self.kind.as_str()) } else { - format!("Unused `noqa` directive ({})", codes_by_reason.join("; ")) + format!( + "Unused {} ({})", + self.kind.as_str(), + codes_by_reason.join("; ") + ) } } - None => "Unused blanket `noqa` directive".to_string(), + None => format!("Unused blanket {}", self.kind.as_str()), } } fn fix_title(&self) -> String { - "Remove unused `noqa` directive".to_string() + format!("Remove unused {}", self.kind.as_str()) } } diff --git a/crates/ruff_linter/src/rules/ruff/rules/unused_suppression.rs b/crates/ruff_linter/src/rules/ruff/rules/unused_suppression.rs deleted file mode 100644 index 85d38aa707153..0000000000000 --- a/crates/ruff_linter/src/rules/ruff/rules/unused_suppression.rs +++ /dev/null @@ -1,48 +0,0 @@ -use crate::AlwaysFixableViolation; -use ruff_macros::{ViolationMetadata, derive_message_formats}; - -/// ## What it does -/// Checks for range suppressions that are no longer used. -/// -/// ## Why is this bad? -/// A range suppression that no longer matches any diagnostic violations is -/// likely included by mistake, and should be removed to avoid confusion. -/// -/// ## Example -/// ```python -/// # ruff: disable[F401] -/// import foo -/// -/// -/// def bar(): -/// foo.bar() -/// ``` -/// -/// Use instead: -/// ```python -/// import foo -/// -/// -/// def bar(): -/// foo.bar() -/// ``` -/// -/// ## Options -/// - `lint.external` -/// -/// ## References -/// - [Ruff error suppression](https://docs.astral.sh/ruff/linter/#error-suppression) -#[derive(ViolationMetadata)] -#[violation_metadata(preview_since = "0.14.9")] -pub(crate) struct UnusedSuppression; - -impl AlwaysFixableViolation for UnusedSuppression { - #[derive_message_formats] - fn message(&self) -> String { - "Unused suppression".to_string() - } - - fn fix_title(&self) -> String { - "Remove unused suppression".to_string() - } -} diff --git a/crates/ruff_linter/src/suppression.rs b/crates/ruff_linter/src/suppression.rs index 82602ea04d414..04d56c87dda10 100644 --- a/crates/ruff_linter/src/suppression.rs +++ b/crates/ruff_linter/src/suppression.rs @@ -17,7 +17,7 @@ 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::UnusedSuppression; +use crate::rules::ruff::rules::{UnusedCodes, UnusedNOQA}; use crate::settings::LinterSettings; #[allow(unused)] @@ -165,7 +165,7 @@ impl Suppressions { } pub(crate) fn check_suppressions(&self, context: &LintContext, locator: &Locator) { - if !context.is_rule_enabled(Rule::UnusedSuppression) { + if !context.is_rule_enabled(Rule::UnusedNOQA) { return; } @@ -206,7 +206,16 @@ impl Suppressions { Edit::range_deletion(code_range) }; - let mut diagnostic = context.report_diagnostic(UnusedSuppression, range); + let mut diagnostic = context.report_diagnostic( + UnusedNOQA { + codes: Some(UnusedCodes { + unmatched: vec![suppression.code.to_string()], + ..UnusedCodes::default() + }), + kind: crate::rules::ruff::rules::UnusedNOQAKind::Suppression, + }, + range, + ); diagnostic.set_fix(Fix::safe_edit(edit)); } } diff --git a/ruff.schema.json b/ruff.schema.json index c096b8bcedc97..1c8a092042a57 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -4050,7 +4050,6 @@ "RUF100", "RUF101", "RUF102", - "RUF103", "RUF2", "RUF20", "RUF200", From d208d6863c0822f3968b7cbda5880c365165df8f Mon Sep 17 00:00:00 2001 From: Amethyst Reese Date: Fri, 5 Dec 2025 17:03:43 -0800 Subject: [PATCH 10/14] Report disabled codes, as well as comments with empty code lists --- ...ules__ruff__tests__range_suppressions.snap | 40 ++++++++++++++++++- crates/ruff_linter/src/suppression.rs | 36 ++++++++++++++--- 2 files changed, 69 insertions(+), 7 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 4e095074822bb..ad3ac67e911a5 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: 9 -Added: 1 +Added: 3 --- Removed --- E741 Ambiguous variable name: `I` @@ -150,6 +150,44 @@ note: This is an unsafe fix and may change runtime behavior --- Added --- +RUF100 [*] Unused suppression (unused: `E501`) + --> 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] + | +help: Remove unused suppression +43 | def f(): +44 | # Neither of these are ignored and warnings are +45 | # logged to user + - # ruff: disable[E501] +46 | I = 1 +47 | # ruff: enable[E501] +48 | + + +RUF100 [*] Unused suppression (unused: `E501`) + --> suppressions.py:48:5 + | +46 | # ruff: disable[E501] +47 | I = 1 +48 | # ruff: enable[E501] + | ^^^^^^^^^^^^^^^^^^^^ + | +help: Remove unused suppression +45 | # logged to user +46 | # ruff: disable[E501] +47 | I = 1 + - # ruff: enable[E501] +48 | +49 | +50 | def f(): + + RUF100 [*] Unused `noqa` directive (unused: `E741`, `F841`) --> suppressions.py:55:12 | diff --git a/crates/ruff_linter/src/suppression.rs b/crates/ruff_linter/src/suppression.rs index 04d56c87dda10..59c97a9b6dd7b 100644 --- a/crates/ruff_linter/src/suppression.rs +++ b/crates/ruff_linter/src/suppression.rs @@ -17,7 +17,7 @@ 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}; +use crate::rules::ruff::rules::{UnusedCodes, UnusedNOQA, UnusedNOQAKind}; use crate::settings::LinterSettings; #[allow(unused)] @@ -165,7 +165,7 @@ impl Suppressions { } pub(crate) fn check_suppressions(&self, context: &LintContext, locator: &Locator) { - if !context.is_rule_enabled(Rule::UnusedNOQA) { + if !context.any_rule_enabled(&[Rule::UnusedNOQA, Rule::InvalidRuleCode]) { return; } @@ -178,9 +178,6 @@ impl Suppressions { let Ok(rule) = Rule::from_code(&suppression.code) else { continue; // TODO: invalid code }; - if !context.is_rule_enabled(rule) { - continue; // don't count disabled rules as unused - } for comment in &suppression.comments { let mut range = comment.range; let edit = if comment.codes.len() == 1 { @@ -206,19 +203,46 @@ impl Suppressions { 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(UnusedCodes { unmatched: vec![suppression.code.to_string()], ..UnusedCodes::default() }), - kind: crate::rules::ruff::rules::UnusedNOQAKind::Suppression, + kind: UnusedNOQAKind::Suppression, }, range, ); diagnostic.set_fix(Fix::safe_edit(edit)); } } + + 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))); + } } } From c12f4a0916a10787d406e6da35474c1bb83f4dc3 Mon Sep 17 00:00:00 2001 From: Amethyst Reese Date: Mon, 8 Dec 2025 16:19:37 -0800 Subject: [PATCH 11/14] Remove unused lint allows --- crates/ruff_linter/src/suppression.rs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/crates/ruff_linter/src/suppression.rs b/crates/ruff_linter/src/suppression.rs index 59c97a9b6dd7b..04c708a5896e5 100644 --- a/crates/ruff_linter/src/suppression.rs +++ b/crates/ruff_linter/src/suppression.rs @@ -20,7 +20,6 @@ use crate::preview::is_range_suppressions_enabled; use crate::rules::ruff::rules::{UnusedCodes, UnusedNOQA, UnusedNOQAKind}; use crate::settings::LinterSettings; -#[allow(unused)] #[derive(Clone, Debug, Eq, PartialEq)] enum SuppressionAction { Disable, @@ -42,7 +41,6 @@ pub(crate) struct SuppressionComment { reason: TextRange, } -#[allow(unused)] impl SuppressionComment { /// Return the suppressed codes as strings fn codes_as_str<'src>(&self, source: &'src str) -> impl Iterator { @@ -59,7 +57,6 @@ pub(crate) struct PendingSuppressionComment<'a> { comment: SuppressionComment, } -#[allow(unused)] impl PendingSuppressionComment<'_> { /// Whether the comment "matches" another comment, based on indentation and suppressed codes /// Expects a "forward search" for matches, ie, will only match if the current comment is a @@ -75,7 +72,6 @@ impl PendingSuppressionComment<'_> { } } -#[allow(unused)] #[derive(Debug)] pub(crate) struct Suppression { /// The lint code being suppressed @@ -91,7 +87,6 @@ pub(crate) struct Suppression { used: Cell, } -#[allow(unused)] #[derive(Copy, Clone, Debug)] pub(crate) enum InvalidSuppressionKind { /// Trailing suppression not supported @@ -124,7 +119,6 @@ pub struct Suppressions { errors: Vec, } -#[allow(unused)] impl Suppressions { pub fn from_tokens(settings: &LinterSettings, source: &str, tokens: &Tokens) -> Suppressions { if is_range_suppressions_enabled(settings) { From 7c88b49619937a16f264b8876dbba7d01a7c6672 Mon Sep 17 00:00:00 2001 From: Amethyst Reese Date: Mon, 8 Dec 2025 16:50:27 -0800 Subject: [PATCH 12/14] Use correct unusedcodes object --- .../ruff_linter__rules__ruff__tests__range_suppressions.snap | 4 ++-- crates/ruff_linter/src/suppression.rs | 5 +---- 2 files changed, 3 insertions(+), 6 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 ad3ac67e911a5..34a6b77b8f04e 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 @@ -150,7 +150,7 @@ note: This is an unsafe fix and may change runtime behavior --- Added --- -RUF100 [*] Unused suppression (unused: `E501`) +RUF100 [*] Unused suppression (non-enabled: `E501`) --> suppressions.py:46:5 | 44 | # Neither of these are ignored and warnings are @@ -170,7 +170,7 @@ help: Remove unused suppression 48 | -RUF100 [*] Unused suppression (unused: `E501`) +RUF100 [*] Unused suppression (non-enabled: `E501`) --> suppressions.py:48:5 | 46 | # ruff: disable[E501] diff --git a/crates/ruff_linter/src/suppression.rs b/crates/ruff_linter/src/suppression.rs index 04c708a5896e5..9eb12d1026206 100644 --- a/crates/ruff_linter/src/suppression.rs +++ b/crates/ruff_linter/src/suppression.rs @@ -211,10 +211,7 @@ impl Suppressions { let mut diagnostic = context.report_diagnostic( UnusedNOQA { - codes: Some(UnusedCodes { - unmatched: vec![suppression.code.to_string()], - ..UnusedCodes::default() - }), + codes: Some(codes), kind: UnusedNOQAKind::Suppression, }, range, From 2f9042c3ab2cbb73f55631a51f7c98b7e5c191ce Mon Sep 17 00:00:00 2001 From: Amethyst Reese Date: Mon, 8 Dec 2025 16:52:05 -0800 Subject: [PATCH 13/14] NOQA -> Noqa to make lint happy --- crates/ruff_linter/src/checkers/noqa.rs | 4 ++-- crates/ruff_linter/src/rules/ruff/rules/unused_noqa.rs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/ruff_linter/src/checkers/noqa.rs b/crates/ruff_linter/src/checkers/noqa.rs index e5da04d93f6b8..f984ef35767ef 100644 --- a/crates/ruff_linter/src/checkers/noqa.rs +++ b/crates/ruff_linter/src/checkers/noqa.rs @@ -146,7 +146,7 @@ pub(crate) fn check_noqa( let mut diagnostic = context.report_diagnostic( UnusedNOQA { codes: None, - kind: ruff::rules::UnusedNOQAKind::NOQA, + kind: ruff::rules::UnusedNOQAKind::Noqa, }, directive.range(), ); @@ -244,7 +244,7 @@ pub(crate) fn check_noqa( .map(|code| (*code).to_string()) .collect(), }), - kind: ruff::rules::UnusedNOQAKind::NOQA, + kind: ruff::rules::UnusedNOQAKind::Noqa, }, directive.range(), ); diff --git a/crates/ruff_linter/src/rules/ruff/rules/unused_noqa.rs b/crates/ruff_linter/src/rules/ruff/rules/unused_noqa.rs index a97beaf8499a3..d6e4ce94d9edd 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/unused_noqa.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/unused_noqa.rs @@ -14,14 +14,14 @@ pub(crate) struct UnusedCodes { #[derive(Debug, PartialEq, Eq)] pub(crate) enum UnusedNOQAKind { - NOQA, + Noqa, Suppression, } impl UnusedNOQAKind { fn as_str(&self) -> &str { match self { - UnusedNOQAKind::NOQA => "`noqa` directive", + UnusedNOQAKind::Noqa => "`noqa` directive", UnusedNOQAKind::Suppression => "suppression", } } From 2f8d4a454935f92bc64f76883b1ad6fa701fdc99 Mon Sep 17 00:00:00 2001 From: Amethyst Reese Date: Mon, 8 Dec 2025 18:12:45 -0800 Subject: [PATCH 14/14] More test cases covering unused suppressions --- .../test/fixtures/ruff/suppressions.py | 32 +++ ...ules__ruff__tests__range_suppressions.snap | 249 +++++++++++++++++- 2 files changed, 279 insertions(+), 2 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/ruff/suppressions.py b/crates/ruff_linter/resources/test/fixtures/ruff/suppressions.py index 7a70c4d548b0c..f8a3c882aab8e 100644 --- a/crates/ruff_linter/resources/test/fixtures/ruff/suppressions.py +++ b/crates/ruff_linter/resources/test/fixtures/ruff/suppressions.py @@ -54,3 +54,35 @@ def f(): # ruff:disable[E741,F841] I = 1 # noqa: E741,F841 # ruff:enable[E741,F841] + + +def f(): + # TODO: Duplicate codes should be counted as duplicate, not unused + # ruff: disable[F841, F841] + foo = 0 + + +def f(): + # Overlapping range suppressions, one should be marked as used, + # and the other should trigger an unused suppression diagnostic + # ruff: disable[F841] + # ruff: disable[F841] + foo = 0 + + +def f(): + # Multiple codes but only one is used + # ruff: disable[E741, F401, F841] + foo = 0 + + +def f(): + # Multiple codes but only two are used + # ruff: disable[E741, F401, F841] + I = 0 + + +def f(): + # Multiple codes but none are used + # ruff: disable[E741, F401, F841] + print("hello") 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 34a6b77b8f04e..24a43ade5ef1a 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: 9 -Added: 3 +Removed: 14 +Added: 11 --- Removed --- E741 Ambiguous variable name: `I` @@ -148,6 +148,96 @@ help: Remove assignment to unused variable `I` note: This is an unsafe fix and may change runtime behavior +F841 [*] Local variable `foo` is assigned to but never used + --> suppressions.py:62:5 + | +60 | # TODO: Duplicate codes should be counted as duplicate, not unused +61 | # ruff: disable[F841, F841] +62 | foo = 0 + | ^^^ + | +help: Remove assignment to unused variable `foo` +59 | def f(): +60 | # TODO: Duplicate codes should be counted as duplicate, not unused +61 | # ruff: disable[F841, F841] + - foo = 0 +62 + pass +63 | +64 | +65 | def f(): +note: This is an unsafe fix and may change runtime behavior + + +F841 [*] Local variable `foo` is assigned to but never used + --> suppressions.py:70:5 + | +68 | # ruff: disable[F841] +69 | # ruff: disable[F841] +70 | foo = 0 + | ^^^ + | +help: Remove assignment to unused variable `foo` +67 | # and the other should trigger an unused suppression diagnostic +68 | # ruff: disable[F841] +69 | # ruff: disable[F841] + - foo = 0 +70 + pass +71 | +72 | +73 | def f(): +note: This is an unsafe fix and may change runtime behavior + + +F841 [*] Local variable `foo` is assigned to but never used + --> suppressions.py:76:5 + | +74 | # Multiple codes but only one is used +75 | # ruff: disable[E741, F401, F841] +76 | foo = 0 + | ^^^ + | +help: Remove assignment to unused variable `foo` +73 | def f(): +74 | # Multiple codes but only one is used +75 | # ruff: disable[E741, F401, F841] + - foo = 0 +76 + pass +77 | +78 | +79 | def f(): +note: This is an unsafe fix and may change runtime behavior + + +E741 Ambiguous variable name: `I` + --> suppressions.py:82:5 + | +80 | # Multiple codes but only two are used +81 | # ruff: disable[E741, F401, F841] +82 | I = 0 + | ^ + | + + +F841 [*] Local variable `I` is assigned to but never used + --> suppressions.py:82:5 + | +80 | # Multiple codes but only two are used +81 | # ruff: disable[E741, F401, F841] +82 | I = 0 + | ^ + | +help: Remove assignment to unused variable `I` +79 | def f(): +80 | # Multiple codes but only two are used +81 | # ruff: disable[E741, F401, F841] + - I = 0 +82 + pass +83 | +84 | +85 | def f(): +note: This is an unsafe fix and may change runtime behavior + + --- Added --- RUF100 [*] Unused suppression (non-enabled: `E501`) @@ -204,3 +294,158 @@ help: Remove unused `noqa` directive - I = 1 # noqa: E741,F841 55 + I = 1 56 | # ruff:enable[E741,F841] +57 | +58 | + + +RUF100 [*] Unused suppression (unused: `F841`) + --> suppressions.py:61:21 + | +59 | def f(): +60 | # TODO: Duplicate codes should be counted as duplicate, not unused +61 | # ruff: disable[F841, F841] + | ^^^^ +62 | foo = 0 + | +help: Remove unused suppression +58 | +59 | def f(): +60 | # TODO: Duplicate codes should be counted as duplicate, not unused + - # ruff: disable[F841, F841] +61 + # ruff: disable[F841] +62 | foo = 0 +63 | +64 | + + +RUF100 [*] Unused suppression (unused: `F841`) + --> 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 +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 | + + +RUF100 [*] Unused suppression (unused: `E741`) + --> suppressions.py:75:21 + | +73 | def f(): +74 | # Multiple codes but only one is used +75 | # ruff: disable[E741, F401, F841] + | ^^^^ +76 | foo = 0 + | +help: Remove unused suppression +72 | +73 | def f(): +74 | # Multiple codes but only one is used + - # ruff: disable[E741, F401, F841] +75 + # ruff: disable[F401, F841] +76 | foo = 0 +77 | +78 | + + +RUF100 [*] Unused suppression (non-enabled: `F401`) + --> suppressions.py:75:27 + | +73 | def f(): +74 | # Multiple codes but only one is used +75 | # ruff: disable[E741, F401, F841] + | ^^^^ +76 | foo = 0 + | +help: Remove unused suppression +72 | +73 | def f(): +74 | # Multiple codes but only one is used + - # ruff: disable[E741, F401, F841] +75 + # ruff: disable[E741, F841] +76 | foo = 0 +77 | +78 | + + +RUF100 [*] Unused suppression (non-enabled: `F401`) + --> suppressions.py:81:27 + | +79 | def f(): +80 | # Multiple codes but only two are used +81 | # ruff: disable[E741, F401, F841] + | ^^^^ +82 | I = 0 + | +help: Remove unused suppression +78 | +79 | def f(): +80 | # Multiple codes but only two are used + - # ruff: disable[E741, F401, F841] +81 + # ruff: disable[E741, F841] +82 | I = 0 +83 | +84 | + + +RUF100 [*] Unused suppression (unused: `E741`) + --> suppressions.py:87:21 + | +85 | def f(): +86 | # Multiple codes but none are used +87 | # ruff: disable[E741, F401, F841] + | ^^^^ +88 | print("hello") + | +help: Remove unused suppression +84 | +85 | def f(): +86 | # Multiple codes but none are used + - # ruff: disable[E741, F401, F841] +87 + # ruff: disable[F401, F841] +88 | print("hello") + + +RUF100 [*] Unused suppression (non-enabled: `F401`) + --> suppressions.py:87:27 + | +85 | def f(): +86 | # Multiple codes but none are used +87 | # ruff: disable[E741, F401, F841] + | ^^^^ +88 | print("hello") + | +help: Remove unused suppression +84 | +85 | def f(): +86 | # Multiple codes but none are used + - # ruff: disable[E741, F401, F841] +87 + # ruff: disable[E741, F841] +88 | print("hello") + + +RUF100 [*] Unused suppression (unused: `F841`) + --> suppressions.py:87:33 + | +85 | def f(): +86 | # Multiple codes but none are used +87 | # ruff: disable[E741, F401, F841] + | ^^^^ +88 | print("hello") + | +help: Remove unused suppression +84 | +85 | def f(): +86 | # Multiple codes but none are used + - # ruff: disable[E741, F401, F841] +87 + # ruff: disable[E741, F401] +88 | print("hello")