pylint icon indicating copy to clipboard operation
pylint copied to clipboard

Nonsensical returns are not reported as `useless-return` if they are indented

Open e-gebes opened this issue 1 year ago • 2 comments

Bug description

# pylint: disable=missing-module-docstring,missing-function-docstring


def function1(parameter):
    parameter.do()
    return  # this is reported as `useless-return` as expected


def function2(parameter):
    if parameter:
        return
    return  # this is not reported as `useless-return`


def function3(parameter):
    if parameter:
        pass
    else:
        return  # this is not reported as `useless-return`


def function4(parameter):
    if parameter:
        return
    else:  # this is reported as `no-else-return` as expected
        return  # this is not reported as `useless-return`


def function5(parameter):
    try:
        parameter.do()
    except RuntimeError:
        parameter.other()
        return  # this is not reported as `useless-return`


def function6(parameter):
    try:
        parameter.do()
    except RuntimeError:
        return  # maybe even this should be reported - `pass` can be used

Configuration

No response

Command used

pylint a.py

Pylint output

************* Module a
4:0: R1711: Useless return at end of function or method (useless-return)
23:4: R1705: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it (no-else-return)

Expected behavior

Maybe more useless-return messages should be reported, see the comments in the code snippet.

Pylint version

pylint 2.17.7
astroid 2.15.8
Python 3.10.12 (main, Nov 20 2023, 15:14:05) [GCC 11.4.0]

OS / Environment

No response

Additional dependencies

No response

e-gebes avatar Feb 20 '24 13:02 e-gebes

Agree we could extend the check to the last line of a (potentially nested) try/if/with

jacobtylerwalls avatar Feb 20 '24 14:02 jacobtylerwalls

These are great examples.

For anyone who wants to fix this, the warning is raised from this function:

    def _check_return_at_the_end(self, node: nodes.FunctionDef) -> None:
        """Check for presence of a *single* return statement at the end of a
        function.

        "return" or "return None" are useless because None is the
        default return type if they are missing.

        NOTE: produces a message only if there is a single return statement
        in the function body. Otherwise _check_consistent_returns() is called!
        Per its implementation and PEP8 we can have a "return None" at the end
        of the function body if there are other return statements before that!
        """
        if len(self._return_nodes[node.name]) > 1:
            return
        if len(node.body) <= 1:
            return

        last = node.body[-1]
        if isinstance(last, nodes.Return):
            # e.g. "return"
            if last.value is None:
                self.add_message("useless-return", node=node)
            # return None"
            elif isinstance(last.value, nodes.Const) and (last.value.value is None):
                self.add_message("useless-return", node=node)

The docstring refers to another function, _check_consistent_returns. The logic there is murky.

nickdrozd avatar Feb 21 '24 17:02 nickdrozd