Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

accept-no-raise-doc is not checked for docstrings with at least one matching section #7208

Closed
bchardin opened this issue Jul 20, 2022 · 3 comments · Fixed by #7581
Closed
Assignees
Labels
Bug 🪲 False Negative 🦋 No message is emitted but something is wrong with the code
Milestone

Comments

@bchardin
Copy link
Contributor

Bug description

The docstyle extension sometimes ignores the accept-no-raise-doc option because the _add_raise_messagemethod is invoked directly instead of _handle_no_raise_doc in visit_raise.

"""Minimal example where a W9006 message is displayed even if the
accept-no-raise-doc option is set to True.

Requires at least one matching section (`Docstring.matching_sections`)."""

def w9006issue(dummy: int):
    """Sample function.

    :param dummy: Unused
    """
    raise AssertionError()

Configuration

[MAIN]

load-plugins=pylint.extensions.docparams

[pylint.extensions.docstyle]

accept-no-raise-doc=yes

Command used

pylint w9006_issue.py

Pylint output

************* Module w9006_issue
w9006_issue.py:6:0: W9006: "AssertionError" not documented as being raised (missing-raises-doc)

------------------------------------------------------------------
Your code has been rated at 5.00/10 (previous run: 5.00/10, +0.00)

Expected behavior

The expected behavior is to have pylint display no message.

Pylint version

pylint 2.14.5
astroid 2.11.7
Python 3.8.5 (default, Sep  7 2020, 16:44:59)

OS / Environment

Debian 9.13 (stretch)

Additional dependencies

No response

@bchardin bchardin added the Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling label Jul 20, 2022
@bchardin
Copy link
Contributor Author

A suggested fix is to call _handle_no_raise_doc.

diff --git a/pylint/extensions/docparams.py b/pylint/extensions/docparams.py
index c3253c89e..a77ac916d 100644
--- a/pylint/extensions/docparams.py
+++ b/pylint/extensions/docparams.py
@@ -313,7 +313,7 @@ class DocstringParameterChecker(BaseChecker):
             else:
                 missing_excs.add(expected.name)
 
-        self._add_raise_message(missing_excs, func_node)
+        self._handle_no_raise_doc(missing_excs, func_node)
 
     def visit_return(self, node: nodes.Return) -> None:
         if not utils.returns_something(node):

@Pierre-Sassoulas Pierre-Sassoulas added Bug 🪲 False Positive 🦟 A message is emitted but nothing is wrong with the code and removed Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling labels Jul 20, 2022
@Pierre-Sassoulas Pierre-Sassoulas self-assigned this Jul 20, 2022
@Pierre-Sassoulas
Copy link
Member

Very nice bug report thank you. I credited you in the fix as you did almost everything, please feel free to review in #7212.

@Pierre-Sassoulas Pierre-Sassoulas added this to the 2.15.0 milestone Jul 20, 2022
@DanielNoord DanielNoord removed this from the 2.15.0 milestone Aug 23, 2022
@bchardin
Copy link
Contributor Author

bchardin commented Oct 6, 2022

I tried to fix this issue in a new PR #7581.

There is still a breaking change, because one test was assuming that the default value for accept-no-raise-doc was False, despite it being True.
I suspect other projects might be affected by this change in the same way.

@Pierre-Sassoulas Pierre-Sassoulas added this to the 2.16.0 milestone Oct 28, 2022
@Pierre-Sassoulas Pierre-Sassoulas added False Negative 🦋 No message is emitted but something is wrong with the code and removed False Positive 🦟 A message is emitted but nothing is wrong with the code labels Oct 28, 2022
Pierre-Sassoulas pushed a commit that referenced this issue Oct 28, 2022
Check the accept-no-raise-doc option when the docstring has matching sections (Fixes #7208)

Defaut value for accept-no-raise-doc is True, but tests in
`missing_doc_required_Sphinx.py` assume that this option is set to False.

Co-authored-by: Brice Chardin <brice.chardin@ensma.fr>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug 🪲 False Negative 🦋 No message is emitted but something is wrong with the code
Projects
None yet
3 participants