Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve violation msg on Generator/Iterator #70

Merged
merged 8 commits into from
Aug 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@

- Added
- Added checking of yield types (between function signature and the
docstring's Yields section)
docstring's Yields section), as well as a corresponding violation: `DOC404`
- Added checking of incompatibility between `Generator`/`Iterator` and the
`yield`/`return` statements, as well as a corresponding violation: `DOC405`
- Full diff
- https://github.com/jsh9/pydoclint/compare/0.1.9...0.2.0

Expand Down
60 changes: 60 additions & 0 deletions docs/notes_generator_vs_iterator.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
# Notes on `Generator` vs `Iterator`

Most likely, you landed on this page because you saw a `DOC405` violation:

> DOC405: Method `myMethod` has "yield" statements, but the return signature is
> `Iterator`. Please use `Generator` instead.

This is because _pydoclint_ uses the following criteria:

- If the return annotation in the signature is `Generator` or `AsyncGenerator`,
there should be a "Yields" section in the docstring
- If the return annotation in the signature is `Iterator`, `AsyncIterator`,
`Iterable`, or `AsyncIterable`, there should be a "Returns" section in the
docstring

Here is the rationale behind:

Firstly, `(Async)Generator` is a sub type of `(Async)Iterator/ble`:

```python
>>> from typing import Iterator, Iterable, Generator
>>> issubclass(Generator, Iterator)
True
>>> issubclass(Iterator, Generator)
False
>>> issubclass(Generator, Iterable)
True
>>> issubclass(Iterable, Generator)
False
```

Secondly, when we use `yield` statements in a function body, what's yielded is
always a `Generator`. But sometimes, we can explicitly return iterators, and
they are not generators:

```python
from typing import (
Any,
Iterator,
Tuple,
List,
)

def zip_lists(
list1: List[Any],
list2: List[Any],
) -> Iterator[Tuple[Any, Any]]:
return zip(list1, list2)
```

Additionally, people can use `return` and `yield` statements in the same
function. Although this may not be the best practice, but it is not a syntax
error.

This means that the **_only_** way to tell whether the docstring should have a
"Returns" or a "Yields" section is by looking at the function signature's
return annotation:

- `Generator`: it should have a "Yields" section
- `Iterator`: it should have a "Returns" section
5 changes: 5 additions & 0 deletions pydoclint/utils/violation.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,11 @@
402: 'has "yield" statements, but the docstring does not have a "Yields" section',
403: 'has a "Yields" section in the docstring, but there are no "yield" statements or a Generator return annotation',
404: 'yield type(s) in docstring not consistent with the return annotation.',
405: ( # noqa: PAR001
'has "yield" statements, but the return signature is `Iterator`.'
' Please use `Generator` instead.'
' (Read more about this topic here: https://jsh9.github.io/pydoclint/notes_generator_vs_iterator.html )'
),

501: 'has "raise" statements, but the docstring does not have a "Raises" section',
502: 'has a "Raises" section in the docstring, but there are not "raise" statements in the body',
Expand Down
5 changes: 5 additions & 0 deletions pydoclint/visitor.py
Original file line number Diff line number Diff line change
Expand Up @@ -614,11 +614,16 @@ def checkYields( # noqa: C901
v402 = Violation(code=402, line=lineNum, msgPrefix=msgPrefix)
v403 = Violation(code=403, line=lineNum, msgPrefix=msgPrefix)
v404 = Violation(code=404, line=lineNum, msgPrefix=msgPrefix)
v405 = Violation(code=405, line=lineNum, msgPrefix=msgPrefix)

docstringHasYieldsSection: bool = doc.hasYieldsSection

hasYieldStmt: bool = hasYieldStatements(node)
hasGenAsRetAnno: bool = hasGeneratorAsReturnAnnotation(node)
hasIterAsRetAnno: bool = hasIteratorOrIterableAsReturnAnnotation(node)

if hasIterAsRetAnno and hasYieldStmt:
violations.append(v405)

if not docstringHasYieldsSection:
if hasGenAsRetAnno:
Expand Down
39 changes: 39 additions & 0 deletions tests/test_main.py
Original file line number Diff line number Diff line change
Expand Up @@ -462,33 +462,57 @@ def testYields(style: str) -> None:
'DOC404: Method `A.method6` 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).',
'DOC405: Method `A.method7a` has "yield" statements, but the return signature '
'is `Iterator`. Please use `Generator` instead. (Read more about this topic '
'here: https://jsh9.github.io/pydoclint/notes_generator_vs_iterator.html ) ',
'DOC404: Method `A.method7a` yield type(s) in docstring not consistent with '
'the return annotation. Return annotation does not exist or is not '
'`Generator[...]`, but docstring "yields" section has 1 type(s).',
'DOC405: Method `A.method8a` has "yield" statements, but the return signature '
'is `Iterator`. Please use `Generator` instead. (Read more about this topic '
'here: https://jsh9.github.io/pydoclint/notes_generator_vs_iterator.html ) ',
'DOC402: Method `A.method8a` has "yield" statements, but the docstring does '
'not have a "Yields" section ',
'DOC404: Method `A.method8a` 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).',
'DOC405: Method `A.method7b` has "yield" statements, but the return signature '
'is `Iterator`. Please use `Generator` instead. (Read more about this topic '
'here: https://jsh9.github.io/pydoclint/notes_generator_vs_iterator.html ) ',
'DOC404: Method `A.method7b` yield type(s) in docstring not consistent with '
'the return annotation. Return annotation does not exist or is not '
'`Generator[...]`, but docstring "yields" section has 1 type(s).',
'DOC405: Method `A.method8b` has "yield" statements, but the return signature '
'is `Iterator`. Please use `Generator` instead. (Read more about this topic '
'here: https://jsh9.github.io/pydoclint/notes_generator_vs_iterator.html ) ',
'DOC402: Method `A.method8b` has "yield" statements, but the docstring does '
'not have a "Yields" section ',
'DOC404: Method `A.method8b` 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).',
'DOC405: Method `A.method7c` has "yield" statements, but the return signature '
'is `Iterator`. Please use `Generator` instead. (Read more about this topic '
'here: https://jsh9.github.io/pydoclint/notes_generator_vs_iterator.html ) ',
'DOC404: Method `A.method7c` yield type(s) in docstring not consistent with '
'the return annotation. Return annotation does not exist or is not '
'`Generator[...]`, but docstring "yields" section has 1 type(s).',
'DOC405: Method `A.method8c` has "yield" statements, but the return signature '
'is `Iterator`. Please use `Generator` instead. (Read more about this topic '
'here: https://jsh9.github.io/pydoclint/notes_generator_vs_iterator.html ) ',
'DOC402: Method `A.method8c` has "yield" statements, but the docstring does '
'not have a "Yields" section ',
'DOC404: Method `A.method8c` 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).',
'DOC405: Method `A.method7d` has "yield" statements, but the return signature '
'is `Iterator`. Please use `Generator` instead. (Read more about this topic '
'here: https://jsh9.github.io/pydoclint/notes_generator_vs_iterator.html ) ',
'DOC404: Method `A.method7d` yield type(s) in docstring not consistent with '
'the return annotation. Return annotation does not exist or is not '
'`Generator[...]`, but docstring "yields" section has 1 type(s).',
'DOC405: Method `A.method8d` has "yield" statements, but the return signature '
'is `Iterator`. Please use `Generator` instead. (Read more about this topic '
'here: https://jsh9.github.io/pydoclint/notes_generator_vs_iterator.html ) ',
'DOC402: Method `A.method8d` has "yield" statements, but the docstring does '
'not have a "Yields" section ',
'DOC404: Method `A.method8d` yield type(s) in docstring not consistent with '
Expand All @@ -497,9 +521,15 @@ def testYields(style: str) -> None:
'DOC201: Method `A.zipLists2` does not have a return section in docstring ',
'DOC403: Method `A.zipLists2` has a "Yields" section in the docstring, but '
'there are no "yield" statements or a Generator return annotation ',
'DOC405: Function `inner9a` has "yield" statements, but the return signature '
'is `Iterator`. Please use `Generator` instead. (Read more about this topic '
'here: https://jsh9.github.io/pydoclint/notes_generator_vs_iterator.html ) ',
'DOC404: Function `inner9a` yield type(s) in docstring not consistent with '
'the return annotation. Return annotation does not exist or is not '
'`Generator[...]`, but docstring "yields" section has 1 type(s).',
'DOC405: Function `inner9b` has "yield" statements, but the return signature '
'is `Iterator`. Please use `Generator` instead. (Read more about this topic '
'here: https://jsh9.github.io/pydoclint/notes_generator_vs_iterator.html ) ',
'DOC402: Function `inner9b` has "yield" statements, but the docstring does '
'not have a "Yields" section ',
'DOC404: Function `inner9b` yield type(s) in docstring not consistent with '
Expand All @@ -508,14 +538,23 @@ def testYields(style: str) -> None:
'DOC201: Method `A.method9c` does not have a return section in docstring ',
'DOC403: Method `A.method9c` has a "Yields" section in the docstring, but '
'there are no "yield" statements or a Generator return annotation ',
'DOC405: Function `inner9c` has "yield" statements, but the return signature '
'is `Iterator`. Please use `Generator` instead. (Read more about this topic '
'here: https://jsh9.github.io/pydoclint/notes_generator_vs_iterator.html ) ',
'DOC404: Function `inner9c` yield type(s) in docstring not consistent with '
'the return annotation. Return annotation does not exist or is not '
'`Generator[...]`, but docstring "yields" section has 1 type(s).',
'DOC405: Method `A.method9d` has "yield" statements, but the return signature '
'is `Iterator`. Please use `Generator` instead. (Read more about this topic '
'here: https://jsh9.github.io/pydoclint/notes_generator_vs_iterator.html ) ',
'DOC402: Method `A.method9d` has "yield" statements, but the docstring does '
'not have a "Yields" section ',
'DOC404: Method `A.method9d` 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).',
'DOC405: Function `inner9d` has "yield" statements, but the return signature '
'is `Iterator`. Please use `Generator` instead. (Read more about this topic '
'here: https://jsh9.github.io/pydoclint/notes_generator_vs_iterator.html ) ',
'DOC402: Function `inner9d` has "yield" statements, but the docstring does '
'not have a "Yields" section ',
'DOC404: Function `inner9d` yield type(s) in docstring not consistent with '
Expand Down