From 4596d8b0b82ecc5b2a511906dfbbd4d316a8f729 Mon Sep 17 00:00:00 2001 From: Sven Panne Date: Wed, 3 Apr 2024 17:32:42 +0200 Subject: [PATCH] Fix "function never returning" computation This fixes 2 bugs in the computation: * `Never` is handled in addition to `NoReturn`. * Give priority to the explicit `--never-returning-functions` option. Closes #7565 --- doc/whatsnew/fragments/7565.false_positive | 3 +++ .../refactoring/refactoring_checker.py | 20 ++++++++++------- .../inconsistent_returns_noreturn.py | 22 ++++++++++++++++++- .../inconsistent_returns_noreturn.rc | 2 +- .../inconsistent_returns_noreturn.txt | 4 ++-- 5 files changed, 39 insertions(+), 12 deletions(-) create mode 100644 doc/whatsnew/fragments/7565.false_positive diff --git a/doc/whatsnew/fragments/7565.false_positive b/doc/whatsnew/fragments/7565.false_positive new file mode 100644 index 00000000000..647eb4611fa --- /dev/null +++ b/doc/whatsnew/fragments/7565.false_positive @@ -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. diff --git a/pylint/checkers/refactoring/refactoring_checker.py b/pylint/checkers/refactoring/refactoring_checker.py index edfff0385e7..1af9ce60066 100644 --- a/pylint/checkers/refactoring/refactoring_checker.py +++ b/pylint/checkers/refactoring/refactoring_checker.py @@ -2054,17 +2054,21 @@ def _is_function_def_never_returning( 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 + 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 diff --git a/tests/functional/i/inconsistent/inconsistent_returns_noreturn.py b/tests/functional/i/inconsistent/inconsistent_returns_noreturn.py index 13878aad696..9ba772efa30 100644 --- a/tests/functional/i/inconsistent/inconsistent_returns_noreturn.py +++ b/tests/functional/i/inconsistent/inconsistent_returns_noreturn.py @@ -3,6 +3,7 @@ import sys import typing +import typing_extensions def parser_error(msg) -> typing.NoReturn: # pylint: disable=unused-argument sys.exit(1) @@ -11,7 +12,7 @@ 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) @@ -95,3 +96,22 @@ 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: int | str) -> int: + if isinstance(arg, int): + return 1 + if isinstance(arg, str): + return 2 + typing_extensions.assert_never(arg) + + +def declared_to_not_return() -> None: + return + +def config_takes_precedence_over_inference(arg: int | str) -> int: + if isinstance(arg, int): + return 1 + if isinstance(arg, str): + return 2 + declared_to_not_return() diff --git a/tests/functional/i/inconsistent/inconsistent_returns_noreturn.rc b/tests/functional/i/inconsistent/inconsistent_returns_noreturn.rc index dd6cbb59899..fdeed8ed4d0 100644 --- a/tests/functional/i/inconsistent/inconsistent_returns_noreturn.rc +++ b/tests/functional/i/inconsistent/inconsistent_returns_noreturn.rc @@ -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 diff --git a/tests/functional/i/inconsistent/inconsistent_returns_noreturn.txt b/tests/functional/i/inconsistent/inconsistent_returns_noreturn.txt index e2ad258f72d..96571e325e5 100644 --- a/tests/functional/i/inconsistent/inconsistent_returns_noreturn.txt +++ b/tests/functional/i/inconsistent/inconsistent_returns_noreturn.txt @@ -1,2 +1,2 @@ -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:33:0:33: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:78:4:78:29:ClassUnderTest.bug_pylint_8747_wrong:Either all return statements in a function should return an expression, or none of them should.:UNDEFINED