Skip to content

Commit 254a0c7

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

19 files changed

Lines changed: 218 additions & 183 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<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: 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(pat.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<BeReplacedLetElse> {
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(BeReplacedLetElse { 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<BeReplacedLetElse>,
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<BeReplacedLetElse>,
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/binding/irrefutable-in-let-chains.stderr‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,54 +1,54 @@
11
warning: irrefutable `if let` pattern
2-
--> $DIR/irrefutable-in-let-chains.rs:13:8
2+
--> $DIR/irrefutable-in-let-chains.rs:13:12
33
|
44
LL | if let first = &opt {}
5-
| ^^^^^^^^^^^^^^^^
5+
| ^^^^^
66
|
77
= note: this pattern will always match, so the `if let` is useless
88
= help: consider replacing the `if let` with a `let`
99
= note: `#[warn(irrefutable_let_patterns)]` on by default
1010

1111
warning: irrefutable `if let` pattern
12-
--> $DIR/irrefutable-in-let-chains.rs:30:8
12+
--> $DIR/irrefutable-in-let-chains.rs:30:12
1313
|
1414
LL | if let Range { start: local_start, end: _ } = (None..Some(1)) {}
15-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
15+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
1616
|
1717
= note: this pattern will always match, so the `if let` is useless
1818
= help: consider replacing the `if let` with a `let`
1919

2020
warning: irrefutable `if let` pattern
21-
--> $DIR/irrefutable-in-let-chains.rs:37:8
21+
--> $DIR/irrefutable-in-let-chains.rs:37:12
2222
|
2323
LL | if let (a, b, c) = (Some(1), Some(1), Some(1)) {}
24-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
24+
| ^^^^^^^^^
2525
|
2626
= note: this pattern will always match, so the `if let` is useless
2727
= help: consider replacing the `if let` with a `let`
2828

2929
warning: irrefutable `if let` pattern
30-
--> $DIR/irrefutable-in-let-chains.rs:49:15
30+
--> $DIR/irrefutable-in-let-chains.rs:49:19
3131
|
3232
LL | } else if let x = opt.clone().map(|_| 1) {
33-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
33+
| ^
3434
|
3535
= note: this pattern will always match, so the `if let` is useless
3636
= help: consider replacing the `if let` with a `let`
3737

3838
warning: irrefutable `if let` guard pattern
39-
--> $DIR/irrefutable-in-let-chains.rs:70:28
39+
--> $DIR/irrefutable-in-let-chains.rs:70:32
4040
|
4141
LL | Some(ref first) if let second = first => {}
42-
| ^^^^^^^^^^^^^^^^^^
42+
| ^^^^^^
4343
|
4444
= note: this pattern will always match, so the guard is useless
4545
= help: consider removing the guard and adding a `let` inside the match arm
4646

4747
warning: irrefutable `while let` pattern
48-
--> $DIR/irrefutable-in-let-chains.rs:99:11
48+
--> $DIR/irrefutable-in-let-chains.rs:99:15
4949
|
5050
LL | while let first = &opt {}
51-
| ^^^^^^^^^^^^^^^^
51+
| ^^^^^
5252
|
5353
= note: this pattern will always match, so the loop will never exit
5454
= help: consider instead using a `loop { ... }` with a `let` inside it

‎tests/ui/closures/2229_closure_analysis/issue-88118-2.stderr‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
warning: irrefutable `if let` guard pattern
2-
--> $DIR/issue-88118-2.rs:9:25
2+
--> $DIR/issue-88118-2.rs:9:29
33
|
44
LL | Registry if let _ = registry.try_find_description() => { }
5-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
5+
| ^
66
|
77
= note: this pattern will always match, so the guard is useless
88
= help: consider removing the guard and adding a `let` inside the match arm

‎tests/ui/closures/2229_closure_analysis/migrations/issue-78720.stderr‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
warning: irrefutable `if let` pattern
2-
--> $DIR/issue-78720.rs:7:8
2+
--> $DIR/issue-78720.rs:7:12
33
|
44
LL | if let a = "" {
5-
| ^^^^^^^^^^
5+
| ^
66
|
77
= note: this pattern will always match, so the `if let` is useless
88
= help: consider replacing the `if let` with a `let`

‎tests/ui/expr/if/if-let.stderr‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -30,37 +30,37 @@ LL | | });
3030
= note: this warning originates in the macro `foo` which comes from the expansion of the macro `bar` (in Nightly builds, run with -Z macro-backtrace for more info)
3131

3232
warning: irrefutable `if let` pattern
33-
--> $DIR/if-let.rs:26:8
33+
--> $DIR/if-let.rs:26:12
3434
|
3535
LL | if let a = 1 {
36-
| ^^^^^^^^^
36+
| ^
3737
|
3838
= note: this pattern will always match, so the `if let` is useless
3939
= help: consider replacing the `if let` with a `let`
4040

4141
warning: irrefutable `if let` pattern
42-
--> $DIR/if-let.rs:30:8
42+
--> $DIR/if-let.rs:30:12
4343
|
4444
LL | if let a = 1 {
45-
| ^^^^^^^^^
45+
| ^
4646
|
4747
= note: this pattern will always match, so the `if let` is useless
4848
= help: consider replacing the `if let` with a `let`
4949

5050
warning: irrefutable `if let` pattern
51-
--> $DIR/if-let.rs:40:15
51+
--> $DIR/if-let.rs:40:19
5252
|
5353
LL | } else if let a = 1 {
54-
| ^^^^^^^^^
54+
| ^
5555
|
5656
= note: this pattern will always match, so the `if let` is useless
5757
= help: consider replacing the `if let` with a `let`
5858

5959
warning: irrefutable `if let` pattern
60-
--> $DIR/if-let.rs:46:15
60+
--> $DIR/if-let.rs:46:19
6161
|
6262
LL | } else if let a = 1 {
63-
| ^^^^^^^^^
63+
| ^
6464
|
6565
= note: this pattern will always match, so the `if let` is useless
6666
= help: consider replacing the `if let` with a `let`

‎tests/ui/for-loop-while/while-let-2.stderr‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,10 @@ LL | | });
3030
= note: this warning originates in the macro `foo` which comes from the expansion of the macro `bar` (in Nightly builds, run with -Z macro-backtrace for more info)
3131

3232
warning: irrefutable `while let` pattern
33-
--> $DIR/while-let-2.rs:27:11
33+
--> $DIR/while-let-2.rs:27:15
3434
|
3535
LL | while let _a = 1 {
36-
| ^^^^^^^^^^
36+
| ^^
3737
|
3838
= note: this pattern will always match, so the loop will never exit
3939
= help: consider instead using a `loop { ... }` with a `let` inside it

‎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
};

0 commit comments

Comments
 (0)