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

[Backport maintenance/2.17.x] fix(inconsistent-return-statements): Fix false positive due to pylint not seeing that a method returns NoReturn #8761

Merged
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/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 @@ -2002,16 +2002,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
) -> 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.

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:
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)

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()

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