diff options
| -rw-r--r-- | compiler/rustc_hir_typeck/src/_match.rs | 116 | ||||
| -rw-r--r-- | compiler/rustc_hir_typeck/src/expr.rs | 2 | ||||
| -rw-r--r-- | tests/ui/binding/irrefutable-if-let-without-else.fixed | 25 | ||||
| -rw-r--r-- | tests/ui/binding/irrefutable-if-let-without-else.rs | 28 | ||||
| -rw-r--r-- | tests/ui/binding/irrefutable-if-let-without-else.stderr | 61 |
5 files changed, 214 insertions, 18 deletions
diff --git a/compiler/rustc_hir_typeck/src/_match.rs b/compiler/rustc_hir_typeck/src/_match.rs index cf1f232229d..7ea0469dedd 100644 --- a/compiler/rustc_hir_typeck/src/_match.rs +++ b/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,29 +288,106 @@ 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 - { - 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; - }, + |err| self.explain_if_expr(err, ret_reason, if_span, cond_expr, then_expr, &mut error), false, ); error } + /// Explain why `if` expressions without `else` evaluate to `()` and detect likely irrefutable + /// `if let PAT = EXPR {}` expressions that could be turned into `let PAT = EXPR;`. + fn explain_if_expr( + &self, + err: &mut Diagnostic, + ret_reason: Option<(Span, String)>, + if_span: Span, + cond_expr: &'tcx hir::Expr<'tcx>, + then_expr: &'tcx hir::Expr<'tcx>, + error: &mut bool, + ) { + 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, + ); + } + } + pub fn maybe_get_coercion_reason( &self, hir_id: hir::HirId, diff --git a/compiler/rustc_hir_typeck/src/expr.rs b/compiler/rustc_hir_typeck/src/expr.rs index 1adde8c21b8..54664f38b19 100644 --- a/compiler/rustc_hir_typeck/src/expr.rs +++ b/compiler/rustc_hir_typeck/src/expr.rs @@ -1118,7 +1118,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); diff --git a/tests/ui/binding/irrefutable-if-let-without-else.fixed b/tests/ui/binding/irrefutable-if-let-without-else.fixed new file mode 100644 index 00000000000..3d7f4695ca8 --- /dev/null +++ b/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)); +} diff --git a/tests/ui/binding/irrefutable-if-let-without-else.rs b/tests/ui/binding/irrefutable-if-let-without-else.rs new file mode 100644 index 00000000000..5aaf4ace3f8 --- /dev/null +++ b/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)); +} diff --git a/tests/ui/binding/irrefutable-if-let-without-else.stderr b/tests/ui/binding/irrefutable-if-let-without-else.stderr new file mode 100644 index 00000000000..e234cfdd945 --- /dev/null +++ b/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`. |