Skip to content

Commit

Permalink
Don't require a returns section for NoReturn (#57)
Browse files Browse the repository at this point in the history
  • Loading branch information
llucax authored Aug 15, 2023
1 parent 397d6b2 commit 170da52
Show file tree
Hide file tree
Showing 16 changed files with 212 additions and 35 deletions.
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,12 @@
# Change Log

## [0.1.6] - 2023-08-13

- Added
- Added handling of the `NoReturn` type annotation
- Full diff
- https://github.com/jsh9/pydoclint/compare/0.1.5...0.1.6

## [0.1.5] - 2023-08-12

- Improved
Expand Down
7 changes: 4 additions & 3 deletions docs/config_options.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ page:
- [6. `--skip-checking-short-docstrings` (shortform: `-scsd`, default: `True`)](#6---skip-checking-short-docstrings-shortform--scsd-default-true)
- [7. `--skip-checking-raises` (shortform: `-scr`, default: `False`)](#7---skip-checking-raises-shortform--scr-default-false)
- [8. `--allow-init-docstring` (shortform: `-aid`, default: `False`)](#8---allow-init-docstring-shortform--aid-default-false)
- [9. `--require-return-section-when-returning-none` (shortform: `-rrs`, default: `False`)](#9---require-return-section-when-returning-none-shortform--rrs-default-false)
- [9. `--require-return-section-when-returning-nothing` (shortform: `-rrs`, default: `False`)](#9---require-return-section-when-returning-nothing-shortform--rrs-default-false)
- [10. `--check-return-types` (shortform: `-crt`, default: `True`)](#10---check-return-types-shortform--crt-default-true)

<!--TOC-->
Expand Down Expand Up @@ -137,11 +137,12 @@ If it is set to `True`, having a docstring for class constructors
(`__init__()`) is allowed, and the arguments are expected to be documented
under `__init__()` rather than in the class docstring.

## 9. `--require-return-section-when-returning-none` (shortform: `-rrs`, default: `False`)
## 9. `--require-return-section-when-returning-nothing` (shortform: `-rrs`, default: `False`)

If `False`, a "return" section is not necessary in the docstring if the
function implicitly returns `None` (for example, doesn't have a return
statement, or has `-> None` as the return annotation).
statement, or has `-> None` as the return annotation) or doesn't return at all
(has return type `NoReturn`).

## 10. `--check-return-types` (shortform: `-crt`, default: `True`)

Expand Down
2 changes: 1 addition & 1 deletion docs/how_to_config.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ For detailed explanations of all options, please read this page:
[tool.pydoclint]
style = 'google'
exclude = '\.git|\.tox|tests/data|some_script\.py'
require-return-section-when-returning-none = true
require-return-section-when-returning-nothing = true
```

- Then, specify the path of the `.toml` file in your command:
Expand Down
35 changes: 29 additions & 6 deletions pydoclint/flake8_entry.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,15 +91,25 @@ def add_options(cls, parser): # noqa: D102
help='If True, allow both __init__() and the class def to have docstrings',
)
parser.add_option(
'-rrs',
'--require-return-section-when-returning-none',
action='store',
default='None',
parse_from_config=True,
help=(
'(Deprecated) Please use'
' --require-return-section-when-returning-nothing instead.'
),
)
parser.add_option(
'-rrs',
'--require-return-section-when-returning-nothing',
action='store',
default='False',
parse_from_config=True,
help=(
'If False, a return section is not needed in docstring if'
' the function body does not have a "return" statement and'
' the return type annotation is "-> None".'
' the return type annotation is "-> None" or "-> NoReturn".'
),
)
parser.add_option(
Expand Down Expand Up @@ -129,6 +139,9 @@ def parse_options(cls, options): # noqa: D102
cls.require_return_section_when_returning_none = (
options.require_return_section_when_returning_none
)
cls.require_return_section_when_returning_nothing = (
options.require_return_section_when_returning_nothing
)
cls.check_return_types = options.check_return_types
cls.style = options.style

Expand All @@ -146,6 +159,14 @@ def run(self) -> Generator[Tuple[int, int, str, Any], None, None]:
' please use `--arg-type-hints-in-signature` instead'
)

# user supplies this option
if self.require_return_section_when_returning_none != 'None':
raise ValueError(
'The option `--require-return-section-when-returning-none`'
' has been renamed; please use'
'`--require-return-section-when-returning-nothing` instead'
)

argTypeHintsInSignature = self._bool(
'--arg-type-hints-in-signature',
self.arg_type_hints_in_signature,
Expand All @@ -167,9 +188,9 @@ def run(self) -> Generator[Tuple[int, int, str, Any], None, None]:
'--allow-init-docstring',
self.allow_init_docstring,
)
requireReturnSectionWhenReturningNone = self._bool(
'--require-return-section-when-returning-none',
self.require_return_section_when_returning_none,
requireReturnSectionWhenReturningNothing = self._bool(
'--require-return-section-when-returning-nothing',
self.require_return_section_when_returning_nothing,
)
checkReturnTypes = self._bool(
'--check-return-types',
Expand All @@ -188,7 +209,9 @@ def run(self) -> Generator[Tuple[int, int, str, Any], None, None]:
skipCheckingShortDocstrings=skipCheckingShortDocstrings,
skipCheckingRaises=skipCheckingRaises,
allowInitDocstring=allowInitDocstring,
requireReturnSectionWhenReturningNone=requireReturnSectionWhenReturningNone,
requireReturnSectionWhenReturningNothing=(
requireReturnSectionWhenReturningNothing
),
checkReturnTypes=checkReturnTypes,
style=self.style,
)
Expand Down
46 changes: 39 additions & 7 deletions pydoclint/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -132,15 +132,24 @@ def validateStyleValue(
help='If True, allow both __init__() and the class def to have docstrings',
)
@click.option(
'-rrs',
'--require-return-section-when-returning-none',
show_default=True,
default='None',
help=(
'(Deprecated) Please use'
' --require-return-section-when-returning-nothing instead.'
),
)
@click.option(
'-rrs',
'--require-return-section-when-returning-nothing',
type=bool,
show_default=True,
default=False,
help=(
'If False, a return section is not needed in docstring if'
' the function body does not have a "return" statement and'
' the return type annotation is "-> None".'
' the return type annotation is "-> None" or "-> NoReturn".'
),
)
@click.option(
Expand Down Expand Up @@ -203,6 +212,7 @@ def main( # noqa: C901
allow_init_docstring: bool,
check_return_types: bool,
require_return_section_when_returning_none: bool,
require_return_section_when_returning_nothing: bool,
config: Optional[str], # don't remove it b/c it's required by `click`
) -> None:
"""Command-line entry point of pydoclint"""
Expand Down Expand Up @@ -236,6 +246,22 @@ def main( # noqa: C901
)
ctx.exit(1)

# it means users supply this option
if require_return_section_when_returning_none != 'None':
click.echo(
click.style(
''.join([
'The option `--require-return-section-when-returning-none`',
' has been renamed; please use',
'`--require-return-section-when-returning-nothing` instead',
]),
fg='red',
bold=True,
),
err=echoAsError,
)
ctx.exit(1)

if paths and src is not None:
click.echo(
main.get_usage(ctx)
Expand Down Expand Up @@ -263,7 +289,9 @@ def main( # noqa: C901
skipCheckingRaises=skip_checking_raises,
allowInitDocstring=allow_init_docstring,
checkReturnTypes=check_return_types,
requireReturnSectionWhenReturningNone=require_return_section_when_returning_none,
requireReturnSectionWhenReturningNothing=(
require_return_section_when_returning_nothing
),
)

violationCounter: int = 0
Expand Down Expand Up @@ -319,7 +347,7 @@ def _checkPaths(
skipCheckingRaises: bool = False,
allowInitDocstring: bool = False,
checkReturnTypes: bool = True,
requireReturnSectionWhenReturningNone: bool = False,
requireReturnSectionWhenReturningNothing: bool = False,
quiet: bool = False,
exclude: str = '',
) -> Dict[str, List[Violation]]:
Expand Down Expand Up @@ -361,7 +389,9 @@ def _checkPaths(
skipCheckingRaises=skipCheckingRaises,
allowInitDocstring=allowInitDocstring,
checkReturnTypes=checkReturnTypes,
requireReturnSectionWhenReturningNone=requireReturnSectionWhenReturningNone,
requireReturnSectionWhenReturningNothing=(
requireReturnSectionWhenReturningNothing
),
)
allViolations[filename.as_posix()] = violationsInThisFile

Expand All @@ -378,7 +408,7 @@ def _checkFile(
skipCheckingRaises: bool = False,
allowInitDocstring: bool = False,
checkReturnTypes: bool = True,
requireReturnSectionWhenReturningNone: bool = False,
requireReturnSectionWhenReturningNothing: bool = False,
) -> List[Violation]:
with open(filename, encoding='utf8') as fp:
src: str = ''.join(fp.readlines())
Expand All @@ -393,7 +423,9 @@ def _checkFile(
skipCheckingRaises=skipCheckingRaises,
allowInitDocstring=allowInitDocstring,
checkReturnTypes=checkReturnTypes,
requireReturnSectionWhenReturningNone=requireReturnSectionWhenReturningNone,
requireReturnSectionWhenReturningNothing=(
requireReturnSectionWhenReturningNothing
),
)
visitor.visit(tree)
return visitor.violations
Expand Down
9 changes: 9 additions & 0 deletions pydoclint/utils/return_yield_raise.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,15 @@ def _isNone(node: ast.AST) -> bool:
return isinstance(node, ast.Constant) and node.value is None


def isReturnAnnotationNoReturn(node: FuncOrAsyncFuncDef) -> bool:
"""Check whether the return type annotation if `NoReturn`"""
if node.returns is None:
return False

returnAnnotation: str = unparseAnnotation(node.returns)
return returnAnnotation == 'NoReturn'


def hasGeneratorAsReturnAnnotation(node: FuncOrAsyncFuncDef) -> bool:
"""Check whether the function node has a 'Generator' return annotation"""
if node.returns is None:
Expand Down
22 changes: 15 additions & 7 deletions pydoclint/visitor.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
hasReturnStatements,
hasYieldStatements,
isReturnAnnotationNone,
isReturnAnnotationNoReturn,
)
from pydoclint.utils.violation import Violation

Expand All @@ -43,7 +44,7 @@ def __init__(
skipCheckingRaises: bool = False,
allowInitDocstring: bool = False,
checkReturnTypes: bool = True,
requireReturnSectionWhenReturningNone: bool = False,
requireReturnSectionWhenReturningNothing: bool = False,
) -> None:
self.style: str = style
self.argTypeHintsInSignature: bool = argTypeHintsInSignature
Expand All @@ -53,8 +54,8 @@ def __init__(
self.skipCheckingRaises: bool = skipCheckingRaises
self.allowInitDocstring: bool = allowInitDocstring
self.checkReturnTypes: bool = checkReturnTypes
self.requireReturnSectionWhenReturningNone: bool = (
requireReturnSectionWhenReturningNone
self.requireReturnSectionWhenReturningNothing: bool = (
requireReturnSectionWhenReturningNothing
)

self.parent: Optional[ast.AST] = None # keep track of parent node
Expand Down Expand Up @@ -457,9 +458,15 @@ def checkReturns( # noqa: C901
# If "Generator[...]" is put in the return type annotation,
# we don't need a "Returns" section in the docstring. Instead,
# we need a "Yields" section.
if self.requireReturnSectionWhenReturningNone:
if self.requireReturnSectionWhenReturningNothing:
violations.append(v201)
elif not isReturnAnnotationNone(node):
elif (
# fmt: off
not isReturnAnnotationNone(node)
and not isReturnAnnotationNoReturn(node)

# fmt: on
):
violations.append(v201)

if docstringHasReturnSection and not (hasReturnStmt or hasReturnAnno):
Expand All @@ -478,8 +485,9 @@ def checkReturns( # noqa: C901

if (
returnSec == [] # no return section in docstring
and returnAnno.annotation == 'None' # `-> None` in signature
and not self.requireReturnSectionWhenReturningNone
# `-> None` or `-> NoReturn` in signature
and returnAnno.annotation in {'None', 'NoReturn'}
and not self.requireReturnSectionWhenReturningNothing
):
return violations # no need to check return type hints at all

Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,4 @@ wrap-line-with-long-string = true
[tool.pydoclint]
style = 'numpy'
exclude = '\.git|\.tox|tests/data|unparser\.py'
require-return-section-when-returning-none = true
require-return-section-when-returning-nothing = true
2 changes: 1 addition & 1 deletion setup.cfg
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[metadata]
name = pydoclint
version = 0.1.5
version = 0.1.6
description = A Python docstring linter that checks arguments, returns, yields, and raises sections
long_description = file: README.md
long_description_content_type = text/markdown
Expand Down
14 changes: 12 additions & 2 deletions tests/data/google/no_return_section/cases.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
from typing import Optional
from typing import NoReturn, Optional


def func1(text: str) -> None:
"""A return section can be omitted if requireReturnSectionWhenReturningNone
"""A return section can be omitted if requireReturnSectionWhenReturningNothing
is set to False.
Args:
Expand Down Expand Up @@ -57,3 +57,13 @@ def func6(text: str):
text (str): Text for the function
"""
print(123)


def func7(text: str) -> NoReturn:
"""A return section is never necessary because it doesn't return
anything or even return ever, as there is a NoReturn annotation.
Args:
text (str): Text for the function
"""
exit(1)
11 changes: 11 additions & 0 deletions tests/data/google/returning_noreturn/cases.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
from typing import NoReturn


def func(arg1: int) -> NoReturn:
"""
Do something
Args:
arg1 (int): Arg 1
"""
exit(1)
16 changes: 14 additions & 2 deletions tests/data/numpy/no_return_section/cases.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
from typing import Optional
from typing import NoReturn, Optional


def func1(text: str) -> None:
"""A return section can be omitted if requireReturnSectionWhenReturningNone
"""A return section can be omitted if requireReturnSectionWhenReturningNothing
is set to False.
Parameters
Expand Down Expand Up @@ -69,3 +69,15 @@ def func6(text: str):
Text for the function
"""
print(123)


def func7(text: str) -> NoReturn:
"""A return section is never necessary because it doesn't return
anything or even return ever, as there is a NoReturn annotation.
Parameters
----------
text : str
Text for the function
"""
exit(1)
13 changes: 13 additions & 0 deletions tests/data/numpy/returning_noreturn/cases.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
from typing import NoReturn


def func(arg1: int) -> NoReturn:
"""
Do something
Parameters
----------
arg1 : int
Arg 1
"""
exit(1)
Loading

0 comments on commit 170da52

Please sign in to comment.