Auto merge of #42649 - estebank:if-cond, r=nikomatsakis

Report error for assignment in `if` condition

For code like `if x = 3 {}`, output:

```
error[E0308]: mismatched types
  --> $DIR/issue-17283.rs:25:8
   |
25 |     if x = x {
   |        ^^^^^
   |        |
   |        help: did you mean to compare equality? `x == x`
   |        expected bool, found ()
   |
   = note: expected type `bool`
              found type `()`
```

Fix #40926.

Author: bors

Diff for: src/librustc_typeck/check/_match.rs

@@ -413,7 +413,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
             // discriminant. This is sort of a workaround, see note (*) in
             // `check_pat` for some details.
             discrim_ty = self.next_ty_var(TypeVariableOrigin::TypeInference(discrim.span));
-            self.check_expr_has_type(discrim, discrim_ty);
+            self.check_expr_has_type_or_error(discrim, discrim_ty);
         };
 
         // If the discriminant diverges, the match is pointless (e.g.,
@@ -480,7 +480,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
         for (i, (arm, pats_diverge)) in arms.iter().zip(all_arm_pats_diverge).enumerate() {
             if let Some(ref e) = arm.guard {
                 self.diverges.set(pats_diverge);
-                self.check_expr_has_type(e, tcx.types.bool);
+                self.check_expr_has_type_or_error(e, tcx.types.bool);
             }
 
             self.diverges.set(pats_diverge);

Diff for: src/librustc_typeck/check/demand.rs

@@ -26,13 +26,21 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
     // Requires that the two types unify, and prints an error message if
     // they don't.
     pub fn demand_suptype(&self, sp: Span, expected: Ty<'tcx>, actual: Ty<'tcx>) {
+        self.demand_suptype_diag(sp, expected, actual).map(|mut e| e.emit());
+    }
+
+    pub fn demand_suptype_diag(&self,
+                               sp: Span,
+                               expected: Ty<'tcx>,
+                               actual: Ty<'tcx>) -> Option<DiagnosticBuilder<'tcx>> {
         let cause = &self.misc(sp);
         match self.at(cause, self.param_env).sup(expected, actual) {
             Ok(InferOk { obligations, value: () }) => {
                 self.register_predicates(obligations);
+                None
             },
             Err(e) => {
-                self.report_mismatched_types(&cause, expected, actual, e).emit();
+                Some(self.report_mismatched_types(&cause, expected, actual, e))
             }
         }
     }

Diff for: src/librustc_typeck/check/mod.rs

@@ -232,6 +232,9 @@ pub enum Expectation<'tcx> {
     /// We know nothing about what type this expression should have.
     NoExpectation,
 
+    /// This expression is an `if` condition, it must resolve to `bool`.
+    ExpectIfCondition,
+
     /// This expression should have the type given (or some subtype)
     ExpectHasType(Ty<'tcx>),
 
@@ -310,9 +313,8 @@ impl<'a, 'gcx, 'tcx> Expectation<'tcx> {
     // no constraints yet present), just returns `None`.
     fn resolve(self, fcx: &FnCtxt<'a, 'gcx, 'tcx>) -> Expectation<'tcx> {
         match self {
-            NoExpectation => {
-                NoExpectation
-            }
+            NoExpectation => NoExpectation,
+            ExpectIfCondition => ExpectIfCondition,
             ExpectCastableToType(t) => {
                 ExpectCastableToType(fcx.resolve_type_vars_if_possible(&t))
             }
@@ -328,6 +330,7 @@ impl<'a, 'gcx, 'tcx> Expectation<'tcx> {
     fn to_option(self, fcx: &FnCtxt<'a, 'gcx, 'tcx>) -> Option<Ty<'tcx>> {
         match self.resolve(fcx) {
             NoExpectation => None,
+            ExpectIfCondition => Some(fcx.tcx.types.bool),
             ExpectCastableToType(ty) |
             ExpectHasType(ty) |
             ExpectRvalueLikeUnsized(ty) => Some(ty),
@@ -341,7 +344,8 @@ impl<'a, 'gcx, 'tcx> Expectation<'tcx> {
     fn only_has_type(self, fcx: &FnCtxt<'a, 'gcx, 'tcx>) -> Option<Ty<'tcx>> {
         match self.resolve(fcx) {
             ExpectHasType(ty) => Some(ty),
-            _ => None
+            ExpectIfCondition => Some(fcx.tcx.types.bool),
+            NoExpectation | ExpectCastableToType(_) | ExpectRvalueLikeUnsized(_) => None,
         }
     }
 
@@ -2644,10 +2648,17 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
         self.demand_eqtype(expr.span, expected, ty);
     }
 
-    pub fn check_expr_has_type(&self,
-                               expr: &'gcx hir::Expr,
-                               expected: Ty<'tcx>) -> Ty<'tcx> {
-        let mut ty = self.check_expr_with_hint(expr, expected);
+    pub fn check_expr_has_type_or_error(&self,
+                                        expr: &'gcx hir::Expr,
+                                        expected: Ty<'tcx>) -> Ty<'tcx> {
+        self.check_expr_meets_expectation_or_error(expr, ExpectHasType(expected))
+    }
+
+    fn check_expr_meets_expectation_or_error(&self,
+                                             expr: &'gcx hir::Expr,
+                                             expected: Expectation<'tcx>) -> Ty<'tcx> {
+        let expected_ty = expected.to_option(&self).unwrap_or(self.tcx.types.bool);
+        let mut ty = self.check_expr_with_expectation(expr, expected);
 
         // While we don't allow *arbitrary* coercions here, we *do* allow
         // coercions from ! to `expected`.
@@ -2663,7 +2674,24 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
             ty = adj_ty;
         }
 
-        self.demand_suptype(expr.span, expected, ty);
+        if let Some(mut err) = self.demand_suptype_diag(expr.span, expected_ty, ty) {
+            // Add help to type error if this is an `if` condition with an assignment
+            match (expected, &expr.node) {
+                (ExpectIfCondition, &hir::ExprAssign(ref lhs, ref rhs)) => {
+                    let msg = "did you mean to compare equality?";
+                    if let (Ok(left), Ok(right)) = (
+                        self.tcx.sess.codemap().span_to_snippet(lhs.span),
+                        self.tcx.sess.codemap().span_to_snippet(rhs.span))
+                    {
+                        err.span_suggestion(expr.span, msg, format!("{} == {}", left, right));
+                    } else {
+                        err.help(msg);
+                    }
+                }
+                _ => (),
+            }
+            err.emit();
+        }
         ty
     }
 
@@ -2838,7 +2866,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
                        opt_else_expr: Option<&'gcx hir::Expr>,
                        sp: Span,
                        expected: Expectation<'tcx>) -> Ty<'tcx> {
-        let cond_ty = self.check_expr_has_type(cond_expr, self.tcx.types.bool);
+        let cond_ty = self.check_expr_meets_expectation_or_error(cond_expr, ExpectIfCondition);
         let cond_diverges = self.diverges.get();
         self.diverges.set(Diverges::Maybe);
 
@@ -3325,7 +3353,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
         self.check_expr_struct_fields(struct_ty, expected, expr.id, path_span, variant, fields,
                                       base_expr.is_none());
         if let &Some(ref base_expr) = base_expr {
-            self.check_expr_has_type(base_expr, struct_ty);
+            self.check_expr_has_type_or_error(base_expr, struct_ty);
             match struct_ty.sty {
                 ty::TyAdt(adt, substs) if adt.is_struct() => {
                     let fru_field_types = adt.struct_variant().fields.iter().map(|f| {
@@ -3638,19 +3666,28 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
           hir::ExprAssign(ref lhs, ref rhs) => {
             let lhs_ty = self.check_expr_with_lvalue_pref(&lhs, PreferMutLvalue);
 
-            let tcx = self.tcx;
-            if !tcx.expr_is_lval(&lhs) {
-                struct_span_err!(
-                    tcx.sess, expr.span, E0070,
-                    "invalid left-hand side expression")
-                .span_label(
-                    expr.span,
-                    "left-hand of expression not valid")
-                .emit();
-            }
-
             let rhs_ty = self.check_expr_coercable_to_type(&rhs, lhs_ty);
 
+            match expected {
+                ExpectIfCondition => {
+                    self.tcx.sess.delay_span_bug(lhs.span, "invalid lhs expression in if;\
+                                                            expected error elsehwere");
+                }
+                _ => {
+                    // Only check this if not in an `if` condition, as the
+                    // mistyped comparison help is more appropriate.
+                    if !self.tcx.expr_is_lval(&lhs) {
+                        struct_span_err!(
+                            self.tcx.sess, expr.span, E0070,
+                            "invalid left-hand side expression")
+                        .span_label(
+                            expr.span,
+                            "left-hand of expression not valid")
+                        .emit();
+                    }
+                }
+            }
+
             self.require_type_is_sized(lhs_ty, lhs.span, traits::AssignmentLhsSized);
 
             if lhs_ty.references_error() || rhs_ty.references_error() {
@@ -3671,7 +3708,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
               };
 
               self.with_breakable_ctxt(expr.id, ctxt, || {
-                  self.check_expr_has_type(&cond, tcx.types.bool);
+                  self.check_expr_has_type_or_error(&cond, tcx.types.bool);
                   let cond_diverging = self.diverges.get();
                   self.check_block_no_value(&body);
 
@@ -3809,7 +3846,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
                 }
                 None => {
                     let t: Ty = self.next_ty_var(TypeVariableOrigin::MiscVariable(element.span));
-                    let element_ty = self.check_expr_has_type(&element, t);
+                    let element_ty = self.check_expr_has_type_or_error(&element, t);
                     (element_ty, t)
                 }
             };
@@ -4060,7 +4097,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
             }
             hir::StmtExpr(ref expr, _) => {
                 // Check with expected type of ()
-                self.check_expr_has_type(&expr, self.tcx.mk_nil());
+                self.check_expr_has_type_or_error(&expr, self.tcx.mk_nil());
             }
             hir::StmtSemi(ref expr, _) => {
                 self.check_expr(&expr);

Diff for: src/test/compile-fail/issue-17283.rs renamed to src/test/ui/type-check/assignment-in-if.rs

@@ -24,25 +24,29 @@ fn main() {
     // `x { ... }` should not be interpreted as a struct literal here
     if x = x {
         //~^ ERROR mismatched types
-        //~| expected type `bool`
-        //~| found type `()`
-        //~| expected bool, found ()
+        //~| HELP did you mean to compare equality?
         println!("{}", x);
     }
     // Explicit parentheses on the left should match behavior of above
     if (x = x) {
         //~^ ERROR mismatched types
-        //~| expected type `bool`
-        //~| found type `()`
-        //~| expected bool, found ()
+        //~| HELP did you mean to compare equality?
         println!("{}", x);
     }
     // The struct literal interpretation is fine with explicit parentheses on the right
     if y = (Foo { foo: x }) {
         //~^ ERROR mismatched types
-        //~| expected type `bool`
-        //~| found type `()`
-        //~| expected bool, found ()
+        //~| HELP did you mean to compare equality?
+        println!("{}", x);
+    }
+    // "invalid left-hand side expression" error is suppresed
+    if 3 = x {
+        //~^ ERROR mismatched types
+        //~| HELP did you mean to compare equality?
+        println!("{}", x);
+    }
+    if (if true { x = 4 } else { x = 5 }) {
+        //~^ ERROR mismatched types
         println!("{}", x);
     }
 }

Diff for: src/test/ui/type-check/assignment-in-if.stderr

@@ -0,0 +1,59 @@
+error[E0308]: mismatched types
+  --> $DIR/assignment-in-if.rs:25:8
+   |
+25 |     if x = x {
+   |        ^^^^^
+   |        |
+   |        help: did you mean to compare equality? `x == x`
+   |        expected bool, found ()
+   |
+   = note: expected type `bool`
+              found type `()`
+
+error[E0308]: mismatched types
+  --> $DIR/assignment-in-if.rs:31:8
+   |
+31 |     if (x = x) {
+   |        ^^^^^^^
+   |        |
+   |        help: did you mean to compare equality? `x == x`
+   |        expected bool, found ()
+   |
+   = note: expected type `bool`
+              found type `()`
+
+error[E0308]: mismatched types
+  --> $DIR/assignment-in-if.rs:37:8
+   |
+37 |     if y = (Foo { foo: x }) {
+   |        ^^^^^^^^^^^^^^^^^^^^
+   |        |
+   |        help: did you mean to compare equality? `y == (Foo { foo: x })`
+   |        expected bool, found ()
+   |
+   = note: expected type `bool`
+              found type `()`
+
+error[E0308]: mismatched types
+  --> $DIR/assignment-in-if.rs:43:8
+   |
+43 |     if 3 = x {
+   |        ^^^^^
+   |        |
+   |        help: did you mean to compare equality? `3 == x`
+   |        expected bool, found ()
+   |
+   = note: expected type `bool`
+              found type `()`
+
+error[E0308]: mismatched types
+  --> $DIR/assignment-in-if.rs:48:8
+   |
+48 |     if (if true { x = 4 } else { x = 5 }) {
+   |        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected bool, found ()
+   |
+   = note: expected type `bool`
+              found type `()`
+
+error: aborting due to previous error(s)
+