On type mismatch caused by assignment, point at assignee

* Do not emit unnecessary E0308 after E0070
* Show fewer errors on `while let` missing `let`
* Hide redundant E0308 on `while let` missing `let`
* Point at binding definition when possible on invalid assignment
* do not point at closure twice
* do not suggest `if let` for literals in lhs
* account for parameter types

Author: estebank

compiler/rustc_ast_lowering/src/expr.rs

@@ -915,14 +915,22 @@ impl<'hir> LoweringContext<'_, 'hir> {
             );
         }
         if !self.sess.features_untracked().destructuring_assignment {
-            feature_err(
+            let mut err = feature_err(
                 &self.sess.parse_sess,
                 sym::destructuring_assignment,
                 eq_sign_span,
                 "destructuring assignments are unstable",
-            )
-            .span_label(lhs.span, "cannot assign to this expression")
-            .emit();
+            );
+            err.span_label(lhs.span, "cannot assign to this expression");
+            if self.is_in_loop_condition {
+                err.span_suggestion_verbose(
+                    lhs.span.shrink_to_lo(),
+                    "you might have meant to use pattern destructuring",
+                    "let ".to_string(),
+                    rustc_errors::Applicability::MachineApplicable,
+                );
+            }
+            err.emit();
         }
 
         let mut assignments = vec![];

compiler/rustc_typeck/src/check/coercion.rs

@@ -1458,7 +1458,7 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {
                             cause,
                             expected,
                             found,
-                            coercion_error,
+                            coercion_error.clone(),
                             fcx,
                             parent_id,
                             expression.map(|expr| (expr, blk_id)),
@@ -1472,7 +1472,7 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {
                             cause,
                             expected,
                             found,
-                            coercion_error,
+                            coercion_error.clone(),
                             fcx,
                             id,
                             None,
@@ -1483,7 +1483,12 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {
                         }
                     }
                     _ => {
-                        err = fcx.report_mismatched_types(cause, expected, found, coercion_error);
+                        err = fcx.report_mismatched_types(
+                            cause,
+                            expected,
+                            found,
+                            coercion_error.clone(),
+                        );
                     }
                 }
 
@@ -1492,7 +1497,14 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {
                 }
 
                 if let Some(expr) = expression {
-                    fcx.emit_coerce_suggestions(&mut err, expr, found, expected, None);
+                    fcx.emit_coerce_suggestions(
+                        &mut err,
+                        expr,
+                        found,
+                        expected,
+                        None,
+                        coercion_error,
+                    );
                 }
 
                 err.emit_unless(unsized_return);

compiler/rustc_typeck/src/check/demand.rs

@@ -10,6 +10,7 @@ use rustc_hir::lang_items::LangItem;
 use rustc_hir::{is_range_literal, Node};
 use rustc_middle::lint::in_external_macro;
 use rustc_middle::ty::adjustment::AllowTwoPhase;
+use rustc_middle::ty::error::{ExpectedFound, TypeError};
 use rustc_middle::ty::print::with_no_trimmed_paths;
 use rustc_middle::ty::{self, AssocItem, Ty, TypeAndMut};
 use rustc_span::symbol::sym;
@@ -27,8 +28,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         expr_ty: Ty<'tcx>,
         expected: Ty<'tcx>,
         expected_ty_expr: Option<&'tcx hir::Expr<'tcx>>,
+        error: TypeError<'tcx>,
     ) {
-        self.annotate_expected_due_to_let_ty(err, expr);
+        self.annotate_expected_due_to_let_ty(err, expr, error);
         self.suggest_box_deref(err, expr, expected, expr_ty);
         self.suggest_compatible_variants(err, expr, expected, expr_ty);
         self.suggest_deref_ref_or_into(err, expr, expected, expr_ty, expected_ty_expr);
@@ -145,9 +147,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         let expr = expr.peel_drop_temps();
         let cause = self.misc(expr.span);
         let expr_ty = self.resolve_vars_with_obligations(checked_ty);
-        let mut err = self.report_mismatched_types(&cause, expected, expr_ty, e);
+        let mut err = self.report_mismatched_types(&cause, expected, expr_ty, e.clone());
 
-        self.emit_coerce_suggestions(&mut err, expr, expr_ty, expected, expected_ty_expr);
+        self.emit_coerce_suggestions(&mut err, expr, expr_ty, expected, expected_ty_expr, e);
 
         (expected, Some(err))
     }
@@ -156,15 +158,80 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         &self,
         err: &mut DiagnosticBuilder<'_>,
         expr: &hir::Expr<'_>,
+        error: TypeError<'_>,
     ) {
         let parent = self.tcx.hir().get_parent_node(expr.hir_id);
-        if let Some(hir::Node::Local(hir::Local { ty: Some(ty), init: Some(init), .. })) =
-            self.tcx.hir().find(parent)
-        {
-            if init.hir_id == expr.hir_id {
+        match (self.tcx.hir().find(parent), error) {
+            (Some(hir::Node::Local(hir::Local { ty: Some(ty), init: Some(init), .. })), _)
+                if init.hir_id == expr.hir_id =>
+            {
                 // Point at `let` assignment type.
                 err.span_label(ty.span, "expected due to this");
             }
+            (
+                Some(hir::Node::Expr(hir::Expr {
+                    kind: hir::ExprKind::Assign(lhs, rhs, _), ..
+                })),
+                TypeError::Sorts(ExpectedFound { expected, .. }),
+            ) if rhs.hir_id == expr.hir_id && !expected.is_closure() => {
+                // We ignore closures explicitly because we already point at them elsewhere.
+                // Point at the assigned-to binding.
+                let mut primary_span = lhs.span;
+                let mut secondary_span = lhs.span;
+                let mut post_message = "";
+                if let hir::ExprKind::Path(hir::QPath::Resolved(
+                    None,
+                    hir::Path { res: hir::def::Res::Local(hir_id), .. },
+                )) = lhs.kind
+                {
+                    if let Some(hir::Node::Binding(pat)) = self.tcx.hir().find(*hir_id) {
+                        let parent = self.tcx.hir().get_parent_node(pat.hir_id);
+                        primary_span = pat.span;
+                        secondary_span = pat.span;
+                        match self.tcx.hir().find(parent) {
+                            Some(hir::Node::Local(hir::Local { ty: Some(ty), .. })) => {
+                                primary_span = ty.span;
+                                post_message = " type";
+                            }
+                            Some(hir::Node::Local(hir::Local { init: Some(init), .. })) => {
+                                primary_span = init.span;
+                                post_message = " value";
+                            }
+                            Some(hir::Node::Param(hir::Param { ty_span, .. })) => {
+                                primary_span = *ty_span;
+                                post_message = " parameter type";
+                            }
+                            _ => {}
+                        }
+                    }
+                }
+
+                if primary_span != secondary_span
+                    && self
+                        .tcx
+                        .sess
+                        .source_map()
+                        .is_multiline(secondary_span.shrink_to_hi().until(primary_span))
+                {
+                    // We are pointing at the binding's type or initializer value, but it's pattern
+                    // is in a different line, so we point at both.
+                    err.span_label(secondary_span, "expected due to the type of this binding");
+                    err.span_label(primary_span, &format!("expected due to this{}", post_message));
+                } else if post_message == "" {
+                    // We are pointing at either the assignment lhs or the binding def pattern.
+                    err.span_label(primary_span, "expected due to the type of this binding");
+                } else {
+                    // We are pointing at the binding's type or initializer value.
+                    err.span_label(primary_span, &format!("expected due to this{}", post_message));
+                }
+
+                if !lhs.is_syntactic_place_expr() {
+                    // We already emitted E0070 "invalid left-hand side of assignment", so we
+                    // silence this.
+                    err.delay_as_bug();
+                }
+            }
+            _ => {}
         }
     }
 

compiler/rustc_typeck/src/check/expr.rs

@@ -833,19 +833,66 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         &self,
         lhs: &'tcx hir::Expr<'tcx>,
         err_code: &'static str,
-        expr_span: &Span,
+        op_span: Span,
     ) {
         if lhs.is_syntactic_place_expr() {
             return;
         }
 
         // FIXME: Make this use SessionDiagnostic once error codes can be dynamically set.
         let mut err = self.tcx.sess.struct_span_err_with_code(
-            *expr_span,
+            op_span,
             "invalid left-hand side of assignment",
             DiagnosticId::Error(err_code.into()),
         );
         err.span_label(lhs.span, "cannot assign to this expression");
+
+        let mut parent = self.tcx.hir().get_parent_node(lhs.hir_id);
+        while let Some(node) = self.tcx.hir().find(parent) {
+            match node {
+                hir::Node::Expr(hir::Expr {
+                    kind:
+                        hir::ExprKind::Loop(
+                            hir::Block {
+                                expr:
+                                    Some(hir::Expr {
+                                        kind:
+                                            hir::ExprKind::Match(expr, ..) | hir::ExprKind::If(expr, ..),
+                                        ..
+                                    }),
+                                ..
+                            },
+                            _,
+                            hir::LoopSource::While,
+                            _,
+                        ),
+                    ..
+                }) => {
+                    // We have a situation like `while Some(0) = value.get(0) {`, where `while let`
+                    // was more likely intended.
+                    err.span_suggestion_verbose(
+                        expr.span.shrink_to_lo(),
+                        "you might have meant to use pattern destructuring",
+                        "let ".to_string(),
+                        Applicability::MachineApplicable,
+                    );
+                    if !self.sess().features_untracked().destructuring_assignment {
+                        // We already emit an E0658 with a suggestion for `while let`, this is
+                        // redundant output.
+                        err.delay_as_bug();
+                    }
+                    break;
+                }
+                hir::Node::Item(_)
+                | hir::Node::ImplItem(_)
+                | hir::Node::TraitItem(_)
+                | hir::Node::Crate(_) => break,
+                _ => {
+                    parent = self.tcx.hir().get_parent_node(parent);
+                }
+            }
+        }
+
         err.emit();
     }
 
@@ -953,7 +1000,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             } else {
                 (Applicability::MaybeIncorrect, false)
             };
-            if !lhs.is_syntactic_place_expr() {
+            if !lhs.is_syntactic_place_expr() && !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 { .. }, .. }) =
@@ -965,7 +1012,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                         "let ".to_string(),
                         applicability,
                     );
-                }
+                };
             }
             if eq {
                 err.span_suggestion_verbose(
@@ -986,7 +1033,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             return self.tcx.ty_error();
         }
 
-        self.check_lhs_assignable(lhs, "E0070", span);
+        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));

compiler/rustc_typeck/src/check/fn_ctxt/checks.rs

@@ -740,6 +740,24 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                         &mut |err| {
                             if let Some(expected_ty) = expected.only_has_type(self) {
                                 self.consider_hint_about_removing_semicolon(blk, expected_ty, err);
+                                if expected_ty == self.tcx.types.bool {
+                                    // If this is caused by a missing `let` in a `while let`,
+                                    // silence this redundant error, as we already emit E0070.
+                                    let parent = self.tcx.hir().get_parent_node(blk.hir_id);
+                                    let parent = self.tcx.hir().get_parent_node(parent);
+                                    let parent = self.tcx.hir().get_parent_node(parent);
+                                    let parent = self.tcx.hir().get_parent_node(parent);
+                                    let parent = self.tcx.hir().get_parent_node(parent);
+                                    match self.tcx.hir().find(parent) {
+                                        Some(hir::Node::Expr(hir::Expr {
+                                            kind: hir::ExprKind::Loop(_, _, hir::LoopSource::While, _),
+                                            ..
+                                        })) => {
+                                            err.delay_as_bug();
+                                        }
+                                        _ => {}
+                                    }
+                                }
                             }
                             if let Some(fn_span) = fn_span {
                                 err.span_label(

compiler/rustc_typeck/src/check/op.rs

@@ -42,7 +42,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                 return_ty
             };
 
-        self.check_lhs_assignable(lhs, "E0067", &op.span);
+        self.check_lhs_assignable(lhs, "E0067", op.span);
 
         ty
     }

src/test/ui/dst/dst-bad-assign-3.stderr

@@ -2,7 +2,9 @@ error[E0308]: mismatched types
   --> $DIR/dst-bad-assign-3.rs:33:12
    |
 LL |     f5.2 = Bar1 {f: 36};
-   |            ^^^^^^^^^^^^ expected trait object `dyn ToBar`, found struct `Bar1`
+   |     ----   ^^^^^^^^^^^^ expected trait object `dyn ToBar`, found struct `Bar1`
+   |     |
+   |     expected due to the type of this binding
    |
    = note: expected trait object `dyn ToBar`
                     found struct `Bar1`

src/test/ui/dst/dst-bad-assign.stderr

@@ -2,7 +2,9 @@ error[E0308]: mismatched types
   --> $DIR/dst-bad-assign.rs:35:14
    |
 LL |     f5.ptr = Bar1 {f: 36};
-   |              ^^^^^^^^^^^^ expected trait object `dyn ToBar`, found struct `Bar1`
+   |     ------   ^^^^^^^^^^^^ expected trait object `dyn ToBar`, found struct `Bar1`
+   |     |
+   |     expected due to the type of this binding
    |
    = note: expected trait object `dyn ToBar`
                     found struct `Bar1`

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

@@ -6,7 +6,6 @@ fn some_function() {
     SOME_CONST = 14; //~ ERROR E0070
     1 = 3; //~ ERROR E0070
     some_other_func() = 4; //~ ERROR E0070
-                           //~^ ERROR E0308
 }
 
 fn main() {

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

@@ -22,13 +22,6 @@ 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
+error: aborting due to 3 previous errors
 
-Some errors have detailed explanations: E0070, E0308.
-For more information about an error, try `rustc --explain E0070`.
+For more information about this error, try `rustc --explain E0070`.

src/test/ui/fully-qualified-type/fully-qualified-type-name1.rs

@@ -1,10 +1,10 @@
 // Test that we use fully-qualified type names in error messages.
 
 fn main() {
-    let x: Option<usize>;
+    let x: //~ NOTE expected due to the type of this binding
+        Option<usize>; //~ NOTE expected due to this type
     x = 5;
     //~^ ERROR mismatched types
-    //~| expected enum `Option<usize>`
-    //~| found type `{integer}`
-    //~| expected enum `Option`, found integer
+    //~| NOTE expected enum `Option<usize>`
+    //~| NOTE expected enum `Option`, found integer
 }

src/test/ui/fully-qualified-type/fully-qualified-type-name1.stderr

@@ -1,6 +1,10 @@
 error[E0308]: mismatched types
-  --> $DIR/fully-qualified-type-name1.rs:5:9
+  --> $DIR/fully-qualified-type-name1.rs:6:9
    |
+LL |     let x:
+   |         - expected due to the type of this binding
+LL |         Option<usize>;
+   |         ------------- expected due to this type
 LL |     x = 5;
    |         ^ expected enum `Option`, found integer
    |