diff --git a/CHANGELOG.md b/CHANGELOG.md index e926948..34df744 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,16 @@ # Change Log +## [0.3.1] - 2023-08-28 + +- Added + + - Added an option `--require-yield-section-when-yielding-nothing` (defaulting + to `False`). When it's False, we don't need a "Yields" section when a + function yields None (https://github.com/jsh9/pydoclint/issues/79) + +- Full diff + - https://github.com/jsh9/pydoclint/compare/0.3.0...0.3.1 + ## [0.3.0] - 2023-08-26 - Improved diff --git a/pydoclint/flake8_entry.py b/pydoclint/flake8_entry.py index e46724a..a4644f2 100644 --- a/pydoclint/flake8_entry.py +++ b/pydoclint/flake8_entry.py @@ -123,6 +123,17 @@ def add_options(cls, parser): # noqa: D102 ' the return annotation in the function signature are consistent' ), ) + parser.add_option( + '-rys', + '--require-yield-section-when-yielding-nothing', + action='store', + default='False', + parse_from_config=True, + help=( + 'If False, a yields section is not needed in docstring if' + ' the function yields None.' + ), + ) parser.add_option( '-cyt', '--check-yield-types', @@ -153,6 +164,9 @@ def parse_options(cls, options): # noqa: D102 cls.require_return_section_when_returning_nothing = ( options.require_return_section_when_returning_nothing ) + cls.require_yield_section_when_yielding_nothing = ( + options.require_yield_section_when_yielding_nothing + ) cls.check_return_types = options.check_return_types cls.check_yield_types = options.check_yield_types cls.style = options.style @@ -204,6 +218,10 @@ def run(self) -> Generator[Tuple[int, int, str, Any], None, None]: '--require-return-section-when-returning-nothing', self.require_return_section_when_returning_nothing, ) + requireYieldSectionWhenYieldingNothing = self._bool( + '--require-yield-section-when-yielding-nothing', + self.require_yield_section_when_yielding_nothing, + ) checkReturnTypes = self._bool( '--check-return-types', self.check_return_types, @@ -228,6 +246,9 @@ def run(self) -> Generator[Tuple[int, int, str, Any], None, None]: requireReturnSectionWhenReturningNothing=( requireReturnSectionWhenReturningNothing ), + requireYieldSectionWhenYieldingNothing=( + requireYieldSectionWhenYieldingNothing + ), checkReturnTypes=checkReturnTypes, checkYieldTypes=checkYieldTypes, style=self.style, diff --git a/pydoclint/main.py b/pydoclint/main.py index 9cb1765..577ce5b 100644 --- a/pydoclint/main.py +++ b/pydoclint/main.py @@ -163,6 +163,17 @@ def validateStyleValue( ' the return annotation in the function signature are consistent' ), ) +@click.option( + '-rys', + '--require-yield-section-when-yielding-nothing', + type=bool, + show_default=True, + default=False, + help=( + 'If False, a yields section is not needed in docstring if' + ' the function yields None.' + ), +) @click.option( '-cyt', '--check-yield-types', @@ -225,6 +236,7 @@ def main( # noqa: C901 check_yield_types: bool, require_return_section_when_returning_none: bool, require_return_section_when_returning_nothing: bool, + require_yield_section_when_yielding_nothing: bool, config: Optional[str], # don't remove it b/c it's required by `click` ) -> None: """Command-line entry point of pydoclint""" @@ -305,6 +317,9 @@ def main( # noqa: C901 requireReturnSectionWhenReturningNothing=( require_return_section_when_returning_nothing ), + requireYieldSectionWhenYieldingNothing=( + require_yield_section_when_yielding_nothing + ), ) violationCounter: int = 0 @@ -362,6 +377,7 @@ def _checkPaths( checkReturnTypes: bool = True, checkYieldTypes: bool = True, requireReturnSectionWhenReturningNothing: bool = False, + requireYieldSectionWhenYieldingNothing: bool = False, quiet: bool = False, exclude: str = '', ) -> Dict[str, List[Violation]]: @@ -407,6 +423,9 @@ def _checkPaths( requireReturnSectionWhenReturningNothing=( requireReturnSectionWhenReturningNothing ), + requireYieldSectionWhenYieldingNothing=( + requireYieldSectionWhenYieldingNothing + ), ) allViolations[filename.as_posix()] = violationsInThisFile @@ -425,6 +444,7 @@ def _checkFile( checkReturnTypes: bool = True, checkYieldTypes: bool = True, requireReturnSectionWhenReturningNothing: bool = False, + requireYieldSectionWhenYieldingNothing: bool = False, ) -> List[Violation]: with open(filename, encoding='utf8') as fp: src: str = ''.join(fp.readlines()) @@ -443,6 +463,9 @@ def _checkFile( requireReturnSectionWhenReturningNothing=( requireReturnSectionWhenReturningNothing ), + requireYieldSectionWhenYieldingNothing=( + requireYieldSectionWhenYieldingNothing + ), ) visitor.visit(tree) return visitor.violations diff --git a/pydoclint/utils/visitor_helper.py b/pydoclint/utils/visitor_helper.py index 4658220..86a640d 100644 --- a/pydoclint/utils/visitor_helper.py +++ b/pydoclint/utils/visitor_helper.py @@ -123,6 +123,7 @@ def checkYieldTypesForViolations( violation: Violation, hasGeneratorAsReturnAnnotation: bool, hasIteratorOrIterableAsReturnAnnotation: bool, + requireYieldSectionWhenYieldingNothing: bool, ) -> None: """Check yield types between function signature and docstring""" # Even though the numpy docstring guide demonstrates that we can @@ -133,6 +134,11 @@ def checkYieldTypesForViolations( # to check and less ambiguous. returnAnnoText: Optional[str] = returnAnnotation.annotation + yieldType: str = extractYieldTypeFromGeneratorOrIteratorAnnotation( + returnAnnoText, + hasGeneratorAsReturnAnnotation, + hasIteratorOrIterableAsReturnAnnotation, + ) if len(yieldSection) > 0: if returnAnnoText is None: @@ -141,12 +147,6 @@ def checkYieldTypesForViolations( msg += ' but docstring "yields" section has 1 type(s).' violationList.append(violation.appendMoreMsg(moreMsg=msg)) else: - yieldType: str = extractYieldTypeFromGeneratorOrIteratorAnnotation( - returnAnnoText, - hasGeneratorAsReturnAnnotation, - hasIteratorOrIterableAsReturnAnnotation, - ) - if yieldSection[0].argType != yieldType: msg = ( 'The yield type (the 0th arg in Generator[...]' @@ -157,7 +157,18 @@ def checkYieldTypesForViolations( msg += str(yieldSection[0].argType) violationList.append(violation.appendMoreMsg(moreMsg=msg)) else: - if returnAnnoText != '': + if ( + ( + hasGeneratorAsReturnAnnotation + or hasIteratorOrIterableAsReturnAnnotation + ) + and yieldType == 'None' + and not requireYieldSectionWhenYieldingNothing + ): + # This means that we don't need to have a "Yields" section in the + # docstring if the yield type is None. + pass + elif returnAnnoText != '': msg = 'Return annotation exists, but docstring' msg += ' "yields" section does not exist or has 0 type(s).' violationList.append(violation.appendMoreMsg(moreMsg=msg)) diff --git a/pydoclint/visitor.py b/pydoclint/visitor.py index 2fdd2a4..37e66c6 100644 --- a/pydoclint/visitor.py +++ b/pydoclint/visitor.py @@ -52,6 +52,7 @@ def __init__( checkReturnTypes: bool = True, checkYieldTypes: bool = True, requireReturnSectionWhenReturningNothing: bool = False, + requireYieldSectionWhenYieldingNothing: bool = False, ) -> None: self.style: str = style self.argTypeHintsInSignature: bool = argTypeHintsInSignature @@ -65,6 +66,9 @@ def __init__( self.requireReturnSectionWhenReturningNothing: bool = ( requireReturnSectionWhenReturningNothing ) + self.requireYieldSectionWhenYieldingNothing: bool = ( + requireYieldSectionWhenYieldingNothing + ) self.parent: Optional[ast.AST] = None # keep track of parent node self.violations: List[Violation] = [] @@ -581,9 +585,30 @@ def checkYields( # noqa: C901 hasIterAsRetAnno: bool = hasIteratorOrIterableAsReturnAnnotation(node) noGenNorIterAsRetAnno = not hasGenAsRetAnno and not hasIterAsRetAnno + if hasGenAsRetAnno or hasIterAsRetAnno: + returnAnno = ReturnAnnotation(unparseAnnotation(node.returns)) + else: + # We don't check other return annotations here, because they + # are checked above, in `checkReturns()`. + returnAnno = ReturnAnnotation(None) + if not docstringHasYieldsSection: + yieldType: str = extractYieldTypeFromGeneratorOrIteratorAnnotation( + returnAnnoText=returnAnno.annotation, + hasGeneratorAsReturnAnnotation=hasGenAsRetAnno, + hasIteratorOrIterableAsReturnAnnotation=hasIterAsRetAnno, + ) if hasYieldStmt: - violations.append(v402) + if ( + yieldType == 'None' + and not self.requireYieldSectionWhenYieldingNothing + ): + # This means that people don't need to add a "Yields" + # section in the docstring, if the yield type in the + # signature's return annotation is None. + pass + else: + violations.append(v402) if docstringHasYieldsSection: if not hasYieldStmt or noGenNorIterAsRetAnno: @@ -591,13 +616,6 @@ def checkYields( # noqa: C901 violations.append(v403) if hasYieldStmt and self.checkYieldTypes: - if hasGenAsRetAnno or hasIterAsRetAnno: - returnAnno = ReturnAnnotation(unparseAnnotation(node.returns)) - else: - # We don't check other return annotations here, because they - # are checked above, in `checkReturns()`. - returnAnno = ReturnAnnotation(None) - if docstringHasYieldsSection: yieldSec: List[YieldArg] = doc.yieldSection else: @@ -610,6 +628,9 @@ def checkYields( # noqa: C901 violation=v404, hasGeneratorAsReturnAnnotation=hasGenAsRetAnno, hasIteratorOrIterableAsReturnAnnotation=hasIterAsRetAnno, + requireYieldSectionWhenYieldingNothing=( + self.requireYieldSectionWhenYieldingNothing + ), ) return violations @@ -667,15 +688,40 @@ def my_function(num: int) -> Generator[int, None, str]: hasGenAsRetAnno: bool = hasGeneratorAsReturnAnnotation(node) hasIterAsRetAnno: bool = hasIteratorOrIterableAsReturnAnnotation(node) + hasReturnStmt: bool = hasReturnStatements(node) + hasYieldStmt: bool = hasYieldStatements(node) + onlyHasYieldStmt: bool = hasYieldStmt and not hasReturnStmt + hasReturnAnno: bool = hasReturnAnnotation(node) + + returnAnno = ReturnAnnotation(unparseAnnotation(node.returns)) + returnSec: List[ReturnArg] = doc.returnSection + # Check the return section in the docstring if not docstringHasReturnSection: - if not self.skipCheckingShortDocstrings: - violations.append(v201) + if doc.isShortDocstring and self.skipCheckingShortDocstrings: + pass + else: + if ( + # fmt: off + not (onlyHasYieldStmt and hasIterAsRetAnno) + and (hasReturnStmt or ( + hasReturnAnno and not hasGenAsRetAnno + )) + + # fmt: on + ): + retTypeInGenerator: str = extractReturnTypeFromGenerator( + returnAnnoText=returnAnno.annotation, + ) + # 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.requireReturnSectionWhenReturningNothing: + violations.append(v201) + elif retTypeInGenerator not in {'None', 'NoReturn'}: + violations.append(v201) else: if self.checkReturnTypes: - returnAnno = ReturnAnnotation(unparseAnnotation(node.returns)) - returnSec: List[ReturnArg] = doc.returnSection - if hasGenAsRetAnno: retTypeInGenerator: str = extractReturnTypeFromGenerator( returnAnnoText=returnAnno.annotation, @@ -716,6 +762,9 @@ def my_function(num: int) -> Generator[int, None, str]: violation=v404, hasGeneratorAsReturnAnnotation=hasGenAsRetAnno, hasIteratorOrIterableAsReturnAnnotation=hasIterAsRetAnno, + requireYieldSectionWhenYieldingNothing=( + self.requireYieldSectionWhenYieldingNothing + ), ) else: violations.append(v405) diff --git a/setup.cfg b/setup.cfg index 715cef4..a5122b4 100644 --- a/setup.cfg +++ b/setup.cfg @@ -1,6 +1,6 @@ [metadata] name = pydoclint -version = 0.3.0 +version = 0.3.1 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 5bc4e4c..ee7d833 100644 --- a/tests/data/google/no_return_section/cases.py +++ b/tests/data/google/no_return_section/cases.py @@ -67,3 +67,42 @@ def func7(text: str) -> NoReturn: text (str): Text for the function """ exit(1) + + +def func8(num: int) -> Generator[None, None, None]: + """ + Do something + + Args: + num (int): A number + """ + for i in range(num): + yield i + + return 'All numbers yielded!' + + +def func9(num: int) -> Generator[None, None, NoReturn]: + """ + Do something + + Args: + num (int): A number + """ + for i in range(num): + yield i + + return 'All numbers yielded!' + + +def func10(num: int) -> Generator[bool, None, str]: + """ + Do something + + Args: + num (int): A number + """ + for i in range(num): + yield i + + return 'All numbers yielded!' diff --git a/tests/data/google/no_yield_section/cases.py b/tests/data/google/no_yield_section/cases.py new file mode 100644 index 0000000..83e9f97 --- /dev/null +++ b/tests/data/google/no_yield_section/cases.py @@ -0,0 +1,66 @@ +def func1(arg1: int) -> Iterator[None]: + """ + This function doesn't yield anything. + + Args: + arg1 (int): Arg 1 + """ + # It doesn't matter what actually is yielded. pydoclint only checks + # the yield type in the signature's return annotation + yield 1 + + +def func2(arg1: int) -> Generator[None, None, None]: + """ + This function doesn't yield anything. + + Args: + arg1 (int): Arg 1 + """ + # It doesn't matter what actually is yielded. pydoclint only checks + # the yield type in the signature's return annotation + yield 2 + + +def func3(arg1: int) -> Generator[str, None, None]: + """ + This function doesn't yield anything. + + Args: + arg1 (int): Arg 1 + """ + # It doesn't matter what actually is yielded. pydoclint only checks + # the yield type in the signature's return annotation + yield 2 + + +def func4(num: int) -> Generator[None, None, str]: + """ + Do something + + Args: + num (int): A number + + Returns: + str: A string + """ + for i in range(num): + yield i + + return 'All numbers yielded!' + + +def func5(num: int) -> Generator[bool, None, str]: + """ + Do something + + Args: + num (int): A number + + Returns: + str: A string + """ + for i in range(num): + yield i + + return 'All numbers yielded!' diff --git a/tests/data/numpy/no_return_section/cases.py b/tests/data/numpy/no_return_section/cases.py index 4304ac7..5653d3a 100644 --- a/tests/data/numpy/no_return_section/cases.py +++ b/tests/data/numpy/no_return_section/cases.py @@ -81,3 +81,48 @@ def func7(text: str) -> NoReturn: Text for the function """ exit(1) + + +def func8(num: int) -> Generator[None, None, None]: + """ + Do something + + Parameters + ---------- + num : int + A number + """ + for i in range(num): + yield i + + return 'All numbers yielded!' + + +def func9(num: int) -> Generator[None, None, NoReturn]: + """ + Do something + + Parameters + ---------- + num : int + A number + """ + for i in range(num): + yield i + + return 'All numbers yielded!' + + +def func10(num: int) -> Generator[bool, None, str]: + """ + Do something + + Parameters + ---------- + num : int + A number + """ + for i in range(num): + yield i + + return 'All numbers yielded!' diff --git a/tests/data/numpy/no_yield_section/cases.py b/tests/data/numpy/no_yield_section/cases.py new file mode 100644 index 0000000..b3365a3 --- /dev/null +++ b/tests/data/numpy/no_yield_section/cases.py @@ -0,0 +1,80 @@ +def func1(arg1: int) -> Iterator[None]: + """ + This function doesn't yield anything. + + Parameters + ---------- + arg1 : int + Arg 1 + """ + # It doesn't matter what actually is yielded. pydoclint only checks + # the yield type in the signature's return annotation + yield 1 + + +def func2(arg1: int) -> Generator[None, None, None]: + """ + This function doesn't yield anything. + + Parameters + ---------- + arg1 : int + Arg 1 + """ + # It doesn't matter what actually is yielded. pydoclint only checks + # the yield type in the signature's return annotation + yield 2 + + +def func3(arg1: int) -> Generator[str, None, None]: + """ + This function doesn't yield anything. + + Parameters + ---------- + arg1 : int + Arg 1 + """ + # It doesn't matter what actually is yielded. pydoclint only checks + # the yield type in the signature's return annotation + yield 2 + + +def func4(num: int) -> Generator[None, None, str]: + """ + Do something + + Parameters + ---------- + num : int + A number + + Returns + ------- + str + A string + """ + for i in range(num): + yield i + + return 'All numbers yielded!' + + +def func5(num: int) -> Generator[bool, None, str]: + """ + Do something + + Parameters + ---------- + num : int + A number + + Returns + ------- + str + A string + """ + for i in range(num): + yield i + + return 'All numbers yielded!' diff --git a/tests/data/playground.py b/tests/data/playground.py index d49585b..8750f81 100644 --- a/tests/data/playground.py +++ b/tests/data/playground.py @@ -3,19 +3,3 @@ # If you'd like to quickly try out some edge cases, put them in this file, # and run `testPlayground()` in `test_main.py`. This allows you to quickly # diagnose and debug some issues. - -from typing import Generator - - -def func82() -> Tuple[int, bool]: - """ - If no summary here, the parser will mis-parse the return section - - Returns - ------- - int - Integer to return - bool - Boolean to return - """ - return (1, 1.1) diff --git a/tests/data/sphinx/no_return_section/cases.py b/tests/data/sphinx/no_return_section/cases.py index 1721b04..0375631 100644 --- a/tests/data/sphinx/no_return_section/cases.py +++ b/tests/data/sphinx/no_return_section/cases.py @@ -67,3 +67,42 @@ def func7(text: str) -> NoReturn: :type text: str """ exit(1) + + +def func8(num: int) -> Generator[None, None, None]: + """ + Do something + + :param num: A number + :type num: int + """ + for i in range(num): + yield i + + return 'All numbers yielded!' + + +def func9(num: int) -> Generator[None, None, NoReturn]: + """ + Do something + + :param num: A number + :type num: int + """ + for i in range(num): + yield i + + return 'All numbers yielded!' + + +def func10(num: int) -> Generator[bool, None, str]: + """ + Do something + + :param num: A number + :type num: int + """ + for i in range(num): + yield i + + return 'All numbers yielded!' diff --git a/tests/data/sphinx/no_yield_section/cases.py b/tests/data/sphinx/no_yield_section/cases.py new file mode 100644 index 0000000..be3763c --- /dev/null +++ b/tests/data/sphinx/no_yield_section/cases.py @@ -0,0 +1,64 @@ +def func1(arg1: int) -> Iterator[None]: + """ + This function doesn't yield anything. + + :param arg1: Arg 1 + :type arg1: int + """ + # It doesn't matter what actually is yielded. pydoclint only checks + # the yield type in the signature's return annotation + yield 1 + + +def func2(arg1: int) -> Generator[None, None, None]: + """ + This function doesn't yield anything. + + :param arg1: Arg 1 + :type arg1: int + """ + # It doesn't matter what actually is yielded. pydoclint only checks + # the yield type in the signature's return annotation + yield 2 + + +def func3(arg1: int) -> Generator[str, None, None]: + """ + This function doesn't yield anything. + + :param arg1: Arg 1 + :type arg1: int + """ + # It doesn't matter what actually is yielded. pydoclint only checks + # the yield type in the signature's return annotation + yield 2 + + +def func4(num: int) -> Generator[None, None, str]: + """ + Do something + + :param num: A number + :type num: int + :return: A string + :rtype: str + """ + for i in range(num): + yield i + + return 'All numbers yielded!' + + +def func5(num: int) -> Generator[bool, None, str]: + """ + Do something + + :param num: A number + :type num: int + :return: A string + :rtype: str + """ + for i in range(num): + yield i + + return 'All numbers yielded!' diff --git a/tests/test_main.py b/tests/test_main.py index 75164ce..4fbb1d3 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -795,17 +795,63 @@ def testNoReturnSection( '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 ', + 'DOC201: Function `func8` does not have a return section in docstring ', + 'DOC201: Function `func9` does not have a return section in docstring ', + 'DOC201: Function `func10` does not have a return section in docstring ', ], False: [ 'DOC201: Function `func2` does not have a return section in docstring ', '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 `func10` does not have a return section in docstring ', ], } assert list(map(str, violations)) == expected_lookup[rrs] +@pytest.mark.parametrize( + 'style, rys', + itertools.product( + ['google', 'numpy', 'sphinx'], + [False, True], + ), +) +def testNoYieldSection(style: str, rys: bool) -> None: + violations = _checkFile( + filename=DATA_DIR / f'{style}/no_yield_section/cases.py', + style=style, + requireYieldSectionWhenYieldingNothing=rys, + ) + expected_lookup = { + True: [ + 'DOC402: Function `func1` has "yield" statements, but the docstring does not ' + 'have a "Yields" section ', + 'DOC404: Function `func1` yield type(s) in docstring not consistent with the ' + 'return annotation. Return annotation exists, but docstring "yields" section ' + 'does not exist or has 0 type(s).', + 'DOC402: Function `func2` has "yield" statements, but the docstring does not ' + 'have a "Yields" section ', + 'DOC404: Function `func2` yield type(s) in docstring not consistent with the ' + 'return annotation. Return annotation exists, but docstring "yields" section ' + 'does not exist or has 0 type(s).', + 'DOC402: Function `func3` has "yield" statements, but the docstring does not ' + 'have a "Yields" section ', + 'DOC404: Function `func3` yield type(s) in docstring not consistent with the ' + 'return annotation. Return annotation exists, but docstring "yields" section ' + 'does not exist or has 0 type(s).', + ], + False: [ + 'DOC402: Function `func3` has "yield" statements, but the docstring does not ' + 'have a "Yields" section ', + 'DOC404: Function `func3` yield type(s) in docstring not consistent with the ' + 'return annotation. Return annotation exists, but docstring "yields" section ' + 'does not exist or has 0 type(s).', + ], + } + assert list(map(str, violations)) == expected_lookup[rys] + + @pytest.mark.parametrize( 'style', ['google', 'numpy', 'sphinx'], @@ -1061,7 +1107,7 @@ def testPlayground() -> None: """ violations = _checkFile( filename=DATA_DIR / 'playground.py', - style='numpy', + style='google', ) expected = [] assert list(map(str, violations)) == expected