Skip to content

Commit 3d03892

Browse files
fix(while_let_loop): detect the pattern when the loop has a label
fixes #17590
1 parent bdce5b9 commit 3d03892

3 files changed

Lines changed: 84 additions & 16 deletions

File tree

clippy_lints/src/loops/while_let_loop.rs

Lines changed: 19 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use clippy_utils::source::{snippet, snippet_indent, snippet_opt};
44
use clippy_utils::ty::needs_ordered_drop;
55
use clippy_utils::visitors::any_temporaries_need_ordered_drop;
66
use clippy_utils::{higher, peel_blocks};
7-
use rustc_ast::BindingMode;
7+
use rustc_ast::{BindingMode, Label};
88
use rustc_errors::Applicability;
99
use rustc_hir::{Block, Expr, ExprKind, LetStmt, MatchSource, Pat, PatKind, Path, QPath, StmtKind, Ty};
1010
use rustc_lint::LateContext;
@@ -26,10 +26,14 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, loop_blo
2626
_ => return,
2727
};
2828
let has_trailing_exprs = loop_block.stmts.len() + usize::from(loop_block.expr.is_some()) > 1;
29-
29+
let loop_label = if let ExprKind::Loop(_, label, ..) = expr.kind {
30+
label
31+
} else {
32+
None
33+
};
3034
if let Some(if_let) = higher::IfLet::hir(cx, init)
3135
&& let Some(else_expr) = if_let.if_else
32-
&& is_simple_break_expr(else_expr)
36+
&& is_simple_break_expr(else_expr, loop_label)
3337
{
3438
could_be_while_let(
3539
cx,
@@ -40,14 +44,14 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, loop_blo
4044
let_info,
4145
Some(if_let.if_then),
4246
);
43-
} else if els.is_some_and(is_simple_break_block)
47+
} else if els.is_some_and(|b| is_simple_break_block(b, loop_label))
4448
&& let Some((pat, _)) = let_info
4549
{
4650
could_be_while_let(cx, expr, pat, init, has_trailing_exprs, let_info, None);
4751
} else if let ExprKind::Match(scrutinee, [arm1, arm2], MatchSource::Normal) = init.kind
4852
&& arm1.guard.is_none()
4953
&& arm2.guard.is_none()
50-
&& is_simple_break_expr(arm2.body)
54+
&& is_simple_break_expr(arm2.body, loop_label)
5155
{
5256
could_be_while_let(
5357
cx,
@@ -61,22 +65,22 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, loop_blo
6165
}
6266
}
6367

64-
/// Checks if `block` contains a single unlabeled `break` expression or statement, possibly embedded
65-
/// inside other blocks.
66-
fn is_simple_break_block(block: &Block<'_>) -> bool {
68+
/// Checks if `block` contains a single unlabeled and importantly now also a labeled `break`
69+
/// expression or statement, possibly embedded inside other blocks.
70+
fn is_simple_break_block(block: &Block<'_>, looplabel: Option<Label>) -> bool {
6771
match (block.stmts, block.expr) {
68-
([s], None) => matches!(s.kind, StmtKind::Expr(e) | StmtKind::Semi(e) if is_simple_break_expr(e)),
69-
([], Some(e)) => is_simple_break_expr(e),
72+
([s], None) => matches!(s.kind, StmtKind::Expr(e) | StmtKind::Semi(e) if is_simple_break_expr(e, looplabel)),
73+
([], Some(e)) => is_simple_break_expr(e, looplabel),
7074
_ => false,
7175
}
7276
}
7377

74-
/// Checks if `expr` contains a single unlabeled `break` expression or statement, possibly embedded
75-
/// inside other blocks.
76-
fn is_simple_break_expr(expr: &Expr<'_>) -> bool {
78+
/// Checks if `expr` contains a single unlabeled and importantly now also a labeled `break`
79+
/// expression or statement, possibly embedded inside other blocks.
80+
fn is_simple_break_expr(expr: &Expr<'_>, looplabel: Option<Label>) -> bool {
7781
match expr.kind {
78-
ExprKind::Block(b, _) => is_simple_break_block(b),
79-
ExprKind::Break(dest, None) => dest.label.is_none(),
82+
ExprKind::Block(b, _) => is_simple_break_block(b, looplabel),
83+
ExprKind::Break(dest, None) => dest.label.is_none() || dest.label == looplabel,
8084
_ => false,
8185
}
8286
}

tests/ui/while_let_loop.rs

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -274,3 +274,35 @@ fn issue16378() {
274274
println!("x = {x}");
275275
}
276276
}
277+
278+
fn issue17590_labeled_loop() {
279+
let mut it = [1, 2, 3].iter();
280+
'cool: loop {
281+
//~^ while_let_loop
282+
match it.next() {
283+
Some(_) => {},
284+
None => break 'cool,
285+
}
286+
}
287+
}
288+
289+
fn issue17590_labeled_if_let() {
290+
let mut it = [1, 2, 3].iter();
291+
'outer: loop {
292+
//~^ while_let_loop
293+
if let Some(x) = it.next() {
294+
println!("{x}");
295+
} else {
296+
break 'outer;
297+
}
298+
}
299+
}
300+
301+
fn issue17590_labeled_let_else() {
302+
let mut it = [1, 2, 3].iter();
303+
'outer: loop {
304+
//~^ while_let_loop
305+
let Some(x) = it.next() else { break 'outer };
306+
println!("{x}");
307+
}
308+
}

tests/ui/while_let_loop.stderr

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -186,5 +186,37 @@ LL + ..
186186
LL + }
187187
|
188188

189-
error: aborting due to 12 previous errors
189+
error: this loop could be written as a `while let` loop
190+
--> tests/ui/while_let_loop.rs:280:5
191+
|
192+
LL | / 'cool: loop {
193+
LL | |
194+
LL | | match it.next() {
195+
LL | | Some(_) => {},
196+
... |
197+
LL | | }
198+
| |_____^ help: try: `while let Some(_) = it.next() { .. }`
199+
200+
error: this loop could be written as a `while let` loop
201+
--> tests/ui/while_let_loop.rs:291:5
202+
|
203+
LL | / 'outer: loop {
204+
LL | |
205+
LL | | if let Some(x) = it.next() {
206+
LL | | println!("{x}");
207+
... |
208+
LL | | }
209+
| |_____^ help: try: `while let Some(x) = it.next() { .. }`
210+
211+
error: this loop could be written as a `while let` loop
212+
--> tests/ui/while_let_loop.rs:303:5
213+
|
214+
LL | / 'outer: loop {
215+
LL | |
216+
LL | | let Some(x) = it.next() else { break 'outer };
217+
LL | | println!("{x}");
218+
LL | | }
219+
| |_____^ help: try: `while let Some(x) = it.next() { .. }`
220+
221+
error: aborting due to 15 previous errors
190222

0 commit comments

Comments
 (0)