From 1f51048fa4a5954526e68a45cd4781040b502254 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Fri, 9 Aug 2024 09:34:14 -0400 Subject: [PATCH] Don't enforce returns and yields in abstract methods (#12771) ## Summary Closes https://github.com/astral-sh/ruff/issues/12685. --- .../test/fixtures/pydoclint/DOC201_google.py | 11 ++ .../test/fixtures/pydoclint/DOC201_numpy.py | 11 ++ .../test/fixtures/pydoclint/DOC202_google.py | 14 +++ .../test/fixtures/pydoclint/DOC202_numpy.py | 16 +++ .../rules/pydoclint/rules/check_docstring.rs | 112 ++++++++++-------- ...ring-missing-returns_DOC201_google.py.snap | 9 ++ ...tring-missing-returns_DOC201_numpy.py.snap | 9 ++ 7 files changed, 134 insertions(+), 48 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/pydoclint/DOC201_google.py b/crates/ruff_linter/resources/test/fixtures/pydoclint/DOC201_google.py index b7c5da754aee1..5cd3f192adf12 100644 --- a/crates/ruff_linter/resources/test/fixtures/pydoclint/DOC201_google.py +++ b/crates/ruff_linter/resources/test/fixtures/pydoclint/DOC201_google.py @@ -108,3 +108,14 @@ def f(num: int): num (int): A number """ return 1 + + +import abc + + +class A(metaclass=abc.abcmeta): + # DOC201 + @abc.abstractmethod + def f(self): + """Lorem ipsum.""" + return True diff --git a/crates/ruff_linter/resources/test/fixtures/pydoclint/DOC201_numpy.py b/crates/ruff_linter/resources/test/fixtures/pydoclint/DOC201_numpy.py index 661b0bed1965e..362836f11d834 100644 --- a/crates/ruff_linter/resources/test/fixtures/pydoclint/DOC201_numpy.py +++ b/crates/ruff_linter/resources/test/fixtures/pydoclint/DOC201_numpy.py @@ -74,3 +74,14 @@ def baz(self) -> str: A number """ return 'test' + + +import abc + + +class A(metaclass=abc.abcmeta): + # DOC201 + @abc.abstractmethod + def f(self): + """Lorem ipsum.""" + return True diff --git a/crates/ruff_linter/resources/test/fixtures/pydoclint/DOC202_google.py b/crates/ruff_linter/resources/test/fixtures/pydoclint/DOC202_google.py index 671a031937a06..97f73a62034a8 100644 --- a/crates/ruff_linter/resources/test/fixtures/pydoclint/DOC202_google.py +++ b/crates/ruff_linter/resources/test/fixtures/pydoclint/DOC202_google.py @@ -59,3 +59,17 @@ def foo(self) -> int: x """ raise NotImplementedError + + +import abc + + +class A(metaclass=abc.abcmeta): + @abc.abstractmethod + def f(self): + """Lorem ipsum + + Returns: + dict: The values + """ + return diff --git a/crates/ruff_linter/resources/test/fixtures/pydoclint/DOC202_numpy.py b/crates/ruff_linter/resources/test/fixtures/pydoclint/DOC202_numpy.py index e05f86afe4ac9..23a0ed1864bc2 100644 --- a/crates/ruff_linter/resources/test/fixtures/pydoclint/DOC202_numpy.py +++ b/crates/ruff_linter/resources/test/fixtures/pydoclint/DOC202_numpy.py @@ -60,3 +60,19 @@ def bar(self) -> str: A number """ print('test') + + +import abc + + +class A(metaclass=abc.abcmeta): + @abc.abstractmethod + def f(self): + """Lorem ipsum + + Returns + ------- + dict: + The values + """ + return 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 ccd85d16d21a4..d8ed49d455e03 100644 --- a/crates/ruff_linter/src/rules/pydoclint/rules/check_docstring.rs +++ b/crates/ruff_linter/src/rules/pydoclint/rules/check_docstring.rs @@ -6,7 +6,7 @@ use ruff_python_ast::helpers::map_callable; use ruff_python_ast::name::QualifiedName; use ruff_python_ast::visitor::Visitor; use ruff_python_ast::{self as ast, visitor, Expr, Stmt}; -use ruff_python_semantic::analyze::function_type; +use ruff_python_semantic::analyze::{function_type, visibility}; use ruff_python_semantic::{Definition, SemanticModel}; use ruff_text_size::{Ranged, TextRange}; @@ -25,6 +25,8 @@ use crate::rules::pydocstyle::settings::Convention; /// Docstrings missing return sections are a sign of incomplete documentation /// or refactors. /// +/// This rule is not enforced for abstract methods and stubs functions. +/// /// ## Example /// ```python /// def calculate_speed(distance: float, time: float) -> float: @@ -73,6 +75,8 @@ impl Violation for DocstringMissingReturns { /// Functions without an explicit return should not have a returns section /// in their docstrings. /// +/// This rule is not enforced for stub functions. +/// /// ## Example /// ```python /// def say_hello(n: int) -> None: @@ -121,6 +125,8 @@ impl Violation for DocstringExtraneousReturns { /// Docstrings missing yields sections are a sign of incomplete documentation /// or refactors. /// +/// This rule is not enforced for abstract methods and stubs functions. +/// /// ## Example /// ```python /// def count_to_n(n: int) -> int: @@ -169,6 +175,8 @@ impl Violation for DocstringMissingYields { /// Functions which don't yield anything should not have a yields section /// in their docstrings. /// +/// This rule is not enforced for stub functions. +/// /// ## Example /// ```python /// def say_hello(n: int) -> None: @@ -218,6 +226,8 @@ impl Violation for DocstringExtraneousYields { /// it can be misleading to users and/or a sign of incomplete documentation or /// refactors. /// +/// This rule is not enforced for abstract methods and stubs functions. +/// /// ## Example /// ```python /// def calculate_speed(distance: float, time: float) -> float: @@ -282,6 +292,8 @@ impl Violation for DocstringMissingException { /// Some conventions prefer non-explicit exceptions be omitted from the /// docstring. /// +/// This rule is not enforced for stub functions. +/// /// ## Example /// ```python /// def calculate_speed(distance: float, time: float) -> float: @@ -343,7 +355,7 @@ impl Violation for DocstringExtraneousException { } } -// A generic docstring section. +/// A generic docstring section. #[derive(Debug)] struct GenericSection { range: TextRange, @@ -363,7 +375,7 @@ impl GenericSection { } } -// A Raises docstring section. +/// A "Raises" section in a docstring. #[derive(Debug)] struct RaisesSection<'a> { raised_exceptions: Vec>, @@ -378,7 +390,7 @@ impl Ranged for RaisesSection<'_> { impl<'a> RaisesSection<'a> { /// Return the raised exceptions for the docstring, or `None` if the docstring does not contain - /// a `Raises` section. + /// a "Raises" section. fn from_section(section: &SectionContext<'a>, style: Option) -> Self { Self { raised_exceptions: parse_entries(section.following_lines_str(), style), @@ -415,7 +427,7 @@ impl<'a> DocstringSections<'a> { } } -/// Parse the entries in a `Raises` section of a docstring. +/// Parse the entries in a "Raises" section of a docstring. /// /// Attempts to parse using the specified [`SectionStyle`], falling back to the other style if no /// entries are found. @@ -519,7 +531,7 @@ struct BodyEntries<'a> { struct BodyVisitor<'a> { returns: Vec, yields: Vec, - currently_suspended_exceptions: Option<&'a ast::Expr>, + currently_suspended_exceptions: Option<&'a Expr>, raised_exceptions: Vec>, semantic: &'a SemanticModel<'a>, } @@ -732,17 +744,6 @@ pub(crate) fn check_docstring( } } - // DOC202 - if checker.enabled(Rule::DocstringExtraneousReturns) { - if let Some(ref docstring_returns) = docstring_sections.returns { - if body_entries.returns.is_empty() { - let diagnostic = - Diagnostic::new(DocstringExtraneousReturns, docstring_returns.range()); - diagnostics.push(diagnostic); - } - } - } - // DOC402 if checker.enabled(Rule::DocstringMissingYields) { if !yields_documented(docstring, &docstring_sections, convention) { @@ -753,17 +754,6 @@ pub(crate) fn check_docstring( } } - // DOC403 - if checker.enabled(Rule::DocstringExtraneousYields) { - if let Some(docstring_yields) = docstring_sections.yields { - if body_entries.yields.is_empty() { - let diagnostic = - Diagnostic::new(DocstringExtraneousYields, docstring_yields.range()); - diagnostics.push(diagnostic); - } - } - } - // DOC501 if checker.enabled(Rule::DocstringMissingException) { for body_raise in &body_entries.raised_exceptions { @@ -794,28 +784,54 @@ pub(crate) fn check_docstring( } } - // DOC502 - if checker.enabled(Rule::DocstringExtraneousException) { - if let Some(docstring_raises) = docstring_sections.raises { - let mut extraneous_exceptions = Vec::new(); - for docstring_raise in &docstring_raises.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()); + // Avoid applying "extraneous" rules to abstract methods. An abstract method's docstring _could_ + // document that it raises an exception without including the exception in the implementation. + if !visibility::is_abstract(&function_def.decorator_list, checker.semantic()) { + // DOC202 + if checker.enabled(Rule::DocstringExtraneousReturns) { + if let Some(ref docstring_returns) = docstring_sections.returns { + if body_entries.returns.is_empty() { + let diagnostic = + Diagnostic::new(DocstringExtraneousReturns, docstring_returns.range()); + diagnostics.push(diagnostic); } } - if !extraneous_exceptions.is_empty() { - let diagnostic = Diagnostic::new( - DocstringExtraneousException { - ids: extraneous_exceptions, - }, - docstring_raises.range(), - ); - diagnostics.push(diagnostic); + } + + // DOC403 + if checker.enabled(Rule::DocstringExtraneousYields) { + if let Some(docstring_yields) = docstring_sections.yields { + if body_entries.yields.is_empty() { + let diagnostic = + Diagnostic::new(DocstringExtraneousYields, docstring_yields.range()); + diagnostics.push(diagnostic); + } + } + } + + // DOC502 + if checker.enabled(Rule::DocstringExtraneousException) { + if let Some(docstring_raises) = docstring_sections.raises { + let mut extraneous_exceptions = Vec::new(); + for docstring_raise in &docstring_raises.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_raises.range(), + ); + diagnostics.push(diagnostic); + } } } } diff --git a/crates/ruff_linter/src/rules/pydoclint/snapshots/ruff_linter__rules__pydoclint__tests__docstring-missing-returns_DOC201_google.py.snap b/crates/ruff_linter/src/rules/pydoclint/snapshots/ruff_linter__rules__pydoclint__tests__docstring-missing-returns_DOC201_google.py.snap index 779d0c4d452eb..16534806d0524 100644 --- a/crates/ruff_linter/src/rules/pydoclint/snapshots/ruff_linter__rules__pydoclint__tests__docstring-missing-returns_DOC201_google.py.snap +++ b/crates/ruff_linter/src/rules/pydoclint/snapshots/ruff_linter__rules__pydoclint__tests__docstring-missing-returns_DOC201_google.py.snap @@ -29,3 +29,12 @@ DOC201_google.py:71:9: DOC201 `return` is not documented in docstring 73 | print("I never return") | = help: Add a "Returns" section to the docstring + +DOC201_google.py:121:9: DOC201 `return` is not documented in docstring + | +119 | def f(self): +120 | """Lorem ipsum.""" +121 | return True + | ^^^^^^^^^^^ DOC201 + | + = 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-returns_DOC201_numpy.py.snap b/crates/ruff_linter/src/rules/pydoclint/snapshots/ruff_linter__rules__pydoclint__tests__docstring-missing-returns_DOC201_numpy.py.snap index 363f87d07c4cd..04d87deb5aa0d 100644 --- a/crates/ruff_linter/src/rules/pydoclint/snapshots/ruff_linter__rules__pydoclint__tests__docstring-missing-returns_DOC201_numpy.py.snap +++ b/crates/ruff_linter/src/rules/pydoclint/snapshots/ruff_linter__rules__pydoclint__tests__docstring-missing-returns_DOC201_numpy.py.snap @@ -18,3 +18,12 @@ DOC201_numpy.py:62:9: DOC201 `return` is not documented in docstring | ^^^^^^^^^^^^^ DOC201 | = help: Add a "Returns" section to the docstring + +DOC201_numpy.py:87:9: DOC201 `return` is not documented in docstring + | +85 | def f(self): +86 | """Lorem ipsum.""" +87 | return True + | ^^^^^^^^^^^ DOC201 + | + = help: Add a "Returns" section to the docstring