Auto merge of #75656 - tirr-c:match-suggest-semi, r=estebank

Provide better spans for the match arm without tail expression

Resolves #75418.

Applied the same logic in the `if`-`else` type mismatch case.

r? @estebank

src/librustc_infer/infer/error_reporting/mod.rs

@@ -609,6 +609,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
                 err.span_label(span, "expected due to this");
             }
             ObligationCauseCode::MatchExpressionArm(box MatchExpressionArmCause {
+                semi_span,
                 source,
                 ref prior_arms,
                 last_ty,
@@ -663,6 +664,14 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
                             format!("this and all prior arms are found to be of type `{}`", t),
                         );
                     }
+                    if let Some(sp) = semi_span {
+                        err.span_suggestion_short(
+                            sp,
+                            "consider removing this semicolon",
+                            String::new(),
+                            Applicability::MachineApplicable,
+                        );
+                    }
                 }
             },
             ObligationCauseCode::IfExpression(box IfExpressionCause { then, outer, semicolon }) => {

src/librustc_middle/traits/mod.rs

@@ -345,6 +345,7 @@ static_assert_size!(ObligationCauseCode<'_>, 32);
 #[derive(Clone, Debug, PartialEq, Eq, Hash, Lift)]
 pub struct MatchExpressionArmCause<'tcx> {
     pub arm_span: Span,
+    pub semi_span: Option<Span>,
     pub source: hir::MatchSource,
     pub prior_arms: Vec<Span>,
     pub last_ty: Ty<'tcx>,

src/librustc_typeck/check/_match.rs

@@ -124,11 +124,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                     }
                 }
             } else {
-                let arm_span = if let hir::ExprKind::Block(blk, _) = &arm.body.kind {
-                    // Point at the block expr instead of the entire block
-                    blk.expr.as_ref().map(|e| e.span).unwrap_or(arm.body.span)
+                let (arm_span, semi_span) = if let hir::ExprKind::Block(blk, _) = &arm.body.kind {
+                    self.find_block_span(blk, prior_arm_ty)
                 } else {
-                    arm.body.span
+                    (arm.body.span, None)
                 };
                 let (span, code) = match i {
                     // The reason for the first arm to fail is not that the match arms diverge,
@@ -138,6 +137,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                         expr.span,
                         ObligationCauseCode::MatchExpressionArm(box MatchExpressionArmCause {
                             arm_span,
+                            semi_span,
                             source: match_src,
                             prior_arms: other_arms.clone(),
                             last_ty: prior_arm_ty.unwrap(),
@@ -295,14 +295,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
 
         let mut remove_semicolon = None;
         let error_sp = if let ExprKind::Block(block, _) = &else_expr.kind {
-            if let Some(expr) = &block.expr {
-                expr.span
-            } else if let Some(stmt) = block.stmts.last() {
-                // possibly incorrect trailing `;` in the else arm
-                remove_semicolon = self.could_remove_semicolon(block, then_ty);
-                stmt.span
-            } else {
-                // empty block; point at its entirety
+            let (error_sp, semi_sp) = self.find_block_span(block, Some(then_ty));
+            remove_semicolon = semi_sp;
+            if block.expr.is_none() && block.stmts.is_empty() {
                 // Avoid overlapping spans that aren't as readable:
                 // ```
                 // 2 |        let x = if true {
@@ -333,26 +328,21 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                 if outer_sp.is_some() {
                     outer_sp = Some(self.tcx.sess.source_map().guess_head_span(span));
                 }
-                else_expr.span
             }
+            error_sp
         } else {
             // shouldn't happen unless the parser has done something weird
             else_expr.span
         };
 
         // Compute `Span` of `then` part of `if`-expression.
         let then_sp = if let ExprKind::Block(block, _) = &then_expr.kind {
-            if let Some(expr) = &block.expr {
-                expr.span
-            } else if let Some(stmt) = block.stmts.last() {
-                // possibly incorrect trailing `;` in the else arm
-                remove_semicolon = remove_semicolon.or(self.could_remove_semicolon(block, else_ty));
-                stmt.span
-            } else {
-                // empty block; point at its entirety
+            let (then_sp, semi_sp) = self.find_block_span(block, Some(else_ty));
+            remove_semicolon = remove_semicolon.or(semi_sp);
+            if block.expr.is_none() && block.stmts.is_empty() {
                 outer_sp = None; // same as in `error_sp`; cleanup output
-                then_expr.span
             }
+            then_sp
         } else {
             // shouldn't happen unless the parser has done something weird
             then_expr.span
@@ -450,4 +440,20 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             scrut_ty
         }
     }
+
+    fn find_block_span(
+        &self,
+        block: &'tcx hir::Block<'tcx>,
+        expected_ty: Option<Ty<'tcx>>,
+    ) -> (Span, Option<Span>) {
+        if let Some(expr) = &block.expr {
+            (expr.span, None)
+        } else if let Some(stmt) = block.stmts.last() {
+            // possibly incorrect trailing `;` in the else arm
+            (stmt.span, expected_ty.and_then(|ty| self.could_remove_semicolon(block, ty)))
+        } else {
+            // empty block; point at its entirety
+            (block.span, None)
+        }
+    }
 }

src/test/ui/match/match-incompat-type-semi.rs

@@ -0,0 +1,42 @@
+// Diagnostic enhancement explained in issue #75418.
+// Point at the last statement in the block if there's no tail expression,
+// and suggest removing the semicolon if appropriate.
+
+fn main() {
+    let _ = match Some(42) {
+        Some(x) => {
+            x
+        },
+        None => {
+            0;
+            //~^ ERROR incompatible types
+            //~| HELP consider removing this semicolon
+        },
+    };
+
+    let _ = if let Some(x) = Some(42) {
+        x
+    } else {
+        0;
+        //~^ ERROR incompatible types
+        //~| HELP consider removing this semicolon
+    };
+
+    let _ = match Some(42) {
+        Some(x) => {
+            x
+        },
+        None => {
+            ();
+            //~^ ERROR incompatible types
+        },
+    };
+
+    let _ = match Some(42) {
+        Some(x) => {
+            x
+        },
+        None => { //~ ERROR incompatible types
+        },
+    };
+}

src/test/ui/match/match-incompat-type-semi.stderr

@@ -0,0 +1,74 @@
+error[E0308]: `match` arms have incompatible types
+  --> $DIR/match-incompat-type-semi.rs:11:13
+   |
+LL |       let _ = match Some(42) {
+   |  _____________-
+LL | |         Some(x) => {
+LL | |             x
+   | |             - this is found to be of type `{integer}`
+LL | |         },
+LL | |         None => {
+LL | |             0;
+   | |             ^-
+   | |             ||
+   | |             |help: consider removing this semicolon
+   | |             expected integer, found `()`
+...  |
+LL | |         },
+LL | |     };
+   | |_____- `match` arms have incompatible types
+
+error[E0308]: `if` and `else` have incompatible types
+  --> $DIR/match-incompat-type-semi.rs:20:9
+   |
+LL |       let _ = if let Some(x) = Some(42) {
+   |  _____________-
+LL | |         x
+   | |         - expected because of this
+LL | |     } else {
+LL | |         0;
+   | |         ^-
+   | |         ||
+   | |         |help: consider removing this semicolon
+   | |         expected integer, found `()`
+LL | |
+LL | |
+LL | |     };
+   | |_____- `if` and `else` have incompatible types
+
+error[E0308]: `match` arms have incompatible types
+  --> $DIR/match-incompat-type-semi.rs:30:13
+   |
+LL |       let _ = match Some(42) {
+   |  _____________-
+LL | |         Some(x) => {
+LL | |             x
+   | |             - this is found to be of type `{integer}`
+LL | |         },
+LL | |         None => {
+LL | |             ();
+   | |             ^^^ expected integer, found `()`
+LL | |
+LL | |         },
+LL | |     };
+   | |_____- `match` arms have incompatible types
+
+error[E0308]: `match` arms have incompatible types
+  --> $DIR/match-incompat-type-semi.rs:39:17
+   |
+LL |        let _ = match Some(42) {
+   |   _____________-
+LL |  |         Some(x) => {
+LL |  |             x
+   |  |             - this is found to be of type `{integer}`
+LL |  |         },
+LL |  |         None => {
+   |  |_________________^
+LL | ||         },
+   | ||_________^ expected integer, found `()`
+LL |  |     };
+   |  |_____- `match` arms have incompatible types
+
+error: aborting due to 4 previous errors
+
+For more information about this error, try `rustc --explain E0308`.