Skip to content

Commit 8591f0f

Browse files
WeiTheShinobiWeiTheShinobi
committed
Tweak irrefutable let else warning output, add new suggestion
1 parent 2bd7a97 commit 8591f0f

10 files changed

Lines changed: 170 additions & 132 deletions

‎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<LetElseReplacementSuggestion>,
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 LetElseReplacementSuggestion {
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: 64 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,45 @@ 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+
if span.from_expansion() {
442+
self.lint_single_let(span, None, None);
443+
return;
444+
}
445+
let let_else_span = self.check_irrefutable_option_some(pat, scrut, span);
446+
447+
let sm = self.tcx.sess.source_map();
448+
let next_token_start = sm.span_extend_while_whitespace(span.clone()).hi();
449+
let line_span = sm.span_extend_to_line(span.clone()).with_lo(next_token_start);
450+
let else_keyword_span = sm.span_until_whitespace(line_span);
451+
self.lint_single_let(span, Some(else_keyword_span), let_else_span);
452+
}
453+
}
454+
455+
/// Check case `let x = Some(y);`, user likely intended to destructure `Option`
456+
fn check_irrefutable_option_some(
457+
&self,
458+
pat: &'p Pat<'tcx>,
459+
initializer: Option<&Expr<'tcx>>,
460+
span: Span,
461+
) -> Option<LetElseReplacementSuggestion> {
462+
if let sm = self.tcx.sess.source_map()
463+
&& let Some(initializer) = initializer
464+
&& let Some(s_ty) = initializer.ty.ty_adt_def()
465+
&& self.tcx.is_diagnostic_item(rustc_span::sym::Option, s_ty.did())
466+
&& let ExprKind::Scope { value, .. } = initializer.kind
467+
&& let initializer_expr = &self.thir[value]
468+
&& let ExprKind::Adt(box AdtExpr { fields, .. }) = &initializer_expr.kind
469+
&& let Some(field) = fields.first()
470+
&& let inner = &self.thir[field.expr]
471+
&& let Some(inner_ty) = inner.ty.ty_adt_def()
472+
&& self.tcx.is_diagnostic_item(rustc_span::sym::Option, inner_ty.did())
473+
&& let Ok(rhs) = sm.span_to_snippet(inner.span)
474+
&& let Ok(lhs) = sm.span_to_snippet(pat.span)
475+
{
476+
let lhs = format!("Some({})", lhs);
477+
Some(LetElseReplacementSuggestion { span, lhs, rhs })
478+
} else {
479+
None
442480
}
443481
}
444482

@@ -546,14 +584,20 @@ impl<'p, 'tcx> MatchVisitor<'p, 'tcx> {
546584
}
547585

548586
#[instrument(level = "trace", skip(self))]
549-
fn lint_single_let(&mut self, let_span: Span, else_span: Option<Span>) {
587+
fn lint_single_let(
588+
&mut self,
589+
let_span: Span,
590+
else_keyword_span: Option<Span>,
591+
let_else_span: Option<LetElseReplacementSuggestion>,
592+
) {
550593
report_irrefutable_let_patterns(
551594
self.tcx,
552595
self.hir_source,
553596
self.let_source,
554597
1,
555598
let_span,
556-
else_span,
599+
else_keyword_span,
600+
let_else_span,
557601
);
558602
}
559603

@@ -849,7 +893,8 @@ fn report_irrefutable_let_patterns(
849893
source: LetSource,
850894
count: usize,
851895
span: Span,
852-
else_span: Option<Span>,
896+
else_keyword_span: Option<Span>,
897+
let_else_span: Option<LetElseReplacementSuggestion>,
853898
) {
854899
macro_rules! emit_diag {
855900
($lint:tt) => {{
@@ -862,11 +907,23 @@ fn report_irrefutable_let_patterns(
862907
LetSource::IfLet | LetSource::ElseIfLet => emit_diag!(IrrefutableLetPatternsIfLet),
863908
LetSource::IfLetGuard => emit_diag!(IrrefutableLetPatternsIfLetGuard),
864909
LetSource::LetElse => {
910+
let spans = match else_keyword_span {
911+
Some(else_keyword_span) => {
912+
let mut spans = MultiSpan::from_span(else_keyword_span);
913+
spans.push_span_label(
914+
span,
915+
msg!("assigning to binding pattern will always succeed"),
916+
);
917+
spans
918+
}
919+
None => span.into(),
920+
};
921+
865922
tcx.emit_node_span_lint(
866923
IRREFUTABLE_LET_PATTERNS,
867924
id,
868-
span,
869-
IrrefutableLetPatternsLetElse { count, else_span },
925+
spans,
926+
IrrefutableLetPatternsLetElse { be_replaced: let_else_span },
870927
);
871928
}
872929
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)