Suggest turnging if let into irrefutable let if appropriate

When encountering an `if let` tail expression without an `else` arm for an enum with a single variant, suggest writing an irrefutable `let` binding instead.

```
error[E0317]: `if` may be missing an `else` clause
  --> $DIR/irrefutable-if-let-without-else.rs:8:5
   |
LL |   fn foo(x: Enum) -> i32 {
   |                      --- expected `i32` because of this return type
LL | /     if let Enum::Variant(value) = x {
LL | |         value
LL | |     }
   | |_____^ expected `i32`, found `()`
   |
   = note: `if` expressions without `else` evaluate to `()`
   = help: consider adding an `else` block that evaluates to the expected type
help: consider using an irrefutable `let` binding instead
   |
LL ~     let Enum::Variant(value) = x;
LL ~         value
   |
```

Fix #61788.

Author: estebank

compiler/rustc_hir_typeck/src/_match.rs

@@ -1,7 +1,11 @@
 use crate::coercion::{AsCoercionSite, CoerceMany};
 use crate::{Diverges, Expectation, FnCtxt, Needs};
-use rustc_errors::Diagnostic;
-use rustc_hir::{self as hir, ExprKind};
+use rustc_errors::{Applicability, Diagnostic};
+use rustc_hir::{
+    self as hir,
+    def::{CtorOf, DefKind, Res},
+    ExprKind, PatKind,
+};
 use rustc_hir_pretty::ty_to_string;
 use rustc_infer::infer::type_variable::{TypeVariableOrigin, TypeVariableOriginKind};
 use rustc_infer::traits::Obligation;
@@ -273,7 +277,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
     /// Returns `true` if there was an error forcing the coercion to the `()` type.
     pub(super) fn if_fallback_coercion<T>(
         &self,
-        span: Span,
+        if_span: Span,
+        cond_expr: &'tcx hir::Expr<'tcx>,
         then_expr: &'tcx hir::Expr<'tcx>,
         coercion: &mut CoerceMany<'tcx, '_, T>,
     ) -> bool
@@ -283,23 +288,88 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         // 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 hir_id = self.tcx.hir().parent_id(self.tcx.hir().parent_id(then_expr.hir_id));
-        let ret_reason = self.maybe_get_coercion_reason(hir_id, span);
-        let cause = self.cause(span, ObligationCauseCode::IfExpressionWithNoElse);
+        let ret_reason = self.maybe_get_coercion_reason(hir_id, if_span);
+        let cause = self.cause(if_span, ObligationCauseCode::IfExpressionWithNoElse);
         let mut error = false;
         coercion.coerce_forced_unit(
             self,
             &cause,
             |err| {
-                if let Some((span, msg)) = &ret_reason {
-                    err.span_label(*span, msg.clone());
-                } else if let ExprKind::Block(block, _) = &then_expr.kind
-                    && let Some(expr) = &block.expr
+                if let Some((if_span, msg)) = &ret_reason {
+                    err.span_label(*if_span, msg.clone());
+                } else if let ExprKind::Block(block, _) = then_expr.kind
+                    && let Some(expr) = block.expr
                 {
                     err.span_label(expr.span, "found here");
                 }
                 err.note("`if` expressions without `else` evaluate to `()`");
                 err.help("consider adding an `else` block that evaluates to the expected type");
                 error = true;
+                if let ExprKind::Let(hir::Let { span, pat, init, .. }) = cond_expr.kind
+                    && let ExprKind::Block(block, _) = then_expr.kind
+                    // Refutability checks occur on the MIR, so we approximate it here by checking
+                    // if we have an enum with a single variant or a struct in the pattern.
+                    && let PatKind::TupleStruct(qpath, ..) | PatKind::Struct(qpath, ..) = pat.kind
+                    && let hir::QPath::Resolved(_, path) = qpath
+                {
+                    match path.res {
+                        Res::Def(DefKind::Ctor(CtorOf::Struct, _), _) => {
+                            // Structs are always irrefutable. Their fields might not be, but we
+                            // don't check for that here, it's only an approximation.
+                        }
+                        Res::Def(DefKind::Ctor(CtorOf::Variant, _), def_id)
+                            if self
+                                .tcx
+                                .adt_def(self.tcx.parent(self.tcx.parent(def_id)))
+                                .variants()
+                                .len()
+                                == 1 =>
+                        {
+                            // There's only a single variant in the `enum`, so we can suggest the
+                            // irrefutable `let` instead of `if let`.
+                        }
+                        _ => return,
+                    }
+
+                    let mut sugg = vec![
+                        // Remove the `if`
+                        (if_span.until(*span), String::new()),
+                    ];
+                    match (block.stmts, block.expr) {
+                        ([first, ..], Some(expr)) => {
+                            let padding = self
+                                .tcx
+                                .sess
+                                .source_map()
+                                .indentation_before(first.span)
+                                .unwrap_or_else(|| String::new());
+                            sugg.extend([
+                                (init.span.between(first.span), format!(";\n{padding}")),
+                                (expr.span.shrink_to_hi().with_hi(block.span.hi()), String::new()),
+                            ]);
+                        }
+                        ([], Some(expr)) => {
+                            let padding = self
+                                .tcx
+                                .sess
+                                .source_map()
+                                .indentation_before(expr.span)
+                                .unwrap_or_else(|| String::new());
+                            sugg.extend([
+                                (init.span.between(expr.span), format!(";\n{padding}")),
+                                (expr.span.shrink_to_hi().with_hi(block.span.hi()), String::new()),
+                            ]);
+                        }
+                        // If there's no value in the body, then the `if` expression would already
+                        // be of type `()`, so checking for those cases is unnecessary.
+                        (_, None) => return,
+                    }
+                    err.multipart_suggestion(
+                        "consider using an irrefutable `let` binding instead",
+                        sugg,
+                        Applicability::MaybeIncorrect,
+                    );
+                }
             },
             false,
         );

compiler/rustc_hir_typeck/src/expr.rs

@@ -1102,7 +1102,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             // We won't diverge unless both branches do (or the condition does).
             self.diverges.set(cond_diverges | then_diverges & else_diverges);
         } else {
-            self.if_fallback_coercion(sp, then_expr, &mut coerce);
+            self.if_fallback_coercion(sp, cond_expr, then_expr, &mut coerce);
 
             // If the condition is false we can't diverge.
             self.diverges.set(cond_diverges);

tests/ui/binding/irrefutable-if-let-without-else.fixed

@@ -0,0 +1,25 @@
+// run-rustfix
+enum Enum {
+    Variant(i32),
+}
+struct Struct(i32);
+
+fn foo(x: Enum) -> i32 {
+    let Enum::Variant(value) = x;
+        value
+}
+fn bar(x: Enum) -> i32 {
+    let Enum::Variant(value) = x;
+        let x = value + 1;
+        x
+}
+fn baz(x: Struct) -> i32 {
+    let Struct(value) = x;
+        let x = value + 1;
+        x
+}
+fn main() {
+    let _ = foo(Enum::Variant(42));
+    let _ = bar(Enum::Variant(42));
+    let _ = baz(Struct(42));
+}

tests/ui/binding/irrefutable-if-let-without-else.rs

@@ -0,0 +1,28 @@
+// run-rustfix
+enum Enum {
+    Variant(i32),
+}
+struct Struct(i32);
+
+fn foo(x: Enum) -> i32 {
+    if let Enum::Variant(value) = x { //~ ERROR `if` may be missing an `else` clause
+        value
+    }
+}
+fn bar(x: Enum) -> i32 {
+    if let Enum::Variant(value) = x { //~ ERROR `if` may be missing an `else` clause
+        let x = value + 1;
+        x
+    }
+}
+fn baz(x: Struct) -> i32 {
+    if let Struct(value) = x { //~ ERROR `if` may be missing an `else` clause
+        let x = value + 1;
+        x
+    }
+}
+fn main() {
+    let _ = foo(Enum::Variant(42));
+    let _ = bar(Enum::Variant(42));
+    let _ = baz(Struct(42));
+}

tests/ui/binding/irrefutable-if-let-without-else.stderr

@@ -0,0 +1,61 @@
+error[E0317]: `if` may be missing an `else` clause
+  --> $DIR/irrefutable-if-let-without-else.rs:8:5
+   |
+LL |   fn foo(x: Enum) -> i32 {
+   |                      --- expected `i32` because of this return type
+LL | /     if let Enum::Variant(value) = x {
+LL | |         value
+LL | |     }
+   | |_____^ expected `i32`, found `()`
+   |
+   = note: `if` expressions without `else` evaluate to `()`
+   = help: consider adding an `else` block that evaluates to the expected type
+help: consider using an irrefutable `let` binding instead
+   |
+LL ~     let Enum::Variant(value) = x;
+LL ~         value
+   |
+
+error[E0317]: `if` may be missing an `else` clause
+  --> $DIR/irrefutable-if-let-without-else.rs:13:5
+   |
+LL |   fn bar(x: Enum) -> i32 {
+   |                      --- expected `i32` because of this return type
+LL | /     if let Enum::Variant(value) = x {
+LL | |         let x = value + 1;
+LL | |         x
+LL | |     }
+   | |_____^ expected `i32`, found `()`
+   |
+   = note: `if` expressions without `else` evaluate to `()`
+   = help: consider adding an `else` block that evaluates to the expected type
+help: consider using an irrefutable `let` binding instead
+   |
+LL ~     let Enum::Variant(value) = x;
+LL ~         let x = value + 1;
+LL ~         x
+   |
+
+error[E0317]: `if` may be missing an `else` clause
+  --> $DIR/irrefutable-if-let-without-else.rs:19:5
+   |
+LL |   fn baz(x: Struct) -> i32 {
+   |                        --- expected `i32` because of this return type
+LL | /     if let Struct(value) = x {
+LL | |         let x = value + 1;
+LL | |         x
+LL | |     }
+   | |_____^ expected `i32`, found `()`
+   |
+   = note: `if` expressions without `else` evaluate to `()`
+   = help: consider adding an `else` block that evaluates to the expected type
+help: consider using an irrefutable `let` binding instead
+   |
+LL ~     let Struct(value) = x;
+LL ~         let x = value + 1;
+LL ~         x
+   |
+
+error: aborting due to 3 previous errors
+
+For more information about this error, try `rustc --explain E0317`.