From 4758ee6ac4e3a25bd1daeecdfb56ee6663acb330 Mon Sep 17 00:00:00 2001 From: Martin Fischer Date: Sat, 21 Jan 2023 07:19:10 +0100 Subject: [PATCH] refactor: Generate Linter -> RuleSelector mapping via macro To enable ruff_dev to automatically generate the rule Markdown tables in the README the ruff library contained the following function: Linter::codes() -> Vec which was slightly changed to `fn prefixes(&self) -> Prefixes` in 9dc66b5a6546926aff0d04a0280401f939f3a3d8 to enable ruff_dev to split up the Markdown tables for linters that have multiple prefixes (pycodestyle has E & W, Pylint has PLC, PLE, PLR & PLW). The definition of this method was however largely redundant with the #[prefix] macro attributes in the Linter enum, which are used to derive the Linter::parse_code function, used by the --explain command. This commit removes the redundant Linter::prefixes by introducing a same-named method with a different signature to the RuleNamespace trait: fn prefixes(&self) -> &'static [&'static str]; As well as implementing IntoIterator for &Linter. We extend the extisting RuleNamespace proc macro to automatically derive both implementations from the Linter enum definition. To support the previously mentioned Markdown table splitting we introduce a very simple hand-written method to the Linter impl: fn categories(&self) -> Option<&'static [LinterCategory]>; --- README.md | 4 +- ruff_cli/tests/black_compatibility_test.rs | 4 +- ruff_dev/src/generate_rules_table.rs | 24 +++---- ruff_macros/src/rule_namespace.rs | 48 ++++++++++++- scripts/add_plugin.py | 6 -- src/registry.rs | 78 +++++----------------- 6 files changed, 77 insertions(+), 87 deletions(-) diff --git a/README.md b/README.md index 28f7ca0c45904..46541616705a7 100644 --- a/README.md +++ b/README.md @@ -134,7 +134,7 @@ developer of [Zulip](https://github.com/zulip/zulip): 1. [eradicate (ERA)](#eradicate-era) 1. [pandas-vet (PD)](#pandas-vet-pd) 1. [pygrep-hooks (PGH)](#pygrep-hooks-pgh) - 1. [Pylint (PLC, PLE, PLR, PLW)](#pylint-plc-ple-plr-plw) + 1. [Pylint (PL)](#pylint-pl) 1. [flake8-pie (PIE)](#flake8-pie-pie) 1. [flake8-commas (COM)](#flake8-commas-com) 1. [flake8-no-pep420 (INP)](#flake8-no-pep420-inp) @@ -1108,7 +1108,7 @@ For more, see [pygrep-hooks](https://github.com/pre-commit/pygrep-hooks) on GitH | PGH003 | blanket-type-ignore | Use specific rule codes when ignoring type issues | | | PGH004 | blanket-noqa | Use specific rule codes when using `noqa` | | -### Pylint (PLC, PLE, PLR, PLW) +### Pylint (PL) For more, see [Pylint](https://pypi.org/project/pylint/2.15.7/) on PyPI. diff --git a/ruff_cli/tests/black_compatibility_test.rs b/ruff_cli/tests/black_compatibility_test.rs index 425d401b209be..938927187b09c 100644 --- a/ruff_cli/tests/black_compatibility_test.rs +++ b/ruff_cli/tests/black_compatibility_test.rs @@ -13,7 +13,7 @@ use assert_cmd::Command; use itertools::Itertools; use log::info; use ruff::logging::{set_up_logging, LogLevel}; -use ruff::registry::Linter; +use ruff::registry::{Linter, RuleNamespace}; use strum::IntoEnumIterator; use walkdir::WalkDir; @@ -180,7 +180,7 @@ fn test_ruff_black_compatibility() -> Result<()> { // problem. Ruff would add a `# noqa: W292` after the first run, black introduces a // newline, and ruff removes the `# noqa: W292` again. .filter(|linter| *linter != Linter::Ruff) - .map(|linter| linter.prefixes().as_list(",")) + .map(|linter| linter.prefixes().join(",")) .join(","); let ruff_args = [ "-", diff --git a/ruff_dev/src/generate_rules_table.rs b/ruff_dev/src/generate_rules_table.rs index d8f840c78a243..98dca6bb1be71 100644 --- a/ruff_dev/src/generate_rules_table.rs +++ b/ruff_dev/src/generate_rules_table.rs @@ -2,7 +2,7 @@ use anyhow::Result; use clap::Args; -use ruff::registry::{Linter, Prefixes, RuleSelector}; +use ruff::registry::{Linter, LinterCategory, Rule, RuleNamespace}; use strum::IntoEnumIterator; use crate::utils::replace_readme_section; @@ -20,12 +20,12 @@ pub struct Cli { pub(crate) dry_run: bool, } -fn generate_table(table_out: &mut String, selector: &RuleSelector) { +fn generate_table(table_out: &mut String, rules: impl IntoIterator) { table_out.push_str("| Code | Name | Message | Fix |"); table_out.push('\n'); table_out.push_str("| ---- | ---- | ------- | --- |"); table_out.push('\n'); - for rule in selector { + for rule in rules { let fix_token = match rule.autofixable() { None => "", Some(_) => "🛠", @@ -48,8 +48,7 @@ pub fn main(cli: &Cli) -> Result<()> { let mut table_out = String::new(); let mut toc_out = String::new(); for linter in Linter::iter() { - let prefixes = linter.prefixes(); - let codes_csv: String = prefixes.as_list(", "); + let codes_csv: String = linter.prefixes().join(", "); table_out.push_str(&format!("### {} ({codes_csv})", linter.name())); table_out.push('\n'); table_out.push('\n'); @@ -86,15 +85,14 @@ pub fn main(cli: &Cli) -> Result<()> { table_out.push('\n'); } - match prefixes { - Prefixes::Single(prefix) => generate_table(&mut table_out, &prefix), - Prefixes::Multiple(entries) => { - for (selector, category) in entries { - table_out.push_str(&format!("#### {category} ({})", selector.as_ref())); - table_out.push('\n'); - generate_table(&mut table_out, &selector); - } + if let Some(categories) = linter.categories() { + for LinterCategory(prefix, name, selector) in categories { + table_out.push_str(&format!("#### {name} ({prefix})")); + table_out.push('\n'); + generate_table(&mut table_out, selector); } + } else { + generate_table(&mut table_out, &linter); } } diff --git a/ruff_macros/src/rule_namespace.rs b/ruff_macros/src/rule_namespace.rs index c6b1002eb1dbd..96ec6d4637d7a 100644 --- a/ruff_macros/src/rule_namespace.rs +++ b/ruff_macros/src/rule_namespace.rs @@ -1,3 +1,4 @@ +use proc_macro2::{Ident, Span}; use quote::quote; use syn::spanned::Spanned; use syn::{Data, DataEnum, DeriveInput, Error, Lit, Meta, MetaNameValue}; @@ -11,6 +12,8 @@ pub fn derive_impl(input: DeriveInput) -> syn::Result let mut parsed = Vec::new(); + let mut prefix_match_arms = quote!(); + for variant in variants { let prefix_attrs: Vec<_> = variant .attrs @@ -25,30 +28,73 @@ pub fn derive_impl(input: DeriveInput) -> syn::Result )); } + let mut prefix_literals = Vec::new(); + for attr in prefix_attrs { let Ok(Meta::NameValue(MetaNameValue{lit: Lit::Str(lit), ..})) = attr.parse_meta() else { return Err(Error::new(attr.span(), r#"expected attribute to be in the form of [#prefix = "..."]"#)) }; - parsed.push((lit, variant.ident.clone())); + parsed.push((lit.clone(), variant.ident.clone())); + prefix_literals.push(lit); } + + let variant_ident = variant.ident; + + prefix_match_arms.extend(quote! { + Self::#variant_ident => &[#(#prefix_literals),*], + }); } parsed.sort_by_key(|(prefix, _)| prefix.value().len()); let mut if_statements = quote!(); + let mut into_iter_match_arms = quote!(); for (prefix, field) in parsed { if_statements.extend(quote! {if let Some(rest) = code.strip_prefix(#prefix) { return Some((#ident::#field, rest)); }}); + + let prefix_ident = Ident::new(&prefix.value(), Span::call_site()); + + if field != "Pycodestyle" { + into_iter_match_arms.extend(quote! { + #ident::#field => RuleSelector::#prefix_ident.into_iter(), + }); + } } + into_iter_match_arms.extend(quote! { + #ident::Pycodestyle => { + let rules: Vec<_> = (&RuleSelector::E).into_iter().chain(&RuleSelector::W).collect(); + rules.into_iter() + } + }); + Ok(quote! { impl crate::registry::RuleNamespace for #ident { fn parse_code(code: &str) -> Option<(Self, &str)> { #if_statements None } + + + fn prefixes(&self) -> &'static [&'static str] { + match self { #prefix_match_arms } + } + } + + impl IntoIterator for &#ident { + type Item = Rule; + type IntoIter = ::std::vec::IntoIter; + + fn into_iter(self) -> Self::IntoIter { + use colored::Colorize; + + match self { + #into_iter_match_arms + } + } } }) } diff --git a/scripts/add_plugin.py b/scripts/add_plugin.py index 716ef0eb08d67..71ff72301256e 100755 --- a/scripts/add_plugin.py +++ b/scripts/add_plugin.py @@ -89,12 +89,6 @@ def main(*, plugin: str, url: str, prefix_code: str) -> None: fp.write(f"{indent}{pascal_case(plugin)},") fp.write("\n") - elif line.strip() == "Linter::Ruff => Prefixes::Single(RuleSelector::RUF),": - fp.write( - f"{indent}Linter::{pascal_case(plugin)} => Prefixes::Single(RuleSelector::{prefix_code})," - ) - fp.write("\n") - fp.write(line) fp.write("\n") diff --git a/src/registry.rs b/src/registry.rs index babdb819191cc..979382d326cec 100644 --- a/src/registry.rs +++ b/src/registry.rs @@ -1,6 +1,5 @@ //! Registry of [`Rule`] to [`DiagnosticKind`] mappings. -use itertools::Itertools; use once_cell::sync::Lazy; use ruff_macros::RuleNamespace; use rustc_hash::FxHashMap; @@ -446,7 +445,7 @@ pub enum Linter { #[prefix = "E"] #[prefix = "W"] Pycodestyle, - #[prefix = "C9"] + #[prefix = "C90"] McCabe, #[prefix = "I"] Isort, @@ -522,76 +521,29 @@ pub enum Linter { pub trait RuleNamespace: Sized { fn parse_code(code: &str) -> Option<(Self, &str)>; -} -pub enum Prefixes { - Single(RuleSelector), - Multiple(Vec<(RuleSelector, &'static str)>), -} - -impl Prefixes { - pub fn as_list(&self, separator: &str) -> String { - match self { - Prefixes::Single(prefix) => prefix.as_ref().to_string(), - Prefixes::Multiple(entries) => entries - .iter() - .map(|(prefix, _)| prefix.as_ref()) - .join(separator), - } - } + fn prefixes(&self) -> &'static [&'static str]; } include!(concat!(env!("OUT_DIR"), "/linter.rs")); +/// The prefix, name and selector for an upstream linter category. +pub struct LinterCategory(pub &'static str, pub &'static str, pub RuleSelector); + impl Linter { - pub fn prefixes(&self) -> Prefixes { + pub fn categories(&self) -> Option<&'static [LinterCategory]> { match self { - Linter::Eradicate => Prefixes::Single(RuleSelector::ERA), - Linter::Flake82020 => Prefixes::Single(RuleSelector::YTT), - Linter::Flake8Annotations => Prefixes::Single(RuleSelector::ANN), - Linter::Flake8Bandit => Prefixes::Single(RuleSelector::S), - Linter::Flake8BlindExcept => Prefixes::Single(RuleSelector::BLE), - Linter::Flake8BooleanTrap => Prefixes::Single(RuleSelector::FBT), - Linter::Flake8Bugbear => Prefixes::Single(RuleSelector::B), - Linter::Flake8Builtins => Prefixes::Single(RuleSelector::A), - Linter::Flake8Comprehensions => Prefixes::Single(RuleSelector::C4), - Linter::Flake8Datetimez => Prefixes::Single(RuleSelector::DTZ), - Linter::Flake8Debugger => Prefixes::Single(RuleSelector::T10), - Linter::Flake8ErrMsg => Prefixes::Single(RuleSelector::EM), - Linter::Flake8ImplicitStrConcat => Prefixes::Single(RuleSelector::ISC), - Linter::Flake8ImportConventions => Prefixes::Single(RuleSelector::ICN), - Linter::Flake8Print => Prefixes::Single(RuleSelector::T20), - Linter::Flake8PytestStyle => Prefixes::Single(RuleSelector::PT), - Linter::Flake8Quotes => Prefixes::Single(RuleSelector::Q), - Linter::Flake8Return => Prefixes::Single(RuleSelector::RET), - Linter::Flake8Simplify => Prefixes::Single(RuleSelector::SIM), - Linter::Flake8TidyImports => Prefixes::Single(RuleSelector::TID), - Linter::Flake8UnusedArguments => Prefixes::Single(RuleSelector::ARG), - Linter::Isort => Prefixes::Single(RuleSelector::I), - Linter::McCabe => Prefixes::Single(RuleSelector::C90), - Linter::PEP8Naming => Prefixes::Single(RuleSelector::N), - Linter::PandasVet => Prefixes::Single(RuleSelector::PD), - Linter::Pycodestyle => Prefixes::Multiple(vec![ - (RuleSelector::E, "Error"), - (RuleSelector::W, "Warning"), + Linter::Pycodestyle => Some(&[ + LinterCategory("E", "Error", RuleSelector::E), + LinterCategory("W", "Warning", RuleSelector::W), ]), - Linter::Pydocstyle => Prefixes::Single(RuleSelector::D), - Linter::Pyflakes => Prefixes::Single(RuleSelector::F), - Linter::PygrepHooks => Prefixes::Single(RuleSelector::PGH), - Linter::Pylint => Prefixes::Multiple(vec![ - (RuleSelector::PLC, "Convention"), - (RuleSelector::PLE, "Error"), - (RuleSelector::PLR, "Refactor"), - (RuleSelector::PLW, "Warning"), + Linter::Pylint => Some(&[ + LinterCategory("PLC", "Convention", RuleSelector::PLC), + LinterCategory("PLE", "Error", RuleSelector::PLE), + LinterCategory("PLR", "Refactor", RuleSelector::PLR), + LinterCategory("PLW", "Warning", RuleSelector::PLW), ]), - Linter::Pyupgrade => Prefixes::Single(RuleSelector::UP), - Linter::Flake8Pie => Prefixes::Single(RuleSelector::PIE), - Linter::Flake8Commas => Prefixes::Single(RuleSelector::COM), - Linter::Flake8NoPep420 => Prefixes::Single(RuleSelector::INP), - Linter::Flake8Executable => Prefixes::Single(RuleSelector::EXE), - Linter::Flake8TypeChecking => Prefixes::Single(RuleSelector::TYP), - Linter::Tryceratops => Prefixes::Single(RuleSelector::TRY), - Linter::Ruff => Prefixes::Single(RuleSelector::RUF), + _ => None, } } }