Skip to content

Commit

Permalink
feat: introduce DOC503 for checking specific raised exceptions
Browse files Browse the repository at this point in the history
This commit compares any raise statements in function bodies to
docstring Raises sections and ensures they match. Exceptions specified
in a docstring are kept as a sorted list, so duplicate lines will throw
DOC503 as well.
  • Loading branch information
Amar1729 committed Jul 28, 2024
1 parent 23b89f6 commit 671dd96
Show file tree
Hide file tree
Showing 9 changed files with 283 additions and 2 deletions.
43 changes: 42 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, Tuple, Type

from pydoclint.utils import walk
from pydoclint.utils.annotation import unparseAnnotation
Expand Down Expand Up @@ -93,6 +93,47 @@ 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]] = {}

for child, parent in walk.walk(node):
childLineNum = _updateFamilyTree(child, parent, familyTree)

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(child):
if isinstance(subnode, ast.Name):
yield subnode.id
break
else:
# if "raise" statement was alone, generally parent is ast.ExceptHandler.
for subnode, _ in walk.walk(parent):
if isinstance(subnode, ast.Name):
yield subnode.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 "raises" section do not match those in 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 messageForMismatchedRaisedExceptions(
*,
docRaises: List[str],
actualRaises: List[str],
violations: List[Violation],
violationForRaisesMismatch: Violation, # such as V503
lineNum: int,
msgPrefix: str,
) -> None:
"""Format a violation message for mismatched raised exceptions between body and docstring"""
msgPostfixTemp: str = ' '.join([
'Raises values in the docstring:'
f' {docRaises}.',
'Raised exceptions in the body:'
f' {actualRaises}.',
])

violations.append(
Violation(
code=503,
line=lineNum,
msgPrefix=msgPrefix,
msgPostfix=msgPostfixTemp,
)
)
31 changes: 31 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 @@ -39,6 +40,7 @@
checkYieldTypesForViolations,
extractReturnTypeFromGenerator,
extractYieldTypeFromGeneratorOrIteratorAnnotation,
messageForMismatchedRaisedExceptions,
)
from pydoclint.utils.yield_arg import YieldArg

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=502, line=lineNum, msgPrefix=msgPrefix)

docstringHasRaisesSection: bool = doc.hasRaisesSection
hasRaiseStmt: bool = hasRaiseStatements(node)
Expand All @@ -822,4 +825,32 @@ 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:
messageForMismatchedRaisedExceptions(
docRaises=docRaises,
actualRaises=actualRaises,
violations=violations,
violationForRaisesMismatch=v503,
lineNum=lineNum,
msgPrefix=msgPrefix,
)

return violations
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
21 changes: 21 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 "raises" section do not match '
"those in 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 "raises" section do not match those '
'in body. Raises values in the docstring: []. Raised exceptions in '
"the body: ['ValueError'].",
'DOC503: Method `B.func4` exceptions in "raises" section do not match those '
"in 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,15 @@ 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 "raises" section do not match those '
'in body. Raises values in the docstring: []. Raised exceptions in '
"the body: ['FileNotFoundError'].",
'DOC503: Method `B.func11` exceptions in "raises" section do not match those '
"in body. Raises values in the docstring: ['TypeError']. Raised exceptions in "
"the body: ['TypeError', 'ValueError'].",
'DOC503: Method `B.func13` exceptions in "raises" section do not match those '
"in 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 +859,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 "raises" section do not match those '
'in body. Raises values in the docstring: []. Raised exceptions in '
"the body: ['ValueError'].",
]
expected1 = []
expected = expected1 if skipRaisesCheck else expected0
Expand Down
Loading

0 comments on commit 671dd96

Please sign in to comment.