Skip to content

Commit d6890e3

Browse files
committed
extend while_let_loop to loop { let else }
1 parent 8428b16 commit d6890e3

File tree

5 files changed

+55
-20
lines changed

5 files changed

+55
-20
lines changed

clippy_lints/src/loops/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -861,6 +861,7 @@ impl<'tcx> LateLintPass<'tcx> for Loops {
861861
// check for `loop { if let {} else break }` that could be `while let`
862862
// (also matches an explicit "match" instead of "if let")
863863
// (even if the "match" or "if let" is used for declaration)
864+
// (also matches on `let {} else break`)
864865
if let ExprKind::Loop(block, label, LoopSource::Loop, _) = expr.kind {
865866
// also check for empty `loop {}` statements, skipping those in #[panic_handler]
866867
empty_loop::check(cx, expr, block);

clippy_lints/src/loops/while_let_loop.rs

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -10,19 +10,19 @@ use rustc_hir::{Block, Expr, ExprKind, LetStmt, MatchSource, Pat, PatKind, Path,
1010
use rustc_lint::LateContext;
1111

1212
pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, loop_block: &'tcx Block<'_>) {
13-
let (init, let_info) = match (loop_block.stmts, loop_block.expr) {
13+
let (init, let_info, els) = match (loop_block.stmts, loop_block.expr) {
1414
([stmt, ..], _) => match stmt.kind {
1515
StmtKind::Let(LetStmt {
1616
init: Some(e),
17-
els: None,
17+
els,
1818
pat,
1919
ty,
2020
..
21-
}) => (*e, Some((*pat, *ty))),
22-
StmtKind::Semi(e) | StmtKind::Expr(e) => (e, None),
21+
}) => (*e, Some((*pat, *ty)), *els),
22+
StmtKind::Semi(e) | StmtKind::Expr(e) => (e, None, None),
2323
_ => return,
2424
},
25-
([], Some(e)) => (e, None),
25+
([], Some(e)) => (e, None, None),
2626
_ => return,
2727
};
2828
let has_trailing_exprs = loop_block.stmts.len() + usize::from(loop_block.expr.is_some()) > 1;
@@ -38,14 +38,26 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, loop_blo
3838
if_let.let_expr,
3939
has_trailing_exprs,
4040
let_info,
41-
if_let.if_then,
41+
Some(if_let.if_then),
4242
);
43+
} else if els.and_then(|x| x.expr).is_some_and(is_simple_break_expr)
44+
&& let Some((pat, _)) = let_info
45+
{
46+
could_be_while_let(cx, expr, pat, init, has_trailing_exprs, let_info, None);
4347
} else if let ExprKind::Match(scrutinee, [arm1, arm2], MatchSource::Normal) = init.kind
4448
&& arm1.guard.is_none()
4549
&& arm2.guard.is_none()
4650
&& is_simple_break_expr(arm2.body)
4751
{
48-
could_be_while_let(cx, expr, arm1.pat, scrutinee, has_trailing_exprs, let_info, arm1.body);
52+
could_be_while_let(
53+
cx,
54+
expr,
55+
arm1.pat,
56+
scrutinee,
57+
has_trailing_exprs,
58+
let_info,
59+
Some(arm1.body),
60+
);
4961
}
5062
}
5163

@@ -70,7 +82,7 @@ fn could_be_while_let<'tcx>(
7082
let_expr: &'tcx Expr<'_>,
7183
has_trailing_exprs: bool,
7284
let_info: Option<(&Pat<'_>, Option<&Ty<'_>>)>,
73-
inner_expr: &Expr<'_>,
85+
inner_expr: Option<&Expr<'_>>,
7486
) {
7587
if has_trailing_exprs
7688
&& (needs_ordered_drop(cx, cx.typeck_results().expr_ty(let_expr))
@@ -85,7 +97,7 @@ fn could_be_while_let<'tcx>(
8597
// 1) it was ugly with big bodies;
8698
// 2) it was not indented properly;
8799
// 3) it wasn’t very smart (see #675).
88-
let inner_content = if let Some((pat, ty)) = let_info
100+
let inner_content = if let Some(((pat, ty), inner_expr)) = let_info.zip(inner_expr)
89101
// Prevent trivial reassignments such as `let x = x;` or `let _ = …;`, but
90102
// keep them if the type has been explicitly specified.
91103
&& (!is_trivial_assignment(pat, peel_blocks(inner_expr)) || ty.is_some())

tests/ui/infinite_loops.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
//@no-rustfix: multiple suggestions add `-> !` to the same fn
22
//@aux-build:proc_macros.rs
33

4-
#![allow(clippy::never_loop)]
4+
#![allow(clippy::never_loop, clippy::while_let_loop)]
55
#![warn(clippy::infinite_loop)]
66

77
extern crate proc_macros;

tests/ui/while_let_loop.rs

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,19 @@ fn main() {
2222
break;
2323
}
2424

25+
loop {
26+
//~^ while_let_loop
27+
let Some(_x) = y else { break };
28+
}
29+
30+
loop {
31+
// no error, else branch does something other than break
32+
let Some(_x) = y else {
33+
let _z = 1;
34+
break;
35+
};
36+
}
37+
2538
loop {
2639
//~^ while_let_loop
2740

tests/ui/while_let_loop.stderr

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,15 @@ error: this loop could be written as a `while let` loop
1717
|
1818
LL | / loop {
1919
LL | |
20+
LL | | let Some(_x) = y else { break };
21+
LL | | }
22+
| |_____^ help: try: `while let Some(_x) = y { .. }`
23+
24+
error: this loop could be written as a `while let` loop
25+
--> tests/ui/while_let_loop.rs:38:5
26+
|
27+
LL | / loop {
28+
LL | |
2029
LL | |
2130
LL | | match y {
2231
... |
@@ -25,7 +34,7 @@ LL | | }
2534
| |_____^ help: try: `while let Some(_x) = y { .. }`
2635

2736
error: this loop could be written as a `while let` loop
28-
--> tests/ui/while_let_loop.rs:34:5
37+
--> tests/ui/while_let_loop.rs:47:5
2938
|
3039
LL | / loop {
3140
LL | |
@@ -37,7 +46,7 @@ LL | | }
3746
| |_____^ help: try: `while let Some(x) = y { .. }`
3847

3948
error: this loop could be written as a `while let` loop
40-
--> tests/ui/while_let_loop.rs:45:5
49+
--> tests/ui/while_let_loop.rs:58:5
4150
|
4251
LL | / loop {
4352
LL | |
@@ -48,7 +57,7 @@ LL | | }
4857
| |_____^ help: try: `while let Some(x) = y { .. }`
4958

5059
error: this loop could be written as a `while let` loop
51-
--> tests/ui/while_let_loop.rs:77:5
60+
--> tests/ui/while_let_loop.rs:90:5
5261
|
5362
LL | / loop {
5463
LL | |
@@ -68,7 +77,7 @@ LL + }
6877
|
6978

7079
error: this loop could be written as a `while let` loop
71-
--> tests/ui/while_let_loop.rs:167:9
80+
--> tests/ui/while_let_loop.rs:180:9
7281
|
7382
LL | / loop {
7483
LL | |
@@ -88,7 +97,7 @@ LL + }
8897
|
8998

9099
error: this loop could be written as a `while let` loop
91-
--> tests/ui/while_let_loop.rs:182:5
100+
--> tests/ui/while_let_loop.rs:195:5
92101
|
93102
LL | / loop {
94103
LL | |
@@ -107,7 +116,7 @@ LL + }
107116
|
108117

109118
error: this loop could be written as a `while let` loop
110-
--> tests/ui/while_let_loop.rs:194:5
119+
--> tests/ui/while_let_loop.rs:207:5
111120
|
112121
LL | / loop {
113122
LL | |
@@ -126,7 +135,7 @@ LL + }
126135
|
127136

128137
error: this loop could be written as a `while let` loop
129-
--> tests/ui/while_let_loop.rs:206:5
138+
--> tests/ui/while_let_loop.rs:219:5
130139
|
131140
LL | / loop {
132141
LL | |
@@ -137,7 +146,7 @@ LL | | }
137146
| |_____^ help: try: `while let Some(x) = Some(3) { .. }`
138147

139148
error: this loop could be written as a `while let` loop
140-
--> tests/ui/while_let_loop.rs:218:5
149+
--> tests/ui/while_let_loop.rs:231:5
141150
|
142151
LL | / loop {
143152
LL | |
@@ -156,7 +165,7 @@ LL + }
156165
|
157166

158167
error: this loop could be written as a `while let` loop
159-
--> tests/ui/while_let_loop.rs:230:5
168+
--> tests/ui/while_let_loop.rs:243:5
160169
|
161170
LL | / loop {
162171
LL | |
@@ -177,5 +186,5 @@ LL + ..
177186
LL + }
178187
|
179188

180-
error: aborting due to 11 previous errors
189+
error: aborting due to 12 previous errors
181190

0 commit comments

Comments
 (0)