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.
This commit is contained in:
Martin Fischer 2023-01-31 17:52:44 +01:00 committed by Charlie Marsh
parent d451c7a506
commit 179ead0157
8 changed files with 64 additions and 37 deletions

View file

@ -7,7 +7,7 @@ use crate::ast::types::Range;
use crate::fix::Fix; use crate::fix::Fix;
use crate::noqa; use crate::noqa;
use crate::noqa::{is_file_exempt, Directive}; 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::rule_redirects::get_redirect_target;
use crate::rules::ruff::rules::{UnusedCodes, UnusedNOQA}; use crate::rules::ruff::rules::{UnusedCodes, UnusedNOQA};
use crate::settings::{flags, Settings}; use crate::settings::{flags, Settings};
@ -20,7 +20,7 @@ pub fn check_noqa(
settings: &Settings, settings: &Settings,
autofix: flags::Autofix, autofix: flags::Autofix,
) { ) {
let mut noqa_directives: IntMap<usize, (Directive, Vec<&str>)> = IntMap::default(); let mut noqa_directives: IntMap<usize, (Directive, Vec<NoqaCode>)> = IntMap::default();
let mut ignored = vec![]; let mut ignored = vec![];
let enforce_noqa = settings.rules.enabled(&Rule::UnusedNOQA); let enforce_noqa = settings.rules.enabled(&Rule::UnusedNOQA);
@ -128,12 +128,12 @@ pub fn check_noqa(
let mut self_ignore = false; let mut self_ignore = false;
for code in codes { for code in codes {
let code = get_redirect_target(code).unwrap_or(code); let code = get_redirect_target(code).unwrap_or(code);
if code == Rule::UnusedNOQA.noqa_code() { if Rule::UnusedNOQA.noqa_code() == code {
self_ignore = true; self_ignore = true;
break; break;
} }
if matches.contains(&code) || settings.external.contains(code) { if matches.iter().any(|m| *m == code) || settings.external.contains(code) {
valid_codes.push(code); valid_codes.push(code);
} else { } else {
if let Ok(rule) = Rule::from_code(code) { if let Ok(rule) = Rule::from_code(code) {

View file

@ -66,7 +66,7 @@ impl Serialize for SerializeRuleAsCode {
where where
S: serde::Serializer, S: serde::Serializer,
{ {
serializer.serialize_str(self.0.noqa_code()) serializer.serialize_str(&self.0.noqa_code().to_string())
} }
} }

View file

@ -1,3 +1,4 @@
use std::fmt::{Display, Write};
use std::fs; use std::fs;
use std::path::Path; 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 /// Returns `true` if the string list of `codes` includes `code` (or an alias
/// thereof). /// thereof).
pub fn includes(needle: &Rule, haystack: &[&str]) -> bool { pub fn includes(needle: &Rule, haystack: &[&str]) -> bool {
let needle: &str = needle.noqa_code(); let needle = needle.noqa_code();
haystack haystack
.iter() .iter()
.any(|candidate| needle == get_redirect_target(candidate).unwrap_or(candidate)) .any(|candidate| needle == get_redirect_target(candidate).unwrap_or(candidate))
@ -205,9 +206,7 @@ fn add_noqa_inner(
output.push_str(" # noqa: "); output.push_str(" # noqa: ");
// Add codes. // Add codes.
let codes: Vec<&str> = rules.iter().map(|r| r.noqa_code()).collect(); push_codes(&mut output, rules.iter().map(|r| r.noqa_code()));
let suffix = codes.join(", ");
output.push_str(&suffix);
output.push_str(line_ending); output.push_str(line_ending);
count += 1; count += 1;
} }
@ -228,14 +227,14 @@ fn add_noqa_inner(
formatted.push_str(" # noqa: "); formatted.push_str(" # noqa: ");
// Add codes. // Add codes.
let codes: Vec<&str> = rules push_codes(
&mut formatted,
rules
.iter() .iter()
.map(|r| r.noqa_code()) .map(|r| r.noqa_code().to_string())
.chain(existing.into_iter()) .chain(existing.into_iter().map(ToString::to_string))
.sorted_unstable() .sorted_unstable(),
.collect(); );
let suffix = codes.join(", ");
formatted.push_str(&suffix);
output.push_str(&formatted); output.push_str(&formatted);
output.push_str(line_ending); output.push_str(line_ending);
@ -253,6 +252,17 @@ fn add_noqa_inner(
(count, output) (count, output)
} }
fn push_codes<I: Display>(str: &mut String, codes: impl Iterator<Item = I>) {
let mut first = true;
for code in codes {
if !first {
str.push_str(", ");
}
let _ = write!(str, "{}", code);
first = false;
}
}
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
use nohash_hasher::IntMap; use nohash_hasher::IntMap;

View file

@ -895,7 +895,7 @@ mod tests {
fn check_code_serialization() { fn check_code_serialization() {
for rule in Rule::iter() { for rule in Rule::iter() {
assert!( 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." "{rule:?} could not be round-trip serialized."
); );
} }
@ -904,9 +904,9 @@ mod tests {
#[test] #[test]
fn test_linter_parse_code() { fn test_linter_parse_code() {
for rule in Rule::iter() { for rule in Rule::iter() {
let code = rule.noqa_code(); let code = format!("{}", rule.noqa_code());
let (linter, rest) = 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())); assert_eq!(code, format!("{}{rest}", linter.common_prefix()));
} }
} }

View file

@ -22,7 +22,7 @@ struct Explanation<'a> {
/// Explain a `Rule` to the user. /// Explain a `Rule` to the user.
pub fn rule(rule: &Rule, format: HelpFormat) -> Result<()> { 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 stdout = BufWriter::new(io::stdout().lock());
let mut output = String::new(); let mut output = String::new();
@ -32,7 +32,7 @@ pub fn rule(rule: &Rule, format: HelpFormat) -> Result<()> {
output.push('\n'); output.push('\n');
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_str(&format!("Derived from the **{}** linter.", linter.name()));
output.push('\n'); output.push('\n');
output.push('\n'); output.push('\n');
@ -58,7 +58,7 @@ pub fn rule(rule: &Rule, format: HelpFormat) -> Result<()> {
} }
HelpFormat::Json => { HelpFormat::Json => {
output.push_str(&serde_json::to_string_pretty(&Explanation { output.push_str(&serde_json::to_string_pretty(&Explanation {
code: rule.noqa_code(), code: &rule.noqa_code().to_string(),
linter: linter.name(), linter: linter.name(),
summary: rule.message_formats()[0], summary: rule.message_formats()[0],
})?); })?);

View file

@ -53,9 +53,9 @@ struct ExpandedMessage<'a> {
} }
#[derive(Serialize)] #[derive(Serialize)]
struct ExpandedStatistics<'a> { struct ExpandedStatistics {
count: usize, count: usize,
code: &'a str, code: String,
message: String, message: String,
fixable: bool, fixable: bool,
} }
@ -67,7 +67,7 @@ impl Serialize for SerializeRuleAsCode<'_> {
where where
S: serde::Serializer, 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!({ json!({
"description": format!("({}) {}", message.kind.rule().noqa_code(), message.kind.body()), "description": format!("({}) {}", message.kind.rule().noqa_code(), message.kind.body()),
"severity": "major", "severity": "major",
"fingerprint": message.kind.rule().noqa_code(), "fingerprint": message.kind.rule().noqa_code().to_string(),
"location": { "location": {
"path": message.filename, "path": message.filename,
"lines": { "lines": {
@ -394,7 +394,7 @@ impl Printer {
let statistics = violations let statistics = violations
.iter() .iter()
.map(|rule| ExpandedStatistics { .map(|rule| ExpandedStatistics {
code: rule.noqa_code(), code: rule.noqa_code().to_string(),
count: diagnostics count: diagnostics
.messages .messages
.iter() .iter()
@ -538,7 +538,7 @@ impl Display for CodeAndBody<'_> {
write!( write!(
f, f,
"{code} {autofix}{body}", "{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()), autofix = format_args!("[{}] ", "*".cyan()),
body = self.0.kind.body(), body = self.0.kind.body(),
) )
@ -546,7 +546,7 @@ impl Display for CodeAndBody<'_> {
write!( write!(
f, f,
"{code} {body}", "{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(), body = self.0.kind.body(),
) )
} }
@ -589,6 +589,7 @@ fn print_message<T: Write>(
} else { } else {
vec![] vec![]
}; };
let label = message.kind.rule().noqa_code().to_string();
let snippet = Snippet { let snippet = Snippet {
title: Some(Annotation { title: Some(Annotation {
label: None, label: None,
@ -601,7 +602,7 @@ fn print_message<T: Write>(
source: &source.contents, source: &source.contents,
line_start: message.location.row(), line_start: message.location.row(),
annotations: vec![SourceAnnotation { annotations: vec![SourceAnnotation {
label: message.kind.rule().noqa_code(), label: &label,
annotation_type: AnnotationType::Error, annotation_type: AnnotationType::Error,
range: source.range, range: source.range,
}], }],
@ -656,7 +657,7 @@ fn print_fixed<T: Write>(stdout: &mut T, fixed: &FxHashMap<String, FixTable>) ->
writeln!( writeln!(
stdout, stdout,
" {count:>num_digits$} × {} ({})", " {count:>num_digits$} × {} ({})",
rule.noqa_code().red().bold(), rule.noqa_code().to_string().red().bold(),
rule.as_ref(), rule.as_ref(),
)?; )?;
} }
@ -694,6 +695,7 @@ fn print_grouped_message<T: Write>(
} else { } else {
vec![] vec![]
}; };
let label = message.kind.rule().noqa_code().to_string();
let snippet = Snippet { let snippet = Snippet {
title: Some(Annotation { title: Some(Annotation {
label: None, label: None,
@ -706,7 +708,7 @@ fn print_grouped_message<T: Write>(
source: &source.contents, source: &source.contents,
line_start: message.location.row(), line_start: message.location.row(),
annotations: vec![SourceAnnotation { annotations: vec![SourceAnnotation {
label: message.kind.rule().noqa_code(), label: &label,
annotation_type: AnnotationType::Error, annotation_type: AnnotationType::Error,
range: source.range, range: source.range,
}], }],

View file

@ -25,7 +25,7 @@ pub fn main(args: &Args) -> Result<()> {
output.push('\n'); output.push('\n');
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_str(&format!("Derived from the **{}** linter.", linter.name()));
output.push('\n'); output.push('\n');
output.push('\n'); output.push('\n');

View file

@ -34,8 +34,8 @@ pub fn define_rule_mapping(mapping: &Mapping) -> proc_macro2::TokenStream {
rule_autofixable_match_arms rule_autofixable_match_arms
.extend(quote! {#(#attr)* Self::#name => <#path as Violation>::AUTOFIX,}); .extend(quote! {#(#attr)* Self::#name => <#path as Violation>::AUTOFIX,});
rule_explanation_match_arms.extend(quote! {#(#attr)* Self::#name => #path::explanation(),}); rule_explanation_match_arms.extend(quote! {#(#attr)* Self::#name => #path::explanation(),});
rule_code_match_arms.extend(quote! {#(#attr)* Self::#name => #code_str,}); 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), }); rule_from_code_match_arms.extend(quote! {#(#attr)* #code_str => Ok(&Rule::#name), });
diagnostic_kind_code_match_arms diagnostic_kind_code_match_arms
.extend(quote! {#(#attr)* Self::#name(..) => &Rule::#name, }); .extend(quote! {#(#attr)* Self::#name(..) => &Rule::#name, });
diagnostic_kind_body_match_arms 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 } 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 } 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 { impl DiagnosticKind {
/// The rule of the diagnostic. /// The rule of the diagnostic.
pub fn rule(&self) -> &'static Rule { pub fn rule(&self) -> &'static Rule {