From e84c82424d2be4e1a27d6651286f9479fc90b0bb Mon Sep 17 00:00:00 2001 From: Auguste Lalande Date: Thu, 16 Jan 2025 17:05:10 -0500 Subject: [PATCH] [`pydoclint`] Allow ignoring one line docstrings for `DOC` rules (#13302) ## Summary Add a setting to allow ignoring one line docstrings for the pydoclint rules. Resolves #13086 Part of #12434 ## Test Plan Run tests with setting enabled. --------- Co-authored-by: dylwil3 --- crates/ruff_linter/src/rules/pydoclint/mod.rs | 29 ++++ .../rules/pydoclint/rules/check_docstring.rs | 18 ++ .../src/rules/pydoclint/settings.rs | 21 +++ ...tion_DOC501_google.py_ignore_one_line.snap | 159 ++++++++++++++++++ ...urns_DOC201_google.py_ignore_one_line.snap | 76 +++++++++ ...elds_DOC402_google.py_ignore_one_line.snap | 32 ++++ crates/ruff_linter/src/settings/mod.rs | 4 +- crates/ruff_workspace/src/configuration.rs | 9 +- crates/ruff_workspace/src/options.rs | 38 ++++- ruff.schema.json | 24 +++ 10 files changed, 407 insertions(+), 3 deletions(-) create mode 100644 crates/ruff_linter/src/rules/pydoclint/settings.rs create mode 100644 crates/ruff_linter/src/rules/pydoclint/snapshots/ruff_linter__rules__pydoclint__tests__docstring-missing-exception_DOC501_google.py_ignore_one_line.snap create mode 100644 crates/ruff_linter/src/rules/pydoclint/snapshots/ruff_linter__rules__pydoclint__tests__docstring-missing-returns_DOC201_google.py_ignore_one_line.snap create mode 100644 crates/ruff_linter/src/rules/pydoclint/snapshots/ruff_linter__rules__pydoclint__tests__docstring-missing-yields_DOC402_google.py_ignore_one_line.snap diff --git a/crates/ruff_linter/src/rules/pydoclint/mod.rs b/crates/ruff_linter/src/rules/pydoclint/mod.rs index c35d585b0e033..af4131e427425 100644 --- a/crates/ruff_linter/src/rules/pydoclint/mod.rs +++ b/crates/ruff_linter/src/rules/pydoclint/mod.rs @@ -1,5 +1,6 @@ //! Rules from [pydoclint](https://pypi.org/project/pydoclint/). pub(crate) mod rules; +pub mod settings; #[cfg(test)] mod tests { @@ -15,6 +16,8 @@ mod tests { use crate::test::test_path; use crate::{assert_messages, settings}; + use super::settings::Settings; + #[test_case(Rule::DocstringMissingException, Path::new("DOC501.py"))] fn rules(rule_code: Rule, path: &Path) -> Result<()> { let snapshot = format!("{}_{}", rule_code.as_ref(), path.to_string_lossy()); @@ -69,4 +72,30 @@ mod tests { assert_messages!(snapshot, diagnostics); Ok(()) } + + #[test_case(Rule::DocstringMissingReturns, Path::new("DOC201_google.py"))] + #[test_case(Rule::DocstringMissingYields, Path::new("DOC402_google.py"))] + #[test_case(Rule::DocstringMissingException, Path::new("DOC501_google.py"))] + fn rules_google_style_ignore_one_line(rule_code: Rule, path: &Path) -> Result<()> { + let snapshot = format!( + "{}_{}_ignore_one_line", + rule_code.as_ref(), + path.to_string_lossy() + ); + let diagnostics = test_path( + Path::new("pydoclint").join(path).as_path(), + &settings::LinterSettings { + pydoclint: Settings { + ignore_one_line_docstrings: true, + }, + pydocstyle: pydocstyle::settings::Settings { + convention: Some(Convention::Google), + ..pydocstyle::settings::Settings::default() + }, + ..settings::LinterSettings::for_rule(rule_code) + }, + )?; + assert_messages!(snapshot, diagnostics); + Ok(()) + } } diff --git a/crates/ruff_linter/src/rules/pydoclint/rules/check_docstring.rs b/crates/ruff_linter/src/rules/pydoclint/rules/check_docstring.rs index a5cd402c550bd..78cf46f4a6f35 100644 --- a/crates/ruff_linter/src/rules/pydoclint/rules/check_docstring.rs +++ b/crates/ruff_linter/src/rules/pydoclint/rules/check_docstring.rs @@ -9,6 +9,7 @@ use ruff_python_ast::visitor::Visitor; use ruff_python_ast::{self as ast, visitor, Expr, Stmt}; use ruff_python_semantic::analyze::{function_type, visibility}; use ruff_python_semantic::{Definition, SemanticModel}; +use ruff_source_file::NewlineWithTrailingNewline; use ruff_text_size::{Ranged, TextRange}; use crate::checkers::ast::Checker; @@ -843,6 +844,19 @@ fn is_generator_function_annotated_as_returning_none( .is_some_and(GeneratorOrIteratorArguments::indicates_none_returned) } +fn is_one_line(docstring: &Docstring) -> bool { + let mut non_empty_line_count = 0; + for line in NewlineWithTrailingNewline::from(docstring.body().as_str()) { + if !line.trim().is_empty() { + non_empty_line_count += 1; + } + if non_empty_line_count > 1 { + return false; + } + } + true +} + /// DOC201, DOC202, DOC402, DOC403, DOC501, DOC502 pub(crate) fn check_docstring( checker: &mut Checker, @@ -858,6 +872,10 @@ pub(crate) fn check_docstring( return; }; + if checker.settings.pydoclint.ignore_one_line_docstrings && is_one_line(docstring) { + return; + } + let semantic = checker.semantic(); if function_type::is_stub(function_def, semantic) { diff --git a/crates/ruff_linter/src/rules/pydoclint/settings.rs b/crates/ruff_linter/src/rules/pydoclint/settings.rs new file mode 100644 index 0000000000000..d54e06c1c795e --- /dev/null +++ b/crates/ruff_linter/src/rules/pydoclint/settings.rs @@ -0,0 +1,21 @@ +//! Settings for the `pydoclint` plugin. + +use crate::display_settings; +use ruff_macros::CacheKey; +use std::fmt::{Display, Formatter}; + +#[derive(Debug, Clone, Default, CacheKey)] +pub struct Settings { + pub ignore_one_line_docstrings: bool, +} + +impl Display for Settings { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + display_settings! { + formatter = f, + namespace = "linter.pydoclint", + fields = [self.ignore_one_line_docstrings] + } + Ok(()) + } +} diff --git a/crates/ruff_linter/src/rules/pydoclint/snapshots/ruff_linter__rules__pydoclint__tests__docstring-missing-exception_DOC501_google.py_ignore_one_line.snap b/crates/ruff_linter/src/rules/pydoclint/snapshots/ruff_linter__rules__pydoclint__tests__docstring-missing-exception_DOC501_google.py_ignore_one_line.snap new file mode 100644 index 0000000000000..b30b47f6f7efc --- /dev/null +++ b/crates/ruff_linter/src/rules/pydoclint/snapshots/ruff_linter__rules__pydoclint__tests__docstring-missing-exception_DOC501_google.py_ignore_one_line.snap @@ -0,0 +1,159 @@ +--- +source: crates/ruff_linter/src/rules/pydoclint/mod.rs +--- +DOC501_google.py:34:5: DOC501 Raised exception `FasterThanLightError` missing from docstring + | +32 | # DOC501 +33 | def calculate_speed(distance: float, time: float) -> float: +34 | / """Calculate speed as distance divided by time. +35 | | +36 | | Args: +37 | | distance: Distance traveled. +38 | | time: Time spent traveling. +39 | | +40 | | Returns: +41 | | Speed as distance divided by time. +42 | | """ + | |_______^ DOC501 +43 | try: +44 | return distance / time + | + = help: Add `FasterThanLightError` to the docstring + +DOC501_google.py:51:5: DOC501 Raised exception `ValueError` missing from docstring + | +49 | # DOC501 +50 | def calculate_speed(distance: float, time: float) -> float: +51 | / """Calculate speed as distance divided by time. +52 | | +53 | | Args: +54 | | distance: Distance traveled. +55 | | time: Time spent traveling. +56 | | +57 | | Returns: +58 | | Speed as distance divided by time. +59 | | """ + | |_______^ DOC501 +60 | try: +61 | return distance / time + | + = help: Add `ValueError` to the docstring + +DOC501_google.py:51:5: DOC501 Raised exception `FasterThanLightError` missing from docstring + | +49 | # DOC501 +50 | def calculate_speed(distance: float, time: float) -> float: +51 | / """Calculate speed as distance divided by time. +52 | | +53 | | Args: +54 | | distance: Distance traveled. +55 | | time: Time spent traveling. +56 | | +57 | | Returns: +58 | | Speed as distance divided by time. +59 | | """ + | |_______^ DOC501 +60 | try: +61 | return distance / time + | + = help: Add `FasterThanLightError` to the docstring + +DOC501_google.py:106:5: DOC501 Raised exception `AnotherError` missing from docstring + | +104 | # DOC501 +105 | def calculate_speed(distance: float, time: float) -> float: +106 | / """Calculate speed as distance divided by time. +107 | | +108 | | Args: +109 | | distance: Distance traveled. +110 | | time: Time spent traveling. +111 | | +112 | | Returns: +113 | | Speed as distance divided by time. +114 | | """ + | |_______^ DOC501 +115 | raise AnotherError + | + = help: Add `AnotherError` to the docstring + +DOC501_google.py:120:5: DOC501 Raised exception `AnotherError` missing from docstring + | +118 | # DOC501 +119 | def calculate_speed(distance: float, time: float) -> float: +120 | / """Calculate speed as distance divided by time. +121 | | +122 | | Args: +123 | | distance: Distance traveled. +124 | | time: Time spent traveling. +125 | | +126 | | Returns: +127 | | Speed as distance divided by time. +128 | | """ + | |_______^ DOC501 +129 | raise AnotherError() + | + = help: Add `AnotherError` to the docstring + +DOC501_google.py:134:5: DOC501 Raised exception `SomeError` missing from docstring + | +132 | # DOC501 +133 | def foo(bar: int): +134 | / """Foo. +135 | | +136 | | Args: +137 | | bar: Bar. +138 | | """ + | |_______^ DOC501 +139 | raise something.SomeError + | + = help: Add `SomeError` to the docstring + +DOC501_google.py:197:5: DOC501 Raised exception `ZeroDivisionError` missing from docstring + | +195 | # DOC501 +196 | def calculate_speed(distance: float, time: float) -> float: +197 | / """Calculate speed as distance divided by time. +198 | | +199 | | Args: +200 | | distance: Distance traveled. +201 | | time: Time spent traveling. +202 | | +203 | | Returns: +204 | | Speed as distance divided by time. +205 | | +206 | | Raises: +207 | | TypeError: if you didn't pass a number for both parameters +208 | | """ + | |_______^ DOC501 +209 | try: +210 | return distance / time + | + = help: Add `ZeroDivisionError` to the docstring + +DOC501_google.py:238:5: DOC501 Raised exception `TypeError` missing from docstring + | +237 | def foo(): +238 | / """Foo. +239 | | +240 | | Returns: +241 | | 42: int. +242 | | """ + | |_______^ DOC501 +243 | if True: +244 | raise TypeError # DOC501 + | + = help: Add `TypeError` to the docstring + +DOC501_google.py:238:5: DOC501 Raised exception `ValueError` missing from docstring + | +237 | def foo(): +238 | / """Foo. +239 | | +240 | | Returns: +241 | | 42: int. +242 | | """ + | |_______^ DOC501 +243 | if True: +244 | raise TypeError # DOC501 + | + = help: Add `ValueError` to the docstring diff --git a/crates/ruff_linter/src/rules/pydoclint/snapshots/ruff_linter__rules__pydoclint__tests__docstring-missing-returns_DOC201_google.py_ignore_one_line.snap b/crates/ruff_linter/src/rules/pydoclint/snapshots/ruff_linter__rules__pydoclint__tests__docstring-missing-returns_DOC201_google.py_ignore_one_line.snap new file mode 100644 index 0000000000000..87a9dd1aae47d --- /dev/null +++ b/crates/ruff_linter/src/rules/pydoclint/snapshots/ruff_linter__rules__pydoclint__tests__docstring-missing-returns_DOC201_google.py_ignore_one_line.snap @@ -0,0 +1,76 @@ +--- +source: crates/ruff_linter/src/rules/pydoclint/mod.rs +--- +DOC201_google.py:3:5: DOC201 `return` is not documented in docstring + | +1 | # DOC201 +2 | def foo(num: int) -> str: +3 | / """ +4 | | Do something +5 | | +6 | | Args: +7 | | num (int): A number +8 | | """ + | |_______^ DOC201 +9 | return 'test' + | + = help: Add a "Returns" section to the docstring + +DOC201_google.py:44:9: DOC201 `return` is not documented in docstring + | +42 | # DOC201 +43 | def bar(self) -> str: +44 | / """ +45 | | Do something +46 | | +47 | | Args: +48 | | num (int): A number +49 | | """ + | |___________^ DOC201 +50 | return 'test' + | + = help: Add a "Returns" section to the docstring + +DOC201_google.py:178:5: DOC201 `return` is not documented in docstring + | +176 | # DOC201 - non-early return explicit None +177 | def foo(x: int) -> int | None: +178 | / """A very helpful docstring. +179 | | +180 | | Args: +181 | | x (int): An integer. +182 | | """ + | |_______^ DOC201 +183 | if x < 0: +184 | return None + | + = help: Add a "Returns" section to the docstring + +DOC201_google.py:191:5: DOC201 `return` is not documented in docstring + | +189 | # DOC201 - non-early return explicit None w/o useful type annotations +190 | def foo(x): +191 | / """A very helpful docstring. +192 | | +193 | | Args: +194 | | x (int): An integer. +195 | | """ + | |_______^ DOC201 +196 | if x < 0: +197 | return None + | + = help: Add a "Returns" section to the docstring + +DOC201_google.py:204:5: DOC201 `return` is not documented in docstring + | +202 | # DOC201 - only returns None, but return annotation is not None +203 | def foo(s: str) -> str | None: +204 | / """A very helpful docstring. +205 | | +206 | | Args: +207 | | s (str): A string. +208 | | """ + | |_______^ DOC201 +209 | return None + | + = help: Add a "Returns" section to the docstring diff --git a/crates/ruff_linter/src/rules/pydoclint/snapshots/ruff_linter__rules__pydoclint__tests__docstring-missing-yields_DOC402_google.py_ignore_one_line.snap b/crates/ruff_linter/src/rules/pydoclint/snapshots/ruff_linter__rules__pydoclint__tests__docstring-missing-yields_DOC402_google.py_ignore_one_line.snap new file mode 100644 index 0000000000000..def7f04c9dc59 --- /dev/null +++ b/crates/ruff_linter/src/rules/pydoclint/snapshots/ruff_linter__rules__pydoclint__tests__docstring-missing-yields_DOC402_google.py_ignore_one_line.snap @@ -0,0 +1,32 @@ +--- +source: crates/ruff_linter/src/rules/pydoclint/mod.rs +--- +DOC402_google.py:3:5: DOC402 `yield` is not documented in docstring + | +1 | # DOC402 +2 | def foo(num: int) -> str: +3 | / """ +4 | | Do something +5 | | +6 | | Args: +7 | | num (int): A number +8 | | """ + | |_______^ DOC402 +9 | yield 'test' + | + = help: Add a "Yields" section to the docstring + +DOC402_google.py:44:9: DOC402 `yield` is not documented in docstring + | +42 | # DOC402 +43 | def bar(self) -> str: +44 | / """ +45 | | Do something +46 | | +47 | | Args: +48 | | num (int): A number +49 | | """ + | |___________^ DOC402 +50 | yield 'test' + | + = help: Add a "Yields" section to the docstring diff --git a/crates/ruff_linter/src/settings/mod.rs b/crates/ruff_linter/src/settings/mod.rs index 2bc98bc930a8f..047c768813c40 100644 --- a/crates/ruff_linter/src/settings/mod.rs +++ b/crates/ruff_linter/src/settings/mod.rs @@ -19,7 +19,7 @@ use crate::rules::{ flake8_comprehensions, flake8_copyright, flake8_errmsg, flake8_gettext, flake8_implicit_str_concat, flake8_import_conventions, flake8_pytest_style, flake8_quotes, flake8_self, flake8_tidy_imports, flake8_type_checking, flake8_unused_arguments, isort, mccabe, - pep8_naming, pycodestyle, pydocstyle, pyflakes, pylint, pyupgrade, ruff, + pep8_naming, pycodestyle, pydoclint, pydocstyle, pyflakes, pylint, pyupgrade, ruff, }; use crate::settings::types::{ CompiledPerFileIgnoreList, ExtensionMapping, FilePatternSet, PythonVersion, @@ -260,6 +260,7 @@ pub struct LinterSettings { pub mccabe: mccabe::settings::Settings, pub pep8_naming: pep8_naming::settings::Settings, pub pycodestyle: pycodestyle::settings::Settings, + pub pydoclint: pydoclint::settings::Settings, pub pydocstyle: pydocstyle::settings::Settings, pub pyflakes: pyflakes::settings::Settings, pub pylint: pylint::settings::Settings, @@ -425,6 +426,7 @@ impl LinterSettings { mccabe: mccabe::settings::Settings::default(), pep8_naming: pep8_naming::settings::Settings::default(), pycodestyle: pycodestyle::settings::Settings::default(), + pydoclint: pydoclint::settings::Settings::default(), pydocstyle: pydocstyle::settings::Settings::default(), pyflakes: pyflakes::settings::Settings::default(), pylint: pylint::settings::Settings::default(), diff --git a/crates/ruff_workspace/src/configuration.rs b/crates/ruff_workspace/src/configuration.rs index 877445b7664fd..53e36230950c2 100644 --- a/crates/ruff_workspace/src/configuration.rs +++ b/crates/ruff_workspace/src/configuration.rs @@ -49,7 +49,7 @@ use crate::options::{ Flake8QuotesOptions, Flake8SelfOptions, Flake8TidyImportsOptions, Flake8TypeCheckingOptions, Flake8UnusedArgumentsOptions, FormatOptions, IsortOptions, LintCommonOptions, LintOptions, McCabeOptions, Options, Pep8NamingOptions, PyUpgradeOptions, PycodestyleOptions, - PydocstyleOptions, PyflakesOptions, PylintOptions, RuffOptions, + PydoclintOptions, PydocstyleOptions, PyflakesOptions, PylintOptions, RuffOptions, }; use crate::settings::{ FileResolverSettings, FormatterSettings, LineEnding, Settings, EXCLUDE, INCLUDE, @@ -404,6 +404,10 @@ impl Configuration { ..pycodestyle::settings::Settings::default() } }, + pydoclint: lint + .pydoclint + .map(PydoclintOptions::into_settings) + .unwrap_or_default(), pydocstyle: lint .pydocstyle .map(PydocstyleOptions::into_settings) @@ -635,6 +639,7 @@ pub struct LintConfiguration { pub mccabe: Option, pub pep8_naming: Option, pub pycodestyle: Option, + pub pydoclint: Option, pub pydocstyle: Option, pub pyflakes: Option, pub pylint: Option, @@ -747,6 +752,7 @@ impl LintConfiguration { mccabe: options.common.mccabe, pep8_naming: options.common.pep8_naming, pycodestyle: options.common.pycodestyle, + pydoclint: options.pydoclint, pydocstyle: options.common.pydocstyle, pyflakes: options.common.pyflakes, pylint: options.common.pylint, @@ -1141,6 +1147,7 @@ impl LintConfiguration { mccabe: self.mccabe.combine(config.mccabe), pep8_naming: self.pep8_naming.combine(config.pep8_naming), pycodestyle: self.pycodestyle.combine(config.pycodestyle), + pydoclint: self.pydoclint.combine(config.pydoclint), pydocstyle: self.pydocstyle.combine(config.pydocstyle), pyflakes: self.pyflakes.combine(config.pyflakes), pylint: self.pylint.combine(config.pylint), diff --git a/crates/ruff_workspace/src/options.rs b/crates/ruff_workspace/src/options.rs index de18d2a42a33a..03e2971d5bc23 100644 --- a/crates/ruff_workspace/src/options.rs +++ b/crates/ruff_workspace/src/options.rs @@ -25,7 +25,7 @@ use ruff_linter::rules::{ flake8_copyright, flake8_errmsg, flake8_gettext, flake8_implicit_str_concat, flake8_import_conventions, flake8_pytest_style, flake8_quotes, flake8_self, flake8_tidy_imports, flake8_type_checking, flake8_unused_arguments, isort, mccabe, pep8_naming, - pycodestyle, pydocstyle, pyflakes, pylint, pyupgrade, ruff, + pycodestyle, pydoclint, pydocstyle, pyflakes, pylint, pyupgrade, ruff, }; use ruff_linter::settings::types::{ IdentifierPattern, OutputFormat, PythonVersion, RequiredVersion, @@ -469,6 +469,10 @@ pub struct LintOptions { )] pub exclude: Option>, + /// Options for the `pydoclint` plugin. + #[option_group] + pub pydoclint: Option, + /// Options for the `ruff` plugin #[option_group] pub ruff: Option, @@ -2938,6 +2942,35 @@ impl PydocstyleOptions { } } +#[derive( + Clone, Debug, PartialEq, Eq, Default, Serialize, Deserialize, OptionsMetadata, CombineOptions, +)] +#[serde(deny_unknown_fields, rename_all = "kebab-case")] +#[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))] +pub struct PydoclintOptions { + /// Skip docstrings which fit on a single line. + /// + /// Note: The corresponding setting in `pydoclint` + /// is named `skip-checking-short-docstrings`. + #[option( + default = r#"false"#, + value_type = "bool", + example = r#" + # Skip docstrings which fit on a single line. + ignore-one-line-docstrings = true + "# + )] + pub ignore_one_line_docstrings: Option, +} + +impl PydoclintOptions { + pub fn into_settings(self) -> pydoclint::settings::Settings { + pydoclint::settings::Settings { + ignore_one_line_docstrings: self.ignore_one_line_docstrings.unwrap_or_default(), + } + } +} + #[derive( Clone, Debug, PartialEq, Eq, Default, Serialize, Deserialize, OptionsMetadata, CombineOptions, )] @@ -3646,6 +3679,7 @@ pub struct LintOptionsWire { extend_per_file_ignores: Option>>, exclude: Option>, + pydoclint: Option, ruff: Option, preview: Option, } @@ -3699,6 +3733,7 @@ impl From for LintOptions { per_file_ignores, extend_per_file_ignores, exclude, + pydoclint, ruff, preview, } = value; @@ -3753,6 +3788,7 @@ impl From for LintOptions { extend_per_file_ignores, }, exclude, + pydoclint, ruff, preview, } diff --git a/ruff.schema.json b/ruff.schema.json index 707e22a133dd0..c676763d8ed5a 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -2296,6 +2296,17 @@ } ] }, + "pydoclint": { + "description": "Options for the `pydoclint` plugin.", + "anyOf": [ + { + "$ref": "#/definitions/PydoclintOptions" + }, + { + "type": "null" + } + ] + }, "pydocstyle": { "description": "Options for the `pydocstyle` plugin.", "anyOf": [ @@ -2545,6 +2556,19 @@ }, "additionalProperties": false }, + "PydoclintOptions": { + "type": "object", + "properties": { + "ignore-one-line-docstrings": { + "description": "Skip docstrings which fit on a single line.\n\nNote: The corresponding setting in `pydoclint` is named `skip-checking-short-docstrings`.", + "type": [ + "boolean", + "null" + ] + } + }, + "additionalProperties": false + }, "PydocstyleOptions": { "type": "object", "properties": {