Rollup merge of #59439 - Centril:generalize-assign-to-bool-diagnostic, r=oli-obk

Generalize diagnostic for `x = y` where `bool` is the expected type

Extracted out of #59288.

Currently we special case a diagnostic for `if x = y { ...` since the expected type is `bool` in this case and we instead suggest `if x == y`. This PR generalizes this such that given an expression of form `x = y` (`ExprKind::Assign(..)`) where the expected type is `bool`, we emit a suggestion `x == y`.

r? @oli-obk

Let's do a perf run to make sure this was not the source of regressions in #59288.

Author: cuviper

src/librustc_typeck/check/coercion.rs

@@ -1233,7 +1233,12 @@ impl<'gcx, 'tcx, 'exprs, E> CoerceMany<'gcx, 'tcx, 'exprs, E>
                     augment_error(&mut db);
                 }
 
-                db.emit();
+                if expression.filter(|e| fcx.is_assign_to_bool(e, expected)).is_some() {
+                    // Error reported in `check_assign` so avoid emitting error again.
+                    db.delay_as_bug();
+                } else {
+                    db.emit();
+                }
 
                 self.final_ty = Some(fcx.tcx.types.err);
             }

src/librustc_typeck/check/demand.rs

@@ -119,44 +119,65 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
         let expr_ty = self.resolve_type_vars_with_obligations(checked_ty);
         let mut err = self.report_mismatched_types(&cause, expected, expr_ty, e);
 
-        // If the expected type is an enum (Issue #55250) with any variants whose
-        // sole field is of the found type, suggest such variants. (Issue #42764)
-        if let ty::Adt(expected_adt, substs) = expected.sty {
-            if expected_adt.is_enum() {
-                let mut compatible_variants = expected_adt.variants
-                    .iter()
-                    .filter(|variant| variant.fields.len() == 1)
-                    .filter_map(|variant| {
-                        let sole_field = &variant.fields[0];
-                        let sole_field_ty = sole_field.ty(self.tcx, substs);
-                        if self.can_coerce(expr_ty, sole_field_ty) {
-                            let variant_path = self.tcx.def_path_str(variant.def_id);
-                            // FIXME #56861: DRYer prelude filtering
-                            Some(variant_path.trim_start_matches("std::prelude::v1::").to_string())
-                        } else {
-                            None
-                        }
-                    }).peekable();
-
-                if compatible_variants.peek().is_some() {
-                    let expr_text = print::to_string(print::NO_ANN, |s| s.print_expr(expr));
-                    let suggestions = compatible_variants
-                        .map(|v| format!("{}({})", v, expr_text));
-                    err.span_suggestions(
-                        expr.span,
-                        "try using a variant of the expected type",
-                        suggestions,
-                        Applicability::MaybeIncorrect,
-                    );
-                }
-            }
+        if self.is_assign_to_bool(expr, expected) {
+            // Error reported in `check_assign` so avoid emitting error again.
+            err.delay_as_bug();
+            return (expected, None)
         }
 
+        self.suggest_compatible_variants(&mut err, expr, expected, expr_ty);
         self.suggest_ref_or_into(&mut err, expr, expected, expr_ty);
 
         (expected, Some(err))
     }
 
+    /// Returns whether the expected type is `bool` and the expression is `x = y`.
+    pub fn is_assign_to_bool(&self, expr: &hir::Expr, expected: Ty<'tcx>) -> bool {
+        if let hir::ExprKind::Assign(..) = expr.node {
+            return expected == self.tcx.types.bool;
+        }
+        false
+    }
+
+    /// If the expected type is an enum (Issue #55250) with any variants whose
+    /// sole field is of the found type, suggest such variants. (Issue #42764)
+    fn suggest_compatible_variants(
+        &self,
+        err: &mut DiagnosticBuilder<'_>,
+        expr: &hir::Expr,
+        expected: Ty<'tcx>,
+        expr_ty: Ty<'tcx>,
+    ) {
+        if let ty::Adt(expected_adt, substs) = expected.sty {
+            if !expected_adt.is_enum() {
+                return;
+            }
+
+            let mut compatible_variants = expected_adt.variants
+                .iter()
+                .filter(|variant| variant.fields.len() == 1)
+                .filter_map(|variant| {
+                    let sole_field = &variant.fields[0];
+                    let sole_field_ty = sole_field.ty(self.tcx, substs);
+                    if self.can_coerce(expr_ty, sole_field_ty) {
+                        let variant_path = self.tcx.def_path_str(variant.def_id);
+                        // FIXME #56861: DRYer prelude filtering
+                        Some(variant_path.trim_start_matches("std::prelude::v1::").to_string())
+                    } else {
+                        None
+                    }
+                }).peekable();
+
+            if compatible_variants.peek().is_some() {
+                let expr_text = print::to_string(print::NO_ANN, |s| s.print_expr(expr));
+                let suggestions = compatible_variants
+                    .map(|v| format!("{}({})", v, expr_text));
+                let msg = "try using a variant of the expected type";
+                err.span_suggestions(expr.span, msg, suggestions, Applicability::MaybeIncorrect);
+            }
+        }
+    }
+
     pub fn get_conversion_methods(&self, span: Span, expected: Ty<'tcx>, checked_ty: Ty<'tcx>)
                               -> Vec<AssociatedItem> {
         let mut methods = self.probe_for_return_type(span,

src/librustc_typeck/check/mod.rs

@@ -246,9 +246,6 @@ 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>),
 
@@ -328,7 +325,6 @@ impl<'a, 'gcx, 'tcx> Expectation<'tcx> {
     fn resolve(self, fcx: &FnCtxt<'a, 'gcx, 'tcx>) -> Expectation<'tcx> {
         match self {
             NoExpectation => NoExpectation,
-            ExpectIfCondition => ExpectIfCondition,
             ExpectCastableToType(t) => {
                 ExpectCastableToType(fcx.resolve_type_vars_if_possible(&t))
             }
@@ -344,7 +340,6 @@ 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),
@@ -358,7 +353,6 @@ 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),
-            ExpectIfCondition => Some(fcx.tcx.types.bool),
             NoExpectation | ExpectCastableToType(_) | ExpectRvalueLikeUnsized(_) => None,
         }
     }
@@ -3148,25 +3142,13 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
         }
 
         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.
-            if let (ExpectIfCondition, &ExprKind::Assign(ref lhs, ref rhs))
-                 = (expected, &expr.node)
-            {
-                let msg = "try comparing for equality";
-                if let (Ok(left), Ok(right)) = (
-                    self.tcx.sess.source_map().span_to_snippet(lhs.span),
-                    self.tcx.sess.source_map().span_to_snippet(rhs.span))
-                {
-                    err.span_suggestion(
-                        expr.span,
-                        msg,
-                        format!("{} == {}", left, right),
-                        Applicability::MaybeIncorrect);
-                } else {
-                    err.help(msg);
-                }
+            if self.is_assign_to_bool(expr, expected_ty) {
+                // Error reported in `check_assign` so avoid emitting error again.
+                // FIXME(centril): Consider removing if/when `if` desugars to `match`.
+                err.delay_as_bug();
+            } else {
+                err.emit();
             }
-            err.emit();
         }
         ty
     }
@@ -3337,7 +3319,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_meets_expectation_or_error(cond_expr, ExpectIfCondition);
+        let cond_ty = self.check_expr_has_type_or_error(cond_expr, self.tcx.types.bool);
         let cond_diverges = self.diverges.get();
         self.diverges.set(Diverges::Maybe);
 
@@ -4422,34 +4404,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
                 tcx.types.never
             }
             ExprKind::Assign(ref lhs, ref rhs) => {
-                let lhs_ty = self.check_expr_with_needs(&lhs, Needs::MutPlace);
-
-                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 !lhs.is_place_expr() {
-                            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() {
-                    tcx.types.err
-                } else {
-                    tcx.mk_unit()
-                }
+                self.check_assign(expr, expected, lhs, rhs)
             }
             ExprKind::If(ref cond, ref then_expr, ref opt_else_expr) => {
                 self.check_then_else(&cond, then_expr, opt_else_expr.as_ref().map(|e| &**e),
@@ -4750,6 +4705,51 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
         }
     }
 
+    /// Type check assignment expression `expr` of form `lhs = rhs`.
+    /// The expected type is `()` and is passsed to the function for the purposes of diagnostics.
+    fn check_assign(
+        &self,
+        expr: &'gcx hir::Expr,
+        expected: Expectation<'tcx>,
+        lhs: &'gcx hir::Expr,
+        rhs: &'gcx hir::Expr,
+    ) -> 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);
+
+        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);
+            } else {
+                err.help(msg);
+            }
+            err.emit();
+        } else if !lhs.is_place_expr() {
+            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() {
+            self.tcx.types.err
+        } else {
+            self.tcx.mk_unit()
+        }
+    }
+
     // Finish resolving a path in a struct expression or pattern `S::A { .. }` if necessary.
     // The newly resolved definition is written into `type_dependent_defs`.
     fn finish_resolving_struct_path(&self,

src/test/ui/type/type-check/assignment-expected-bool.rs

@@ -0,0 +1,34 @@
+// The purpose of this text is to ensure that we get good
+// diagnostics when a `bool` is expected but that due to
+// an assignment expression `x = y` the type is `()`.
+
+fn main() {
+    let _: bool = 0 = 0; //~ ERROR mismatched types [E0308]
+
+    let _: bool = match 0 {
+        0 => 0 = 0, //~ ERROR mismatched types [E0308]
+        _ => 0 = 0, //~ ERROR mismatched types [E0308]
+    };
+
+    let _: bool = match true {
+        true => 0 = 0, //~ ERROR mismatched types [E0308]
+        _ => (),
+    };
+
+    if 0 = 0 {} //~ ERROR mismatched types [E0308]
+
+    let _: bool = if { 0 = 0 } { //~ ERROR mismatched types [E0308]
+        0 = 0 //~ ERROR mismatched types [E0308]
+    } else {
+        0 = 0 //~ ERROR mismatched types [E0308]
+    };
+
+    let _ = (0 = 0) //~ ERROR mismatched types [E0308]
+        && { 0 = 0 } //~ ERROR mismatched types [E0308]
+        || (0 = 0); //~ ERROR mismatched types [E0308]
+
+    // A test to check that not expecting `bool` behaves well:
+    let _: usize = 0 = 0;
+    //~^ ERROR mismatched types [E0308]
+    //~| ERROR invalid left-hand side expression [E0070]
+}