Skip to content

Commit

Permalink
Check consistency in return types (#33)
Browse files Browse the repository at this point in the history
  • Loading branch information
jsh9 authored Jun 26, 2023
1 parent 31e6a0b commit 3e044e0
Show file tree
Hide file tree
Showing 23 changed files with 544 additions and 63 deletions.
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

0 comments on commit 3e044e0

Please sign in to comment.