diff --git a/crates/ruff/tests/cli/lint.rs b/crates/ruff/tests/cli/lint.rs index 8e4b23aec639c..bacd40530c15c 100644 --- a/crates/ruff/tests/cli/lint.rs +++ b/crates/ruff/tests/cli/lint.rs @@ -3720,6 +3720,48 @@ def func(t: _T) -> _T: ); } +/// Test that `noqa` comments with rule codes +/// 1. Get replaced with Ruff-specific suppression comments (RUF105) +/// 2. Use human-readable rule names instead of codes (RUF106) +#[test] +fn noqa_comments_to_human_readable_ruff_ignores() -> Result<()> { + let fixture = CliTest::new()?; + let source = "# ruff: noqa: F401 +import os + +def foo(): + value = 1 # noqa: F841 +"; + + assert_cmd_snapshot!( + fixture + .check_command() + .args([ + "--select=F401,F841,RUF105,RUF106", + "--stdin-filename=test.py", + "--fix", + "--preview", + "-", + ]) + .pass_stdin(source), + @" + success: true + exit_code: 0 + ----- stdout ----- + # ruff:file-ignore[unused-import] + import os + + def foo(): + value = 1 # ruff:ignore[unused-variable] + + ----- stderr ----- + Found 4 errors (4 fixed, 0 remaining). + ", + ); + + Ok(()) +} + /// Test that we do not rename two different type parameters to the same name /// in one execution of Ruff (autofixing this to `class Foo[T, T]: ...` would /// introduce invalid syntax) diff --git a/crates/ruff_linter/resources/mdtest/ruff/rule-codes-in-suppression-comments.md b/crates/ruff_linter/resources/mdtest/ruff/rule-codes-in-suppression-comments.md new file mode 100644 index 0000000000000..e2023004319ef --- /dev/null +++ b/crates/ruff_linter/resources/mdtest/ruff/rule-codes-in-suppression-comments.md @@ -0,0 +1,308 @@ +# `rule-codes-in-suppression-comments` (`RUF106`) + +```toml +[lint] +preview = true +select = ["RUF106"] +external = ["EXT"] +``` + +## `ruff:ignore` + +Each Ruff rule code receives a separate diagnostic. Rule names and external or unknown codes are +preserved: + +```py +# snapshot: rule-codes-in-suppression-comments +# snapshot: rule-codes-in-suppression-comments +# ruff:ignore[F401, undefined-name, EXT001, UNKNOWN, F841] +value = 1 +``` + +```snapshot +error[RUF106]: Rule code used instead of name in suppression comment + --> src/mdtest_snippet.py:3:15 + | +3 | # ruff:ignore[F401, undefined-name, EXT001, UNKNOWN, F841] + | ^^^^ + | +help: Replace rule code with name + | +2 | # snapshot: rule-codes-in-suppression-comments + - # ruff:ignore[F401, undefined-name, EXT001, UNKNOWN, F841] +3 + # ruff:ignore[unused-import, undefined-name, EXT001, UNKNOWN, F841] +4 | value = 1 + | + + +error[RUF106]: Rule code used instead of name in suppression comment + --> src/mdtest_snippet.py:3:54 + | +3 | # ruff:ignore[F401, undefined-name, EXT001, UNKNOWN, F841] + | ^^^^ + | +help: Replace rule code with name + | +2 | # snapshot: rule-codes-in-suppression-comments + - # ruff:ignore[F401, undefined-name, EXT001, UNKNOWN, F841] +3 + # ruff:ignore[F401, undefined-name, EXT001, UNKNOWN, unused-variable] +4 | value = 1 + | +``` + +Valid human-readable names are unaffected: + +```py +# snapshot: rule-codes-in-suppression-comments +# snapshot: rule-codes-in-suppression-comments +# ruff:ignore[F401, undefined-name, F841] +value = 1 +``` + +```snapshot +error[RUF106]: Rule code used instead of name in suppression comment + --> src/mdtest_snippet.py:7:15 + | +7 | # ruff:ignore[F401, undefined-name, F841] + | ^^^^ + | +help: Replace rule code with name + | +6 | # snapshot: rule-codes-in-suppression-comments + - # ruff:ignore[F401, undefined-name, F841] +7 + # ruff:ignore[unused-import, undefined-name, F841] +8 | value = 1 + | + + +error[RUF106]: Rule code used instead of name in suppression comment + --> src/mdtest_snippet.py:7:37 + | +7 | # ruff:ignore[F401, undefined-name, F841] + | ^^^^ + | +help: Replace rule code with name + | +6 | # snapshot: rule-codes-in-suppression-comments + - # ruff:ignore[F401, undefined-name, F841] +7 + # ruff:ignore[F401, undefined-name, unused-variable] +8 | value = 1 + | +``` + +## `ruff:file-ignore` + +```py +# snapshot: rule-codes-in-suppression-comments +# snapshot: rule-codes-in-suppression-comments +# ruff:file-ignore[F401, F841] +``` + +```snapshot +error[RUF106]: Rule code used instead of name in suppression comment + --> src/mdtest_snippet.py:3:20 + | +3 | # ruff:file-ignore[F401, F841] + | ^^^^ + | +help: Replace rule code with name + | +2 | # snapshot: rule-codes-in-suppression-comments + - # ruff:file-ignore[F401, F841] +3 + # ruff:file-ignore[unused-import, F841] + | + + +error[RUF106]: Rule code used instead of name in suppression comment + --> src/mdtest_snippet.py:3:26 + | +3 | # ruff:file-ignore[F401, F841] + | ^^^^ + | +help: Replace rule code with name + | +2 | # snapshot: rule-codes-in-suppression-comments + - # ruff:file-ignore[F401, F841] +3 + # ruff:file-ignore[F401, unused-variable] + | +``` + +## Matched `ruff:disable` and `ruff:enable` + +Matching comments are reported and fixed together: + +```py +# snapshot: rule-codes-in-suppression-comments +# snapshot: rule-codes-in-suppression-comments +# ruff:disable[F401, undefined-name, F841] +value = 1 +# ruff:enable[F401, undefined-name, F841] +``` + +```snapshot +error[RUF106]: Rule code used instead of name in suppression comment + --> src/mdtest_snippet.py:3:16 + | +3 | # ruff:disable[F401, undefined-name, F841] + | ^^^^ +4 | value = 1 +5 | # ruff:enable[F401, undefined-name, F841] + | ---- + | +help: Replace rule code with name + | +2 | # snapshot: rule-codes-in-suppression-comments + - # ruff:disable[F401, undefined-name, F841] +3 + # ruff:disable[unused-import, undefined-name, F841] +4 | value = 1 + - # ruff:enable[F401, undefined-name, F841] +5 + # ruff:enable[unused-import, undefined-name, F841] + | + + +error[RUF106]: Rule code used instead of name in suppression comment + --> src/mdtest_snippet.py:3:38 + | +3 | # ruff:disable[F401, undefined-name, F841] + | ^^^^ +4 | value = 1 +5 | # ruff:enable[F401, undefined-name, F841] + | ---- + | +help: Replace rule code with name + | +2 | # snapshot: rule-codes-in-suppression-comments + - # ruff:disable[F401, undefined-name, F841] +3 + # ruff:disable[F401, undefined-name, unused-variable] +4 | value = 1 + - # ruff:enable[F401, undefined-name, F841] +5 + # ruff:enable[F401, undefined-name, unused-variable] + | +``` + +## Unmatched `ruff:disable` + +An unmatched disable comment is still an effective suppression through the end of its indentation +level: + +```py +# snapshot: rule-codes-in-suppression-comments +# ruff:disable[F401] +``` + +```snapshot +error[RUF106]: Rule code used instead of name in suppression comment + --> src/mdtest_snippet.py:2:16 + | +2 | # ruff:disable[F401] + | ^^^^ + | +help: Replace rule code with name + | +1 | # snapshot: rule-codes-in-suppression-comments + - # ruff:disable[F401] +2 + # ruff:disable[unused-import] + | +``` + +## Unmatched `ruff:enable` + +An unmatched enable comment is invalid and is left to `invalid-suppression-comment`: + +```py +# ruff:enable[F401] +``` + +## Redirected codes + +Redirected codes are replaced with the name of their canonical rule: + +```py +# snapshot: rule-codes-in-suppression-comments +# ruff:ignore[PGH001] +value = 1 +``` + +```snapshot +error[RUF106]: Rule code used instead of name in suppression comment + --> src/mdtest_snippet.py:2:15 + | +2 | # ruff:ignore[PGH001] + | ^^^^^^ + | +help: Replace rule code with name + | +1 | # snapshot: rule-codes-in-suppression-comments + - # ruff:ignore[PGH001] +2 + # ruff:ignore[suspicious-eval-usage] +3 | value = 1 + | +``` + +## Nested suppression comments + +Only the rule codes within a nested suppression comment are replaced: + +```py +# snapshot: rule-codes-in-suppression-comments +# snapshot: rule-codes-in-suppression-comments +value = 1 # explanation # ruff:ignore[F401, F841] reason # another +``` + +```snapshot +error[RUF106]: Rule code used instead of name in suppression comment + --> src/mdtest_snippet.py:3:40 + | +3 | value = 1 # explanation # ruff:ignore[F401, F841] reason # another + | ^^^^ + | +help: Replace rule code with name + | +2 | # snapshot: rule-codes-in-suppression-comments + - value = 1 # explanation # ruff:ignore[F401, F841] reason # another +3 + value = 1 # explanation # ruff:ignore[unused-import, F841] reason # another + | + + +error[RUF106]: Rule code used instead of name in suppression comment + --> src/mdtest_snippet.py:3:46 + | +3 | value = 1 # explanation # ruff:ignore[F401, F841] reason # another + | ^^^^ + | +help: Replace rule code with name + | +2 | # snapshot: rule-codes-in-suppression-comments + - value = 1 # explanation # ruff:ignore[F401, F841] reason # another +3 + value = 1 # explanation # ruff:ignore[F401, unused-variable] reason # another + | +``` + +## Comments without Ruff rule codes + +Comments containing only names and external or unknown codes are unchanged: + +```py +# ruff:ignore[unused-import, EXT001, UNKNOWN] +value = 1 +``` + +## Self-suppression + +The rule can be suppressed by its code or name: + +```py +# ruff:ignore[F401, RUF106] +value = 1 +``` + +```py +# ruff:ignore[F401, rule-codes-in-suppression-comments] +value = 1 +``` + +The diagnostic can also be suppressed with a `noqa` comment: + +```py +value = 1 # ruff:ignore[F401] # noqa: RUF106 +``` diff --git a/crates/ruff_linter/resources/mdtest/suppression/ignore.md b/crates/ruff_linter/resources/mdtest/suppression/ignore.md index 9eed7d355901e..752f1dd75764f 100644 --- a/crates/ruff_linter/resources/mdtest/suppression/ignore.md +++ b/crates/ruff_linter/resources/mdtest/suppression/ignore.md @@ -226,7 +226,7 @@ values = [ ```toml [lint] preview = true -select = ["E501", "F401", "RUF10"] +select = ["E501", "F401", "RUF100", "RUF103", "RUF104"] ``` An intervening `ruff:ignore` directive shouldn't cause a `disable`/`enable` pair to be reported as @@ -304,7 +304,7 @@ def f(): ```toml [lint] preview = true -select = ["F401", "RUF10"] +select = ["F401", "RUF100", "RUF104"] ``` A `file-ignore` within a range suppression takes precedence and marks the `disable` as unused: @@ -568,7 +568,7 @@ help: Remove unused suppression ```toml [lint] preview = true -select = ["F401", "RUF10"] +select = ["F401", "RUF103", "RUF104"] ``` `ruff:ignore` comments nested within other comments should still work: @@ -618,7 +618,7 @@ import foo ```toml [lint] preview = true -select = ["F401", "RUF10"] +select = ["F401", "RUF103", "RUF104"] ``` Nested `disable` and `file-ignore` comments are also invalid and don't suppress diagnostics on the @@ -700,7 +700,7 @@ a = 10 ```toml [lint] preview = true -select = ["E501", "F821", "RUF10"] +select = ["E501", "F821", "RUF100", "RUF103"] ``` `RUF100` should have an unsafe fix when deleting a leading suppression would change the placement @@ -774,7 +774,7 @@ note: This is an unsafe fix and may change runtime behavior ```toml [lint] preview = true -select = ["E501", "RUF10", "FIX002"] +select = ["E501", "RUF100", "FIX002"] ``` Deleting either half of a `disable`/`enable` pair should make the fix unsafe if in a nested context: @@ -812,7 +812,7 @@ note: This is an unsafe fix and may change runtime behavior ```toml [lint] preview = true -select = ["E501", "F401", "F821", "RUF10"] +select = ["E501", "F401", "F821", "RUF100", "RUF103"] ``` Removing a code from a multi-code suppression doesn't promote the later suppression, so the fix is @@ -846,7 +846,7 @@ help: Remove unused suppression ```toml [lint] preview = true -select = ["F821", "RUF10"] +select = ["F821", "RUF102", "RUF103"] ``` The `RUF102` fix should also be unsafe when it would promote a later suppression: @@ -882,7 +882,7 @@ note: This is an unsafe fix and may change runtime behavior ```toml [lint] preview = true -select = ["F401", "F821", "RUF10"] +select = ["F401", "F821", "RUF100", "RUF103"] ``` The same applies to fixes for invalid suppression placement: diff --git a/crates/ruff_linter/src/checkers/noqa.rs b/crates/ruff_linter/src/checkers/noqa.rs index afd190c66c04a..8de18382a77ed 100644 --- a/crates/ruff_linter/src/checkers/noqa.rs +++ b/crates/ruff_linter/src/checkers/noqa.rs @@ -48,6 +48,10 @@ pub(crate) fn check_noqa( let exemption = FileExemption::from(&file_noqa_directives); + // Generate diagnostics for suppression comments before applying suppressions so that the + // diagnostics can themselves be suppressed. + suppressions.check_rule_codes(context, locator); + // Indices of diagnostics that were ignored by a `noqa` directive. let mut ignored_diagnostics = vec![]; diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index a51f84a442399..a554f9fee2168 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -1090,6 +1090,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Ruff, "103") => rules::ruff::rules::InvalidSuppressionComment, (Ruff, "104") => rules::ruff::rules::UnmatchedSuppressionComment, (Ruff, "105") => rules::ruff::rules::NoqaComments, + (Ruff, "106") => rules::ruff::rules::RuleCodesInSuppressionComments, (Ruff, "200") => rules::ruff::rules::InvalidPyprojectToml, #[cfg(any(feature = "test-rules", test))] diff --git a/crates/ruff_linter/src/registry.rs b/crates/ruff_linter/src/registry.rs index 5afccdda5a6e3..990352abfb0dc 100644 --- a/crates/ruff_linter/src/registry.rs +++ b/crates/ruff_linter/src/registry.rs @@ -250,9 +250,11 @@ impl Rule { pub const fn lint_source(&self) -> LintSource { match self { Rule::InvalidPyprojectToml => LintSource::PyprojectToml, - Rule::BlanketNOQA | Rule::NoqaComments | Rule::RedirectedNOQA | Rule::UnusedNOQA => { - LintSource::Noqa - } + Rule::BlanketNOQA + | Rule::NoqaComments + | Rule::RedirectedNOQA + | Rule::RuleCodesInSuppressionComments + | Rule::UnusedNOQA => LintSource::Noqa, Rule::BidirectionalUnicode | Rule::BlankLineWithWhitespace | Rule::DocLineTooLong diff --git a/crates/ruff_linter/src/rules/ruff/rules/mod.rs b/crates/ruff_linter/src/rules/ruff/rules/mod.rs index e470647b0f007..71cd885d0f19c 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/mod.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/mod.rs @@ -50,6 +50,7 @@ pub(crate) use pytest_raises_ambiguous_pattern::*; pub(crate) use quadratic_list_summation::*; pub(crate) use redirected_noqa::*; pub(crate) use redundant_bool_literal::*; +pub(crate) use rule_codes_in_suppression_comments::*; pub(crate) use sort_dunder_all::*; pub(crate) use sort_dunder_slots::*; pub(crate) use starmap_zip::*; @@ -129,6 +130,7 @@ mod pytest_raises_ambiguous_pattern; mod quadratic_list_summation; mod redirected_noqa; mod redundant_bool_literal; +mod rule_codes_in_suppression_comments; mod sequence_sorting; mod sort_dunder_all; mod sort_dunder_slots; diff --git a/crates/ruff_linter/src/rules/ruff/rules/rule_codes_in_suppression_comments.rs b/crates/ruff_linter/src/rules/ruff/rules/rule_codes_in_suppression_comments.rs new file mode 100644 index 0000000000000..0ee0ff0742dfa --- /dev/null +++ b/crates/ruff_linter/src/rules/ruff/rules/rule_codes_in_suppression_comments.rs @@ -0,0 +1,40 @@ +use ruff_macros::{ViolationMetadata, derive_message_formats}; + +use crate::AlwaysFixableViolation; + +/// ## What it does +/// +/// Checks for rule codes in Ruff-specific suppression comments. +/// +/// ## Why is this bad? +/// +/// Human-readable rule names are easier to understand than rule codes. Using names also avoids +/// requiring readers to look up the meaning of each code. +/// +/// This rule applies to `ruff:ignore`, `ruff:file-ignore`, `ruff:disable`, and `ruff:enable` +/// comments. +/// +/// ## Example +/// +/// ```python +/// import os # ruff:ignore[F401] +/// ``` +/// +/// Use instead: +/// ```python +/// import os # ruff:ignore[unused-import] +/// ``` +#[derive(ViolationMetadata)] +#[violation_metadata(preview_since = "NEXT_RUFF_VERSION")] +pub(crate) struct RuleCodesInSuppressionComments; + +impl AlwaysFixableViolation for RuleCodesInSuppressionComments { + #[derive_message_formats] + fn message(&self) -> String { + "Rule code used instead of name in suppression comment".to_string() + } + + fn fix_title(&self) -> String { + "Replace rule code with name".to_string() + } +} diff --git a/crates/ruff_linter/src/suppression.rs b/crates/ruff_linter/src/suppression.rs index 85c2d7b0f12df..01fe547eb9228 100644 --- a/crates/ruff_linter/src/suppression.rs +++ b/crates/ruff_linter/src/suppression.rs @@ -23,7 +23,8 @@ use crate::preview::{is_human_readable_names_enabled, is_ruff_ignore_enabled}; use crate::rule_redirects::get_redirect_target; use crate::rules::ruff::rules::{ InvalidRuleCode, InvalidRuleCodeKind, InvalidSuppressionComment, InvalidSuppressionCommentKind, - UnmatchedSuppressionComment, UnusedCodes, UnusedNOQA, UnusedNOQAKind, code_is_valid, + RuleCodesInSuppressionComments, UnmatchedSuppressionComment, UnusedCodes, UnusedNOQA, + UnusedNOQAKind, code_is_valid, }; use crate::settings::LinterSettings; use crate::settings::types::PreviewMode; @@ -395,6 +396,49 @@ impl Suppressions { false } + /// Check for rule codes in valid suppression comments. + pub(crate) fn check_rule_codes(&self, context: &LintContext, locator: &Locator) { + if !context.is_rule_enabled(Rule::RuleCodesInSuppressionComments) { + return; + } + + // Each comment or matched pair produces one valid suppression per code, all sharing the + // same first comment range. + let mut seen_comments = FxHashSet::default(); + + for suppression in &self.valid { + let first_comment = suppression.comments.first(); + if !seen_comments.insert(first_comment.range) { + continue; + } + + let second_comment = suppression.comments.second(); + for (index, range) in first_comment.codes.iter().enumerate() { + let original = locator.slice(range); + let code = get_redirect_target(original).unwrap_or(original); + let Ok(rule) = Rule::from_code(code) else { + continue; + }; + + let mut diagnostic = + context.report_diagnostic(RuleCodesInSuppressionComments, *range); + let name = rule.name().to_string(); + let fix = if let Some(second_range) = + second_comment.and_then(|comment| comment.codes.get(index)) + { + diagnostic.secondary_annotation_without_message(*second_range); + Fix::safe_edits( + Edit::range_replacement(name.clone(), *range), + [Edit::range_replacement(name, *second_range)], + ) + } else { + Fix::safe_edit(Edit::range_replacement(name, *range)) + }; + diagnostic.set_fix(fix); + } + } + } + pub(crate) fn check_suppressions(&self, context: &LintContext, locator: &Locator) { fn process_pending_diagnostics( key: Option, diff --git a/ruff.schema.json b/ruff.schema.json index 2507c70894ca6..f846497844eb9 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -4266,6 +4266,7 @@ "RUF103", "RUF104", "RUF105", + "RUF106", "RUF2", "RUF20", "RUF200", @@ -5208,6 +5209,7 @@ "return-outside-function", "reuse-of-groupby-generator", "root-logger-call", + "rule-codes-in-suppression-comments", "run-process-in-async-function", "runtime-cast-value", "runtime-import-in-type-checking-block",