From 179ead015791be54696ac22f05312f8a471ef39a Mon Sep 17 00:00:00 2001 From: Martin Fischer Date: Tue, 31 Jan 2023 17:52:44 +0100 Subject: [PATCH] many-to-one 2/9: Newtype Rule::noqa_code return type Rule::noqa_code previously return a single &'static str, which was possible because we had one enum listing all rule code prefixes. This commit series will however split up the RuleCodePrefix enum into several enums ... so we'll end up with two &'static str ... this commit wraps the return type of Rule::noqa_code into a newtype so that we can easily change it to return two &'static str in the 6th commit of this series. --- crates/ruff/src/checkers/noqa.rs | 8 ++--- crates/ruff/src/lib_wasm.rs | 2 +- crates/ruff/src/noqa.rs | 34 ++++++++++++------- crates/ruff/src/registry.rs | 6 ++-- crates/ruff_cli/src/commands/rule.rs | 6 ++-- crates/ruff_cli/src/printer.rs | 22 ++++++------ crates/ruff_dev/src/generate_docs.rs | 2 +- crates/ruff_macros/src/define_rule_mapping.rs | 21 ++++++++++-- 8 files changed, 64 insertions(+), 37 deletions(-) diff --git a/crates/ruff/src/checkers/noqa.rs b/crates/ruff/src/checkers/noqa.rs index 9224b81d9d..3d3727bc15 100644 --- a/crates/ruff/src/checkers/noqa.rs +++ b/crates/ruff/src/checkers/noqa.rs @@ -7,7 +7,7 @@ use crate::ast::types::Range; use crate::fix::Fix; use crate::noqa; use crate::noqa::{is_file_exempt, Directive}; -use crate::registry::{Diagnostic, DiagnosticKind, Rule}; +use crate::registry::{Diagnostic, DiagnosticKind, NoqaCode, Rule}; use crate::rule_redirects::get_redirect_target; use crate::rules::ruff::rules::{UnusedCodes, UnusedNOQA}; use crate::settings::{flags, Settings}; @@ -20,7 +20,7 @@ pub fn check_noqa( settings: &Settings, autofix: flags::Autofix, ) { - let mut noqa_directives: IntMap)> = IntMap::default(); + let mut noqa_directives: IntMap)> = IntMap::default(); let mut ignored = vec![]; let enforce_noqa = settings.rules.enabled(&Rule::UnusedNOQA); @@ -128,12 +128,12 @@ pub fn check_noqa( let mut self_ignore = false; for code in codes { let code = get_redirect_target(code).unwrap_or(code); - if code == Rule::UnusedNOQA.noqa_code() { + if Rule::UnusedNOQA.noqa_code() == code { self_ignore = true; break; } - if matches.contains(&code) || settings.external.contains(code) { + if matches.iter().any(|m| *m == code) || settings.external.contains(code) { valid_codes.push(code); } else { if let Ok(rule) = Rule::from_code(code) { diff --git a/crates/ruff/src/lib_wasm.rs b/crates/ruff/src/lib_wasm.rs index c888bd7d85..10cdd7ec80 100644 --- a/crates/ruff/src/lib_wasm.rs +++ b/crates/ruff/src/lib_wasm.rs @@ -66,7 +66,7 @@ impl Serialize for SerializeRuleAsCode { where S: serde::Serializer, { - serializer.serialize_str(self.0.noqa_code()) + serializer.serialize_str(&self.0.noqa_code().to_string()) } } diff --git a/crates/ruff/src/noqa.rs b/crates/ruff/src/noqa.rs index 082bb81fb9..f663eb389f 100644 --- a/crates/ruff/src/noqa.rs +++ b/crates/ruff/src/noqa.rs @@ -1,3 +1,4 @@ +use std::fmt::{Display, Write}; use std::fs; use std::path::Path; @@ -72,7 +73,7 @@ pub fn extract_noqa_directive(line: &str) -> Directive { /// Returns `true` if the string list of `codes` includes `code` (or an alias /// thereof). pub fn includes(needle: &Rule, haystack: &[&str]) -> bool { - let needle: &str = needle.noqa_code(); + let needle = needle.noqa_code(); haystack .iter() .any(|candidate| needle == get_redirect_target(candidate).unwrap_or(candidate)) @@ -205,9 +206,7 @@ fn add_noqa_inner( output.push_str(" # noqa: "); // Add codes. - let codes: Vec<&str> = rules.iter().map(|r| r.noqa_code()).collect(); - let suffix = codes.join(", "); - output.push_str(&suffix); + push_codes(&mut output, rules.iter().map(|r| r.noqa_code())); output.push_str(line_ending); count += 1; } @@ -228,14 +227,14 @@ fn add_noqa_inner( formatted.push_str(" # noqa: "); // Add codes. - let codes: Vec<&str> = rules - .iter() - .map(|r| r.noqa_code()) - .chain(existing.into_iter()) - .sorted_unstable() - .collect(); - let suffix = codes.join(", "); - formatted.push_str(&suffix); + push_codes( + &mut formatted, + rules + .iter() + .map(|r| r.noqa_code().to_string()) + .chain(existing.into_iter().map(ToString::to_string)) + .sorted_unstable(), + ); output.push_str(&formatted); output.push_str(line_ending); @@ -253,6 +252,17 @@ fn add_noqa_inner( (count, output) } +fn push_codes(str: &mut String, codes: impl Iterator) { + let mut first = true; + for code in codes { + if !first { + str.push_str(", "); + } + let _ = write!(str, "{}", code); + first = false; + } +} + #[cfg(test)] mod tests { use nohash_hasher::IntMap; diff --git a/crates/ruff/src/registry.rs b/crates/ruff/src/registry.rs index 1214a6a924..936deecd09 100644 --- a/crates/ruff/src/registry.rs +++ b/crates/ruff/src/registry.rs @@ -895,7 +895,7 @@ mod tests { fn check_code_serialization() { for rule in Rule::iter() { assert!( - Rule::from_code(rule.noqa_code()).is_ok(), + Rule::from_code(&format!("{}", rule.noqa_code())).is_ok(), "{rule:?} could not be round-trip serialized." ); } @@ -904,9 +904,9 @@ mod tests { #[test] fn test_linter_parse_code() { for rule in Rule::iter() { - let code = rule.noqa_code(); + let code = format!("{}", rule.noqa_code()); let (linter, rest) = - Linter::parse_code(code).unwrap_or_else(|| panic!("couldn't parse {code:?}")); + Linter::parse_code(&code).unwrap_or_else(|| panic!("couldn't parse {code:?}")); assert_eq!(code, format!("{}{rest}", linter.common_prefix())); } } diff --git a/crates/ruff_cli/src/commands/rule.rs b/crates/ruff_cli/src/commands/rule.rs index 88d1158e9e..008b34f186 100644 --- a/crates/ruff_cli/src/commands/rule.rs +++ b/crates/ruff_cli/src/commands/rule.rs @@ -22,7 +22,7 @@ struct Explanation<'a> { /// Explain a `Rule` to the user. pub fn rule(rule: &Rule, format: HelpFormat) -> Result<()> { - let (linter, _) = Linter::parse_code(rule.noqa_code()).unwrap(); + let (linter, _) = Linter::parse_code(&rule.noqa_code().to_string()).unwrap(); let mut stdout = BufWriter::new(io::stdout().lock()); let mut output = String::new(); @@ -32,7 +32,7 @@ pub fn rule(rule: &Rule, format: HelpFormat) -> Result<()> { output.push('\n'); output.push('\n'); - let (linter, _) = Linter::parse_code(rule.noqa_code()).unwrap(); + let (linter, _) = Linter::parse_code(&rule.noqa_code().to_string()).unwrap(); output.push_str(&format!("Derived from the **{}** linter.", linter.name())); output.push('\n'); output.push('\n'); @@ -58,7 +58,7 @@ pub fn rule(rule: &Rule, format: HelpFormat) -> Result<()> { } HelpFormat::Json => { output.push_str(&serde_json::to_string_pretty(&Explanation { - code: rule.noqa_code(), + code: &rule.noqa_code().to_string(), linter: linter.name(), summary: rule.message_formats()[0], })?); diff --git a/crates/ruff_cli/src/printer.rs b/crates/ruff_cli/src/printer.rs index 896c96e320..70098ea67e 100644 --- a/crates/ruff_cli/src/printer.rs +++ b/crates/ruff_cli/src/printer.rs @@ -53,9 +53,9 @@ struct ExpandedMessage<'a> { } #[derive(Serialize)] -struct ExpandedStatistics<'a> { +struct ExpandedStatistics { count: usize, - code: &'a str, + code: String, message: String, fixable: bool, } @@ -67,7 +67,7 @@ impl Serialize for SerializeRuleAsCode<'_> { where S: serde::Serializer, { - serializer.serialize_str(self.0.noqa_code()) + serializer.serialize_str(&self.0.noqa_code().to_string()) } } @@ -343,7 +343,7 @@ impl Printer { json!({ "description": format!("({}) {}", message.kind.rule().noqa_code(), message.kind.body()), "severity": "major", - "fingerprint": message.kind.rule().noqa_code(), + "fingerprint": message.kind.rule().noqa_code().to_string(), "location": { "path": message.filename, "lines": { @@ -394,7 +394,7 @@ impl Printer { let statistics = violations .iter() .map(|rule| ExpandedStatistics { - code: rule.noqa_code(), + code: rule.noqa_code().to_string(), count: diagnostics .messages .iter() @@ -538,7 +538,7 @@ impl Display for CodeAndBody<'_> { write!( f, "{code} {autofix}{body}", - code = self.0.kind.rule().noqa_code().red().bold(), + code = self.0.kind.rule().noqa_code().to_string().red().bold(), autofix = format_args!("[{}] ", "*".cyan()), body = self.0.kind.body(), ) @@ -546,7 +546,7 @@ impl Display for CodeAndBody<'_> { write!( f, "{code} {body}", - code = self.0.kind.rule().noqa_code().red().bold(), + code = self.0.kind.rule().noqa_code().to_string().red().bold(), body = self.0.kind.body(), ) } @@ -589,6 +589,7 @@ fn print_message( } else { vec![] }; + let label = message.kind.rule().noqa_code().to_string(); let snippet = Snippet { title: Some(Annotation { label: None, @@ -601,7 +602,7 @@ fn print_message( source: &source.contents, line_start: message.location.row(), annotations: vec![SourceAnnotation { - label: message.kind.rule().noqa_code(), + label: &label, annotation_type: AnnotationType::Error, range: source.range, }], @@ -656,7 +657,7 @@ fn print_fixed(stdout: &mut T, fixed: &FxHashMap) -> writeln!( stdout, " {count:>num_digits$} × {} ({})", - rule.noqa_code().red().bold(), + rule.noqa_code().to_string().red().bold(), rule.as_ref(), )?; } @@ -694,6 +695,7 @@ fn print_grouped_message( } else { vec![] }; + let label = message.kind.rule().noqa_code().to_string(); let snippet = Snippet { title: Some(Annotation { label: None, @@ -706,7 +708,7 @@ fn print_grouped_message( source: &source.contents, line_start: message.location.row(), annotations: vec![SourceAnnotation { - label: message.kind.rule().noqa_code(), + label: &label, annotation_type: AnnotationType::Error, range: source.range, }], diff --git a/crates/ruff_dev/src/generate_docs.rs b/crates/ruff_dev/src/generate_docs.rs index 8d71a6d229..528f2a9a42 100644 --- a/crates/ruff_dev/src/generate_docs.rs +++ b/crates/ruff_dev/src/generate_docs.rs @@ -25,7 +25,7 @@ pub fn main(args: &Args) -> Result<()> { output.push('\n'); output.push('\n'); - let (linter, _) = Linter::parse_code(rule.noqa_code()).unwrap(); + let (linter, _) = Linter::parse_code(&rule.noqa_code().to_string()).unwrap(); output.push_str(&format!("Derived from the **{}** linter.", linter.name())); output.push('\n'); output.push('\n'); diff --git a/crates/ruff_macros/src/define_rule_mapping.rs b/crates/ruff_macros/src/define_rule_mapping.rs index e341f320b7..4e675c60df 100644 --- a/crates/ruff_macros/src/define_rule_mapping.rs +++ b/crates/ruff_macros/src/define_rule_mapping.rs @@ -34,8 +34,8 @@ pub fn define_rule_mapping(mapping: &Mapping) -> proc_macro2::TokenStream { rule_autofixable_match_arms .extend(quote! {#(#attr)* Self::#name => <#path as Violation>::AUTOFIX,}); rule_explanation_match_arms.extend(quote! {#(#attr)* Self::#name => #path::explanation(),}); - rule_code_match_arms.extend(quote! {#(#attr)* Self::#name => #code_str,}); - rule_from_code_match_arms.extend(quote! {#(#attr)* #code_str => Ok(Rule::#name), }); + rule_code_match_arms.extend(quote! {#(#attr)* Self::#name => NoqaCode(#code_str),}); + rule_from_code_match_arms.extend(quote! {#(#attr)* #code_str => Ok(&Rule::#name), }); diagnostic_kind_code_match_arms .extend(quote! {#(#attr)* Self::#name(..) => &Rule::#name, }); diagnostic_kind_body_match_arms @@ -106,7 +106,7 @@ pub fn define_rule_mapping(mapping: &Mapping) -> proc_macro2::TokenStream { match self { #rule_autofixable_match_arms } } - pub fn noqa_code(&self) -> &'static str { + pub fn noqa_code(&self) -> NoqaCode { match self { #rule_code_match_arms } } @@ -118,6 +118,21 @@ pub fn define_rule_mapping(mapping: &Mapping) -> proc_macro2::TokenStream { } } + #[derive(PartialEq, Eq, PartialOrd, Ord)] + pub struct NoqaCode(&'static str); + + impl std::fmt::Display for NoqaCode { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> Result<(), std::fmt::Error> { + self.0.fmt(f) + } + } + + impl PartialEq<&str> for NoqaCode { + fn eq(&self, other: &&str) -> bool { + self.0 == *other + } + } + impl DiagnosticKind { /// The rule of the diagnostic. pub fn rule(&self) -> &'static Rule {