Rollup merge of #81769 - estebank:tail-expr-as-potential-return, r=lcnr

Suggest `return`ing tail expressions that match return type

Some newcomers are confused by the behavior of tail expressions,
interpreting that "leaving out the `;` makes it the return value".
To help them go in the right direction, suggest using `return` instead
when applicable.

compiler/rustc_typeck/src/check/callee.rs

@@ -346,7 +346,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                             if call_is_multiline {
                                 err.span_suggestion(
                                     callee.span.shrink_to_hi(),
-                                    "try adding a semicolon",
+                                    "consider using a semicolon here",
                                     ";".to_owned(),
                                     Applicability::MaybeIncorrect,
                                 );

compiler/rustc_typeck/src/check/coercion.rs

@@ -1450,7 +1450,9 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {
             ) {
                 if cond_expr.span.desugaring_kind().is_none() {
                     err.span_label(cond_expr.span, "expected this to be `()`");
-                    fcx.suggest_semicolon_at_end(cond_expr.span, &mut err);
+                    if expr.can_have_side_effects() {
+                        fcx.suggest_semicolon_at_end(cond_expr.span, &mut err);
+                    }
                 }
             }
             fcx.get_node_fn_decl(parent).map(|(fn_decl, _, is_main)| (fn_decl, is_main))
@@ -1458,7 +1460,7 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {
             fcx.get_fn_decl(parent_id)
         };
 
-        if let (Some((fn_decl, can_suggest)), _) = (fn_decl, pointing_at_return_type) {
+        if let Some((fn_decl, can_suggest)) = fn_decl {
             if expression.is_none() {
                 pointing_at_return_type |= fcx.suggest_missing_return_type(
                     &mut err,
@@ -1472,6 +1474,16 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {
                 fn_output = Some(&fn_decl.output); // `impl Trait` return type
             }
         }
+
+        let parent_id = fcx.tcx.hir().get_parent_item(id);
+        let parent_item = fcx.tcx.hir().get(parent_id);
+
+        if let (Some((expr, _)), Some((fn_decl, _, _))) =
+            (expression, fcx.get_node_fn_decl(parent_item))
+        {
+            fcx.suggest_missing_return_expr(&mut err, expr, fn_decl, expected, found);
+        }
+
         if let (Some(sp), Some(fn_output)) = (fcx.ret_coercion_span.get(), fn_output) {
             self.add_impl_trait_explanation(&mut err, cause, fcx, expected, sp, fn_output);
         }

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

@@ -561,7 +561,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             hir::StmtKind::Expr(ref expr) => {
                 // Check with expected type of `()`.
                 self.check_expr_has_type_or_error(&expr, self.tcx.mk_unit(), |err| {
-                    self.suggest_semicolon_at_end(expr.span, err);
+                    if expr.can_have_side_effects() {
+                        self.suggest_semicolon_at_end(expr.span, err);
+                    }
                 });
             }
             hir::StmtKind::Semi(ref expr) => {

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

@@ -44,11 +44,14 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         blk_id: hir::HirId,
     ) -> bool {
         let expr = expr.peel_drop_temps();
-        self.suggest_missing_semicolon(err, expr, expected, cause_span);
+        if expr.can_have_side_effects() {
+            self.suggest_missing_semicolon(err, expr, expected, cause_span);
+        }
         let mut pointing_at_return_type = false;
         if let Some((fn_decl, can_suggest)) = self.get_fn_decl(blk_id) {
             pointing_at_return_type =
                 self.suggest_missing_return_type(err, &fn_decl, expected, found, can_suggest);
+            self.suggest_missing_return_expr(err, expr, &fn_decl, expected, found);
         }
         pointing_at_return_type
     }
@@ -392,10 +395,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                 | ExprKind::Loop(..)
                 | ExprKind::If(..)
                 | ExprKind::Match(..)
-                | ExprKind::Block(..) => {
+                | ExprKind::Block(..)
+                    if expression.can_have_side_effects() =>
+                {
                     err.span_suggestion(
                         cause_span.shrink_to_hi(),
-                        "try adding a semicolon",
+                        "consider using a semicolon here",
                         ";".to_string(),
                         Applicability::MachineApplicable,
                     );
@@ -464,6 +469,34 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         }
     }
 
+    pub(in super::super) fn suggest_missing_return_expr(
+        &self,
+        err: &mut DiagnosticBuilder<'_>,
+        expr: &'tcx hir::Expr<'tcx>,
+        fn_decl: &hir::FnDecl<'_>,
+        expected: Ty<'tcx>,
+        found: Ty<'tcx>,
+    ) {
+        if !expected.is_unit() {
+            return;
+        }
+        let found = self.resolve_vars_with_obligations(found);
+        if let hir::FnRetTy::Return(ty) = fn_decl.output {
+            let ty = AstConv::ast_ty_to_ty(self, ty);
+            let ty = self.normalize_associated_types_in(expr.span, ty);
+            if self.can_coerce(found, ty) {
+                err.multipart_suggestion(
+                    "you might have meant to return this value",
+                    vec![
+                        (expr.span.shrink_to_lo(), "return ".to_string()),
+                        (expr.span.shrink_to_hi(), ";".to_string()),
+                    ],
+                    Applicability::MaybeIncorrect,
+                );
+            }
+        }
+    }
+
     pub(in super::super) fn suggest_missing_parentheses(
         &self,
         err: &mut DiagnosticBuilder<'_>,