address review comments

Author: estebank

src/librustc_typeck/check/mod.rs

@@ -3474,37 +3474,16 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
         } else {
             // If this `if` expr is the parent's function return expr, the cause of the type
             // coercion is the return type, point at it. (#25228)
-            let mut ret_reason = None;
-            if let Node::Block(block) = self.tcx.hir().get_by_hir_id(
-                self.tcx.hir().get_parent_node_by_hir_id(
-                    self.tcx.hir().get_parent_node_by_hir_id(then_expr.hir_id),
-                ),
-            ) {
-                // check that the body's parent is an fn
-                let parent = self.tcx.hir().get_by_hir_id(
-                    self.tcx.hir().get_parent_node_by_hir_id(
-                        self.tcx.hir().get_parent_node_by_hir_id(block.hir_id),
-                    ),
-                );
-                if let (Some(expr), Node::Item(hir::Item {
-                    node: hir::ItemKind::Fn(..), ..
-                })) = (&block.expr, parent) {
-                    // check that the `if` expr without `else` is the fn body's expr
-                    if expr.span == sp {
-                        ret_reason = self.get_fn_decl(then_expr.hir_id).map(|(fn_decl, _)| (
-                            fn_decl.output.span(),
-                            format!("found `{}` because of this return type", fn_decl.output),
-                        ));
-                    }
-                }
-            }
+            let ret_reason = self.maybe_get_coercion_reason(then_expr.hir_id, sp);
+
             let else_cause = self.cause(sp, ObligationCauseCode::IfExpressionWithNoElse);
             coerce.coerce_forced_unit(self, &else_cause, &mut |err| {
                 if let Some((sp, msg)) = &ret_reason {
                     err.span_label(*sp, msg.as_str());
                 }
-                err.note("`if` expressions without `else` must evaluate to `()`");
-            }, true);
+                err.note("`if` expressions without `else` evaluate to `()`");
+                err.help("consider adding an `else` block that evaluates to the expected type");
+            }, ret_reason.is_none());
 
             // If the condition is false we can't diverge.
             self.diverges.set(cond_diverges);
@@ -3518,6 +3497,33 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
         }
     }
 
+    fn maybe_get_coercion_reason(&self, hir_id: hir::HirId, sp: Span) -> Option<(Span, String)> {
+        if let Node::Block(block) = self.tcx.hir().get_by_hir_id(
+            self.tcx.hir().get_parent_node_by_hir_id(
+                self.tcx.hir().get_parent_node_by_hir_id(hir_id),
+            ),
+        ) {
+            // check that the body's parent is an fn
+            let parent = self.tcx.hir().get_by_hir_id(
+                self.tcx.hir().get_parent_node_by_hir_id(
+                    self.tcx.hir().get_parent_node_by_hir_id(block.hir_id),
+                ),
+            );
+            if let (Some(expr), Node::Item(hir::Item {
+                node: hir::ItemKind::Fn(..), ..
+            })) = (&block.expr, parent) {
+                // 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, _)| (
+                        fn_decl.output.span(),
+                        format!("expected `{}` because of this return type", fn_decl.output),
+                    ));
+                }
+            }
+        }
+        None
+    }
+
     // Check field access expressions
     fn check_field(&self,
                    expr: &'gcx hir::Expr,

src/test/ui/if/if-without-else-as-fn-expr.stderr

@@ -2,15 +2,16 @@ error[E0317]: if may be missing an else clause
   --> $DIR/if-without-else-as-fn-expr.rs:2:5
    |
 LL |   fn foo(bar: usize) -> usize {
-   |                         ----- found `usize` because of this return type
+   |                         ----- expected `usize` because of this return type
 LL | /     if bar % 5 == 0 {
 LL | |         return 3;
 LL | |     }
-   | |_____^ expected (), found usize
+   | |_____^ expected usize, found ()
    |
-   = note: expected type `()`
-              found type `usize`
-   = note: `if` expressions without `else` must evaluate to `()`
+   = note: expected type `usize`
+              found type `()`
+   = note: `if` expressions without `else` evaluate to `()`
+   = help: consider adding an `else` block that evaluates to the expected type
 
 error: aborting due to previous error
 

src/test/ui/if/if-without-else-result.stderr

@@ -6,7 +6,8 @@ LL |     let a = if true { true };
    |
    = note: expected type `()`
               found type `bool`
-   = note: `if` expressions without `else` must evaluate to `()`
+   = note: `if` expressions without `else` evaluate to `()`
+   = help: consider adding an `else` block that evaluates to the expected type
 
 error: aborting due to previous error
 

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

@@ -13,7 +13,8 @@ LL | |     };
    |
    = note: expected type `()`
               found type `{integer}`
-   = note: `if` expressions without `else` must evaluate to `()`
+   = note: `if` expressions without `else` evaluate to `()`
+   = help: consider adding an `else` block that evaluates to the expected type
 
 error: aborting due to previous error
 

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

@@ -6,7 +6,8 @@ LL |         Drop = assert_eq!(1, 1)
    |
    = note: expected type `()`
               found type `isize`
-   = note: `if` expressions without `else` must evaluate to `()`
+   = note: `if` expressions without `else` evaluate to `()`
+   = help: consider adding an `else` block that evaluates to the expected type
    = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)
 
 error: aborting due to previous error