Skip to content

Commit

Permalink
Fix false positive DOC405 & DOC201 for bare returns (#205)
Browse files Browse the repository at this point in the history
  • Loading branch information
jsh9 authored Jan 12, 2025
1 parent 1f2ea57 commit 22b9b37
Show file tree
Hide file tree
Showing 6 changed files with 175 additions and 1 deletion.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# Change Log

## [Unpublished]

- Fixed
- False positive DOC405 and DOC201 when we have bare return statements
together with `yield` statements

## [0.5.18] - 2025-01-12

- Fixed
Expand Down
12 changes: 12 additions & 0 deletions pydoclint/utils/return_yield_raise.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,18 @@ def isThisNodeAReturnStmt(node_: ast.AST) -> bool:
return _hasExpectedStatements(node, isThisNodeAReturnStmt)


def hasBareReturnStatements(node: FuncOrAsyncFuncDef) -> bool:
"""
Check whether the function node has bare return statements (i.e.,
just a "return" without anything behind it)
"""

def isThisNodeABareReturnStmt(node_: ast.AST) -> bool:
return isinstance(node_, ast.Return) and node_.value is None

return _hasExpectedStatements(node, isThisNodeABareReturnStmt)


def hasRaiseStatements(node: FuncOrAsyncFuncDef) -> bool:
"""Check whether the function node has any raise statements"""

Expand Down
15 changes: 14 additions & 1 deletion pydoclint/visitor.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
from pydoclint.utils.return_arg import ReturnArg
from pydoclint.utils.return_yield_raise import (
getRaisedExceptions,
hasBareReturnStatements,
hasGeneratorAsReturnAnnotation,
hasIteratorOrIterableAsReturnAnnotation,
hasRaiseStatements,
Expand Down Expand Up @@ -736,6 +737,11 @@ def my_function(num: int) -> Generator[int, None, str]:
onlyHasYieldStmt: bool = hasYieldStmt and not hasReturnStmt
hasReturnAnno: bool = hasReturnAnnotation(node)

if hasReturnStmt:
hasBareReturnStmt: bool = hasBareReturnStatements(node)
else:
hasBareReturnStmt = False # to save some time

returnAnno = ReturnAnnotation(unparseName(node.returns))
returnSec: list[ReturnArg] = doc.returnSection

Expand All @@ -752,6 +758,11 @@ def my_function(num: int) -> Generator[int, None, str]:
hasReturnAnno and not hasGenAsRetAnno
))

# If the return statement in the function body is a bare
# return, we don't throw DOC201 or DOC405. See more at:
# https://github.com/jsh9/pydoclint/issues/126#issuecomment-2136497913
and not hasBareReturnStmt

# fmt: on
):
retTypeInGenerator = extractReturnTypeFromGenerator(
Expand Down Expand Up @@ -813,7 +824,9 @@ def my_function(num: int) -> Generator[int, None, str]:
else:
violations.append(v405)
else:
if not hasGenAsRetAnno or not hasIterAsRetAnno:
if (
not hasGenAsRetAnno or not hasIterAsRetAnno
) and not hasBareReturnStmt:
violations.append(v405)

return violations
Expand Down
34 changes: 34 additions & 0 deletions tests/data/edge_cases/23_bare_return_stmt_with_yield/google.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
# From: https://github.com/jsh9/pydoclint/issues/126

from contextlib import contextmanager


@contextmanager
def my_func_1(db: Optional[int]) -> Iterator[int]:
"""Test a function.
Args:
db: the database
Yields:
Some stuff.
"""
if db is not None:
yield db
return

db = ...
yield db


def my_func_2(arg1: int) -> None:
"""
Test a function.
Args:
arg1: some argument
Returns:
The return value
"""
pass
57 changes: 57 additions & 0 deletions tests/test_main.py
Original file line number Diff line number Diff line change
Expand Up @@ -1600,6 +1600,61 @@ def testNonAscii() -> None:
{'style': 'numpy'},
[],
),
(
'23_bare_return_stmt_with_yield/google.py',
{
'style': 'google',
'argTypeHintsInDocstring': False,
'checkYieldTypes': False,
'checkReturnTypes': True,
},
[
'DOC203: Function `my_func_2` return type(s) in docstring not consistent with '
"the return annotation. Return annotation types: ['None']; docstring return "
"section types: ['']"
],
),
(
'23_bare_return_stmt_with_yield/google.py',
{
'style': 'google',
'argTypeHintsInDocstring': False,
'checkYieldTypes': False,
'checkReturnTypes': False,
},
[],
),
(
'23_bare_return_stmt_with_yield/google.py',
{
'style': 'google',
'argTypeHintsInDocstring': False,
'checkYieldTypes': True,
'checkReturnTypes': True,
},
[
'DOC404: Function `my_func_1` yield type(s) in docstring not consistent with '
'the return annotation. The yield type (the 0th arg in '
'Generator[...]/Iterator[...]): int; docstring "yields" section types:',
'DOC203: Function `my_func_2` return type(s) in docstring not consistent with '
"the return annotation. Return annotation types: ['None']; docstring return "
"section types: ['']",
],
),
(
'23_bare_return_stmt_with_yield/google.py',
{
'style': 'google',
'argTypeHintsInDocstring': False,
'checkYieldTypes': True,
'checkReturnTypes': False,
},
[
'DOC404: Function `my_func_1` yield type(s) in docstring not consistent with '
'the return annotation. The yield type (the 0th arg in '
'Generator[...]/Iterator[...]): int; docstring "yields" section types:',
],
),
],
)
def testEdgeCases(
Expand Down Expand Up @@ -1627,6 +1682,8 @@ def testPlayground() -> None:
filename=DATA_DIR / 'playground.py',
style='google',
skipCheckingRaises=True,
argTypeHintsInDocstring=False,
checkYieldTypes=False,
)
expected = []
assert list(map(str, violations)) == expected
52 changes: 52 additions & 0 deletions tests/utils/test_returns_yields_raise.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
from pydoclint.utils.generic import getFunctionId
from pydoclint.utils.return_yield_raise import (
getRaisedExceptions,
hasBareReturnStatements,
hasGeneratorAsReturnAnnotation,
hasRaiseStatements,
hasReturnAnnotation,
Expand Down Expand Up @@ -83,6 +84,33 @@ def classmethod1_child1():
"""


src8 = """
def func8():
return
"""


src9 = """
def func9():
# In tested function, so it doesn't
# count as having a return statement
def func9_child1():
return
"""


src10 = """
def func10():
# When mixed, we still consider it
# as having a bare return statement
if 1 > 2:
return 501
if 2 > 6:
return
"""


@pytest.mark.parametrize(
'src, expected',
[
Expand All @@ -92,6 +120,9 @@ def classmethod1_child1():
(src4, False),
(src5, True),
(src6, True),
(src8, True),
(src9, False),
(src10, True),
],
)
def testHasReturnStatements(src: str, expected: bool) -> None:
Expand All @@ -101,6 +132,27 @@ def testHasReturnStatements(src: str, expected: bool) -> None:
assert hasReturnStatements(tree.body[0]) == expected


@pytest.mark.parametrize(
'src, expected',
[
(src1, False),
(src2, False),
(src3, False),
(src4, False),
(src5, False),
(src6, False),
(src8, True),
(src9, False),
(src10, True),
],
)
def testHasBareReturnStatements(src: str, expected: bool) -> None:
tree = ast.parse(src)
assert len(tree.body) == 1 # sanity check
assert isinstance(tree.body[0], (ast.FunctionDef, ast.AsyncFunctionDef))
assert hasBareReturnStatements(tree.body[0]) == expected


def testHasReturnStatements_inClass() -> None:
tree = ast.parse(src7)
assert len(tree.body) == 1 # sanity check
Expand Down

0 comments on commit 22b9b37

Please sign in to comment.