Skip to content

Commit

Permalink
Fix useless-return false negatives
Browse files Browse the repository at this point in the history
Returns inside try or if/else conditions were not being detected as
useless. Said nodes are now checked for return statements to ensure
their last component is not a useless return as well.

Closes #9449.
  • Loading branch information
crazybolillo authored and Pierre-Sassoulas committed Mar 15, 2024
1 parent 0b212cf commit 96b1c47
Show file tree
Hide file tree
Showing 6 changed files with 45 additions and 4 deletions.
3 changes: 3 additions & 0 deletions doc/whatsnew/fragments/9449.false_negative
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
If and Try nodes are now checked for useless return statements as well.

Closes #9449.
10 changes: 8 additions & 2 deletions pylint/checkers/refactoring/refactoring_checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -2078,12 +2078,18 @@ def _check_return_at_the_end(self, node: nodes.FunctionDef) -> None:
Per its implementation and PEP8 we can have a "return None" at the end
of the function body if there are other return statements before that!
"""
if len(self._return_nodes[node.name]) > 1:
if len(self._return_nodes[node.name]) != 1:
return
if len(node.body) <= 1:
if not node.body:
return

last = node.body[-1]
if isinstance(last, nodes.Return) and len(node.body) == 1:
return

while isinstance(last, (nodes.If, nodes.Try, nodes.ExceptHandler)):
last = last.last_child()

if isinstance(last, nodes.Return):
# e.g. "return"
if last.value is None:
Expand Down
2 changes: 1 addition & 1 deletion tests/functional/i/inconsistent/inconsistent_returns.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#pylint: disable=missing-docstring, no-else-return, no-else-break, invalid-name, unused-variable, superfluous-parens, try-except-raise
#pylint: disable=disallowed-name,too-few-public-methods,no-member,useless-else-on-loop
#pylint: disable=disallowed-name,too-few-public-methods,no-member,useless-else-on-loop,useless-return
"""Testing inconsistent returns"""
import math
import sys
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
"""If the else block returns, it is generally safe to rely on assignments in the except."""
# pylint: disable=missing-function-docstring, invalid-name
# pylint: disable=missing-function-docstring, invalid-name, useless-return
import sys

def valid():
Expand Down
28 changes: 28 additions & 0 deletions tests/functional/u/useless/useless_return.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,31 @@ def mymethod(self): # [useless-return]
# These are not emitted
def item_at(self):
return None


def function2(parameter): # [useless-return]
if parameter:
pass
return


def function3(parameter): # [useless-return]
if parameter:
pass
else:
return


def function4(parameter): # [useless-return]
try:
parameter.do()
except RuntimeError:
parameter.other()
return


def function5(parameter): # [useless-return]
try:
parameter.do()
except RuntimeError:
return
4 changes: 4 additions & 0 deletions tests/functional/u/useless/useless_return.txt
Original file line number Diff line number Diff line change
@@ -1,2 +1,6 @@
useless-return:4:0:4:10:myfunc:Useless return at end of function or method:UNDEFINED
useless-return:9:4:9:16:SomeClass.mymethod:Useless return at end of function or method:UNDEFINED
useless-return:18:0:18:13:function2:Useless return at end of function or method:UNDEFINED
useless-return:24:0:24:13:function3:Useless return at end of function or method:UNDEFINED
useless-return:31:0:31:13:function4:Useless return at end of function or method:UNDEFINED
useless-return:39:0:39:13:function5:Useless return at end of function or method:UNDEFINED

0 comments on commit 96b1c47

Please sign in to comment.