Rollup merge of #129260 - wafarm:dont-suggest-closures, r=compiler-errors

Don't suggest adding return type for closures with default return type

Follow up of #129223

r? ``@compiler-errors``

compiler/rustc_hir_typeck/src/_match.rs

@@ -389,7 +389,7 @@ pub(crate) fn maybe_get_coercion_reason(
             {
                 // check that the `if` expr without `else` is the fn body's expr
                 if expr.span == sp {
-                    return self.get_fn_decl(hir_id).map(|(_, fn_decl, _)| {
+                    return self.get_fn_decl(hir_id).map(|(_, fn_decl)| {
                         let (ty, span) = match fn_decl.output {
                             hir::FnRetTy::DefaultReturn(span) => ("()".to_string(), span),
                             hir::FnRetTy::Return(ty) => (ty_to_string(&self.tcx, ty), ty.span),

compiler/rustc_hir_typeck/src/coercion.rs

@@ -1860,10 +1860,10 @@ fn report_return_mismatched_types<'infcx>(
         };
 
         // If this is due to an explicit `return`, suggest adding a return type.
-        if let Some((fn_id, fn_decl, can_suggest)) = fcx.get_fn_decl(block_or_return_id)
+        if let Some((fn_id, fn_decl)) = fcx.get_fn_decl(block_or_return_id)
             && !due_to_block
         {
-            fcx.suggest_missing_return_type(&mut err, fn_decl, expected, found, can_suggest, fn_id);
+            fcx.suggest_missing_return_type(&mut err, fn_decl, expected, found, fn_id);
         }
 
         // If this is due to a block, then maybe we forgot a `return`/`break`.

compiler/rustc_hir_typeck/src/expr.rs

@@ -774,7 +774,7 @@ fn check_expr_return(
                 self.ret_coercion_span.set(Some(expr.span));
             }
             let cause = self.cause(expr.span, ObligationCauseCode::ReturnNoExpression);
-            if let Some((_, fn_decl, _)) = self.get_fn_decl(expr.hir_id) {
+            if let Some((_, fn_decl)) = self.get_fn_decl(expr.hir_id) {
                 coercion.coerce_forced_unit(
                     self,
                     &cause,

compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs

@@ -30,7 +30,7 @@
 use rustc_session::lint;
 use rustc_span::def_id::LocalDefId;
 use rustc_span::hygiene::DesugaringKind;
-use rustc_span::symbol::{kw, sym};
+use rustc_span::symbol::kw;
 use rustc_span::Span;
 use rustc_target::abi::FieldIdx;
 use rustc_trait_selection::error_reporting::infer::need_type_info::TypeAnnotationNeeded;
@@ -859,38 +859,28 @@ pub fn resolve_ty_and_res_fully_qualified_call(
         )
     }
 
-    /// Given a `HirId`, return the `HirId` of the enclosing function, its `FnDecl`, and whether a
+    /// Given a `HirId`, return the `HirId` of the enclosing function and its `FnDecl`.
-    /// suggestion can be made, `None` otherwise.
     pub(crate) fn get_fn_decl(
         &self,
         blk_id: HirId,
-    ) -> Option<(LocalDefId, &'tcx hir::FnDecl<'tcx>, bool)> {
+    ) -> Option<(LocalDefId, &'tcx hir::FnDecl<'tcx>)> {
         // Get enclosing Fn, if it is a function or a trait method, unless there's a `loop` or
         // `while` before reaching it, as block tail returns are not available in them.
         self.tcx.hir().get_fn_id_for_return_block(blk_id).and_then(|item_id| {
             match self.tcx.hir_node(item_id) {
                 Node::Item(&hir::Item {
-                    ident,
+                    kind: hir::ItemKind::Fn(ref sig, ..), owner_id, ..
-                    kind: hir::ItemKind::Fn(ref sig, ..),
+                }) => Some((owner_id.def_id, sig.decl)),
-                    owner_id,
-                    ..
-                }) => {
-                    // This is less than ideal, it will not suggest a return type span on any
-                    // method called `main`, regardless of whether it is actually the entry point,
-                    // but it will still present it as the reason for the expected type.
-                    Some((owner_id.def_id, sig.decl, ident.name != sym::main))
-                }
                 Node::TraitItem(&hir::TraitItem {
                     kind: hir::TraitItemKind::Fn(ref sig, ..),
                     owner_id,
                     ..
-                }) => Some((owner_id.def_id, sig.decl, true)),
+                }) => Some((owner_id.def_id, sig.decl)),
-                // FIXME: Suggestable if this is not a trait implementation
                 Node::ImplItem(&hir::ImplItem {
                     kind: hir::ImplItemKind::Fn(ref sig, ..),
                     owner_id,
                     ..
-                }) => Some((owner_id.def_id, sig.decl, false)),
+                }) => Some((owner_id.def_id, sig.decl)),
                 Node::Expr(&hir::Expr {
                     hir_id,
                     kind: hir::ExprKind::Closure(&hir::Closure { def_id, kind, fn_decl, .. }),
@@ -901,33 +891,30 @@ pub(crate) fn get_fn_decl(
                             // FIXME(async_closures): Implement this.
                             return None;
                         }
-                        hir::ClosureKind::Closure => Some((def_id, fn_decl, true)),
+                        hir::ClosureKind::Closure => Some((def_id, fn_decl)),
                         hir::ClosureKind::Coroutine(hir::CoroutineKind::Desugared(
                             _,
                             hir::CoroutineSource::Fn,
                         )) => {
-                            let (ident, sig, owner_id) = match self.tcx.parent_hir_node(hir_id) {
+                            let (sig, owner_id) = match self.tcx.parent_hir_node(hir_id) {
                                 Node::Item(&hir::Item {
-                                    ident,
                                     kind: hir::ItemKind::Fn(ref sig, ..),
                                     owner_id,
                                     ..
-                                }) => (ident, sig, owner_id),
+                                }) => (sig, owner_id),
                                 Node::TraitItem(&hir::TraitItem {
-                                    ident,
                                     kind: hir::TraitItemKind::Fn(ref sig, ..),
                                     owner_id,
                                     ..
-                                }) => (ident, sig, owner_id),
+                                }) => (sig, owner_id),
                                 Node::ImplItem(&hir::ImplItem {
-                                    ident,
                                     kind: hir::ImplItemKind::Fn(ref sig, ..),
                                     owner_id,
                                     ..
-                                }) => (ident, sig, owner_id),
+                                }) => (sig, owner_id),
                                 _ => return None,
                             };
-                            Some((owner_id.def_id, sig.decl, ident.name != sym::main))
+                            Some((owner_id.def_id, sig.decl))
                         }
                         _ => None,
                     }

compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs

@@ -1873,7 +1873,7 @@ pub(in super::super) fn check_block_with_expected(
                     // that highlight errors inline.
                     let mut sp = blk.span;
                     let mut fn_span = None;
-                    if let Some((fn_def_id, decl, _)) = self.get_fn_decl(blk.hir_id) {
+                    if let Some((fn_def_id, decl)) = self.get_fn_decl(blk.hir_id) {
                         let ret_sp = decl.output.span();
                         if let Some(block_sp) = self.parent_item_span(blk.hir_id) {
                             // HACK: on some cases (`ui/liveness/liveness-issue-2163.rs`) the

compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs

@@ -79,9 +79,9 @@ pub(crate) fn suggest_mismatched_types_on_tail(
             // `break` type mismatches provide better context for tail `loop` expressions.
             return false;
         }
-        if let Some((fn_id, fn_decl, can_suggest)) = self.get_fn_decl(blk_id) {
+        if let Some((fn_id, fn_decl)) = self.get_fn_decl(blk_id) {
             pointing_at_return_type =
-                self.suggest_missing_return_type(err, fn_decl, expected, found, can_suggest, fn_id);
+                self.suggest_missing_return_type(err, fn_decl, expected, found, fn_id);
             self.suggest_missing_break_or_return_expr(
                 err, expr, fn_decl, expected, found, blk_id, fn_id,
             );
@@ -813,7 +813,6 @@ pub(in super::super) fn suggest_missing_return_type(
         fn_decl: &hir::FnDecl<'tcx>,
         expected: Ty<'tcx>,
         found: Ty<'tcx>,
-        can_suggest: bool,
         fn_id: LocalDefId,
     ) -> bool {
         // Can't suggest `->` on a block-like coroutine
@@ -826,28 +825,26 @@ pub(in super::super) fn suggest_missing_return_type(
         let found =
             self.resolve_numeric_literals_with_default(self.resolve_vars_if_possible(found));
         // Only suggest changing the return type for methods that
-        // haven't set a return type at all (and aren't `fn main()` or an impl).
+        // haven't set a return type at all (and aren't `fn main()`, impl or closure).
         match &fn_decl.output {
-            &hir::FnRetTy::DefaultReturn(span) if expected.is_unit() && !can_suggest => {
+            // For closure with default returns, don't suggest adding return type
-                // `fn main()` must return `()`, do not suggest changing return type
+            &hir::FnRetTy::DefaultReturn(_) if self.tcx.is_closure_like(fn_id.to_def_id()) => {}
-                err.subdiagnostic(errors::ExpectedReturnTypeLabel::Unit { span });
-                return true;
-            }
             &hir::FnRetTy::DefaultReturn(span) if expected.is_unit() => {
-                if let Some(found) = found.make_suggestable(self.tcx, false, None) {
+                if !self.can_add_return_type(fn_id) {
+                    err.subdiagnostic(errors::ExpectedReturnTypeLabel::Unit { span });
+                } else if let Some(found) = found.make_suggestable(self.tcx, false, None) {
                     err.subdiagnostic(errors::AddReturnTypeSuggestion::Add {
                         span,
                         found: found.to_string(),
                     });
-                    return true;
                 } else if let Some(sugg) = suggest_impl_trait(self, self.param_env, found) {
                     err.subdiagnostic(errors::AddReturnTypeSuggestion::Add { span, found: sugg });
-                    return true;
                 } else {
                     // FIXME: if `found` could be `impl Iterator` we should suggest that.
                     err.subdiagnostic(errors::AddReturnTypeSuggestion::MissingHere { span });
-                    return true;
                 }
+
+                return true;
             }
             hir::FnRetTy::Return(hir_ty) => {
                 if let hir::TyKind::OpaqueDef(item_id, ..) = hir_ty.kind
@@ -905,6 +902,32 @@ pub(in super::super) fn suggest_missing_return_type(
         false
     }
 
+    /// Checks whether we can add a return type to a function.
+    /// Assumes given function doesn't have a explicit return type.
+    fn can_add_return_type(&self, fn_id: LocalDefId) -> bool {
+        match self.tcx.hir_node_by_def_id(fn_id) {
+            Node::Item(&hir::Item { ident, .. }) => {
+                // This is less than ideal, it will not suggest a return type span on any
+                // method called `main`, regardless of whether it is actually the entry point,
+                // but it will still present it as the reason for the expected type.
+                ident.name != sym::main
+            }
+            Node::ImplItem(item) => {
+                // If it doesn't impl a trait, we can add a return type
+                let Node::Item(&hir::Item {
+                    kind: hir::ItemKind::Impl(&hir::Impl { of_trait, .. }),
+                    ..
+                }) = self.tcx.parent_hir_node(item.hir_id())
+                else {
+                    unreachable!();
+                };
+
+                of_trait.is_none()
+            }
+            _ => true,
+        }
+    }
+
     fn try_note_caller_chooses_ty_for_ty_param(
         &self,
         diag: &mut Diag<'_>,

tests/ui/closures/add_semicolon_non_block_closure.rs

@@ -8,5 +8,4 @@ fn main() {
     foo(|| bar())
     //~^ ERROR mismatched types [E0308]
     //~| HELP consider using a semicolon here
-    //~| HELP try adding a return type
 }

tests/ui/closures/add_semicolon_non_block_closure.stderr

@@ -8,10 +8,6 @@ help: consider using a semicolon here
    |
 LL |     foo(|| { bar(); })
    |            +      +++
-help: try adding a return type
-   |
-LL |     foo(|| -> i32 bar())
-   |            ++++++
 
 error: aborting due to 1 previous error
 

tests/ui/suggestions/try-operator-dont-suggest-semicolon.rs

@@ -3,7 +3,6 @@
 
 fn main() -> Result<(), ()> {
     a(|| {
-        //~^ HELP: try adding a return type
         b()
         //~^ ERROR: mismatched types [E0308]
         //~| NOTE: expected `()`, found `i32`

tests/ui/suggestions/try-operator-dont-suggest-semicolon.stderr

@@ -1,20 +1,13 @@
 error[E0308]: mismatched types
-  --> $DIR/try-operator-dont-suggest-semicolon.rs:7:9
+  --> $DIR/try-operator-dont-suggest-semicolon.rs:6:9
    |
 LL |         b()
-   |         ^^^ expected `()`, found `i32`
+   |         ^^^- help: consider using a semicolon here: `;`
-   |
+   |         |
-help: consider using a semicolon here
+   |         expected `()`, found `i32`
-   |
-LL |         b();
-   |            +
-help: try adding a return type
-   |
-LL |     a(|| -> i32 {
-   |          ++++++
 
 error[E0308]: mismatched types
-  --> $DIR/try-operator-dont-suggest-semicolon.rs:17:9
+  --> $DIR/try-operator-dont-suggest-semicolon.rs:16:9
    |
 LL | /     if true {
 LL | |

tests/ui/typeck/issue-81943.stderr

@@ -2,9 +2,7 @@ error[E0308]: mismatched types
   --> $DIR/issue-81943.rs:7:9
    |
 LL |   f(|x| lib::d!(x));
-   |        -^^^^^^^^^^ expected `()`, found `i32`
+   |         ^^^^^^^^^^ expected `()`, found `i32`
-   |        |
-   |        help: try adding a return type: `-> i32`
    |
    = note: this error originates in the macro `lib::d` (in Nightly builds, run with -Z macro-backtrace for more info)
 
@@ -12,22 +10,28 @@ error[E0308]: mismatched types
   --> $DIR/issue-81943.rs:8:28
    |
 LL |   f(|x| match x { tmp => { g(tmp) } });
-   |                            ^^^^^^ expected `()`, found `i32`
+   |         -------------------^^^^^^----
+   |         |                  |
+   |         |                  expected `()`, found `i32`
+   |         expected this to be `()`
    |
 help: consider using a semicolon here
    |
 LL |   f(|x| match x { tmp => { g(tmp); } });
    |                                  +
-help: try adding a return type
+help: consider using a semicolon here
    |
-LL |   f(|x| -> i32 match x { tmp => { g(tmp) } });
+LL |   f(|x| match x { tmp => { g(tmp) } };);
-   |         ++++++
+   |                                      +
 
 error[E0308]: mismatched types
   --> $DIR/issue-81943.rs:10:38
    |
 LL |     ($e:expr) => { match $e { x => { g(x) } } }
-   |                                      ^^^^ expected `()`, found `i32`
+   |                    ------------------^^^^----
+   |                    |                 |
+   |                    |                 expected `()`, found `i32`
+   |                    expected this to be `()`
 LL |   }
 LL |   f(|x| d!(x));
    |         ----- in this macro invocation
@@ -37,10 +41,10 @@ help: consider using a semicolon here
    |
 LL |     ($e:expr) => { match $e { x => { g(x); } } }
    |                                          +
-help: try adding a return type
+help: consider using a semicolon here
    |
-LL |   f(|x| -> i32 d!(x));
+LL |     ($e:expr) => { match $e { x => { g(x) } }; }
-   |         ++++++
+   |                                              +
 
 error: aborting due to 3 previous errors