Do not ignore statements before a break when simplifying loop (#16379)

The previous tests ignored statements in a block and only looked at the
final expression to determine if the block was made of a single `break`.

changelog: [`while_let_loop`]: do not ignore statements before a `break`
when simplifying loop

Fixes rust-lang/rust-clippy#16378

clippy_lints/src/loops/while_let_loop.rs

@@ -40,7 +40,7 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, loop_blo
             let_info,
             Some(if_let.if_then),
         );
-    } else if els.and_then(|x| x.expr).is_some_and(is_simple_break_expr)
+    } else if els.is_some_and(is_simple_break_block)
         && let Some((pat, _)) = let_info
     {
         could_be_while_let(cx, expr, pat, init, has_trailing_exprs, let_info, None);
@@ -61,17 +61,23 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, loop_blo
     }
 }
 
-/// Returns `true` if expr contains a single break expression without a label or sub-expression,
-/// possibly embedded in blocks.
-fn is_simple_break_expr(e: &Expr<'_>) -> bool {
-    if let ExprKind::Block(b, _) = e.kind {
-        match (b.stmts, b.expr) {
-            ([s], None) => matches!(s.kind, StmtKind::Expr(e) | StmtKind::Semi(e) if is_simple_break_expr(e)),
-            ([], Some(e)) => is_simple_break_expr(e),
-            _ => false,
-        }
-    } else {
-        matches!(e.kind, ExprKind::Break(dest, None) if dest.label.is_none())
+/// Checks if `block` contains a single unlabeled `break` expression or statement, possibly embedded
+/// inside other blocks.
+fn is_simple_break_block(block: &Block<'_>) -> bool {
+    match (block.stmts, block.expr) {
+        ([s], None) => matches!(s.kind, StmtKind::Expr(e) | StmtKind::Semi(e) if is_simple_break_expr(e)),
+        ([], Some(e)) => is_simple_break_expr(e),
+        _ => false,
+    }
+}
+
+/// Checks if `expr` contains a single unlabeled `break` expression or statement, possibly embedded
+/// inside other blocks.
+fn is_simple_break_expr(expr: &Expr<'_>) -> bool {
+    match expr.kind {
+        ExprKind::Block(b, _) => is_simple_break_block(b),
+        ExprKind::Break(dest, None) => dest.label.is_none(),
+        _ => false,
     }
 }
 

tests/ui/while_let_loop.rs

@@ -253,3 +253,24 @@ fn let_assign() {
         }
     }
 }
+
+fn issue16378() {
+    // This does not lint today because of the extra statement(s)
+    // before the `break`.
+    // TODO: When the `break` statement/expr in the `let`/`else` is the
+    // only way to leave the loop, the lint could trigger and move
+    // the statements preceeding the `break` after the loop, as in:
+    // ```rust
+    // while let Some(x) = std::hint::black_box(None::<i32>) {
+    //     println!("x = {x}");
+    // }
+    // println!("fail");
+    // ```
+    loop {
+        let Some(x) = std::hint::black_box(None::<i32>) else {
+            println!("fail");
+            break;
+        };
+        println!("x = {x}");
+    }
+}