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

fix(inconsistent-return-statements): Fix false positive due to pylint not seeing that a method returns NoReturn #8750

Merged
3 changes: 3 additions & 0 deletions doc/whatsnew/fragments/8747.false_positive
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Fix a false positive where pylint was ignoring method calls annotated as ``NoReturn`` during the ``inconsistent-return-statements`` check.

Closes #8747
8 changes: 5 additions & 3 deletions pylint/checkers/refactoring/refactoring_checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -1998,16 +1998,18 @@ def _has_return_in_siblings(node: nodes.NodeNG) -> bool:
next_sibling = next_sibling.next_sibling()
return False

def _is_function_def_never_returning(self, node: nodes.FunctionDef) -> bool:
def _is_function_def_never_returning(
self, node: nodes.FunctionDef | astroid.BoundMethod
Pierre-Sassoulas marked this conversation as resolved.
Show resolved Hide resolved
) -> bool:
"""Return True if the function never returns, False otherwise.

Args:
node (nodes.FunctionDef): function definition node to be analyzed.
node (nodes.FunctionDef or astroid.BoundMethod): function definition node to be analyzed.
Pierre-Sassoulas marked this conversation as resolved.
Show resolved Hide resolved

Returns:
bool: True if the function never returns, False otherwise.
"""
if isinstance(node, nodes.FunctionDef) and node.returns:
if isinstance(node, (nodes.FunctionDef, astroid.BoundMethod)) and node.returns:
Pierre-Sassoulas marked this conversation as resolved.
Show resolved Hide resolved
return (
isinstance(node.returns, nodes.Attribute)
and node.returns.attrname == "NoReturn"
Expand Down
42 changes: 42 additions & 0 deletions tests/functional/i/inconsistent/inconsistent_returns_noreturn.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,3 +53,45 @@ def bug_pylint_4122_bis(s):
return n
except ValueError:
parser_error_name('parser error')

class ClassUnderTest:
def _no_return_method(self) -> typing.NoReturn:
sys.exit(1)
kdestin marked this conversation as resolved.
Show resolved Hide resolved

def _falsely_no_return_method(self) -> typing.NoReturn:
return 1

def _does_return_method(self) -> int:
return 1

def bug_pylint_8747(self, s: str) -> int:
"""Every return is consistent because self._no_return_method hints NoReturn"""
try:
n = int(s)
if n < 1:
raise ValueError
return n
except ValueError:
self._no_return_method()

kdestin marked this conversation as resolved.
Show resolved Hide resolved
def bug_pylint_8747_wrong(self, s: str) -> int: # [inconsistent-return-statements]
"""Every return is not consistent because self._does_return_method() returns a value"""
try:
n = int(s)
if n < 1:
raise ValueError
return n
except ValueError:
self._does_return_method()

def bug_pylint_8747_incorrect_annotation(self, s: str) -> int:
"""Every return is consistent since pylint does not attempt to detect that the
NoReturn annotation is incorrect and the function actually returns
"""
try:
n = int(s)
if n < 1:
raise ValueError
return n
except ValueError:
self._falsely_no_return_method()
Original file line number Diff line number Diff line change
@@ -1 +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