Auto merge of rust-lang#75931 - estebank:suggest-if-let, r=petrochenkov

Suggest `if let x = y` when encountering `if x = y`

Detect potential cases where `if let` was meant but `let` was left out.

Fix rust-lang#44990.

Author: bors

compiler/rustc_resolve/src/late.rs

@@ -378,6 +378,9 @@ struct DiagnosticMetadata<'ast> {
 
     /// Only used for better errors on `let <pat>: <expr, not type>;`.
     current_let_binding: Option<(Span, Option<Span>, Option<Span>)>,
+
+    /// Used to detect possible `if let` written without `let` and to provide structured suggestion.
+    in_if_condition: Option<&'ast Expr>,
 }
 
 struct LateResolutionVisitor<'a, 'b, 'ast> {
@@ -403,7 +406,7 @@ struct LateResolutionVisitor<'a, 'b, 'ast> {
     ///
     /// In particular, rustdoc uses this to avoid giving errors for `cfg()` items.
     /// In most cases this will be `None`, in which case errors will always be reported.
-    /// If it is `Some(_)`, then it will be updated when entering a nested function or trait body.
+    /// If it is `true`, then it will be updated when entering a nested function or trait body.
     in_func_body: bool,
 }
 
@@ -2199,7 +2202,9 @@ impl<'a, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> {
 
             ExprKind::If(ref cond, ref then, ref opt_else) => {
                 self.with_rib(ValueNS, NormalRibKind, |this| {
+                    let old = this.diagnostic_metadata.in_if_condition.replace(cond);
                     this.visit_expr(cond);
+                    this.diagnostic_metadata.in_if_condition = old;
                     this.visit_block(then);
                 });
                 if let Some(expr) = opt_else {

compiler/rustc_resolve/src/late/diagnostics.rs

@@ -176,6 +176,19 @@ impl<'a> LateResolutionVisitor<'a, '_, '_> {
         let code = source.error_code(res.is_some());
         let mut err = self.r.session.struct_span_err_with_code(base_span, &base_msg, code);
 
+        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,
+                );
+                self.r.session.if_let_suggestions.borrow_mut().insert(*span);
+            }
+            _ => {}
+        }
+
         let is_assoc_fn = self.self_type_is_available(span);
         // Emit help message for fake-self from other languages (e.g., `this` in Javascript).
         if ["this", "my"].contains(&&*item_str.as_str()) && is_assoc_fn {

compiler/rustc_session/src/session.rs

@@ -213,6 +213,9 @@ pub struct Session {
 
     known_attrs: Lock<MarkedAttrs>,
     used_attrs: Lock<MarkedAttrs>,
+
+    /// `Span`s for `if` conditions that we have suggested turning into `if let`.
+    pub if_let_suggestions: Lock<FxHashSet<Span>>,
 }
 
 pub struct PerfStats {
@@ -1354,6 +1357,7 @@ pub fn build_session(
         target_features: FxHashSet::default(),
         known_attrs: Lock::new(MarkedAttrs::new()),
         used_attrs: Lock::new(MarkedAttrs::new()),
+        if_let_suggestions: Default::default(),
     };
 
     validate_commandline_args_with_session_available(&sess);

compiler/rustc_typeck/src/check/expr.rs

@@ -764,30 +764,59 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         rhs: &'tcx hir::Expr<'tcx>,
         span: &Span,
     ) -> Ty<'tcx> {
-        let lhs_ty = self.check_expr_with_needs(&lhs, Needs::MutPlace);
-        let rhs_ty = self.check_expr_coercable_to_type(&rhs, lhs_ty, Some(lhs));
-
         let expected_ty = expected.coercion_target_type(self, expr.span);
         if expected_ty == self.tcx.types.bool {
             // The expected type is `bool` but this will result in `()` so we can reasonably
             // say that the user intended to write `lhs == rhs` instead of `lhs = rhs`.
             // The likely cause of this is `if foo = bar { .. }`.
             let actual_ty = self.tcx.mk_unit();
             let mut err = self.demand_suptype_diag(expr.span, expected_ty, actual_ty).unwrap();
-            let msg = "try comparing for equality";
-            let left = self.tcx.sess.source_map().span_to_snippet(lhs.span);
-            let right = self.tcx.sess.source_map().span_to_snippet(rhs.span);
-            if let (Ok(left), Ok(right)) = (left, right) {
-                let help = format!("{} == {}", left, right);
-                err.span_suggestion(expr.span, msg, help, Applicability::MaybeIncorrect);
+            let lhs_ty = self.check_expr(&lhs);
+            let rhs_ty = self.check_expr(&rhs);
+            if self.can_coerce(lhs_ty, rhs_ty) {
+                if !lhs.is_syntactic_place_expr() {
+                    // Do not suggest `if let x = y` as `==` is way more likely to be the intention.
+                    if let hir::Node::Expr(hir::Expr {
+                        kind: ExprKind::Match(_, _, hir::MatchSource::IfDesugar { .. }),
+                        ..
+                    }) = self.tcx.hir().get(
+                        self.tcx.hir().get_parent_node(self.tcx.hir().get_parent_node(expr.hir_id)),
+                    ) {
+                        // Likely `if let` intended.
+                        err.span_suggestion_verbose(
+                            expr.span.shrink_to_lo(),
+                            "you might have meant to use pattern matching",
+                            "let ".to_string(),
+                            Applicability::MaybeIncorrect,
+                        );
+                    }
+                }
+                err.span_suggestion_verbose(
+                    *span,
+                    "you might have meant to compare for equality",
+                    "==".to_string(),
+                    Applicability::MaybeIncorrect,
+                );
             } else {
-                err.help(msg);
+                // Do this to cause extra errors about the assignment.
+                let lhs_ty = self.check_expr_with_needs(&lhs, Needs::MutPlace);
+                let _ = self.check_expr_coercable_to_type(&rhs, lhs_ty, Some(lhs));
             }
-            err.emit();
-        } else {
-            self.check_lhs_assignable(lhs, "E0070", span);
+
+            if self.sess().if_let_suggestions.borrow().get(&expr.span).is_some() {
+                // We already emitted an `if let` suggestion due to an identifier not found.
+                err.delay_as_bug();
+            } else {
+                err.emit();
+            }
+            return self.tcx.ty_error();
         }
 
+        self.check_lhs_assignable(lhs, "E0070", span);
+
+        let lhs_ty = self.check_expr_with_needs(&lhs, Needs::MutPlace);
+        let rhs_ty = self.check_expr_coercable_to_type(&rhs, lhs_ty, Some(lhs));
+
         self.require_type_is_sized(lhs_ty, lhs.span, traits::AssignmentLhsSized);
 
         if lhs_ty.references_error() || rhs_ty.references_error() {

src/test/ui/error-codes/E0070.stderr

@@ -14,12 +14,6 @@ LL |     1 = 3;
    |     |
    |     cannot assign to this expression
 
-error[E0308]: mismatched types
-  --> $DIR/E0070.rs:8:25
-   |
-LL |     some_other_func() = 4;
-   |                         ^ expected `()`, found integer
-
 error[E0070]: invalid left-hand side of assignment
   --> $DIR/E0070.rs:8:23
    |
@@ -28,6 +22,12 @@ LL |     some_other_func() = 4;
    |     |
    |     cannot assign to this expression
 
+error[E0308]: mismatched types
+  --> $DIR/E0070.rs:8:25
+   |
+LL |     some_other_func() = 4;
+   |                         ^ expected `()`, found integer
+
 error: aborting due to 4 previous errors
 
 Some errors have detailed explanations: E0070, E0308.

src/test/ui/issues/issue-13407.stderr

@@ -10,12 +10,6 @@ note: the unit struct `C` is defined here
 LL |     struct C;
    |     ^^^^^^^^^
 
-error[E0308]: mismatched types
-  --> $DIR/issue-13407.rs:6:12
-   |
-LL |     A::C = 1;
-   |            ^ expected struct `A::C`, found integer
-
 error[E0070]: invalid left-hand side of assignment
   --> $DIR/issue-13407.rs:6:10
    |
@@ -24,6 +18,12 @@ LL |     A::C = 1;
    |     |
    |     cannot assign to this expression
 
+error[E0308]: mismatched types
+  --> $DIR/issue-13407.rs:6:12
+   |
+LL |     A::C = 1;
+   |            ^ expected struct `A::C`, found integer
+
 error: aborting due to 3 previous errors
 
 Some errors have detailed explanations: E0070, E0308, E0603.

src/test/ui/rfc-2497-if-let-chains/disallowed-positions.stderr

@@ -568,10 +568,12 @@ error[E0308]: mismatched types
   --> $DIR/disallowed-positions.rs:56:8
    |
 LL |     if x = let 0 = 0 {}
-   |        ^^^^^^^^^^^^^
-   |        |
-   |        expected `bool`, found `()`
-   |        help: try comparing for equality: `x == let 0 = 0`
+   |        ^^^^^^^^^^^^^ expected `bool`, found `()`
+   |
+help: you might have meant to compare for equality
+   |
+LL |     if x == let 0 = 0 {}
+   |          ^^
 
 error[E0308]: mismatched types
   --> $DIR/disallowed-positions.rs:59:8
@@ -754,10 +756,12 @@ error[E0308]: mismatched types
   --> $DIR/disallowed-positions.rs:120:11
    |
 LL |     while x = let 0 = 0 {}
-   |           ^^^^^^^^^^^^^
-   |           |
-   |           expected `bool`, found `()`
-   |           help: try comparing for equality: `x == let 0 = 0`
+   |           ^^^^^^^^^^^^^ expected `bool`, found `()`
+   |
+help: you might have meant to compare for equality
+   |
+LL |     while x == let 0 = 0 {}
+   |             ^^
 
 error[E0308]: mismatched types
   --> $DIR/disallowed-positions.rs:123:11

src/test/ui/suggestions/if-let-typo.rs

@@ -0,0 +1,9 @@
+fn main() {
+    let foo = Some(0);
+    let bar = None;
+    if Some(x) = foo {} //~ ERROR cannot find value `x` in this scope
+    if Some(foo) = bar {} //~ ERROR mismatched types
+    if 3 = foo {} //~ ERROR mismatched types
+    //~^ ERROR mismatched types
+    if Some(3) = foo {} //~ ERROR mismatched types
+}

src/test/ui/suggestions/if-let-typo.stderr

@@ -0,0 +1,60 @@
+error[E0425]: cannot find value `x` in this scope
+  --> $DIR/if-let-typo.rs:4:13
+   |
+LL |     if Some(x) = foo {}
+   |             ^ not found in this scope
+   |
+help: you might have meant to use pattern matching
+   |
+LL |     if let Some(x) = foo {}
+   |        ^^^
+
+error[E0308]: mismatched types
+  --> $DIR/if-let-typo.rs:5:8
+   |
+LL |     if Some(foo) = bar {}
+   |        ^^^^^^^^^^^^^^^ expected `bool`, found `()`
+   |
+help: you might have meant to use pattern matching
+   |
+LL |     if let Some(foo) = bar {}
+   |        ^^^
+help: you might have meant to compare for equality
+   |
+LL |     if Some(foo) == bar {}
+   |                  ^^
+
+error[E0308]: mismatched types
+  --> $DIR/if-let-typo.rs:6:12
+   |
+LL |     if 3 = foo {}
+   |            ^^^ expected integer, found enum `std::option::Option`
+   |
+   = note: expected type `{integer}`
+              found enum `std::option::Option<{integer}>`
+
+error[E0308]: mismatched types
+  --> $DIR/if-let-typo.rs:6:8
+   |
+LL |     if 3 = foo {}
+   |        ^^^^^^^ expected `bool`, found `()`
+
+error[E0308]: mismatched types
+  --> $DIR/if-let-typo.rs:8:8
+   |
+LL |     if Some(3) = foo {}
+   |        ^^^^^^^^^^^^^ expected `bool`, found `()`
+   |
+help: you might have meant to use pattern matching
+   |
+LL |     if let Some(3) = foo {}
+   |        ^^^
+help: you might have meant to compare for equality
+   |
+LL |     if Some(3) == foo {}
+   |                ^^
+
+error: aborting due to 5 previous errors
+
+Some errors have detailed explanations: E0308, E0425.
+For more information about an error, try `rustc --explain E0308`.