Skip to content

Commit

Permalink
fix: DOC503 catch namespaced exceptions
Browse files Browse the repository at this point in the history
Previously, AST walking was not properly identifying the name of
exceptions like `m.Exception`. Update logic for determining names from
an AST to correctly get the whole name (as well as related tests).
  • Loading branch information
Amar1729 committed Sep 20, 2024
1 parent f758604 commit b044ece
Show file tree
Hide file tree
Showing 6 changed files with 152 additions and 7 deletions.
44 changes: 39 additions & 5 deletions pydoclint/utils/return_yield_raise.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,14 @@
import ast
from typing import Callable, Dict, Generator, List, Optional, Tuple, Type
from typing import (
Callable,
Dict,
Generator,
List,
Optional,
Tuple,
Type,
Union,
)

from pydoclint.utils import walk
from pydoclint.utils.annotation import unparseAnnotation
Expand Down Expand Up @@ -98,6 +107,14 @@ def getRaisedExceptions(node: FuncOrAsyncFuncDef) -> List[str]:
return sorted(set(_getRaisedExceptions(node)))


def _getFullAttribute(node: Union[ast.Attribute, ast.Name]) -> str:
"""Get the full name of a symbol like a.b.c.foo"""
if isinstance(node, ast.Name):
return node.id

return _getFullAttribute(node.value) + '.' + node.attr


def _getRaisedExceptions(
node: FuncOrAsyncFuncDef,
) -> Generator[str, None, None]:
Expand Down Expand Up @@ -132,7 +149,17 @@ def _getRaisedExceptions(
):
for subnode, _ in walk.walk_dfs(child):
if isinstance(subnode, ast.Name):
yield subnode.id
if isinstance(child.exc, ast.Attribute):
# case: looks like m.n.exception
yield _getFullAttribute(child.exc)
elif isinstance(child.exc, ast.Call) and isinstance(
child.exc.func, ast.Attribute
):
# case: looks like m.n.exception()
yield _getFullAttribute(child.exc.func)
else:
yield subnode.id

break
else:
# if "raise" statement was alone, it must be inside an "except"
Expand All @@ -148,10 +175,17 @@ def _extractExceptionsFromExcept(
if isinstance(node.type, ast.Name):
yield node.type.id

if isinstance(node.type, ast.Attribute):
# case: looks like m.n.exception
yield _getFullAttribute(node.type)

if isinstance(node.type, ast.Tuple):
for child, _ in walk.walk(node.type):
if isinstance(child, ast.Name):
yield child.id
for elt in node.type.elts:
if isinstance(elt, ast.Attribute):
# case: looks like m.n.exception
yield _getFullAttribute(elt)
elif isinstance(elt, ast.Name):
yield elt.id


def _hasExpectedStatements(
Expand Down
23 changes: 23 additions & 0 deletions tests/data/google/raises/cases.py
Original file line number Diff line number Diff line change
Expand Up @@ -182,3 +182,26 @@ def func13(self) -> None:
ValueError: typo!
"""
raise ValueError

def func14(self) -> None:
"""
Should fail, expects `exceptions.CustomError`.
Raises:
CustomError: every time.
"""
exceptions = object()
exceptions.CustomError = CustomError
raise exceptions.CustomError()

def func15(self) -> None:
"""
Should fail, expects `exceptions.m.CustomError`.
Raises:
CustomError: every time.
"""
exceptions = object()
exceptions.m = object()
exceptions.m.CustomError = CustomError
raise exceptions.m.CustomError
27 changes: 27 additions & 0 deletions tests/data/numpy/raises/cases.py
Original file line number Diff line number Diff line change
Expand Up @@ -229,3 +229,30 @@ def func13(self) -> None:
typo!
"""
raise ValueError

def func14(self) -> None:
"""
Should fail, expects `exceptions.CustomError`.
Raises
------
CustomError
every time.
"""
exceptions = object()
exceptions.CustomError = CustomError
raise exceptions.CustomError()

def func15(self) -> None:
"""
Should fail, expects `exceptions.m.CustomError`.
Raises
------
CustomError
every time.
"""
exceptions = object()
exceptions.m = object()
exceptions.m.CustomError = CustomError
raise exceptions.m.CustomError
21 changes: 21 additions & 0 deletions tests/data/sphinx/raises/cases.py
Original file line number Diff line number Diff line change
Expand Up @@ -153,3 +153,24 @@ def func13(self) -> None:
:raises ValueError: typo!
"""
raise ValueError

def func14(self) -> None:
"""
Should fail, expects `exceptions.CustomError`.
:raises CustomError: every time.
"""
exceptions = object()
exceptions.CustomError = CustomError
raise exceptions.CustomError()

def func15(self) -> None:
"""
Should fail, expects `exceptions.m.CustomError`.
:raises CustomError: every time.
"""
exceptions = object()
exceptions.m = object()
exceptions.m.CustomError = CustomError
raise exceptions.m.CustomError
8 changes: 8 additions & 0 deletions tests/test_main.py
Original file line number Diff line number Diff line change
Expand Up @@ -832,6 +832,14 @@ def testRaises(style: str, skipRaisesCheck: bool) -> None:
'docstring do not match those in the function body Raises values in the '
"docstring: ['ValueError', 'ValueError']. Raised exceptions in the body: "
"['ValueError'].",
'DOC503: Method `B.func14` exceptions in the "Raises" section in the '
'docstring do not match those in the function body Raises values in the '
"docstring: ['CustomError']. Raised exceptions in the body: "
"['exceptions.CustomError'].",
'DOC503: Method `B.func15` exceptions in the "Raises" section in the '
'docstring do not match those in the function body Raises values in the '
"docstring: ['CustomError']. Raised exceptions in the body: "
"['exceptions.m.CustomError'].",
]
expected1 = []
expected = expected1 if skipRaisesCheck else expected0
Expand Down
36 changes: 34 additions & 2 deletions tests/utils/test_returns_yields_raise.py
Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,7 @@ def func7(arg0):
def func8(d):
try:
d[0][0]
except (KeyError, TypeError):
except (KeyError, TypeError, m.ValueError):
raise
finally:
pass
Expand Down Expand Up @@ -416,6 +416,28 @@ def func12(a):
if a < 3:
raise Error3
def func13(a):
# ensure we get `Exception` and `Exception()`
if a < 1:
raise ValueError
else:
raise TypeError()
def func14(a):
# check that we properly identify submodule exceptions.
if a < 1:
raise m.ValueError
elif a < 2:
raise m.n.ValueError()
else:
raise a.b.c.ValueError(msg="some msg")
def func15():
try:
x = 1
except other.Exception:
raise
"""


Expand All @@ -439,6 +461,9 @@ def testHasRaiseStatements() -> None:
(75, 0, 'func10'): True,
(83, 0, 'func11'): True,
(100, 0, 'func12'): True,
(117, 0, 'func13'): True,
(124, 0, 'func14'): True,
(133, 0, 'func15'): True,
}

assert result == expected
Expand All @@ -464,11 +489,18 @@ def testWhichRaiseStatements() -> None:
'RuntimeError',
'TypeError',
],
(54, 0, 'func8'): ['KeyError', 'TypeError'],
(54, 0, 'func8'): ['KeyError', 'TypeError', 'm.ValueError'],
(62, 0, 'func9'): ['AssertionError', 'IndexError'],
(75, 0, 'func10'): ['GError'],
(83, 0, 'func11'): ['ValueError'],
(100, 0, 'func12'): ['Error1', 'Error2', 'Error3'],
(117, 0, 'func13'): ['TypeError', 'ValueError'],
(124, 0, 'func14'): [
'a.b.c.ValueError',
'm.ValueError',
'm.n.ValueError',
],
(133, 0, 'func15'): ['other.Exception'],
}

assert result == expected

0 comments on commit b044ece

Please sign in to comment.