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

Check consistency in return types #33

Merged
merged 10 commits into from
Jun 26, 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
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,13 @@
# Change Log

## [0.0.11] - 2023-06-26

- Added
- A new violation code, DOC203, which is about inconsistency between return
types in the docstring and in the return annotation
- Full diff
- https://github.com/jsh9/pydoclint/compare/0.0.10...0.0.11

## [0.0.10] - 2023-06-12

- Fixed
Expand Down
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,7 @@ Other potential causes to `DOC103` include:
| -------- | ---------------------------------------------------------------------------------------------------- |
| `DOC201` | Function/method does not have a return section in docstring |
| `DOC202` | Function/method has a return section in docstring, but there are no return statements or annotations |
| `DOC203` | Return type(s) in the docstring not consistent with the return annotation |

Note on `DOC201`: Methods with `@property` as its last decorator do not need to
have a return section.
Expand Down
11 changes: 11 additions & 0 deletions pydoclint/flake8_entry.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,17 @@ def add_options(cls, parser): # noqa: D102
' the return type annotation is "-> None".'
),
)
parser.add_option(
'-crt',
'--check-return-types',
action='store',
default='True',
parse_from_config=True,
help=(
'If True, check that the type(s) in the docstring return section and'
' the return annotation in the function signature are consistent'
),
)

@classmethod
def parse_options(cls, options): # noqa: D102
Expand Down
17 changes: 17 additions & 0 deletions pydoclint/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,17 @@ def validateStyleValue(
' the return type annotation is "-> None".'
),
)
@click.option(
'-crt',
'--check-return-types',
type=bool,
show_default=True,
default=True,
help=(
'If True, check that the type(s) in the docstring return section and'
' the return annotation in the function signature are consistent'
),
)
@click.argument(
'paths',
nargs=-1,
Expand Down Expand Up @@ -172,6 +183,7 @@ def main( # noqa: C901
skip_checking_short_docstrings: bool,
skip_checking_raises: bool,
allow_init_docstring: bool,
check_return_types: bool,
require_return_section_when_returning_none: bool,
config: Optional[str], # don't remove it b/c it's required by `click`
) -> None:
Expand Down Expand Up @@ -204,6 +216,7 @@ def main( # noqa: C901
skipCheckingShortDocstrings=skip_checking_short_docstrings,
skipCheckingRaises=skip_checking_raises,
allowInitDocstring=allow_init_docstring,
checkReturnTypes=check_return_types,
requireReturnSectionWhenReturningNone=require_return_section_when_returning_none,
)

Expand Down Expand Up @@ -259,6 +272,7 @@ def _checkPaths(
skipCheckingShortDocstrings: bool = True,
skipCheckingRaises: bool = False,
allowInitDocstring: bool = False,
checkReturnTypes: bool = True,
requireReturnSectionWhenReturningNone: bool = False,
quiet: bool = False,
exclude: str = '',
Expand Down Expand Up @@ -300,6 +314,7 @@ def _checkPaths(
skipCheckingShortDocstrings=skipCheckingShortDocstrings,
skipCheckingRaises=skipCheckingRaises,
allowInitDocstring=allowInitDocstring,
checkReturnTypes=checkReturnTypes,
requireReturnSectionWhenReturningNone=requireReturnSectionWhenReturningNone,
)
allViolations[filename.as_posix()] = violationsInThisFile
Expand All @@ -316,6 +331,7 @@ def _checkFile(
skipCheckingShortDocstrings: bool = True,
skipCheckingRaises: bool = False,
allowInitDocstring: bool = False,
checkReturnTypes: bool = True,
requireReturnSectionWhenReturningNone: bool = False,
) -> List[Violation]:
with open(filename, encoding='utf8') as fp:
Expand All @@ -330,6 +346,7 @@ def _checkFile(
skipCheckingShortDocstrings=skipCheckingShortDocstrings,
skipCheckingRaises=skipCheckingRaises,
allowInitDocstring=allowInitDocstring,
checkReturnTypes=checkReturnTypes,
requireReturnSectionWhenReturningNone=requireReturnSectionWhenReturningNone,
)
visitor.visit(tree)
Expand Down
36 changes: 35 additions & 1 deletion pydoclint/utils/doc.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
from docstring_parser.common import DocstringReturns
from typing import Any, List

from docstring_parser.common import Docstring, DocstringReturns
from docstring_parser.google import GoogleParser
from numpydoc.docscrape import NumpyDocString

from pydoclint.utils.arg import ArgList
from pydoclint.utils.internal_error import InternalError
from pydoclint.utils.return_arg import ReturnArg


class Doc:
Expand Down Expand Up @@ -117,6 +120,37 @@ def hasRaisesSection(self) -> bool:

self._raiseException() # noqa: R503

@property
def returnSection(self) -> List[ReturnArg]:
"""Get the return section of the docstring"""
if isinstance(self.parsed, Docstring): # Google style
returnArg = ReturnArg(
argName=self._str(self.parsed.returns.return_name),
argType=self._str(self.parsed.returns.type_name),
argDescr=self._str(self.parsed.returns.description),
)
return [returnArg] # Google style always has only 1 return arg

if isinstance(self.parsed, NumpyDocString): # numpy style
returnSection = self.parsed.get('Returns')
result = []
for element in returnSection:
result.append(
ReturnArg(
argName=self._str(element.name),
argType=self._str(element.type),
argDescr=' '.join(element.desc),
)
)

return result

return []

def _raiseException(self) -> None:
msg = f'Unknown style "{self.style}"; please contact the authors'
raise InternalError(msg)

@classmethod
def _str(cls, something: Any) -> str:
return '' if something is None else str(something)
66 changes: 66 additions & 0 deletions pydoclint/utils/return_anno.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
import ast
from typing import List, Optional

from pydoclint.utils.annotation import unparseAnnotation
from pydoclint.utils.internal_error import InternalError


class ReturnAnnotation:
"""A class to hold the return annotation in a function's signature"""

def __init__(self, annotation: Optional[str]) -> None:
self.annotation = annotation

def decompose(self) -> List[str]:
"""
Numpy style allows decomposing the returning tuple into individual
element. For example, if the return annotation is `Tuple[int, bool]`,
you can put 2 return values in the return section: int, and bool.

This method decomposes such return annotation into individual elements.

Returns
-------
List[str]
The decomposed element

Raises
------
InternalError
When the annotation string has strange values
"""
if self._isTuple(): # noqa: R506
if not self.annotation.endswith(']'):
raise InternalError('Return annotation not ending with `]`')

if len(self.annotation) < 7:
raise InternalError(f'Impossible annotation {self.annotation}')

if self.annotation.lower() == 'tuple[]':
return []

insideTuple: str = self.annotation[6:-1]
if insideTuple.endswith('...'): # like this: Tuple[int, ...]
return [self.annotation] # b/c we don't know the tuple's length

parsedBody0: ast.Expr = ast.parse(insideTuple).body[0]
if isinstance(parsedBody0.value, ast.Name): # like this: Tuple[int]
return [insideTuple]

if isinstance(parsedBody0.value, ast.Tuple): # like Tuple[int, str]
elts: List = parsedBody0.value.elts
return [unparseAnnotation(_) for _ in elts]

raise InternalError('decompose(): This should not have happened')
else:
return self.putAnnotationInList()

def _isTuple(self) -> bool:
return (
self.annotation is not None
and self.annotation.lower().startswith('tuple[')
)

def putAnnotationInList(self) -> List[str]:
"""Put annotation string in a list"""
return [] if self.annotation is None else [self.annotation]
10 changes: 10 additions & 0 deletions pydoclint/utils/return_arg.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
from dataclasses import dataclass


@dataclass
class ReturnArg:
"""A class to hold one return argument in the docstring's return section"""

argName: str
argType: str
argDescr: str
8 changes: 8 additions & 0 deletions pydoclint/utils/violation.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import types
from copy import deepcopy
from typing import Tuple

from pydoclint.utils.internal_error import InternalError
Expand All @@ -23,6 +24,7 @@

201: 'does not have a return section in docstring',
202: 'has a return section in docstring, but there are no return statements or annotations',
203: 'return type(s) in docstring not consistent with the return annotation.',

301: '__init__() should not have a docstring; please combine it with the docstring of the class',
302: 'The class docstring does not need a "Returns" section, because __init__() cannot return anything',
Expand Down Expand Up @@ -80,3 +82,9 @@ def getInfoForFlake8(self) -> Tuple[int, int, str]:
colOffset: int = 0 # we don't need column offset to locate the issue
msg = f'{self.fullErrorCode} {self.msg}' # no colon b/c that would cause 'yesqa' issues
return self.line, colOffset, msg

def appendMoreMsg(self, moreMsg: str) -> 'Violation':
"""Append more error message, and return a new Violation object"""
new = deepcopy(self)
new.msg += moreMsg
return new
83 changes: 82 additions & 1 deletion pydoclint/visitor.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import ast
from typing import List, Optional, Set

from pydoclint.utils.annotation import unparseAnnotation
from pydoclint.utils.arg import Arg, ArgList
from pydoclint.utils.astTypes import FuncOrAsyncFuncDef
from pydoclint.utils.doc import Doc
Expand All @@ -13,6 +14,8 @@
)
from pydoclint.utils.internal_error import InternalError
from pydoclint.utils.method_type import MethodType
from pydoclint.utils.return_anno import ReturnAnnotation
from pydoclint.utils.return_arg import ReturnArg
from pydoclint.utils.return_yield_raise import (
hasGeneratorAsReturnAnnotation,
hasIteratorOrIterableAsReturnAnnotation,
Expand All @@ -37,6 +40,7 @@ def __init__(
skipCheckingShortDocstrings: bool = True,
skipCheckingRaises: bool = False,
allowInitDocstring: bool = False,
checkReturnTypes: bool = True,
requireReturnSectionWhenReturningNone: bool = False,
) -> None:
self.style: str = style
Expand All @@ -46,6 +50,7 @@ def __init__(
self.skipCheckingShortDocstrings: bool = skipCheckingShortDocstrings
self.skipCheckingRaises: bool = skipCheckingRaises
self.allowInitDocstring: bool = allowInitDocstring
self.checkReturnTypes: bool = checkReturnTypes
self.requireReturnSectionWhenReturningNone: bool = (
requireReturnSectionWhenReturningNone
)
Expand Down Expand Up @@ -408,7 +413,7 @@ def checkArguments( # noqa: C901

return violations

def checkReturns(
def checkReturns( # noqa: C901
self,
node: FuncOrAsyncFuncDef,
parent: ast.AST,
Expand All @@ -420,6 +425,7 @@ def checkReturns(

v201 = Violation(code=201, line=lineNum, msgPrefix=msgPrefix)
v202 = Violation(code=202, line=lineNum, msgPrefix=msgPrefix)
v203 = Violation(code=203, line=lineNum, msgPrefix=msgPrefix)

hasReturnStmt: bool = hasReturnStatements(node)
hasReturnAnno: bool = hasReturnAnnotation(node)
Expand Down Expand Up @@ -449,6 +455,81 @@ def checkReturns(
if docstringHasReturnSection and not (hasReturnStmt or hasReturnAnno):
violations.append(v202)

if self.checkReturnTypes:
if hasReturnAnno:
returnAnno = ReturnAnnotation(unparseAnnotation(node.returns))
else:
returnAnno = ReturnAnnotation(annotation=None)

if docstringHasReturnSection:
returnSec: List[ReturnArg] = doc.returnSection
else:
returnSec = []

if self.style == 'numpy':
# If the return annotation is a tuple (such as Tuple[int, str]),
# we consider both in the docstring to be a valid style:
#
# Option 1:
# > Returns
# > -------
# > Tuple[int, str]
# > ...
#
# Option 2:
# > Returns
# > -------
# > int
# > ...
# > str
# > ...
#
# This is why we are comparing both the decomposed annotation
# types and the original annotation type
returnAnnoItems: List[str] = returnAnno.decompose()
returnAnnoInList: List[str] = returnAnno.putAnnotationInList()

returnSecTypes: List[str] = [_.argType for _ in returnSec]

if returnAnnoInList != returnSecTypes:
if len(returnAnnoItems) != len(returnSec):
msg = f'Return annotation has {len(returnAnnoItems)}'
msg += ' type(s); docstring return section has'
msg += f' {len(returnSec)} type(s).'
violations.append(v203.appendMoreMsg(moreMsg=msg))
else:
if returnSecTypes != returnAnnoItems:
msg1 = (
f'Return annotation types: {returnAnnoItems}; '
)
msg2 = f'docstring return section types: {returnSecTypes}'
violations.append(v203.appendMoreMsg(msg1 + msg2))

else: # Google style
# The Google docstring style does not allow (or at least does
# not encourage) splitting tuple return types (such as
# Tuple[int, str, bool]) into individual types (int, str, and
# bool).
# Therefore, in Google-style docstrings, people should always
# use one compound style for tuples.

if len(returnSec) > 0:
if returnAnno.annotation is None:
msg = 'Return annotation has 0 type(s); docstring'
msg += ' return section has 1 type(s).'
violations.append(v203.appendMoreMsg(moreMsg=msg))
elif returnSec[0].argType != returnAnno.annotation:
msg = 'Return annotation types: '
msg += str([returnAnno.annotation]) + '; '
msg += 'docstring return section types: '
msg += str([returnSec[0].argType])
violations.append(v203.appendMoreMsg(moreMsg=msg))
else:
if returnAnno.annotation != '':
msg = 'Return annotation has 1 type(s); docstring'
msg += ' return section has 0 type(s).'
violations.append(v203.appendMoreMsg(moreMsg=msg))

return violations

@classmethod
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.0.10
version = 0.0.11
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
Loading