False positive missing-return-doc when a function is not supposed to be documented

[finite-state-machine]

Issue #2738 appears to persist on pylint 2.9.5 as well as 3.0.0-a4 (as retrieved using pip install --upgrade [--pre] pylint on this date). The merge request for that issue seems to imply this fix should have gone out with pylint 2.7.x.

Has something gone wrong here, or have I misunderstood the merge?

Steps to reproduce

File to be analyzed:

'''module docstring goes here'''

# this function doesn't require a docstring, because its name starts
# with an '_' (no-docstring-rgx):
def _function(some_arg: int) -> int:
    _ = some_arg
    return 0

rcfile:

[MASTER]
load-plugins=pylint.extensions.docparams

[PARAMETER DOCUMENTATION]
accept-no-param-doc=no
accept-no-return-doc=no

Current behaviour

************* Module scrap
scrap.py:5:0: W9011: Missing return documentation (missing-return-doc)

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

Expected behavior

There should be no errors; _function requires no docstring and should therefore require no return documentation.

pylint --version output

Result of pylint --version output:

pylint 3.0.0-a4
astroid 2.6.5
Python 3.8.3 (default, Jul  6 2020, 09:12:34)
[Clang 10.0.1 (clang-1001.0.46.4)]

[finite-state-machine]

In case it's helpful to mention them, the author and reviewers of the pull request relating to #2738 are: @komodo472, @Pierre-Sassoulas, and @hippo91.

[mwchase]

I was looking into this, and I think I found the issue. In the docparams extension, the visit_return, visit_yield, and visit_raise methods need an extra block after the

        if not isinstance(func_node, astroid.FunctionDef):
            return

Something along the lines of

        # skip functions that match the 'no-docstring-rgx' config option
        no_docstring_rgx = self.linter.config.no_docstring_rgx
        if no_docstring_rgx and re.match(no_docstring_rgx, func_node.name):
            return

I can put together a PR for this. But if I'm doing it, I'd first like to understand why there's both check_functiondef_returns and visit_return, for example. It looks like they're handling different things?

[Pierre-Sassoulas]

Sorry for the delay @mwchase. This is indeed strange, it seems that the checks could be duplicated, in particular return_nodes = node.nodes_of_class(astroid.Return) feel "wrong" (not actually wrong: inefficient). I'm supposing that recovering the docstring from the functiondef in:

https://github.com/PyCQA/pylint/blob/c33d237e6ea5795632295bb6390778a21e6f4080/pylint/extensions/docparams.py#L199

and recovering the return nodes of a single function is better than recovering the function def node of all return function then recovering their docstrings. And we'd need to do it several time for other nodes too if it wasn't done in visit_functiondef, for yields, for params...

https://github.com/PyCQA/pylint/blob/c33d237e6ea5795632295bb6390778a21e6f4080/pylint/extensions/docparams.py#L216

Just a supposition, I did not run actual performance comparison. Do you want to investigate ? Docparam is very well tested, I would not be afraid to refactor it. But at least a little explanation added in the code as a comment would not hurt.

[mwchase]

Ah, I guess I should file a followup issue for raises and yields. I can try to get that together tomorrow, if nobody else gets there first.

[DanielNoord]

Oh I completely misread your comment and looked over the raises and yields part.

I can probably write a PR for this somewhere this afternoon.

@Pierre-Sassoulas I'm reopening this issue and adding the blocker label. I think it makes sense to fix these three issues in the same patch release. Review shouldn't be too difficult now that we have done #7410.

Edit: PR at #7421.