Skip to content

Commit

Permalink
Turn Linters', etc. implicit into_iter()s into explicit rules() (#…
Browse files Browse the repository at this point in the history
…5436)

## Summary

As discussed on ~IRC~ Discord, this will make it easier for e.g. the
docs generation stuff to get all rules for a linter (using
`all_rules()`) instead of just non-nursery ones, and it also makes it
more Explicit Is Better Than Implicit to iterate over linter rules.

Grepping for `Item = Rule` reveals some remaining implicit
`IntoIterator`s that I didn't feel were necessarily in scope for this
(and honestly, iterating over a `RuleSet` makes sense).
  • Loading branch information
akx authored Jul 3, 2023
1 parent a647f31 commit 6acc316
Show file tree
Hide file tree
Showing 8 changed files with 50 additions and 52 deletions.
2 changes: 1 addition & 1 deletion crates/ruff/src/flake8_to_ruff/plugin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,7 @@ pub(crate) fn infer_plugins_from_codes(selectors: &HashSet<RuleSelector>) -> Vec
for selector in selectors {
if selector
.into_iter()
.any(|rule| Linter::from(plugin).into_iter().any(|r| r == rule))
.any(|rule| Linter::from(plugin).rules().any(|r| r == rule))
{
return true;
}
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff/src/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ impl Rule {
pub fn from_code(code: &str) -> Result<Self, FromCodeError> {
let (linter, code) = Linter::parse_code(code).ok_or(FromCodeError::Unknown)?;
let prefix: RuleCodePrefix = RuleCodePrefix::parse(&linter, code)?;
Ok(prefix.into_iter().next().unwrap())
Ok(prefix.rules().next().unwrap())
}
}

Expand Down
14 changes: 7 additions & 7 deletions crates/ruff/src/rule_selector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -158,16 +158,16 @@ impl IntoIterator for &RuleSelector {
}
RuleSelector::C => RuleSelectorIter::Chain(
Linter::Flake8Comprehensions
.into_iter()
.chain(Linter::McCabe.into_iter()),
.rules()
.chain(Linter::McCabe.rules()),
),
RuleSelector::T => RuleSelectorIter::Chain(
Linter::Flake8Debugger
.into_iter()
.chain(Linter::Flake8Print.into_iter()),
.rules()
.chain(Linter::Flake8Print.rules()),
),
RuleSelector::Linter(linter) => RuleSelectorIter::Vec(linter.into_iter()),
RuleSelector::Prefix { prefix, .. } => RuleSelectorIter::Vec(prefix.into_iter()),
RuleSelector::Linter(linter) => RuleSelectorIter::Vec(linter.rules()),
RuleSelector::Prefix { prefix, .. } => RuleSelectorIter::Vec(prefix.clone().rules()),
}
}
}
Expand Down Expand Up @@ -346,7 +346,7 @@ mod clap_completion {
let prefix = p.linter().common_prefix();
let code = p.short_code();

let mut rules_iter = p.into_iter();
let mut rules_iter = p.rules();
let rule1 = rules_iter.next();
let rule2 = rules_iter.next();

Expand Down
2 changes: 1 addition & 1 deletion crates/ruff/src/rules/flake8_type_checking/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,7 @@ mod tests {
fn contents(contents: &str, snapshot: &str) {
let diagnostics = test_snippet(
contents,
&settings::Settings::for_rules(&Linter::Flake8TypeChecking),
&settings::Settings::for_rules(Linter::Flake8TypeChecking.rules()),
);
assert_messages!(snapshot, diagnostics);
}
Expand Down
6 changes: 4 additions & 2 deletions crates/ruff/src/rules/pandas_vet/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -353,8 +353,10 @@ mod tests {
"PD901_fail_df_var"
)]
fn contents(contents: &str, snapshot: &str) {
let diagnostics =
test_snippet(contents, &settings::Settings::for_rules(&Linter::PandasVet));
let diagnostics = test_snippet(
contents,
&settings::Settings::for_rules(Linter::PandasVet.rules()),
);
assert_messages!(snapshot, diagnostics);
}

Expand Down
4 changes: 2 additions & 2 deletions crates/ruff/src/rules/pyflakes/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -490,15 +490,15 @@ mod tests {
"load_after_unbind_from_class_scope"
)]
fn contents(contents: &str, snapshot: &str) {
let diagnostics = test_snippet(contents, &Settings::for_rules(&Linter::Pyflakes));
let diagnostics = test_snippet(contents, &Settings::for_rules(Linter::Pyflakes.rules()));
assert_messages!(snapshot, diagnostics);
}

/// A re-implementation of the Pyflakes test runner.
/// Note that all tests marked with `#[ignore]` should be considered TODOs.
fn flakes(contents: &str, expected: &[Rule]) {
let contents = dedent(contents);
let settings = Settings::for_rules(&Linter::Pyflakes);
let settings = Settings::for_rules(Linter::Pyflakes.rules());
let tokens: Vec<LexResult> = ruff_rustpython::tokenize(&contents);
let locator = Locator::new(&contents);
let stylist = Stylist::from_tokens(&tokens, &locator);
Expand Down
4 changes: 2 additions & 2 deletions crates/ruff_dev/src/generate_rules_table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,10 +102,10 @@ pub(crate) fn generate() -> String {
));
table_out.push('\n');
table_out.push('\n');
generate_table(&mut table_out, prefix, &linter);
generate_table(&mut table_out, prefix.clone().rules(), &linter);
}
} else {
generate_table(&mut table_out, &linter, &linter);
generate_table(&mut table_out, linter.rules(), &linter);
}
}

Expand Down
68 changes: 32 additions & 36 deletions crates/ruff_macros/src/map_codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -155,30 +155,13 @@ pub(crate) fn map_codes(func: &ItemFn) -> syn::Result<TokenStream> {
}

output.extend(quote! {
impl IntoIterator for &#linter {
type Item = Rule;
type IntoIter = ::std::vec::IntoIter<Self::Item>;

fn into_iter(self) -> Self::IntoIter {
impl #linter {
pub fn rules(self) -> ::std::vec::IntoIter<Rule> {
match self { #prefix_into_iter_match_arms }
}
}
});
}

output.extend(quote! {
impl IntoIterator for &RuleCodePrefix {
type Item = Rule;
type IntoIter = ::std::vec::IntoIter<Self::Item>;

fn into_iter(self) -> Self::IntoIter {
match self {
#(RuleCodePrefix::#linter_idents(prefix) => prefix.into_iter(),)*
}
}
}
});

output.extend(quote! {
impl RuleCodePrefix {
pub fn parse(linter: &Linter, code: &str) -> Result<Self, crate::registry::FromCodeError> {
Expand All @@ -188,6 +171,12 @@ pub(crate) fn map_codes(func: &ItemFn) -> syn::Result<TokenStream> {
#(Linter::#linter_idents => RuleCodePrefix::#linter_idents(#linter_idents::from_str(code).map_err(|_| crate::registry::FromCodeError::Unknown)?),)*
})
}

pub fn rules(self) -> ::std::vec::IntoIter<Rule> {
match self {
#(RuleCodePrefix::#linter_idents(prefix) => prefix.clone().rules(),)*
}
}
}
});

Expand Down Expand Up @@ -344,32 +333,39 @@ fn generate_iter_impl(
linter_to_rules: &BTreeMap<Ident, BTreeMap<String, Rule>>,
linter_idents: &[&Ident],
) -> TokenStream {
let mut linter_into_iter_match_arms = quote!();
let mut linter_rules_match_arms = quote!();
let mut linter_all_rules_match_arms = quote!();
for (linter, map) in linter_to_rules {
let rule_paths = map
.values()
.filter(|rule| {
// Nursery rules have to be explicitly selected, so we ignore them when looking at
// linter-level selectors (e.g., `--select SIM`).
!is_nursery(&rule.group)
})
.map(|Rule { attrs, path, .. }| {
let rule_paths = map.values().filter(|rule| !is_nursery(&rule.group)).map(
|Rule { attrs, path, .. }| {
let rule_name = path.segments.last().unwrap();
quote!(#(#attrs)* Rule::#rule_name)
});
linter_into_iter_match_arms.extend(quote! {
},
);
linter_rules_match_arms.extend(quote! {
Linter::#linter => vec![#(#rule_paths,)*].into_iter(),
});
let rule_paths = map.values().map(|Rule { attrs, path, .. }| {
let rule_name = path.segments.last().unwrap();
quote!(#(#attrs)* Rule::#rule_name)
});
linter_all_rules_match_arms.extend(quote! {
Linter::#linter => vec![#(#rule_paths,)*].into_iter(),
});
}

quote! {
impl IntoIterator for &Linter {
type Item = Rule;
type IntoIter = ::std::vec::IntoIter<Self::Item>;

fn into_iter(self) -> Self::IntoIter {
impl Linter {
/// Rules not in the nursery.
pub fn rules(self: &Linter) -> ::std::vec::IntoIter<Rule> {
match self {
#linter_rules_match_arms
}
}
/// All rules, including those in the nursery.
pub fn all_rules(self: &Linter) -> ::std::vec::IntoIter<Rule> {
match self {
#linter_into_iter_match_arms
#linter_all_rules_match_arms
}
}
}
Expand Down

0 comments on commit 6acc316

Please sign in to comment.