Skip to content

Commit

Permalink
refactor: Generate Linter -> RuleSelector mapping via macro
Browse files Browse the repository at this point in the history
To enable ruff_dev to automatically generate the rule Markdown tables in
the README the ruff library contained the following function:

    Linter::codes() -> Vec<RuleSelector>

which was slightly changed to `fn prefixes(&self) -> Prefixes` in
9dc66b5 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<Rule> 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]>;
  • Loading branch information
not-my-profile authored and charliermarsh committed Jan 22, 2023
1 parent c3dd1b0 commit 4758ee6
Show file tree
Hide file tree
Showing 6 changed files with 77 additions and 87 deletions.
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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.

Expand Down
4 changes: 2 additions & 2 deletions ruff_cli/tests/black_compatibility_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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 = [
"-",
Expand Down
24 changes: 11 additions & 13 deletions ruff_dev/src/generate_rules_table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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<Item = Rule>) {
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(_) => "🛠",
Expand All @@ -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');
Expand Down Expand Up @@ -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);
}
}

Expand Down
48 changes: 47 additions & 1 deletion ruff_macros/src/rule_namespace.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand All @@ -11,6 +12,8 @@ pub fn derive_impl(input: DeriveInput) -> syn::Result<proc_macro2::TokenStream>

let mut parsed = Vec::new();

let mut prefix_match_arms = quote!();

for variant in variants {
let prefix_attrs: Vec<_> = variant
.attrs
Expand All @@ -25,30 +28,73 @@ pub fn derive_impl(input: DeriveInput) -> syn::Result<proc_macro2::TokenStream>
));
}

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<Self::Item>;

fn into_iter(self) -> Self::IntoIter {
use colored::Colorize;

match self {
#into_iter_match_arms
}
}
}
})
}
6 changes: 0 additions & 6 deletions scripts/add_plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")

Expand Down
78 changes: 15 additions & 63 deletions src/registry.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -446,7 +445,7 @@ pub enum Linter {
#[prefix = "E"]
#[prefix = "W"]
Pycodestyle,
#[prefix = "C9"]
#[prefix = "C90"]
McCabe,
#[prefix = "I"]
Isort,
Expand Down Expand Up @@ -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,
}
}
}
Expand Down

0 comments on commit 4758ee6

Please sign in to comment.