diff --git a/crates/ruff/src/cache.rs b/crates/ruff/src/cache.rs index 4c0f11392f..69530ea953 100644 --- a/crates/ruff/src/cache.rs +++ b/crates/ruff/src/cache.rs @@ -439,7 +439,10 @@ impl LintCacheData { let messages = messages .iter() - .filter_map(|msg| msg.to_rule().map(|rule| (rule, 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!( diff --git a/crates/ruff/src/commands/rule.rs b/crates/ruff/src/commands/rule.rs index 45b071d2ea..adc761b3e5 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/src/diagnostics.rs b/crates/ruff/src/diagnostics.rs index d562c009b1..dff416bc55 100644 --- a/crates/ruff/src/diagnostics.rs +++ b/crates/ruff/src/diagnostics.rs @@ -165,9 +165,9 @@ impl AddAssign for FixMap { continue; } let fixed_in_file = self.0.entry(filename).or_default(); - for (rule, count) in fixed { + for (rule, name, count) in fixed.iter() { if count > 0 { - *fixed_in_file.entry(rule).or_default() += count; + *fixed_in_file.entry(rule).or_default(name) += count; } } } @@ -305,7 +305,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 +319,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 +473,7 @@ pub(crate) fn lint_stdin( } let transformed = source_kind; - let fixed = FxHashMap::default(); + let fixed = FixTable::default(); (result, transformed, fixed) } } else { @@ -487,7 +487,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 2f72e2bcd5..38530d18f4 100644 --- a/crates/ruff/src/printer.rs +++ b/crates/ruff/src/printer.rs @@ -7,6 +7,7 @@ use bitflags::bitflags; use colored::Colorize; use itertools::{Itertools, iterate}; 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) { @@ -302,7 +303,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![], @@ -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,12 +499,11 @@ 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, name, count) in table.iter().sorted_by_key(|(.., count)| Reverse(*count)) { writeln!( writer, - " {count:>num_digits$} × {} ({})", - rule.noqa_code().to_string().red().bold(), - rule.as_ref(), + " {count:>num_digits$} × {code} ({name})", + code = code.to_string().red().bold(), )?; } } diff --git a/crates/ruff_dev/src/generate_docs.rs b/crates/ruff_dev/src/generate_docs.rs index d191e82649..5f2309328e 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 48b1cdc2c9..3255f8f42b 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 69ac2201ce..c31e989a46 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -4,13 +4,13 @@ /// `--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; 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 ae77973ed7..93541a24f6 100644 --- a/crates/ruff_linter/src/fix/mod.rs +++ b/crates/ruff_linter/src/fix/mod.rs @@ -1,7 +1,7 @@ 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}; @@ -59,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 (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, 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() @@ -110,7 +110,7 @@ fn apply_fixes<'a>( } applied.extend(applied_edits.drain(..)); - *fixed.entry(rule).or_default() += 1; + *fixed.entry(code).or_default(name) += 1; } // Add the remaining content. @@ -125,34 +125,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(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 - 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.name().as_str(); + if (name1, name2) == (redefined_while_unused, redefined_while_unused) { + std::cmp::Ordering::Equal + } else if name1 == redefined_while_unused { + std::cmp::Ordering::Less + } else if name2 == 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 = (name1, name2); // Apply `MissingTrailingPeriod` fixes before `NewLineAfterLastParagraph` fixes. - (Rule::MissingTrailingPeriod, Rule::NewLineAfterLastParagraph) => std::cmp::Ordering::Less, - (Rule::NewLineAfterLastParagraph, Rule::MissingTrailingPeriod) => { + 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) { 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, }) } @@ -197,7 +207,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()); } @@ -234,7 +244,7 @@ print("hello world") "# .trim() ); - assert_eq!(fixes.values().sum::(), 1); + assert_eq!(fixes.counts().sum::(), 1); assert_eq!( source_map.markers(), &[ @@ -275,7 +285,7 @@ class A(Bar): " .trim(), ); - assert_eq!(fixes.values().sum::(), 1); + assert_eq!(fixes.counts().sum::(), 1); assert_eq!( source_map.markers(), &[ @@ -312,7 +322,7 @@ class A: " .trim() ); - assert_eq!(fixes.values().sum::(), 1); + assert_eq!(fixes.counts().sum::(), 1); assert_eq!( source_map.markers(), &[ @@ -353,7 +363,7 @@ class A(object): " .trim() ); - assert_eq!(fixes.values().sum::(), 2); + assert_eq!(fixes.counts().sum::(), 2); assert_eq!( source_map.markers(), &[ @@ -395,7 +405,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 99e35681ec..2b31495b2a 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::collections::hash_map::Entry; use std::path::Path; use anyhow::{Result, anyhow}; @@ -22,6 +23,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 +86,53 @@ impl LinterResult { } } -pub type FixTable = 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(FxHashMap); + +impl FixTable { + pub fn counts(&self) -> impl Iterator { + self.0.values().map(|fc| fc.count) + } + + pub fn entry(&mut self, code: NoqaCode) -> FixTableEntry { + FixTableEntry(self.0.entry(code)) + } + + 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() + } +} + +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) + } +} pub struct FixerResult<'a> { /// The result returned by the linter, after applying any fixes. @@ -581,7 +629,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; @@ -650,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")); } } @@ -670,8 +713,8 @@ 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, count) in applied.iter() { + *fixed.entry(rule).or_default(name) += count; } transformed = Cow::Owned(transformed.updated(fixed_contents, &source_map)); @@ -698,10 +741,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 +752,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::noqa_code)); if cfg!(debug_assertions) { eprintln!( "{}{} Failed to converge after {} iterations in `{}` with rule codes {}:---\n{}\n---", @@ -745,7 +788,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) { diff --git a/crates/ruff_linter/src/message/azure.rs b/crates/ruff_linter/src/message/azure.rs index 107224d61e..1f0fe2c5b5 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 748af78964..5c31dc134c 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 3d7fd85e27..b04cd86ad4 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 3492047f44..e116f55e96 100644 --- a/crates/ruff_linter/src/message/json.rs +++ b/crates/ruff_linter/src/message/json.rs @@ -81,8 +81,8 @@ 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()), + "code": message.noqa_code().map(|code| code.to_string()), + "url": message.to_url(), "message": message.body(), "fix": fix, "cell": notebook_cell_index, diff --git a/crates/ruff_linter/src/message/junit.rs b/crates/ruff_linter/src/message/junit.rs index 38575d93d3..d0bbaca515 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 49bd23cd1e..ce4f1041a6 100644 --- a/crates/ruff_linter/src/message/mod.rs +++ b/crates/ruff_linter/src/message/mod.rs @@ -224,30 +224,22 @@ 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 { + pub fn noqa_code(&self) -> Option { self.noqa_code } /// 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. diff --git a/crates/ruff_linter/src/message/pylint.rs b/crates/ruff_linter/src/message/pylint.rs index 60c7a182dc..a4bf41225f 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 34c89d0bd0..28be271ee0 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 5b5021b531..b14a93497f 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.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.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) } diff --git a/crates/ruff_linter/src/message/text.rs b/crates/ruff_linter/src/message/text.rs index 65b98d9c1b..c8f89fc495 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 a7ff0c0c45..3d7ada3a11 100644 --- a/crates/ruff_linter/src/noqa.rs +++ b/crates/ruff_linter/src/noqa.rs @@ -12,13 +12,14 @@ 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; 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 +781,7 @@ fn build_noqa_edits_by_diagnostic( if let Some(noqa_edit) = generate_noqa_edit( comment.directive, comment.line, - RuleSet::from_rule(comment.rule), + FxHashSet::from_iter([comment.code]), locator, line_ending, ) { @@ -816,7 +817,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 +830,7 @@ fn build_noqa_edits_by_line<'a>( struct NoqaComment<'a> { line: TextSize, - rule: Rule, + code: NoqaCode, directive: Option<&'a Directive<'a>>, } @@ -845,13 +846,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.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 +899,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 +911,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 +921,7 @@ fn find_noqa_comments<'a>( struct NoqaEdit<'a> { edit_range: TextRange, - rules: RuleSet, + noqa_codes: FxHashSet, codes: Option<&'a Codes<'a>>, line_ending: LineEnding, } @@ -941,18 +940,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 +964,7 @@ impl Ranged for NoqaEdit<'_> { fn generate_noqa_edit<'a>( directive: Option<&'a Directive>, offset: TextSize, - rules: RuleSet, + noqa_codes: FxHashSet, locator: &Locator, line_ending: LineEnding, ) -> Option> { @@ -997,7 +993,7 @@ fn generate_noqa_edit<'a>( Some(NoqaEdit { edit_range, - rules, + noqa_codes, codes, line_ending, }) diff --git a/crates/ruff_linter/src/registry.rs b/crates/ruff_linter/src/registry.rs index 9b03a8b7f3..9351cb5de2 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 d83ff07712..62601d7229 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 7588aa9f64..74f069b976 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 90f52b7d66..60f1809737 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 d2433fc8b0..44071d1b78 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 09d440526e..cd385932ac 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 d2d636b1e8..1780454533 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 a445b40baf..7a3f83ddd3 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 7a4129a78b..486c46af04 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 f5b777ec46..acdb788f52 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 7a313eac2e..9403f25016 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 af4131e427..6e2ff94021 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/pyflakes/mod.rs b/crates/ruff_linter/src/rules/pyflakes/mod.rs index ce2162e73d..51f3255395 100644 --- a/crates/ruff_linter/src/rules/pyflakes/mod.rs +++ b/crates/ruff_linter/src/rules/pyflakes/mod.rs @@ -776,8 +776,10 @@ mod tests { messages.sort_by_key(Ranged::start); let actual = messages .iter() - .filter_map(Message::to_rule) + .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); } diff --git a/crates/ruff_linter/src/rules/tryceratops/mod.rs b/crates/ruff_linter/src/rules/tryceratops/mod.rs index 21f3a01382..8d431aae67 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_linter/src/test.rs b/crates/ruff_linter/src/test.rs index 7148206df8..ed05d62673 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.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(), diff --git a/crates/ruff_macros/src/map_codes.rs b/crates/ruff_macros/src/map_codes.rs index 7d1ccaf028..39993af6af 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/lint.rs b/crates/ruff_server/src/lint.rs index bf20172cba..92c11068de 100644 --- a/crates/ruff_server/src/lint.rs +++ b/crates/ruff_server/src/lint.rs @@ -242,7 +242,7 @@ fn to_lsp_diagnostic( let body = diagnostic.body().to_string(); let fix = diagnostic.fix(); let suggestion = diagnostic.suggestion(); - let code = diagnostic.to_noqa_code(); + let code = diagnostic.noqa_code(); let fix = fix.and_then(|fix| fix.applies(Applicability::Unsafe).then_some(fix)); diff --git a/crates/ruff_server/src/server/api/requests/hover.rs b/crates/ruff_server/src/server/api/requests/hover.rs index 6b391e15bc..846f3654c5 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_wasm/src/lib.rs b/crates/ruff_wasm/src/lib.rs index 24e683042b..3548882503 100644 --- a/crates/ruff_wasm/src/lib.rs +++ b/crates/ruff_wasm/src/lib.rs @@ -208,7 +208,7 @@ impl Workspace { let messages: Vec = messages .into_iter() .map(|msg| ExpandedMessage { - code: msg.to_noqa_code().map(|code| code.to_string()), + code: msg.noqa_code().map(|code| code.to_string()), message: msg.body().to_string(), start_location: source_code.line_column(msg.start()).into(), end_location: source_code.line_column(msg.end()).into(), diff --git a/crates/ruff_workspace/src/configuration.rs b/crates/ruff_workspace/src/configuration.rs index 684a2b434d..ae0d231504 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); } }