Skip to content

Commit f9b5eb1

Browse files
committed
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 rust-lang#61788.
1 parent 9581d85 commit f9b5eb1

File tree

5 files changed

+194
-10
lines changed

5 files changed

+194
-10
lines changed

Diff for: compiler/rustc_hir_typeck/src/_match.rs

+79-9
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,11 @@
11
use crate::coercion::{AsCoercionSite, CoerceMany};
22
use crate::{Diverges, Expectation, FnCtxt, Needs};
3-
use rustc_errors::Diagnostic;
4-
use rustc_hir::{self as hir, ExprKind};
3+
use rustc_errors::{Applicability, Diagnostic};
4+
use rustc_hir::{
5+
self as hir,
6+
def::{CtorOf, DefKind, Res},
7+
ExprKind, PatKind,
8+
};
59
use rustc_hir_pretty::ty_to_string;
610
use rustc_infer::infer::type_variable::{TypeVariableOrigin, TypeVariableOriginKind};
711
use rustc_infer::traits::Obligation;
@@ -273,7 +277,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
273277
/// Returns `true` if there was an error forcing the coercion to the `()` type.
274278
pub(super) fn if_fallback_coercion<T>(
275279
&self,
276-
span: Span,
280+
if_span: Span,
281+
cond_expr: &'tcx hir::Expr<'tcx>,
277282
then_expr: &'tcx hir::Expr<'tcx>,
278283
coercion: &mut CoerceMany<'tcx, '_, T>,
279284
) -> bool
@@ -283,23 +288,88 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
283288
// If this `if` expr is the parent's function return expr,
284289
// the cause of the type coercion is the return type, point at it. (#25228)
285290
let hir_id = self.tcx.hir().parent_id(self.tcx.hir().parent_id(then_expr.hir_id));
286-
let ret_reason = self.maybe_get_coercion_reason(hir_id, span);
287-
let cause = self.cause(span, ObligationCauseCode::IfExpressionWithNoElse);
291+
let ret_reason = self.maybe_get_coercion_reason(hir_id, if_span);
292+
let cause = self.cause(if_span, ObligationCauseCode::IfExpressionWithNoElse);
288293
let mut error = false;
289294
coercion.coerce_forced_unit(
290295
self,
291296
&cause,
292297
|err| {
293-
if let Some((span, msg)) = &ret_reason {
294-
err.span_label(*span, msg.clone());
295-
} else if let ExprKind::Block(block, _) = &then_expr.kind
296-
&& let Some(expr) = &block.expr
298+
if let Some((if_span, msg)) = &ret_reason {
299+
err.span_label(*if_span, msg.clone());
300+
} else if let ExprKind::Block(block, _) = then_expr.kind
301+
&& let Some(expr) = block.expr
297302
{
298303
err.span_label(expr.span, "found here");
299304
}
300305
err.note("`if` expressions without `else` evaluate to `()`");
301306
err.help("consider adding an `else` block that evaluates to the expected type");
302307
error = true;
308+
if let ExprKind::Let(hir::Let { span, pat, init, .. }) = cond_expr.kind
309+
&& let ExprKind::Block(block, _) = then_expr.kind
310+
// Refutability checks occur on the MIR, so we approximate it here by checking
311+
// if we have an enum with a single variant or a struct in the pattern.
312+
&& let PatKind::TupleStruct(qpath, ..) | PatKind::Struct(qpath, ..) = pat.kind
313+
&& let hir::QPath::Resolved(_, path) = qpath
314+
{
315+
match path.res {
316+
Res::Def(DefKind::Ctor(CtorOf::Struct, _), _) => {
317+
// Structs are always irrefutable. Their fields might not be, but we
318+
// don't check for that here, it's only an approximation.
319+
}
320+
Res::Def(DefKind::Ctor(CtorOf::Variant, _), def_id)
321+
if self
322+
.tcx
323+
.adt_def(self.tcx.parent(self.tcx.parent(def_id)))
324+
.variants()
325+
.len()
326+
== 1 =>
327+
{
328+
// There's only a single variant in the `enum`, so we can suggest the
329+
// irrefutable `let` instead of `if let`.
330+
}
331+
_ => return,
332+
}
333+
334+
let mut sugg = vec![
335+
// Remove the `if`
336+
(if_span.until(*span), String::new()),
337+
];
338+
match (block.stmts, block.expr) {
339+
([first, ..], Some(expr)) => {
340+
let padding = self
341+
.tcx
342+
.sess
343+
.source_map()
344+
.indentation_before(first.span)
345+
.unwrap_or_else(|| String::new());
346+
sugg.extend([
347+
(init.span.between(first.span), format!(";\n{padding}")),
348+
(expr.span.shrink_to_hi().with_hi(block.span.hi()), String::new()),
349+
]);
350+
}
351+
([], Some(expr)) => {
352+
let padding = self
353+
.tcx
354+
.sess
355+
.source_map()
356+
.indentation_before(expr.span)
357+
.unwrap_or_else(|| String::new());
358+
sugg.extend([
359+
(init.span.between(expr.span), format!(";\n{padding}")),
360+
(expr.span.shrink_to_hi().with_hi(block.span.hi()), String::new()),
361+
]);
362+
}
363+
// If there's no value in the body, then the `if` expression would already
364+
// be of type `()`, so checking for those cases is unnecessary.
365+
(_, None) => return,
366+
}
367+
err.multipart_suggestion(
368+
"consider using an irrefutable `let` binding instead",
369+
sugg,
370+
Applicability::MaybeIncorrect,
371+
);
372+
}
303373
},
304374
false,
305375
);

Diff for: compiler/rustc_hir_typeck/src/expr.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1102,7 +1102,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
11021102
// We won't diverge unless both branches do (or the condition does).
11031103
self.diverges.set(cond_diverges | then_diverges & else_diverges);
11041104
} else {
1105-
self.if_fallback_coercion(sp, then_expr, &mut coerce);
1105+
self.if_fallback_coercion(sp, cond_expr, then_expr, &mut coerce);
11061106

11071107
// If the condition is false we can't diverge.
11081108
self.diverges.set(cond_diverges);
+25
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
// run-rustfix
2+
enum Enum {
3+
Variant(i32),
4+
}
5+
struct Struct(i32);
6+
7+
fn foo(x: Enum) -> i32 {
8+
let Enum::Variant(value) = x;
9+
value
10+
}
11+
fn bar(x: Enum) -> i32 {
12+
let Enum::Variant(value) = x;
13+
let x = value + 1;
14+
x
15+
}
16+
fn baz(x: Struct) -> i32 {
17+
let Struct(value) = x;
18+
let x = value + 1;
19+
x
20+
}
21+
fn main() {
22+
let _ = foo(Enum::Variant(42));
23+
let _ = bar(Enum::Variant(42));
24+
let _ = baz(Struct(42));
25+
}

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

+28
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
// run-rustfix
2+
enum Enum {
3+
Variant(i32),
4+
}
5+
struct Struct(i32);
6+
7+
fn foo(x: Enum) -> i32 {
8+
if let Enum::Variant(value) = x { //~ ERROR `if` may be missing an `else` clause
9+
value
10+
}
11+
}
12+
fn bar(x: Enum) -> i32 {
13+
if let Enum::Variant(value) = x { //~ ERROR `if` may be missing an `else` clause
14+
let x = value + 1;
15+
x
16+
}
17+
}
18+
fn baz(x: Struct) -> i32 {
19+
if let Struct(value) = x { //~ ERROR `if` may be missing an `else` clause
20+
let x = value + 1;
21+
x
22+
}
23+
}
24+
fn main() {
25+
let _ = foo(Enum::Variant(42));
26+
let _ = bar(Enum::Variant(42));
27+
let _ = baz(Struct(42));
28+
}
+61
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
error[E0317]: `if` may be missing an `else` clause
2+
--> $DIR/irrefutable-if-let-without-else.rs:8:5
3+
|
4+
LL | fn foo(x: Enum) -> i32 {
5+
| --- expected `i32` because of this return type
6+
LL | / if let Enum::Variant(value) = x {
7+
LL | | value
8+
LL | | }
9+
| |_____^ expected `i32`, found `()`
10+
|
11+
= note: `if` expressions without `else` evaluate to `()`
12+
= help: consider adding an `else` block that evaluates to the expected type
13+
help: consider using an irrefutable `let` binding instead
14+
|
15+
LL ~ let Enum::Variant(value) = x;
16+
LL ~ value
17+
|
18+
19+
error[E0317]: `if` may be missing an `else` clause
20+
--> $DIR/irrefutable-if-let-without-else.rs:13:5
21+
|
22+
LL | fn bar(x: Enum) -> i32 {
23+
| --- expected `i32` because of this return type
24+
LL | / if let Enum::Variant(value) = x {
25+
LL | | let x = value + 1;
26+
LL | | x
27+
LL | | }
28+
| |_____^ expected `i32`, found `()`
29+
|
30+
= note: `if` expressions without `else` evaluate to `()`
31+
= help: consider adding an `else` block that evaluates to the expected type
32+
help: consider using an irrefutable `let` binding instead
33+
|
34+
LL ~ let Enum::Variant(value) = x;
35+
LL ~ let x = value + 1;
36+
LL ~ x
37+
|
38+
39+
error[E0317]: `if` may be missing an `else` clause
40+
--> $DIR/irrefutable-if-let-without-else.rs:19:5
41+
|
42+
LL | fn baz(x: Struct) -> i32 {
43+
| --- expected `i32` because of this return type
44+
LL | / if let Struct(value) = x {
45+
LL | | let x = value + 1;
46+
LL | | x
47+
LL | | }
48+
| |_____^ expected `i32`, found `()`
49+
|
50+
= note: `if` expressions without `else` evaluate to `()`
51+
= help: consider adding an `else` block that evaluates to the expected type
52+
help: consider using an irrefutable `let` binding instead
53+
|
54+
LL ~ let Struct(value) = x;
55+
LL ~ let x = value + 1;
56+
LL ~ x
57+
|
58+
59+
error: aborting due to 3 previous errors
60+
61+
For more information about this error, try `rustc --explain E0317`.

0 commit comments

Comments
 (0)