Skip to content

Commit

Permalink
Fix false positive inconsistent-return-statement for method marked …
Browse files Browse the repository at this point in the history
…`NoReturn` (#8750) (#8761)

Allow inspection of BoundMethod when checking if annotated as NoReturn

(cherry picked from commit 8614ccf)

Co-authored-by: kdestin <[email protected]>
  • Loading branch information
github-actions[bot] and kdestin authored Jun 12, 2023
1 parent b59dfea commit 80e024a
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 3 deletions.
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

0 comments on commit 80e024a

Please sign in to comment.