Skip to content

Commit

Permalink
Do not require Yields section if yielding None (#80)
Browse files Browse the repository at this point in the history
  • Loading branch information
jsh9 authored Aug 29, 2023
1 parent a86157c commit 0a2b2d9
Show file tree
Hide file tree
Showing 14 changed files with 516 additions and 38 deletions.
11 changes: 11 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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
Expand Down
21 changes: 21 additions & 0 deletions pydoclint/flake8_entry.py
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down
23 changes: 23 additions & 0 deletions pydoclint/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -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"""
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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]]:
Expand Down Expand Up @@ -407,6 +423,9 @@ def _checkPaths(
requireReturnSectionWhenReturningNothing=(
requireReturnSectionWhenReturningNothing
),
requireYieldSectionWhenYieldingNothing=(
requireYieldSectionWhenYieldingNothing
),
)
allViolations[filename.as_posix()] = violationsInThisFile

Expand All @@ -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())
Expand All @@ -443,6 +463,9 @@ def _checkFile(
requireReturnSectionWhenReturningNothing=(
requireReturnSectionWhenReturningNothing
),
requireYieldSectionWhenYieldingNothing=(
requireYieldSectionWhenYieldingNothing
),
)
visitor.visit(tree)
return visitor.violations
Expand Down
25 changes: 18 additions & 7 deletions pydoclint/utils/visitor_helper.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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:
Expand All @@ -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[...]'
Expand All @@ -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))
Expand Down
75 changes: 62 additions & 13 deletions pydoclint/visitor.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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] = []
Expand Down Expand Up @@ -581,23 +585,37 @@ 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:
if not self.isAbstractMethod:
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:
Expand All @@ -610,6 +628,9 @@ def checkYields( # noqa: C901
violation=v404,
hasGeneratorAsReturnAnnotation=hasGenAsRetAnno,
hasIteratorOrIterableAsReturnAnnotation=hasIterAsRetAnno,
requireYieldSectionWhenYieldingNothing=(
self.requireYieldSectionWhenYieldingNothing
),
)

return violations
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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)
Expand Down
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.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
Expand Down
39 changes: 39 additions & 0 deletions tests/data/google/no_return_section/cases.py
Original file line number Diff line number Diff line change
Expand Up @@ -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!'
Loading

0 comments on commit 0a2b2d9

Please sign in to comment.