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

feat: introduce DOC503 for checking specific raised exceptions #161

Merged
merged 6 commits into from
Sep 3, 2024
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
7 changes: 6 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
# Change Log

## [unpublished] - 2024-08-05
## [0.5.7] - 2024-09-02

- Added

- A new violation code, `DOC503`, which checks that exceptions in the
function body match those in the "Raises" section of the docstring

- Changed
- Switched from tab to 4 spaces in baseline
Expand Down
1 change: 1 addition & 0 deletions docs/violation_codes.md
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ have a return section.
| -------- | --------------------------------------------------------------------------------------------------------- |
| `DOC501` | Function/method has "raise" statements, but the docstring does not have a "Raises" section |
| `DOC502` | Function/method has a "Raises" section in the docstring, but there are not "raise" statements in the body |
| `DOC503` | Exceptions in the "Raises" section in the docstring do not match those in the function body |

## 6. `DOC6xx`: Violations about class attributes

Expand Down
63 changes: 62 additions & 1 deletion pydoclint/utils/return_yield_raise.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import ast
from typing import Callable, Dict, Tuple, Type
from typing import Callable, Dict, Generator, List, Optional, Tuple, Type

from pydoclint.utils import walk
from pydoclint.utils.annotation import unparseAnnotation
Expand Down Expand Up @@ -93,6 +93,67 @@ def isThisNodeARaiseStmt(node_: ast.AST) -> bool:
return _hasExpectedStatements(node, isThisNodeARaiseStmt)


def getRaisedExceptions(node: FuncOrAsyncFuncDef) -> List[str]:
"""Get the raised exceptions in a function node as a sorted list"""
return sorted(set(_getRaisedExceptions(node)))


def _getRaisedExceptions(
node: FuncOrAsyncFuncDef,
) -> Generator[str, None, None]:
"""Yield the raised exceptions in a function node"""
childLineNum: int = -999

# key: child lineno, value: (parent lineno, is parent a function?)
familyTree: Dict[int, Tuple[int, bool]] = {}

currentParentExceptHandler: Optional[ast.ExceptHandler] = None

# Depth-first guarantees the last-seen exception handler
# is a parent of child.
for child, parent in walk.walk_dfs(node):
childLineNum = _updateFamilyTree(child, parent, familyTree)

if isinstance(parent, ast.ExceptHandler):
currentParentExceptHandler = parent

if (
isinstance(child, ast.Raise)
and isinstance(
parent,
(ast.AsyncFunctionDef, ast.FunctionDef, BlockType),
)
and _confirmThisStmtIsNotWithinNestedFunc(
foundStatementTemp=True,
familyTree=familyTree,
lineNumOfStatement=childLineNum,
lineNumOfThisNode=node.lineno,
)
):
for subnode, _ in walk.walk_dfs(child):
if isinstance(subnode, ast.Name):
yield subnode.id
break
else:
# if "raise" statement was alone, it must be inside an "except"
if currentParentExceptHandler:
yield from _extractExceptionsFromExcept(
currentParentExceptHandler,
)


def _extractExceptionsFromExcept(
node: ast.ExceptHandler,
) -> Generator[str, None, None]:
if isinstance(node.type, ast.Name):
yield node.type.id

if isinstance(node.type, ast.Tuple):
for child, _ in walk.walk(node.type):
if isinstance(child, ast.Name):
yield child.id


def _hasExpectedStatements(
node: FuncOrAsyncFuncDef,
isThisNodeAnExpectedStmt: Callable[[ast.AST], bool],
Expand Down
1 change: 1 addition & 0 deletions pydoclint/utils/violation.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@

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',
503: 'exceptions in the "Raises" section in the docstring do not match those in the function body',

601: 'Class docstring contains fewer class attributes than actual class attributes.',
602: 'Class docstring contains more class attributes than in actual class attributes.',
Expand Down
27 changes: 27 additions & 0 deletions pydoclint/utils/visitor_helper.py
Original file line number Diff line number Diff line change
Expand Up @@ -521,3 +521,30 @@ def extractReturnTypeFromGenerator(returnAnnoText: str) -> str:
returnType = returnAnnoText

return stripQuotes(returnType)


def addMismatchedRaisesExceptionViolation(
*,
docRaises: List[str],
actualRaises: List[str],
violations: List[Violation],
violationForRaisesMismatch: Violation, # such as V503
lineNum: int,
msgPrefix: str,
) -> None:
"""
Add a violation for mismatched exception type between function
body and docstring
"""
msgPostfix: str = (
f'Raises values in the docstring: {docRaises}.'
f' Raised exceptions in the body: {actualRaises}.'
)
violations.append(
Violation(
code=violationForRaisesMismatch.code,
line=lineNum,
msgPrefix=msgPrefix,
msgPostfix=msgPostfix,
)
)
7 changes: 7 additions & 0 deletions pydoclint/utils/walk.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,13 @@ def walk(node):
yield node, parent


def walk_dfs(node):
"""Depth-first traversal of AST. Modified from `walk()` in this file"""
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: I changed the wording "above" because there's no guarantee that the relative positions of these 2 functions won't be changed in the future.

for child, parent in iter_child_nodes(node):
yield child, parent
yield from walk_dfs(child)


def iter_child_nodes(node):
"""
Yield all direct child nodes of *node*, that is, all fields that are nodes
Expand Down
30 changes: 30 additions & 0 deletions pydoclint/visitor.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
from pydoclint.utils.return_anno import ReturnAnnotation
from pydoclint.utils.return_arg import ReturnArg
from pydoclint.utils.return_yield_raise import (
getRaisedExceptions,
hasGeneratorAsReturnAnnotation,
hasIteratorOrIterableAsReturnAnnotation,
hasRaiseStatements,
Expand All @@ -32,6 +33,7 @@
)
from pydoclint.utils.violation import Violation
from pydoclint.utils.visitor_helper import (
addMismatchedRaisesExceptionViolation,
checkClassAttributesAgainstClassDocstring,
checkDocArgsLengthAgainstActualArgs,
checkNameOrderAndTypeHintsOfDocArgsAgainstActualArgs,
Expand Down Expand Up @@ -811,6 +813,7 @@ def checkRaises(

v501 = Violation(code=501, line=lineNum, msgPrefix=msgPrefix)
v502 = Violation(code=502, line=lineNum, msgPrefix=msgPrefix)
v503 = Violation(code=503, line=lineNum, msgPrefix=msgPrefix)

docstringHasRaisesSection: bool = doc.hasRaisesSection
hasRaiseStmt: bool = hasRaiseStatements(node)
Expand All @@ -822,4 +825,31 @@ def checkRaises(
if not self.isAbstractMethod:
violations.append(v502)

# check that the raise statements match those in body.
if hasRaiseStmt:
docRaises: List[str] = []

for raises in doc.parsed.raises:
if raises.type_name:
docRaises.append(raises.type_name)
elif doc.style == 'sphinx' and raises.description:
# :raises: Exception: -> 'Exception'
splitDesc = raises.description.split(':')
if len(splitDesc) > 1 and ' ' not in splitDesc[0].strip():
exc = splitDesc[0].strip()
docRaises.append(exc)

docRaises.sort()
actualRaises = getRaisedExceptions(node)

if docRaises != actualRaises:
addMismatchedRaisesExceptionViolation(
docRaises=docRaises,
actualRaises=actualRaises,
violations=violations,
violationForRaisesMismatch=v503,
lineNum=lineNum,
msgPrefix=msgPrefix,
)

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.5.6
version = 0.5.7
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/raises/cases.py
Original file line number Diff line number Diff line change
Expand Up @@ -143,3 +143,42 @@ def inner9a(arg1) -> None:
raise

print(arg0)

def func11(self, arg0) -> None:
"""
This docstring doesn't specify all the raised exceptions.

Args:
arg0: Arg 0

Raises:
TypeError: if arg0 is 0.
"""
if arg0 == 0:
raise TypeError
raise ValueError

def func12(self, arg0) -> None:
"""
There should not be any violations in this method.

Args:
arg0: Arg 0

Raises:
TypeError: if arg0 is 0.
ValueError: otherwise.
"""
if arg0 == 0:
raise TypeError
raise ValueError

def func13(self) -> None:
"""
Should raise an error due to duplicated raises.

Raises:
ValueError: all the time.
ValueError: typo!
"""
raise ValueError
51 changes: 51 additions & 0 deletions tests/data/numpy/raises/cases.py
Original file line number Diff line number Diff line change
Expand Up @@ -178,3 +178,54 @@ def inner9a(arg1) -> None:
raise

print(arg0)

def func11(self, arg0) -> None:
"""
This docstring doesn't specify all the raised exceptions.

Parameters
----------
arg0
Arg 0

Raises
------
TypeError
if arg0 is 0.
"""
if arg0 == 0:
raise TypeError
raise ValueError

def func12(self, arg0) -> None:
"""
There should not be any violations in this method.

Parameters
----------
arg0
Arg 0

Raises
------
TypeError
if arg0 is 0.
ValueError
otherwise.
"""
if arg0 == 0:
raise TypeError
raise ValueError

def func13(self) -> None:
"""
Should raise an error due to duplicated raises.

Raises
------
ValueError
all the time.
ValueError
typo!
"""
raise ValueError
32 changes: 32 additions & 0 deletions tests/data/sphinx/raises/cases.py
Original file line number Diff line number Diff line change
Expand Up @@ -121,3 +121,35 @@ def inner9a(arg1) -> None:
raise

print(arg0)

def func11(self, arg0) -> None:
"""
This docstring doesn't specify all the raised exceptions.

:param arg0: Arg 0
:raises TypeError: if arg0 is zero.
"""
if arg0 == 0:
raise TypeError
raise ValueError

def func12(self, arg0) -> None:
"""
There should not be any violations in this method.

:param arg0: Arg 0
:raises TypeError: if arg0 is zero.
:raises ValueError: otherwise.
"""
if arg0 == 0:
raise TypeError
raise ValueError

def func13(self) -> None:
"""
Should raise an error due to duplicated raises.

:raises ValueError: all the time.
:raises ValueError: typo!
"""
raise ValueError
23 changes: 23 additions & 0 deletions tests/test_main.py
Original file line number Diff line number Diff line change
Expand Up @@ -609,6 +609,9 @@ def testAllowInitDocstring(style: str) -> None:
'because __init__() cannot return anything',
'DOC305: Class `C`: Class docstring has a "Raises" section; please put it in '
'the __init__() docstring',
'DOC503: Method `C.__init__` exceptions in the "Raises" section in the '
'docstring do not match those in the function body Raises values in the '
"docstring: ['TypeError']. Raised exceptions in the body: ['ValueError'].",
'DOC306: Class `D`: The class docstring does not need a "Yields" section, '
'because __init__() cannot yield anything',
'DOC307: Class `D`: The __init__() docstring does not need a "Yields" '
Expand Down Expand Up @@ -804,6 +807,12 @@ def testRaises(style: str, skipRaisesCheck: bool) -> None:
expected0 = [
'DOC501: Method `B.func1` has "raise" statements, but the docstring does not '
'have a "Raises" section',
'DOC503: Method `B.func1` exceptions in the "Raises" section in the docstring '
'do not match those in the function body Raises values in the docstring: []. '
"Raised exceptions in the body: ['ValueError'].",
'DOC503: Method `B.func4` exceptions in the "Raises" section in the docstring '
'do not match those in the function body Raises values in the docstring: '
"['CurtomError']. Raised exceptions in the body: ['CustomError'].",
'DOC502: Method `B.func5` has a "Raises" section in the docstring, but there '
'are not "raise" statements in the body',
'DOC502: Method `B.func7` has a "Raises" section in the docstring, but there '
Expand All @@ -812,6 +821,17 @@ def testRaises(style: str, skipRaisesCheck: bool) -> None:
'are not "raise" statements in the body',
'DOC501: Function `inner9a` has "raise" statements, but the docstring does '
'not have a "Raises" section',
'DOC503: Function `inner9a` exceptions in the "Raises" section in the '
'docstring do not match those in the function body Raises values in the '
"docstring: []. Raised exceptions in the body: ['FileNotFoundError'].",
'DOC503: Method `B.func11` exceptions in the "Raises" section in the '
'docstring do not match those in the function body Raises values in the '
"docstring: ['TypeError']. Raised exceptions in the body: ['TypeError', "
"'ValueError'].",
'DOC503: Method `B.func13` exceptions in the "Raises" section in the '
'docstring do not match those in the function body Raises values in the '
"docstring: ['ValueError', 'ValueError']. Raised exceptions in the body: "
"['ValueError'].",
]
expected1 = []
expected = expected1 if skipRaisesCheck else expected0
Expand Down Expand Up @@ -841,6 +861,9 @@ def testRaisesPy310plus(style: str, skipRaisesCheck: bool) -> None:
expected0 = [
'DOC501: Method `B.func10` has "raise" statements, but the docstring does not '
'have a "Raises" section',
'DOC503: Method `B.func10` exceptions in the "Raises" section in the '
'docstring do not match those in the function body Raises values in the '
"docstring: []. Raised exceptions in the body: ['ValueError'].",
]
expected1 = []
expected = expected1 if skipRaisesCheck else expected0
Expand Down
Loading
Loading