From 170da520b91aa6d13e7306e4e6a3ab57012d9dd7 Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Tue, 15 Aug 2023 10:15:51 +0200 Subject: [PATCH] Don't require a returns section for `NoReturn` (#57) --- CHANGELOG.md | 7 +++ docs/config_options.md | 7 +-- docs/how_to_config.md | 2 +- pydoclint/flake8_entry.py | 35 +++++++++++--- pydoclint/main.py | 46 ++++++++++++++++--- pydoclint/utils/return_yield_raise.py | 9 ++++ pydoclint/visitor.py | 22 ++++++--- pyproject.toml | 2 +- setup.cfg | 2 +- tests/data/google/no_return_section/cases.py | 14 +++++- tests/data/google/returning_noreturn/cases.py | 11 +++++ tests/data/numpy/no_return_section/cases.py | 16 ++++++- tests/data/numpy/returning_noreturn/cases.py | 13 ++++++ tests/data/sphinx/no_return_section/cases.py | 14 +++++- tests/data/sphinx/returning_noreturn/cases.py | 11 +++++ tests/test_main.py | 36 +++++++++++++-- 16 files changed, 212 insertions(+), 35 deletions(-) create mode 100644 tests/data/google/returning_noreturn/cases.py create mode 100644 tests/data/numpy/returning_noreturn/cases.py create mode 100644 tests/data/sphinx/returning_noreturn/cases.py diff --git a/CHANGELOG.md b/CHANGELOG.md index f631a37..0a98f08 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/docs/config_options.md b/docs/config_options.md index 0143886..dc415fc 100644 --- a/docs/config_options.md +++ b/docs/config_options.md @@ -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) @@ -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`) diff --git a/docs/how_to_config.md b/docs/how_to_config.md index 4a1fa69..7d61dc9 100644 --- a/docs/how_to_config.md +++ b/docs/how_to_config.md @@ -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: diff --git a/pydoclint/flake8_entry.py b/pydoclint/flake8_entry.py index 5be3f4c..272da66 100644 --- a/pydoclint/flake8_entry.py +++ b/pydoclint/flake8_entry.py @@ -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( @@ -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 @@ -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, @@ -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', @@ -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, ) diff --git a/pydoclint/main.py b/pydoclint/main.py index da16e3c..955d3ae 100644 --- a/pydoclint/main.py +++ b/pydoclint/main.py @@ -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( @@ -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""" @@ -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) @@ -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 @@ -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]]: @@ -361,7 +389,9 @@ def _checkPaths( skipCheckingRaises=skipCheckingRaises, allowInitDocstring=allowInitDocstring, checkReturnTypes=checkReturnTypes, - requireReturnSectionWhenReturningNone=requireReturnSectionWhenReturningNone, + requireReturnSectionWhenReturningNothing=( + requireReturnSectionWhenReturningNothing + ), ) allViolations[filename.as_posix()] = violationsInThisFile @@ -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()) @@ -393,7 +423,9 @@ def _checkFile( skipCheckingRaises=skipCheckingRaises, allowInitDocstring=allowInitDocstring, checkReturnTypes=checkReturnTypes, - requireReturnSectionWhenReturningNone=requireReturnSectionWhenReturningNone, + requireReturnSectionWhenReturningNothing=( + requireReturnSectionWhenReturningNothing + ), ) visitor.visit(tree) return visitor.violations diff --git a/pydoclint/utils/return_yield_raise.py b/pydoclint/utils/return_yield_raise.py index e6913ce..8aa02ec 100644 --- a/pydoclint/utils/return_yield_raise.py +++ b/pydoclint/utils/return_yield_raise.py @@ -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: diff --git a/pydoclint/visitor.py b/pydoclint/visitor.py index db2f584..b9d40b9 100644 --- a/pydoclint/visitor.py +++ b/pydoclint/visitor.py @@ -26,6 +26,7 @@ hasReturnStatements, hasYieldStatements, isReturnAnnotationNone, + isReturnAnnotationNoReturn, ) from pydoclint.utils.violation import Violation @@ -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 @@ -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 @@ -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): @@ -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 diff --git a/pyproject.toml b/pyproject.toml index 477db8d..8118431 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -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 diff --git a/setup.cfg b/setup.cfg index c0eaaab..07c7d3d 100644 --- a/setup.cfg +++ b/setup.cfg @@ -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 diff --git a/tests/data/google/no_return_section/cases.py b/tests/data/google/no_return_section/cases.py index b15bdf5..5bc4e4c 100644 --- a/tests/data/google/no_return_section/cases.py +++ b/tests/data/google/no_return_section/cases.py @@ -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: @@ -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) diff --git a/tests/data/google/returning_noreturn/cases.py b/tests/data/google/returning_noreturn/cases.py new file mode 100644 index 0000000..00f6538 --- /dev/null +++ b/tests/data/google/returning_noreturn/cases.py @@ -0,0 +1,11 @@ +from typing import NoReturn + + +def func(arg1: int) -> NoReturn: + """ + Do something + + Args: + arg1 (int): Arg 1 + """ + exit(1) diff --git a/tests/data/numpy/no_return_section/cases.py b/tests/data/numpy/no_return_section/cases.py index 8d5e65a..4304ac7 100644 --- a/tests/data/numpy/no_return_section/cases.py +++ b/tests/data/numpy/no_return_section/cases.py @@ -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 @@ -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) diff --git a/tests/data/numpy/returning_noreturn/cases.py b/tests/data/numpy/returning_noreturn/cases.py new file mode 100644 index 0000000..33fb147 --- /dev/null +++ b/tests/data/numpy/returning_noreturn/cases.py @@ -0,0 +1,13 @@ +from typing import NoReturn + + +def func(arg1: int) -> NoReturn: + """ + Do something + + Parameters + ---------- + arg1 : int + Arg 1 + """ + exit(1) diff --git a/tests/data/sphinx/no_return_section/cases.py b/tests/data/sphinx/no_return_section/cases.py index a8898fc..1721b04 100644 --- a/tests/data/sphinx/no_return_section/cases.py +++ b/tests/data/sphinx/no_return_section/cases.py @@ -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. :param text: Text for the function @@ -57,3 +57,13 @@ def func6(text: str): :type text: str """ 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. + + :param text: Text for the function + :type text: str + """ + exit(1) diff --git a/tests/data/sphinx/returning_noreturn/cases.py b/tests/data/sphinx/returning_noreturn/cases.py new file mode 100644 index 0000000..c5d5b2e --- /dev/null +++ b/tests/data/sphinx/returning_noreturn/cases.py @@ -0,0 +1,11 @@ +from typing import NoReturn + + +def func(arg1: int) -> NoReturn: + """ + Do something + + :param arg1: Arg 1 + :type arg1: int + """ + exit(1) diff --git a/tests/test_main.py b/tests/test_main.py index a370065..93f6dd3 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -148,7 +148,7 @@ def testReturns(style: str, filename: str) -> None: violations = _checkFile( filename=DATA_DIR / f'{style}/returns/{filename}', skipCheckingShortDocstrings=True, - requireReturnSectionWhenReturningNone=True, + requireReturnSectionWhenReturningNothing=True, style=style, ) @@ -230,7 +230,36 @@ def testReturns_returningNone(style: str, require: bool) -> None: violations = _checkFile( filename=DATA_DIR / f'{style}/returning_none/cases.py', skipCheckingShortDocstrings=True, - requireReturnSectionWhenReturningNone=require, + requireReturnSectionWhenReturningNothing=require, + style=style, + ) + expectedViolationsCopy = ( + [ + 'DOC201: Function `func` does not have a return section in docstring ', + 'DOC203: Function `func` return type(s) in docstring not consistent with the ' + 'return annotation. Return annotation has 1 type(s); docstring return section ' + 'has 0 type(s).', + ] + if require + else [] + ) + assert list(map(str, violations)) == expectedViolationsCopy + + +@pytest.mark.parametrize( + 'style, require', + list( + itertools.product( + ['numpy', 'google', 'sphinx'], + [True, False], + ), + ), +) +def testReturns_returningNoReturn(style: str, require: bool) -> None: + violations = _checkFile( + filename=DATA_DIR / f'{style}/returning_noreturn/cases.py', + skipCheckingShortDocstrings=True, + requireReturnSectionWhenReturningNothing=require, style=style, ) expectedViolationsCopy = ( @@ -584,7 +613,7 @@ def testNoReturnSection( filename=DATA_DIR / f'{style}/no_return_section/cases.py', style=style, checkReturnTypes=False, - requireReturnSectionWhenReturningNone=rrs, + requireReturnSectionWhenReturningNothing=rrs, ) expected_lookup = { True: [ @@ -593,6 +622,7 @@ def testNoReturnSection( 'DOC201: Function `func3` does not have a return section in docstring ', 'DOC201: Function `func4` does not have a return section in docstring ', 'DOC201: Function `func5` does not have a return section in docstring ', + 'DOC201: Function `func7` does not have a return section in docstring ', ], False: [ 'DOC201: Function `func2` does not have a return section in docstring ',