Rollup merge of #120696 - estebank:issue-115405, r=oli-obk

matthiaskrgr · web-flow · commit 020e84652dbc · 2024-02-13T06:27:37.000+01:00
Properly handle `async` block and `async fn` in `if` exprs without `else` When encountering a tail expression in the then arm of an `if` expression without an `else` arm, account for `async fn` and `async` blocks to suggest `return`ing the value and pointing at the return type of the `async fn`. We now also account for AFIT when looking for the return type to point at. Fix #115405.
diff --git a/compiler/rustc_hir_typeck/src/coercion.rs b/compiler/rustc_hir_typeck/src/coercion.rs
@@ -98,8 +98,11 @@ struct CollectRetsVisitor<'tcx> {
 
 impl<'tcx> Visitor<'tcx> for CollectRetsVisitor<'tcx> {
     fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) {
-        if let hir::ExprKind::Ret(_) = expr.kind {
-            self.ret_exprs.push(expr);
+        match expr.kind {
+            hir::ExprKind::Ret(_) => self.ret_exprs.push(expr),
+            // `return` in closures does not return from the outer function
+            hir::ExprKind::Closure(_) => return,
+            _ => {}
         }
         intravisit::walk_expr(self, expr);
     }
@@ -1845,13 +1848,31 @@ 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_node_by_def_id(parent_id.def_id);
+        let mut parent_item = fcx.tcx.hir_node_by_def_id(parent_id.def_id);
+        // When suggesting return, we need to account for closures and async blocks, not just items.
+        for (_, node) in fcx.tcx.hir().parent_iter(id) {
+            match node {
+                hir::Node::Expr(&hir::Expr {
+                    kind: hir::ExprKind::Closure(hir::Closure { .. }),
+                    ..
+                }) => {
+                    parent_item = node;
+                    break;
+                }
+                hir::Node::Item(_) | hir::Node::TraitItem(_) | hir::Node::ImplItem(_) => break,
+                _ => {}
+            }
+        }
 
-        if let (Some(expr), Some(_), Some((fn_id, fn_decl, _, _))) =
-            (expression, blk_id, fcx.get_node_fn_decl(parent_item))
-        {
+        if let (Some(expr), Some(_), Some(fn_decl)) = (expression, blk_id, parent_item.fn_decl()) {
             fcx.suggest_missing_break_or_return_expr(
-                &mut err, expr, fn_decl, expected, found, id, fn_id,
+                &mut err,
+                expr,
+                fn_decl,
+                expected,
+                found,
+                id,
+                parent_id.into(),
             );
         }
 
diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs
@@ -963,14 +963,35 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                 owner_id,
                 ..
             }) => Some((hir::HirId::make_owner(owner_id.def_id), &sig.decl, ident, false)),
-            Node::Expr(&hir::Expr { hir_id, kind: hir::ExprKind::Closure(..), .. })
-                if let Node::Item(&hir::Item {
-                    ident,
-                    kind: hir::ItemKind::Fn(ref sig, ..),
-                    owner_id,
-                    ..
-                }) = self.tcx.parent_hir_node(hir_id) =>
-            {
+            Node::Expr(&hir::Expr {
+                hir_id,
+                kind:
+                    hir::ExprKind::Closure(hir::Closure {
+                        kind: hir::ClosureKind::Coroutine(..), ..
+                    }),
+                ..
+            }) => {
+                let (ident, 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),
+                    Node::TraitItem(&hir::TraitItem {
+                        ident,
+                        kind: hir::TraitItemKind::Fn(ref sig, ..),
+                        owner_id,
+                        ..
+                    }) => (ident, sig, owner_id),
+                    Node::ImplItem(&hir::ImplItem {
+                        ident,
+                        kind: hir::ImplItemKind::Fn(ref sig, ..),
+                        owner_id,
+                        ..
+                    }) => (ident, sig, owner_id),
+                    _ => return None,
+                };
                 Some((
                     hir::HirId::make_owner(owner_id.def_id),
                     &sig.decl,
diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs
@@ -1726,7 +1726,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
     }
 
     /// Given a function block's `HirId`, returns its `FnDecl` if it exists, or `None` otherwise.
-    fn get_parent_fn_decl(&self, blk_id: hir::HirId) -> Option<(&'tcx hir::FnDecl<'tcx>, Ident)> {
+    pub(crate) fn get_parent_fn_decl(
+        &self,
+        blk_id: hir::HirId,
+    ) -> Option<(&'tcx hir::FnDecl<'tcx>, Ident)> {
         let parent = self.tcx.hir_node_by_def_id(self.tcx.hir().get_parent_item(blk_id).def_id);
         self.get_node_fn_decl(parent).map(|(_, fn_decl, ident, _)| (fn_decl, ident))
     }
diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs
@@ -827,6 +827,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             }
             hir::FnRetTy::Return(hir_ty) => {
                 if let hir::TyKind::OpaqueDef(item_id, ..) = hir_ty.kind
+                    // FIXME: account for RPITIT.
                     && let hir::Node::Item(hir::Item {
                         kind: hir::ItemKind::OpaqueTy(op_ty), ..
                     }) = self.tcx.hir_node(item_id.hir_id())
@@ -1038,33 +1039,62 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             return;
         }
 
-        if let hir::FnRetTy::Return(ty) = fn_decl.output {
-            let ty = self.astconv().ast_ty_to_ty(ty);
-            let bound_vars = self.tcx.late_bound_vars(fn_id);
-            let ty = self
-                .tcx
-                .instantiate_bound_regions_with_erased(Binder::bind_with_vars(ty, bound_vars));
-            let ty = match self.tcx.asyncness(fn_id.owner) {
-                ty::Asyncness::Yes => self.get_impl_future_output_ty(ty).unwrap_or_else(|| {
-                    span_bug!(fn_decl.output.span(), "failed to get output type of async function")
-                }),
-                ty::Asyncness::No => ty,
-            };
-            let ty = self.normalize(expr.span, ty);
-            if self.can_coerce(found, ty) {
-                if let Some(owner_node) = self.tcx.hir_node(fn_id).as_owner()
-                    && let Some(span) = expr.span.find_ancestor_inside(*owner_node.span())
-                {
-                    err.multipart_suggestion(
-                        "you might have meant to return this value",
-                        vec![
-                            (span.shrink_to_lo(), "return ".to_string()),
-                            (span.shrink_to_hi(), ";".to_string()),
-                        ],
-                        Applicability::MaybeIncorrect,
-                    );
-                }
+        let scope = self
+            .tcx
+            .hir()
+            .parent_iter(id)
+            .filter(|(_, node)| {
+                matches!(
+                    node,
+                    Node::Expr(Expr { kind: ExprKind::Closure(..), .. })
+                        | Node::Item(_)
+                        | Node::TraitItem(_)
+                        | Node::ImplItem(_)
+                )
+            })
+            .next();
+        let in_closure =
+            matches!(scope, Some((_, Node::Expr(Expr { kind: ExprKind::Closure(..), .. }))));
+
+        let can_return = match fn_decl.output {
+            hir::FnRetTy::Return(ty) => {
+                let ty = self.astconv().ast_ty_to_ty(ty);
+                let bound_vars = self.tcx.late_bound_vars(fn_id);
+                let ty = self
+                    .tcx
+                    .instantiate_bound_regions_with_erased(Binder::bind_with_vars(ty, bound_vars));
+                let ty = match self.tcx.asyncness(fn_id.owner) {
+                    ty::Asyncness::Yes => self.get_impl_future_output_ty(ty).unwrap_or_else(|| {
+                        span_bug!(
+                            fn_decl.output.span(),
+                            "failed to get output type of async function"
+                        )
+                    }),
+                    ty::Asyncness::No => ty,
+                };
+                let ty = self.normalize(expr.span, ty);
+                self.can_coerce(found, ty)
+            }
+            hir::FnRetTy::DefaultReturn(_) if in_closure => {
+                self.ret_coercion.as_ref().map_or(false, |ret| {
+                    let ret_ty = ret.borrow().expected_ty();
+                    self.can_coerce(found, ret_ty)
+                })
             }
+            _ => false,
+        };
+        if can_return
+            && let Some(owner_node) = self.tcx.hir_node(fn_id).as_owner()
+            && let Some(span) = expr.span.find_ancestor_inside(*owner_node.span())
+        {
+            err.multipart_suggestion(
+                "you might have meant to return this value",
+                vec![
+                    (span.shrink_to_lo(), "return ".to_string()),
+                    (span.shrink_to_hi(), ";".to_string()),
+                ],
+                Applicability::MaybeIncorrect,
+            );
         }
     }
 
diff --git a/compiler/rustc_middle/src/hir/map/mod.rs b/compiler/rustc_middle/src/hir/map/mod.rs
@@ -617,7 +617,7 @@ impl<'hir> Map<'hir> {
                 Node::Item(_)
                 | Node::ForeignItem(_)
                 | Node::TraitItem(_)
-                | Node::Expr(Expr { kind: ExprKind::Closure { .. }, .. })
+                | Node::Expr(Expr { kind: ExprKind::Closure(_), .. })
                 | Node::ImplItem(_)
                     // The input node `id` must be enclosed in the method's body as opposed
                     // to some other place such as its return type (fixes #114918).
diff --git a/compiler/rustc_parse/src/parser/diagnostics.rs b/compiler/rustc_parse/src/parser/diagnostics.rs
@@ -900,7 +900,7 @@ impl<'a> Parser<'a> {
             // fn foo() -> Foo {
             //     field: value,
             // }
-            info!(?maybe_struct_name, ?self.token);
+            debug!(?maybe_struct_name, ?self.token);
             let mut snapshot = self.create_snapshot_for_diagnostic();
             let path = Path {
                 segments: ThinVec::new(),
diff --git a/tests/ui/async-await/missing-return-in-async-block.fixed b/tests/ui/async-await/missing-return-in-async-block.fixed
@@ -0,0 +1,22 @@
+// run-rustfix
+// edition:2021
+use std::future::Future;
+use std::pin::Pin;
+pub struct S;
+pub fn foo() {
+    let _ = Box::pin(async move {
+        if true {
+            return Ok(S); //~ ERROR mismatched types
+        }
+        Err(())
+    });
+}
+pub fn bar() -> Pin<Box<dyn Future<Output = Result<S, ()>> + 'static>> {
+    Box::pin(async move {
+        if true {
+            return Ok(S); //~ ERROR mismatched types
+        }
+        Err(())
+    })
+}
+fn main() {}
diff --git a/tests/ui/async-await/missing-return-in-async-block.rs b/tests/ui/async-await/missing-return-in-async-block.rs
@@ -0,0 +1,22 @@
+// run-rustfix
+// edition:2021
+use std::future::Future;
+use std::pin::Pin;
+pub struct S;
+pub fn foo() {
+    let _ = Box::pin(async move {
+        if true {
+            Ok(S) //~ ERROR mismatched types
+        }
+        Err(())
+    });
+}
+pub fn bar() -> Pin<Box<dyn Future<Output = Result<S, ()>> + 'static>> {
+    Box::pin(async move {
+        if true {
+            Ok(S) //~ ERROR mismatched types
+        }
+        Err(())
+    })
+}
+fn main() {}
diff --git a/tests/ui/async-await/missing-return-in-async-block.stderr b/tests/ui/async-await/missing-return-in-async-block.stderr
@@ -0,0 +1,35 @@
+error[E0308]: mismatched types
+  --> $DIR/missing-return-in-async-block.rs:9:13
+   |
+LL | /         if true {
+LL | |             Ok(S)
+   | |             ^^^^^ expected `()`, found `Result<S, _>`
+LL | |         }
+   | |_________- expected this to be `()`
+   |
+   = note: expected unit type `()`
+                   found enum `Result<S, _>`
+help: you might have meant to return this value
+   |
+LL |             return Ok(S);
+   |             ++++++      +
+
+error[E0308]: mismatched types
+  --> $DIR/missing-return-in-async-block.rs:17:13
+   |
+LL | /         if true {
+LL | |             Ok(S)
+   | |             ^^^^^ expected `()`, found `Result<S, _>`
+LL | |         }
+   | |_________- expected this to be `()`
+   |
+   = note: expected unit type `()`
+                   found enum `Result<S, _>`
+help: you might have meant to return this value
+   |
+LL |             return Ok(S);
+   |             ++++++      +
+
+error: aborting due to 2 previous errors
+
+For more information about this error, try `rustc --explain E0308`.
diff --git a/tests/ui/impl-trait/in-trait/default-body-type-err-2.stderr b/tests/ui/impl-trait/in-trait/default-body-type-err-2.stderr
@@ -1,6 +1,8 @@
 error[E0308]: mismatched types
   --> $DIR/default-body-type-err-2.rs:7:9
    |
+LL |     async fn woopsie_async(&self) -> String {
+   |                                      ------ expected `String` because of return type
 LL |         42
    |         ^^- help: try using a conversion method: `.to_string()`
    |         |
diff --git a/tests/ui/loops/dont-suggest-break-thru-item.rs b/tests/ui/loops/dont-suggest-break-thru-item.rs
@@ -8,6 +8,7 @@ fn closure() {
             if true {
                 Err(1)
                 //~^ ERROR mismatched types
+                //~| HELP you might have meant to return this value
             }
 
             Ok(())
@@ -21,6 +22,7 @@ fn async_block() {
             if true {
                 Err(1)
                 //~^ ERROR mismatched types
+                //~| HELP you might have meant to return this value
             }
 
             Ok(())
diff --git a/tests/ui/loops/dont-suggest-break-thru-item.stderr b/tests/ui/loops/dont-suggest-break-thru-item.stderr
@@ -5,27 +5,37 @@ LL | /             if true {
 LL | |                 Err(1)
    | |                 ^^^^^^ expected `()`, found `Result<_, {integer}>`
 LL | |
+LL | |
 LL | |             }
    | |_____________- expected this to be `()`
    |
    = note: expected unit type `()`
                    found enum `Result<_, {integer}>`
+help: you might have meant to return this value
+   |
+LL |                 return Err(1);
+   |                 ++++++       +
 
 error[E0308]: mismatched types
-  --> $DIR/dont-suggest-break-thru-item.rs:22:17
+  --> $DIR/dont-suggest-break-thru-item.rs:23:17
    |
 LL | /             if true {
 LL | |                 Err(1)
    | |                 ^^^^^^ expected `()`, found `Result<_, {integer}>`
 LL | |
+LL | |
 LL | |             }
    | |_____________- expected this to be `()`
    |
    = note: expected unit type `()`
                    found enum `Result<_, {integer}>`
+help: you might have meant to return this value
+   |
+LL |                 return Err(1);
+   |                 ++++++       +
 
 error[E0308]: mismatched types
-  --> $DIR/dont-suggest-break-thru-item.rs:35:17
+  --> $DIR/dont-suggest-break-thru-item.rs:37:17
    |
 LL | /             if true {
 LL | |                 Err(1)
@@ -38,7 +48,7 @@ LL | |             }
                    found enum `Result<_, {integer}>`
 
 error[E0308]: mismatched types
-  --> $DIR/dont-suggest-break-thru-item.rs:47:17
+  --> $DIR/dont-suggest-break-thru-item.rs:49:17
    |
 LL | /             if true {
 LL | |                 Err(1)

Original file line number	Diff line number	Diff line change
`@@ -1726,7 +1726,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {`
`1726`	`1726`	`}`
`1727`	`1727`
`1728`	`1728`	/// Given a function block's `HirId`, returns its `FnDecl` if it exists, or `None` otherwise.
`1729`		`- fn get_parent_fn_decl(&self, blk_id: hir::HirId) -> Option<(&'tcx hir::FnDecl<'tcx>, Ident)> {`
	`1729`	`+ pub(crate) fn get_parent_fn_decl(`
	`1730`	`+ &self,`
	`1731`	`+ blk_id: hir::HirId,`
	`1732`	`+ ) -> Option<(&'tcx hir::FnDecl<'tcx>, Ident)> {`
`1730`	`1733`	`let parent = self.tcx.hir_node_by_def_id(self.tcx.hir().get_parent_item(blk_id).def_id);`
`1731`	`1734`	`self.get_node_fn_decl(parent).map(\|(_, fn_decl, ident, _)\| (fn_decl, ident))`
`1732`	`1735`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1,6 +1,8 @@`
`1`	`1`	`error[E0308]: mismatched types`
`2`	`2`	`--> $DIR/default-body-type-err-2.rs:7:9`
`3`	`3`	`\|`
	`4`	`+LL \| async fn woopsie_async(&self) -> String {`
	`5`	+ \| ------ expected `String` because of return type
`4`	`6`	`LL \| 42`
`5`	`7`	\| ^^- help: try using a conversion method: `.to_string()`
`6`	`8`	`\| \|`
Original file line number	Diff line number	Diff line change
`@@ -8,6 +8,7 @@ fn closure() {`
`8`	`8`	`if true {`
`9`	`9`	`Err(1)`
`10`	`10`	`//~^ ERROR mismatched types`
	`11`	`+ //~\| HELP you might have meant to return this value`
`11`	`12`	`}`
`12`	`13`
`13`	`14`	`Ok(())`
`@@ -21,6 +22,7 @@ fn async_block() {`
`21`	`22`	`if true {`
`22`	`23`	`Err(1)`
`23`	`24`	`//~^ ERROR mismatched types`
	`25`	`+ //~\| HELP you might have meant to return this value`
`24`	`26`	`}`
`25`	`27`
`26`	`28`	`Ok(())`