diff --git a/pydoclint/utils/return_yield_raise.py b/pydoclint/utils/return_yield_raise.py index 3108fde..020f97a 100644 --- a/pydoclint/utils/return_yield_raise.py +++ b/pydoclint/utils/return_yield_raise.py @@ -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 @@ -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], diff --git a/pydoclint/utils/violation.py b/pydoclint/utils/violation.py index 4a987b0..4dab839 100644 --- a/pydoclint/utils/violation.py +++ b/pydoclint/utils/violation.py @@ -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.', diff --git a/pydoclint/utils/visitor_helper.py b/pydoclint/utils/visitor_helper.py index 5721bdc..f41ec2b 100644 --- a/pydoclint/utils/visitor_helper.py +++ b/pydoclint/utils/visitor_helper.py @@ -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, + ) + ) diff --git a/pydoclint/visitor.py b/pydoclint/visitor.py index f501551..ea2e55f 100644 --- a/pydoclint/visitor.py +++ b/pydoclint/visitor.py @@ -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, @@ -39,6 +40,7 @@ checkYieldTypesForViolations, extractReturnTypeFromGenerator, extractYieldTypeFromGeneratorOrIteratorAnnotation, + messageForMismatchedRaisedExceptions, ) from pydoclint.utils.yield_arg import YieldArg @@ -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) @@ -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 diff --git a/tests/data/google/raises/cases.py b/tests/data/google/raises/cases.py index 21b0565..acc898f 100644 --- a/tests/data/google/raises/cases.py +++ b/tests/data/google/raises/cases.py @@ -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 diff --git a/tests/data/numpy/raises/cases.py b/tests/data/numpy/raises/cases.py index 5982582..2e7db3f 100644 --- a/tests/data/numpy/raises/cases.py +++ b/tests/data/numpy/raises/cases.py @@ -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 diff --git a/tests/data/sphinx/raises/cases.py b/tests/data/sphinx/raises/cases.py index 2070682..d4368fb 100644 --- a/tests/data/sphinx/raises/cases.py +++ b/tests/data/sphinx/raises/cases.py @@ -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 diff --git a/tests/test_main.py b/tests/test_main.py index 939b7dd..26ce3da 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -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" ' @@ -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 ' @@ -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 @@ -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 diff --git a/tests/utils/test_returns_yields_raise.py b/tests/utils/test_returns_yields_raise.py index d9a0168..7387d2e 100644 --- a/tests/utils/test_returns_yields_raise.py +++ b/tests/utils/test_returns_yields_raise.py @@ -1,11 +1,12 @@ import ast -from typing import Dict, Tuple +from typing import Dict, List, Tuple import pytest from pydoclint.utils.astTypes import FuncOrAsyncFuncDef from pydoclint.utils.generic import getFunctionId from pydoclint.utils.return_yield_raise import ( + getRaisedExceptions, hasGeneratorAsReturnAnnotation, hasRaiseStatements, hasReturnAnnotation, @@ -118,6 +119,7 @@ def __init__(self): self.returnStatements: Dict[Tuple[int, int, str], bool] = {} self.yieldStatements: Dict[Tuple[int, int, str], bool] = {} self.raiseStatements: Dict[Tuple[int, int, str], bool] = {} + self.raisedExceptions: Dict[Tuple[int, int, str], List[str]] = {} self.returnAnnotations: Dict[Tuple[int, int, str], bool] = {} self.generatorAnnotations: Dict[Tuple[int, int, str], bool] = {} @@ -126,6 +128,7 @@ def visit_FunctionDef(self, node: FuncOrAsyncFuncDef): self.returnStatements[functionId] = hasReturnStatements(node) self.yieldStatements[functionId] = hasYieldStatements(node) self.raiseStatements[functionId] = hasRaiseStatements(node) + self.raisedExceptions[functionId] = getRaisedExceptions(node) self.returnAnnotations[functionId] = hasReturnAnnotation(node) self.generatorAnnotations[functionId] = hasGeneratorAsReturnAnnotation( node, @@ -328,6 +331,20 @@ def func6(arg1): raise TypeError return arg1 + 2 + +def func7(arg0): + if True: + raise TypeError + + try: + "foo"[-10] + except IndexError as e: + raise + + try: + 1 / 0 + except ZeroDivisionError: + raise RuntimeError("a different error") """ @@ -345,6 +362,27 @@ def testHasRaiseStatements() -> None: (20, 0, 'func5'): False, (26, 0, 'func6'): True, (21, 4, 'func5_child1'): True, + (32, 0, 'func7'): True, + } + + assert result == expected + + +def testWhichRaiseStatements() -> None: + tree = ast.parse(srcRaises) + visitor = HelperVisitor() + visitor.visit(tree) + result = visitor.raisedExceptions + + expected = { + (2, 0, 'func1'): ['ValueError'], + (7, 0, 'func2'): ['Exception'], + (10, 0, 'func3'): ['TypeError'], + (17, 0, 'func4'): ['CustomError'], + (20, 0, 'func5'): [], + (26, 0, 'func6'): ['TypeError'], + (21, 4, 'func5_child1'): ['ValueError'], + (32, 0, 'func7'): ['IndexError', 'RuntimeError', 'TypeError'], } assert result == expected