Skip to content

Commit 94d6023

Browse files
WeiTheShinobiWeiTheShinobi
committed
Tweak irrefutable let else warning output, add new suggestion
1 parent 562dee4 commit 94d6023

8 files changed

Lines changed: 166 additions & 132 deletions

File tree

‎compiler/rustc_mir_build/src/errors.rs‎

Lines changed: 17 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -924,22 +924,24 @@ pub(crate) struct IrrefutableLetPatternsIfLetGuard {
924924
}
925925

926926
#[derive(Diagnostic)]
927-
#[diag(
928-
"irrefutable `let...else` {$count ->
929-
[one] pattern
930-
*[other] patterns
931-
}"
932-
)]
933-
#[note(
934-
"{$count ->
935-
[one] this pattern always matches, so the else clause is unreachable
936-
*[other] these patterns always match, so the else clause is unreachable
937-
}"
938-
)]
927+
#[diag("unreachable `else` clause")]
928+
#[note("this pattern always matches, so the else clause is unreachable")]
939929
pub(crate) struct IrrefutableLetPatternsLetElse {
940-
pub(crate) count: usize,
941-
#[help("remove this `else` block")]
942-
pub(crate) else_span: Option<Span>,
930+
#[subdiagnostic]
931+
pub(crate) be_replaced: Option<BeReplacedLetElse>,
932+
}
933+
934+
#[derive(Subdiagnostic, Debug)]
935+
#[suggestion(
936+
"consider using `let {$lhs} = {$rhs}` to match on a specific variant",
937+
code = "let {lhs} = {rhs}",
938+
applicability = "machine-applicable"
939+
)]
940+
pub(crate) struct BeReplacedLetElse {
941+
#[primary_span]
942+
pub(crate) span: Span,
943+
pub(crate) lhs: String,
944+
pub(crate) rhs: String,
943945
}
944946

945947
#[derive(Diagnostic)]

‎compiler/rustc_mir_build/src/thir/pattern/check_match.rs‎

Lines changed: 60 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ impl<'p, 'tcx> Visitor<'p, 'tcx> for MatchVisitor<'p, 'tcx> {
168168
let Ok(()) = self.visit_land(ex, &mut chain_refutabilities) else { return };
169169
// Lint only single irrefutable let binding.
170170
if let [Some((_, Irrefutable))] = chain_refutabilities[..] {
171-
self.lint_single_let(ex.span, None);
171+
self.lint_single_let(ex.span, None, None);
172172
}
173173
return;
174174
}
@@ -438,7 +438,41 @@ impl<'p, 'tcx> MatchVisitor<'p, 'tcx> {
438438
if let LetSource::PlainLet = self.let_source {
439439
self.check_binding_is_irrefutable(pat, "local binding", scrut, Some(span));
440440
} else if let Ok(Irrefutable) = self.is_let_irrefutable(pat, scrut) {
441-
self.lint_single_let(span, else_span);
441+
let let_else_span = self.check_irrefutable_option_some(pat, scrut, span);
442+
443+
let sm = self.tcx.sess.source_map();
444+
let next_token_start = sm.span_extend_while_whitespace(span.clone()).hi();
445+
let line_span = sm.span_extend_to_line(span.clone()).with_lo(next_token_start);
446+
let else_keyword_span = sm.span_until_whitespace(line_span);
447+
self.lint_single_let(pat.span, Some(else_keyword_span), let_else_span);
448+
}
449+
}
450+
451+
/// Check case `let x = Some(y);`, user likely intended to destructure `Option`
452+
fn check_irrefutable_option_some(
453+
&self,
454+
pat: &'p Pat<'tcx>,
455+
initializer: Option<&Expr<'tcx>>,
456+
span: Span,
457+
) -> Option<BeReplacedLetElse> {
458+
if let sm = self.tcx.sess.source_map()
459+
&& let Some(initializer) = initializer
460+
&& let Some(s_ty) = initializer.ty.ty_adt_def()
461+
&& self.tcx.is_diagnostic_item(rustc_span::sym::Option, s_ty.did())
462+
&& let ExprKind::Scope { value, .. } = initializer.kind
463+
&& let initializer_expr = &self.thir[value]
464+
&& let ExprKind::Adt(box AdtExpr { fields, .. }) = &initializer_expr.kind
465+
&& let Some(field) = fields.first()
466+
&& let inner = &self.thir[field.expr]
467+
&& let Some(inner_ty) = inner.ty.ty_adt_def()
468+
&& self.tcx.is_diagnostic_item(rustc_span::sym::Option, inner_ty.did())
469+
&& let Ok(rhs) = sm.span_to_snippet(inner.span)
470+
&& let Ok(lhs) = sm.span_to_snippet(pat.span)
471+
{
472+
let lhs = format!("Some({})", lhs);
473+
Some(BeReplacedLetElse { span, lhs, rhs })
474+
} else {
475+
None
442476
}
443477
}
444478

@@ -546,14 +580,20 @@ impl<'p, 'tcx> MatchVisitor<'p, 'tcx> {
546580
}
547581

548582
#[instrument(level = "trace", skip(self))]
549-
fn lint_single_let(&mut self, let_span: Span, else_span: Option<Span>) {
583+
fn lint_single_let(
584+
&mut self,
585+
let_span: Span,
586+
else_keyword_span: Option<Span>,
587+
let_else_span: Option<BeReplacedLetElse>,
588+
) {
550589
report_irrefutable_let_patterns(
551590
self.tcx,
552591
self.hir_source,
553592
self.let_source,
554593
1,
555594
let_span,
556-
else_span,
595+
else_keyword_span,
596+
let_else_span,
557597
);
558598
}
559599

@@ -849,7 +889,8 @@ fn report_irrefutable_let_patterns(
849889
source: LetSource,
850890
count: usize,
851891
span: Span,
852-
else_span: Option<Span>,
892+
else_keyword_span: Option<Span>,
893+
let_else_span: Option<BeReplacedLetElse>,
853894
) {
854895
macro_rules! emit_diag {
855896
($lint:tt) => {{
@@ -862,11 +903,23 @@ fn report_irrefutable_let_patterns(
862903
LetSource::IfLet | LetSource::ElseIfLet => emit_diag!(IrrefutableLetPatternsIfLet),
863904
LetSource::IfLetGuard => emit_diag!(IrrefutableLetPatternsIfLetGuard),
864905
LetSource::LetElse => {
906+
let spans = match else_keyword_span {
907+
Some(else_keyword_span) => {
908+
let mut spans = MultiSpan::from_span(else_keyword_span);
909+
spans.push_span_label(
910+
span,
911+
msg!("assigning to binding pattern will always succeed"),
912+
);
913+
spans
914+
}
915+
None => span.into(),
916+
};
917+
865918
tcx.emit_node_span_lint(
866919
IRREFUTABLE_LET_PATTERNS,
867920
id,
868-
span,
869-
IrrefutableLetPatternsLetElse { count, else_span },
921+
spans,
922+
IrrefutableLetPatternsLetElse { be_replaced: let_else_span },
870923
);
871924
}
872925
LetSource::WhileLet => emit_diag!(IrrefutableLetPatternsWhileLet),

‎tests/ui/let-else/let-else-irrefutable-152938.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66

77
pub fn say_hello(name: Option<String>) {
88
let name_str = Some(name) else { return; };
9-
//~^ WARN irrefutable `let...else` pattern
9+
//~^ WARN unreachable `else` clause
1010
drop(name_str);
1111
}
1212

‎tests/ui/let-else/let-else-irrefutable-152938.stderr‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,18 @@
1-
warning: irrefutable `let...else` pattern
2-
--> $DIR/let-else-irrefutable-152938.rs:8:5
1+
warning: unreachable `else` clause
2+
--> $DIR/let-else-irrefutable-152938.rs:8:31
33
|
44
LL | let name_str = Some(name) else { return; };
5-
| ^^^^^^^^^^^^^^^^^^^^^^^^^
5+
| -------- ^^^^
6+
| |
7+
| assigning to binding pattern will always succeed
68
|
79
= note: this pattern always matches, so the else clause is unreachable
8-
help: remove this `else` block
9-
--> $DIR/let-else-irrefutable-152938.rs:8:36
10-
|
11-
LL | let name_str = Some(name) else { return; };
12-
| ^^^^^^^^^^^
1310
= note: `#[warn(irrefutable_let_patterns)]` on by default
11+
help: consider using `let Some(name_str) = name` to match on a specific variant
12+
|
13+
LL - let name_str = Some(name) else { return; };
14+
LL + let Some(name_str) = name else { return; };
15+
|
1416

1517
warning: 1 warning emitted
1618

‎tests/ui/let-else/let-else-irrefutable.rs‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,18 @@
11
//@ check-pass
22

33
fn main() {
4-
let x = 1 else { return }; //~ WARN irrefutable `let...else` pattern
4+
let x = 1 else { return }; //~ WARN unreachable `else` clause
55

66
// Multiline else blocks should not get printed
7-
let x = 1 else { //~ WARN irrefutable `let...else` pattern
7+
let x = 1 else { //~ WARN unreachable `else` clause
8+
eprintln!("problem case encountered");
9+
return
10+
};
11+
12+
let case = Some("a");
13+
let name = Some(case) else {
14+
//~^ WARN unreachable `else` clause
15+
//~| HELP consider using `let Some(name) = case` to match on a specific variant
816
eprintln!("problem case encountered");
917
return
1018
};

‎tests/ui/let-else/let-else-irrefutable.stderr‎

Lines changed: 26 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,33 +1,38 @@
1-
warning: irrefutable `let...else` pattern
2-
--> $DIR/let-else-irrefutable.rs:4:5
1+
warning: unreachable `else` clause
2+
--> $DIR/let-else-irrefutable.rs:4:15
33
|
44
LL | let x = 1 else { return };
5-
| ^^^^^^^^^
5+
| - ^^^^
6+
| |
7+
| assigning to binding pattern will always succeed
68
|
79
= note: this pattern always matches, so the else clause is unreachable
8-
help: remove this `else` block
9-
--> $DIR/let-else-irrefutable.rs:4:20
10-
|
11-
LL | let x = 1 else { return };
12-
| ^^^^^^^^^^
1310
= note: `#[warn(irrefutable_let_patterns)]` on by default
1411

15-
warning: irrefutable `let...else` pattern
16-
--> $DIR/let-else-irrefutable.rs:7:5
12+
warning: unreachable `else` clause
13+
--> $DIR/let-else-irrefutable.rs:7:15
1714
|
1815
LL | let x = 1 else {
19-
| ^^^^^^^^^
16+
| - ^^^^
17+
| |
18+
| assigning to binding pattern will always succeed
19+
|
20+
= note: this pattern always matches, so the else clause is unreachable
21+
22+
warning: unreachable `else` clause
23+
--> $DIR/let-else-irrefutable.rs:13:27
24+
|
25+
LL | let name = Some(case) else {
26+
| ---- ^^^^
27+
| |
28+
| assigning to binding pattern will always succeed
2029
|
2130
= note: this pattern always matches, so the else clause is unreachable
22-
help: remove this `else` block
23-
--> $DIR/let-else-irrefutable.rs:7:20
24-
|
25-
LL | let x = 1 else {
26-
| ____________________^
27-
LL | | eprintln!("problem case encountered");
28-
LL | | return
29-
LL | | };
30-
| |_____^
31+
help: consider using `let Some(name) = case` to match on a specific variant
32+
|
33+
LL - let name = Some(case) else {
34+
LL + let Some(name) = case else {
35+
|
3136

32-
warning: 2 warnings emitted
37+
warning: 3 warnings emitted
3338

‎tests/ui/parser/bad-let-else-statement.rs‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -93,10 +93,10 @@ fn i() {
9393
fn j() {
9494
let mut bar = 0;
9595
let foo = bar = {
96-
//~^ WARN irrefutable `let...else` pattern
9796
1
9897
} else {
99-
//~^ ERROR right curly brace `}` before `else` in a `let...else` statement not allowed
98+
//~^ WARN unreachable `else` clause
99+
//~| ERROR right curly brace `}` before `else` in a `let...else` statement not allowed
100100
return;
101101
};
102102
}
@@ -158,21 +158,21 @@ fn o() -> Result<(), ()> {
158158

159159
fn q() {
160160
let foo = |x: i32| {
161-
//~^ WARN irrefutable `let...else` pattern
162161
x
163162
} else {
164-
//~^ ERROR right curly brace `}` before `else` in a `let...else` statement not allowed
163+
//~^ WARN unreachable `else` clause
164+
//~| ERROR right curly brace `}` before `else` in a `let...else` statement not allowed
165165
return;
166166
};
167167
}
168168

169169
fn r() {
170170
let ok = format_args!("") else { return; };
171-
//~^ WARN irrefutable `let...else` pattern
171+
//~^ WARN unreachable `else` clause
172172

173173
let bad = format_args! {""} else { return; };
174174
//~^ ERROR right curly brace `}` before `else` in a `let...else` statement not allowed
175-
//~| WARN irrefutable `let...else` pattern
175+
//~| WARN unreachable `else` clause
176176
}
177177

178178
fn s() {
@@ -202,10 +202,10 @@ fn t() {
202202
}
203203

204204
let foo = &std::ptr::null as &'static dyn std::ops::Fn() -> *const primitive! {
205-
//~^ WARN irrefutable `let...else` pattern
206205
8
207206
} else {
208-
//~^ ERROR right curly brace `}` before `else` in a `let...else` statement not allowed
207+
//~^ WARN unreachable `else` clause
208+
//~| ERROR right curly brace `}` before `else` in a `let...else` statement not allowed
209209
return;
210210
};
211211
}

0 commit comments

Comments
 (0)