Skip to content

Commit

Permalink
Fix parsing and comparison of chained custom exceptions (#203)
Browse files Browse the repository at this point in the history
  • Loading branch information
jsh9 authored Jan 12, 2025
1 parent 64ef923 commit 91263b8
Show file tree
Hide file tree
Showing 11 changed files with 181 additions and 16 deletions.
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,10 +1,18 @@
# Change Log

## [Unpublished] - 2025-01-12

- Fixed
- An issue where custom exceptions such as `a.b.c.MyException.from_str`
cannot be properly parsed and compared

## [0.5.17] - 2025-01-12

- Added
- A new config option `--auto-regenerate-baseline` to automatically
regenerate the baseline file for every successful _pydoclint_ run
- Full diff
- https://github.com/jsh9/pydoclint/compare/0.5.16...0.5.17

## [0.5.16] - 2025-01-11

Expand Down
23 changes: 18 additions & 5 deletions pydoclint/utils/generic.py
Original file line number Diff line number Diff line change
Expand Up @@ -247,9 +247,22 @@ def specialEqual(str1: str, str2: str) -> bool:
return True


def getFullAttributeName(node: 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
def doList1ItemsStartWithList2Items(
list1: list[str],
list2: list[str],
) -> bool:
"""
Check whether all the elements in list1 start with the corresponding
element in list2.
"""
if len(list1) != len(list2):
return False

return getFullAttributeName(node.value) + '.' + node.attr # type:ignore[arg-type]
if list1 == list2: # short-circuit, maybe faster than explicit for loop
return True

for elem1, elem2 in zip(list1, list2):
if not elem1.startswith(elem2):
return False

return True
22 changes: 17 additions & 5 deletions pydoclint/utils/return_yield_raise.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

from pydoclint.utils import walk
from pydoclint.utils.astTypes import BlockType, FuncOrAsyncFuncDef
from pydoclint.utils.generic import getFullAttributeName, stringStartsWith
from pydoclint.utils.generic import stringStartsWith
from pydoclint.utils.unparser_custom import unparseName

ReturnType = Type[ast.Return]
Expand Down Expand Up @@ -111,6 +111,8 @@ def _getRaisedExceptions(

currentParentExceptHandler: ast.ExceptHandler | None = None

exceptionName: str | None

# Depth-first guarantees the last-seen exception handler
# is a parent of child.
for child, parent in walk.walk_dfs(node):
Expand All @@ -136,12 +138,16 @@ def _getRaisedExceptions(
if isinstance(subnode, ast.Name):
if isinstance(child.exc, ast.Attribute):
# case: looks like m.n.exception
yield getFullAttributeName(child.exc)
exceptionName = unparseName(child.exc)
assert isinstance(exceptionName, str) # make mypy happy
yield exceptionName
elif isinstance(child.exc, ast.Call) and isinstance(
child.exc.func, ast.Attribute
):
# case: looks like m.n.exception()
yield getFullAttributeName(child.exc.func)
exceptionName = unparseName(child.exc.func)
assert isinstance(exceptionName, str) # make mypy happy
yield exceptionName
elif (
currentParentExceptHandler
and currentParentExceptHandler.name
Expand Down Expand Up @@ -175,15 +181,21 @@ def _extractExceptionsFromExcept(
if isinstance(node.type, ast.Name):
yield node.type.id

exceptionName: str | None

if isinstance(node.type, ast.Attribute):
# case: looks like m.n.exception
yield getFullAttributeName(node.type)
exceptionName = unparseName(node.type)
assert isinstance(exceptionName, str) # to make mypy happy
yield exceptionName

if isinstance(node.type, ast.Tuple):
for elt in node.type.elts:
if isinstance(elt, ast.Attribute):
# case: looks like m.n.exception
yield getFullAttributeName(elt)
exceptionName = unparseName(elt)
assert isinstance(exceptionName, str) # to make mypy happy
yield exceptionName
elif isinstance(elt, ast.Name):
yield elt.id

Expand Down
12 changes: 9 additions & 3 deletions pydoclint/visitor.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
from pydoclint.utils.generic import (
collectFuncArgs,
detectMethodType,
doList1ItemsStartWithList2Items,
generateClassMsgPrefix,
generateFuncMsgPrefix,
getDocstring,
Expand Down Expand Up @@ -858,9 +859,14 @@ def checkRaises(
docRaises.append(exc)

docRaises.sort()
actualRaises = getRaisedExceptions(node)

if docRaises != actualRaises:
actualRaises: list[str] = getRaisedExceptions(node)

if not doList1ItemsStartWithList2Items(actualRaises, docRaises):
# We only do partial string comparison because there are
# cases like `raise a.b.c.MyException.e.f(1, 2)`, where the
# expected docstring exception is `a.b.c.MyException`, but
# there isn't an effective way to cleanly remove `e.f` at the
# end solely based on AST manipulation.
addMismatchedRaisesExceptionViolation(
docRaises=docRaises,
actualRaises=actualRaises,
Expand Down
21 changes: 21 additions & 0 deletions tests/data/google/raises/cases.py
Original file line number Diff line number Diff line change
Expand Up @@ -205,3 +205,24 @@ def func15(self) -> None:
exceptions.m = object()
exceptions.m.CustomError = CustomError
raise exceptions.m.CustomError

def func16(self) -> None:
"""
It should pass.
Raises:
MyException: if a < 1
YourException: if a < 2
a.b.c.TheirException: if a < 3
a.b.c.d.e.f.g.WhoseException: if a < 4
"""
if a < 1:
raise MyException.a.b.c(('a', 'b'))
elif a < 2:
raise YourException.a.b.c(1)
elif a < 3:
raise a.b.c.TheirException.from_str.d.e('my_str')
elif a < 4:
raise a.b.c.d.e.f.g.WhoseException.h.i.j.k
else:
pass
26 changes: 26 additions & 0 deletions tests/data/numpy/raises/cases.py
Original file line number Diff line number Diff line change
Expand Up @@ -256,3 +256,29 @@ def func15(self) -> None:
exceptions.m = object()
exceptions.m.CustomError = CustomError
raise exceptions.m.CustomError

def func16(self) -> None:
"""
It should pass.
Raises
------
MyException
if a < 1
YourException
if a < 2
a.b.c.TheirException
if a < 3
a.b.c.d.e.f.g.WhoseException
if a < 4
"""
if a < 1:
raise MyException.a.b.c(('a', 'b'))
elif a < 2:
raise YourException.a.b.c(1)
elif a < 3:
raise a.b.c.TheirException.from_str.d.e('my_str')
elif a < 4:
raise a.b.c.d.e.f.g.WhoseException.h.i.j.k
else:
pass
2 changes: 1 addition & 1 deletion tests/data/numpy/raises/py310+/cases.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# This can be merged into the parent folder after Py39 support is dropped.
# Because Python 3.9 does not suport match-case syntax.
# Because Python 3.9 does not support match-case syntax.


class B:
Expand Down
20 changes: 20 additions & 0 deletions tests/data/sphinx/raises/cases.py
Original file line number Diff line number Diff line change
Expand Up @@ -174,3 +174,23 @@ def func15(self) -> None:
exceptions.m = object()
exceptions.m.CustomError = CustomError
raise exceptions.m.CustomError

def func16(self) -> None:
"""
It should pass.
:raises MyException: if a < 1
:raises YourException: if a < 2
:raises a.b.c.TheirException: if a < 3
:raises a.b.c.d.e.f.g.WhoseException: if a < 4
"""
if a < 1:
raise MyException.a.b.c(('a', 'b'))
elif a < 2:
raise YourException.a.b.c(1)
elif a < 3:
raise a.b.c.TheirException.from_str.d.e('my_str')
elif a < 4:
raise a.b.c.d.e.f.g.WhoseException.h.i.j.k
else:
pass
2 changes: 1 addition & 1 deletion tests/data/sphinx/raises/py310+/cases.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# This can be merged into the parent folder after Py39 support is dropped.
# Because Python 3.9 does not suport match-case syntax.
# Because Python 3.9 does not support match-case syntax.


class B:
Expand Down
40 changes: 39 additions & 1 deletion tests/test_generic.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
import pytest

from pydoclint.utils.generic import stripQuotes
from pydoclint.utils.generic import (
doList1ItemsStartWithList2Items,
stripQuotes,
)


@pytest.mark.parametrize(
Expand All @@ -26,3 +29,38 @@
def testStripQuotes(inputStr: str, expected: str) -> None:
output = stripQuotes(inputStr)
assert output == expected


@pytest.mark.parametrize(
'list1, list2, expected',
[
([], [], True),
(
['abc', 'def', 'ghi'],
['abc', 'def', 'ghi'],
True,
),
(
['abc', 'def', 'ghi'],
['abc', 'def', 'ghi', 'jkl'],
False,
),
(
['abc123', 'def456', 'ghi789'],
['abc', 'def', 'ghi'],
True,
),
(
['abc', 'def', 'ghi'],
['abc123', 'def456', 'ghi789'],
False,
),
],
)
def testDoList1ItemsStartWithList2Items(
list1: list[str],
list2: list[str],
expected: bool,
) -> None:
output = doList1ItemsStartWithList2Items(list1, list2)
assert output == expected
21 changes: 21 additions & 0 deletions tests/utils/test_returns_yields_raise.py
Original file line number Diff line number Diff line change
Expand Up @@ -449,6 +449,18 @@ def func16():
raise e
except (KeyError, IndexError) as e:
raise e
def func17():
if a < 1:
raise MyException.a.b.c(('a', 'b'))
elif a < 2:
raise YourException.a.b.c(1)
elif a < 3:
raise a.b.c.TheirException.from_str.d.e('my_str')
elif a < 4:
raise a.b.c.d.e.f.g.WhoseException.h.i.j.k
else:
pass
"""


Expand Down Expand Up @@ -476,6 +488,7 @@ def testHasRaiseStatements() -> None:
(126, 0, 'func14'): True,
(135, 0, 'func15'): True,
(141, 0, 'func16'): True,
(150, 0, 'func17'): True,
}

assert result == expected
Expand Down Expand Up @@ -514,6 +527,14 @@ def testWhichRaiseStatements() -> None:
],
(135, 0, 'func15'): ['other.Exception'],
(141, 0, 'func16'): ['IOError', 'IndexError', 'KeyError'],
(150, 0, 'func17'): [
# We are unable to detect and drop the parts after "Exception"
# here, so we will perform partial string matching later.
'MyException.a.b.c',
'YourException.a.b.c',
'a.b.c.TheirException.from_str.d.e',
'a.b.c.d.e.f.g.WhoseException.h.i.j.k',
],
}

assert result == expected

0 comments on commit 91263b8

Please sign in to comment.