diagnostics: fix borrowck suggestions for if/while let conditionals

This code detects the case where one of the borrows is inside the
let init expr while the other end is not. If that happens, we don't
want to suggest adding a semicolon, because it won't work.

3 files changed, 227 insertions, 18 deletions

diff --git a/compiler/rustc_borrowck/src/diagnostics/explain_borrow.rs b/compiler/rustc_borrowck/src/diagnostics/explain_borrow.rs
index 2656e0bb6a4..841cb782bc3 100644
--- a/compiler/rustc_borrowck/src/diagnostics/explain_borrow.rs
+++ b/compiler/rustc_borrowck/src/diagnostics/explain_borrow.rs
@@ -248,7 +248,98 @@ impl<'tcx> BorrowExplanation<'tcx> {
                         );
                         err.span_label(body.source_info(drop_loc).span, message);
 
-                        if let LocalInfo::BlockTailTemp(info) = local_decl.local_info() {
+                        struct FindLetExpr<'hir> {
+                            span: Span,
+                            result: Option<(Span, &'hir hir::Pat<'hir>, &'hir hir::Expr<'hir>)>,
+                            tcx: TyCtxt<'hir>,
+                        }
+
+                        impl<'hir> rustc_hir::intravisit::Visitor<'hir> for FindLetExpr<'hir> {
+                            type NestedFilter = rustc_middle::hir::nested_filter::OnlyBodies;
+                            fn nested_visit_map(&mut self) -> Self::Map {
+                                self.tcx.hir()
+                            }
+                            fn visit_expr(&mut self, expr: &'hir hir::Expr<'hir>) {
+                                if let hir::ExprKind::If(cond, _conseq, _alt)
+                                | hir::ExprKind::Loop(
+                                    hir::Block {
+                                        expr:
+                                            Some(&hir::Expr {
+                                                kind: hir::ExprKind::If(cond, _conseq, _alt),
+                                                ..
+                                            }),
+                                        ..
+                                    },
+                                    _,
+                                    hir::LoopSource::While,
+                                    _,
+                                ) = expr.kind
+                                    && let hir::ExprKind::Let(hir::LetExpr {
+                                        init: let_expr_init,
+                                        span: let_expr_span,
+                                        pat: let_expr_pat,
+                                        ..
+                                    }) = cond.kind
+                                    && let_expr_init.span.contains(self.span)
+                                {
+                                    self.result =
+                                        Some((*let_expr_span, let_expr_pat, let_expr_init))
+                                } else {
+                                    hir::intravisit::walk_expr(self, expr);
+                                }
+                            }
+                        }
+
+                        if let &LocalInfo::IfThenRescopeTemp { if_then } = local_decl.local_info()
+                            && let hir::Node::Expr(expr) = tcx.hir_node(if_then)
+                            && let hir::ExprKind::If(cond, conseq, alt) = expr.kind
+                            && let hir::ExprKind::Let(&hir::LetExpr {
+                                span: _,
+                                pat,
+                                init,
+                                // FIXME(#101728): enable rewrite when type ascription is
+                                // stabilized again.
+                                ty: None,
+                                recovered: _,
+                            }) = cond.kind
+                            && pat.span.can_be_used_for_suggestions()
+                            && let Ok(pat) = tcx.sess.source_map().span_to_snippet(pat.span)
+                        {
+                            suggest_rewrite_if_let(tcx, expr, &pat, init, conseq, alt, err);
+                        } else if let Some((old, new)) = multiple_borrow_span
+                            && let def_id = body.source.def_id()
+                            && let Some(node) = tcx.hir().get_if_local(def_id)
+                            && let Some(body_id) = node.body_id()
+                            && let hir_body = tcx.hir().body(body_id)
+                            && let mut expr_finder = (FindLetExpr { span: old, result: None, tcx })
+                            && let Some((let_expr_span, let_expr_pat, let_expr_init)) = {
+                                expr_finder.visit_expr(hir_body.value);
+                                expr_finder.result
+                            }
+                            && !let_expr_span.contains(new)
+                        {
+                            // #133941: The `old` expression is at the conditional part of an
+                            // if/while let expression. Adding a semicolon won't work.
+                            // Instead, try suggesting the `matches!` macro or a temporary.
+                            if let_expr_pat
+                                .walk_short(|pat| !matches!(pat.kind, hir::PatKind::Binding(..)))
+                            {
+                                if let Ok(pat_snippet) =
+                                    tcx.sess.source_map().span_to_snippet(let_expr_pat.span)
+                                    && let Ok(init_snippet) =
+                                        tcx.sess.source_map().span_to_snippet(let_expr_init.span)
+                                {
+                                    err.span_suggestion_verbose(
+                                        let_expr_span,
+                                        "consider using the `matches!` macro",
+                                        format!("matches!({init_snippet}, {pat_snippet})"),
+                                        Applicability::MaybeIncorrect,
+                                    );
+                                } else {
+                                    err.note("consider using the `matches!` macro");
+                                }
+                            }
+                        } else if let LocalInfo::BlockTailTemp(info) = local_decl.local_info() {
                             if info.tail_result_is_ignored {
                                 // #85581: If the first mutable borrow's scope contains
                                 // the second borrow, this suggestion isn't helpful.
@@ -281,23 +372,6 @@ impl<'tcx> BorrowExplanation<'tcx> {
                                     Applicability::MaybeIncorrect,
                                 );
                             };
-                        } else if let &LocalInfo::IfThenRescopeTemp { if_then } =
-                            local_decl.local_info()
-                            && let hir::Node::Expr(expr) = tcx.hir_node(if_then)
-                            && let hir::ExprKind::If(cond, conseq, alt) = expr.kind
-                            && let hir::ExprKind::Let(&hir::LetExpr {
-                                span: _,
-                                pat,
-                                init,
-                                // FIXME(#101728): enable rewrite when type ascription is
-                                // stabilized again.
-                                ty: None,
-                                recovered: _,
-                            }) = cond.kind
-                            && pat.span.can_be_used_for_suggestions()
-                            && let Ok(pat) = tcx.sess.source_map().span_to_snippet(pat.span)
-                        {
-                            suggest_rewrite_if_let(tcx, expr, &pat, init, conseq, alt, err);
                         }
                     }
                 }
diff --git a/tests/ui/borrowck/already-borrowed-as-mutable-if-let-133941.rs b/tests/ui/borrowck/already-borrowed-as-mutable-if-let-133941.rs
new file mode 100644
index 00000000000..934aac8383d
--- /dev/null
+++ b/tests/ui/borrowck/already-borrowed-as-mutable-if-let-133941.rs
@@ -0,0 +1,46 @@
+// https://github.com/rust-lang/rust/issues/133941
+use std::marker::PhantomData;
+
+struct Bar<'a>(PhantomData<&'a mut i32>);
+
+impl<'a> Drop for Bar<'a> {
+    fn drop(&mut self) {}
+}
+
+struct Foo();
+
+impl Foo {
+    fn f(&mut self) -> Option<Bar<'_>> {
+        None
+    }
+
+    fn g(&mut self) {}
+}
+
+fn main() {
+    let mut foo = Foo();
+    while let Some(_) = foo.f() {
+        //~^ HELP matches!
+        foo.g();
+        //~^ ERROR [E0499]
+    }
+    if let Some(_) = foo.f() {
+        //~^ HELP matches!
+        foo.g();
+        //~^ ERROR [E0499]
+    }
+    while let Some(_x) = foo.f() {
+        foo.g();
+        //~^ ERROR [E0499]
+    }
+    if let Some(_x) = foo.f() {
+        foo.g();
+        //~^ ERROR [E0499]
+    }
+    while let Some(_x) = {let _x = foo.f(); foo.g(); None::<()>} {
+        //~^ ERROR [E0499]
+    }
+    if let Some(_x) = {let _x = foo.f(); foo.g(); None::<()>} {
+        //~^ ERROR [E0499]
+    }
+}
diff --git a/tests/ui/borrowck/already-borrowed-as-mutable-if-let-133941.stderr b/tests/ui/borrowck/already-borrowed-as-mutable-if-let-133941.stderr
new file mode 100644
index 00000000000..bb21caccbaf
--- /dev/null
+++ b/tests/ui/borrowck/already-borrowed-as-mutable-if-let-133941.stderr
@@ -0,0 +1,89 @@
+error[E0499]: cannot borrow `foo` as mutable more than once at a time
+  --> $DIR/already-borrowed-as-mutable-if-let-133941.rs:24:9
+   |
+LL |     while let Some(_) = foo.f() {
+   |                         -------
+   |                         |
+   |                         first mutable borrow occurs here
+   |                         a temporary with access to the first borrow is created here ...
+LL |
+LL |         foo.g();
+   |         ^^^ second mutable borrow occurs here
+LL |
+LL |     }
+   |     - ... and the first borrow might be used here, when that temporary is dropped and runs the destructor for type `Option<Bar<'_>>`
+   |
+help: consider using the `matches!` macro
+   |
+LL |     while matches!(foo.f(), Some(_)) {
+   |           ~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+error[E0499]: cannot borrow `foo` as mutable more than once at a time
+  --> $DIR/already-borrowed-as-mutable-if-let-133941.rs:29:9
+   |
+LL |     if let Some(_) = foo.f() {
+   |                      -------
+   |                      |
+   |                      first mutable borrow occurs here
+   |                      a temporary with access to the first borrow is created here ...
+LL |
+LL |         foo.g();
+   |         ^^^ second mutable borrow occurs here
+LL |
+LL |     }
+   |     - ... and the first borrow might be used here, when that temporary is dropped and runs the destructor for type `Option<Bar<'_>>`
+   |
+help: consider using the `matches!` macro
+   |
+LL |     if matches!(foo.f(), Some(_)) {
+   |        ~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+error[E0499]: cannot borrow `foo` as mutable more than once at a time
+  --> $DIR/already-borrowed-as-mutable-if-let-133941.rs:33:9
+   |
+LL |     while let Some(_x) = foo.f() {
+   |                          -------
+   |                          |
+   |                          first mutable borrow occurs here
+   |                          a temporary with access to the first borrow is created here ...
+LL |         foo.g();
+   |         ^^^ second mutable borrow occurs here
+LL |
+LL |     }
+   |     - ... and the first borrow might be used here, when that temporary is dropped and runs the destructor for type `Option<Bar<'_>>`
+
+error[E0499]: cannot borrow `foo` as mutable more than once at a time
+  --> $DIR/already-borrowed-as-mutable-if-let-133941.rs:37:9
+   |
+LL |     if let Some(_x) = foo.f() {
+   |                       -------
+   |                       |
+   |                       first mutable borrow occurs here
+   |                       a temporary with access to the first borrow is created here ...
+LL |         foo.g();
+   |         ^^^ second mutable borrow occurs here
+LL |
+LL |     }
+   |     - ... and the first borrow might be used here, when that temporary is dropped and runs the destructor for type `Option<Bar<'_>>`
+
+error[E0499]: cannot borrow `foo` as mutable more than once at a time
+  --> $DIR/already-borrowed-as-mutable-if-let-133941.rs:40:45
+   |
+LL |     while let Some(_x) = {let _x = foo.f(); foo.g(); None::<()>} {
+   |                                    ---      ^^^                - first borrow might be used here, when `_x` is dropped and runs the destructor for type `Option<Bar<'_>>`
+   |                                    |        |
+   |                                    |        second mutable borrow occurs here
+   |                                    first mutable borrow occurs here
+
+error[E0499]: cannot borrow `foo` as mutable more than once at a time
+  --> $DIR/already-borrowed-as-mutable-if-let-133941.rs:43:42
+   |
+LL |     if let Some(_x) = {let _x = foo.f(); foo.g(); None::<()>} {
+   |                                 ---      ^^^                - first borrow might be used here, when `_x` is dropped and runs the destructor for type `Option<Bar<'_>>`
+   |                                 |        |
+   |                                 |        second mutable borrow occurs here
+   |                                 first mutable borrow occurs here
+
+error: aborting due to 6 previous errors
+
+For more information about this error, try `rustc --explain E0499`.