Rollup merge of #97371 - ChayimFriedman2:closure-non-block-add-semicolon, r=oli-obk

Suggest adding a semicolon to a closure without block

This transforms `|| expr` into `|| { expr; }`.

Closes #97359.

compiler/rustc_typeck/src/check/coercion.rs

@@ -1500,7 +1500,8 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {
                             coercion_error.clone(),
                             fcx,
                             parent_id,
-                            expression.map(|expr| (expr, blk_id)),
+                            expression,
+                            Some(blk_id),
                         );
                         if !fcx.tcx.features().unsized_locals {
                             unsized_return = self.is_return_ty_unsized(fcx, blk_id);
@@ -1514,6 +1515,7 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {
                             coercion_error.clone(),
                             fcx,
                             id,
+                            expression,
                             None,
                         );
                         if !fcx.tcx.features().unsized_locals {
@@ -1564,21 +1566,28 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {
         ty_err: TypeError<'tcx>,
         fcx: &FnCtxt<'a, 'tcx>,
         id: hir::HirId,
-        expression: Option<(&'tcx hir::Expr<'tcx>, hir::HirId)>,
+        expression: Option<&'tcx hir::Expr<'tcx>>,
+        blk_id: Option<hir::HirId>,
     ) -> DiagnosticBuilder<'a, ErrorGuaranteed> {
         let mut err = fcx.report_mismatched_types(cause, expected, found, ty_err);
 
         let mut pointing_at_return_type = false;
         let mut fn_output = None;
 
+        let parent_id = fcx.tcx.hir().get_parent_node(id);
+        let parent = fcx.tcx.hir().get(parent_id);
+        if let Some(expr) = expression
+            && let hir::Node::Expr(hir::Expr { kind: hir::ExprKind::Closure(_, _, body_id, ..), .. }) = parent
+            && !matches!(fcx.tcx.hir().get(body_id.hir_id), hir::Node::Expr(hir::Expr { kind: hir::ExprKind::Block(..), .. }))
+        {
+            fcx.suggest_missing_semicolon(&mut err, expr, expected, true);
+        }
         // Verify that this is a tail expression of a function, otherwise the
         // label pointing out the cause for the type coercion will be wrong
         // as prior return coercions would not be relevant (#57664).
-        let parent_id = fcx.tcx.hir().get_parent_node(id);
+        let fn_decl = if let (Some(expr), Some(blk_id)) = (expression, blk_id) {
-        let fn_decl = if let Some((expr, blk_id)) = expression {
             pointing_at_return_type =
                 fcx.suggest_mismatched_types_on_tail(&mut err, expr, expected, found, blk_id);
-            let parent = fcx.tcx.hir().get(parent_id);
             if let (Some(cond_expr), true, false) = (
                 fcx.tcx.hir().get_if_cause(expr.hir_id),
                 expected.is_unit(),
@@ -1607,7 +1616,7 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {
         };
 
         if let Some((fn_decl, can_suggest)) = fn_decl {
-            if expression.is_none() {
+            if blk_id.is_none() {
                 pointing_at_return_type |= fcx.suggest_missing_return_type(
                     &mut err,
                     &fn_decl,
@@ -1625,8 +1634,8 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {
         let parent_id = fcx.tcx.hir().get_parent_item(id);
         let parent_item = fcx.tcx.hir().get_by_def_id(parent_id);
 
-        if let (Some((expr, _)), Some((fn_decl, _, _))) =
+        if let (Some(expr), Some(_), Some((fn_decl, _, _))) =
-            (expression, fcx.get_node_fn_decl(parent_item))
+            (expression, blk_id, fcx.get_node_fn_decl(parent_item))
         {
             fcx.suggest_missing_break_or_return_expr(
                 &mut err,

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

@@ -46,12 +46,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         blk_id: hir::HirId,
     ) -> bool {
         let expr = expr.peel_drop_temps();
-        // If the expression is from an external macro, then do not suggest
+        self.suggest_missing_semicolon(err, expr, expected, false);
-        // adding a semicolon, because there's nowhere to put it.
-        // See issue #81943.
-        if expr.can_have_side_effects() && !in_external_macro(self.tcx.sess, expr.span) {
-            self.suggest_missing_semicolon(err, expr, expected);
-        }
         let mut pointing_at_return_type = false;
         if let Some((fn_decl, can_suggest)) = self.get_fn_decl(blk_id) {
             let fn_id = self.tcx.hir().get_return_block(blk_id).unwrap();
@@ -473,11 +468,15 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
     /// This routine checks if the return expression in a block would make sense on its own as a
     /// statement and the return type has been left as default or has been specified as `()`. If so,
     /// it suggests adding a semicolon.
-    fn suggest_missing_semicolon(
+    ///
+    /// If the expression is the expression of a closure without block (`|| expr`), a
+    /// block is needed to be added too (`|| { expr; }`). This is denoted by `needs_block`.
+    pub fn suggest_missing_semicolon(
         &self,
         err: &mut Diagnostic,
         expression: &'tcx hir::Expr<'tcx>,
         expected: Ty<'tcx>,
+        needs_block: bool,
     ) {
         if expected.is_unit() {
             // `BlockTailExpression` only relevant if the tail expr would be
@@ -489,14 +488,29 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                 | ExprKind::If(..)
                 | ExprKind::Match(..)
                 | ExprKind::Block(..)
-                    if expression.can_have_side_effects() =>
+                    if expression.can_have_side_effects()
+                        // If the expression is from an external macro, then do not suggest
+                        // adding a semicolon, because there's nowhere to put it.
+                        // See issue #81943.
+                        && !in_external_macro(self.tcx.sess, expression.span) =>
                 {
-                    err.span_suggestion(
+                    if needs_block {
-                        expression.span.shrink_to_hi(),
+                        err.multipart_suggestion(
-                        "consider using a semicolon here",
+                            "consider using a semicolon here",
-                        ";".to_string(),
+                            vec![
-                        Applicability::MachineApplicable,
+                                (expression.span.shrink_to_lo(), "{ ".to_owned()),
-                    );
+                                (expression.span.shrink_to_hi(), "; }".to_owned()),
+                            ],
+                            Applicability::MachineApplicable,
+                        );
+                    } else {
+                        err.span_suggestion(
+                            expression.span.shrink_to_hi(),
+                            "consider using a semicolon here",
+                            ";".to_string(),
+                            Applicability::MachineApplicable,
+                        );
+                    }
                 }
                 _ => (),
             }

src/test/ui/closures/add_semicolon_non_block_closure.rs

@@ -0,0 +1,11 @@
+fn foo(_f: impl Fn()) {}
+
+fn bar() -> i32 {
+    1
+}
+
+fn main() {
+    foo(|| bar())
+    //~^ ERROR mismatched types [E0308]
+    //~| HELP consider using a semicolon here
+}

src/test/ui/closures/add_semicolon_non_block_closure.stderr

@@ -0,0 +1,16 @@
+error[E0308]: mismatched types
+  --> $DIR/add_semicolon_non_block_closure.rs:8:12
+   |
+LL | fn main() {
+   |           - expected `()` because of default return type
+LL |     foo(|| bar())
+   |            ^^^^^ expected `()`, found `i32`
+   |
+help: consider using a semicolon here
+   |
+LL |     foo(|| { bar(); })
+   |            +      +++
+
+error: aborting due to previous error
+
+For more information about this error, try `rustc --explain E0308`.