Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions clippy_lints/src/loops/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -861,6 +861,7 @@ impl<'tcx> LateLintPass<'tcx> for Loops {
// check for `loop { if let {} else break }` that could be `while let`
// (also matches an explicit "match" instead of "if let")
// (even if the "match" or "if let" is used for declaration)
// (also matches on `let {} else break`)
if let ExprKind::Loop(block, label, LoopSource::Loop, _) = expr.kind {
// also check for empty `loop {}` statements, skipping those in #[panic_handler]
empty_loop::check(cx, expr, block);
Expand Down
30 changes: 21 additions & 9 deletions clippy_lints/src/loops/while_let_loop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,19 +10,19 @@ use rustc_hir::{Block, Expr, ExprKind, LetStmt, MatchSource, Pat, PatKind, Path,
use rustc_lint::LateContext;

pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, loop_block: &'tcx Block<'_>) {
let (init, let_info) = match (loop_block.stmts, loop_block.expr) {
let (init, let_info, els) = match (loop_block.stmts, loop_block.expr) {
([stmt, ..], _) => match stmt.kind {
StmtKind::Let(LetStmt {
init: Some(e),
els: None,
els,
pat,
ty,
..
}) => (*e, Some((*pat, *ty))),
StmtKind::Semi(e) | StmtKind::Expr(e) => (e, None),
}) => (*e, Some((*pat, *ty)), *els),
StmtKind::Semi(e) | StmtKind::Expr(e) => (e, None, None),
_ => return,
},
([], Some(e)) => (e, None),
([], Some(e)) => (e, None, None),
_ => return,
};
let has_trailing_exprs = loop_block.stmts.len() + usize::from(loop_block.expr.is_some()) > 1;
Expand All @@ -38,14 +38,26 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, loop_blo
if_let.let_expr,
has_trailing_exprs,
let_info,
if_let.if_then,
Some(if_let.if_then),
);
} else if els.and_then(|x| x.expr).is_some_and(is_simple_break_expr)
&& let Some((pat, _)) = let_info
{
could_be_while_let(cx, expr, pat, init, has_trailing_exprs, let_info, None);
} else if let ExprKind::Match(scrutinee, [arm1, arm2], MatchSource::Normal) = init.kind
&& arm1.guard.is_none()
&& arm2.guard.is_none()
&& is_simple_break_expr(arm2.body)
{
could_be_while_let(cx, expr, arm1.pat, scrutinee, has_trailing_exprs, let_info, arm1.body);
could_be_while_let(
cx,
expr,
arm1.pat,
scrutinee,
has_trailing_exprs,
let_info,
Some(arm1.body),
);
}
}

Expand All @@ -70,7 +82,7 @@ fn could_be_while_let<'tcx>(
let_expr: &'tcx Expr<'_>,
has_trailing_exprs: bool,
let_info: Option<(&Pat<'_>, Option<&Ty<'_>>)>,
inner_expr: &Expr<'_>,
inner_expr: Option<&Expr<'_>>,
) {
if has_trailing_exprs
&& (needs_ordered_drop(cx, cx.typeck_results().expr_ty(let_expr))
Expand All @@ -85,7 +97,7 @@ fn could_be_while_let<'tcx>(
// 1) it was ugly with big bodies;
// 2) it was not indented properly;
// 3) it wasn’t very smart (see #675).
let inner_content = if let Some((pat, ty)) = let_info
let inner_content = if let Some(((pat, ty), inner_expr)) = let_info.zip(inner_expr)
// Prevent trivial reassignments such as `let x = x;` or `let _ = …;`, but
// keep them if the type has been explicitly specified.
&& (!is_trivial_assignment(pat, peel_blocks(inner_expr)) || ty.is_some())
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/infinite_loops.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
//@no-rustfix: multiple suggestions add `-> !` to the same fn
//@aux-build:proc_macros.rs

#![allow(clippy::never_loop)]
#![allow(clippy::never_loop, clippy::while_let_loop)]
#![warn(clippy::infinite_loop)]

extern crate proc_macros;
Expand Down
13 changes: 13 additions & 0 deletions tests/ui/while_let_loop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,19 @@ fn main() {
break;
}

loop {
//~^ while_let_loop
let Some(_x) = y else { break };
}

loop {
// no error, else branch does something other than break
let Some(_x) = y else {
let _z = 1;
break;
};
}

loop {
//~^ while_let_loop

Expand Down
29 changes: 19 additions & 10 deletions tests/ui/while_let_loop.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,15 @@ error: this loop could be written as a `while let` loop
|
LL | / loop {
LL | |
LL | | let Some(_x) = y else { break };
LL | | }
| |_____^ help: try: `while let Some(_x) = y { .. }`

error: this loop could be written as a `while let` loop
--> tests/ui/while_let_loop.rs:38:5
|
LL | / loop {
LL | |
LL | |
LL | | match y {
... |
Expand All @@ -25,7 +34,7 @@ LL | | }
| |_____^ help: try: `while let Some(_x) = y { .. }`

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

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

error: this loop could be written as a `while let` loop
--> tests/ui/while_let_loop.rs:77:5
--> tests/ui/while_let_loop.rs:90:5
|
LL | / loop {
LL | |
Expand All @@ -68,7 +77,7 @@ LL + }
|

error: this loop could be written as a `while let` loop
--> tests/ui/while_let_loop.rs:167:9
--> tests/ui/while_let_loop.rs:180:9
|
LL | / loop {
LL | |
Expand All @@ -88,7 +97,7 @@ LL + }
|

error: this loop could be written as a `while let` loop
--> tests/ui/while_let_loop.rs:182:5
--> tests/ui/while_let_loop.rs:195:5
|
LL | / loop {
LL | |
Expand All @@ -107,7 +116,7 @@ LL + }
|

error: this loop could be written as a `while let` loop
--> tests/ui/while_let_loop.rs:194:5
--> tests/ui/while_let_loop.rs:207:5
|
LL | / loop {
LL | |
Expand All @@ -126,7 +135,7 @@ LL + }
|

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

error: this loop could be written as a `while let` loop
--> tests/ui/while_let_loop.rs:218:5
--> tests/ui/while_let_loop.rs:231:5
|
LL | / loop {
LL | |
Expand All @@ -156,7 +165,7 @@ LL + }
|

error: this loop could be written as a `while let` loop
--> tests/ui/while_let_loop.rs:230:5
--> tests/ui/while_let_loop.rs:243:5
|
LL | / loop {
LL | |
Expand All @@ -177,5 +186,5 @@ LL + ..
LL + }
|

error: aborting due to 11 previous errors
error: aborting due to 12 previous errors