From 0302b9c8a06f2357a81423374c77bb06c611d33d Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Tue, 3 Jun 2025 12:28:05 -0400 Subject: [PATCH 01/18] replace one call with to_url --- crates/ruff_linter/src/message/json.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/ruff_linter/src/message/json.rs b/crates/ruff_linter/src/message/json.rs index 3492047f440b80..47ce1f70e1a21f 100644 --- a/crates/ruff_linter/src/message/json.rs +++ b/crates/ruff_linter/src/message/json.rs @@ -82,7 +82,7 @@ pub(crate) fn message_to_json_value(message: &Message, context: &EmitterContext) json!({ "code": message.to_noqa_code().map(|code| code.to_string()), - "url": message.to_rule().and_then(|rule| rule.url()), + "url": message.to_url(), "message": message.body(), "fix": fix, "cell": notebook_cell_index, From 4f71b7ecb5992c725f0f9be0aa3ef57b957aa6b5 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Tue, 3 Jun 2025 12:33:55 -0400 Subject: [PATCH 02/18] store the rule_name instead of rule in the cache --- crates/ruff/src/cache.rs | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/crates/ruff/src/cache.rs b/crates/ruff/src/cache.rs index 4c0f11392f96ba..6bbb01d57bf6ff 100644 --- a/crates/ruff/src/cache.rs +++ b/crates/ruff/src/cache.rs @@ -13,7 +13,6 @@ use itertools::Itertools; use log::{debug, error}; use rayon::iter::ParallelIterator; use rayon::iter::{IntoParallelIterator, ParallelBridge}; -use ruff_linter::codes::Rule; use rustc_hash::FxHashMap; use tempfile::NamedTempFile; @@ -356,7 +355,9 @@ impl FileCache { msg.parent, file.clone(), msg.noqa_offset, - msg.rule, + msg.rule_name + .parse() + .expect("Expected a valid rule name in the cache"), ) }) .collect() @@ -439,8 +440,8 @@ impl LintCacheData { let messages = messages .iter() - .filter_map(|msg| msg.to_rule().map(|rule| (rule, msg))) - .map(|(rule, msg)| { + .filter(|msg| !msg.is_syntax_error()) + .map(|msg| { // Make sure that all message use the same source file. assert_eq!( msg.source_file(), @@ -448,7 +449,7 @@ impl LintCacheData { "message uses a different source file" ); CacheMessage { - rule, + rule_name: msg.name().to_string(), body: msg.body().to_string(), suggestion: msg.suggestion().map(ToString::to_string), range: msg.range(), @@ -470,9 +471,9 @@ impl LintCacheData { /// On disk representation of a diagnostic message. #[derive(bincode::Decode, Debug, bincode::Encode, PartialEq)] pub(super) struct CacheMessage { - /// The rule for the cached diagnostic. + /// The kebab-case name of the rule for the cached diagnostic. #[bincode(with_serde)] - rule: Rule, + rule_name: String, /// The message body to display to the user, to explain the diagnostic. body: String, /// The message to display to the user, to explain the suggested fix. From a779e5cae5367a1300d8212f1c840264ef8e2db6 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Tue, 3 Jun 2025 12:54:02 -0400 Subject: [PATCH 03/18] avoid converting to Rule to collect rule codes, use Rule on output --- crates/ruff/src/printer.rs | 9 +++--- crates/ruff_linter/src/codes.rs | 2 +- crates/ruff_linter/src/fix/mod.rs | 47 +++++++++++++++++++------------ crates/ruff_linter/src/linter.rs | 11 ++++---- 4 files changed, 41 insertions(+), 28 deletions(-) diff --git a/crates/ruff/src/printer.rs b/crates/ruff/src/printer.rs index 2f72e2bcd592f4..f39d72442a8b5e 100644 --- a/crates/ruff/src/printer.rs +++ b/crates/ruff/src/printer.rs @@ -6,7 +6,7 @@ use anyhow::Result; use bitflags::bitflags; use colored::Colorize; use itertools::{Itertools, iterate}; -use ruff_linter::codes::NoqaCode; +use ruff_linter::codes::{NoqaCode, Rule}; use serde::Serialize; use ruff_linter::fs::relativize_path; @@ -498,12 +498,13 @@ fn print_fix_summary(writer: &mut dyn Write, fixed: &FixMap) -> Result<()> { relativize_path(filename).bold(), ":".cyan() )?; - for (rule, count) in table.iter().sorted_by_key(|(.., count)| Reverse(*count)) { + for (code, count) in table.iter().sorted_by_key(|(.., count)| Reverse(*count)) { + let code = code.to_string(); writeln!( writer, " {count:>num_digits$} × {} ({})", - rule.noqa_code().to_string().red().bold(), - rule.as_ref(), + code.red().bold(), + Rule::from_code(&code).map_or(code, |rule| rule.to_string()), )?; } } diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index 69ac2201ce5405..fafb840a0b5000 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -10,7 +10,7 @@ use crate::registry::Linter; use crate::rule_selector::is_single_rule_selector; use crate::rules; -#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] +#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] pub struct NoqaCode(&'static str, &'static str); impl NoqaCode { diff --git a/crates/ruff_linter/src/fix/mod.rs b/crates/ruff_linter/src/fix/mod.rs index ae77973ed7f99b..1f389b98677bba 100644 --- a/crates/ruff_linter/src/fix/mod.rs +++ b/crates/ruff_linter/src/fix/mod.rs @@ -7,6 +7,7 @@ use ruff_diagnostics::{IsolationLevel, SourceMap}; use ruff_text_size::{Ranged, TextLen, TextRange, TextSize}; use crate::Locator; +use crate::codes::NoqaCode; use crate::linter::FixTable; use crate::message::Message; use crate::registry::Rule; @@ -62,10 +63,10 @@ fn apply_fixes<'a>( let mut fixed = FxHashMap::default(); let mut source_map = SourceMap::default(); - for (rule, fix) in diagnostics - .filter_map(|msg| msg.to_rule().map(|rule| (rule, msg))) - .filter_map(|(rule, diagnostic)| diagnostic.fix().map(|fix| (rule, fix))) - .sorted_by(|(rule1, fix1), (rule2, fix2)| cmp_fix(*rule1, *rule2, fix1, fix2)) + for (code, fix) in diagnostics + .filter_map(|msg| msg.to_noqa_code().map(|code| (code, msg))) + .filter_map(|(code, diagnostic)| diagnostic.fix().map(|fix| (code, fix))) + .sorted_by(|(code1, fix1), (code2, fix2)| cmp_fix(*code1, *code2, fix1, fix2)) { let mut edits = fix .edits() @@ -110,7 +111,7 @@ fn apply_fixes<'a>( } applied.extend(applied_edits.drain(..)); - *fixed.entry(rule).or_default() += 1; + *fixed.entry(code).or_default() += 1; } // Add the remaining content. @@ -125,34 +126,44 @@ fn apply_fixes<'a>( } /// Compare two fixes. -fn cmp_fix(rule1: Rule, rule2: Rule, fix1: &Fix, fix2: &Fix) -> std::cmp::Ordering { +fn cmp_fix(code1: NoqaCode, code2: NoqaCode, fix1: &Fix, fix2: &Fix) -> std::cmp::Ordering { // Always apply `RedefinedWhileUnused` before `UnusedImport`, as the latter can end up fixing // the former. But we can't apply this just for `RedefinedWhileUnused` and `UnusedImport` because it violates // `< is transitive: a < b and b < c implies a < c. The same must hold for both == and >.` // See https://github.com/astral-sh/ruff/issues/12469#issuecomment-2244392085 - match (rule1, rule2) { - (Rule::RedefinedWhileUnused, Rule::RedefinedWhileUnused) => std::cmp::Ordering::Equal, - (Rule::RedefinedWhileUnused, _) => std::cmp::Ordering::Less, - (_, Rule::RedefinedWhileUnused) => std::cmp::Ordering::Greater, - _ => std::cmp::Ordering::Equal, + let redefined_while_unused = Rule::RedefinedWhileUnused.noqa_code(); + if (code1, code2) == (redefined_while_unused, redefined_while_unused) { + std::cmp::Ordering::Equal + } else if code1 == redefined_while_unused { + std::cmp::Ordering::Less + } else if code2 == redefined_while_unused { + std::cmp::Ordering::Greater + } else { + std::cmp::Ordering::Equal } // Apply fixes in order of their start position. .then_with(|| fix1.min_start().cmp(&fix2.min_start())) // Break ties in the event of overlapping rules, for some specific combinations. - .then_with(|| match (&rule1, &rule2) { + .then_with(|| { + let rules = (code1, code2); // Apply `MissingTrailingPeriod` fixes before `NewLineAfterLastParagraph` fixes. - (Rule::MissingTrailingPeriod, Rule::NewLineAfterLastParagraph) => std::cmp::Ordering::Less, - (Rule::NewLineAfterLastParagraph, Rule::MissingTrailingPeriod) => { + let missing_trailing_period = Rule::MissingTrailingPeriod.noqa_code(); + let newline_after_last_paragraph = Rule::NewLineAfterLastParagraph.noqa_code(); + let if_else_instead_of_dict_get = Rule::IfElseBlockInsteadOfDictGet.noqa_code(); + let if_else_instead_of_if_exp = Rule::IfElseBlockInsteadOfIfExp.noqa_code(); + if rules == (missing_trailing_period, newline_after_last_paragraph) { + std::cmp::Ordering::Less + } else if rules == (newline_after_last_paragraph, missing_trailing_period) { std::cmp::Ordering::Greater } // Apply `IfElseBlockInsteadOfDictGet` fixes before `IfElseBlockInsteadOfIfExp` fixes. - (Rule::IfElseBlockInsteadOfDictGet, Rule::IfElseBlockInsteadOfIfExp) => { + else if rules == (if_else_instead_of_dict_get, if_else_instead_of_if_exp) { std::cmp::Ordering::Less - } - (Rule::IfElseBlockInsteadOfIfExp, Rule::IfElseBlockInsteadOfDictGet) => { + } else if rules == (if_else_instead_of_if_exp, if_else_instead_of_dict_get) { std::cmp::Ordering::Greater + } else { + std::cmp::Ordering::Equal } - _ => std::cmp::Ordering::Equal, }) } diff --git a/crates/ruff_linter/src/linter.rs b/crates/ruff_linter/src/linter.rs index 99e35681ec6ce7..bf8a39ef88553f 100644 --- a/crates/ruff_linter/src/linter.rs +++ b/crates/ruff_linter/src/linter.rs @@ -22,6 +22,7 @@ use crate::checkers::imports::check_imports; use crate::checkers::noqa::check_noqa; use crate::checkers::physical_lines::check_physical_lines; use crate::checkers::tokens::check_tokens; +use crate::codes::NoqaCode; use crate::directives::Directives; use crate::doc_lines::{doc_lines_from_ast, doc_lines_from_tokens}; use crate::fix::{FixResult, fix_file}; @@ -84,7 +85,7 @@ impl LinterResult { } } -pub type FixTable = FxHashMap; +pub type FixTable = FxHashMap; pub struct FixerResult<'a> { /// The result returned by the linter, after applying any fixes. @@ -698,10 +699,10 @@ pub fn lint_fix<'a>( } } -fn collect_rule_codes(rules: impl IntoIterator) -> String { +fn collect_rule_codes(rules: impl IntoIterator) -> String { rules .into_iter() - .map(|rule| rule.noqa_code().to_string()) + .map(|rule| rule.to_string()) .sorted_unstable() .dedup() .join(", ") @@ -709,7 +710,7 @@ fn collect_rule_codes(rules: impl IntoIterator) -> String { #[expect(clippy::print_stderr)] fn report_failed_to_converge_error(path: &Path, transformed: &str, messages: &[Message]) { - let codes = collect_rule_codes(messages.iter().filter_map(Message::to_rule)); + let codes = collect_rule_codes(messages.iter().filter_map(Message::to_noqa_code)); if cfg!(debug_assertions) { eprintln!( "{}{} Failed to converge after {} iterations in `{}` with rule codes {}:---\n{}\n---", @@ -745,7 +746,7 @@ fn report_fix_syntax_error( path: &Path, transformed: &str, error: &ParseError, - rules: impl IntoIterator, + rules: impl IntoIterator, ) { let codes = collect_rule_codes(rules); if cfg!(debug_assertions) { From 6e2f059029296f0018dc2748b3b15a978a8a149d Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Tue, 3 Jun 2025 13:10:30 -0400 Subject: [PATCH 04/18] avoid to_rule for sarif output format --- crates/ruff_linter/src/message/sarif.rs | 28 +++++++++++++++---------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/crates/ruff_linter/src/message/sarif.rs b/crates/ruff_linter/src/message/sarif.rs index 5b5021b53162f5..c7da41fb43f75f 100644 --- a/crates/ruff_linter/src/message/sarif.rs +++ b/crates/ruff_linter/src/message/sarif.rs @@ -8,7 +8,7 @@ use serde_json::json; use ruff_source_file::OneIndexed; use crate::VERSION; -use crate::codes::Rule; +use crate::codes::NoqaCode; use crate::fs::normalize_path; use crate::message::{Emitter, EmitterContext, Message}; use crate::registry::{Linter, RuleNamespace}; @@ -27,7 +27,7 @@ impl Emitter for SarifEmitter { .map(SarifResult::from_message) .collect::>>()?; - let unique_rules: HashSet<_> = results.iter().filter_map(|result| result.rule).collect(); + let unique_rules: HashSet<_> = results.iter().filter_map(|result| result.code).collect(); let mut rules: Vec = unique_rules.into_iter().map(SarifRule::from).collect(); rules.sort_by(|a, b| a.code.cmp(&b.code)); @@ -61,13 +61,19 @@ struct SarifRule<'a> { url: Option, } -impl From for SarifRule<'_> { - fn from(rule: Rule) -> Self { - let code = rule.noqa_code().to_string(); - let (linter, _) = Linter::parse_code(&code).unwrap(); +impl From for SarifRule<'_> { + fn from(code: NoqaCode) -> Self { + 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(); + let rule = linter + .all_rules() + .find(|rule| rule.noqa_code().suffix() == suffix) + .expect("Expected a valid noqa code corresponding to a rule"); Self { name: rule.into(), - code, + code: code_str, linter: linter.name(), summary: rule.message_formats()[0], explanation: rule.explanation(), @@ -106,7 +112,7 @@ impl Serialize for SarifRule<'_> { #[derive(Debug)] struct SarifResult { - rule: Option, + code: Option, level: String, message: String, uri: String, @@ -123,7 +129,7 @@ impl SarifResult { let end_location = message.compute_end_location(); let path = normalize_path(&*message.filename()); Ok(Self { - rule: message.to_rule(), + code: message.to_noqa_code(), level: "error".to_string(), message: message.body().to_string(), uri: url::Url::from_file_path(&path) @@ -143,7 +149,7 @@ impl SarifResult { let end_location = message.compute_end_location(); let path = normalize_path(&*message.filename()); Ok(Self { - rule: message.to_rule(), + code: message.to_noqa_code(), level: "error".to_string(), message: message.body().to_string(), uri: path.display().to_string(), @@ -178,7 +184,7 @@ impl Serialize for SarifResult { } } }], - "ruleId": self.rule.map(|rule| rule.noqa_code().to_string()), + "ruleId": self.code.map(|code| code.to_string()), }) .serialize(serializer) } From c030bfada81b7edb15120aa1df5bae240c651e3f Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Tue, 3 Jun 2025 13:18:05 -0400 Subject: [PATCH 05/18] use Vec directly instead of constructing a RuleSet --- crates/ruff_linter/src/noqa.rs | 31 +++++++++++++------------------ 1 file changed, 13 insertions(+), 18 deletions(-) diff --git a/crates/ruff_linter/src/noqa.rs b/crates/ruff_linter/src/noqa.rs index a7ff0c0c45b926..bced95744a748a 100644 --- a/crates/ruff_linter/src/noqa.rs +++ b/crates/ruff_linter/src/noqa.rs @@ -18,7 +18,7 @@ use crate::Locator; use crate::codes::NoqaCode; use crate::fs::relativize_path; use crate::message::Message; -use crate::registry::{Rule, RuleSet}; +use crate::registry::Rule; use crate::rule_redirects::get_redirect_target; /// Generates an array of edits that matches the length of `messages`. @@ -780,7 +780,7 @@ fn build_noqa_edits_by_diagnostic( if let Some(noqa_edit) = generate_noqa_edit( comment.directive, comment.line, - RuleSet::from_rule(comment.rule), + vec![comment.code], locator, line_ending, ) { @@ -816,7 +816,7 @@ fn build_noqa_edits_by_line<'a>( offset, matches .into_iter() - .map(|NoqaComment { rule, .. }| rule) + .map(|NoqaComment { code, .. }| code) .collect(), locator, line_ending, @@ -829,7 +829,7 @@ fn build_noqa_edits_by_line<'a>( struct NoqaComment<'a> { line: TextSize, - rule: Rule, + code: NoqaCode, directive: Option<&'a Directive<'a>>, } @@ -845,13 +845,11 @@ fn find_noqa_comments<'a>( // Mark any non-ignored diagnostics. for message in messages { - let Some(rule) = message.to_rule() else { + let Some(code) = message.to_noqa_code() else { comments_by_line.push(None); continue; }; - let code = rule.noqa_code(); - match &exemption { FileExemption::All(_) => { // If the file is exempted, don't add any noqa directives. @@ -900,7 +898,7 @@ fn find_noqa_comments<'a>( if !codes.includes(code) { comments_by_line.push(Some(NoqaComment { line: directive_line.start(), - rule, + code, directive: Some(directive), })); } @@ -912,7 +910,7 @@ fn find_noqa_comments<'a>( // There's no existing noqa directive that suppresses the diagnostic. comments_by_line.push(Some(NoqaComment { line: locator.line_start(noqa_offset), - rule, + code, directive: None, })); } @@ -922,7 +920,7 @@ fn find_noqa_comments<'a>( struct NoqaEdit<'a> { edit_range: TextRange, - rules: RuleSet, + noqa_codes: Vec, codes: Option<&'a Codes<'a>>, line_ending: LineEnding, } @@ -941,18 +939,15 @@ impl NoqaEdit<'_> { Some(codes) => { push_codes( writer, - self.rules + self.noqa_codes .iter() - .map(|rule| rule.noqa_code().to_string()) + .map(ToString::to_string) .chain(codes.iter().map(ToString::to_string)) .sorted_unstable(), ); } None => { - push_codes( - writer, - self.rules.iter().map(|rule| rule.noqa_code().to_string()), - ); + push_codes(writer, self.noqa_codes.iter().map(ToString::to_string)); } } write!(writer, "{}", self.line_ending.as_str()).unwrap(); @@ -968,7 +963,7 @@ impl Ranged for NoqaEdit<'_> { fn generate_noqa_edit<'a>( directive: Option<&'a Directive>, offset: TextSize, - rules: RuleSet, + noqa_codes: Vec, locator: &Locator, line_ending: LineEnding, ) -> Option> { @@ -997,7 +992,7 @@ fn generate_noqa_edit<'a>( Some(NoqaEdit { edit_range, - rules, + noqa_codes, codes, line_ending, }) From de723880d604d27a1b2310f1e9f723456de0b6ba Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Tue, 3 Jun 2025 13:25:18 -0400 Subject: [PATCH 06/18] convert Rule from noqa_code in some tests --- crates/ruff_linter/src/rules/pyflakes/mod.rs | 6 ++++-- crates/ruff_linter/src/test.rs | 6 ++++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/crates/ruff_linter/src/rules/pyflakes/mod.rs b/crates/ruff_linter/src/rules/pyflakes/mod.rs index ce2162e73d7fdc..4c5a6cff56a2f1 100644 --- a/crates/ruff_linter/src/rules/pyflakes/mod.rs +++ b/crates/ruff_linter/src/rules/pyflakes/mod.rs @@ -23,7 +23,6 @@ mod tests { use crate::Locator; use crate::linter::check_path; - use crate::message::Message; use crate::registry::{Linter, Rule}; use crate::rules::isort; use crate::rules::pyflakes; @@ -776,7 +775,10 @@ mod tests { messages.sort_by_key(Ranged::start); let actual = messages .iter() - .filter_map(Message::to_rule) + .filter_map(|msg| { + msg.to_noqa_code() + .map(|code| Rule::from_code(&code.to_string()).unwrap()) + }) .collect::>(); assert_eq!(actual, expected); } diff --git a/crates/ruff_linter/src/test.rs b/crates/ruff_linter/src/test.rs index 7148206df82f0c..20f1a0df33fc99 100644 --- a/crates/ruff_linter/src/test.rs +++ b/crates/ruff_linter/src/test.rs @@ -20,6 +20,7 @@ use ruff_python_parser::{ParseError, ParseOptions}; use ruff_python_trivia::textwrap::dedent; use ruff_source_file::SourceFileBuilder; +use crate::codes::Rule; use crate::fix::{FixResult, fix_file}; use crate::linter::check_path; use crate::message::{Emitter, EmitterContext, Message, TextEmitter}; @@ -233,8 +234,9 @@ Source with applied fixes: let messages = messages .into_iter() - .filter_map(|msg| Some((msg.to_rule()?, msg))) - .map(|(rule, mut diagnostic)| { + .filter_map(|msg| Some((msg.to_noqa_code()?, msg))) + .map(|(code, mut diagnostic)| { + let rule = Rule::from_code(&code.to_string()).unwrap(); let fixable = diagnostic.fix().is_some_and(|fix| { matches!( fix.applicability(), From 2269ff9d84eb2c7fad96efda5f62cb6bff297a4a Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Tue, 3 Jun 2025 13:30:09 -0400 Subject: [PATCH 07/18] avoid to_rule in ruff_server --- crates/ruff_server/src/lint.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/crates/ruff_server/src/lint.rs b/crates/ruff_server/src/lint.rs index 9ad6be52762511..7d6eddff7b71a7 100644 --- a/crates/ruff_server/src/lint.rs +++ b/crates/ruff_server/src/lint.rs @@ -12,7 +12,7 @@ use crate::{ use ruff_diagnostics::{Applicability, Edit, Fix}; use ruff_linter::{ Locator, - codes::Rule, + codes::NoqaCode, directives::{Flags, extract_directives}, generate_noqa_edits, linter::check_path, @@ -166,9 +166,9 @@ pub(crate) fn check( messages .into_iter() .zip(noqa_edits) - .filter_map(|(message, noqa_edit)| match message.to_rule() { - Some(rule) => Some(to_lsp_diagnostic( - rule, + .filter_map(|(message, noqa_edit)| match message.to_noqa_code() { + Some(code) => Some(to_lsp_diagnostic( + code, &message, noqa_edit, &source_kind, @@ -241,7 +241,7 @@ pub(crate) fn fixes_for_diagnostics( /// Generates an LSP diagnostic with an associated cell index for the diagnostic to go in. /// If the source kind is a text document, the cell index will always be `0`. fn to_lsp_diagnostic( - rule: Rule, + code: NoqaCode, diagnostic: &Message, noqa_edit: Option, source_kind: &SourceKind, @@ -274,13 +274,13 @@ fn to_lsp_diagnostic( title: suggestion.unwrap_or(name).to_string(), noqa_edit, edits, - code: rule.noqa_code().to_string(), + code: code.to_string(), }) .ok() }) .flatten(); - let code = rule.noqa_code().to_string(); + let code = code.to_string(); let range: lsp_types::Range; let cell: usize; @@ -304,7 +304,7 @@ fn to_lsp_diagnostic( severity: Some(severity(&code)), tags: tags(&code), code: Some(lsp_types::NumberOrString::String(code)), - code_description: rule.url().and_then(|url| { + code_description: diagnostic.to_url().and_then(|url| { Some(lsp_types::CodeDescription { href: lsp_types::Url::parse(&url).ok()?, }) From 46d2e9e3fa0f786f1aa5f5948e86768765d278a1 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Tue, 3 Jun 2025 13:48:47 -0400 Subject: [PATCH 08/18] use my TODO idea for to_url --- crates/ruff_linter/src/message/mod.rs | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/crates/ruff_linter/src/message/mod.rs b/crates/ruff_linter/src/message/mod.rs index 49bd23cd1ebbf6..d9d6eb43de8eda 100644 --- a/crates/ruff_linter/src/message/mod.rs +++ b/crates/ruff_linter/src/message/mod.rs @@ -240,14 +240,15 @@ impl Message { /// Returns the URL for the rule documentation, if it exists. pub fn to_url(&self) -> Option { - // TODO(brent) Rule::url calls Rule::explanation, which calls ViolationMetadata::explain, - // which when derived (seems always to be the case?) is always `Some`, so I think it's - // pretty safe to inline the Rule::url implementation here, using `self.name()`: - // - // format!("{}/rules/{}", env!("CARGO_PKG_HOMEPAGE"), self.name()) - // - // at least in the case of diagnostics, I guess syntax errors will return None - self.to_rule().and_then(|rule| rule.url()) + if self.is_syntax_error() { + None + } else { + Some(format!( + "{}/rules/{}", + env!("CARGO_PKG_HOMEPAGE"), + self.name() + )) + } } /// Returns the filename for the message. From 43277a1536e74379d04d249777567f2c245b829c Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Tue, 3 Jun 2025 13:49:03 -0400 Subject: [PATCH 09/18] delete Message::to_rule --- crates/ruff_linter/src/message/mod.rs | 9 --------- 1 file changed, 9 deletions(-) diff --git a/crates/ruff_linter/src/message/mod.rs b/crates/ruff_linter/src/message/mod.rs index d9d6eb43de8eda..3deabefab6539a 100644 --- a/crates/ruff_linter/src/message/mod.rs +++ b/crates/ruff_linter/src/message/mod.rs @@ -224,15 +224,6 @@ impl Message { self.fix().is_some() } - /// Returns the [`Rule`] corresponding to the diagnostic message. - pub fn to_rule(&self) -> Option { - if self.is_syntax_error() { - None - } else { - Some(self.name().parse().expect("Expected a valid rule name")) - } - } - /// Returns the [`NoqaCode`] corresponding to the diagnostic message. pub fn to_noqa_code(&self) -> Option { self.noqa_code From 2e90bd19b81c04fd4031b70d224ddcaa398f8ac7 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Wed, 4 Jun 2025 09:58:00 -0400 Subject: [PATCH 10/18] restore CacheMessage::rule, parse the name on input --- crates/ruff/src/cache.rs | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/crates/ruff/src/cache.rs b/crates/ruff/src/cache.rs index 6bbb01d57bf6ff..69530ea9531c61 100644 --- a/crates/ruff/src/cache.rs +++ b/crates/ruff/src/cache.rs @@ -13,6 +13,7 @@ use itertools::Itertools; use log::{debug, error}; use rayon::iter::ParallelIterator; use rayon::iter::{IntoParallelIterator, ParallelBridge}; +use ruff_linter::codes::Rule; use rustc_hash::FxHashMap; use tempfile::NamedTempFile; @@ -355,9 +356,7 @@ impl FileCache { msg.parent, file.clone(), msg.noqa_offset, - msg.rule_name - .parse() - .expect("Expected a valid rule name in the cache"), + msg.rule, ) }) .collect() @@ -440,8 +439,11 @@ impl LintCacheData { let messages = messages .iter() - .filter(|msg| !msg.is_syntax_error()) - .map(|msg| { + // 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.name().parse().ok()?, msg))) + .map(|(rule, msg)| { // Make sure that all message use the same source file. assert_eq!( msg.source_file(), @@ -449,7 +451,7 @@ impl LintCacheData { "message uses a different source file" ); CacheMessage { - rule_name: msg.name().to_string(), + rule, body: msg.body().to_string(), suggestion: msg.suggestion().map(ToString::to_string), range: msg.range(), @@ -471,9 +473,9 @@ impl LintCacheData { /// On disk representation of a diagnostic message. #[derive(bincode::Decode, Debug, bincode::Encode, PartialEq)] pub(super) struct CacheMessage { - /// The kebab-case name of the rule for the cached diagnostic. + /// The rule for the cached diagnostic. #[bincode(with_serde)] - rule_name: String, + rule: Rule, /// The message body to display to the user, to explain the diagnostic. body: String, /// The message to display to the user, to explain the suggested fix. From 62e42d9041123c326dbd573c1597e31e5ac333b3 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Wed, 4 Jun 2025 10:01:39 -0400 Subject: [PATCH 11/18] Message::to_noqa_code -> noqa_code --- crates/ruff/src/printer.rs | 2 +- crates/ruff_linter/src/fix/mod.rs | 2 +- crates/ruff_linter/src/linter.rs | 2 +- crates/ruff_linter/src/message/azure.rs | 2 +- crates/ruff_linter/src/message/github.rs | 4 ++-- crates/ruff_linter/src/message/gitlab.rs | 2 +- crates/ruff_linter/src/message/json.rs | 2 +- crates/ruff_linter/src/message/junit.rs | 2 +- crates/ruff_linter/src/message/mod.rs | 2 +- crates/ruff_linter/src/message/pylint.rs | 2 +- crates/ruff_linter/src/message/rdjson.rs | 4 ++-- crates/ruff_linter/src/message/sarif.rs | 2 +- crates/ruff_linter/src/message/text.rs | 6 +++--- crates/ruff_linter/src/noqa.rs | 2 +- crates/ruff_linter/src/rules/pyflakes/mod.rs | 2 +- crates/ruff_linter/src/test.rs | 2 +- crates/ruff_server/src/lint.rs | 2 +- crates/ruff_wasm/src/lib.rs | 2 +- 18 files changed, 22 insertions(+), 22 deletions(-) diff --git a/crates/ruff/src/printer.rs b/crates/ruff/src/printer.rs index f39d72442a8b5e..e1cce2b1f44386 100644 --- a/crates/ruff/src/printer.rs +++ b/crates/ruff/src/printer.rs @@ -302,7 +302,7 @@ impl Printer { let statistics: Vec = diagnostics .messages .iter() - .map(|message| (message.to_noqa_code(), message)) + .map(|message| (message.noqa_code(), message)) .sorted_by_key(|(code, message)| (*code, message.fixable())) .fold( vec![], diff --git a/crates/ruff_linter/src/fix/mod.rs b/crates/ruff_linter/src/fix/mod.rs index 1f389b98677bba..c9910358c10a89 100644 --- a/crates/ruff_linter/src/fix/mod.rs +++ b/crates/ruff_linter/src/fix/mod.rs @@ -64,7 +64,7 @@ fn apply_fixes<'a>( let mut source_map = SourceMap::default(); for (code, fix) in diagnostics - .filter_map(|msg| msg.to_noqa_code().map(|code| (code, msg))) + .filter_map(|msg| msg.noqa_code().map(|code| (code, msg))) .filter_map(|(code, diagnostic)| diagnostic.fix().map(|fix| (code, fix))) .sorted_by(|(code1, fix1), (code2, fix2)| cmp_fix(*code1, *code2, fix1, fix2)) { diff --git a/crates/ruff_linter/src/linter.rs b/crates/ruff_linter/src/linter.rs index bf8a39ef88553f..f805a5189f99df 100644 --- a/crates/ruff_linter/src/linter.rs +++ b/crates/ruff_linter/src/linter.rs @@ -710,7 +710,7 @@ fn collect_rule_codes(rules: impl IntoIterator) -> String { #[expect(clippy::print_stderr)] fn report_failed_to_converge_error(path: &Path, transformed: &str, messages: &[Message]) { - let codes = collect_rule_codes(messages.iter().filter_map(Message::to_noqa_code)); + let codes = collect_rule_codes(messages.iter().filter_map(Message::noqa_code)); if cfg!(debug_assertions) { eprintln!( "{}{} Failed to converge after {} iterations in `{}` with rule codes {}:---\n{}\n---", diff --git a/crates/ruff_linter/src/message/azure.rs b/crates/ruff_linter/src/message/azure.rs index 107224d61ed4d8..1f0fe2c5b5207f 100644 --- a/crates/ruff_linter/src/message/azure.rs +++ b/crates/ruff_linter/src/message/azure.rs @@ -33,7 +33,7 @@ impl Emitter for AzureEmitter { line = location.line, col = location.column, code = message - .to_noqa_code() + .noqa_code() .map_or_else(String::new, |code| format!("code={code};")), body = message.body(), )?; diff --git a/crates/ruff_linter/src/message/github.rs b/crates/ruff_linter/src/message/github.rs index 748af7896439b5..5c31dc134cf1cf 100644 --- a/crates/ruff_linter/src/message/github.rs +++ b/crates/ruff_linter/src/message/github.rs @@ -33,7 +33,7 @@ impl Emitter for GithubEmitter { writer, "::error title=Ruff{code},file={file},line={row},col={column},endLine={end_row},endColumn={end_column}::", code = message - .to_noqa_code() + .noqa_code() .map_or_else(String::new, |code| format!(" ({code})")), file = message.filename(), row = source_location.line, @@ -50,7 +50,7 @@ impl Emitter for GithubEmitter { column = location.column, )?; - if let Some(code) = message.to_noqa_code() { + if let Some(code) = message.noqa_code() { write!(writer, " {code}")?; } diff --git a/crates/ruff_linter/src/message/gitlab.rs b/crates/ruff_linter/src/message/gitlab.rs index 3d7fd85e270c5c..b04cd86ad47013 100644 --- a/crates/ruff_linter/src/message/gitlab.rs +++ b/crates/ruff_linter/src/message/gitlab.rs @@ -90,7 +90,7 @@ impl Serialize for SerializedMessages<'_> { } fingerprints.insert(message_fingerprint); - let (description, check_name) = if let Some(code) = message.to_noqa_code() { + let (description, check_name) = if let Some(code) = message.noqa_code() { (message.body().to_string(), code.to_string()) } else { let description = message.body(); diff --git a/crates/ruff_linter/src/message/json.rs b/crates/ruff_linter/src/message/json.rs index 47ce1f70e1a21f..e116f55e96a2f4 100644 --- a/crates/ruff_linter/src/message/json.rs +++ b/crates/ruff_linter/src/message/json.rs @@ -81,7 +81,7 @@ pub(crate) fn message_to_json_value(message: &Message, context: &EmitterContext) } json!({ - "code": message.to_noqa_code().map(|code| code.to_string()), + "code": message.noqa_code().map(|code| code.to_string()), "url": message.to_url(), "message": message.body(), "fix": fix, diff --git a/crates/ruff_linter/src/message/junit.rs b/crates/ruff_linter/src/message/junit.rs index 38575d93d38b19..d0bbaca51530d6 100644 --- a/crates/ruff_linter/src/message/junit.rs +++ b/crates/ruff_linter/src/message/junit.rs @@ -59,7 +59,7 @@ impl Emitter for JunitEmitter { body = message.body() )); let mut case = TestCase::new( - if let Some(code) = message.to_noqa_code() { + if let Some(code) = message.noqa_code() { format!("org.ruff.{code}") } else { "org.ruff".to_string() diff --git a/crates/ruff_linter/src/message/mod.rs b/crates/ruff_linter/src/message/mod.rs index 3deabefab6539a..ce4f1041a6f9d8 100644 --- a/crates/ruff_linter/src/message/mod.rs +++ b/crates/ruff_linter/src/message/mod.rs @@ -225,7 +225,7 @@ impl Message { } /// Returns the [`NoqaCode`] corresponding to the diagnostic message. - pub fn to_noqa_code(&self) -> Option { + pub fn noqa_code(&self) -> Option { self.noqa_code } diff --git a/crates/ruff_linter/src/message/pylint.rs b/crates/ruff_linter/src/message/pylint.rs index 60c7a182dce1f0..a4bf41225f851f 100644 --- a/crates/ruff_linter/src/message/pylint.rs +++ b/crates/ruff_linter/src/message/pylint.rs @@ -26,7 +26,7 @@ impl Emitter for PylintEmitter { message.compute_start_location().line }; - let body = if let Some(code) = message.to_noqa_code() { + let body = if let Some(code) = message.noqa_code() { format!("[{code}] {body}", body = message.body()) } else { message.body().to_string() diff --git a/crates/ruff_linter/src/message/rdjson.rs b/crates/ruff_linter/src/message/rdjson.rs index 34c89d0bd05430..28be271ee05cca 100644 --- a/crates/ruff_linter/src/message/rdjson.rs +++ b/crates/ruff_linter/src/message/rdjson.rs @@ -71,7 +71,7 @@ fn message_to_rdjson_value(message: &Message) -> Value { "range": rdjson_range(start_location, end_location), }, "code": { - "value": message.to_noqa_code().map(|code| code.to_string()), + "value": message.noqa_code().map(|code| code.to_string()), "url": message.to_url(), }, "suggestions": rdjson_suggestions(fix.edits(), &source_code), @@ -84,7 +84,7 @@ fn message_to_rdjson_value(message: &Message) -> Value { "range": rdjson_range(start_location, end_location), }, "code": { - "value": message.to_noqa_code().map(|code| code.to_string()), + "value": message.noqa_code().map(|code| code.to_string()), "url": message.to_url(), }, }) diff --git a/crates/ruff_linter/src/message/sarif.rs b/crates/ruff_linter/src/message/sarif.rs index c7da41fb43f75f..197f9874c834e7 100644 --- a/crates/ruff_linter/src/message/sarif.rs +++ b/crates/ruff_linter/src/message/sarif.rs @@ -129,7 +129,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(), level: "error".to_string(), message: message.body().to_string(), uri: url::Url::from_file_path(&path) diff --git a/crates/ruff_linter/src/message/text.rs b/crates/ruff_linter/src/message/text.rs index 65b98d9c1bd1bf..c8f89fc49574fa 100644 --- a/crates/ruff_linter/src/message/text.rs +++ b/crates/ruff_linter/src/message/text.rs @@ -151,7 +151,7 @@ impl Display for RuleCodeAndBody<'_> { if let Some(fix) = self.message.fix() { // Do not display an indicator for inapplicable fixes if fix.applies(self.unsafe_fixes.required_applicability()) { - if let Some(code) = self.message.to_noqa_code() { + if let Some(code) = self.message.noqa_code() { write!(f, "{} ", code.to_string().red().bold())?; } return write!( @@ -164,7 +164,7 @@ impl Display for RuleCodeAndBody<'_> { } } - if let Some(code) = self.message.to_noqa_code() { + if let Some(code) = self.message.noqa_code() { write!( f, "{code} {body}", @@ -254,7 +254,7 @@ impl Display for MessageCodeFrame<'_> { let label = self .message - .to_noqa_code() + .noqa_code() .map_or_else(String::new, |code| code.to_string()); let line_start = self.notebook_index.map_or_else( diff --git a/crates/ruff_linter/src/noqa.rs b/crates/ruff_linter/src/noqa.rs index bced95744a748a..458856e7cd035b 100644 --- a/crates/ruff_linter/src/noqa.rs +++ b/crates/ruff_linter/src/noqa.rs @@ -845,7 +845,7 @@ fn find_noqa_comments<'a>( // Mark any non-ignored diagnostics. for message in messages { - let Some(code) = message.to_noqa_code() else { + let Some(code) = message.noqa_code() else { comments_by_line.push(None); continue; }; diff --git a/crates/ruff_linter/src/rules/pyflakes/mod.rs b/crates/ruff_linter/src/rules/pyflakes/mod.rs index 4c5a6cff56a2f1..ee787508795dc7 100644 --- a/crates/ruff_linter/src/rules/pyflakes/mod.rs +++ b/crates/ruff_linter/src/rules/pyflakes/mod.rs @@ -776,7 +776,7 @@ mod tests { let actual = messages .iter() .filter_map(|msg| { - msg.to_noqa_code() + msg.noqa_code() .map(|code| Rule::from_code(&code.to_string()).unwrap()) }) .collect::>(); diff --git a/crates/ruff_linter/src/test.rs b/crates/ruff_linter/src/test.rs index 20f1a0df33fc99..ed05d626732b8a 100644 --- a/crates/ruff_linter/src/test.rs +++ b/crates/ruff_linter/src/test.rs @@ -234,7 +234,7 @@ Source with applied fixes: let messages = messages .into_iter() - .filter_map(|msg| Some((msg.to_noqa_code()?, msg))) + .filter_map(|msg| Some((msg.noqa_code()?, msg))) .map(|(code, mut diagnostic)| { let rule = Rule::from_code(&code.to_string()).unwrap(); let fixable = diagnostic.fix().is_some_and(|fix| { diff --git a/crates/ruff_server/src/lint.rs b/crates/ruff_server/src/lint.rs index 7d6eddff7b71a7..4c6936816c5025 100644 --- a/crates/ruff_server/src/lint.rs +++ b/crates/ruff_server/src/lint.rs @@ -166,7 +166,7 @@ pub(crate) fn check( messages .into_iter() .zip(noqa_edits) - .filter_map(|(message, noqa_edit)| match message.to_noqa_code() { + .filter_map(|(message, noqa_edit)| match message.noqa_code() { Some(code) => Some(to_lsp_diagnostic( code, &message, diff --git a/crates/ruff_wasm/src/lib.rs b/crates/ruff_wasm/src/lib.rs index 55c339c53c41b6..a95495bcacc6b9 100644 --- a/crates/ruff_wasm/src/lib.rs +++ b/crates/ruff_wasm/src/lib.rs @@ -210,7 +210,7 @@ impl Workspace { .map(|msg| { let message = msg.body().to_string(); let range = msg.range(); - match msg.to_noqa_code() { + match msg.noqa_code() { Some(code) => ExpandedMessage { code: Some(code.to_string()), message, From 17888e533ef4d8f4bee676b008c7b92a73b1d1e5 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Wed, 4 Jun 2025 10:29:15 -0400 Subject: [PATCH 12/18] add Rule::name, remove AsRefStr impl, many pub to pub(crate) Co-authored-by: Micha Reiser --- crates/ruff/src/commands/rule.rs | 4 +-- crates/ruff_dev/src/generate_docs.rs | 4 +-- crates/ruff_dev/src/generate_rules_table.rs | 2 +- crates/ruff_linter/src/codes.rs | 2 +- crates/ruff_linter/src/registry.rs | 24 ++++++++++++------ crates/ruff_linter/src/registry/rule_set.rs | 3 +-- crates/ruff_linter/src/rule_selector.rs | 3 +-- crates/ruff_linter/src/rules/fastapi/mod.rs | 5 ++-- .../ruff_linter/src/rules/flake8_fixme/mod.rs | 2 +- .../src/rules/flake8_gettext/mod.rs | 2 +- .../ruff_linter/src/rules/flake8_raise/mod.rs | 3 +-- .../ruff_linter/src/rules/flake8_self/mod.rs | 3 +-- .../ruff_linter/src/rules/flake8_todos/mod.rs | 3 +-- .../src/rules/flake8_type_checking/mod.rs | 25 ++++++++----------- crates/ruff_linter/src/rules/numpy/mod.rs | 3 +-- crates/ruff_linter/src/rules/pydoclint/mod.rs | 9 +++---- .../ruff_linter/src/rules/tryceratops/mod.rs | 3 +-- crates/ruff_macros/src/map_codes.rs | 13 +++++----- .../src/server/api/requests/hover.rs | 2 +- crates/ruff_workspace/src/configuration.rs | 2 +- 20 files changed, 56 insertions(+), 61 deletions(-) diff --git a/crates/ruff/src/commands/rule.rs b/crates/ruff/src/commands/rule.rs index 45b071d2ea1b76..adc761b3e5160a 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 d191e826491efd..5f2309328ef0a9 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 48b1cdc2c9569d..3255f8f42b276a 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, FixAvailability::None => format!(""), }; - 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 fafb840a0b5000..c31e989a46c1b3 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/registry.rs b/crates/ruff_linter/src/registry.rs index 9b03a8b7f35e09..9351cb5de26a09 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 { - 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 + '_>> { Some(Box::new(Rule::iter().map(|rule| { let name = rule.noqa_code().to_string(); - let help = rule.as_ref().to_string(); + let help = rule.name().as_str(); 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 d83ff07712c0cc..62601d7229a1c6 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 7588aa9f64479c..74f069b976497c 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 90f52b7d66ec6d..60f18097372b44 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 d2433fc8b0495d..44071d1b7814ff 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 09d440526eb06e..cd385932ac07bc 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 d2d636b1e8e412..1780454533a881 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 a445b40bafaf14..7a3f83ddd3bbac 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 7a4129a78b6825..486c46af04bdd8 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 f5b777ec46b32f..acdb788f520669 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 7a313eac2ecdfd..9403f25016ddeb 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 af4131e4274252..6e2ff94021f612 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 21f3a01382ddc2..8d431aae67e150 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 7d1ccaf028f219..39993af6afb01a 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 { output.extend(quote! { impl #linter { - pub fn rules(&self) -> ::std::vec::IntoIter { + pub(crate) fn rules(&self) -> ::std::vec::IntoIter { match self { #prefix_into_iter_match_arms } } } @@ -182,7 +182,7 @@ pub(crate) fn map_codes(func: &ItemFn) -> syn::Result { } output.extend(quote! { impl RuleCodePrefix { - pub fn parse(linter: &Linter, code: &str) -> Result { + pub(crate) fn parse(linter: &Linter, code: &str) -> Result { use std::str::FromStr; Ok(match linter { @@ -190,7 +190,7 @@ pub(crate) fn map_codes(func: &ItemFn) -> syn::Result { }) } - pub fn rules(&self) -> ::std::vec::IntoIter { + pub(crate) fn rules(&self) -> ::std::vec::IntoIter { 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 { + pub(crate) fn rules(self: &Linter) -> ::std::vec::IntoIter { match self { #linter_rules_match_arms } @@ -385,7 +385,7 @@ fn generate_iter_impl( } impl RuleCodePrefix { - pub fn iter() -> impl Iterator { + pub(crate) fn iter() -> impl Iterator { use strum::IntoEnumIterator; let mut prefixes = Vec::new(); @@ -436,7 +436,6 @@ fn register_rules<'a>(input: impl Iterator) -> 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 6b391e15bcc436..846f3654c5cc5b 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 684a2b434dfa05..ae0d231504caaf 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); } } From 73347b7442285b73f10a5d69e8b94a3d697c8af9 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Wed, 4 Jun 2025 10:39:22 -0400 Subject: [PATCH 13/18] avoid Rule::from_code in pyflakes --- crates/ruff_linter/src/rules/pyflakes/mod.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/ruff_linter/src/rules/pyflakes/mod.rs b/crates/ruff_linter/src/rules/pyflakes/mod.rs index ee787508795dc7..51f32553950a2a 100644 --- a/crates/ruff_linter/src/rules/pyflakes/mod.rs +++ b/crates/ruff_linter/src/rules/pyflakes/mod.rs @@ -23,6 +23,7 @@ mod tests { use crate::Locator; use crate::linter::check_path; + use crate::message::Message; use crate::registry::{Linter, Rule}; use crate::rules::isort; use crate::rules::pyflakes; @@ -775,11 +776,10 @@ mod tests { messages.sort_by_key(Ranged::start); let actual = messages .iter() - .filter_map(|msg| { - msg.noqa_code() - .map(|code| Rule::from_code(&code.to_string()).unwrap()) - }) + .filter(|msg| !msg.is_syntax_error()) + .map(Message::name) .collect::>(); + let expected: Vec<_> = expected.iter().map(|rule| rule.name().as_str()).collect(); assert_eq!(actual, expected); } From 0c22912a1da98199225a8d4c2a444754fe7945c1 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Wed, 4 Jun 2025 13:12:32 -0400 Subject: [PATCH 14/18] make FixTable a struct and store the rule name along with the code --- crates/ruff/src/diagnostics.rs | 15 +++++----- crates/ruff/src/printer.rs | 19 ++++++------ crates/ruff_linter/src/fix/mod.rs | 48 +++++++++++++++---------------- crates/ruff_linter/src/linter.rs | 34 +++++++++++++++++++--- 4 files changed, 71 insertions(+), 45 deletions(-) diff --git a/crates/ruff/src/diagnostics.rs b/crates/ruff/src/diagnostics.rs index d562c009b1ffd1..b5e757f9d5c060 100644 --- a/crates/ruff/src/diagnostics.rs +++ b/crates/ruff/src/diagnostics.rs @@ -165,9 +165,10 @@ impl AddAssign for FixMap { continue; } let fixed_in_file = self.0.entry(filename).or_default(); - for (rule, count) in fixed { - if count > 0 { - *fixed_in_file.entry(rule).or_default() += count; + for (rule, (name, count)) in &*fixed { + if *count > 0 { + let (_name, entry) = fixed_in_file.entry(*rule).or_insert((name, 0)); + *entry += count; } } } @@ -305,7 +306,7 @@ pub(crate) fn lint_path( ParseSource::None, ); let transformed = source_kind; - let fixed = FxHashMap::default(); + let fixed = FixTable::default(); (result, transformed, fixed) } } else { @@ -319,7 +320,7 @@ pub(crate) fn lint_path( ParseSource::None, ); let transformed = source_kind; - let fixed = FxHashMap::default(); + let fixed = FixTable::default(); (result, transformed, fixed) }; @@ -473,7 +474,7 @@ pub(crate) fn lint_stdin( } let transformed = source_kind; - let fixed = FxHashMap::default(); + let fixed = FixTable::default(); (result, transformed, fixed) } } else { @@ -487,7 +488,7 @@ pub(crate) fn lint_stdin( ParseSource::None, ); let transformed = source_kind; - let fixed = FxHashMap::default(); + let fixed = FixTable::default(); (result, transformed, fixed) }; diff --git a/crates/ruff/src/printer.rs b/crates/ruff/src/printer.rs index e1cce2b1f44386..6c776c58f9d8a0 100644 --- a/crates/ruff/src/printer.rs +++ b/crates/ruff/src/printer.rs @@ -6,7 +6,8 @@ use anyhow::Result; use bitflags::bitflags; use colored::Colorize; use itertools::{Itertools, iterate}; -use ruff_linter::codes::{NoqaCode, Rule}; +use ruff_linter::codes::NoqaCode; +use ruff_linter::linter::FixTable; use serde::Serialize; use ruff_linter::fs::relativize_path; @@ -80,7 +81,7 @@ impl Printer { let fixed = diagnostics .fixed .values() - .flat_map(std::collections::HashMap::values) + .flat_map(FixTable::counts) .sum::(); if self.flags.intersects(Flags::SHOW_VIOLATIONS) { @@ -472,13 +473,13 @@ fn show_fix_status(fix_mode: flags::FixMode, fixables: Option<&FixableStatistics fn print_fix_summary(writer: &mut dyn Write, fixed: &FixMap) -> Result<()> { let total = fixed .values() - .map(|table| table.values().sum::()) + .map(|table| table.counts().sum::()) .sum::(); assert!(total > 0); let num_digits = num_digits( - *fixed + fixed .values() - .filter_map(|table| table.values().max()) + .filter_map(|table| table.counts().max()) .max() .unwrap(), ); @@ -498,13 +499,11 @@ fn print_fix_summary(writer: &mut dyn Write, fixed: &FixMap) -> Result<()> { relativize_path(filename).bold(), ":".cyan() )?; - for (code, count) in table.iter().sorted_by_key(|(.., count)| Reverse(*count)) { - let code = code.to_string(); + for (code, (name, count)) in table.iter().sorted_by_key(|(.., count)| Reverse(*count)) { writeln!( writer, - " {count:>num_digits$} × {} ({})", - code.red().bold(), - Rule::from_code(&code).map_or(code, |rule| rule.to_string()), + " {count:>num_digits$} × {code} ({name})", + code = code.to_string().red().bold(), )?; } } diff --git a/crates/ruff_linter/src/fix/mod.rs b/crates/ruff_linter/src/fix/mod.rs index c9910358c10a89..5afdb4c7c72615 100644 --- a/crates/ruff_linter/src/fix/mod.rs +++ b/crates/ruff_linter/src/fix/mod.rs @@ -1,13 +1,12 @@ use std::collections::BTreeSet; use itertools::Itertools; -use rustc_hash::{FxHashMap, FxHashSet}; +use rustc_hash::FxHashSet; use ruff_diagnostics::{IsolationLevel, SourceMap}; use ruff_text_size::{Ranged, TextLen, TextRange, TextSize}; use crate::Locator; -use crate::codes::NoqaCode; use crate::linter::FixTable; use crate::message::Message; use crate::registry::Rule; @@ -60,13 +59,13 @@ fn apply_fixes<'a>( let mut last_pos: Option = None; let mut applied: BTreeSet<&Edit> = BTreeSet::default(); let mut isolated: FxHashSet = FxHashSet::default(); - let mut fixed = FxHashMap::default(); + let mut fixed = FixTable::default(); let mut source_map = SourceMap::default(); - for (code, fix) in diagnostics - .filter_map(|msg| msg.noqa_code().map(|code| (code, msg))) - .filter_map(|(code, diagnostic)| diagnostic.fix().map(|fix| (code, fix))) - .sorted_by(|(code1, fix1), (code2, fix2)| cmp_fix(*code1, *code2, fix1, fix2)) + for (code, name, fix) in diagnostics + .filter_map(|msg| msg.noqa_code().map(|code| (code, msg.name(), msg))) + .filter_map(|(code, name, diagnostic)| diagnostic.fix().map(|fix| (code, name, fix))) + .sorted_by(|(_, name1, fix1), (_, name2, fix2)| cmp_fix(name1, name2, fix1, fix2)) { let mut edits = fix .edits() @@ -111,7 +110,8 @@ fn apply_fixes<'a>( } applied.extend(applied_edits.drain(..)); - *fixed.entry(code).or_default() += 1; + let (_name, count) = fixed.entry(code).or_insert((name, 0)); + *count += 1; } // Add the remaining content. @@ -126,17 +126,17 @@ fn apply_fixes<'a>( } /// Compare two fixes. -fn cmp_fix(code1: NoqaCode, code2: NoqaCode, fix1: &Fix, fix2: &Fix) -> std::cmp::Ordering { +fn cmp_fix(name1: &str, name2: &str, fix1: &Fix, fix2: &Fix) -> std::cmp::Ordering { // Always apply `RedefinedWhileUnused` before `UnusedImport`, as the latter can end up fixing // the former. But we can't apply this just for `RedefinedWhileUnused` and `UnusedImport` because it violates // `< is transitive: a < b and b < c implies a < c. The same must hold for both == and >.` // See https://github.com/astral-sh/ruff/issues/12469#issuecomment-2244392085 - let redefined_while_unused = Rule::RedefinedWhileUnused.noqa_code(); - if (code1, code2) == (redefined_while_unused, redefined_while_unused) { + let redefined_while_unused = Rule::RedefinedWhileUnused.name().as_str(); + if (name1, name2) == (redefined_while_unused, redefined_while_unused) { std::cmp::Ordering::Equal - } else if code1 == redefined_while_unused { + } else if name1 == redefined_while_unused { std::cmp::Ordering::Less - } else if code2 == redefined_while_unused { + } else if name2 == redefined_while_unused { std::cmp::Ordering::Greater } else { std::cmp::Ordering::Equal @@ -145,12 +145,12 @@ fn cmp_fix(code1: NoqaCode, code2: NoqaCode, fix1: &Fix, fix2: &Fix) -> std::cmp .then_with(|| fix1.min_start().cmp(&fix2.min_start())) // Break ties in the event of overlapping rules, for some specific combinations. .then_with(|| { - let rules = (code1, code2); + let rules = (name1, name2); // Apply `MissingTrailingPeriod` fixes before `NewLineAfterLastParagraph` fixes. - let missing_trailing_period = Rule::MissingTrailingPeriod.noqa_code(); - let newline_after_last_paragraph = Rule::NewLineAfterLastParagraph.noqa_code(); - let if_else_instead_of_dict_get = Rule::IfElseBlockInsteadOfDictGet.noqa_code(); - let if_else_instead_of_if_exp = Rule::IfElseBlockInsteadOfIfExp.noqa_code(); + let missing_trailing_period = Rule::MissingTrailingPeriod.name().as_str(); + let newline_after_last_paragraph = Rule::NewLineAfterLastParagraph.name().as_str(); + let if_else_instead_of_dict_get = Rule::IfElseBlockInsteadOfDictGet.name().as_str(); + let if_else_instead_of_if_exp = Rule::IfElseBlockInsteadOfIfExp.name().as_str(); if rules == (missing_trailing_period, newline_after_last_paragraph) { std::cmp::Ordering::Less } else if rules == (newline_after_last_paragraph, missing_trailing_period) { @@ -208,7 +208,7 @@ mod tests { source_map, } = apply_fixes(diagnostics.iter(), &locator); assert_eq!(code, ""); - assert_eq!(fixes.values().sum::(), 0); + assert_eq!(fixes.counts().sum::(), 0); assert!(source_map.markers().is_empty()); } @@ -245,7 +245,7 @@ print("hello world") "# .trim() ); - assert_eq!(fixes.values().sum::(), 1); + assert_eq!(fixes.counts().sum::(), 1); assert_eq!( source_map.markers(), &[ @@ -286,7 +286,7 @@ class A(Bar): " .trim(), ); - assert_eq!(fixes.values().sum::(), 1); + assert_eq!(fixes.counts().sum::(), 1); assert_eq!( source_map.markers(), &[ @@ -323,7 +323,7 @@ class A: " .trim() ); - assert_eq!(fixes.values().sum::(), 1); + assert_eq!(fixes.counts().sum::(), 1); assert_eq!( source_map.markers(), &[ @@ -364,7 +364,7 @@ class A(object): " .trim() ); - assert_eq!(fixes.values().sum::(), 2); + assert_eq!(fixes.counts().sum::(), 2); assert_eq!( source_map.markers(), &[ @@ -406,7 +406,7 @@ class A: " .trim(), ); - assert_eq!(fixes.values().sum::(), 1); + assert_eq!(fixes.counts().sum::(), 1); assert_eq!( source_map.markers(), &[ diff --git a/crates/ruff_linter/src/linter.rs b/crates/ruff_linter/src/linter.rs index f805a5189f99df..22618a0fc3457d 100644 --- a/crates/ruff_linter/src/linter.rs +++ b/crates/ruff_linter/src/linter.rs @@ -1,4 +1,5 @@ use std::borrow::Cow; +use std::ops::{Deref, DerefMut}; use std::path::Path; use anyhow::{Result, anyhow}; @@ -85,7 +86,31 @@ impl LinterResult { } } -pub type FixTable = FxHashMap; +type FixTableInner = FxHashMap; + +/// A mapping from a noqa code to the corresponding lint name and a count of applied fixes. +#[derive(Debug, Default, PartialEq)] +pub struct FixTable(FixTableInner); + +impl FixTable { + pub fn counts(&self) -> impl Iterator { + self.0.values().map(|(_name, count)| *count) + } +} + +impl Deref for FixTable { + type Target = FixTableInner; + + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +impl DerefMut for FixTable { + fn deref_mut(&mut self) -> &mut Self::Target { + &mut self.0 + } +} pub struct FixerResult<'a> { /// The result returned by the linter, after applying any fixes. @@ -582,7 +607,7 @@ pub fn lint_fix<'a>( let mut transformed = Cow::Borrowed(source_kind); // Track the number of fixed errors across iterations. - let mut fixed = FxHashMap::default(); + let mut fixed = FixTable::default(); // As an escape hatch, bail after 100 iterations. let mut iterations = 0; @@ -671,8 +696,9 @@ pub fn lint_fix<'a>( { if iterations < MAX_ITERATIONS { // Count the number of fixed errors. - for (rule, count) in applied { - *fixed.entry(rule).or_default() += count; + for (rule, (name, applied_count)) in &*applied { + let (_name, count) = fixed.entry(*rule).or_insert((*name, 0)); + *count += applied_count; } transformed = Cow::Owned(transformed.updated(fixed_contents, &source_map)); From 96a6ba214f497a6eed535a9cee88f2e12932252b Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Wed, 4 Jun 2025 14:25:28 -0400 Subject: [PATCH 15/18] use HashSet instead of Vec for noqa_codes, closer to RuleSet in case of duplicates --- crates/ruff_linter/src/noqa.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/crates/ruff_linter/src/noqa.rs b/crates/ruff_linter/src/noqa.rs index 458856e7cd035b..3d7ada3a110488 100644 --- a/crates/ruff_linter/src/noqa.rs +++ b/crates/ruff_linter/src/noqa.rs @@ -12,6 +12,7 @@ use log::warn; use ruff_python_trivia::{CommentRanges, Cursor, indentation_at_offset}; use ruff_source_file::{LineEnding, LineRanges}; use ruff_text_size::{Ranged, TextLen, TextRange, TextSize}; +use rustc_hash::FxHashSet; use crate::Edit; use crate::Locator; @@ -780,7 +781,7 @@ fn build_noqa_edits_by_diagnostic( if let Some(noqa_edit) = generate_noqa_edit( comment.directive, comment.line, - vec![comment.code], + FxHashSet::from_iter([comment.code]), locator, line_ending, ) { @@ -920,7 +921,7 @@ fn find_noqa_comments<'a>( struct NoqaEdit<'a> { edit_range: TextRange, - noqa_codes: Vec, + noqa_codes: FxHashSet, codes: Option<&'a Codes<'a>>, line_ending: LineEnding, } @@ -963,7 +964,7 @@ impl Ranged for NoqaEdit<'_> { fn generate_noqa_edit<'a>( directive: Option<&'a Directive>, offset: TextSize, - noqa_codes: Vec, + noqa_codes: FxHashSet, locator: &Locator, line_ending: LineEnding, ) -> Option> { From 71e658ab0af7d04506803a7b25cc31a78b843acd Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Wed, 4 Jun 2025 15:29:09 -0400 Subject: [PATCH 16/18] fix wasm build --- crates/ruff_linter/src/message/sarif.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/ruff_linter/src/message/sarif.rs b/crates/ruff_linter/src/message/sarif.rs index 197f9874c834e7..b14a93497fa02a 100644 --- a/crates/ruff_linter/src/message/sarif.rs +++ b/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(), level: "error".to_string(), message: message.body().to_string(), uri: path.display().to_string(), From 51ab83062975aa25552eb6fd8d01102fa3a5e37d Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Thu, 5 Jun 2025 08:56:59 -0400 Subject: [PATCH 17/18] fix count destructuring --- crates/ruff/src/printer.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/crates/ruff/src/printer.rs b/crates/ruff/src/printer.rs index 6c776c58f9d8a0..c1e18512ae2e01 100644 --- a/crates/ruff/src/printer.rs +++ b/crates/ruff/src/printer.rs @@ -499,7 +499,10 @@ fn print_fix_summary(writer: &mut dyn Write, fixed: &FixMap) -> Result<()> { relativize_path(filename).bold(), ":".cyan() )?; - for (code, (name, count)) in table.iter().sorted_by_key(|(.., count)| Reverse(*count)) { + for (code, (name, count)) in table + .iter() + .sorted_by_key(|(.., (.., count))| Reverse(*count)) + { writeln!( writer, " {count:>num_digits$} × {code} ({name})", From 13a586c640a1ba5830d427cf9fcb4f5b6062e664 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Thu, 5 Jun 2025 12:05:48 -0400 Subject: [PATCH 18/18] improve FixTable interface --- crates/ruff/src/diagnostics.rs | 7 ++-- crates/ruff/src/printer.rs | 5 +-- crates/ruff_linter/src/fix/mod.rs | 3 +- crates/ruff_linter/src/linter.rs | 58 ++++++++++++++++++++----------- 4 files changed, 42 insertions(+), 31 deletions(-) diff --git a/crates/ruff/src/diagnostics.rs b/crates/ruff/src/diagnostics.rs index b5e757f9d5c060..dff416bc552fee 100644 --- a/crates/ruff/src/diagnostics.rs +++ b/crates/ruff/src/diagnostics.rs @@ -165,10 +165,9 @@ impl AddAssign for FixMap { continue; } let fixed_in_file = self.0.entry(filename).or_default(); - for (rule, (name, count)) in &*fixed { - if *count > 0 { - let (_name, entry) = fixed_in_file.entry(*rule).or_insert((name, 0)); - *entry += count; + for (rule, name, count) in fixed.iter() { + if count > 0 { + *fixed_in_file.entry(rule).or_default(name) += count; } } } diff --git a/crates/ruff/src/printer.rs b/crates/ruff/src/printer.rs index c1e18512ae2e01..38530d18f4354b 100644 --- a/crates/ruff/src/printer.rs +++ b/crates/ruff/src/printer.rs @@ -499,10 +499,7 @@ fn print_fix_summary(writer: &mut dyn Write, fixed: &FixMap) -> Result<()> { relativize_path(filename).bold(), ":".cyan() )?; - for (code, (name, count)) in table - .iter() - .sorted_by_key(|(.., (.., count))| Reverse(*count)) - { + for (code, name, count) in table.iter().sorted_by_key(|(.., count)| Reverse(*count)) { writeln!( writer, " {count:>num_digits$} × {code} ({name})", diff --git a/crates/ruff_linter/src/fix/mod.rs b/crates/ruff_linter/src/fix/mod.rs index 5afdb4c7c72615..93541a24f660c7 100644 --- a/crates/ruff_linter/src/fix/mod.rs +++ b/crates/ruff_linter/src/fix/mod.rs @@ -110,8 +110,7 @@ fn apply_fixes<'a>( } applied.extend(applied_edits.drain(..)); - let (_name, count) = fixed.entry(code).or_insert((name, 0)); - *count += 1; + *fixed.entry(code).or_default(name) += 1; } // Add the remaining content. diff --git a/crates/ruff_linter/src/linter.rs b/crates/ruff_linter/src/linter.rs index 22618a0fc3457d..2b31495b2a9ca0 100644 --- a/crates/ruff_linter/src/linter.rs +++ b/crates/ruff_linter/src/linter.rs @@ -1,5 +1,5 @@ use std::borrow::Cow; -use std::ops::{Deref, DerefMut}; +use std::collections::hash_map::Entry; use std::path::Path; use anyhow::{Result, anyhow}; @@ -86,29 +86,51 @@ impl LinterResult { } } -type FixTableInner = FxHashMap; +#[derive(Debug, Default, PartialEq)] +struct FixCount { + rule_name: &'static str, + count: usize, +} /// A mapping from a noqa code to the corresponding lint name and a count of applied fixes. #[derive(Debug, Default, PartialEq)] -pub struct FixTable(FixTableInner); +pub struct FixTable(FxHashMap); impl FixTable { pub fn counts(&self) -> impl Iterator { - self.0.values().map(|(_name, count)| *count) + self.0.values().map(|fc| fc.count) } -} -impl Deref for FixTable { - type Target = FixTableInner; + pub fn entry(&mut self, code: NoqaCode) -> FixTableEntry { + FixTableEntry(self.0.entry(code)) + } - fn deref(&self) -> &Self::Target { - &self.0 + pub fn iter(&self) -> impl Iterator { + self.0 + .iter() + .map(|(code, FixCount { rule_name, count })| (*code, *rule_name, *count)) + } + + pub fn keys(&self) -> impl Iterator { + self.0.keys().copied() + } + + pub fn is_empty(&self) -> bool { + self.0.is_empty() } } -impl DerefMut for FixTable { - fn deref_mut(&mut self) -> &mut Self::Target { - &mut self.0 +pub struct FixTableEntry<'a>(Entry<'a, NoqaCode, FixCount>); + +impl<'a> FixTableEntry<'a> { + pub fn or_default(self, rule_name: &'static str) -> &'a mut usize { + &mut (self + .0 + .or_insert(FixCount { + rule_name, + count: 0, + }) + .count) } } @@ -676,12 +698,7 @@ pub fn lint_fix<'a>( // syntax error. Return the original code. if has_valid_syntax && has_no_syntax_errors { if let Some(error) = parsed.errors().first() { - report_fix_syntax_error( - path, - transformed.source_code(), - error, - fixed.keys().copied(), - ); + report_fix_syntax_error(path, transformed.source_code(), error, fixed.keys()); return Err(anyhow!("Fix introduced a syntax error")); } } @@ -696,9 +713,8 @@ pub fn lint_fix<'a>( { if iterations < MAX_ITERATIONS { // Count the number of fixed errors. - for (rule, (name, applied_count)) in &*applied { - let (_name, count) = fixed.entry(*rule).or_insert((*name, 0)); - *count += applied_count; + for (rule, name, count) in applied.iter() { + *fixed.entry(rule).or_default(name) += count; } transformed = Cow::Owned(transformed.updated(fixed_contents, &source_map));