🐛 False positive DOC503 when Exception is created via class method

[s-weigand]

When creating an Exeption from a class constructor method a DOC503 is raised saying <class name> does not match <class name>.<method name>.

Example

def ensure_unique_tuple(value: tuple[int, ...]):
    """Ensure that all values in the tuple are unique.

    Parameters
    ----------
    value : tuple[int,...]
        Tuple to check

    Raises
    ------
    TupleDuplicateException
        If the tuple contains duplicate values.
    """
    if len(value) != len(set(value)):
        raise TupleDuplicateException.from_tuple(value)

will result in

DOC503: Function `ensure_unique_tuple` exceptions in the "Raises" section in the docstring do not
match those in the function body Raises values in the docstring: ['TupleDuplicateException'].
Raised exceptions in the body: ['TupleDuplicateException.from_tuple'].

Reproduction

I was preparing a ninja PR to fix this bug (see full reproducible example on my fork).

However, the problem seems to me that at the point where the visitor comes around there isn't enough information to determine is it of style module.Exception() or Exception.method() (but might just be my AST skill issues 😅).

Here is the AST representation of raise TupleDuplicateException.from_tuple(value) (thanks astpretty to make it more readable 😅)

Raise(
    lineno=45,
    col_offset=8,
    end_lineno=45,
    end_col_offset=55,
    exc=Call(
        lineno=45,
        col_offset=14,
        end_lineno=45,
        end_col_offset=55,
        func=Attribute(
            lineno=45,
            col_offset=14,
            end_lineno=45,
            end_col_offset=48,
            value=Name(lineno=45, col_offset=14, end_lineno=45, end_col_offset=37, id='TupleDuplicateException', ctx=Load()),
            attr='from_tuple',
            ctx=Load(),
        ),
        args=[Name(lineno=45, col_offset=49, end_lineno=45, end_col_offset=54, id='value', ctx=Load())],
        keywords=[],
    ),
    cause=None,
),

Possible solutions

The only thing that comes to my mind currently would be a remapping configuration option to run a string replace the output of getRaisedExceptions (creating an exception from a class method is pretty much an edge case, my actual use case is ValidationError.from_exception_data to concatenate validations).

Env

• pydoclint==0.5.15
• py310

[s-weigand]

As a side note, using a pattern like

error = TupleDuplicateException.from_tuple(value)
raise error 

Also does not work

[jsh9]

Ugh, this is like a whack-a-mole. Thanks for discovering this edge case.

In #168, @Amar1729 added a fix to correctly extracting exception names from things like a.b.c.d.MyException(msg='my message').

But now your use case is the reverse problem.

I traced the code down to this function:

pydoclint/pydoclint/utils/generic.py

Lines 250 to 255 in 4021d8d

	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
	return getFullAttributeName(node.value) + '.' + node.attr # type:ignore[arg-type]

• If the node is a.b.c.d.MyException
  • node.value is a.b.c.d
  • node.attr is MyException
• If the node is TupleDuplicateException.from_tuple
  • node.value is TupleDpulicateException
  • node.attr is from_tuple

I haven't been able to find a way effectively distinguish the two. I wonder whether @Amar1729 has some ideas.

A potential solution is to attempt a partial string match: as long as the docstring exception name is a substring of node's string representation (or maybe a more strict criterion), it's considered a match. What do you think about this?

[jsh9]

My fix is to perform partial string matching instead of strict string matching. See the code here:

pydoclint/pydoclint/utils/generic.py

Lines 250 to 268 in 91263b8

	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
	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

This way, if you have a.b.c.d.e.MyException.f.g.h.i.j.k('something') in your function body, and you have a.b.c.d.e.MyException in your docstring, the check passes without any violations. But if you only have MyException in your docstring, pydoclint still shows you a violation --- this is a deliberate design choice which @Amar1729 established. I'm may be open to discussion on this, if there are compelling reasons to further relax this.

[Amar1729]

huh, I'm not sure that I've ever seen that pattern for exceptions before! @jsh9 I think you're right that the only way to distinguish between these two is a substring check like you introduced. I would think the AST alone wouldn't be enough and there would have to be actual name resolution introduced, which seems out of scope.

[jsh9]

The fix is published in version 0.5.18.