Skip to content

Commit

Permalink
Allow overriding pydocstyle convention rules (#8586)
Browse files Browse the repository at this point in the history
## 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`
  • Loading branch information
alanhdu authored Nov 10, 2023
1 parent 3e00ddc commit 5a1a8be
Show file tree
Hide file tree
Showing 4 changed files with 182 additions and 7 deletions.
59 changes: 59 additions & 0 deletions crates/ruff_cli/tests/integration_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1564,3 +1564,62 @@ extend-safe-fixes = ["UP03"]

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(())
}
113 changes: 110 additions & 3 deletions crates/ruff_workspace/src/configuration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -705,6 +705,10 @@ impl LintConfiguration {
let mut deprecated_nursery_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 {
// If a selection only specifies extend-select we cannot directly
// apply its rule selectors to the select_set because we firstly have
Expand All @@ -716,6 +720,8 @@ impl LintConfiguration {
let mut select_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_unfixables = carryover_unfixables.take();

Expand All @@ -730,6 +736,10 @@ impl LintConfiguration {
{
for rule in selector.rules(&preview) {
select_map_updates.insert(rule, true);

if spec == Specificity::Rule {
docstring_override_updates.insert(rule);
}
}
}
for selector in selection
Expand All @@ -742,6 +752,7 @@ impl LintConfiguration {
select_map_updates.insert(rule, false);
}
}

// Apply the same logic to `fixable` and `unfixable`.
for selector in selection
.fixable
Expand Down Expand Up @@ -780,6 +791,8 @@ impl LintConfiguration {
{
carryover_ignores = Some(&selection.ignore);
}

docstring_overrides = docstring_override_updates;
} else {
// Otherwise we apply the updates on top of the existing select_set.
for (rule, enabled) in select_map_updates {
Expand All @@ -789,6 +802,10 @@ impl LintConfiguration {
select_set.remove(rule);
}
}

for rule in docstring_override_updates {
docstring_overrides.insert(rule);
}
}

// Apply the same logic to `fixable` and `unfixable`.
Expand Down Expand Up @@ -881,15 +898,17 @@ impl LintConfiguration {
rules.enable(rule, fix);
}

// If a docstring convention is specified, force-disable any incompatible error
// codes.
// If a docstring convention is specified, disable any incompatible error
// codes unless we are specifically overridden.
if let Some(convention) = self
.pydocstyle
.as_ref()
.and_then(|pydocstyle| pydocstyle.convention)
{
for rule in convention.rules_to_be_ignored() {
rules.disable(*rule);
if !docstring_overrides.contains(rule) {
rules.disable(*rule);
}
}
}

Expand Down Expand Up @@ -1073,11 +1092,13 @@ pub fn resolve_src(src: &[String], project_root: &Path) -> Result<Vec<PathBuf>>
#[cfg(test)]
mod tests {
use crate::configuration::{LintConfiguration, RuleSelection};
use crate::options::PydocstyleOptions;
use ruff_linter::codes::{Flake8Copyright, Pycodestyle, Refurb};
use ruff_linter::registry::{Linter, Rule, RuleSet};
use ruff_linter::rule_selector::PreviewOptions;
use ruff_linter::settings::types::PreviewMode;
use ruff_linter::RuleSelector;
use std::str::FromStr;

const NURSERY_RULES: &[Rule] = &[
Rule::MissingCopyrightNotice,
Expand Down Expand Up @@ -1569,4 +1590,90 @@ mod tests {
let expected = RuleSet::from_rules(NURSERY_RULES);
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,
);
}
}
15 changes: 12 additions & 3 deletions crates/ruff_workspace/src/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2405,9 +2405,18 @@ pub struct PydocstyleOptions {
/// convention = "google"
/// ```
///
/// As conventions force-disable all rules not included in the convention,
/// enabling _additional_ rules on top of a convention is currently
/// unsupported.
/// To 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`):
///
/// ```toml
/// [tool.ruff.lint]
/// # Enable D400 on top of the Google convention.
/// extend-select = ["D400"]
///
/// [tool.ruff.lint.pydocstyle]
/// convention = "google"
/// ```
#[option(
default = r#"null"#,
value_type = r#""google" | "numpy" | "pep257""#,
Expand Down
2 changes: 1 addition & 1 deletion ruff.schema.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 5a1a8be

Please sign in to comment.