Remove `Message::to_rule` #18447

ntBre · 2025-06-03T18:55:47Z

Summary

As the title says, this PR removes the Message::to_rule method by replacing related uses of Rule with NoqaCode (or the rule's name in the case of the cache). Where it seemed a Rule was really needed, we convert back to the Rule by parsing either the rule name (with str::parse) or the NoqaCode (with Rule::from_code).

I thought this was kind of like cheating and that it might not resolve this part of Micha's comment:

because we can't add Rule to Diagnostic or have it anywhere in our shared rendering logic

but after looking again, the only remaining Rule conversion in rendering code is for the SARIF output format. The other two non-test Rule conversions are for caching and writing a fix summary, which I don't think fall into the shared rendering logic. That leaves the SARIF format as the only real problem, but maybe we can delay that for now.

The motivation here is that we won't be able to store a Rule on the new Diagnostic type, but we should be able to store a NoqaCode, likely as a string.

Test Plan

Existing tests

Benchmarks

Almost no perf regression, only -1% on linter/default-rules[large/dataset.py].

github-actions · 2025-06-03T19:01:57Z

`ruff-ecosystem` results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

MichaReiser

Thank you. This is great. I think we can avoid the from_code in a few more places, but this, overall looks good

crates/ruff/src/cache.rs

MichaReiser · 2025-06-04T06:39:42Z

crates/ruff/src/printer.rs

-                rule.noqa_code().to_string().red().bold(),
-                rule.as_ref(),
+                code.red().bold(),
+                Rule::from_code(&code).map_or(code, |rule| rule.to_string()),


Does rule.to_string return the rule name? I'm inclined to change FixTable to a map from noqa to (name, usize). This removes the need to lookup the rule.

crates/ruff_linter/src/fix/mod.rs

crates/ruff_linter/src/message/mod.rs

crates/ruff_linter/src/message/sarif.rs

MichaReiser · 2025-06-04T06:57:15Z

crates/ruff_linter/src/message/sarif.rs

+        let code_str = code.to_string();
+        // This is a manual re-implementation of Rule::from_code, but we also want the Linter. This
+        // avoids calling Linter::parse_code twice.
+        let (linter, suffix) = Linter::parse_code(&code_str).unwrap();


It's a bit silly that parse_code strips the prefix again, when NoqaCode already has the prefix. If not too hard, would it make sense to add a parse_noqa method that matches on code.prefix()?

I looked into this a bit last week, and the tricky thing is that despite NoqaCode having two &'static strs and a prefix and suffix method, they don't always do what you expect because in cases with multiple common prefixes (at least I think this is the cause, from what I remember) the "prefix" will actually be empty and the whole code is in the "suffix." That's why parse_code is implemented like this, I think.

This is also what I was trying to work around with my proc macro changes in #18391.

Here's the macro code for generating the common_prefix method:

ruff/crates/ruff_macros/src/rule_namespace.rs

Lines 89 to 96 in 43277a1

if let [prefix] = &prefixes[..] {

common_prefix_match_arms.extend(quote! { Self::#variant_ident => #prefix, });

} else {

// There is more than one prefix. We already previously asserted

// that prefixes of the same variant don't start with the same character

// so the common prefix for this variant is the empty string.

common_prefix_match_arms.extend(quote! { Self::#variant_ident => "", });

}

And then the construction of NoqaCodes uses it:

ruff/crates/ruff_macros/src/map_codes.rs

Lines 291 to 293 in 43277a1

rule_noqa_code_match_arms.extend(quote! {

#(#attrs)* Rule::#rule_name => NoqaCode(crate::registry::Linter::#linter.common_prefix(), #code),

});

I think your idea about storing a single &str and a usize here might help, but it's still a bit tricky to get everything into a single &'static str. We might still need my const tricks from the other PR to concat! them here.

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

crates/ruff_linter/src/noqa.rs

crates/ruff_server/src/lint.rs

MichaReiser · 2025-06-04T07:32:13Z

Here a patch for adding a rule.name method (it also allows us to remove the AsStrRef derive:

Patch

diff --git a/crates/ruff/src/commands/rule.rs b/crates/ruff/src/commands/rule.rs
index 45b071d2e..adc761b3e 100644
--- a/crates/ruff/src/commands/rule.rs
+++ b/crates/ruff/src/commands/rule.rs
@@ -30,7 +30,7 @@ impl<'a> Explanation<'a> {
         let (linter, _) = Linter::parse_code(&code).unwrap();
         let fix = rule.fixable().to_string();
         Self {
-            name: rule.as_ref(),
+            name: rule.name().as_str(),
             code,
             linter: linter.name(),
             summary: rule.message_formats()[0],
@@ -44,7 +44,7 @@ impl<'a> Explanation<'a> {
 
 fn format_rule_text(rule: Rule) -> String {
     let mut output = String::new();
-    let _ = write!(&mut output, "# {} ({})", rule.as_ref(), rule.noqa_code());
+    let _ = write!(&mut output, "# {} ({})", rule.name(), rule.noqa_code());
     output.push('\n');
     output.push('\n');
 
diff --git a/crates/ruff_dev/src/generate_docs.rs b/crates/ruff_dev/src/generate_docs.rs
index d191e8264..5f2309328 100644
--- a/crates/ruff_dev/src/generate_docs.rs
+++ b/crates/ruff_dev/src/generate_docs.rs
@@ -29,7 +29,7 @@ pub(crate) fn main(args: &Args) -> Result<()> {
         if let Some(explanation) = rule.explanation() {
             let mut output = String::new();
 
-            let _ = writeln!(&mut output, "# {} ({})", rule.as_ref(), rule.noqa_code());
+            let _ = writeln!(&mut output, "# {} ({})", rule.name(), rule.noqa_code());
 
             let (linter, _) = Linter::parse_code(&rule.noqa_code().to_string()).unwrap();
             if linter.url().is_some() {
@@ -101,7 +101,7 @@ pub(crate) fn main(args: &Args) -> Result<()> {
             let filename = PathBuf::from(ROOT_DIR)
                 .join("docs")
                 .join("rules")
-                .join(rule.as_ref())
+                .join(&*rule.name())
                 .with_extension("md");
 
             if args.dry_run {
diff --git a/crates/ruff_dev/src/generate_rules_table.rs b/crates/ruff_dev/src/generate_rules_table.rs
index 48b1cdc2c..3255f8f42 100644
--- a/crates/ruff_dev/src/generate_rules_table.rs
+++ b/crates/ruff_dev/src/generate_rules_table.rs
@@ -55,7 +55,7 @@ fn generate_table(table_out: &mut String, rules: impl IntoIterator<Item = Rule>,
             FixAvailability::None => format!("<span {SYMBOL_STYLE}></span>"),
         };
 
-        let rule_name = rule.as_ref();
+        let rule_name = rule.name();
 
         // If the message ends in a bracketed expression (like: "Use {replacement}"), escape the
         // brackets. Otherwise, it'll be interpreted as an HTML attribute via the `attr_list`
diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs
index fafb840a0..c31e989a4 100644
--- a/crates/ruff_linter/src/codes.rs
+++ b/crates/ruff_linter/src/codes.rs
@@ -4,7 +4,7 @@
 /// `--select`. For pylint this is e.g. C0414 and E0118 but also C and E01.
 use std::fmt::Formatter;
 
-use strum_macros::{AsRefStr, EnumIter};
+use strum_macros::EnumIter;
 
 use crate::registry::Linter;
 use crate::rule_selector::is_single_rule_selector;
diff --git a/crates/ruff_linter/src/logging.rs b/crates/ruff_linter/src/logging.rs
index dadf6ce25..b59c63737 100644
--- a/crates/ruff_linter/src/logging.rs
+++ b/crates/ruff_linter/src/logging.rs
@@ -20,7 +20,7 @@ pub static IDENTIFIERS: LazyLock<Mutex<Vec<&'static str>>> = LazyLock::new(Mutex
 /// Warn a user once, with uniqueness determined by the given ID.
 #[macro_export]
 macro_rules! warn_user_once_by_id {
-    ($id:expr, $($arg:tt)*) => {
+    ($id:expr, $($arg:tt)*) => {{
         use colored::Colorize;
         use log::warn;
 
@@ -31,7 +31,7 @@ macro_rules! warn_user_once_by_id {
                 states.push($id);
             }
         }
-    };
+    }};
 }
 
 pub static MESSAGES: LazyLock<Mutex<FxHashSet<String>>> = LazyLock::new(Mutex::default);
diff --git a/crates/ruff_linter/src/registry.rs b/crates/ruff_linter/src/registry.rs
index 9b03a8b7f..2f8a92324 100644
--- a/crates/ruff_linter/src/registry.rs
+++ b/crates/ruff_linter/src/registry.rs
@@ -1,6 +1,7 @@
 //! Remnant of the registry of all [`Rule`] implementations, now it's reexporting from codes.rs
 //! with some helper symbols
 
+use ruff_db::diagnostic::LintName;
 use strum_macros::EnumIter;
 
 pub use codes::Rule;
@@ -348,9 +349,18 @@ impl Rule {
 
     /// Return the URL for the rule documentation, if it exists.
     pub fn url(&self) -> Option<String> {
-        self.explanation()
-            .is_some()
-            .then(|| format!("{}/rules/{}", env!("CARGO_PKG_HOMEPAGE"), self.as_ref()))
+        self.explanation().is_some().then(|| {
+            format!(
+                "{}/rules/{name}",
+                env!("CARGO_PKG_HOMEPAGE"),
+                name = self.name()
+            )
+        })
+    }
+
+    pub fn name(&self) -> LintName {
+        let name: &'static str = self.into();
+        LintName::of(name)
     }
 }
 
@@ -421,7 +431,7 @@ pub mod clap_completion {
         fn possible_values(&self) -> Option<Box<dyn Iterator<Item = PossibleValue> + '_>> {
             Some(Box::new(Rule::iter().map(|rule| {
                 let name = rule.noqa_code().to_string();
-                let help = rule.as_ref().to_string();
+                let help: &'static str = rule.into();
                 PossibleValue::new(name).help(help)
             })))
         }
@@ -443,7 +453,7 @@ mod tests {
             assert!(
                 rule.explanation().is_some(),
                 "Rule {} is missing documentation",
-                rule.as_ref()
+                rule.name()
             );
         }
     }
@@ -460,10 +470,10 @@ mod tests {
             .collect();
 
         for rule in Rule::iter() {
-            let rule_name = rule.as_ref();
+            let rule_name = rule.name();
             for pattern in &patterns {
                 assert!(
-                    !pattern.matches(rule_name),
+                    !pattern.matches(&*rule_name),
                     "{rule_name} does not match naming convention, see CONTRIBUTING.md"
                 );
             }
diff --git a/crates/ruff_linter/src/registry/rule_set.rs b/crates/ruff_linter/src/registry/rule_set.rs
index d83ff0771..62601d722 100644
--- a/crates/ruff_linter/src/registry/rule_set.rs
+++ b/crates/ruff_linter/src/registry/rule_set.rs
@@ -302,9 +302,8 @@ impl Display for RuleSet {
         } else {
             writeln!(f, "[")?;
             for rule in self {
-                let name = rule.as_ref();
                 let code = rule.noqa_code();
-                writeln!(f, "\t{name} ({code}),")?;
+                writeln!(f, "\t{name} ({code}),", name = rule.name())?;
             }
             write!(f, "]")?;
         }
diff --git a/crates/ruff_linter/src/rule_selector.rs b/crates/ruff_linter/src/rule_selector.rs
index 7588aa9f6..74f069b97 100644
--- a/crates/ruff_linter/src/rule_selector.rs
+++ b/crates/ruff_linter/src/rule_selector.rs
@@ -485,8 +485,7 @@ pub mod clap_completion {
                                     prefix.linter().common_prefix(),
                                     prefix.short_code()
                                 );
-                                let name: &'static str = rule.into();
-                                return Some(PossibleValue::new(code).help(name));
+                                return Some(PossibleValue::new(code).help(rule.name().as_str()));
                             }
 
                             None
diff --git a/crates/ruff_linter/src/rules/fastapi/mod.rs b/crates/ruff_linter/src/rules/fastapi/mod.rs
index 90f52b7d6..60f180973 100644
--- a/crates/ruff_linter/src/rules/fastapi/mod.rs
+++ b/crates/ruff_linter/src/rules/fastapi/mod.rs
@@ -3,7 +3,6 @@ pub(crate) mod rules;
 
 #[cfg(test)]
 mod tests {
-    use std::convert::AsRef;
     use std::path::Path;
 
     use anyhow::Result;
@@ -18,7 +17,7 @@ mod tests {
     #[test_case(Rule::FastApiNonAnnotatedDependency, Path::new("FAST002_1.py"))]
     #[test_case(Rule::FastApiUnusedPathParameter, Path::new("FAST003.py"))]
     fn rules(rule_code: Rule, path: &Path) -> Result<()> {
-        let snapshot = format!("{}_{}", rule_code.as_ref(), path.to_string_lossy());
+        let snapshot = format!("{}_{}", rule_code.name(), path.to_string_lossy());
         let diagnostics = test_path(
             Path::new("fastapi").join(path).as_path(),
             &settings::LinterSettings::for_rule(rule_code),
@@ -32,7 +31,7 @@ mod tests {
     #[test_case(Rule::FastApiNonAnnotatedDependency, Path::new("FAST002_0.py"))]
     #[test_case(Rule::FastApiNonAnnotatedDependency, Path::new("FAST002_1.py"))]
     fn rules_py38(rule_code: Rule, path: &Path) -> Result<()> {
-        let snapshot = format!("{}_{}_py38", rule_code.as_ref(), path.to_string_lossy());
+        let snapshot = format!("{}_{}_py38", rule_code.name(), path.to_string_lossy());
         let diagnostics = test_path(
             Path::new("fastapi").join(path).as_path(),
             &settings::LinterSettings {
diff --git a/crates/ruff_linter/src/rules/flake8_fixme/mod.rs b/crates/ruff_linter/src/rules/flake8_fixme/mod.rs
index d2433fc8b..44071d1b7 100644
--- a/crates/ruff_linter/src/rules/flake8_fixme/mod.rs
+++ b/crates/ruff_linter/src/rules/flake8_fixme/mod.rs
@@ -16,7 +16,7 @@ mod tests {
     #[test_case(Rule::LineContainsTodo; "T003")]
     #[test_case(Rule::LineContainsXxx; "T004")]
     fn rules(rule_code: Rule) -> Result<()> {
-        let snapshot = format!("{}_T00.py", rule_code.as_ref());
+        let snapshot = format!("{}_T00.py", rule_code.name());
         let diagnostics = test_path(
             Path::new("flake8_fixme/T00.py"),
             &settings::LinterSettings::for_rule(rule_code),
diff --git a/crates/ruff_linter/src/rules/flake8_gettext/mod.rs b/crates/ruff_linter/src/rules/flake8_gettext/mod.rs
index 09d440526..cd385932a 100644
--- a/crates/ruff_linter/src/rules/flake8_gettext/mod.rs
+++ b/crates/ruff_linter/src/rules/flake8_gettext/mod.rs
@@ -29,7 +29,7 @@ mod tests {
     #[test_case(Rule::FormatInGetTextFuncCall, Path::new("INT002.py"))]
     #[test_case(Rule::PrintfInGetTextFuncCall, Path::new("INT003.py"))]
     fn rules(rule_code: Rule, path: &Path) -> Result<()> {
-        let snapshot = format!("{}_{}", rule_code.as_ref(), path.to_string_lossy());
+        let snapshot = format!("{}_{}", rule_code.name(), path.to_string_lossy());
         let diagnostics = test_path(
             Path::new("flake8_gettext").join(path).as_path(),
             &settings::LinterSettings::for_rule(rule_code),
diff --git a/crates/ruff_linter/src/rules/flake8_raise/mod.rs b/crates/ruff_linter/src/rules/flake8_raise/mod.rs
index d2d636b1e..178045453 100644
--- a/crates/ruff_linter/src/rules/flake8_raise/mod.rs
+++ b/crates/ruff_linter/src/rules/flake8_raise/mod.rs
@@ -3,7 +3,6 @@ pub(crate) mod rules;
 
 #[cfg(test)]
 mod tests {
-    use std::convert::AsRef;
     use std::path::Path;
 
     use anyhow::Result;
@@ -15,7 +14,7 @@ mod tests {
 
     #[test_case(Rule::UnnecessaryParenOnRaiseException, Path::new("RSE102.py"))]
     fn rules(rule_code: Rule, path: &Path) -> Result<()> {
-        let snapshot = format!("{}_{}", rule_code.as_ref(), path.to_string_lossy());
+        let snapshot = format!("{}_{}", rule_code.name(), path.to_string_lossy());
         let diagnostics = test_path(
             Path::new("flake8_raise").join(path).as_path(),
             &settings::LinterSettings::for_rule(rule_code),
diff --git a/crates/ruff_linter/src/rules/flake8_self/mod.rs b/crates/ruff_linter/src/rules/flake8_self/mod.rs
index a445b40ba..7a3f83ddd 100644
--- a/crates/ruff_linter/src/rules/flake8_self/mod.rs
+++ b/crates/ruff_linter/src/rules/flake8_self/mod.rs
@@ -4,7 +4,6 @@ pub mod settings;
 
 #[cfg(test)]
 mod tests {
-    use std::convert::AsRef;
     use std::path::Path;
 
     use crate::registry::Rule;
@@ -18,7 +17,7 @@ mod tests {
     #[test_case(Rule::PrivateMemberAccess, Path::new("SLF001.py"))]
     #[test_case(Rule::PrivateMemberAccess, Path::new("SLF001_1.py"))]
     fn rules(rule_code: Rule, path: &Path) -> Result<()> {
-        let snapshot = format!("{}_{}", rule_code.as_ref(), path.to_string_lossy());
+        let snapshot = format!("{}_{}", rule_code.name(), path.to_string_lossy());
         let diagnostics = test_path(
             Path::new("flake8_self").join(path).as_path(),
             &settings::LinterSettings::for_rule(rule_code),
diff --git a/crates/ruff_linter/src/rules/flake8_todos/mod.rs b/crates/ruff_linter/src/rules/flake8_todos/mod.rs
index 7a4129a78..486c46af0 100644
--- a/crates/ruff_linter/src/rules/flake8_todos/mod.rs
+++ b/crates/ruff_linter/src/rules/flake8_todos/mod.rs
@@ -2,7 +2,6 @@ pub(crate) mod rules;
 
 #[cfg(test)]
 mod tests {
-    use std::convert::AsRef;
     use std::path::Path;
 
     use anyhow::Result;
@@ -20,7 +19,7 @@ mod tests {
     #[test_case(Rule::InvalidTodoCapitalization, Path::new("TD006.py"))]
     #[test_case(Rule::MissingSpaceAfterTodoColon, Path::new("TD007.py"))]
     fn rules(rule_code: Rule, path: &Path) -> Result<()> {
-        let snapshot = format!("{}_{}", rule_code.as_ref(), path.to_string_lossy());
+        let snapshot = format!("{}_{}", rule_code.name(), path.to_string_lossy());
         let diagnostics = test_path(
             Path::new("flake8_todos").join(path).as_path(),
             &settings::LinterSettings::for_rule(rule_code),
diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/mod.rs b/crates/ruff_linter/src/rules/flake8_type_checking/mod.rs
index f5b777ec4..acdb788f5 100644
--- a/crates/ruff_linter/src/rules/flake8_type_checking/mod.rs
+++ b/crates/ruff_linter/src/rules/flake8_type_checking/mod.rs
@@ -6,7 +6,6 @@ pub mod settings;
 
 #[cfg(test)]
 mod tests {
-    use std::convert::AsRef;
     use std::path::Path;
 
     use anyhow::Result;
@@ -55,7 +54,7 @@ mod tests {
     #[test_case(Rule::TypingOnlyThirdPartyImport, Path::new("typing_modules_1.py"))]
     #[test_case(Rule::TypingOnlyThirdPartyImport, Path::new("typing_modules_2.py"))]
     fn rules(rule_code: Rule, path: &Path) -> Result<()> {
-        let snapshot = format!("{}_{}", rule_code.as_ref(), path.to_string_lossy());
+        let snapshot = format!("{}_{}", rule_code.name(), path.to_string_lossy());
         let diagnostics = test_path(
             Path::new("flake8_type_checking").join(path).as_path(),
             &settings::LinterSettings::for_rule(rule_code),
@@ -70,7 +69,7 @@ mod tests {
     #[test_case(Rule::QuotedTypeAlias, Path::new("TC008.py"))]
     #[test_case(Rule::QuotedTypeAlias, Path::new("TC008_typing_execution_context.py"))]
     fn type_alias_rules(rule_code: Rule, path: &Path) -> Result<()> {
-        let snapshot = format!("{}_{}", rule_code.as_ref(), path.to_string_lossy());
+        let snapshot = format!("{}_{}", rule_code.name(), path.to_string_lossy());
         let diagnostics = test_path(
             Path::new("flake8_type_checking").join(path).as_path(),
             &settings::LinterSettings::for_rules(vec![
@@ -84,11 +83,7 @@ mod tests {
 
     #[test_case(Rule::QuotedTypeAlias, Path::new("TC008_union_syntax_pre_py310.py"))]
     fn type_alias_rules_pre_py310(rule_code: Rule, path: &Path) -> Result<()> {
-        let snapshot = format!(
-            "pre_py310_{}_{}",
-            rule_code.as_ref(),
-            path.to_string_lossy()
-        );
+        let snapshot = format!("pre_py310_{}_{}", rule_code.name(), path.to_string_lossy());
         let diagnostics = test_path(
             Path::new("flake8_type_checking").join(path).as_path(),
             &settings::LinterSettings {
@@ -107,7 +102,7 @@ mod tests {
     #[test_case(Rule::RuntimeImportInTypeCheckingBlock, Path::new("quote3.py"))]
     #[test_case(Rule::TypingOnlyThirdPartyImport, Path::new("quote3.py"))]
     fn quote(rule_code: Rule, path: &Path) -> Result<()> {
-        let snapshot = format!("quote_{}_{}", rule_code.as_ref(), path.to_string_lossy());
+        let snapshot = format!("quote_{}_{}", rule_code.name(), path.to_string_lossy());
         let diagnostics = test_path(
             Path::new("flake8_type_checking").join(path).as_path(),
             &settings::LinterSettings {
@@ -126,7 +121,7 @@ mod tests {
     #[test_case(Rule::TypingOnlyStandardLibraryImport, Path::new("init_var.py"))]
     #[test_case(Rule::TypingOnlyStandardLibraryImport, Path::new("kw_only.py"))]
     fn strict(rule_code: Rule, path: &Path) -> Result<()> {
-        let snapshot = format!("strict_{}_{}", rule_code.as_ref(), path.to_string_lossy());
+        let snapshot = format!("strict_{}_{}", rule_code.name(), path.to_string_lossy());
         let diagnostics = test_path(
             Path::new("flake8_type_checking").join(path).as_path(),
             &settings::LinterSettings {
@@ -170,7 +165,7 @@ mod tests {
         Path::new("exempt_type_checking_3.py")
     )]
     fn exempt_type_checking(rule_code: Rule, path: &Path) -> Result<()> {
-        let snapshot = format!("{}_{}", rule_code.as_ref(), path.to_string_lossy());
+        let snapshot = format!("{}_{}", rule_code.name(), path.to_string_lossy());
         let diagnostics = test_path(
             Path::new("flake8_type_checking").join(path).as_path(),
             &settings::LinterSettings {
@@ -207,7 +202,7 @@ mod tests {
         Path::new("runtime_evaluated_base_classes_5.py")
     )]
     fn runtime_evaluated_base_classes(rule_code: Rule, path: &Path) -> Result<()> {
-        let snapshot = format!("{}_{}", rule_code.as_ref(), path.to_string_lossy());
+        let snapshot = format!("{}_{}", rule_code.name(), path.to_string_lossy());
         let diagnostics = test_path(
             Path::new("flake8_type_checking").join(path).as_path(),
             &settings::LinterSettings {
@@ -238,7 +233,7 @@ mod tests {
         Path::new("runtime_evaluated_decorators_3.py")
     )]
     fn runtime_evaluated_decorators(rule_code: Rule, path: &Path) -> Result<()> {
-        let snapshot = format!("{}_{}", rule_code.as_ref(), path.to_string_lossy());
+        let snapshot = format!("{}_{}", rule_code.name(), path.to_string_lossy());
         let diagnostics = test_path(
             Path::new("flake8_type_checking").join(path).as_path(),
             &settings::LinterSettings {
@@ -264,7 +259,7 @@ mod tests {
         Path::new("module/undefined.py")
     )]
     fn base_class_same_file(rule_code: Rule, path: &Path) -> Result<()> {
-        let snapshot = format!("{}_{}", rule_code.as_ref(), path.to_string_lossy());
+        let snapshot = format!("{}_{}", rule_code.name(), path.to_string_lossy());
         let diagnostics = test_path(
             Path::new("flake8_type_checking").join(path).as_path(),
             &settings::LinterSettings {
@@ -282,7 +277,7 @@ mod tests {
     #[test_case(Rule::RuntimeImportInTypeCheckingBlock, Path::new("module/app.py"))]
     #[test_case(Rule::TypingOnlyStandardLibraryImport, Path::new("module/routes.py"))]
     fn decorator_same_file(rule_code: Rule, path: &Path) -> Result<()> {
-        let snapshot = format!("{}_{}", rule_code.as_ref(), path.to_string_lossy());
+        let snapshot = format!("{}_{}", rule_code.name(), path.to_string_lossy());
         let diagnostics = test_path(
             Path::new("flake8_type_checking").join(path).as_path(),
             &settings::LinterSettings {
diff --git a/crates/ruff_linter/src/rules/numpy/mod.rs b/crates/ruff_linter/src/rules/numpy/mod.rs
index 7a313eac2..9403f2501 100644
--- a/crates/ruff_linter/src/rules/numpy/mod.rs
+++ b/crates/ruff_linter/src/rules/numpy/mod.rs
@@ -4,7 +4,6 @@ pub(crate) mod rules;
 
 #[cfg(test)]
 mod tests {
-    use std::convert::AsRef;
     use std::path::Path;
 
     use anyhow::Result;
@@ -22,7 +21,7 @@ mod tests {
     #[test_case(Rule::Numpy2Deprecation, Path::new("NPY201_2.py"))]
     #[test_case(Rule::Numpy2Deprecation, Path::new("NPY201_3.py"))]
     fn rules(rule_code: Rule, path: &Path) -> Result<()> {
-        let snapshot = format!("{}_{}", rule_code.as_ref(), path.to_string_lossy());
+        let snapshot = format!("{}_{}", rule_code.name(), path.to_string_lossy());
         let diagnostics = test_path(
             Path::new("numpy").join(path).as_path(),
             &settings::LinterSettings::for_rule(rule_code),
diff --git a/crates/ruff_linter/src/rules/pydoclint/mod.rs b/crates/ruff_linter/src/rules/pydoclint/mod.rs
index af4131e42..6e2ff9402 100644
--- a/crates/ruff_linter/src/rules/pydoclint/mod.rs
+++ b/crates/ruff_linter/src/rules/pydoclint/mod.rs
@@ -4,7 +4,6 @@ pub mod settings;
 
 #[cfg(test)]
 mod tests {
-    use std::convert::AsRef;
     use std::path::Path;
 
     use anyhow::Result;
@@ -20,7 +19,7 @@ mod tests {
 
     #[test_case(Rule::DocstringMissingException, Path::new("DOC501.py"))]
     fn rules(rule_code: Rule, path: &Path) -> Result<()> {
-        let snapshot = format!("{}_{}", rule_code.as_ref(), path.to_string_lossy());
+        let snapshot = format!("{}_{}", rule_code.name(), path.to_string_lossy());
         let diagnostics = test_path(
             Path::new("pydoclint").join(path).as_path(),
             &settings::LinterSettings::for_rule(rule_code),
@@ -36,7 +35,7 @@ mod tests {
     #[test_case(Rule::DocstringMissingException, Path::new("DOC501_google.py"))]
     #[test_case(Rule::DocstringExtraneousException, Path::new("DOC502_google.py"))]
     fn rules_google_style(rule_code: Rule, path: &Path) -> Result<()> {
-        let snapshot = format!("{}_{}", rule_code.as_ref(), path.to_string_lossy());
+        let snapshot = format!("{}_{}", rule_code.name(), path.to_string_lossy());
         let diagnostics = test_path(
             Path::new("pydoclint").join(path).as_path(),
             &settings::LinterSettings {
@@ -58,7 +57,7 @@ mod tests {
     #[test_case(Rule::DocstringMissingException, Path::new("DOC501_numpy.py"))]
     #[test_case(Rule::DocstringExtraneousException, Path::new("DOC502_numpy.py"))]
     fn rules_numpy_style(rule_code: Rule, path: &Path) -> Result<()> {
-        let snapshot = format!("{}_{}", rule_code.as_ref(), path.to_string_lossy());
+        let snapshot = format!("{}_{}", rule_code.name(), path.to_string_lossy());
         let diagnostics = test_path(
             Path::new("pydoclint").join(path).as_path(),
             &settings::LinterSettings {
@@ -79,7 +78,7 @@ mod tests {
     fn rules_google_style_ignore_one_line(rule_code: Rule, path: &Path) -> Result<()> {
         let snapshot = format!(
             "{}_{}_ignore_one_line",
-            rule_code.as_ref(),
+            rule_code.name(),
             path.to_string_lossy()
         );
         let diagnostics = test_path(
diff --git a/crates/ruff_linter/src/rules/tryceratops/mod.rs b/crates/ruff_linter/src/rules/tryceratops/mod.rs
index 21f3a0138..8d431aae6 100644
--- a/crates/ruff_linter/src/rules/tryceratops/mod.rs
+++ b/crates/ruff_linter/src/rules/tryceratops/mod.rs
@@ -4,7 +4,6 @@ pub(crate) mod rules;
 
 #[cfg(test)]
 mod tests {
-    use std::convert::AsRef;
     use std::path::Path;
 
     use anyhow::Result;
@@ -25,7 +24,7 @@ mod tests {
     #[test_case(Rule::ErrorInsteadOfException, Path::new("TRY400.py"))]
     #[test_case(Rule::VerboseLogMessage, Path::new("TRY401.py"))]
     fn rules(rule_code: Rule, path: &Path) -> Result<()> {
-        let snapshot = format!("{}_{}", rule_code.as_ref(), path.to_string_lossy());
+        let snapshot = format!("{}_{}", rule_code.name(), path.to_string_lossy());
         let diagnostics = test_path(
             Path::new("tryceratops").join(path).as_path(),
             &settings::LinterSettings::for_rule(rule_code),
diff --git a/crates/ruff_macros/src/map_codes.rs b/crates/ruff_macros/src/map_codes.rs
index 7d1ccaf02..39993af6a 100644
--- a/crates/ruff_macros/src/map_codes.rs
+++ b/crates/ruff_macros/src/map_codes.rs
@@ -174,7 +174,7 @@ pub(crate) fn map_codes(func: &ItemFn) -> syn::Result<TokenStream> {
 
         output.extend(quote! {
             impl #linter {
-                pub fn rules(&self) -> ::std::vec::IntoIter<Rule> {
+                pub(crate) fn rules(&self) -> ::std::vec::IntoIter<Rule> {
                     match self { #prefix_into_iter_match_arms }
                 }
             }
@@ -182,7 +182,7 @@ pub(crate) fn map_codes(func: &ItemFn) -> syn::Result<TokenStream> {
     }
     output.extend(quote! {
         impl RuleCodePrefix {
-            pub fn parse(linter: &Linter, code: &str) -> Result<Self, crate::registry::FromCodeError> {
+            pub(crate) fn parse(linter: &Linter, code: &str) -> Result<Self, crate::registry::FromCodeError> {
                 use std::str::FromStr;
 
                 Ok(match linter {
@@ -190,7 +190,7 @@ pub(crate) fn map_codes(func: &ItemFn) -> syn::Result<TokenStream> {
                 })
             }
 
-            pub fn rules(&self) -> ::std::vec::IntoIter<Rule> {
+            pub(crate) fn rules(&self) -> ::std::vec::IntoIter<Rule> {
                 match self {
                     #(RuleCodePrefix::#linter_idents(prefix) => prefix.clone().rules(),)*
                 }
@@ -319,7 +319,7 @@ See also https://github.com/astral-sh/ruff/issues/2186.
                 matches!(self.group(), RuleGroup::Preview)
             }
 
-            pub fn is_stable(&self) -> bool {
+            pub(crate) fn is_stable(&self) -> bool {
                 matches!(self.group(), RuleGroup::Stable)
             }
 
@@ -371,7 +371,7 @@ fn generate_iter_impl(
     quote! {
         impl Linter {
             /// Rules not in the preview.
-            pub fn rules(self: &Linter) -> ::std::vec::IntoIter<Rule> {
+            pub(crate) fn rules(self: &Linter) -> ::std::vec::IntoIter<Rule> {
                 match self {
                     #linter_rules_match_arms
                 }
@@ -385,7 +385,7 @@ fn generate_iter_impl(
         }
 
         impl RuleCodePrefix {
-            pub fn iter() -> impl Iterator<Item = RuleCodePrefix> {
+            pub(crate) fn iter() -> impl Iterator<Item = RuleCodePrefix> {
                 use strum::IntoEnumIterator;
 
                 let mut prefixes = Vec::new();
@@ -436,7 +436,6 @@ fn register_rules<'a>(input: impl Iterator<Item = &'a Rule>) -> TokenStream {
             PartialOrd,
             Ord,
             ::ruff_macros::CacheKey,
-            AsRefStr,
             ::strum_macros::IntoStaticStr,
             ::strum_macros::EnumString,
             ::serde::Serialize,
diff --git a/crates/ruff_server/src/server/api/requests/hover.rs b/crates/ruff_server/src/server/api/requests/hover.rs
index 6b391e15b..846f3654c 100644
--- a/crates/ruff_server/src/server/api/requests/hover.rs
+++ b/crates/ruff_server/src/server/api/requests/hover.rs
@@ -85,7 +85,7 @@ pub(crate) fn hover(
 
 fn format_rule_text(rule: Rule) -> String {
     let mut output = String::new();
-    let _ = write!(&mut output, "# {} ({})", rule.as_ref(), rule.noqa_code());
+    let _ = write!(&mut output, "# {} ({})", rule.name(), rule.noqa_code());
     output.push('\n');
     output.push('\n');
 
diff --git a/crates/ruff_workspace/src/configuration.rs b/crates/ruff_workspace/src/configuration.rs
index 684a2b434..ae0d23150 100644
--- a/crates/ruff_workspace/src/configuration.rs
+++ b/crates/ruff_workspace/src/configuration.rs
@@ -1098,7 +1098,7 @@ impl LintConfiguration {
         // approach to give each pair it's own `warn_user_once`.
         for (preferred, expendable, message) in INCOMPATIBLE_CODES {
             if rules.enabled(*preferred) && rules.enabled(*expendable) {
-                warn_user_once_by_id!(expendable.as_ref(), "{}", message);
+                warn_user_once_by_id!(expendable.name().as_str(), "{}", message);
                 rules.disable(*expendable);
             }
         }

IMO, it improves readability over the to_string, as_ref or let name: &'static str = rule.into() calls

Co-authored-by: Micha Reiser <[email protected]>

ntBre · 2025-06-04T14:31:24Z

Thanks for the patch! I also got confused a couple times by the various options but hadn't tried to fix it. This should help a lot.

in case of duplicates

ntBre · 2025-06-04T19:32:35Z

crates/ruff_linter/src/message/sarif.rs

@@ -149,7 +149,7 @@ impl SarifResult {
        let end_location = message.compute_end_location();
        let path = normalize_path(&*message.filename());
        Ok(Self {
-            code: message.to_noqa_code(),
+            code: message.noqa_code(),


How do you feel about moving the cfg attrs here just to the uri attribute? Or alternatively something like

let uri = cfg!(target_arch = "wasm32") { path.display().to_string() } else { url::Url::from_file_path(&path) .map_err(|()| anyhow::anyhow!("Failed to convert path to URL: {}", path.display()))? .to_string() };

That's the only difference between these two from_message implementations. I guess it's pretty minor in this case, but I'm curious about your thoughts in general. I've managed to forget to update the wasm version at least twice in this PR.

I don't know if you can use the inline cfg but I'm definetely in favor of reducing unnecessary code duplication

Ah you're right. Url::from_file_path isn't available on WASM. I'll just leave it alone for now, but good to know. Thanks!

The easiest would be to extract the URI conversion into it's own function and gate that

## Summary This is a spin-off from #18447 (comment) to avoid using `Message::noqa_code` to differentiate between lints and syntax errors. I went through all of the calls on `main` and on the branch from #18447, and the instance in `ruff_server` noted in the linked comment was actually the primary place where this was being done. Other calls to `noqa_code` are typically some variation of `message.noqa_code().map_or(String::new, format!(...))`, with the major exception of the gitlab output format: https://github.com/astral-sh/ruff/blob/a120610b5b01a9e7bb91740a23f6c2b5bbcd4b5f/crates/ruff_linter/src/message/gitlab.rs#L93-L105 which obviously assumes that `None` means syntax error. A simple fix here would be to use `message.name()` for `check_name` instead of the noqa code, but I'm not sure how breaking that would be. This could just be: ```rust let description = message.body(); let description = description.strip_prefix("SyntaxError: ").unwrap_or(description).to_string(); let check_name = message.name(); ``` In that case. This sounds reasonable based on the [Code Quality report format](https://docs.gitlab.com/ci/testing/code_quality/#code-quality-report-format) docs: > | Name | Type | Description| > |-----|-----|----| > |`check_name` | String | A unique name representing the check, or rule, associated with this violation. | ## Test Plan Existing tests

MichaReiser · 2025-06-05T16:34:20Z

crates/ruff/src/diagnostics.rs

Nice cleanup in this commit!

ntBre added 9 commits June 3, 2025 12:28

replace one call with to_url

0302b9c

store the rule_name instead of rule in the cache

4f71b7e

avoid converting to Rule to collect rule codes, use Rule on output

a779e5c

avoid to_rule for sarif output format

6e2f059

use Vec<NoqaCode> directly instead of constructing a RuleSet

c030bfa

convert Rule from noqa_code in some tests

de72388

avoid to_rule in ruff_server

2269ff9

use my TODO idea for to_url

46d2e9e

delete Message::to_rule

43277a1

ntBre added internal An internal refactor or improvement diagnostics Related to reporting of diagnostics. labels Jun 3, 2025

ntBre marked this pull request as ready for review June 3, 2025 19:13

ntBre requested a review from MichaReiser June 3, 2025 19:13

ntBre marked this pull request as draft June 3, 2025 19:14

ntBre marked this pull request as ready for review June 3, 2025 19:16

MichaReiser approved these changes Jun 4, 2025

View reviewed changes

ntBre and others added 3 commits June 4, 2025 09:58

restore CacheMessage::rule, parse the name on input

2e90bd1

Message::to_noqa_code -> noqa_code

62e42d9

add Rule::name, remove AsRefStr impl, many pub to pub(crate)

17888e5

Co-authored-by: Micha Reiser <[email protected]>

ntBre added 4 commits June 4, 2025 10:39

avoid Rule::from_code in pyflakes

73347b7

make FixTable a struct and store the rule name along with the code

0c22912

use HashSet instead of Vec for noqa_codes, closer to RuleSet

96a6ba2

in case of duplicates

fix wasm build

71e658a

ntBre commented Jun 4, 2025

View reviewed changes

ntBre mentioned this pull request Jun 4, 2025

Combine lint and syntax error handling #18471

Merged

fix count destructuring

51ab830

ntBre added 2 commits June 5, 2025 09:04

Merge branch 'main' into brent/remove-to-rule

2d2c05f

improve FixTable interface

13a586c

MichaReiser reviewed Jun 5, 2025

View reviewed changes

ntBre merged commit ce216c7 into main Jun 5, 2025
34 checks passed

ntBre deleted the brent/remove-to-rule branch June 5, 2025 16:48

	if let [prefix] = &prefixes[..] {
	common_prefix_match_arms.extend(quote! { Self::#variant_ident => #prefix, });
	} else {
	// There is more than one prefix. We already previously asserted
	// that prefixes of the same variant don't start with the same character
	// so the common prefix for this variant is the empty string.
	common_prefix_match_arms.extend(quote! { Self::#variant_ident => "", });
	}

	rule_noqa_code_match_arms.extend(quote! {
	#(#attrs)* Rule::#rule_name => NoqaCode(crate::registry::Linter::#linter.common_prefix(), #code),
	});

Remove Message::to_rule #18447

Remove Message::to_rule #18447

Uh oh!

Conversation

ntBre commented Jun 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test Plan

Uh oh!

github-actions bot commented Jun 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

ruff-ecosystem results

Linter (stable)

Linter (preview)

Uh oh!

MichaReiser left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

MichaReiser commented Jun 4, 2025 • edited by ntBre Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ntBre commented Jun 4, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MichaReiser Jun 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Remove `Message::to_rule` #18447

Remove `Message::to_rule` #18447

ntBre commented Jun 3, 2025 •

edited

Loading

github-actions bot commented Jun 3, 2025 •

edited

Loading

`ruff-ecosystem` results

MichaReiser commented Jun 4, 2025 •

edited by ntBre

Loading

MichaReiser Jun 5, 2025 •

edited

Loading