Allow overriding pydocstyle convention rules (#8586)

## Summary

This fixes #2606 by moving where we apply the convention ignores --
instead of applying that at the very end, e track, we now track which
rules have been specifically enabled (via `Specificity::Rule`). If they
have, then we do *not* apply the docstring overrides at the end.

## Test Plan

Added unit tests to `ruff_workspace` and an integration test to
`ruff_cli`
This commit is contained in:
Alan Du 2023-11-10 13:47:37 -05:00 committed by GitHub
parent 3e00ddce38
commit 5a1a8bebca
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 182 additions and 7 deletions

View file

@ -1564,3 +1564,62 @@ extend-safe-fixes = ["UP03"]
Ok(()) Ok(())
} }
#[test]
fn check_docstring_conventions_overrides() -> Result<()> {
// But if we explicitly select it, we override the convention
let tempdir = TempDir::new()?;
let ruff_toml = tempdir.path().join("ruff.toml");
fs::write(
&ruff_toml,
r#"
[lint.pydocstyle]
convention = "numpy"
"#,
)?;
let stdin = r#"
def log(x, base) -> float:
"""Calculate natural log of a value
Parameters
----------
x :
Hello
"""
return math.log(x)
"#;
// If we only select the prefix, then everything passes
assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME))
.args(["check", "-", "--config"])
.arg(&ruff_toml)
.args(["--output-format", "text", "--no-cache", "--select", "D41"])
.pass_stdin(stdin),
@r###"
success: true
exit_code: 0
----- stdout -----
----- stderr -----
"###
);
// But if we select the exact code, we get an error
assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME))
.args(["check", "-", "--config"])
.arg(&ruff_toml)
.args(["--output-format", "text", "--no-cache", "--select", "D417"])
.pass_stdin(stdin),
@r###"
success: false
exit_code: 1
----- stdout -----
-:2:5: D417 Missing argument description in the docstring for `log`: `base`
Found 1 error.
----- stderr -----
"###
);
Ok(())
}

View file

@ -705,6 +705,10 @@ impl LintConfiguration {
let mut deprecated_nursery_selectors = FxHashSet::default(); let mut deprecated_nursery_selectors = FxHashSet::default();
let mut ignored_preview_selectors = FxHashSet::default(); let mut ignored_preview_selectors = FxHashSet::default();
// Track which docstring rules are specifically enabled
// which lets us override the docstring convention ignore-list
let mut docstring_overrides: FxHashSet<Rule> = FxHashSet::default();
for selection in &self.rule_selections { for selection in &self.rule_selections {
// If a selection only specifies extend-select we cannot directly // If a selection only specifies extend-select we cannot directly
// apply its rule selectors to the select_set because we firstly have // apply its rule selectors to the select_set because we firstly have
@ -716,6 +720,8 @@ impl LintConfiguration {
let mut select_map_updates: FxHashMap<Rule, bool> = FxHashMap::default(); let mut select_map_updates: FxHashMap<Rule, bool> = FxHashMap::default();
let mut fixable_map_updates: FxHashMap<Rule, bool> = FxHashMap::default(); let mut fixable_map_updates: FxHashMap<Rule, bool> = FxHashMap::default();
let mut docstring_override_updates: FxHashSet<Rule> = FxHashSet::default();
let carriedover_ignores = carryover_ignores.take(); let carriedover_ignores = carryover_ignores.take();
let carriedover_unfixables = carryover_unfixables.take(); let carriedover_unfixables = carryover_unfixables.take();
@ -730,6 +736,10 @@ impl LintConfiguration {
{ {
for rule in selector.rules(&preview) { for rule in selector.rules(&preview) {
select_map_updates.insert(rule, true); select_map_updates.insert(rule, true);
if spec == Specificity::Rule {
docstring_override_updates.insert(rule);
}
} }
} }
for selector in selection for selector in selection
@ -742,6 +752,7 @@ impl LintConfiguration {
select_map_updates.insert(rule, false); select_map_updates.insert(rule, false);
} }
} }
// Apply the same logic to `fixable` and `unfixable`. // Apply the same logic to `fixable` and `unfixable`.
for selector in selection for selector in selection
.fixable .fixable
@ -780,6 +791,8 @@ impl LintConfiguration {
{ {
carryover_ignores = Some(&selection.ignore); carryover_ignores = Some(&selection.ignore);
} }
docstring_overrides = docstring_override_updates;
} else { } else {
// Otherwise we apply the updates on top of the existing select_set. // Otherwise we apply the updates on top of the existing select_set.
for (rule, enabled) in select_map_updates { for (rule, enabled) in select_map_updates {
@ -789,6 +802,10 @@ impl LintConfiguration {
select_set.remove(rule); select_set.remove(rule);
} }
} }
for rule in docstring_override_updates {
docstring_overrides.insert(rule);
}
} }
// Apply the same logic to `fixable` and `unfixable`. // Apply the same logic to `fixable` and `unfixable`.
@ -881,15 +898,17 @@ impl LintConfiguration {
rules.enable(rule, fix); rules.enable(rule, fix);
} }
// If a docstring convention is specified, force-disable any incompatible error // If a docstring convention is specified, disable any incompatible error
// codes. // codes unless we are specifically overridden.
if let Some(convention) = self if let Some(convention) = self
.pydocstyle .pydocstyle
.as_ref() .as_ref()
.and_then(|pydocstyle| pydocstyle.convention) .and_then(|pydocstyle| pydocstyle.convention)
{ {
for rule in convention.rules_to_be_ignored() { for rule in convention.rules_to_be_ignored() {
rules.disable(*rule); if !docstring_overrides.contains(rule) {
rules.disable(*rule);
}
} }
} }
@ -1073,11 +1092,13 @@ pub fn resolve_src(src: &[String], project_root: &Path) -> Result<Vec<PathBuf>>
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
use crate::configuration::{LintConfiguration, RuleSelection}; use crate::configuration::{LintConfiguration, RuleSelection};
use crate::options::PydocstyleOptions;
use ruff_linter::codes::{Flake8Copyright, Pycodestyle, Refurb}; use ruff_linter::codes::{Flake8Copyright, Pycodestyle, Refurb};
use ruff_linter::registry::{Linter, Rule, RuleSet}; use ruff_linter::registry::{Linter, Rule, RuleSet};
use ruff_linter::rule_selector::PreviewOptions; use ruff_linter::rule_selector::PreviewOptions;
use ruff_linter::settings::types::PreviewMode; use ruff_linter::settings::types::PreviewMode;
use ruff_linter::RuleSelector; use ruff_linter::RuleSelector;
use std::str::FromStr;
const NURSERY_RULES: &[Rule] = &[ const NURSERY_RULES: &[Rule] = &[
Rule::MissingCopyrightNotice, Rule::MissingCopyrightNotice,
@ -1569,4 +1590,90 @@ mod tests {
let expected = RuleSet::from_rules(NURSERY_RULES); let expected = RuleSet::from_rules(NURSERY_RULES);
assert_eq!(actual, expected); assert_eq!(actual, expected);
} }
#[test]
fn select_docstring_convention_override() {
fn assert_override(rule_selections: Vec<RuleSelection>, should_be_overridden: bool) {
use ruff_linter::rules::pydocstyle::settings::Convention;
let config = LintConfiguration {
rule_selections,
pydocstyle: Some(PydocstyleOptions {
convention: Some(Convention::Numpy),
..PydocstyleOptions::default()
}),
..LintConfiguration::default()
};
let mut expected = RuleSet::from_rules(&[
Rule::from_code("D410").unwrap(),
Rule::from_code("D411").unwrap(),
Rule::from_code("D412").unwrap(),
Rule::from_code("D414").unwrap(),
Rule::from_code("D418").unwrap(),
Rule::from_code("D419").unwrap(),
]);
if should_be_overridden {
expected.insert(Rule::from_code("D417").unwrap());
}
assert_eq!(
config
.as_rule_table(PreviewMode::Disabled)
.iter_enabled()
.collect::<RuleSet>(),
expected,
);
}
let d41 = RuleSelector::from_str("D41").unwrap();
let d417 = RuleSelector::from_str("D417").unwrap();
// D417 does not appear when D41 is provided...
assert_override(
vec![RuleSelection {
select: Some(vec![d41.clone()]),
..RuleSelection::default()
}],
false,
);
// ...but does appear when specified directly.
assert_override(
vec![RuleSelection {
select: Some(vec![d41.clone(), d417.clone()]),
..RuleSelection::default()
}],
true,
);
// ...but disappears if there's a subsequent `--select`.
assert_override(
vec![
RuleSelection {
select: Some(vec![d417.clone()]),
..RuleSelection::default()
},
RuleSelection {
select: Some(vec![d41.clone()]),
..RuleSelection::default()
},
],
false,
);
// ...although an `--extend-select` is fine.
assert_override(
vec![
RuleSelection {
select: Some(vec![d417.clone()]),
..RuleSelection::default()
},
RuleSelection {
extend_select: vec![d41.clone()],
..RuleSelection::default()
},
],
true,
);
}
} }

View file

@ -2405,9 +2405,18 @@ pub struct PydocstyleOptions {
/// convention = "google" /// convention = "google"
/// ``` /// ```
/// ///
/// As conventions force-disable all rules not included in the convention, /// To modify a convention (i.e., to enable an additional rule that's excluded
/// enabling _additional_ rules on top of a convention is currently /// from the convention by default), select the desired rule via its fully
/// unsupported. /// qualified rule code (e.g., `D400` instead of `D4` or `D40`):
///
/// ```toml
/// [tool.ruff.lint]
/// # Enable D400 on top of the Google convention.
/// extend-select = ["D400"]
///
/// [tool.ruff.lint.pydocstyle]
/// convention = "google"
/// ```
#[option( #[option(
default = r#"null"#, default = r#"null"#,
value_type = r#""google" | "numpy" | "pep257""#, value_type = r#""google" | "numpy" | "pep257""#,

2
ruff.schema.json generated
View file

@ -2241,7 +2241,7 @@
"type": "object", "type": "object",
"properties": { "properties": {
"convention": { "convention": {
"description": "Whether to use Google-style or NumPy-style conventions or the [PEP 257](https://peps.python.org/pep-0257/) defaults when analyzing docstring sections.\n\nEnabling a convention will force-disable any rules that are not included in the specified convention. As such, the intended use is to enable a convention and then selectively disable any additional rules on top of it.\n\nFor example, to use Google-style conventions but avoid requiring documentation for every function parameter:\n\n```toml [tool.ruff.lint] # Enable all `pydocstyle` rules, limiting to those that adhere to the # Google convention via `convention = \"google\"`, below. select = [\"D\"]\n\n# On top of the Google convention, disable `D417`, which requires # documentation for every function parameter. ignore = [\"D417\"]\n\n[tool.ruff.lint.pydocstyle] convention = \"google\" ```\n\nAs conventions force-disable all rules not included in the convention, enabling _additional_ rules on top of a convention is currently unsupported.", "description": "Whether to use Google-style or NumPy-style conventions or the [PEP 257](https://peps.python.org/pep-0257/) defaults when analyzing docstring sections.\n\nEnabling a convention will force-disable any rules that are not included in the specified convention. As such, the intended use is to enable a convention and then selectively disable any additional rules on top of it.\n\nFor example, to use Google-style conventions but avoid requiring documentation for every function parameter:\n\n```toml [tool.ruff.lint] # Enable all `pydocstyle` rules, limiting to those that adhere to the # Google convention via `convention = \"google\"`, below. select = [\"D\"]\n\n# On top of the Google convention, disable `D417`, which requires # documentation for every function parameter. ignore = [\"D417\"]\n\n[tool.ruff.lint.pydocstyle] convention = \"google\" ```\n\nTo modify a convention (i.e., to enable an additional rule that's excluded from the convention by default), select the desired rule via its fully qualified rule code (e.g., `D400` instead of `D4` or `D40`):\n\n```toml [tool.ruff.lint] # Enable D400 on top of the Google convention. extend-select = [\"D400\"]\n\n[tool.ruff.lint.pydocstyle] convention = \"google\" ```",
"anyOf": [ "anyOf": [
{ {
"$ref": "#/definitions/Convention" "$ref": "#/definitions/Convention"