From 26c9042825a6549c9f889b2cd17b5e08ba784ab2 Mon Sep 17 00:00:00 2001 From: kasium <15907922+kasium@users.noreply.github.com> Date: Wed, 15 Dec 2021 00:19:44 +0100 Subject: Enable missing-raises-doc to understand class hierarchies (#5278) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Co-authored-by: Daniël van Noord <13665637+DanielNoord@users.noreply.github.com> --- ChangeLog | 14 +++++++ doc/whatsnew/2.13.rst | 14 +++++++ pylint/extensions/_check_docs_utils.py | 29 +++++++------- pylint/extensions/docparams.py | 15 +++++++- tests/extensions/test_check_docs_utils.py | 6 ++- .../missing_raises_doc_required_exc_inheritance.py | 45 ++++++++++++++++++++++ .../missing_raises_doc_required_exc_inheritance.rc | 5 +++ ...missing_raises_doc_required_exc_inheritance.txt | 1 + 8 files changed, 111 insertions(+), 18 deletions(-) create mode 100644 tests/functional/ext/docparams/raise/missing_raises_doc_required_exc_inheritance.py create mode 100644 tests/functional/ext/docparams/raise/missing_raises_doc_required_exc_inheritance.rc create mode 100644 tests/functional/ext/docparams/raise/missing_raises_doc_required_exc_inheritance.txt diff --git a/ChangeLog b/ChangeLog index 36b75c95d..c2f77182e 100644 --- a/ChangeLog +++ b/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) diff --git a/doc/whatsnew/2.13.rst b/doc/whatsnew/2.13.rst index 9c7d93d2c..da595d225 100644 --- a/doc/whatsnew/2.13.rst +++ b/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 diff --git a/pylint/extensions/_check_docs_utils.py b/pylint/extensions/_check_docs_utils.py index bfab0e1e4..2a26fe0e2 100644 --- a/pylint/extensions/_check_docs_utils.py +++ b/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,28 +130,30 @@ 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: @@ -159,15 +161,14 @@ def possible_exc_types(node): 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() diff --git a/pylint/extensions/docparams.py b/pylint/extensions/docparams.py index e7b293988..a0c66f16c 100644 --- a/pylint/extensions/docparams.py +++ b/pylint/extensions/docparams.py @@ -309,14 +309,25 @@ class DocstringParameterChecker(BaseChecker): 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: diff --git a/tests/extensions/test_check_docs_utils.py b/tests/extensions/test_check_docs_utils.py index c1552a0c3..0414d05a4 100644 --- a/tests/extensions/test_check_docs_utils.py +++ b/tests/extensions/test_check_docs_utils.py @@ -147,5 +147,7 @@ def test_space_indentation(string: str, count: int) -> None: ], ) 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 diff --git a/tests/functional/ext/docparams/raise/missing_raises_doc_required_exc_inheritance.py b/tests/functional/ext/docparams/raise/missing_raises_doc_required_exc_inheritance.py new file mode 100644 index 000000000..16334818d --- /dev/null +++ b/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") diff --git a/tests/functional/ext/docparams/raise/missing_raises_doc_required_exc_inheritance.rc b/tests/functional/ext/docparams/raise/missing_raises_doc_required_exc_inheritance.rc new file mode 100644 index 000000000..098921070 --- /dev/null +++ b/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 diff --git a/tests/functional/ext/docparams/raise/missing_raises_doc_required_exc_inheritance.txt b/tests/functional/ext/docparams/raise/missing_raises_doc_required_exc_inheritance.txt new file mode 100644 index 000000000..411cb77d5 --- /dev/null +++ b/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 -- cgit v1.2.1