Skip to content

Give preference to --never-returning-functions in inconsistent-return-statements check #9591

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

Merged
merged 8 commits into from
Jul 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions doc/whatsnew/fragments/7565.false_negative
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Fix computation of never-returning function: `Never` is handled in addition to `NoReturn`, and priority is given to the explicit `--never-returning-functions` option.

Closes #7565.
33 changes: 19 additions & 14 deletions pylint/checkers/refactoring/refactoring_checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -2030,12 +2030,13 @@
if isinstance(node, nodes.Return):
return True
if isinstance(node, nodes.Call):
try:
funcdef_node = node.func.inferred()[0]
if self._is_function_def_never_returning(funcdef_node):
return True
except astroid.InferenceError:
pass
return any(
(
isinstance(maybe_func, (nodes.FunctionDef, bases.BoundMethod))
and self._is_function_def_never_returning(maybe_func)
)
for maybe_func in utils.infer_all(node.func)
)
if isinstance(node, nodes.While):
# A while-loop is considered return-ended if it has a
# truthy test and no break statements
Expand Down Expand Up @@ -2087,17 +2088,21 @@
Returns:
bool: True if the function never returns, False otherwise.
"""
if isinstance(node, (nodes.FunctionDef, astroid.BoundMethod)) and node.returns:
return (
try:
if node.qname() in self._never_returning_functions:
return True
except (TypeError, AttributeError):
pass

Check warning on line 2095 in pylint/checkers/refactoring/refactoring_checker.py

View check run for this annotation

Codecov / codecov/patch

pylint/checkers/refactoring/refactoring_checker.py#L2094-L2095

Added lines #L2094 - L2095 were not covered by tests
return (
isinstance(node, (nodes.FunctionDef, astroid.BoundMethod))
and node.returns
and (
isinstance(node.returns, nodes.Attribute)
and node.returns.attrname == "NoReturn"
and node.returns.attrname in {"NoReturn", "Never"}
or isinstance(node.returns, nodes.Name)
and node.returns.name == "NoReturn"
and node.returns.name in {"NoReturn", "Never"}
)
try:
return node.qname() in self._never_returning_functions
except (TypeError, AttributeError):
return False
)

def _check_return_at_the_end(self, node: nodes.FunctionDef) -> None:
"""Check for presence of a *single* return statement at the end of a
Expand Down
1 change: 1 addition & 0 deletions pylint/checkers/variables.py
Original file line number Diff line number Diff line change
Expand Up @@ -2613,6 +2613,7 @@ def _loopvar_name(self, node: astroid.Name) -> None:
):
return
# TODO: 4.0: Consider using utils.is_terminating_func
# after merging it with RefactoringChecker._is_function_def_never_returning
if isinstance(else_stmt, nodes.Expr) and isinstance(
else_stmt.value, nodes.Call
):
Expand Down
41 changes: 40 additions & 1 deletion tests/functional/i/inconsistent/inconsistent_returns_noreturn.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,22 @@
import sys
import typing

from pylint.constants import PY311_PLUS

if PY311_PLUS:
from typing import assert_never # pylint: disable=no-name-in-module
else:
from typing_extensions import assert_never


def parser_error(msg) -> typing.NoReturn: # pylint: disable=unused-argument
sys.exit(1)

def parser_error_nortype(msg): # pylint: disable=unused-argument
sys.exit(2)


from typing import NoReturn # pylint: disable=wrong-import-position
from typing import NoReturn # pylint: disable=wrong-import-position,wrong-import-order

def parser_error_name(msg) -> NoReturn: # pylint: disable=unused-argument
sys.exit(3)
Expand Down Expand Up @@ -95,3 +103,34 @@ def bug_pylint_8747_incorrect_annotation(self, s: str) -> int:
return n
except ValueError:
self._falsely_no_return_method()

# https://github.com/pylint-dev/pylint/issues/7565
def never_is_handled_like_noreturn(arg: typing.Union[int, str]) -> int:
if isinstance(arg, int):
return 1
if isinstance(arg, str):
return 2
assert_never(arg)


def declared_to_not_return() -> None:
return


def config_takes_precedence_over_inference(arg: typing.Union[int, str]) -> int:
if isinstance(arg, int):
return 1
if isinstance(arg, str):
return 2
declared_to_not_return()


def unrelated() -> None:
return


# +1: [inconsistent-return-statements]
def ensure_message(arg: typing.Union[int, str]) -> int:
if isinstance(arg, int):
return 1
unrelated()
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
[REFACTORING]
never-returning-functions=sys.exit,sys.getdefaultencoding
never-returning-functions=sys.exit,sys.getdefaultencoding,inconsistent_returns_noreturn.declared_to_not_return
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
inconsistent-return-statements:32:0:32:25:bug_pylint_4122_wrong:Either all return statements in a function should return an expression, or none of them should.:UNDEFINED
inconsistent-return-statements:77:4:77:29:ClassUnderTest.bug_pylint_8747_wrong:Either all return statements in a function should return an expression, or none of them should.:UNDEFINED
inconsistent-return-statements:40:0:40:25:bug_pylint_4122_wrong:Either all return statements in a function should return an expression, or none of them should.:UNDEFINED
inconsistent-return-statements:85:4:85:29:ClassUnderTest.bug_pylint_8747_wrong:Either all return statements in a function should return an expression, or none of them should.:UNDEFINED
inconsistent-return-statements:133:0:133:18:ensure_message:Either all return statements in a function should return an expression, or none of them should.:UNDEFINED
Loading