Auto merge of #57230 - estebank:return-mismatch, r=varkor

Modify mismatched type error for functions with no return

Fix #50009.

```
error[E0308]: mismatched types
  --> $DIR/coercion-missing-tail-expected-type.rs:3:24
   |
LL | fn plus_one(x: i32) -> i32 { //~ ERROR mismatched types
   |    --------            ^^^ expected i32, found ()
   |    |
   |    this function's body doesn't return
LL |     x + 1;
   |          - help: consider removing this semicolon
   |
   = note: expected type `i32`
              found type `()`
```

instead of

```
error[E0308]: mismatched types
  --> $DIR/coercion-missing-tail-expected-type.rs:3:28
   |
LL |   fn plus_one(x: i32) -> i32 { //~ ERROR mismatched types
   |  ____________________________^
LL | |     x + 1;
   | |          - help: consider removing this semicolon
LL | | }
   | |_^ expected i32, found ()
   |
   = note: expected type `i32`
              found type `()`
```

Author: bors

src/librustc/hir/map/mod.rs

@@ -667,6 +667,7 @@ impl<'hir> Map<'hir> {
                 Node::Item(_) |
                 Node::ForeignItem(_) |
                 Node::TraitItem(_) |
+                Node::Expr(Expr { node: ExprKind::Closure(..), ..}) |
                 Node::ImplItem(_) => true,
                 _ => false,
             }
@@ -675,7 +676,7 @@ impl<'hir> Map<'hir> {
             match *node {
                 Node::Expr(ref expr) => {
                     match expr.node {
-                        ExprKind::While(..) | ExprKind::Loop(..) => true,
+                        ExprKind::While(..) | ExprKind::Loop(..) | ExprKind::Ret(..) => true,
                         _ => false,
                     }
                 }

src/librustc_typeck/check/mod.rs

@@ -4843,11 +4843,31 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
                 // #41425 -- label the implicit `()` as being the
                 // "found type" here, rather than the "expected type".
                 if !self.diverges.get().always() {
-                    coerce.coerce_forced_unit(self, &self.misc(blk.span), &mut |err| {
+                    // #50009 -- Do not point at the entire fn block span, point at the return type
+                    // span, as it is the cause of the requirement, and
+                    // `consider_hint_about_removing_semicolon` will point at the last expression
+                    // if it were a relevant part of the error. This improves usability in editors
+                    // that highlight errors inline.
+                    let mut sp = blk.span;
+                    let mut fn_span = None;
+                    if let Some((decl, ident)) = self.get_parent_fn_decl(blk.id) {
+                        let ret_sp = decl.output.span();
+                        if let Some(block_sp) = self.parent_item_span(blk.id) {
+                            // HACK: on some cases (`ui/liveness/liveness-issue-2163.rs`) the
+                            // output would otherwise be incorrect and even misleading. Make sure
+                            // the span we're aiming at correspond to a `fn` body.
+                            if block_sp == blk.span {
+                                sp = ret_sp;
+                                fn_span = Some(ident.span);
+                            }
+                        }
+                    }
+                    coerce.coerce_forced_unit(self, &self.misc(sp), &mut |err| {
                         if let Some(expected_ty) = expected.only_has_type(self) {
-                            self.consider_hint_about_removing_semicolon(blk,
-                                                                        expected_ty,
-                                                                        err);
+                            self.consider_hint_about_removing_semicolon(blk, expected_ty, err);
+                        }
+                        if let Some(fn_span) = fn_span {
+                            err.span_label(fn_span, "this function's body doesn't return");
                         }
                     }, false);
                 }
@@ -4872,59 +4892,81 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
         ty
     }
 
-    /// Given a `NodeId`, return the `FnDecl` of the method it is enclosed by and whether a
-    /// suggestion can be made, `None` otherwise.
-    pub fn get_fn_decl(&self, blk_id: ast::NodeId) -> Option<(hir::FnDecl, bool)> {
-        // 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.
-        if let Some(fn_id) = self.tcx.hir().get_return_block(blk_id) {
-            let parent = self.tcx.hir().get(fn_id);
+    fn parent_item_span(&self, id: ast::NodeId) -> Option<Span> {
+        let node = self.tcx.hir().get(self.tcx.hir().get_parent(id));
+        match node {
+            Node::Item(&hir::Item {
+                node: hir::ItemKind::Fn(_, _, _, body_id), ..
+            }) |
+            Node::ImplItem(&hir::ImplItem {
+                node: hir::ImplItemKind::Method(_, body_id), ..
+            }) => {
+                let body = self.tcx.hir().body(body_id);
+                if let ExprKind::Block(block, _) = &body.value.node {
+                    return Some(block.span);
+                }
+            }
+            _ => {}
+        }
+        None
+    }
 
-            if let Node::Item(&hir::Item {
+    /// Given a function block's `NodeId`, return its `FnDecl` if it exists, or `None` otherwise.
+    fn get_parent_fn_decl(&self, blk_id: ast::NodeId) -> Option<(hir::FnDecl, ast::Ident)> {
+        let parent = self.tcx.hir().get(self.tcx.hir().get_parent(blk_id));
+        self.get_node_fn_decl(parent).map(|(fn_decl, ident, _)| (fn_decl, ident))
+    }
+
+    /// Given a function `Node`, return its `FnDecl` if it exists, or `None` otherwise.
+    fn get_node_fn_decl(&self, node: Node) -> Option<(hir::FnDecl, ast::Ident, bool)> {
+        match node {
+            Node::Item(&hir::Item {
                 ident, node: hir::ItemKind::Fn(ref decl, ..), ..
-            }) = parent {
-                decl.clone().and_then(|decl| {
-                    // 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((decl, ident.name != Symbol::intern("main")))
-                })
-            } else if let Node::TraitItem(&hir::TraitItem {
-                node: hir::TraitItemKind::Method(hir::MethodSig {
+            }) => decl.clone().and_then(|decl| {
+                // 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((decl, ident, ident.name != Symbol::intern("main")))
+            }),
+            Node::TraitItem(&hir::TraitItem {
+                ident, node: hir::TraitItemKind::Method(hir::MethodSig {
                     ref decl, ..
                 }, ..), ..
-            }) = parent {
-                decl.clone().and_then(|decl| {
-                    Some((decl, true))
-                })
-            } else if let Node::ImplItem(&hir::ImplItem {
-                node: hir::ImplItemKind::Method(hir::MethodSig {
+            }) => decl.clone().and_then(|decl| Some((decl, ident, true))),
+            Node::ImplItem(&hir::ImplItem {
+                ident, node: hir::ImplItemKind::Method(hir::MethodSig {
                     ref decl, ..
                 }, ..), ..
-            }) = parent {
-                decl.clone().and_then(|decl| {
-                    Some((decl, false))
-                })
-            } else {
-                None
-            }
-        } else {
-            None
+            }) => decl.clone().and_then(|decl| Some((decl, ident, false))),
+            _ => None,
         }
     }
 
+    /// Given a `NodeId`, return the `FnDecl` of the method it is enclosed by and whether a
+    /// suggestion can be made, `None` otherwise.
+    pub fn get_fn_decl(&self, blk_id: ast::NodeId) -> Option<(hir::FnDecl, bool)> {
+        // 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_return_block(blk_id).and_then(|blk_id| {
+            let parent = self.tcx.hir().get(blk_id);
+            self.get_node_fn_decl(parent).map(|(fn_decl, _, is_main)| (fn_decl, is_main))
+        })
+    }
+
     /// On implicit return expressions with mismatched types, provide the following suggestions:
     ///
     ///  - Point out the method's return type as the reason for the expected type
     ///  - Possible missing semicolon
     ///  - Possible missing return type if the return type is the default, and not `fn main()`
-    pub fn suggest_mismatched_types_on_tail(&self,
-                                            err: &mut DiagnosticBuilder<'tcx>,
-                                            expression: &'gcx hir::Expr,
-                                            expected: Ty<'tcx>,
-                                            found: Ty<'tcx>,
-                                            cause_span: Span,
-                                            blk_id: ast::NodeId) {
+    pub fn suggest_mismatched_types_on_tail(
+        &self,
+        err: &mut DiagnosticBuilder<'tcx>,
+        expression: &'gcx hir::Expr,
+        expected: Ty<'tcx>,
+        found: Ty<'tcx>,
+        cause_span: Span,
+        blk_id: ast::NodeId,
+    ) {
         self.suggest_missing_semicolon(err, expression, expected, cause_span);
         if let Some((fn_decl, can_suggest)) = self.get_fn_decl(blk_id) {
             self.suggest_missing_return_type(err, &fn_decl, expected, found, can_suggest);

src/test/ui/block-result/consider-removing-last-semi.stderr

@@ -1,27 +1,27 @@
 error[E0308]: mismatched types
-  --> $DIR/consider-removing-last-semi.rs:1:18
+  --> $DIR/consider-removing-last-semi.rs:1:11
    |
-LL |   fn f() -> String {  //~ ERROR mismatched types
-   |  __________________^
-LL | |     0u8;
-LL | |     "bla".to_string();
-   | |                      - help: consider removing this semicolon
-LL | | }
-   | |_^ expected struct `std::string::String`, found ()
+LL | fn f() -> String {  //~ ERROR mismatched types
+   |    -      ^^^^^^ expected struct `std::string::String`, found ()
+   |    |
+   |    this function's body doesn't return
+LL |     0u8;
+LL |     "bla".to_string();
+   |                      - help: consider removing this semicolon
    |
    = note: expected type `std::string::String`
               found type `()`
 
 error[E0308]: mismatched types
-  --> $DIR/consider-removing-last-semi.rs:6:18
+  --> $DIR/consider-removing-last-semi.rs:6:11
    |
-LL |   fn g() -> String {  //~ ERROR mismatched types
-   |  __________________^
-LL | |     "this won't work".to_string();
-LL | |     "removeme".to_string();
-   | |                           - help: consider removing this semicolon
-LL | | }
-   | |_^ expected struct `std::string::String`, found ()
+LL | fn g() -> String {  //~ ERROR mismatched types
+   |    -      ^^^^^^ expected struct `std::string::String`, found ()
+   |    |
+   |    this function's body doesn't return
+LL |     "this won't work".to_string();
+LL |     "removeme".to_string();
+   |                           - help: consider removing this semicolon
    |
    = note: expected type `std::string::String`
               found type `()`

src/test/ui/block-result/issue-11714.stderr

@@ -1,14 +1,13 @@
 error[E0308]: mismatched types
-  --> $DIR/issue-11714.rs:1:18
+  --> $DIR/issue-11714.rs:1:14
    |
-LL |   fn blah() -> i32 { //~ ERROR mismatched types
-   |  __________________^
-LL | |     1
-LL | |
-LL | |     ;
-   | |     - help: consider removing this semicolon
-LL | | }
-   | |_^ expected i32, found ()
+LL | fn blah() -> i32 { //~ ERROR mismatched types
+   |    ----      ^^^ expected i32, found ()
+   |    |
+   |    this function's body doesn't return
+...
+LL |     ;
+   |     - help: consider removing this semicolon
    |
    = note: expected type `i32`
               found type `()`

src/test/ui/block-result/issue-13428.stderr

@@ -1,30 +1,27 @@
 error[E0308]: mismatched types
-  --> $DIR/issue-13428.rs:3:20
+  --> $DIR/issue-13428.rs:3:13
    |
-LL |   fn foo() -> String {  //~ ERROR mismatched types
-   |  ____________________^
-LL | |     format!("Hello {}",
-LL | |             "world")
-LL | |     // Put the trailing semicolon on its own line to test that the
-LL | |     // note message gets the offending semicolon exactly
-LL | |     ;
-   | |     - help: consider removing this semicolon
-LL | | }
-   | |_^ expected struct `std::string::String`, found ()
+LL | fn foo() -> String {  //~ ERROR mismatched types
+   |    ---      ^^^^^^ expected struct `std::string::String`, found ()
+   |    |
+   |    this function's body doesn't return
+...
+LL |     ;
+   |     - help: consider removing this semicolon
    |
    = note: expected type `std::string::String`
               found type `()`
 
 error[E0308]: mismatched types
-  --> $DIR/issue-13428.rs:11:20
+  --> $DIR/issue-13428.rs:11:13
    |
-LL |   fn bar() -> String {  //~ ERROR mismatched types
-   |  ____________________^
-LL | |     "foobar".to_string()
-LL | |     ;
-   | |     - help: consider removing this semicolon
-LL | | }
-   | |_^ expected struct `std::string::String`, found ()
+LL | fn bar() -> String {  //~ ERROR mismatched types
+   |    ---      ^^^^^^ expected struct `std::string::String`, found ()
+   |    |
+   |    this function's body doesn't return
+LL |     "foobar".to_string()
+LL |     ;
+   |     - help: consider removing this semicolon
    |
    = note: expected type `std::string::String`
               found type `()`

src/test/ui/coercion/coercion-missing-tail-expected-type.stderr

@@ -1,25 +1,25 @@
 error[E0308]: mismatched types
-  --> $DIR/coercion-missing-tail-expected-type.rs:3:28
+  --> $DIR/coercion-missing-tail-expected-type.rs:3:24
    |
-LL |   fn plus_one(x: i32) -> i32 { //~ ERROR mismatched types
-   |  ____________________________^
-LL | |     x + 1;
-   | |          - help: consider removing this semicolon
-LL | | }
-   | |_^ expected i32, found ()
+LL | fn plus_one(x: i32) -> i32 { //~ ERROR mismatched types
+   |    --------            ^^^ expected i32, found ()
+   |    |
+   |    this function's body doesn't return
+LL |     x + 1;
+   |          - help: consider removing this semicolon
    |
    = note: expected type `i32`
               found type `()`
 
 error[E0308]: mismatched types
-  --> $DIR/coercion-missing-tail-expected-type.rs:7:29
+  --> $DIR/coercion-missing-tail-expected-type.rs:7:13
    |
-LL |   fn foo() -> Result<u8, u64> { //~ ERROR mismatched types
-   |  _____________________________^
-LL | |     Ok(1);
-   | |          - help: consider removing this semicolon
-LL | | }
-   | |_^ expected enum `std::result::Result`, found ()
+LL | fn foo() -> Result<u8, u64> { //~ ERROR mismatched types
+   |    ---      ^^^^^^^^^^^^^^^ expected enum `std::result::Result`, found ()
+   |    |
+   |    this function's body doesn't return
+LL |     Ok(1);
+   |          - help: consider removing this semicolon
    |
    = note: expected type `std::result::Result<u8, u64>`
               found type `()`

src/test/ui/issues/issue-32323.stderr

@@ -1,8 +1,10 @@
 error[E0308]: mismatched types
-  --> $DIR/issue-32323.rs:5:49
+  --> $DIR/issue-32323.rs:5:30
    |
 LL | pub fn f<'a, T: Tr<'a>>() -> <T as Tr<'a>>::Out {}
-   |                                                 ^^ expected associated type, found ()
+   |        -                     ^^^^^^^^^^^^^^^^^^ expected associated type, found ()
+   |        |
+   |        this function's body doesn't return
    |
    = note: expected type `<T as Tr<'a>>::Out`
               found type `()`

src/test/ui/issues/issue-43162.stderr

@@ -11,15 +11,15 @@ LL |     break {}; //~ ERROR E0268
    |     ^^^^^^^^ cannot break outside of a loop
 
 error[E0308]: mismatched types
-  --> $DIR/issue-43162.rs:1:18
+  --> $DIR/issue-43162.rs:1:13
    |
-LL |   fn foo() -> bool {
-   |  __________________^
-LL | |     //~^ ERROR E0308
-LL | |     break true; //~ ERROR E0268
-   | |               - help: consider removing this semicolon
-LL | | }
-   | |_^ expected bool, found ()
+LL | fn foo() -> bool {
+   |    ---      ^^^^ expected bool, found ()
+   |    |
+   |    this function's body doesn't return
+LL |     //~^ ERROR E0308
+LL |     break true; //~ ERROR E0268
+   |               - help: consider removing this semicolon
    |
    = note: expected type `bool`
               found type `()`

src/test/ui/issues/issue-44023.stderr

@@ -1,10 +1,10 @@
 error[E0308]: mismatched types
-  --> $DIR/issue-44023.rs:5:42
+  --> $DIR/issue-44023.rs:5:36
    |
-LL |   fn საჭმელად_გემრიელი_სადილი ( ) -> isize { //~ ERROR mismatched types
-   |  __________________________________________^
-LL | | }
-   | |_^ expected isize, found ()
+LL | fn საჭმელად_გემრიელი_სადილი ( ) -> isize { //~ ERROR mismatched types
+   |    ------------------------        ^^^^^ expected isize, found ()
+   |    |
+   |    this function's body doesn't return
    |
    = note: expected type `isize`
               found type `()`