Skip to content

Commit

Permalink
Disable D203, D212 and D213 by default
Browse files Browse the repository at this point in the history
D203 from pydocstyle conflicts with PEP 257[1]
since its wording has been changed[2] and thus
shouldn't be enabled by `--select D` or `--select ALL`

D212 and D213 conflict with each other which can lead to ruff performing
an infinite autofix iteration loop (Ruff goes back-and-forth, adding and
removing a newline between the docstring and the class definition, until
you hit 100 iterations). We display a warning, but it's just a bad
experience, and it comes up way too often.

Rules that have been disabled by default can still be explicitly
enabled. `ruff explain` now mentions when rules have been disabled by
default as well as the reason and the README table generation has also
been updated accordingly.

[1]: https://peps.python.org/pep-0257/
[2]: python/peps@eba2eac
  • Loading branch information
not-my-profile committed Jan 28, 2023
1 parent b3b65bf commit 57d1e13
Show file tree
Hide file tree
Showing 6 changed files with 98 additions and 15 deletions.
10 changes: 7 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -652,7 +652,6 @@ For more, see [pydocstyle](https://pypi.org/project/pydocstyle/) on PyPI.
| D200 | fits-on-one-line | One-line docstring should fit on one line | 🛠 |
| D201 | no-blank-line-before-function | No blank lines allowed before function docstring (found {num_lines}) | 🛠 |
| D202 | no-blank-line-after-function | No blank lines allowed after function docstring (found {num_lines}) | 🛠 |
| D203 | one-blank-line-before-class | 1 blank line required before class docstring | 🛠 |
| D204 | one-blank-line-after-class | 1 blank line required after class docstring | 🛠 |
| D205 | blank-line-after-summary | 1 blank line required between summary line and description | 🛠 |
| D206 | indent-with-spaces | Docstring should be indented with spaces, not tabs | |
Expand All @@ -661,8 +660,6 @@ For more, see [pydocstyle](https://pypi.org/project/pydocstyle/) on PyPI.
| D209 | new-line-after-last-paragraph | Multi-line docstring closing quotes should be on a separate line | 🛠 |
| D210 | no-surrounding-whitespace | No whitespaces allowed surrounding docstring text | 🛠 |
| D211 | no-blank-line-before-class | No blank lines allowed before class docstring | 🛠 |
| D212 | multi-line-summary-first-line | Multi-line docstring summary should start at the first line | 🛠 |
| D213 | multi-line-summary-second-line | Multi-line docstring summary should start at the second line | 🛠 |
| D214 | section-not-over-indented | Section is over-indented ("{name}") | 🛠 |
| D215 | section-underline-not-over-indented | Section underline is over-indented ("{name}") | 🛠 |
| D300 | uses-triple-quotes | Use """triple double quotes""" | |
Expand All @@ -688,6 +685,13 @@ For more, see [pydocstyle](https://pypi.org/project/pydocstyle/) on PyPI.
| D418 | skip-docstring | Function decorated with `@overload` shouldn't contain a docstring | |
| D419 | non-empty | Docstring is empty | |


The following rules have been disabled by default:

* D203 (one-blank-line-before-class): Conflicts with PEP 257.
* D212 (multi-line-summary-first-line): Not part of PEP 257 and conflicts with D213.
* D213 (multi-line-summary-second-line): Not part of PEP 257 and conflicts with D212.

### pyupgrade (UP)

For more, see [pyupgrade](https://pypi.org/project/pyupgrade/) on PyPI.
Expand Down
6 changes: 6 additions & 0 deletions ruff_cli/src/commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ use ruff::message::{Location, Message};
use ruff::registry::{Linter, Rule, RuleNamespace};
use ruff::resolver::PyprojectDiscovery;
use ruff::settings::flags;
use ruff::settings::nursery::nursery_reason;
use ruff::{fix, fs, packaging, resolver, warn_user_once, AutofixAvailability, IOError};
use serde::Serialize;
use walkdir::WalkDir;
Expand Down Expand Up @@ -275,6 +276,11 @@ pub fn explain(rule: &Rule, format: HelpFormat) -> Result<()> {
println!("{}\n", rule.as_ref());
println!("Code: {} ({})\n", rule.code(), linter.name());

if let Some(reason) = nursery_reason(rule) {
println!("This rule is disabled by default: {reason}");
println!("(but you can still choose to enable it by explicitly selecting it)\n");
}

if let Some(autofix) = rule.autofixable() {
println!(
"{}",
Expand Down
29 changes: 26 additions & 3 deletions ruff_dev/src/generate_rules_table.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
//! Generate a Markdown-compatible table of supported lint rules.

use anyhow::Result;
use ruff::registry::{Linter, LinterCategory, Rule, RuleNamespace};
use ruff::{
registry::{Linter, LinterCategory, Rule, RuleNamespace},
settings::nursery::nursery_reason,
};
use strum::IntoEnumIterator;

use crate::utils::replace_readme_section;
Expand All @@ -19,6 +22,26 @@ pub struct Args {
pub(crate) dry_run: bool,
}

fn document_rules(table_out: &mut String, rules: impl IntoIterator<Item = Rule>) {
let (stable, nursery): (Vec<_>, Vec<_>) =
rules.into_iter().partition(|r| nursery_reason(r).is_none());

generate_table(table_out, stable);

if !nursery.is_empty() {
table_out.push_str("\nThe following rules have been disabled by default:\n\n");
for rule in nursery {
table_out.push_str(&format!(
"* {} ({}): {}\n",
rule.code(),
rule.as_ref(),
nursery_reason(&rule).unwrap()
));
}
table_out.push('\n');
}
}

fn generate_table(table_out: &mut String, rules: impl IntoIterator<Item = Rule>) {
table_out.push_str("| Code | Name | Message | Fix |");
table_out.push('\n');
Expand Down Expand Up @@ -88,10 +111,10 @@ pub fn main(args: &Args) -> Result<()> {
for LinterCategory(prefix, name, selector) in categories {
table_out.push_str(&format!("#### {name} ({prefix})"));
table_out.push('\n');
generate_table(&mut table_out, selector);
document_rules(&mut table_out, selector);
}
} else {
generate_table(&mut table_out, &linter);
document_rules(&mut table_out, &linter);
}
}

Expand Down
27 changes: 21 additions & 6 deletions ruff_macros/src/rule_code_prefix.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::collections::{BTreeMap, BTreeSet};
use std::collections::{BTreeMap, BTreeSet, HashSet};

use proc_macro2::Span;
use quote::quote;
Expand All @@ -16,6 +16,8 @@ pub fn expand<'a>(
let mut all_codes = BTreeSet::new();
let mut pl_codes = BTreeSet::new();

let code_idents: HashSet<_> = code_idents.collect();

for code in code_idents {
let code_str = code.to_string();
let code_prefix_len = code_str
Expand Down Expand Up @@ -76,7 +78,10 @@ fn generate_impls<'a>(
prefix_to_codes: &BTreeMap<String, BTreeSet<String>>,
variant_name: impl Fn(&str) -> &'a Ident,
) -> proc_macro2::TokenStream {
let into_iter_match_arms = prefix_to_codes.iter().map(|(prefix_str, codes)| {
let mut into_iter_match_arms = quote!();
let mut identifies_specific_rule_match_arms = quote!();

for (prefix_str, codes) in prefix_to_codes {
let rule_variants = codes.iter().map(|code| {
let rule_variant = variant_name(code);
quote! {
Expand All @@ -85,10 +90,14 @@ fn generate_impls<'a>(
});
let prefix = Ident::new(prefix_str, Span::call_site());

quote! {
let identifies_specific_rule = codes.contains(prefix_str);
identifies_specific_rule_match_arms.extend(quote! {
#prefix_ident::#prefix => #identifies_specific_rule,
});
into_iter_match_arms.extend(quote! {
#prefix_ident::#prefix => vec![#(#rule_variants),*].into_iter(),
}
});
});
}

let specificity_match_arms = prefix_to_codes.keys().map(|prefix_str| {
let prefix = Ident::new(prefix_str, Span::call_site());
Expand Down Expand Up @@ -120,6 +129,12 @@ fn generate_impls<'a>(
#(#specificity_match_arms)*
}
}

pub(crate) fn identifies_specific_rule(&self) -> bool {
match self {
#identifies_specific_rule_match_arms
}
}
}

impl IntoIterator for &#prefix_ident {
Expand All @@ -129,7 +144,7 @@ fn generate_impls<'a>(
fn into_iter(self) -> Self::IntoIter {
#[allow(clippy::match_same_arms)]
match self {
#(#into_iter_match_arms)*
#into_iter_match_arms
}
}
}
Expand Down
28 changes: 25 additions & 3 deletions src/settings/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ pub mod configuration;
pub mod defaults;
pub mod flags;
pub mod hashable;
pub mod nursery;
pub mod options;
pub mod options_base;
pub mod pyproject;
Expand Down Expand Up @@ -243,7 +244,24 @@ impl From<&Configuration> for RuleTable {
ignore: config.unfixable.as_deref().unwrap_or_default(),
}]);

for code in resolve_codes(
let explicitly_selected: FxHashSet<_> = config
.select
.iter()
.flatten()
.chain(config.extend_select.iter().flatten())
.filter_map(|rs| match rs {
RuleSelector::All => None,
RuleSelector::Prefix { prefix, .. } => {
if prefix.identifies_specific_rule() {
Some(prefix.into_iter().next().unwrap())
} else {
None
}
}
})
.collect();

for rule in resolve_codes(
[RuleCodeSpec {
select: config.select.as_deref().unwrap_or(defaults::PREFIXES),
ignore: config.ignore.as_deref().unwrap_or_default(),
Expand All @@ -257,8 +275,12 @@ impl From<&Configuration> for RuleTable {
.map(|(select, ignore)| RuleCodeSpec { select, ignore }),
),
) {
let fix = fixable.contains(&code);
rules.enable(code, fix);
if nursery::nursery_reason(&rule).is_some() && !explicitly_selected.contains(&rule) {
// Skip rules that are known to be problematic and that haven't been explicitly selected.
continue;
}
let fix = fixable.contains(&rule);
rules.enable(rule, fix);
}

// If a docstring convention is specified, force-disable any incompatible error
Expand Down
13 changes: 13 additions & 0 deletions src/settings/nursery.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
use crate::registry::Rule;

/// Rules for which a reason is returned here are disabled by default unless they are explicitly selected.
/// So for example `one-blank-line-before-class` (D203) will not be selected by "D" or "ALL" since it's listed here.
/// The returned reason is documented in the autogenerated README as well as reported by the explain command.
pub fn nursery_reason(rule: &Rule) -> Option<&'static str> {
match rule {
Rule::OneBlankLineBeforeClass => Some("Conflicts with PEP 257."),
Rule::MultiLineSummaryFirstLine => Some("Not part of PEP 257 and conflicts with D213."),
Rule::MultiLineSummarySecondLine => Some("Not part of PEP 257 and conflicts with D212."),
_ => None,
}
}

0 comments on commit 57d1e13

Please sign in to comment.