[ruff] Implement redirected-noqa (RUF101) (#11052)

## Summary

Based on discussion in #10850.

As it stands today `RUF100` will attempt to replace code redirects with
their target codes even though this is not the "goal" of `RUF100`. This
behavior is confusing and inconsistent, since code redirects which don't
otherwise violate `RUF100` will not be updated. The behavior is also
undocumented. Additionally, users who want to use `RUF100` but do not
want to update redirects have no way to opt out.

This PR explicitly detects redirects with a new rule `RUF101` and
patches `RUF100` to keep original codes in fixes and reporting.

## Test Plan

Added fixture.

crates/ruff_linter/resources/test/fixtures/ruff/RUF100_0.py

@@ -106,3 +106,8 @@ def f():
     # Invalid - nonexistant error code with multibyte character
     d = 1  #…noqa: F841, E50
     e = 1  #…noqa: E50
+
+
+def f():
+    # Disabled - check redirects are reported correctly
+    eval(command)  # noqa: PGH001

crates/ruff_linter/resources/test/fixtures/ruff/RUF101.py

@@ -0,0 +1,6 @@
+x = 2 # noqa: RUF940
+x = 2 # noqa: RUF950
+x = 2 # noqa: RUF940, RUF950
+x = 2 # noqa: RUF950, RUF940, RUF950, RUF950, RUF950
+x = 2 # noqa: RUF940, RUF950, RUF940
+x = 2 # noqa: RUF940, RUF950, RUF940, RUF950

crates/ruff_linter/src/checkers/noqa.rs

@@ -10,11 +10,11 @@ use ruff_python_trivia::CommentRanges;
 use ruff_source_file::Locator;
 
 use crate::fix::edits::delete_comment;
-use crate::noqa;
-use crate::noqa::{Directive, FileExemption, NoqaDirectives, NoqaMapping};
+use crate::noqa::{Code, Directive, FileExemption, NoqaDirectives, NoqaMapping};
 use crate::registry::{AsRule, Rule, RuleSet};
 use crate::rule_redirects::get_redirect_target;
 use crate::rules::pygrep_hooks;
+use crate::rules::ruff;
 use crate::rules::ruff::rules::{UnusedCodes, UnusedNOQA};
 use crate::settings::LinterSettings;
 
@@ -85,7 +85,7 @@ pub(crate) fn check_noqa(
                         true
                     }
                     Directive::Codes(directive) => {
-                        if noqa::includes(diagnostic.kind.rule(), directive.codes()) {
+                        if directive.includes(diagnostic.kind.rule()) {
                             directive_line
                                 .matches
                                 .push(diagnostic.kind.rule().noqa_code());
@@ -132,8 +132,8 @@ pub(crate) fn check_noqa(
                     let mut unmatched_codes = vec![];
                     let mut valid_codes = vec![];
                     let mut self_ignore = false;
-                    for code in directive.codes() {
-                        let code = get_redirect_target(code).unwrap_or(code);
+                    for original_code in directive.iter().map(Code::as_str) {
+                        let code = get_redirect_target(original_code).unwrap_or(original_code);
                         if Rule::UnusedNOQA.noqa_code() == code {
                             self_ignore = true;
                             break;
@@ -145,16 +145,16 @@ pub(crate) fn check_noqa(
                                 .iter()
                                 .any(|external| code.starts_with(external))
                         {
-                            valid_codes.push(code);
+                            valid_codes.push(original_code);
                         } else {
                             if let Ok(rule) = Rule::from_code(code) {
                                 if settings.rules.enabled(rule) {
-                                    unmatched_codes.push(code);
+                                    unmatched_codes.push(original_code);
                                 } else {
-                                    disabled_codes.push(code);
+                                    disabled_codes.push(original_code);
                                 }
                             } else {
-                                unknown_codes.push(code);
+                                unknown_codes.push(original_code);
                             }
                         }
                     }
@@ -208,6 +208,10 @@ pub(crate) fn check_noqa(
         pygrep_hooks::rules::blanket_noqa(diagnostics, &noqa_directives, locator);
     }
 
+    if settings.rules.enabled(Rule::RedirectedNOQA) {
+        ruff::rules::redirected_noqa(diagnostics, &noqa_directives);
+    }
+
     ignored_diagnostics.sort_unstable();
     ignored_diagnostics
 }

crates/ruff_linter/src/codes.rs

@@ -969,6 +969,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
         (Ruff, "028") => (RuleGroup::Preview, rules::ruff::rules::InvalidFormatterSuppressionComment),
         (Ruff, "029") => (RuleGroup::Preview, rules::ruff::rules::UnusedAsync),
         (Ruff, "100") => (RuleGroup::Stable, rules::ruff::rules::UnusedNOQA),
+        (Ruff, "101") => (RuleGroup::Preview, rules::ruff::rules::RedirectedNOQA),
         (Ruff, "200") => (RuleGroup::Stable, rules::ruff::rules::InvalidPyprojectToml),
         #[cfg(feature = "test-rules")]
         (Ruff, "900") => (RuleGroup::Stable, rules::ruff::rules::StableTestRule),

crates/ruff_linter/src/noqa.rs

@@ -85,8 +85,16 @@ impl<'a> Directive<'a> {
                     let mut codes_end = codes_start;
                     let mut leading_space = 0;
                     while let Some(code) = Self::lex_code(&text[codes_end + leading_space..]) {
-                        codes.push(code);
                         codes_end += leading_space;
+                        codes.push(Code {
+                            code,
+                            range: TextRange::at(
+                                TextSize::try_from(codes_end).unwrap(),
+                                code.text_len(),
+                            )
+                            .add(offset),
+                        });
+
                         codes_end += code.len();
 
                         // Codes can be comma- or whitespace-delimited. Compute the length of the
@@ -175,16 +183,52 @@ impl Ranged for All {
     }
 }
 
+/// An individual rule code in a `noqa` directive (e.g., `F401`).
+#[derive(Debug)]
+pub(crate) struct Code<'a> {
+    code: &'a str,
+    range: TextRange,
+}
+
+impl<'a> Code<'a> {
+    /// The code that is ignored by the `noqa` directive.
+    pub(crate) fn as_str(&self) -> &'a str {
+        self.code
+    }
+}
+
+impl Display for Code<'_> {
+    fn fmt(&self, fmt: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        fmt.write_str(self.code)
+    }
+}
+
+impl<'a> Ranged for Code<'a> {
+    /// The range of the rule code.
+    fn range(&self) -> TextRange {
+        self.range
+    }
+}
+
 #[derive(Debug)]
 pub(crate) struct Codes<'a> {
     range: TextRange,
-    codes: Vec<&'a str>,
+    codes: Vec<Code<'a>>,
 }
 
-impl Codes<'_> {
-    /// The codes that are ignored by the `noqa` directive.
-    pub(crate) fn codes(&self) -> &[&str] {
-        &self.codes
+impl<'a> Codes<'a> {
+    /// Returns an iterator over the [`Code`]s in the `noqa` directive.
+    pub(crate) fn iter(&self) -> std::slice::Iter<Code> {
+        self.codes.iter()
+    }
+
+    /// Returns `true` if the string list of `codes` includes `code` (or an alias
+    /// thereof).
+    pub(crate) fn includes(&self, needle: Rule) -> bool {
+        let needle = needle.noqa_code();
+
+        self.iter()
+            .any(|code| needle == get_redirect_target(code.as_str()).unwrap_or(code.as_str()))
     }
 }
 
@@ -195,15 +239,6 @@ impl Ranged for Codes<'_> {
     }
 }
 
-/// Returns `true` if the string list of `codes` includes `code` (or an alias
-/// thereof).
-pub(crate) fn includes(needle: Rule, haystack: &[&str]) -> bool {
-    let needle = needle.noqa_code();
-    haystack
-        .iter()
-        .any(|candidate| needle == get_redirect_target(candidate).unwrap_or(candidate))
-}
-
 /// Returns `true` if the given [`Rule`] is ignored at the specified `lineno`.
 pub(crate) fn rule_is_ignored(
     code: Rule,
@@ -215,7 +250,7 @@ pub(crate) fn rule_is_ignored(
     let line_range = locator.line_range(offset);
     match Directive::try_extract(locator.slice(line_range), line_range.start()) {
         Ok(Some(Directive::All(_))) => true,
-        Ok(Some(Directive::Codes(Codes { codes, range: _ }))) => includes(code, &codes),
+        Ok(Some(Directive::Codes(codes))) => codes.includes(code),
         _ => false,
     }
 }
@@ -525,8 +560,8 @@ fn add_noqa_inner(
                     Directive::All(_) => {
                         continue;
                     }
-                    Directive::Codes(Codes { codes, range: _ }) => {
-                        if includes(diagnostic.kind.rule(), codes) {
+                    Directive::Codes(codes) => {
+                        if codes.includes(diagnostic.kind.rule()) {
                             continue;
                         }
                     }
@@ -542,9 +577,9 @@ fn add_noqa_inner(
                 Directive::All(_) => {
                     continue;
                 }
-                Directive::Codes(Codes { codes, range: _ }) => {
+                Directive::Codes(codes) => {
                     let rule = diagnostic.kind.rule();
-                    if !includes(rule, codes) {
+                    if !codes.includes(rule) {
                         matches_by_line
                             .entry(directive_line.start())
                             .or_insert_with(|| {
@@ -591,15 +626,15 @@ fn add_noqa_inner(
             Some(Directive::All(_)) => {
                 // Does not get inserted into the map.
             }
-            Some(Directive::Codes(Codes { range, codes })) => {
+            Some(Directive::Codes(codes)) => {
                 // Reconstruct the line based on the preserved rule codes.
                 // This enables us to tally the number of edits.
                 let output_start = output.len();
 
                 // Add existing content.
                 output.push_str(
                     locator
-                        .slice(TextRange::new(offset, range.start()))
+                        .slice(TextRange::new(offset, codes.start()))
                         .trim_end(),
                 );
 

crates/ruff_linter/src/registry.rs

@@ -246,7 +246,7 @@ impl Rule {
     pub const fn lint_source(&self) -> LintSource {
         match self {
             Rule::InvalidPyprojectToml => LintSource::PyprojectToml,
-            Rule::BlanketNOQA | Rule::UnusedNOQA => LintSource::Noqa,
+            Rule::BlanketNOQA | Rule::RedirectedNOQA | Rule::UnusedNOQA => LintSource::Noqa,
             Rule::BidirectionalUnicode
             | Rule::BlankLineWithWhitespace
             | Rule::DocLineTooLong

crates/ruff_linter/src/rules/ruff/mod.rs

@@ -54,6 +54,7 @@ mod tests {
     #[test_case(Rule::MissingFStringSyntax, Path::new("RUF027_2.py"))]
     #[test_case(Rule::InvalidFormatterSuppressionComment, Path::new("RUF028.py"))]
     #[test_case(Rule::UnusedAsync, Path::new("RUF029.py"))]
+    #[test_case(Rule::RedirectedNOQA, Path::new("RUF101.py"))]
     fn rules(rule_code: Rule, path: &Path) -> Result<()> {
         let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy());
         let diagnostics = test_path(

crates/ruff_linter/src/rules/ruff/rules/mod.rs

@@ -17,6 +17,7 @@ pub(crate) use never_union::*;
 pub(crate) use pairwise_over_zipped::*;
 pub(crate) use parenthesize_logical_operators::*;
 pub(crate) use quadratic_list_summation::*;
+pub(crate) use redirected_noqa::*;
 pub(crate) use sort_dunder_all::*;
 pub(crate) use sort_dunder_slots::*;
 pub(crate) use static_key_dict_comprehension::*;
@@ -49,6 +50,7 @@ mod never_union;
 mod pairwise_over_zipped;
 mod parenthesize_logical_operators;
 mod quadratic_list_summation;
+mod redirected_noqa;
 mod sequence_sorting;
 mod sort_dunder_all;
 mod sort_dunder_slots;

crates/ruff_linter/src/rules/ruff/rules/redirected_noqa.rs

@@ -0,0 +1,69 @@
+use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix};
+use ruff_macros::{derive_message_formats, violation};
+use ruff_text_size::Ranged;
+
+use crate::noqa::{Directive, NoqaDirectives};
+use crate::rule_redirects::get_redirect_target;
+
+/// ## What it does
+/// Checks for `noqa` directives that use redirected rule codes.
+///
+/// ## Why is this bad?
+/// When a rule code has been redirected, the implication is that the rule has
+/// been deprecated in favor of another rule or code. To keep the codebase
+/// consistent and up-to-date, prefer the canonical rule code over the deprecated
+/// code.
+///
+/// ## Example
+/// ```python
+/// x = eval(command)  # noqa: PGH001
+/// ```
+///
+/// Use instead:
+/// ```python
+/// x = eval(command)  # noqa: S307
+/// ```
+#[violation]
+pub struct RedirectedNOQA {
+    original: String,
+    target: String,
+}
+
+impl AlwaysFixableViolation for RedirectedNOQA {
+    #[derive_message_formats]
+    fn message(&self) -> String {
+        let RedirectedNOQA { original, target } = self;
+        format!("`{original}` is a redirect to `{target}`")
+    }
+
+    fn fix_title(&self) -> String {
+        let RedirectedNOQA { target, .. } = self;
+        format!("Replace with `{target}`")
+    }
+}
+
+/// RUF101
+pub(crate) fn redirected_noqa(diagnostics: &mut Vec<Diagnostic>, noqa_directives: &NoqaDirectives) {
+    for line in noqa_directives.lines() {
+        let Directive::Codes(directive) = &line.directive else {
+            continue;
+        };
+
+        for code in directive.iter() {
+            if let Some(redirected) = get_redirect_target(code.as_str()) {
+                let mut diagnostic = Diagnostic::new(
+                    RedirectedNOQA {
+                        original: code.to_string(),
+                        target: redirected.to_string(),
+                    },
+                    code.range(),
+                );
+                diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement(
+                    redirected.to_string(),
+                    code.range(),
+                )));
+                diagnostics.push(diagnostic);
+            }
+        }
+    }
+}