edge case involving implicit returns

astral-sh · Sep 1, 2024 · fe982fe · fe982fe
1 parent 0a7a84d
commit fe982fe
Show file tree

Hide file tree

Showing 3 changed files with 57 additions and 5 deletions.
diff --git a/crates/ruff_linter/resources/test/fixtures/pydoclint/DOC202_google.py b/crates/ruff_linter/resources/test/fixtures/pydoclint/DOC202_google.py
@@ -73,3 +73,15 @@ def f(self):
             dict: The values
         """
         return
+
+
+# DOC202 -- never explicitly returns anything, just short-circuits
+def foo(s: str, condition: bool):
+    """Fooey things.
+
+    Returns:
+        None
+    """
+    if not condition:
+        return
+    print(s)
diff --git a/crates/ruff_linter/src/rules/pydoclint/rules/check_docstring.rs b/crates/ruff_linter/src/rules/pydoclint/rules/check_docstring.rs
@@ -510,11 +510,32 @@ impl Ranged for YieldEntry {
     }
 }
 
+#[allow(clippy::enum_variant_names)]
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+enum ReturnEntryKind {
+    NoneNone,
+    ImplicitNone,
+    ExplicitNone,
+}
+
 /// An individual `return` statement in a function body.
 #[derive(Debug)]
 struct ReturnEntry {
     range: TextRange,
-    is_none_return: bool,
+    kind: ReturnEntryKind,
+}
+
+impl ReturnEntry {
+    const fn is_none_return(&self) -> bool {
+        matches!(
+            &self.kind,
+            ReturnEntryKind::ExplicitNone | ReturnEntryKind::ImplicitNone
+        )
+    }
+
+    const fn is_implicit(&self) -> bool {
+        matches!(&self.kind, ReturnEntryKind::ImplicitNone)
+    }
 }
 
 impl Ranged for ReturnEntry {
@@ -644,13 +665,17 @@ impl<'a> Visitor<'a> for BodyVisitor<'a> {
             }) => {
                 self.returns.push(ReturnEntry {
                     range: *range,
-                    is_none_return: value.is_none_literal_expr(),
+                    kind: if value.is_none_literal_expr() {
+                        ReturnEntryKind::ExplicitNone
+                    } else {
+                        ReturnEntryKind::NoneNone
+                    },
                 });
             }
             Stmt::Return(ast::StmtReturn { range, value: None }) => {
                 self.returns.push(ReturnEntry {
                     range: *range,
-                    is_none_return: true,
+                    kind: ReturnEntryKind::ImplicitNone,
                 });
             }
             Stmt::FunctionDef(_) | Stmt::ClassDef(_) => return,
@@ -866,7 +891,7 @@ pub(crate) fn check_docstring(
                         None if body_entries
                             .returns
                             .iter()
-                            .any(|entry| !entry.is_none_return) =>
+                            .any(|entry| !entry.is_none_return()) =>
                         {
                             diagnostics.push(Diagnostic::new(
                                 DocstringMissingReturns,
@@ -939,7 +964,9 @@ 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() {
+                if body_entries.returns.is_empty()
+                    || body_entries.returns.iter().all(ReturnEntry::is_implicit)
+                {
                     let diagnostic =
                         Diagnostic::new(DocstringExtraneousReturns, docstring_returns.range());
                     diagnostics.push(diagnostic);

diff --git a/.../ruff_linter__rules__pydoclint__tests__docstring-extraneous-returns_DOC202_google.py.snap b/.../ruff_linter__rules__pydoclint__tests__docstring-extraneous-returns_DOC202_google.py.snap
@@ -24,3 +24,16 @@ DOC202_google.py:36:1: DOC202 Docstring should not have a returns section becaus
 39 |           print('test')
    |
    = help: Remove the "Returns" section
+
+DOC202_google.py:82:1: DOC202 Docstring should not have a returns section because the function doesn't return anything
+   |
+80 |       """Fooey things.
+81 |   
+82 | /     Returns:
+83 | |         None
+84 | |     """
+   | |____^ DOC202
+85 |       if not condition:
+86 |           return
+   |
+   = help: Remove the "Returns" section