diff --git a/crates/ruff_cli/tests/integration_test.rs b/crates/ruff_cli/tests/integration_test.rs index 0cd9b9dd5ba4fd..59c372aac18065 100644 --- a/crates/ruff_cli/tests/integration_test.rs +++ b/crates/ruff_cli/tests/integration_test.rs @@ -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(()) +} diff --git a/crates/ruff_workspace/src/configuration.rs b/crates/ruff_workspace/src/configuration.rs index 1937596dbbe21f..459fd77fa062bc 100644 --- a/crates/ruff_workspace/src/configuration.rs +++ b/crates/ruff_workspace/src/configuration.rs @@ -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 = 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 @@ -716,6 +720,8 @@ impl LintConfiguration { let mut select_map_updates: FxHashMap = FxHashMap::default(); let mut fixable_map_updates: FxHashMap = FxHashMap::default(); + let mut docstring_override_updates: FxHashSet = FxHashSet::default(); + let carriedover_ignores = carryover_ignores.take(); let carriedover_unfixables = carryover_unfixables.take(); @@ -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 @@ -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 @@ -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 { @@ -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`. @@ -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); + } } } @@ -1073,11 +1092,13 @@ pub fn resolve_src(src: &[String], project_root: &Path) -> Result> #[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, @@ -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, 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::(), + 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, + ); + } } diff --git a/crates/ruff_workspace/src/options.rs b/crates/ruff_workspace/src/options.rs index 2b66a4653a4616..05094e145b48de 100644 --- a/crates/ruff_workspace/src/options.rs +++ b/crates/ruff_workspace/src/options.rs @@ -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""#, diff --git a/ruff.schema.json b/ruff.schema.json index 56aaa3c59cdb63..634946f8637e94 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -2241,7 +2241,7 @@ "type": "object", "properties": { "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": [ { "$ref": "#/definitions/Convention"