Skip to content

Commit 35ee088

Browse files
[ruff] Treat yield before break from a terminal loop as terminal (RUF075) (#25447)
## Summary Closes #25378. Extends the yield-before-return terminal check to also cover yield-before-break, but only when the enclosing loop is itself in a terminal position. break exits the innermost loop, so a yield right before it has no cleanup code after it in that case. ## Test Plan mdtest passes for fallible-context-manager.md across the new and existing cases
1 parent 1a8520e commit 35ee088

2 files changed

Lines changed: 101 additions & 10 deletions

File tree

crates/ruff_linter/resources/mdtest/ruff/fallible-context-manager.md

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -608,3 +608,79 @@ def good_multiple_yields():
608608
finally:
609609
print("cleanup")
610610
```
611+
612+
## Yield before `break` in a terminal loop is OK
613+
614+
A `yield` immediately followed by `break` is terminal when the enclosing loop itself sits
615+
in a terminal position, since `break` exits the loop and no cleanup code can run after.
616+
617+
```py
618+
from contextlib import contextmanager
619+
620+
621+
@contextmanager
622+
def good_yield_before_break_in_terminal_for():
623+
for value in items:
624+
yield
625+
break
626+
627+
628+
@contextmanager
629+
def good_yield_before_break_in_terminal_while():
630+
while condition:
631+
yield
632+
break
633+
634+
635+
@contextmanager
636+
def good_yield_before_break_in_terminal_for_in_if():
637+
if condition:
638+
for value in items:
639+
yield
640+
break
641+
```
642+
643+
## Yield before `break` in a non-terminal loop
644+
645+
```py
646+
from contextlib import contextmanager
647+
648+
649+
@contextmanager
650+
def bad_yield_before_break_with_trailing_code():
651+
for value in items:
652+
yield # error: [fallible-context-manager]
653+
break
654+
print("cleanup")
655+
```
656+
657+
## Yield before `break` from an inner loop
658+
659+
`break` only exits the innermost loop, so a `yield` before `break` inside a nested loop
660+
is not terminal: control falls through to the rest of the outer loop body.
661+
662+
```py
663+
from contextlib import contextmanager
664+
665+
666+
@contextmanager
667+
def bad_yield_before_break_in_inner_loop():
668+
for outer in items:
669+
for inner in stuff:
670+
yield # error: [fallible-context-manager]
671+
break
672+
cleanup()
673+
```
674+
675+
## Yield before `continue` is not terminal
676+
677+
```py
678+
from contextlib import contextmanager
679+
680+
681+
@contextmanager
682+
def bad_yield_before_continue():
683+
for value in items:
684+
yield # error: [fallible-context-manager]
685+
continue
686+
```

crates/ruff_linter/src/rules/ruff/rules/fallible_context_manager.rs

Lines changed: 25 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -89,17 +89,22 @@ fn has_contextmanager_decorator(checker: &Checker, function_def: &StmtFunctionDe
8989
/// - It is inside a `try` block that has `finally` or `except` handlers.
9090
/// - It is in a terminal position within a `with` block body (the context manager's
9191
/// `__exit__` handles cleanup).
92-
/// - It is in a terminal position (last statement in the function body, or immediately
93-
/// followed by `return`), meaning there is no cleanup code that could be skipped.
92+
/// - It is in a terminal position (last statement in the function body, immediately
93+
/// followed by `return`, or immediately followed by `break` from a loop that itself
94+
/// sits in a terminal position), meaning there is no cleanup code that could be skipped.
9495
struct YieldFinallyVisitor<'a, 'b> {
9596
/// The checker used to emit diagnostics.
9697
checker: &'a Checker<'b>,
9798
/// Whether the visitor is currently inside a `try` block that has
9899
/// `finally` or `except` handlers.
99100
in_protected_try: bool,
100101
/// Whether the visitor is at a terminal position: the last statement in
101-
/// the function body, or a `yield` immediately before a `return`.
102+
/// the function body, a `yield` immediately before a `return`, or a `yield`
103+
/// immediately before a `break` from a terminal loop.
102104
in_terminal_position: bool,
105+
/// Whether a `break` from the innermost enclosing loop would exit to
106+
/// a terminal position.
107+
in_terminal_loop: bool,
103108
}
104109

105110
impl<'a, 'b> YieldFinallyVisitor<'a, 'b> {
@@ -109,6 +114,7 @@ impl<'a, 'b> YieldFinallyVisitor<'a, 'b> {
109114
checker,
110115
in_protected_try: false,
111116
in_terminal_position: false,
117+
in_terminal_loop: false,
112118
}
113119
}
114120
}
@@ -175,7 +181,10 @@ impl Visitor<'_> for YieldFinallyVisitor<'_, '_> {
175181
self.visit_expr(iter);
176182
self.visit_expr(target);
177183
let terminal = self.in_terminal_position;
184+
let prev_loop = self.in_terminal_loop;
185+
self.in_terminal_loop = terminal;
178186
self.visit_body_with_terminal(body, terminal);
187+
self.in_terminal_loop = prev_loop;
179188
self.visit_body_with_terminal(orelse, terminal);
180189
}
181190

@@ -184,7 +193,10 @@ impl Visitor<'_> for YieldFinallyVisitor<'_, '_> {
184193
}) => {
185194
self.visit_expr(test);
186195
let terminal = self.in_terminal_position;
196+
let prev_loop = self.in_terminal_loop;
197+
self.in_terminal_loop = terminal;
187198
self.visit_body_with_terminal(body, terminal);
199+
self.in_terminal_loop = prev_loop;
188200
self.visit_body_with_terminal(orelse, terminal);
189201
}
190202

@@ -232,18 +244,21 @@ impl YieldFinallyVisitor<'_, '_> {
232244

233245
/// Visits each statement in `body`, tracking whether each is in a terminal position.
234246
///
235-
/// A statement is considered terminal if it is the last in the body (when `terminal` is true)
236-
/// or if it is a yield statement immediately followed by a `return`.
247+
/// A statement is considered terminal if it is the last in the body (when `terminal` is true),
248+
/// if it is a yield statement immediately followed by a `return`, or if it is a yield
249+
/// statement immediately followed by a `break` from a terminal loop.
237250
fn visit_body_with_terminal(&mut self, body: &[Stmt], terminal: bool) {
238251
for (i, stmt) in body.iter().enumerate() {
239252
let is_last = i == body.len() - 1;
240-
let is_yield_before_return = Self::is_yield_statement(stmt)
241-
&& body
242-
.get(i + 1)
243-
.is_some_and(|next| matches!(next, Stmt::Return(_)));
253+
let is_yield_before_terminator = Self::is_yield_statement(stmt)
254+
&& body.get(i + 1).is_some_and(|next| match next {
255+
Stmt::Return(_) => true,
256+
Stmt::Break(_) => self.in_terminal_loop,
257+
_ => false,
258+
});
244259

245260
let prev = self.in_terminal_position;
246-
self.in_terminal_position = (is_last && terminal) || is_yield_before_return;
261+
self.in_terminal_position = (is_last && terminal) || is_yield_before_terminator;
247262
self.visit_stmt(stmt);
248263
self.in_terminal_position = prev;
249264
}

0 commit comments

Comments
 (0)