From 4bc73dd87e6693a65e880d11eb1f2c28f6c57368 Mon Sep 17 00:00:00 2001 From: Auguste Lalande Date: Sat, 20 Jul 2024 15:41:51 -0400 Subject: [PATCH] [`pydoclint`] Implement `docstring-missing-exception` and `docstring-extraneous-exception` (`DOC501`, `DOC502`) (#11471) ## Summary These are the first rules implemented as part of #458, but I plan to implement more. Specifically, this implements `docstring-missing-exception` which checks for raised exceptions not documented in the docstring, and `docstring-extraneous-exception` which checks for exceptions in the docstring not present in the body. ## Test Plan Test fixtures added for both google and numpy style. --- LICENSE | 25 ++ .../test/fixtures/pydoclint/DOC501_google.py | 192 +++++++++ .../test/fixtures/pydoclint/DOC501_numpy.py | 78 ++++ .../test/fixtures/pydoclint/DOC502_google.py | 58 +++ .../test/fixtures/pydoclint/DOC502_numpy.py | 84 ++++ .../src/checkers/ast/analyze/definitions.rs | 38 +- crates/ruff_linter/src/codes.rs | 4 + crates/ruff_linter/src/docstrings/sections.rs | 8 +- crates/ruff_linter/src/registry.rs | 3 + crates/ruff_linter/src/rules/mod.rs | 1 + crates/ruff_linter/src/rules/pydoclint/mod.rs | 55 +++ .../rules/pydoclint/rules/check_docstring.rs | 382 ++++++++++++++++++ .../src/rules/pydoclint/rules/mod.rs | 3 + ...extraneous-exception_DOC502_google.py.snap | 38 ++ ...-extraneous-exception_DOC502_numpy.py.snap | 46 +++ ...ng-missing-exception_DOC501_google.py.snap | 52 +++ ...ing-missing-exception_DOC501_numpy.py.snap | 28 ++ .../src/rules/pydocstyle/helpers.rs | 61 +++ .../src/rules/pydocstyle/rules/sections.rs | 65 +-- ruff.schema.json | 5 + scripts/add_plugin.py | 2 +- 21 files changed, 1161 insertions(+), 67 deletions(-) create mode 100644 crates/ruff_linter/resources/test/fixtures/pydoclint/DOC501_google.py create mode 100644 crates/ruff_linter/resources/test/fixtures/pydoclint/DOC501_numpy.py create mode 100644 crates/ruff_linter/resources/test/fixtures/pydoclint/DOC502_google.py create mode 100644 crates/ruff_linter/resources/test/fixtures/pydoclint/DOC502_numpy.py create mode 100644 crates/ruff_linter/src/rules/pydoclint/mod.rs create mode 100644 crates/ruff_linter/src/rules/pydoclint/rules/check_docstring.rs create mode 100644 crates/ruff_linter/src/rules/pydoclint/rules/mod.rs create mode 100644 crates/ruff_linter/src/rules/pydoclint/snapshots/ruff_linter__rules__pydoclint__tests__docstring-extraneous-exception_DOC502_google.py.snap create mode 100644 crates/ruff_linter/src/rules/pydoclint/snapshots/ruff_linter__rules__pydoclint__tests__docstring-extraneous-exception_DOC502_numpy.py.snap create mode 100644 crates/ruff_linter/src/rules/pydoclint/snapshots/ruff_linter__rules__pydoclint__tests__docstring-missing-exception_DOC501_google.py.snap create mode 100644 crates/ruff_linter/src/rules/pydoclint/snapshots/ruff_linter__rules__pydoclint__tests__docstring-missing-exception_DOC501_numpy.py.snap diff --git a/LICENSE b/LICENSE index 04ed9285de85a..f5c3b02beccb0 100644 --- a/LICENSE +++ b/LICENSE @@ -1371,3 +1371,28 @@ are: OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. """ + +- pydoclint, licensed as follows: + """ + MIT License + + Copyright (c) 2023 jsh9 + + Permission is hereby granted, free of charge, to any person obtaining a copy + of this software and associated documentation files (the "Software"), to deal + in the Software without restriction, including without limitation the rights + to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + copies of the Software, and to permit persons to whom the Software is + furnished to do so, subject to the following conditions: + + The above copyright notice and this permission notice shall be included in all + copies or substantial portions of the Software. + + THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + SOFTWARE. + """ diff --git a/crates/ruff_linter/resources/test/fixtures/pydoclint/DOC501_google.py b/crates/ruff_linter/resources/test/fixtures/pydoclint/DOC501_google.py new file mode 100644 index 0000000000000..c5dc038b22497 --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/pydoclint/DOC501_google.py @@ -0,0 +1,192 @@ +import something +from somewhere import AnotherError + + +class FasterThanLightError(Exception): + ... + + +_some_error = Exception + + +# OK +def calculate_speed(distance: float, time: float) -> float: + """Calculate speed as distance divided by time. + + Args: + distance: Distance traveled. + time: Time spent traveling. + + Returns: + Speed as distance divided by time. + + Raises: + FasterThanLightError: If speed is greater than the speed of light. + """ + try: + return distance / time + except ZeroDivisionError as exc: + raise FasterThanLightError from exc + + +# DOC501 +def calculate_speed(distance: float, time: float) -> float: + """Calculate speed as distance divided by time. + + Args: + distance: Distance traveled. + time: Time spent traveling. + + Returns: + Speed as distance divided by time. + """ + try: + return distance / time + except ZeroDivisionError as exc: + raise FasterThanLightError from exc + + +# DOC501 +def calculate_speed(distance: float, time: float) -> float: + """Calculate speed as distance divided by time. + + Args: + distance: Distance traveled. + time: Time spent traveling. + + Returns: + Speed as distance divided by time. + """ + try: + return distance / time + except ZeroDivisionError as exc: + raise FasterThanLightError from exc + except: + raise ValueError + + +# DOC501 +def calculate_speed(distance: float, time: float) -> float: + """Calculate speed as distance divided by time. + + Args: + distance: Distance traveled. + time: Time spent traveling. + + Returns: + Speed as distance divided by time. + """ + try: + return distance / time + except ZeroDivisionError as exc: + print('oops') + raise exc + + +# DOC501 +def calculate_speed(distance: float, time: float) -> float: + """Calculate speed as distance divided by time. + + Args: + distance: Distance traveled. + time: Time spent traveling. + + Returns: + Speed as distance divided by time. + """ + try: + return distance / time + except (ZeroDivisionError, ValueError) as exc: + print('oops') + raise exc + + +# DOC501 +def calculate_speed(distance: float, time: float) -> float: + """Calculate speed as distance divided by time. + + Args: + distance: Distance traveled. + time: Time spent traveling. + + Returns: + Speed as distance divided by time. + """ + raise AnotherError + + +# DOC501 +def calculate_speed(distance: float, time: float) -> float: + """Calculate speed as distance divided by time. + + Args: + distance: Distance traveled. + time: Time spent traveling. + + Returns: + Speed as distance divided by time. + """ + raise AnotherError() + + +# DOC501 +def foo(bar: int): + """Foo. + + Args: + bar: Bar. + """ + raise something.SomeError + + +# DOC501, but can't resolve the error +def calculate_speed(distance: float, time: float) -> float: + """Calculate speed as distance divided by time. + + Args: + distance: Distance traveled. + time: Time spent traveling. + + Returns: + Speed as distance divided by time. + """ + raise _some_error + + +# OK +def calculate_speed(distance: float, time: float) -> float: + try: + return distance / time + except ZeroDivisionError as exc: + raise FasterThanLightError from exc + + +# OK +def calculate_speed(distance: float, time: float) -> float: + raise NotImplementedError + + +# OK +def foo(bar: int): + """Foo. + + Args: + bar: Bar. + + Raises: + SomeError: Wow. + """ + raise something.SomeError + + +# OK +def foo(bar: int): + """Foo. + + Args: + bar: Bar. + + Raises: + something.SomeError: Wow. + """ + raise something.SomeError diff --git a/crates/ruff_linter/resources/test/fixtures/pydoclint/DOC501_numpy.py b/crates/ruff_linter/resources/test/fixtures/pydoclint/DOC501_numpy.py new file mode 100644 index 0000000000000..f78beaec3f701 --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/pydoclint/DOC501_numpy.py @@ -0,0 +1,78 @@ +class FasterThanLightError(Exception): + ... + + +# OK +def calculate_speed(distance: float, time: float) -> float: + """ + Calculate speed as distance divided by time. + + Parameters + ---------- + distance : float + Distance traveled. + time : float + Time spent traveling. + + Returns + ------- + float + Speed as distance divided by time. + + Raises + ------ + FasterThanLightError + If speed is greater than the speed of light. + """ + try: + return distance / time + except ZeroDivisionError as exc: + raise FasterThanLightError from exc + + +# DOC501 +def calculate_speed(distance: float, time: float) -> float: + """ + Calculate speed as distance divided by time. + + Parameters + ---------- + distance : float + Distance traveled. + time : float + Time spent traveling. + + Returns + ------- + float + Speed as distance divided by time. + """ + try: + return distance / time + except ZeroDivisionError as exc: + raise FasterThanLightError from exc + + +# DOC501 +def calculate_speed(distance: float, time: float) -> float: + """ + Calculate speed as distance divided by time. + + Parameters + ---------- + distance : float + Distance traveled. + time : float + Time spent traveling. + + Returns + ------- + float + Speed as distance divided by time. + """ + try: + return distance / time + except ZeroDivisionError as exc: + raise FasterThanLightError from exc + except: + raise ValueError diff --git a/crates/ruff_linter/resources/test/fixtures/pydoclint/DOC502_google.py b/crates/ruff_linter/resources/test/fixtures/pydoclint/DOC502_google.py new file mode 100644 index 0000000000000..639a7965134f7 --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/pydoclint/DOC502_google.py @@ -0,0 +1,58 @@ +class FasterThanLightError(Exception): + ... + + +# DOC502 +def calculate_speed(distance: float, time: float) -> float: + """Calculate speed as distance divided by time. + + Args: + distance: Distance traveled. + time: Time spent traveling. + + Returns: + Speed as distance divided by time. + + Raises: + FasterThanLightError: If speed is greater than the speed of light. + """ + return distance / time + + +# DOC502 +def calculate_speed(distance: float, time: float) -> float: + """Calculate speed as distance divided by time. + + Args: + distance: Distance traveled. + time: Time spent traveling. + + Returns: + Speed as distance divided by time. + + Raises: + FasterThanLightError: If speed is greater than the speed of light. + DivisionByZero: Divide by zero. + """ + return distance / time + + +# DOC502 +def calculate_speed(distance: float, time: float) -> float: + """Calculate speed as distance divided by time. + + Args: + distance: Distance traveled. + time: Time spent traveling. + + Returns: + Speed as distance divided by time. + + Raises: + FasterThanLightError: If speed is greater than the speed of light. + DivisionByZero: Divide by zero. + """ + try: + return distance / time + except ZeroDivisionError as exc: + raise FasterThanLightError from exc diff --git a/crates/ruff_linter/resources/test/fixtures/pydoclint/DOC502_numpy.py b/crates/ruff_linter/resources/test/fixtures/pydoclint/DOC502_numpy.py new file mode 100644 index 0000000000000..95b84e813495c --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/pydoclint/DOC502_numpy.py @@ -0,0 +1,84 @@ +class FasterThanLightError(Exception): + ... + + +# DOC502 +def calculate_speed(distance: float, time: float) -> float: + """ + Calculate speed as distance divided by time. + + Parameters + ---------- + distance : float + Distance traveled. + time : float + Time spent traveling. + + Returns + ------- + float + Speed as distance divided by time. + + Raises + ------ + FasterThanLightError + If speed is greater than the speed of light. + """ + return distance / time + + +# DOC502 +def calculate_speed(distance: float, time: float) -> float: + """ + Calculate speed as distance divided by time. + + Parameters + ---------- + distance : float + Distance traveled. + time : float + Time spent traveling. + + Returns + ------- + float + Speed as distance divided by time. + + Raises + ------ + FasterThanLightError + If speed is greater than the speed of light. + DivisionByZero + If attempting to divide by zero. + """ + return distance / time + + +# DOC502 +def calculate_speed(distance: float, time: float) -> float: + """ + Calculate speed as distance divided by time. + + Parameters + ---------- + distance : float + Distance traveled. + time : float + Time spent traveling. + + Returns + ------- + float + Speed as distance divided by time. + + Raises + ------ + FasterThanLightError + If speed is greater than the speed of light. + DivisionByZero + If attempting to divide by zero. + """ + try: + return distance / time + except ZeroDivisionError as exc: + raise FasterThanLightError from exc diff --git a/crates/ruff_linter/src/checkers/ast/analyze/definitions.rs b/crates/ruff_linter/src/checkers/ast/analyze/definitions.rs index ab191fb4b5160..e119ac5dd39dc 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/definitions.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/definitions.rs @@ -10,7 +10,7 @@ use crate::checkers::ast::Checker; use crate::codes::Rule; use crate::docstrings::Docstring; use crate::fs::relativize_path; -use crate::rules::{flake8_annotations, flake8_pyi, pydocstyle, pylint}; +use crate::rules::{flake8_annotations, flake8_pyi, pydoclint, pydocstyle, pylint}; use crate::{docstrings, warn_user}; /// Run lint rules over all [`Definition`] nodes in the [`SemanticModel`]. @@ -83,12 +83,17 @@ pub(crate) fn definitions(checker: &mut Checker) { Rule::UndocumentedPublicNestedClass, Rule::UndocumentedPublicPackage, ]); + let enforce_pydoclint = checker.any_enabled(&[ + Rule::DocstringMissingException, + Rule::DocstringExtraneousException, + ]); if !enforce_annotations && !enforce_docstrings && !enforce_stubs && !enforce_stubs_and_runtime && !enforce_dunder_method + && !enforce_pydoclint { return; } @@ -163,8 +168,8 @@ pub(crate) fn definitions(checker: &mut Checker) { } } - // pydocstyle - if enforce_docstrings { + // pydocstyle, pydoclint + if enforce_docstrings || enforce_pydoclint { if pydocstyle::helpers::should_ignore_definition( definition, &checker.settings.pydocstyle.ignore_decorators, @@ -282,7 +287,8 @@ pub(crate) fn definitions(checker: &mut Checker) { if checker.enabled(Rule::OverloadWithDocstring) { pydocstyle::rules::if_needed(checker, &docstring); } - if checker.any_enabled(&[ + + let enforce_sections = checker.any_enabled(&[ Rule::BlankLineAfterLastSection, Rule::BlankLinesBetweenHeaderAndContent, Rule::CapitalizeSectionName, @@ -298,12 +304,30 @@ pub(crate) fn definitions(checker: &mut Checker) { Rule::SectionUnderlineMatchesSectionLength, Rule::SectionUnderlineNotOverIndented, Rule::UndocumentedParam, - ]) { - pydocstyle::rules::sections( - checker, + ]); + if enforce_sections || enforce_pydoclint { + let section_contexts = pydocstyle::helpers::get_section_contexts( &docstring, checker.settings.pydocstyle.convention.as_ref(), ); + + if enforce_sections { + pydocstyle::rules::sections( + checker, + &docstring, + §ion_contexts, + checker.settings.pydocstyle.convention.as_ref(), + ); + } + + if enforce_pydoclint { + pydoclint::rules::check_docstring( + checker, + definition, + §ion_contexts, + checker.settings.pydocstyle.convention.as_ref(), + ); + } } } } diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index 2116c89dd9eb7..4d4e5452ced49 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -912,6 +912,10 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Numpy, "003") => (RuleGroup::Stable, rules::numpy::rules::NumpyDeprecatedFunction), (Numpy, "201") => (RuleGroup::Stable, rules::numpy::rules::Numpy2Deprecation), + // pydoclint + (Pydoclint, "501") => (RuleGroup::Preview, rules::pydoclint::rules::DocstringMissingException), + (Pydoclint, "502") => (RuleGroup::Preview, rules::pydoclint::rules::DocstringExtraneousException), + // ruff (Ruff, "001") => (RuleGroup::Stable, rules::ruff::rules::AmbiguousUnicodeCharacterString), (Ruff, "002") => (RuleGroup::Stable, rules::ruff::rules::AmbiguousUnicodeCharacterDocstring), diff --git a/crates/ruff_linter/src/docstrings/sections.rs b/crates/ruff_linter/src/docstrings/sections.rs index a6560084ff48b..1068a0db07535 100644 --- a/crates/ruff_linter/src/docstrings/sections.rs +++ b/crates/ruff_linter/src/docstrings/sections.rs @@ -163,6 +163,7 @@ impl SectionKind { pub(crate) struct SectionContexts<'a> { contexts: Vec, docstring: &'a Docstring<'a>, + style: SectionStyle, } impl<'a> SectionContexts<'a> { @@ -221,9 +222,14 @@ impl<'a> SectionContexts<'a> { Self { contexts, docstring, + style, } } + pub(crate) fn style(&self) -> SectionStyle { + self.style + } + pub(crate) fn len(&self) -> usize { self.contexts.len() } @@ -396,7 +402,7 @@ impl<'a> SectionContext<'a> { NewlineWithTrailingNewline::with_offset(lines, self.offset() + self.data.summary_full_end) } - fn following_lines_str(&self) -> &'a str { + pub(crate) fn following_lines_str(&self) -> &'a str { &self.docstring_body.as_str()[self.following_range_relative()] } diff --git a/crates/ruff_linter/src/registry.rs b/crates/ruff_linter/src/registry.rs index 6cb5b39c922fb..35c797779ba98 100644 --- a/crates/ruff_linter/src/registry.rs +++ b/crates/ruff_linter/src/registry.rs @@ -202,6 +202,9 @@ pub enum Linter { /// [refurb](https://pypi.org/project/refurb/) #[prefix = "FURB"] Refurb, + /// [pydoclint](https://pypi.org/project/pydoclint/) + #[prefix = "DOC"] + Pydoclint, /// Ruff-specific rules #[prefix = "RUF"] Ruff, diff --git a/crates/ruff_linter/src/rules/mod.rs b/crates/ruff_linter/src/rules/mod.rs index 6240d93d12719..f1eba35e85c3d 100644 --- a/crates/ruff_linter/src/rules/mod.rs +++ b/crates/ruff_linter/src/rules/mod.rs @@ -48,6 +48,7 @@ pub mod pandas_vet; pub mod pep8_naming; pub mod perflint; pub mod pycodestyle; +pub mod pydoclint; pub mod pydocstyle; pub mod pyflakes; pub mod pygrep_hooks; diff --git a/crates/ruff_linter/src/rules/pydoclint/mod.rs b/crates/ruff_linter/src/rules/pydoclint/mod.rs new file mode 100644 index 0000000000000..539f310b91e51 --- /dev/null +++ b/crates/ruff_linter/src/rules/pydoclint/mod.rs @@ -0,0 +1,55 @@ +//! Rules from [pydoclint](https://pypi.org/project/pydoclint/). +pub(crate) mod rules; + +#[cfg(test)] +mod tests { + use std::collections::BTreeSet; + use std::convert::AsRef; + use std::path::Path; + + use anyhow::Result; + use test_case::test_case; + + use crate::registry::Rule; + use crate::rules::pydocstyle::settings::{Convention, Settings}; + use crate::test::test_path; + use crate::{assert_messages, settings}; + + #[test_case(Rule::DocstringMissingException, Path::new("DOC501_google.py"))] + #[test_case(Rule::DocstringExtraneousException, Path::new("DOC502_google.py"))] + fn rules_google_style(rule_code: Rule, path: &Path) -> Result<()> { + let snapshot = format!("{}_{}", rule_code.as_ref(), path.to_string_lossy()); + let diagnostics = test_path( + Path::new("pydoclint").join(path).as_path(), + &settings::LinterSettings { + pydocstyle: Settings { + convention: Some(Convention::Google), + ignore_decorators: BTreeSet::new(), + property_decorators: BTreeSet::new(), + }, + ..settings::LinterSettings::for_rule(rule_code) + }, + )?; + assert_messages!(snapshot, diagnostics); + Ok(()) + } + + #[test_case(Rule::DocstringMissingException, Path::new("DOC501_numpy.py"))] + #[test_case(Rule::DocstringExtraneousException, Path::new("DOC502_numpy.py"))] + fn rules_numpy_style(rule_code: Rule, path: &Path) -> Result<()> { + let snapshot = format!("{}_{}", rule_code.as_ref(), path.to_string_lossy()); + let diagnostics = test_path( + Path::new("pydoclint").join(path).as_path(), + &settings::LinterSettings { + pydocstyle: Settings { + convention: Some(Convention::Numpy), + ignore_decorators: BTreeSet::new(), + property_decorators: BTreeSet::new(), + }, + ..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 new file mode 100644 index 0000000000000..e85d91fd1cca2 --- /dev/null +++ b/crates/ruff_linter/src/rules/pydoclint/rules/check_docstring.rs @@ -0,0 +1,382 @@ +use itertools::Itertools; +use ruff_diagnostics::Diagnostic; +use ruff_diagnostics::Violation; +use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast::name::QualifiedName; +use ruff_python_ast::visitor::{self, Visitor}; +use ruff_python_ast::{self as ast, Expr, Stmt}; +use ruff_python_semantic::{Definition, MemberKind, SemanticModel}; +use ruff_text_size::{Ranged, TextRange}; + +use crate::checkers::ast::Checker; +use crate::docstrings::sections::{SectionContexts, SectionKind}; +use crate::docstrings::styles::SectionStyle; +use crate::registry::Rule; +use crate::rules::pydocstyle::settings::Convention; + +/// ## What it does +/// Checks for function docstrings that do not include documentation for all +/// explicitly-raised exceptions. +/// +/// ## Why is this bad? +/// If a raise is mentioned in a docstring, but the function itself does not +/// explicitly raise it, it can be misleading to users and/or a sign of +/// incomplete documentation or refactors. +/// +/// ## Example +/// ```python +/// def calculate_speed(distance: float, time: float) -> float: +/// """Calculate speed as distance divided by time. +/// +/// Args: +/// distance: Distance traveled. +/// time: Time spent traveling. +/// +/// Returns: +/// Speed as distance divided by time. +/// """ +/// try: +/// return distance / time +/// except ZeroDivisionError as exc: +/// raise FasterThanLightError from exc +/// ``` +/// +/// Use instead: +/// ```python +/// def calculate_speed(distance: float, time: float) -> float: +/// """Calculate speed as distance divided by time. +/// +/// Args: +/// distance: Distance traveled. +/// time: Time spent traveling. +/// +/// Returns: +/// Speed as distance divided by time. +/// +/// Raises: +/// FasterThanLightError: If speed is greater than the speed of light. +/// """ +/// try: +/// return distance / time +/// except ZeroDivisionError as exc: +/// raise FasterThanLightError from exc +/// ``` +#[violation] +pub struct DocstringMissingException { + id: String, +} + +impl Violation for DocstringMissingException { + #[derive_message_formats] + fn message(&self) -> String { + let DocstringMissingException { id } = self; + format!("Raised exception `{id}` missing from docstring") + } +} + +/// ## What it does +/// Checks for function docstrings that include exceptions which are not +/// explicitly raised. +/// +/// ## Why is this bad? +/// Some conventions prefer non-explicit exceptions be omitted from the +/// docstring. +/// +/// ## Example +/// ```python +/// def calculate_speed(distance: float, time: float) -> float: +/// """Calculate speed as distance divided by time. +/// +/// Args: +/// distance: Distance traveled. +/// time: Time spent traveling. +/// +/// Returns: +/// Speed as distance divided by time. +/// +/// Raises: +/// ZeroDivisionError: Divided by zero. +/// """ +/// return distance / time +/// ``` +/// +/// Use instead: +/// ```python +/// def calculate_speed(distance: float, time: float) -> float: +/// """Calculate speed as distance divided by time. +/// +/// Args: +/// distance: Distance traveled. +/// time: Time spent traveling. +/// +/// Returns: +/// Speed as distance divided by time. +/// """ +/// return distance / time +/// ``` +#[violation] +pub struct DocstringExtraneousException { + ids: Vec, +} + +impl Violation for DocstringExtraneousException { + #[derive_message_formats] + fn message(&self) -> String { + let DocstringExtraneousException { ids } = self; + + if let [id] = ids.as_slice() { + format!("Raised exception is not explicitly raised: `{id}`") + } else { + format!( + "Raised exceptions are not explicitly raised: {}", + ids.iter().map(|id| format!("`{id}`")).join(", ") + ) + } + } +} + +#[derive(Debug)] +struct DocstringEntries<'a> { + raised_exceptions: Vec>, + raised_exceptions_range: TextRange, +} + +impl<'a> DocstringEntries<'a> { + /// Return the raised exceptions for the docstring, or `None` if the docstring does not contain + /// a `Raises` section. + fn from_sections(sections: &'a SectionContexts, style: SectionStyle) -> Option { + for section in sections.iter() { + if section.kind() == SectionKind::Raises { + return Some(Self { + raised_exceptions: parse_entries(section.following_lines_str(), style), + raised_exceptions_range: section.range(), + }); + } + } + None + } +} + +impl Ranged for DocstringEntries<'_> { + fn range(&self) -> TextRange { + self.raised_exceptions_range + } +} + +/// Parse the entries in a `Raises` section of a docstring. +fn parse_entries(content: &str, style: SectionStyle) -> Vec { + match style { + SectionStyle::Google => parse_entries_google(content), + SectionStyle::Numpy => parse_entries_numpy(content), + } +} + +/// Parses Google-style docstring sections of the form: +/// +/// ```python +/// Raises: +/// FasterThanLightError: If speed is greater than the speed of light. +/// DivisionByZero: If attempting to divide by zero. +/// ``` +fn parse_entries_google(content: &str) -> Vec { + let mut entries: Vec = Vec::new(); + for potential in content.split('\n') { + let Some(colon_idx) = potential.find(':') else { + continue; + }; + let entry = potential[..colon_idx].trim(); + entries.push(QualifiedName::user_defined(entry)); + } + entries +} + +/// Parses NumPy-style docstring sections of the form: +/// +/// ```python +/// Raises +/// ------ +/// FasterThanLightError +/// If speed is greater than the speed of light. +/// DivisionByZero +/// If attempting to divide by zero. +/// ``` +fn parse_entries_numpy(content: &str) -> Vec { + let mut entries: Vec = Vec::new(); + let mut split = content.split('\n'); + let Some(dashes) = split.next() else { + return entries; + }; + let indentation = dashes.len() - dashes.trim_start().len(); + for potential in split { + if let Some(first_char) = potential.chars().nth(indentation) { + if !first_char.is_whitespace() { + let entry = potential[indentation..].trim(); + entries.push(QualifiedName::user_defined(entry)); + } + } + } + entries +} + +/// An individual exception raised in a function body. +#[derive(Debug)] +struct Entry<'a> { + qualified_name: QualifiedName<'a>, + range: TextRange, +} + +impl Ranged for Entry<'_> { + fn range(&self) -> TextRange { + self.range + } +} + +/// The exceptions raised in a function body. +#[derive(Debug)] +struct BodyEntries<'a> { + raised_exceptions: Vec>, +} + +/// An AST visitor to extract the raised exceptions from a function body. +struct BodyVisitor<'a> { + raised_exceptions: Vec>, + semantic: &'a SemanticModel<'a>, +} + +impl<'a> BodyVisitor<'a> { + fn new(semantic: &'a SemanticModel) -> Self { + Self { + raised_exceptions: Vec::new(), + semantic, + } + } + + fn finish(self) -> BodyEntries<'a> { + BodyEntries { + raised_exceptions: self.raised_exceptions, + } + } +} + +impl<'a> Visitor<'a> for BodyVisitor<'a> { + fn visit_stmt(&mut self, stmt: &'a Stmt) { + if let Stmt::Raise(ast::StmtRaise { exc: Some(exc), .. }) = stmt { + if let Some(qualified_name) = extract_raised_exception(self.semantic, exc.as_ref()) { + self.raised_exceptions.push(Entry { + qualified_name, + range: exc.as_ref().range(), + }); + } + } + visitor::walk_stmt(self, stmt); + } +} + +fn extract_raised_exception<'a>( + semantic: &SemanticModel<'a>, + exc: &'a Expr, +) -> Option> { + if let Some(qualified_name) = semantic.resolve_qualified_name(exc) { + return Some(qualified_name); + } + if let Expr::Call(ast::ExprCall { func, .. }) = exc { + return extract_raised_exception(semantic, func.as_ref()); + } + None +} + +/// DOC501, DOC502 +pub(crate) fn check_docstring( + checker: &mut Checker, + definition: &Definition, + section_contexts: &SectionContexts, + convention: Option<&Convention>, +) { + let mut diagnostics = Vec::new(); + let Definition::Member(member) = definition else { + return; + }; + + // Only check function docstrings. + if matches!( + member.kind, + MemberKind::Class(_) | MemberKind::NestedClass(_) + ) { + return; + } + + // Prioritize the specified convention over the determined style. + let docstring_entries = match convention { + Some(Convention::Google) => { + DocstringEntries::from_sections(section_contexts, SectionStyle::Google) + } + Some(Convention::Numpy) => { + DocstringEntries::from_sections(section_contexts, SectionStyle::Numpy) + } + _ => DocstringEntries::from_sections(section_contexts, section_contexts.style()), + }; + + let body_entries = { + let mut visitor = BodyVisitor::new(checker.semantic()); + visitor::walk_body(&mut visitor, member.body()); + visitor.finish() + }; + + // DOC501 + if checker.enabled(Rule::DocstringMissingException) { + for body_raise in &body_entries.raised_exceptions { + let Some(name) = body_raise.qualified_name.segments().last() else { + continue; + }; + + if *name == "NotImplementedError" { + continue; + } + + if !docstring_entries.as_ref().is_some_and(|entries| { + entries.raised_exceptions.iter().any(|exception| { + body_raise + .qualified_name + .segments() + .ends_with(exception.segments()) + }) + }) { + let diagnostic = Diagnostic::new( + DocstringMissingException { + id: (*name).to_string(), + }, + body_raise.range(), + ); + diagnostics.push(diagnostic); + } + } + } + + // DOC502 + if checker.enabled(Rule::DocstringExtraneousException) { + if let Some(docstring_entries) = docstring_entries { + let mut extraneous_exceptions = Vec::new(); + for docstring_raise in &docstring_entries.raised_exceptions { + if !body_entries.raised_exceptions.iter().any(|exception| { + exception + .qualified_name + .segments() + .ends_with(docstring_raise.segments()) + }) { + extraneous_exceptions.push(docstring_raise.to_string()); + } + } + if !extraneous_exceptions.is_empty() { + let diagnostic = Diagnostic::new( + DocstringExtraneousException { + ids: extraneous_exceptions, + }, + docstring_entries.range(), + ); + diagnostics.push(diagnostic); + } + } + } + + checker.diagnostics.extend(diagnostics); +} diff --git a/crates/ruff_linter/src/rules/pydoclint/rules/mod.rs b/crates/ruff_linter/src/rules/pydoclint/rules/mod.rs new file mode 100644 index 0000000000000..de7b36c2c5c71 --- /dev/null +++ b/crates/ruff_linter/src/rules/pydoclint/rules/mod.rs @@ -0,0 +1,3 @@ +pub(crate) use check_docstring::*; + +mod check_docstring; diff --git a/crates/ruff_linter/src/rules/pydoclint/snapshots/ruff_linter__rules__pydoclint__tests__docstring-extraneous-exception_DOC502_google.py.snap b/crates/ruff_linter/src/rules/pydoclint/snapshots/ruff_linter__rules__pydoclint__tests__docstring-extraneous-exception_DOC502_google.py.snap new file mode 100644 index 0000000000000..8ef9ed882159f --- /dev/null +++ b/crates/ruff_linter/src/rules/pydoclint/snapshots/ruff_linter__rules__pydoclint__tests__docstring-extraneous-exception_DOC502_google.py.snap @@ -0,0 +1,38 @@ +--- +source: crates/ruff_linter/src/rules/pydoclint/mod.rs +--- +DOC502_google.py:16:1: DOC502 Raised exception is not explicitly raised: `FasterThanLightError` + | +14 | Speed as distance divided by time. +15 | +16 | / Raises: +17 | | FasterThanLightError: If speed is greater than the speed of light. +18 | | """ + | |____^ DOC502 +19 | return distance / time + | + +DOC502_google.py:33:1: DOC502 Raised exceptions are not explicitly raised: `FasterThanLightError`, `DivisionByZero` + | +31 | Speed as distance divided by time. +32 | +33 | / Raises: +34 | | FasterThanLightError: If speed is greater than the speed of light. +35 | | DivisionByZero: Divide by zero. +36 | | """ + | |____^ DOC502 +37 | return distance / time + | + +DOC502_google.py:51:1: DOC502 Raised exception is not explicitly raised: `DivisionByZero` + | +49 | Speed as distance divided by time. +50 | +51 | / Raises: +52 | | FasterThanLightError: If speed is greater than the speed of light. +53 | | DivisionByZero: Divide by zero. +54 | | """ + | |____^ DOC502 +55 | try: +56 | return distance / time + | diff --git a/crates/ruff_linter/src/rules/pydoclint/snapshots/ruff_linter__rules__pydoclint__tests__docstring-extraneous-exception_DOC502_numpy.py.snap b/crates/ruff_linter/src/rules/pydoclint/snapshots/ruff_linter__rules__pydoclint__tests__docstring-extraneous-exception_DOC502_numpy.py.snap new file mode 100644 index 0000000000000..41498f2f6e03b --- /dev/null +++ b/crates/ruff_linter/src/rules/pydoclint/snapshots/ruff_linter__rules__pydoclint__tests__docstring-extraneous-exception_DOC502_numpy.py.snap @@ -0,0 +1,46 @@ +--- +source: crates/ruff_linter/src/rules/pydoclint/mod.rs +--- +DOC502_numpy.py:22:1: DOC502 Raised exception is not explicitly raised: `FasterThanLightError` + | +20 | Speed as distance divided by time. +21 | +22 | / Raises +23 | | ------ +24 | | FasterThanLightError +25 | | If speed is greater than the speed of light. +26 | | """ + | |____^ DOC502 +27 | return distance / time + | + +DOC502_numpy.py:47:1: DOC502 Raised exceptions are not explicitly raised: `FasterThanLightError`, `DivisionByZero` + | +45 | Speed as distance divided by time. +46 | +47 | / Raises +48 | | ------ +49 | | FasterThanLightError +50 | | If speed is greater than the speed of light. +51 | | DivisionByZero +52 | | If attempting to divide by zero. +53 | | """ + | |____^ DOC502 +54 | return distance / time + | + +DOC502_numpy.py:74:1: DOC502 Raised exception is not explicitly raised: `DivisionByZero` + | +72 | Speed as distance divided by time. +73 | +74 | / Raises +75 | | ------ +76 | | FasterThanLightError +77 | | If speed is greater than the speed of light. +78 | | DivisionByZero +79 | | If attempting to divide by zero. +80 | | """ + | |____^ DOC502 +81 | try: +82 | return distance / time + | diff --git a/crates/ruff_linter/src/rules/pydoclint/snapshots/ruff_linter__rules__pydoclint__tests__docstring-missing-exception_DOC501_google.py.snap b/crates/ruff_linter/src/rules/pydoclint/snapshots/ruff_linter__rules__pydoclint__tests__docstring-missing-exception_DOC501_google.py.snap new file mode 100644 index 0000000000000..8ea9749d5246d --- /dev/null +++ b/crates/ruff_linter/src/rules/pydoclint/snapshots/ruff_linter__rules__pydoclint__tests__docstring-missing-exception_DOC501_google.py.snap @@ -0,0 +1,52 @@ +--- +source: crates/ruff_linter/src/rules/pydoclint/mod.rs +--- +DOC501_google.py:46:15: DOC501 Raised exception `FasterThanLightError` missing from docstring + | +44 | return distance / time +45 | except ZeroDivisionError as exc: +46 | raise FasterThanLightError from exc + | ^^^^^^^^^^^^^^^^^^^^ DOC501 + | + +DOC501_google.py:63:15: DOC501 Raised exception `FasterThanLightError` missing from docstring + | +61 | return distance / time +62 | except ZeroDivisionError as exc: +63 | raise FasterThanLightError from exc + | ^^^^^^^^^^^^^^^^^^^^ DOC501 +64 | except: +65 | raise ValueError + | + +DOC501_google.py:65:15: DOC501 Raised exception `ValueError` missing from docstring + | +63 | raise FasterThanLightError from exc +64 | except: +65 | raise ValueError + | ^^^^^^^^^^ DOC501 + | + +DOC501_google.py:115:11: DOC501 Raised exception `AnotherError` missing from docstring + | +113 | Speed as distance divided by time. +114 | """ +115 | raise AnotherError + | ^^^^^^^^^^^^ DOC501 + | + +DOC501_google.py:129:11: DOC501 Raised exception `AnotherError` missing from docstring + | +127 | Speed as distance divided by time. +128 | """ +129 | raise AnotherError() + | ^^^^^^^^^^^^^^ DOC501 + | + +DOC501_google.py:139:11: DOC501 Raised exception `SomeError` missing from docstring + | +137 | bar: Bar. +138 | """ +139 | raise something.SomeError + | ^^^^^^^^^^^^^^^^^^^ DOC501 + | diff --git a/crates/ruff_linter/src/rules/pydoclint/snapshots/ruff_linter__rules__pydoclint__tests__docstring-missing-exception_DOC501_numpy.py.snap b/crates/ruff_linter/src/rules/pydoclint/snapshots/ruff_linter__rules__pydoclint__tests__docstring-missing-exception_DOC501_numpy.py.snap new file mode 100644 index 0000000000000..f91ec86eb3b1b --- /dev/null +++ b/crates/ruff_linter/src/rules/pydoclint/snapshots/ruff_linter__rules__pydoclint__tests__docstring-missing-exception_DOC501_numpy.py.snap @@ -0,0 +1,28 @@ +--- +source: crates/ruff_linter/src/rules/pydoclint/mod.rs +--- +DOC501_numpy.py:53:15: DOC501 Raised exception `FasterThanLightError` missing from docstring + | +51 | return distance / time +52 | except ZeroDivisionError as exc: +53 | raise FasterThanLightError from exc + | ^^^^^^^^^^^^^^^^^^^^ DOC501 + | + +DOC501_numpy.py:76:15: DOC501 Raised exception `FasterThanLightError` missing from docstring + | +74 | return distance / time +75 | except ZeroDivisionError as exc: +76 | raise FasterThanLightError from exc + | ^^^^^^^^^^^^^^^^^^^^ DOC501 +77 | except: +78 | raise ValueError + | + +DOC501_numpy.py:78:15: DOC501 Raised exception `ValueError` missing from docstring + | +76 | raise FasterThanLightError from exc +77 | except: +78 | raise ValueError + | ^^^^^^^^^^ DOC501 + | diff --git a/crates/ruff_linter/src/rules/pydocstyle/helpers.rs b/crates/ruff_linter/src/rules/pydocstyle/helpers.rs index 9ce0a757ac58b..3b78c003c9b8d 100644 --- a/crates/ruff_linter/src/rules/pydocstyle/helpers.rs +++ b/crates/ruff_linter/src/rules/pydocstyle/helpers.rs @@ -5,6 +5,11 @@ use ruff_python_ast::name::QualifiedName; use ruff_python_semantic::{Definition, SemanticModel}; use ruff_source_file::UniversalNewlines; +use crate::docstrings::sections::{SectionContexts, SectionKind}; +use crate::docstrings::styles::SectionStyle; +use crate::docstrings::Docstring; +use crate::rules::pydocstyle::settings::Convention; + /// Return the index of the first logical line in a string. pub(super) fn logical_line(content: &str) -> Option { // Find the first logical line. @@ -61,3 +66,59 @@ pub(crate) fn should_ignore_definition( }) }) } + +pub(crate) fn get_section_contexts<'a>( + docstring: &'a Docstring<'a>, + convention: Option<&'a Convention>, +) -> SectionContexts<'a> { + match convention { + Some(Convention::Google) => { + return SectionContexts::from_docstring(docstring, SectionStyle::Google); + } + Some(Convention::Numpy) => { + return SectionContexts::from_docstring(docstring, SectionStyle::Numpy); + } + Some(Convention::Pep257) | None => { + // There are some overlapping section names, between the Google and NumPy conventions + // (e.g., "Returns", "Raises"). Break ties by checking for the presence of some of the + // section names that are unique to each convention. + + // If the docstring contains `Parameters:` or `Other Parameters:`, use the NumPy + // convention. + let numpy_sections = SectionContexts::from_docstring(docstring, SectionStyle::Numpy); + if numpy_sections.iter().any(|context| { + matches!( + context.kind(), + SectionKind::Parameters + | SectionKind::OtherParams + | SectionKind::OtherParameters + ) + }) { + return numpy_sections; + } + + // If the docstring contains any argument specifier, use the Google convention. + let google_sections = SectionContexts::from_docstring(docstring, SectionStyle::Google); + if google_sections.iter().any(|context| { + matches!( + context.kind(), + SectionKind::Args + | SectionKind::Arguments + | SectionKind::KeywordArgs + | SectionKind::KeywordArguments + | SectionKind::OtherArgs + | SectionKind::OtherArguments + ) + }) { + return google_sections; + } + + // Otherwise, use whichever convention matched more sections. + if google_sections.len() > numpy_sections.len() { + google_sections + } else { + numpy_sections + } + } + } +} diff --git a/crates/ruff_linter/src/rules/pydocstyle/rules/sections.rs b/crates/ruff_linter/src/rules/pydocstyle/rules/sections.rs index bbf6c2227a9f5..7385226e0904b 100644 --- a/crates/ruff_linter/src/rules/pydocstyle/rules/sections.rs +++ b/crates/ruff_linter/src/rules/pydocstyle/rules/sections.rs @@ -1324,67 +1324,16 @@ impl AlwaysFixableViolation for BlankLinesBetweenHeaderAndContent { pub(crate) fn sections( checker: &mut Checker, docstring: &Docstring, + section_contexts: &SectionContexts, convention: Option<&Convention>, ) { match convention { - Some(Convention::Google) => { - parse_google_sections( - checker, - docstring, - &SectionContexts::from_docstring(docstring, SectionStyle::Google), - ); - } - Some(Convention::Numpy) => { - parse_numpy_sections( - checker, - docstring, - &SectionContexts::from_docstring(docstring, SectionStyle::Numpy), - ); - } - Some(Convention::Pep257) | None => { - // There are some overlapping section names, between the Google and NumPy conventions - // (e.g., "Returns", "Raises"). Break ties by checking for the presence of some of the - // section names that are unique to each convention. - - // If the docstring contains `Parameters:` or `Other Parameters:`, use the NumPy - // convention. - let numpy_sections = SectionContexts::from_docstring(docstring, SectionStyle::Numpy); - if numpy_sections.iter().any(|context| { - matches!( - context.kind(), - SectionKind::Parameters - | SectionKind::OtherParams - | SectionKind::OtherParameters - ) - }) { - parse_numpy_sections(checker, docstring, &numpy_sections); - return; - } - - // If the docstring contains any argument specifier, use the Google convention. - let google_sections = SectionContexts::from_docstring(docstring, SectionStyle::Google); - if google_sections.iter().any(|context| { - matches!( - context.kind(), - SectionKind::Args - | SectionKind::Arguments - | SectionKind::KeywordArgs - | SectionKind::KeywordArguments - | SectionKind::OtherArgs - | SectionKind::OtherArguments - ) - }) { - parse_google_sections(checker, docstring, &google_sections); - return; - } - - // Otherwise, use whichever convention matched more sections. - if google_sections.len() > numpy_sections.len() { - parse_google_sections(checker, docstring, &google_sections); - } else { - parse_numpy_sections(checker, docstring, &numpy_sections); - } - } + Some(Convention::Google) => parse_google_sections(checker, docstring, section_contexts), + Some(Convention::Numpy) => parse_numpy_sections(checker, docstring, section_contexts), + Some(Convention::Pep257) | None => match section_contexts.style() { + SectionStyle::Google => parse_google_sections(checker, docstring, section_contexts), + SectionStyle::Numpy => parse_numpy_sections(checker, docstring, section_contexts), + }, } } diff --git a/ruff.schema.json b/ruff.schema.json index 02fd6a5e4d5de..29f54d5c2b36d 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -2874,6 +2874,11 @@ "DJ01", "DJ012", "DJ013", + "DOC", + "DOC5", + "DOC50", + "DOC501", + "DOC502", "DTZ", "DTZ0", "DTZ00", diff --git a/scripts/add_plugin.py b/scripts/add_plugin.py index 0de67c188dfd7..d50bcb9f461cb 100755 --- a/scripts/add_plugin.py +++ b/scripts/add_plugin.py @@ -48,7 +48,7 @@ def main(*, plugin: str, url: str, prefix_code: str) -> None: let snapshot = format!("{}_{}", rule_code.as_ref(), path.to_string_lossy()); let diagnostics = test_path( Path::new("%s").join(path).as_path(), - &settings::Settings::for_rule(rule_code), + &settings::LinterSettings::for_rule(rule_code), )?; assert_messages!(snapshot, diagnostics); Ok(())