Skip to content

Commit 9235647

Browse files
committed
add checker for return/yield in finally
1 parent b968fa0 commit 9235647

File tree

11 files changed

+102
-2
lines changed

11 files changed

+102
-2
lines changed
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
def second_favorite():
2+
fruits = ["kiwi", "pineapple"]
3+
try:
4+
return fruits[1]
5+
finally:
6+
# because of this `return` statement, this function will always return "kiwi"
7+
return fruits[0] # [return-or-yield-in-finally]
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
def all_favorites():
2+
fruits = ["orange", "kiwi", "pineapple"]
3+
4+
for fruit in fruits:
5+
try:
6+
yield fruit
7+
finally:
8+
# this `yield` statement will add `None` after every fruit
9+
# list(all_favorites()) will result in:
10+
# ["orange", None, "kiwi", None, "pineapple", None]
11+
yield # [return-or-yield-in-finally]
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
def second_favorite():
2+
fruits = ["apple"]
3+
try:
4+
return fruits[1]
5+
except KeyError:
6+
...
7+
8+
return fruits[0]
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
def all_favorites():
2+
fruits = ["orange", "kiwi", "pineapple"]
3+
4+
for fruit in fruits:
5+
try:
6+
yield fruit
7+
finally:
8+
...
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
- `Python 3 docs 'finally' clause <https://docs.python.org/3/reference/compound_stmts.html#finally-clause>`_

doc/whatsnew/fragments/8260.new_check

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
Add ``return-or-yield-in-finally`` to emit a message if a yield or return statement
2+
was found in a finally clause.
3+
4+
Closes #8260

pylint/checkers/base/basic_checker.py

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -266,6 +266,12 @@ class BasicChecker(_BasicChecker):
266266
"Used when an exception is created without being assigned, raised or returned "
267267
"for subsequent use elsewhere.",
268268
),
269+
"W0134": (
270+
"'%s' shadowed by the 'finally' clause.",
271+
"return-or-yield-in-finally",
272+
"Emitted when a 'return' or 'yield' statement is found in a 'finally' block. "
273+
"This will overwrite the return value of a function and should be avoided.",
274+
),
269275
}
270276

271277
reports = (("RP0101", "Statistics by type", report_by_type_stats),)
@@ -765,6 +771,16 @@ def visit_tryfinally(self, node: nodes.TryFinally) -> None:
765771
assert self._tryfinallys is not None
766772
self._tryfinallys.append(node)
767773

774+
for final_node in node.finalbody:
775+
for return_node in final_node.nodes_of_class(nodes.Return):
776+
self.add_message(
777+
"return-or-yield-in-finally", node=return_node, args=("return",)
778+
)
779+
for yield_node in final_node.nodes_of_class(nodes.Yield):
780+
self.add_message(
781+
"return-or-yield-in-finally", node=yield_node, args=("yield",)
782+
)
783+
768784
def leave_tryfinally(self, _: nodes.TryFinally) -> None:
769785
"""Update try...finally flag."""
770786
assert self._tryfinallys is not None

tests/functional/l/lost_exception.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
# pylint: disable=missing-docstring, using-constant-test,cell-var-from-loop
1+
# pylint: disable=missing-docstring, using-constant-test,cell-var-from-loop,return-or-yield-in-finally
22

33
def insidious_break_and_return():
44
for i in range(0, -5, -1):
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
# pylint: disable=missing-docstring,too-few-public-methods,useless-return,lost-exception
2+
3+
def second_favorite():
4+
fruits = ["apple"]
5+
try:
6+
return fruits[1]
7+
finally:
8+
return fruits[0] # [return-or-yield-in-finally]
9+
10+
11+
def all_favorites():
12+
fruits = ["orange", "kiwi", "pineapple"]
13+
14+
for fruit in fruits:
15+
try:
16+
yield fruit
17+
finally:
18+
yield # [return-or-yield-in-finally]
19+
20+
21+
def more_favorites():
22+
fruits = ["orange", "kiwi", "pineapple"]
23+
24+
for fruit in fruits:
25+
try:
26+
yield fruit
27+
finally:
28+
if len(fruit) > 7:
29+
yield "too many fruits" # [return-or-yield-in-finally]
30+
31+
32+
def even_more_favorites():
33+
fruits = ["orange", "kiwi", "pineapple"]
34+
35+
for fruit in fruits:
36+
try:
37+
yield fruit
38+
finally:
39+
if len(fruit) > 7:
40+
for fruit_name in fruits:
41+
yield f"please remove {fruit_name}" # [return-or-yield-in-finally]
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
return-or-yield-in-finally:8:8:8:24:second_favorite:'return' shadowed by the 'finally' clause.:UNDEFINED
2+
return-or-yield-in-finally:18:12:18:17:all_favorites:'yield' shadowed by the 'finally' clause.:UNDEFINED
3+
return-or-yield-in-finally:29:16:29:39:more_favorites:'yield' shadowed by the 'finally' clause.:UNDEFINED
4+
return-or-yield-in-finally:41:20:41:55:even_more_favorites:'yield' shadowed by the 'finally' clause.:UNDEFINED

tests/functional/u/used/used_before_assignment_488.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
# pylint: disable=missing-docstring
1+
# pylint: disable=missing-docstring,return-or-yield-in-finally
22
def func():
33
"""Test that a variable defined in a finally clause does not trigger a false positive"""
44
try:

0 commit comments

Comments
 (0)