Skip to content

Commit

Permalink
Fix false positives in abstract methods (#35)
Browse files Browse the repository at this point in the history
  • Loading branch information
jsh9 authored Jun 27, 2023
1 parent 6aac3a6 commit 1a3596f
Show file tree
Hide file tree
Showing 8 changed files with 199 additions and 8 deletions.
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,12 @@
# Change Log

## [0.0.13] - 2023-06-26

- Fixed
- False positives when checking abstract methods
- Full diff
- https://github.com/jsh9/pydoclint/compare/0.0.12...0.0.13

## [0.0.12] - 2023-06-26

- Fixed
Expand Down
13 changes: 13 additions & 0 deletions pydoclint/utils/generic.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,19 @@ def detectMethodType(node: ast.FunctionDef) -> MethodType:
return MethodType.INSTANCE_METHOD


def checkIsAbstractMethod(node: ast.FunctionDef) -> bool:
"""Check whether `node` is an abstract method"""
if len(node.decorator_list) == 0:
return False

for decorator in node.decorator_list:
if isinstance(decorator, ast.Name):
if decorator.id == 'abstractmethod':
return True

return False


def getDocstring(node: ClassOrFunctionDef) -> str:
"""Get docstring from a class definition or a function definition"""
docstring_: Optional[str] = ast.get_docstring(node)
Expand Down
22 changes: 16 additions & 6 deletions pydoclint/visitor.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
from pydoclint.utils.astTypes import FuncOrAsyncFuncDef
from pydoclint.utils.doc import Doc
from pydoclint.utils.generic import (
checkIsAbstractMethod,
collectFuncArgs,
detectMethodType,
generateMsgPrefix,
Expand Down Expand Up @@ -76,6 +77,8 @@ def visit_FunctionDef(self, node: FuncOrAsyncFuncDef): # noqa: D102

docstring: str = getDocstring(node)

self.isAbstractMethod = checkIsAbstractMethod(node)

if isClassConstructor:
docstring = self._checkClassConstructorDocstrings(
node=node,
Expand Down Expand Up @@ -473,6 +476,13 @@ def checkReturns( # noqa: C901
):
return violations # no need to check return type hints at all

if returnSec == [] and hasGenAsRetAnno:
# This is because if the return annotation is `Generator[...]`,
# we don't need a "Returns" section. (Instead, we need a
# "Yields" section in the docstring.) Therefore, we don't need
# to check for DOC203 violations.
return violations

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:
Expand Down Expand Up @@ -567,9 +577,8 @@ def checkReturnsAndYieldsInClassConstructor(

return violations

@classmethod
def checkYields(
cls,
self,
node: FuncOrAsyncFuncDef,
parent: ast.AST,
doc: Doc,
Expand Down Expand Up @@ -598,13 +607,13 @@ def checkYields(

if docstringHasYieldsSection:
if not hasYieldStmt and not hasGenAsRetAnno:
violations.append(v403)
if not self.isAbstractMethod:
violations.append(v403)

return violations

@classmethod
def checkRaises(
cls,
self,
node: FuncOrAsyncFuncDef,
parent: ast.AST,
doc: Doc,
Expand All @@ -625,6 +634,7 @@ def checkRaises(
violations.append(v501)

if not hasRaiseStmt and docstringHasRaisesSection:
violations.append(v502)
if not self.isAbstractMethod:
violations.append(v502)

return violations
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.12
version = 0.0.13
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
54 changes: 54 additions & 0 deletions tests/data/google/abstract_method/cases.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
from abc import ABC, abstractmethod
from collections.abc import Generator, Iterator


class AbstractClass(ABC):
"""Example abstract class."""

@abstractmethod
def abstract_method(self, var1: str) -> Generator[str, None, None]:
"""Abstract method.
No violations in this method.
Args:
var1 (str): Variable.
Raises:
ValueError: Example exception
Yields:
str: Paths to the files and directories listed.
"""

@abstractmethod
def another_abstract_method(self, var1: str) -> Iterator[str]:
"""Another abstract method.
The linter will complain about not having a return section, because
if the return type annotation is `Iterator`, it is supposed to be
returning something, rather than yielding something. (To yield
something, use `Generator` as the return type annotation.)
Args:
var1 (str): Variable.
Raises:
ValueError: Example exception
Yields:
str: Paths to the files and directories listed.
"""

@abstractmethod
def third_abstract_method(self, var1: str) -> str:
"""The 3rd abstract method.
The linter will complain about not having a return section.
Args:
var1 (str): Variable.
Raises:
ValueError: Example exception
"""
70 changes: 70 additions & 0 deletions tests/data/numpy/abstract_method/cases.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
from abc import ABC, abstractmethod
from collections.abc import Generator, Iterator


class AbstractClass(ABC):
"""Example abstract class."""

@abstractmethod
def abstract_method(self, var1: str) -> Generator[str, None, None]:
"""Abstract method.
No violations in this method.
Parameters
----------
var1 : str
Variable.
Raises
------
ValueError
Example exception
Yields
------
str
Paths to the files and directories listed.
"""

@abstractmethod
def another_abstract_method(self, var1: str) -> Iterator[str]:
"""Another abstract method.
The linter will complain about not having a return section, because
if the return type annotation is `Iterator`, it is supposed to be
returning something, rather than yielding something. (To yield
something, use `Generator` as the return type annotation.)
Parameters
----------
var1 : str
Variable.
Raises
------
ValueError
Example exception
Yields
------
str
Paths to the files and directories listed.
"""

@abstractmethod
def third_abstract_method(self, var1: str) -> str:
"""The 3rd abstract method.
The linter will complain about not having a return section.
Parameters
----------
var1 : str
Variable.
Raises
------
ValueError
Example exception
"""
2 changes: 1 addition & 1 deletion tests/data/numpy/yields/cases.py
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ def zipLists2(
The second list
Yields
-------
------
Iterator[Tuple[Any, Any]]
The zipped result
"""
Expand Down
37 changes: 37 additions & 0 deletions tests/test_main.py
Original file line number Diff line number Diff line change
Expand Up @@ -575,6 +575,43 @@ def testPropertyMethod(style: str) -> None:
assert list(map(str, violations)) == expected


@pytest.mark.parametrize(
'style, checkReturnTypes',
itertools.product(
['numpy', 'google'],
[False, True],
),
)
def testAbstractMethod(style: str, checkReturnTypes: bool) -> None:
violations = _checkFile(
filename=DATA_DIR / f'{style}/abstract_method/cases.py',
checkReturnTypes=checkReturnTypes,
style=style,
)
if checkReturnTypes:
expected = [
'DOC201: Method `AbstractClass.another_abstract_method` does not have a '
'return section in docstring ',
'DOC203: Method `AbstractClass.another_abstract_method` return type(s) in '
'docstring not consistent with the return annotation. Return annotation has 1 '
'type(s); docstring return section has 0 type(s).',
'DOC201: Method `AbstractClass.third_abstract_method` does not have a return '
'section in docstring ',
'DOC203: Method `AbstractClass.third_abstract_method` return type(s) in '
'docstring not consistent with the return annotation. Return annotation has 1 '
'type(s); docstring return section has 0 type(s).',
]
else:
expected = [
'DOC201: Method `AbstractClass.another_abstract_method` does not have a '
'return section in docstring ',
'DOC201: Method `AbstractClass.third_abstract_method` does not have a return '
'section in docstring ',
]

assert list(map(str, violations)) == expected


@pytest.mark.parametrize(
'style, typeHintsInDocstring, typeHintsInSignature',
itertools.product(
Expand Down

0 comments on commit 1a3596f

Please sign in to comment.