add SecondaryCode

Author: ntBre

crates/ruff/src/printer.rs

@@ -14,7 +14,7 @@ use ruff_linter::logging::LogLevel;
 use ruff_linter::message::{
     AzureEmitter, Emitter, EmitterContext, GithubEmitter, GitlabEmitter, GroupedEmitter,
     JsonEmitter, JsonLinesEmitter, JunitEmitter, OldDiagnostic, PylintEmitter, RdjsonEmitter,
-    SarifEmitter, TextEmitter,
+    SarifEmitter, SecondaryCode, TextEmitter,
 };
 use ruff_linter::notify_user;
 use ruff_linter::settings::flags::{self};
@@ -36,7 +36,7 @@ bitflags! {
 
 #[derive(Serialize)]
 struct ExpandedStatistics<'a> {
-    code: Option<&'a str>,
+    code: Option<&'a SecondaryCode>,
     name: &'static str,
     count: usize,
     fixable: bool,
@@ -306,7 +306,8 @@ impl Printer {
             .sorted_by_key(|(code, message)| (*code, message.fixable()))
             .fold(
                 vec![],
-                |mut acc: Vec<((Option<&str>, &OldDiagnostic), usize)>, (code, message)| {
+                |mut acc: Vec<((Option<&SecondaryCode>, &OldDiagnostic), usize)>,
+                 (code, message)| {
                     if let Some(((prev_code, _prev_message), count)) = acc.last_mut() {
                         if *prev_code == code {
                             *count += 1;
@@ -348,7 +349,7 @@ impl Printer {
                 );
                 let code_width = statistics
                     .iter()
-                    .map(|statistic| statistic.code.map_or(0, str::len))
+                    .map(|statistic| statistic.code.map_or(0, |s| s.len()))
                     .max()
                     .unwrap();
                 let any_fixable = statistics.iter().any(|statistic| statistic.fixable);
@@ -362,7 +363,7 @@ impl Printer {
                         writer,
                         "{:>count_width$}\t{:<code_width$}\t{}{}",
                         statistic.count.to_string().bold(),
-                        statistic.code.unwrap_or_default().red().bold(),
+                        statistic.code.map_or("", |s| s.as_str()).red().bold(),
                         if any_fixable {
                             if statistic.fixable {
                                 &fixable

crates/ruff_linter/src/checkers/noqa.rs

@@ -51,11 +51,11 @@ pub(crate) fn check_noqa(
             continue;
         };
 
-        if code == Rule::BlanketNOQA.noqa_code() {
+        if *code == Rule::BlanketNOQA.noqa_code() {
             continue;
         }
 
-        if exemption.contains(&code) {
+        if exemption.contains(code) {
             ignored_diagnostics.push(index);
             continue;
         }
@@ -72,13 +72,13 @@ pub(crate) fn check_noqa(
             {
                 let suppressed = match &directive_line.directive {
                     Directive::All(_) => {
-                        directive_line.matches.push(code.to_string());
+                        directive_line.matches.push(code.clone());
                         ignored_diagnostics.push(index);
                         true
                     }
                     Directive::Codes(directive) => {
-                        if directive.includes(&code) {
-                            directive_line.matches.push(code.to_string());
+                        if directive.includes(code) {
+                            directive_line.matches.push(code.clone());
                             ignored_diagnostics.push(index);
                             true
                         } else {
@@ -138,7 +138,7 @@ pub(crate) fn check_noqa(
                         if seen_codes.insert(original_code) {
                             let is_code_used = if is_file_level {
                                 context.iter().any(|diag| {
-                                    diag.secondary_code().is_some_and(|noqa| noqa == code)
+                                    diag.secondary_code().is_some_and(|noqa| *noqa == code)
                                 })
                             } else {
                                 matches.iter().any(|match_| *match_ == code)

crates/ruff_linter/src/linter.rs

@@ -25,6 +25,7 @@ use crate::checkers::tokens::check_tokens;
 use crate::directives::Directives;
 use crate::doc_lines::{doc_lines_from_ast, doc_lines_from_tokens};
 use crate::fix::{FixResult, fix_file};
+use crate::message::SecondaryCode;
 use crate::noqa::add_noqa;
 use crate::package::PackageRoot;
 use crate::preview::is_py314_support_enabled;
@@ -93,25 +94,25 @@ struct FixCount {
 
 /// A mapping from a noqa code to the corresponding lint name and a count of applied fixes.
 #[derive(Debug, Default, PartialEq)]
-pub struct FixTable(hashbrown::HashMap<String, FixCount, rustc_hash::FxBuildHasher>);
+pub struct FixTable(hashbrown::HashMap<SecondaryCode, FixCount, rustc_hash::FxBuildHasher>);
 
 impl FixTable {
     pub fn counts(&self) -> impl Iterator<Item = usize> {
         self.0.values().map(|fc| fc.count)
     }
 
-    pub fn entry<'a>(&'a mut self, code: &'a str) -> FixTableEntry<'a> {
+    pub fn entry<'a>(&'a mut self, code: &'a SecondaryCode) -> FixTableEntry<'a> {
         FixTableEntry(self.0.entry_ref(code))
     }
 
-    pub fn iter(&self) -> impl Iterator<Item = (&str, &'static str, usize)> {
+    pub fn iter(&self) -> impl Iterator<Item = (&SecondaryCode, &'static str, usize)> {
         self.0
             .iter()
-            .map(|(code, FixCount { rule_name, count })| (code.as_str(), *rule_name, *count))
+            .map(|(code, FixCount { rule_name, count })| (code, *rule_name, *count))
     }
 
-    pub fn keys(&self) -> impl Iterator<Item = &str> {
-        self.0.keys().map(String::as_str)
+    pub fn keys(&self) -> impl Iterator<Item = &SecondaryCode> {
+        self.0.keys()
     }
 
     pub fn is_empty(&self) -> bool {
@@ -120,7 +121,7 @@ impl FixTable {
 }
 
 pub struct FixTableEntry<'a>(
-    hashbrown::hash_map::EntryRef<'a, 'a, String, str, FixCount, FxBuildHasher>,
+    hashbrown::hash_map::EntryRef<'a, 'a, SecondaryCode, SecondaryCode, FixCount, FxBuildHasher>,
 );
 
 impl<'a> FixTableEntry<'a> {
@@ -678,7 +679,10 @@ pub fn lint_fix<'a>(
     }
 }
 
-fn collect_rule_codes<'a>(rules: impl IntoIterator<Item = &'a str>) -> String {
+fn collect_rule_codes<T>(rules: impl IntoIterator<Item = T>) -> String
+where
+    T: Ord + PartialEq + std::fmt::Display,
+{
     rules.into_iter().sorted_unstable().dedup().join(", ")
 }
 
@@ -720,7 +724,7 @@ fn report_fix_syntax_error<'a>(
     path: &Path,
     transformed: &str,
     error: &ParseError,
-    rules: impl IntoIterator<Item = &'a str>,
+    rules: impl IntoIterator<Item = &'a SecondaryCode>,
 ) {
     let codes = collect_rule_codes(rules);
     if cfg!(debug_assertions) {

crates/ruff_linter/src/message/gitlab.rs

@@ -91,7 +91,7 @@ impl Serialize for SerializedMessages<'_> {
             fingerprints.insert(message_fingerprint);
 
             let (description, check_name) = if let Some(code) = diagnostic.secondary_code() {
-                (diagnostic.body().to_string(), code)
+                (diagnostic.body().to_string(), code.as_str())
             } else {
                 let description = diagnostic.body();
                 let description_without_prefix = description

crates/ruff_linter/src/message/mod.rs

@@ -25,6 +25,7 @@ pub use sarif::SarifEmitter;
 pub use text::TextEmitter;
 
 use crate::Fix;
+use crate::codes::NoqaCode;
 use crate::logging::DisplayParseErrorType;
 use crate::registry::Rule;
 use crate::{Locator, Violation};
@@ -61,7 +62,7 @@ pub struct OldDiagnostic {
     pub fix: Option<Fix>,
     pub parent: Option<TextSize>,
     pub(crate) noqa_offset: Option<TextSize>,
-    pub(crate) secondary_code: Option<String>,
+    pub(crate) secondary_code: Option<SecondaryCode>,
 }
 
 impl OldDiagnostic {
@@ -114,7 +115,7 @@ impl OldDiagnostic {
             fix,
             parent,
             noqa_offset,
-            secondary_code: Some(rule.noqa_code().to_string()),
+            secondary_code: Some(SecondaryCode(rule.noqa_code().to_string())),
         }
     }
 
@@ -247,8 +248,8 @@ impl OldDiagnostic {
     }
 
     /// Returns the noqa code for the diagnostic message as a string.
-    pub fn secondary_code(&self) -> Option<&str> {
-        self.secondary_code.as_deref()
+    pub fn secondary_code(&self) -> Option<&SecondaryCode> {
+        self.secondary_code.as_ref()
     }
 
     /// Returns the URL for the rule documentation, if it exists.
@@ -383,6 +384,76 @@ impl<'a> EmitterContext<'a> {
     }
 }
 
+/// A secondary identifier for a lint diagnostic.
+///
+/// For Ruff rules this means the noqa code.
+#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Default, Hash)]
+pub struct SecondaryCode(String);
+
+impl SecondaryCode {
+    pub fn new(code: String) -> Self {
+        Self(code)
+    }
+
+    pub fn as_str(&self) -> &str {
+        &self.0
+    }
+}
+
+impl Display for SecondaryCode {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        f.write_str(&self.0)
+    }
+}
+
+impl std::ops::Deref for SecondaryCode {
+    type Target = str;
+
+    fn deref(&self) -> &Self::Target {
+        &self.0
+    }
+}
+
+impl PartialEq<&str> for SecondaryCode {
+    fn eq(&self, other: &&str) -> bool {
+        self.0 == *other
+    }
+}
+
+impl PartialEq<SecondaryCode> for &str {
+    fn eq(&self, other: &SecondaryCode) -> bool {
+        other.eq(self)
+    }
+}
+
+impl PartialEq<NoqaCode> for SecondaryCode {
+    fn eq(&self, other: &NoqaCode) -> bool {
+        &self.as_str() == other
+    }
+}
+
+impl PartialEq<SecondaryCode> for NoqaCode {
+    fn eq(&self, other: &SecondaryCode) -> bool {
+        other.eq(self)
+    }
+}
+
+impl serde::Serialize for SecondaryCode {
+    fn serialize<S>(&self, serializer: S) -> std::result::Result<S::Ok, S::Error>
+    where
+        S: serde::Serializer,
+    {
+        serializer.serialize_str(&self.0)
+    }
+}
+
+// for `hashbrown::EntryRef`
+impl From<&SecondaryCode> for SecondaryCode {
+    fn from(value: &SecondaryCode) -> Self {
+        value.clone()
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use rustc_hash::FxHashMap;

crates/ruff_linter/src/message/sarif.rs

@@ -9,7 +9,7 @@ use ruff_source_file::OneIndexed;
 
 use crate::VERSION;
 use crate::fs::normalize_path;
-use crate::message::{Emitter, EmitterContext, OldDiagnostic};
+use crate::message::{Emitter, EmitterContext, OldDiagnostic, SecondaryCode};
 use crate::registry::{Linter, RuleNamespace};
 
 pub struct SarifEmitter;
@@ -53,15 +53,15 @@ impl Emitter for SarifEmitter {
 #[derive(Debug, Clone)]
 struct SarifRule<'a> {
     name: &'a str,
-    code: &'a str,
+    code: &'a SecondaryCode,
     linter: &'a str,
     summary: &'a str,
     explanation: Option<&'a str>,
     url: Option<String>,
 }
 
-impl<'a> From<&'a str> for SarifRule<'a> {
-    fn from(code: &'a str) -> Self {
+impl<'a> From<&'a SecondaryCode> for SarifRule<'a> {
+    fn from(code: &'a SecondaryCode) -> Self {
         // 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).unwrap();
@@ -110,7 +110,7 @@ impl Serialize for SarifRule<'_> {
 
 #[derive(Debug)]
 struct SarifResult<'a> {
-    code: Option<&'a str>,
+    code: Option<&'a SecondaryCode>,
     level: String,
     message: String,
     uri: String,

crates/ruff_linter/src/message/text.rs

@@ -14,7 +14,7 @@ use crate::Locator;
 use crate::fs::relativize_path;
 use crate::line_width::{IndentWidth, LineWidthBuilder};
 use crate::message::diff::Diff;
-use crate::message::{Emitter, EmitterContext, OldDiagnostic};
+use crate::message::{Emitter, EmitterContext, OldDiagnostic, SecondaryCode};
 use crate::settings::types::UnsafeFixes;
 
 bitflags! {
@@ -252,7 +252,11 @@ impl Display for MessageCodeFrame<'_> {
         )
         .fix_up_empty_spans_after_line_terminator();
 
-        let label = self.message.secondary_code().unwrap_or_default();
+        let label = self
+            .message
+            .secondary_code()
+            .map(SecondaryCode::as_str)
+            .unwrap_or_default();
 
         let line_start = self.notebook_index.map_or_else(
             || start_index.get(),