Enable missing-raises-doc to understand class hierarchies (#5278)

Before, missing-raises-doc could not understand class hierarchies but just compared names.
So, when a method documented a raise of a parent class, but a child exception was raised, the check failed.

With this change, if the name compare doesn't help, the exception class hierarchy is checked.
For that, possible_exc_types was changed to return classes instead of names

Resolved #4955

Co-authored-by: Pierre Sassoulas <[email protected]>

Co-authored-by: Daniël van Noord <[email protected]>

Author: kasium

ChangeLog

@@ -87,6 +87,20 @@ Release date: TBA
 * The ``testutils`` for unittests now accept ``end_lineno`` and ``end_column``. Tests
   without these will trigger a ``DeprecationWarning``.
 
+* ``missing-raises-doc`` will now check the class hierarchy of the raised exceptions
+
+  .. code-block:: python
+
+    def my_function()
+      """My function.
+
+      Raises:
+        Exception: if something fails
+      """
+      raise ValueError
+
+  Closes #4955
+
 ..
   Insert your changelog randomly, it will reduce merge conflicts
   (Ie. not necessarily at the end)

doc/whatsnew/2.13.rst

@@ -80,3 +80,17 @@ Other Changes
 
 * The ``testutils`` for unittests now accept ``end_lineno`` and ``end_column``. Tests
   without these will trigger a ``DeprecationWarning``.
+
+* ``missing-raises-doc`` will now check the class hierarchy of the raised exceptions
+
+  .. code-block:: python
+
+    def my_function()
+      """My function.
+
+      Raises:
+        Exception: if something fails
+      """
+      raise ValueError
+
+  Closes #4955

pylint/extensions/_check_docs_utils.py

@@ -120,7 +120,7 @@ def _split_multiple_exc_types(target: str) -> List[str]:
     return re.split(delimiters, target)
 
 
-def possible_exc_types(node):
+def possible_exc_types(node: nodes.NodeNG) -> Set[nodes.ClassDef]:
     """
     Gets all of the possible raised exception types for the given raise node.
 
@@ -130,44 +130,45 @@ def possible_exc_types(node):
 
 
     :param node: The raise node to find exception types for.
-    :type node: nodes.NodeNG
 
     :returns: A list of exception types possibly raised by :param:`node`.
-    :rtype: set(str)
     """
     excs = []
     if isinstance(node.exc, nodes.Name):
         inferred = utils.safe_infer(node.exc)
         if inferred:
-            excs = [inferred.name]
+            excs = [inferred]
     elif node.exc is None:
         handler = node.parent
         while handler and not isinstance(handler, nodes.ExceptHandler):
             handler = handler.parent
 
         if handler and handler.type:
-            inferred_excs = astroid.unpack_infer(handler.type)
-            excs = (exc.name for exc in inferred_excs if exc is not astroid.Uninferable)
+            try:
+                for exc in astroid.unpack_infer(handler.type):
+                    if exc is not astroid.Uninferable:
+                        excs.append(exc)
+            except astroid.InferenceError:
+                pass
     else:
         target = _get_raise_target(node)
         if isinstance(target, nodes.ClassDef):
-            excs = [target.name]
+            excs = [target]
         elif isinstance(target, nodes.FunctionDef):
             for ret in target.nodes_of_class(nodes.Return):
                 if ret.frame() != target:
                     # return from inner function - ignore it
                     continue
 
                 val = utils.safe_infer(ret.value)
-                if (
-                    val
-                    and isinstance(val, (astroid.Instance, nodes.ClassDef))
-                    and utils.inherit_from_std_ex(val)
-                ):
-                    excs.append(val.name)
+                if val and utils.inherit_from_std_ex(val):
+                    if isinstance(val, nodes.ClassDef):
+                        excs.append(val)
+                    elif isinstance(val, astroid.Instance):
+                        excs.append(val.getattr("__class__")[0])
 
     try:
-        return {exc for exc in excs if not utils.node_ignores_exception(node, exc)}
+        return {exc for exc in excs if not utils.node_ignores_exception(node, exc.name)}
     except astroid.InferenceError:
         return set()
 

pylint/extensions/docparams.py

@@ -309,14 +309,25 @@ def visit_raise(self, node: nodes.Raise) -> None:
         doc = utils.docstringify(func_node.doc, self.config.default_docstring_type)
         if not doc.matching_sections():
             if doc.doc:
-                self._handle_no_raise_doc(expected_excs, func_node)
+                missing = {exc.name for exc in expected_excs}
+                self._handle_no_raise_doc(missing, func_node)
             return
 
         found_excs_full_names = doc.exceptions()
 
         # Extract just the class name, e.g. "error" from "re.error"
         found_excs_class_names = {exc.split(".")[-1] for exc in found_excs_full_names}
-        missing_excs = expected_excs - found_excs_class_names
+
+        missing_excs = set()
+        for expected in expected_excs:
+            for found_exc in found_excs_class_names:
+                if found_exc == expected.name:
+                    break
+                if any(found_exc == ancestor.name for ancestor in expected.ancestors()):
+                    break
+            else:
+                missing_excs.add(expected.name)
+
         self._add_raise_message(missing_excs, func_node)
 
     def visit_return(self, node: nodes.Return) -> None:

tests/extensions/test_check_docs_utils.py

@@ -147,5 +147,7 @@ def my_func():
     ],
 )
 def test_exception(raise_node, expected):
-    found = utils.possible_exc_types(raise_node)
-    assert found == expected
+    found_nodes = utils.possible_exc_types(raise_node)
+    for node in found_nodes:
+        assert isinstance(node, astroid.nodes.ClassDef)
+    assert {node.name for node in found_nodes} == expected

tests/functional/ext/docparams/raise/missing_raises_doc_required_exc_inheritance.py

@@ -0,0 +1,45 @@
+"""Tests for missing-raises-doc for exception class inheritance."""
+# pylint: disable=missing-class-docstring
+
+class CustomError(NameError):
+    pass
+
+
+class CustomChildError(CustomError):
+    pass
+
+
+def test_find_missing_raise_for_parent():  # [missing-raises-doc]
+    """This is a docstring.
+
+    Raises:
+        CustomError: Never
+    """
+    raise NameError("hi")
+
+
+def test_no_missing_raise_for_child_builtin():
+    """This is a docstring.
+
+    Raises:
+        Exception: Never
+    """
+    raise ValueError("hi")
+
+
+def test_no_missing_raise_for_child_custom():
+    """This is a docstring.
+
+    Raises:
+        NameError: Never
+    """
+    raise CustomError("hi")
+
+
+def test_no_missing_raise_for_child_custom_nested():
+    """This is a docstring.
+
+    Raises:
+        NameError: Never
+    """
+    raise CustomChildError("hi")

tests/functional/ext/docparams/raise/missing_raises_doc_required_exc_inheritance.rc

@@ -0,0 +1,5 @@
+[MASTER]
+load-plugins = pylint.extensions.docparams
+
+[BASIC]
+accept-no-raise-doc=no

tests/functional/ext/docparams/raise/missing_raises_doc_required_exc_inheritance.txt

@@ -0,0 +1 @@
+missing-raises-doc:12:0:18:25:test_find_missing_raise_for_parent:"""NameError""" not documented as being raised:UNDEFINED