Apply fix availability and applicability when adding to DiagnosticGuard and remove NoqaCode::rule (#18834)

## Summary

This PR removes the last two places we were using `NoqaCode::rule` in
`linter.rs` (see
#18391 (comment) and
#18391 (comment)) by
checking whether fixes are actually desired before adding them to a
`DiagnosticGuard`. I implemented this by storing a `Violation`'s `Rule`
on the `DiagnosticGuard` so that we could check if it was enabled in the
embedded `LinterSettings` when trying to set a fix.

All of the corresponding `set_fix` methods on `OldDiagnostic` were now
unused (except in tests where I just set `.fix` directly), so I moved
these to the guard instead of keeping both sets.

The very last place where we were using `NoqaCode::rule` was in the
cache. I just reverted this to parsing the `Rule` from the name. I had
forgotten to update the comment there anyway. Hopefully this doesn't
cause too much of a perf hit.

In terms of binary size, we're back down almost to where `main` was two
days ago
(#18391 (comment)):

```
41,559,344 bytes for main 2 days ago
41,669,840 bytes for #18391
41,653,760 bytes for main now (after #18391 merged)
41,602,224 bytes for this branch
```

Only 43 kb up, but that shouldn't all be me this time :)

## Test Plan

Existing tests and benchmarks on this PR

Author: ntBre

crates/ruff/src/cache.rs

@@ -442,7 +442,7 @@ impl LintCacheData {
             // Parse the kebab-case rule name into a `Rule`. This will fail for syntax errors, so
             // this also serves to filter them out, but we shouldn't be caching files with syntax
             // errors anyway.
-            .filter_map(|msg| Some((msg.noqa_code().and_then(|code| code.rule())?, msg)))
+            .filter_map(|msg| Some((msg.name().parse().ok()?, msg)))
             .map(|(rule, msg)| {
                 // Make sure that all message use the same source file.
                 assert_eq!(

crates/ruff_linter/src/checkers/ast/mod.rs

@@ -28,7 +28,7 @@ use itertools::Itertools;
 use log::debug;
 use rustc_hash::{FxHashMap, FxHashSet};
 
-use ruff_diagnostics::IsolationLevel;
+use ruff_diagnostics::{Applicability, Fix, IsolationLevel};
 use ruff_notebook::{CellOffsets, NotebookIndex};
 use ruff_python_ast::helpers::{collect_import_from_member, is_docstring_stmt, to_module_path};
 use ruff_python_ast::identifier::Identifier;
@@ -3154,6 +3154,7 @@ impl<'a> LintContext<'a> {
         DiagnosticGuard {
             context: self,
             diagnostic: Some(OldDiagnostic::new(kind, range, &self.source_file)),
+            rule: T::rule(),
         }
     }
 
@@ -3167,10 +3168,12 @@ impl<'a> LintContext<'a> {
         kind: T,
         range: TextRange,
     ) -> Option<DiagnosticGuard<'chk, 'a>> {
-        if self.is_rule_enabled(T::rule()) {
+        let rule = T::rule();
+        if self.is_rule_enabled(rule) {
             Some(DiagnosticGuard {
                 context: self,
                 diagnostic: Some(OldDiagnostic::new(kind, range, &self.source_file)),
+                rule,
             })
         } else {
             None
@@ -3222,6 +3225,7 @@ pub(crate) struct DiagnosticGuard<'a, 'b> {
     ///
     /// This is always `Some` until the `Drop` (or `defuse`) call.
     diagnostic: Option<OldDiagnostic>,
+    rule: Rule,
 }
 
 impl DiagnosticGuard<'_, '_> {
@@ -3234,6 +3238,50 @@ impl DiagnosticGuard<'_, '_> {
     }
 }
 
+impl DiagnosticGuard<'_, '_> {
+    fn resolve_applicability(&self, fix: &Fix) -> Applicability {
+        self.context
+            .settings
+            .fix_safety
+            .resolve_applicability(self.rule, fix.applicability())
+    }
+
+    /// Set the [`Fix`] used to fix the diagnostic.
+    #[inline]
+    pub(crate) fn set_fix(&mut self, fix: Fix) {
+        if !self.context.rules.should_fix(self.rule) {
+            self.fix = None;
+            return;
+        }
+        let applicability = self.resolve_applicability(&fix);
+        self.fix = Some(fix.with_applicability(applicability));
+    }
+
+    /// Set the [`Fix`] used to fix the diagnostic, if the provided function returns `Ok`.
+    /// Otherwise, log the error.
+    #[inline]
+    pub(crate) fn try_set_fix(&mut self, func: impl FnOnce() -> anyhow::Result<Fix>) {
+        match func() {
+            Ok(fix) => self.set_fix(fix),
+            Err(err) => log::debug!("Failed to create fix for {}: {}", self.name(), err),
+        }
+    }
+
+    /// Set the [`Fix`] used to fix the diagnostic, if the provided function returns `Ok`.
+    /// Otherwise, log the error.
+    #[inline]
+    pub(crate) fn try_set_optional_fix(
+        &mut self,
+        func: impl FnOnce() -> anyhow::Result<Option<Fix>>,
+    ) {
+        match func() {
+            Ok(None) => {}
+            Ok(Some(fix)) => self.set_fix(fix),
+            Err(err) => log::debug!("Failed to create fix for {}: {}", self.name(), err),
+        }
+    }
+}
+
 impl std::ops::Deref for DiagnosticGuard<'_, '_> {
     type Target = OldDiagnostic;
 

crates/ruff_linter/src/fix/edits.rs

@@ -749,15 +749,16 @@ x = 1 \
         let diag = {
             use crate::rules::pycodestyle::rules::MissingNewlineAtEndOfFile;
             let mut iter = edits.into_iter();
-            OldDiagnostic::new(
+            let mut diagnostic = OldDiagnostic::new(
                 MissingNewlineAtEndOfFile, // The choice of rule here is arbitrary.
                 TextRange::default(),
                 &SourceFileBuilder::new("<filename>", "<code>").finish(),
-            )
-            .with_fix(Fix::safe_edits(
+            );
+            diagnostic.fix = Some(Fix::safe_edits(
                 iter.next().ok_or(anyhow!("expected edits nonempty"))?,
                 iter,
-            ))
+            ));
+            diagnostic
         };
         assert_eq!(apply_fixes([diag].iter(), &locator).code, expect);
         Ok(())

crates/ruff_linter/src/fix/mod.rs

@@ -186,12 +186,13 @@ mod tests {
         edit.into_iter()
             .map(|edit| {
                 // The choice of rule here is arbitrary.
-                let diagnostic = OldDiagnostic::new(
+                let mut diagnostic = OldDiagnostic::new(
                     MissingNewlineAtEndOfFile,
                     edit.range(),
                     &SourceFileBuilder::new(filename, source).finish(),
                 );
-                diagnostic.with_fix(Fix::safe_edit(edit))
+                diagnostic.fix = Some(Fix::safe_edit(edit));
+                diagnostic
             })
             .collect()
     }

crates/ruff_linter/src/linter.rs

@@ -378,32 +378,7 @@ pub fn check_path(
 
     let (mut diagnostics, source_file) = context.into_parts();
 
-    if parsed.has_valid_syntax() {
-        // Remove fixes for any rules marked as unfixable.
-        for diagnostic in &mut diagnostics {
-            if diagnostic
-                .noqa_code()
-                .and_then(|code| code.rule())
-                .is_none_or(|rule| !settings.rules.should_fix(rule))
-            {
-                diagnostic.fix = None;
-            }
-        }
-
-        // Update fix applicability to account for overrides
-        if !settings.fix_safety.is_empty() {
-            for diagnostic in &mut diagnostics {
-                if let Some(fix) = diagnostic.fix.take() {
-                    if let Some(rule) = diagnostic.noqa_code().and_then(|code| code.rule()) {
-                        let fixed_applicability = settings
-                            .fix_safety
-                            .resolve_applicability(rule, fix.applicability());
-                        diagnostic.set_fix(fix.with_applicability(fixed_applicability));
-                    }
-                }
-            }
-        }
-    } else {
+    if !parsed.has_valid_syntax() {
         // Avoid fixing in case the source code contains syntax errors.
         for diagnostic in &mut diagnostics {
             diagnostic.fix = None;

crates/ruff_linter/src/message/mod.rs

@@ -186,41 +186,6 @@ impl OldDiagnostic {
         )
     }
 
-    /// Consumes `self` and returns a new `Diagnostic` with the given `fix`.
-    #[inline]
-    #[must_use]
-    pub fn with_fix(mut self, fix: Fix) -> Self {
-        self.set_fix(fix);
-        self
-    }
-
-    /// Set the [`Fix`] used to fix the diagnostic.
-    #[inline]
-    pub fn set_fix(&mut self, fix: Fix) {
-        self.fix = Some(fix);
-    }
-
-    /// Set the [`Fix`] used to fix the diagnostic, if the provided function returns `Ok`.
-    /// Otherwise, log the error.
-    #[inline]
-    pub fn try_set_fix(&mut self, func: impl FnOnce() -> anyhow::Result<Fix>) {
-        match func() {
-            Ok(fix) => self.fix = Some(fix),
-            Err(err) => log::debug!("Failed to create fix for {}: {}", self.name(), err),
-        }
-    }
-
-    /// Set the [`Fix`] used to fix the diagnostic, if the provided function returns `Ok`.
-    /// Otherwise, log the error.
-    #[inline]
-    pub fn try_set_optional_fix(&mut self, func: impl FnOnce() -> anyhow::Result<Option<Fix>>) {
-        match func() {
-            Ok(None) => {}
-            Ok(Some(fix)) => self.fix = Some(fix),
-            Err(err) => log::debug!("Failed to create fix for {}: {}", self.name(), err),
-        }
-    }
-
     /// Consumes `self` and returns a new `Diagnostic` with the given parent node.
     #[inline]
     #[must_use]

crates/ruff_linter/src/registry.rs

@@ -215,6 +215,12 @@ pub enum Linter {
 }
 
 pub trait RuleNamespace: Sized {
+    /// Returns the prefix that every single code that ruff uses to identify
+    /// rules from this linter starts with.  In the case that multiple
+    /// `#[prefix]`es are configured for the variant in the `Linter` enum
+    /// definition this is the empty string.
+    fn common_prefix(&self) -> &'static str;
+
     /// Attempts to parse the given rule code. If the prefix is recognized
     /// returns the respective variant along with the code with the common
     /// prefix stripped.

crates/ruff_linter/src/rule_selector.rs

@@ -265,6 +265,7 @@ mod schema {
     use strum::IntoEnumIterator;
 
     use crate::RuleSelector;
+    use crate::registry::RuleNamespace;
     use crate::rule_selector::{Linter, RuleCodePrefix};
 
     impl JsonSchema for RuleSelector {

crates/ruff_macros/src/map_codes.rs

@@ -254,11 +254,9 @@ fn generate_rule_to_code(linter_to_rules: &BTreeMap<Ident, BTreeMap<String, Rule
     }
 
     let mut rule_noqa_code_match_arms = quote!();
-    let mut noqa_code_rule_match_arms = quote!();
     let mut rule_group_match_arms = quote!();
-    let mut noqa_code_consts = quote!();
 
-    for (i, (rule, codes)) in rule_to_codes.into_iter().enumerate() {
+    for (rule, codes) in rule_to_codes {
         let rule_name = rule.segments.last().unwrap();
         assert_eq!(
             codes.len(),
@@ -294,14 +292,6 @@ See also https://github.com/astral-sh/ruff/issues/2186.
             #(#attrs)* Rule::#rule_name => NoqaCode(crate::registry::Linter::#linter.common_prefix(), #code),
         });
 
-        let const_ident = quote::format_ident!("NOQA_PREFIX_{}", i);
-        noqa_code_consts.extend(quote! {
-            const #const_ident: &str = crate::registry::Linter::#linter.common_prefix();
-        });
-        noqa_code_rule_match_arms.extend(quote! {
-            #(#attrs)* NoqaCode(#const_ident, #code) => Some(Rule::#rule_name),
-        });
-
         rule_group_match_arms.extend(quote! {
             #(#attrs)* Rule::#rule_name => #group,
         });
@@ -350,16 +340,6 @@ See also https://github.com/astral-sh/ruff/issues/2186.
                 }
             }
         }
-
-        impl NoqaCode {
-            pub fn rule(&self) -> Option<Rule> {
-                #noqa_code_consts
-                match self {
-                    #noqa_code_rule_match_arms
-                    _ => None
-                }
-            }
-        }
     };
     rule_to_code
 }

crates/ruff_macros/src/rule_namespace.rs

@@ -118,6 +118,10 @@ pub(crate) fn derive_impl(input: DeriveInput) -> syn::Result<proc_macro2::TokenS
                 None
             }
 
+            fn common_prefix(&self) -> &'static str {
+                match self { #common_prefix_match_arms }
+            }
+
             fn name(&self) -> &'static str {
                 match self { #name_match_arms }
             }
@@ -126,16 +130,6 @@ pub(crate) fn derive_impl(input: DeriveInput) -> syn::Result<proc_macro2::TokenS
                 match self { #url_match_arms }
             }
         }
-
-        impl #ident {
-            /// Returns the prefix that every single code that ruff uses to identify
-            /// rules from this linter starts with.  In the case that multiple
-            /// `#[prefix]`es are configured for the variant in the `Linter` enum
-            /// definition this is the empty string.
-            pub const fn common_prefix(&self) -> &'static str {
-                match self { #common_prefix_match_arms }
-            }
-        }
     })
 }