Skip to content

Commit

Permalink
feat: introduce DOC503 for checking specific raised exceptions (#161)
Browse files Browse the repository at this point in the history
Co-authored-by: jsh9 <25124332+jsh9@users.noreply.github.com>
  • Loading branch information
Amar1729 and jsh9 authored Sep 3, 2024
1 parent ae28589 commit f758604
Show file tree
Hide file tree
Showing 14 changed files with 452 additions and 5 deletions.
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"""
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

0 comments on commit f758604

Please sign in to comment.