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

Cannot use --never-returning-functions for some functions #4167

Open
timgkeller opened this issue Mar 1, 2021 · 3 comments
Open

Cannot use --never-returning-functions for some functions #4167

timgkeller opened this issue Mar 1, 2021 · 3 comments
Labels
Astroid Related to astroid Bug 🪲 Needs astroid Brain 🧠 Needs a brain tip in astroid (then an astroid upgrade) Needs astroid update Needs an astroid update (probably a release too) before being mergable

Comments

@timgkeller
Copy link

timgkeller commented Mar 1, 2021

Version

pylint==2.7.2

Dependency

pytest

Steps to reproduce

Create a python file debug.py:

import pytest

def apparently_inconsistent_returning_function(return_something: bool) -> None:
    """A function that looks like not all paths return something."""
    if return_something:
        return None
    pytest.exit("Raises an exception.")
# pylint debug.py

=> R1710: Either all return statements in a function should return an expression, or none of them should. (inconsistent-return-statements)

This message is raised as pylint does not understand that pytest.exit raises an exception. Using --never-returning-functions="pytest.exit" should fix this behavior, however it doesn't:

# pylint debug.py --never-returning-functions="pytest.exit"

=> R1710: Either all return statements in a function should return an expression, or none of them should. (inconsistent-return-statements)

Expected behavior

pylint debug.py --never-returning-functions="pytest.exit" should not return an error for the above code.

Actual behavior

pylint debug.py --never-returning-functions="pytest.exit" still gives the error "inconsistent-return-statements"

Initial analysis

https://github.com/PyCQA/pylint/blob/master/pylint/checkers/refactoring/refactoring_checker.py#L1362

node.func is Attribute.exit(attrname='exit', expr=<Name.pytest l.7 at 0x7fab9184d898>)
node.func.inferred()[0] is "Uninferable", as a result there is no value we can set for --never-returning-functions that would get rid of the error "inconsistent-return-statements".

Workaround

import pytest

def pytest_exit(*args, **kwargs):
    """Call pytest.exit."""
    pytest.exit(*args, **kwargs)

def apparently_inconsistent_returning_function(return_something: bool) -> None:
    """A function that looks like not all paths return something."""
    if return_something:
        return None
    pytest_exit("Raises an exception.")

By wrapping pytest.exit in another function pytest_exit, node.func.inferred()[0] is now:

FunctionDef.pytest_exit(name='pytest_exit',
                        doc='Call pytest.exit.',
                        decorators=None,
                        args=<Arguments l.3 at 0x7f895a6b90f0>,
                        returns=None,
                        body=[<Expr l.5 at 0x7f895a6b9780>])

=> node.func.inferred()[0].qname() is "debug.pytest_exit" which we can use to get rid of the error "inconsistent-return-statements":

pylint debug.py --never-returning-functions="debug.pytest_exit"
@michael-k
Copy link
Contributor

@timgkeller
Copy link
Author

See #4122

pytest.exit seems to be annotated with NoReturn: https://github.com/pytest-dev/pytest/blob/c9e9a599fe5231da90967b6f77c73e17c12740b7/src/_pytest/outcomes.py#L113

I like the suggestion in #4122. This would certainly help in this case and many others. Maybe it would also allow to get rid of --never-returning-functions. However, as long as this option persists, I think it should work for functions that do not have the NoReturn type hint.

@cdce8p cdce8p added Astroid Related to astroid Bug 🪲 labels Apr 7, 2021
@cdce8p
Copy link
Member

cdce8p commented Apr 7, 2021

Support for typing.NoReturn was just added with #4304.

The issue here however seems to be related to astroid. pytest.exit("Raises an exception.") is inferred as Uninferable and thus can't be checked against --never-returning-functions.

@Pierre-Sassoulas Pierre-Sassoulas added Needs astroid Brain 🧠 Needs a brain tip in astroid (then an astroid upgrade) Needs astroid update Needs an astroid update (probably a release too) before being mergable labels Jul 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Astroid Related to astroid Bug 🪲 Needs astroid Brain 🧠 Needs a brain tip in astroid (then an astroid upgrade) Needs astroid update Needs an astroid update (probably a release too) before being mergable
Projects
None yet
Development

No branches or pull requests

4 participants