Point at enclosing match when expecting () in arm

estebank · estebank · commit 8a167edbcad1 · 2019-09-28T09:56:28.000-07:00
When encountering code like the following: ```rust fn main() { match 3 { 4 => 1, 3 => { println!("Yep it maches."); 2 } _ => 2 } println!("Bye!") } ``` point at the enclosing `match` expression and suggest ignoring the returned value: ``` error[E0308]: mismatched types --> $DIR/match-needing-semi.rs:8:13 | LL | / match 3 { LL | | 4 => 1, LL | | 3 => { LL | | 2 | | ^ expected (), found integer LL | | } LL | | _ => 2 LL | | } | | -- help: consider using a semicolon here | |_____| | expected this to be `()` | = note: expected type `()` found type `{integer} ``` Fix #40799.
diff --git a/src/librustc/hir/lowering/expr.rs b/src/librustc/hir/lowering/expr.rs
@@ -1039,10 +1039,9 @@ impl LoweringContext<'_> {
     ) -> hir::Expr {
         // expand <head>
         let mut head = self.lower_expr(head);
-        let head_sp = head.span;
         let desugared_span = self.mark_span_with_reason(
             DesugaringKind::ForLoop,
-            head_sp,
+            head.span,
             None,
         );
         head.span = desugared_span;
@@ -1088,21 +1087,21 @@ impl LoweringContext<'_> {
 
         // `match ::std::iter::Iterator::next(&mut iter) { ... }`
         let match_expr = {
-            let iter = P(self.expr_ident(head_sp, iter, iter_pat_nid));
-            let ref_mut_iter = self.expr_mut_addr_of(head_sp, iter);
+            let iter = P(self.expr_ident(desugared_span, iter, iter_pat_nid));
+            let ref_mut_iter = self.expr_mut_addr_of(desugared_span, iter);
             let next_path = &[sym::iter, sym::Iterator, sym::next];
             let next_expr = P(self.expr_call_std_path(
-                head_sp,
+                desugared_span,
                 next_path,
                 hir_vec![ref_mut_iter],
             ));
             let arms = hir_vec![pat_arm, break_arm];
 
-            self.expr_match(head_sp, next_expr, arms, hir::MatchSource::ForLoopDesugar)
+            self.expr_match(desugared_span, next_expr, arms, hir::MatchSource::ForLoopDesugar)
         };
-        let match_stmt = self.stmt_expr(head_sp, match_expr);
+        let match_stmt = self.stmt_expr(desugared_span, match_expr);
 
-        let next_expr = P(self.expr_ident(head_sp, next_ident, next_pat_hid));
+        let next_expr = P(self.expr_ident(desugared_span, next_ident, next_pat_hid));
 
         // `let mut __next`
         let next_let = self.stmt_let_pat(
@@ -1117,7 +1116,7 @@ impl LoweringContext<'_> {
         let pat = self.lower_pat(pat);
         let pat_let = self.stmt_let_pat(
             ThinVec::new(),
-            head_sp,
+            desugared_span,
             Some(next_expr),
             pat,
             hir::LocalSource::ForLoopDesugar,
@@ -1154,14 +1153,14 @@ impl LoweringContext<'_> {
             let into_iter_path =
                 &[sym::iter, sym::IntoIterator, sym::into_iter];
             P(self.expr_call_std_path(
-                head_sp,
+                desugared_span,
                 into_iter_path,
                 hir_vec![head],
             ))
         };
 
         let match_expr = P(self.expr_match(
-            head_sp,
+            desugared_span,
             into_iter_expr,
             hir_vec![iter_arm],
             hir::MatchSource::ForLoopDesugar,
@@ -1173,7 +1172,7 @@ impl LoweringContext<'_> {
         // surrounding scope of the `match` since the `match` is not a terminating scope.
         //
         // Also, add the attributes to the outer returned expr node.
-        self.expr_drop_temps(head_sp, match_expr, e.attrs.clone())
+        self.expr_drop_temps(desugared_span, match_expr, e.attrs.clone())
     }
 
     /// Desugar `ExprKind::Try` from: `<expr>?` into:
diff --git a/src/librustc/hir/map/mod.rs b/src/librustc/hir/map/mod.rs
@@ -818,6 +818,27 @@ impl<'hir> Map<'hir> {
         CRATE_HIR_ID
     }
 
+    pub fn get_match_if_cause(&self, hir_id: HirId) -> Option<&Expr> {
+        for (_, node) in ParentHirIterator::new(hir_id, &self) {
+            match node {
+                Node::Item(_) |
+                Node::ForeignItem(_) |
+                Node::TraitItem(_) |
+                Node::ImplItem(_) => break,
+                Node::Expr(expr) => match expr.node {
+                    ExprKind::Match(_, _, _) => return Some(expr),
+                    _ => {}
+                },
+                Node::Stmt(stmt) => match stmt.node {
+                    StmtKind::Local(_) => break,
+                    _ => {}
+                }
+                _ => {}
+            }
+        }
+        None
+    }
+
     /// Returns the nearest enclosing scope. A scope is roughly an item or block.
     pub fn get_enclosing_scope(&self, hir_id: HirId) -> Option<HirId> {
         for (hir_id, node) in ParentHirIterator::new(hir_id, &self) {
diff --git a/src/librustc_typeck/check/_match.rs b/src/librustc_typeck/check/_match.rs
@@ -36,7 +36,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             // 2. By expecting `bool` for `expr` we get nice diagnostics for e.g. `if x = y { .. }`.
             //
             // FIXME(60707): Consider removing hack with principled solution.
-            self.check_expr_has_type_or_error(discrim, self.tcx.types.bool)
+            self.check_expr_has_type_or_error(discrim, self.tcx.types.bool, |_| {})
         } else {
             self.demand_discriminant_type(arms, discrim)
         };
@@ -106,7 +106,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             if let Some(g) = &arm.guard {
                 self.diverges.set(pats_diverge);
                 match g {
-                    hir::Guard::If(e) => self.check_expr_has_type_or_error(e, tcx.types.bool),
+                    hir::Guard::If(e) => {
+                        self.check_expr_has_type_or_error(e, tcx.types.bool, |_| {})
+                    }
                 };
             }
 
@@ -442,7 +444,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                 kind: TypeVariableOriginKind::TypeInference,
                 span: discrim.span,
             });
-            self.check_expr_has_type_or_error(discrim, discrim_ty);
+            self.check_expr_has_type_or_error(discrim, discrim_ty, |_| {});
             discrim_ty
         }
     }
diff --git a/src/librustc_typeck/check/coercion.rs b/src/librustc_typeck/check/coercion.rs
@@ -51,7 +51,7 @@
 //! we may want to adjust precisely when coercions occur.
 
 use crate::check::{FnCtxt, Needs};
-use errors::DiagnosticBuilder;
+use errors::{Applicability, DiagnosticBuilder};
 use rustc::hir;
 use rustc::hir::def_id::DefId;
 use rustc::hir::ptr::P;
@@ -1163,18 +1163,20 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {
                 fcx.try_coerce(expression, expression_ty, self.expected_ty, AllowTwoPhase::No)
             } else {
                 match self.expressions {
-                    Expressions::Dynamic(ref exprs) =>
-                        fcx.try_find_coercion_lub(cause,
-                                                  exprs,
-                                                  self.merged_ty(),
-                                                  expression,
-                                                  expression_ty),
-                    Expressions::UpFront(ref coercion_sites) =>
-                        fcx.try_find_coercion_lub(cause,
-                                                  &coercion_sites[0..self.pushed],
-                                                  self.merged_ty(),
-                                                  expression,
-                                                  expression_ty),
+                    Expressions::Dynamic(ref exprs) => fcx.try_find_coercion_lub(
+                        cause,
+                        exprs,
+                        self.merged_ty(),
+                        expression,
+                        expression_ty,
+                    ),
+                    Expressions::UpFront(ref coercion_sites) => fcx.try_find_coercion_lub(
+                        cause,
+                        &coercion_sites[0..self.pushed],
+                        self.merged_ty(),
+                        expression,
+                        expression_ty,
+                    ),
                 }
             }
         } else {
@@ -1302,6 +1304,21 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {
                 blk_id,
             );
             let parent = fcx.tcx.hir().get(parent_id);
+            if let (Some(match_expr), true, false) = (
+                fcx.tcx.hir().get_match_if_cause(expr.hir_id),
+                expected.is_unit(),
+                pointing_at_return_type,
+            ) {
+                if match_expr.span.desugaring_kind().is_none() {
+                    db.span_label(match_expr.span, "expected this to be `()`");
+                    db.span_suggestion_short(
+                        match_expr.span.shrink_to_hi(),
+                        "consider using a semicolon here",
+                        ";".to_string(),
+                        Applicability::MachineApplicable,
+                    );
+                }
+            }
             fcx.get_node_fn_decl(parent).map(|(fn_decl, _, is_main)| (fn_decl, is_main))
         } else {
             fcx.get_fn_decl(parent_id)
diff --git a/src/librustc_typeck/check/expr.rs b/src/librustc_typeck/check/expr.rs
@@ -53,14 +53,16 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         &self,
         expr: &'tcx hir::Expr,
         expected: Ty<'tcx>,
+        extend_err: impl Fn(&mut DiagnosticBuilder<'_>),
     ) -> Ty<'tcx> {
-        self.check_expr_meets_expectation_or_error(expr, ExpectHasType(expected))
+        self.check_expr_meets_expectation_or_error(expr, ExpectHasType(expected), extend_err)
     }
 
     fn check_expr_meets_expectation_or_error(
         &self,
         expr: &'tcx hir::Expr,
         expected: Expectation<'tcx>,
+        extend_err: impl Fn(&mut DiagnosticBuilder<'_>),
     ) -> 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);
@@ -88,6 +90,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                 ExprKind::DropTemps(expr) => expr,
                 _ => expr,
             };
+            extend_err(&mut err);
             // Error possibly reported in `check_assign` so avoid emitting error again.
             err.emit_unless(self.is_assign_to_bool(expr, expected_ty));
         }
@@ -971,7 +974,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                     kind: TypeVariableOriginKind::MiscVariable,
                     span: element.span,
                 });
-                let element_ty = self.check_expr_has_type_or_error(&element, ty);
+                let element_ty = self.check_expr_has_type_or_error(&element, ty, |_| {});
                 (element_ty, ty)
             }
         };
@@ -1058,7 +1061,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             // the fields with the base_expr. This could cause us to hit errors later
             // when certain fields are assumed to exist that in fact do not.
             if !error_happened {
-                self.check_expr_has_type_or_error(base_expr, adt_ty);
+                self.check_expr_has_type_or_error(base_expr, adt_ty, |_| {});
                 match adt_ty.kind {
                     ty::Adt(adt, substs) if adt.is_struct() => {
                         let fru_field_types = adt.non_enum_variant().fields.iter().map(|f| {
diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs
@@ -3881,7 +3881,15 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             hir::StmtKind::Item(_) => {}
             hir::StmtKind::Expr(ref expr) => {
                 // Check with expected type of `()`.
-                self.check_expr_has_type_or_error(&expr, self.tcx.mk_unit());
+
+                self.check_expr_has_type_or_error(&expr, self.tcx.mk_unit(), |err| {
+                    err.span_suggestion_short(
+                        expr.span.shrink_to_hi(),
+                        "consider using a semicolon here",
+                        ";".to_string(),
+                        Applicability::MachineApplicable,
+                    );
+                });
             }
             hir::StmtKind::Semi(ref expr) => {
                 self.check_expr(&expr);
diff --git a/src/test/ui/struct-literal-variant-in-if.stderr b/src/test/ui/struct-literal-variant-in-if.stderr
@@ -50,7 +50,10 @@ error[E0308]: mismatched types
   --> $DIR/struct-literal-variant-in-if.rs:10:20
    |
 LL |     if x == E::V { field } {}
-   |                    ^^^^^ expected (), found bool
+   |     ---------------^^^^^--- help: consider using a semicolon here
+   |     |              |
+   |     |              expected (), found bool
+   |     expected this to be `()`
    |
    = note: expected type `()`
               found type `bool`
diff --git a/src/test/ui/suggestions/match-needing-semi.fixed b/src/test/ui/suggestions/match-needing-semi.fixed
@@ -0,0 +1,18 @@
+// check-fail
+// run-rustfix
+
+fn main() {
+    match 3 {
+        4 => 1,
+        3 => {
+            2 //~ ERROR mismatched types
+        }
+        _ => 2
+    };
+    match 3 { //~ ERROR mismatched types
+        4 => 1,
+        3 => 2,
+        _ => 2
+    };
+    let _ = ();
+}
diff --git a/src/test/ui/suggestions/match-needing-semi.rs b/src/test/ui/suggestions/match-needing-semi.rs
@@ -0,0 +1,18 @@
+// check-fail
+// run-rustfix
+
+fn main() {
+    match 3 {
+        4 => 1,
+        3 => {
+            2 //~ ERROR mismatched types
+        }
+        _ => 2
+    }
+    match 3 { //~ ERROR mismatched types
+        4 => 1,
+        3 => 2,
+        _ => 2
+    }
+    let _ = ();
+}
diff --git a/src/test/ui/suggestions/match-needing-semi.stderr b/src/test/ui/suggestions/match-needing-semi.stderr
@@ -0,0 +1,36 @@
+error[E0308]: mismatched types
+  --> $DIR/match-needing-semi.rs:8:13
+   |
+LL | /     match 3 {
+LL | |         4 => 1,
+LL | |         3 => {
+LL | |             2
+   | |             ^ expected (), found integer
+LL | |         }
+LL | |         _ => 2
+LL | |     }
+   | |     -- help: consider using a semicolon here
+   | |_____|
+   |       expected this to be `()`
+   |
+   = note: expected type `()`
+              found type `{integer}`
+
+error[E0308]: mismatched types
+  --> $DIR/match-needing-semi.rs:12:5
+   |
+LL | /     match 3 {
+LL | |         4 => 1,
+LL | |         3 => 2,
+LL | |         _ => 2
+LL | |     }
+   | |     ^- help: consider using a semicolon here
+   | |_____|
+   |       expected (), found integer
+   |
+   = note: expected type `()`
+              found type `{integer}`
+
+error: aborting due to 2 previous errors
+
+For more information about this error, try `rustc --explain E0308`.

Original file line number	Diff line number	Diff line change
`@@ -50,7 +50,10 @@ error[E0308]: mismatched types`
`50`	`50`	`--> $DIR/struct-literal-variant-in-if.rs:10:20`
`51`	`51`	`\|`
`52`	`52`	`LL \| if x == E::V { field } {}`
`53`		`- \| ^^^^^ expected (), found bool`
	`53`	`+ \| ---------------^^^^^--- help: consider using a semicolon here`
	`54`	`+ \| \| \|`
	`55`	`+ \| \| expected (), found bool`
	`56`	+ \| expected this to be `()`
`54`	`57`	`\|`
`55`	`58`	= note: expected type `()`
`56`	`59`	found type `bool`