Rollup merge of #97856 - compiler-errors:bad-let-suggestions, r=estebank

Don't suggest adding `let` in certain `if` conditions

Avoid being too eager to suggest `let` in an `if` condition with an `=`, namely when the LHS of the `=` isn't even valid as a pattern (to a first degree approximation).

This heustic I came up with kinda sucks. Let me know if it needs to be refined.

Author: compiler-errors

compiler/rustc_ast/src/ast.rs

@@ -1278,6 +1278,22 @@ impl Expr {
             },
         )
     }
+
+    // To a first-order approximation, is this a pattern
+    pub fn is_approximately_pattern(&self) -> bool {
+        match &self.peel_parens().kind {
+            ExprKind::Box(_)
+            | ExprKind::Array(_)
+            | ExprKind::Call(_, _)
+            | ExprKind::Tup(_)
+            | ExprKind::Lit(_)
+            | ExprKind::Range(_, _, _)
+            | ExprKind::Underscore
+            | ExprKind::Path(_, _)
+            | ExprKind::Struct(_) => true,
+            _ => false,
+        }
+    }
 }
 
 /// Limit types of a range (inclusive or exclusive)

compiler/rustc_hir/src/hir.rs

@@ -1813,6 +1813,20 @@ impl Expr<'_> {
             | ExprKind::Err => true,
         }
     }
+
+    // To a first-order approximation, is this a pattern
+    pub fn is_approximately_pattern(&self) -> bool {
+        match &self.kind {
+            ExprKind::Box(_)
+            | ExprKind::Array(_)
+            | ExprKind::Call(..)
+            | ExprKind::Tup(_)
+            | ExprKind::Lit(_)
+            | ExprKind::Path(_)
+            | ExprKind::Struct(..) => true,
+            _ => false,
+        }
+    }
 }
 
 /// Checks if the specified expression is a built-in range literal.

compiler/rustc_resolve/src/late/diagnostics.rs

@@ -265,13 +265,21 @@ impl<'a: 'ast, 'ast> LateResolutionVisitor<'a, '_, 'ast> {
             );
         }
         match (source, self.diagnostic_metadata.in_if_condition) {
-            (PathSource::Expr(_), Some(Expr { span, kind: ExprKind::Assign(..), .. })) => {
-                err.span_suggestion_verbose(
-                    span.shrink_to_lo(),
-                    "you might have meant to use pattern matching",
-                    "let ".to_string(),
-                    Applicability::MaybeIncorrect,
-                );
+            (
+                PathSource::Expr(_),
+                Some(Expr { span: expr_span, kind: ExprKind::Assign(lhs, _, _), .. }),
+            ) => {
+                // Icky heuristic so we don't suggest:
+                // `if (i + 2) = 2` => `if let (i + 2) = 2` (approximately pattern)
+                // `if 2 = i` => `if let 2 = i` (lhs needs to contain error span)
+                if lhs.is_approximately_pattern() && lhs.span.contains(span) {
+                    err.span_suggestion_verbose(
+                        expr_span.shrink_to_lo(),
+                        "you might have meant to use pattern matching",
+                        "let ".to_string(),
+                        Applicability::MaybeIncorrect,
+                    );
+                }
             }
             _ => {}
         }

compiler/rustc_typeck/src/check/expr.rs

@@ -1035,7 +1035,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             } else {
                 (Applicability::MaybeIncorrect, false)
             };
-            if !lhs.is_syntactic_place_expr() && !matches!(lhs.kind, hir::ExprKind::Lit(_)) {
+            if !lhs.is_syntactic_place_expr()
+                && lhs.is_approximately_pattern()
+                && !matches!(lhs.kind, hir::ExprKind::Lit(_))
+            {
                 // Do not suggest `if let x = y` as `==` is way more likely to be the intention.
                 let hir = self.tcx.hir();
                 if let hir::Node::Expr(hir::Expr { kind: ExprKind::If { .. }, .. }) =

src/test/ui/expr/if/bad-if-let-suggestion.rs

@@ -0,0 +1,24 @@
+// FIXME(compiler-errors): This really should suggest `let` on the RHS of the
+// `&&` operator, but that's kinda hard to do because of precedence.
+// Instead, for now we just make sure not to suggest `if let let`.
+fn a() {
+    if let x = 1 && i = 2 {}
+    //~^ ERROR cannot find value `i` in this scope
+    //~| ERROR `let` expressions in this position are unstable
+    //~| ERROR mismatched types
+    //~| ERROR `let` expressions are not supported here
+}
+
+fn b() {
+    if (i + j) = i {}
+    //~^ ERROR cannot find value `i` in this scope
+    //~| ERROR cannot find value `i` in this scope
+    //~| ERROR cannot find value `j` in this scope
+}
+
+fn c() {
+    if x[0] = 1 {}
+    //~^ ERROR cannot find value `x` in this scope
+}
+
+fn main() {}

src/test/ui/expr/if/bad-if-let-suggestion.stderr

@@ -0,0 +1,69 @@
+error: `let` expressions are not supported here
+  --> $DIR/bad-if-let-suggestion.rs:5:8
+   |
+LL |     if let x = 1 && i = 2 {}
+   |        ^^^^^^^^^
+   |
+   = note: only supported directly in conditions of `if` and `while` expressions
+
+error[E0425]: cannot find value `i` in this scope
+  --> $DIR/bad-if-let-suggestion.rs:5:21
+   |
+LL |     if let x = 1 && i = 2 {}
+   |                     ^ not found in this scope
+
+error[E0425]: cannot find value `i` in this scope
+  --> $DIR/bad-if-let-suggestion.rs:13:9
+   |
+LL | fn a() {
+   | ------ similarly named function `a` defined here
+...
+LL |     if (i + j) = i {}
+   |         ^ help: a function with a similar name exists: `a`
+
+error[E0425]: cannot find value `j` in this scope
+  --> $DIR/bad-if-let-suggestion.rs:13:13
+   |
+LL | fn a() {
+   | ------ similarly named function `a` defined here
+...
+LL |     if (i + j) = i {}
+   |             ^ help: a function with a similar name exists: `a`
+
+error[E0425]: cannot find value `i` in this scope
+  --> $DIR/bad-if-let-suggestion.rs:13:18
+   |
+LL | fn a() {
+   | ------ similarly named function `a` defined here
+...
+LL |     if (i + j) = i {}
+   |                  ^ help: a function with a similar name exists: `a`
+
+error[E0425]: cannot find value `x` in this scope
+  --> $DIR/bad-if-let-suggestion.rs:20:8
+   |
+LL | fn a() {
+   | ------ similarly named function `a` defined here
+...
+LL |     if x[0] = 1 {}
+   |        ^ help: a function with a similar name exists: `a`
+
+error[E0658]: `let` expressions in this position are unstable
+  --> $DIR/bad-if-let-suggestion.rs:5:8
+   |
+LL |     if let x = 1 && i = 2 {}
+   |        ^^^^^^^^^
+   |
+   = note: see issue #53667 <https://github.com/rust-lang/rust/issues/53667> for more information
+   = help: add `#![feature(let_chains)]` to the crate attributes to enable
+
+error[E0308]: mismatched types
+  --> $DIR/bad-if-let-suggestion.rs:5:8
+   |
+LL |     if let x = 1 && i = 2 {}
+   |        ^^^^^^^^^^^^^^^^^^ expected `bool`, found `()`
+
+error: aborting due to 8 previous errors
+
+Some errors have detailed explanations: E0308, E0425, E0658.
+For more information about an error, try `rustc --explain E0308`.