Skip to content

Commit a8d4eee

Browse files
ozankenangungorozankenangungor
committed
Improve irrefutable let-else lint wording
1 parent ec818fd commit a8d4eee

7 files changed

Lines changed: 128 additions & 28 deletions

File tree

‎compiler/rustc_mir_build/src/errors.rs‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -932,13 +932,14 @@ pub(crate) struct IrrefutableLetPatternsIfLetGuard {
932932
)]
933933
#[note(
934934
"{$count ->
935-
[one] this pattern
936-
*[other] these patterns
937-
} will always match, so the `else` clause is useless"
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+
}"
938938
)]
939-
#[help("consider removing the `else` clause")]
940939
pub(crate) struct IrrefutableLetPatternsLetElse {
941940
pub(crate) count: usize,
941+
#[help("remove this `else` block")]
942+
pub(crate) else_span: Option<Span>,
942943
}
943944

944945
#[derive(Diagnostic)]

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

Lines changed: 30 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,7 @@ impl<'p, 'tcx> Visitor<'p, 'tcx> for MatchVisitor<'p, 'tcx> {
160160
self.check_match(scrutinee, arms, MatchSource::Normal, span);
161161
}
162162
ExprKind::Let { box ref pat, expr } => {
163-
self.check_let(pat, Some(expr), ex.span);
163+
self.check_let(pat, Some(expr), ex.span, None);
164164
}
165165
ExprKind::LogicalOp { op: LogicalOp::And, .. }
166166
if !matches!(self.let_source, LetSource::None) =>
@@ -169,7 +169,7 @@ impl<'p, 'tcx> Visitor<'p, 'tcx> for MatchVisitor<'p, 'tcx> {
169169
let Ok(()) = self.visit_land(ex, &mut chain_refutabilities) else { return };
170170
// Lint only single irrefutable let binding.
171171
if let [Some((_, Irrefutable))] = chain_refutabilities[..] {
172-
self.lint_single_let(ex.span);
172+
self.lint_single_let(ex.span, None);
173173
}
174174
return;
175175
}
@@ -184,8 +184,9 @@ impl<'p, 'tcx> Visitor<'p, 'tcx> for MatchVisitor<'p, 'tcx> {
184184
self.with_hir_source(hir_id, |this| {
185185
let let_source =
186186
if else_block.is_some() { LetSource::LetElse } else { LetSource::PlainLet };
187+
let else_span = else_block.map(|bid| this.thir.blocks[bid].span);
187188
this.with_let_source(let_source, |this| {
188-
this.check_let(pattern, initializer, span)
189+
this.check_let(pattern, initializer, span, else_span)
189190
});
190191
visit::walk_stmt(this, stmt);
191192
});
@@ -426,13 +427,19 @@ impl<'p, 'tcx> MatchVisitor<'p, 'tcx> {
426427
}
427428

428429
#[instrument(level = "trace", skip(self))]
429-
fn check_let(&mut self, pat: &'p Pat<'tcx>, scrutinee: Option<ExprId>, span: Span) {
430+
fn check_let(
431+
&mut self,
432+
pat: &'p Pat<'tcx>,
433+
scrutinee: Option<ExprId>,
434+
span: Span,
435+
else_span: Option<Span>,
436+
) {
430437
assert!(self.let_source != LetSource::None);
431438
let scrut = scrutinee.map(|id| &self.thir[id]);
432439
if let LetSource::PlainLet = self.let_source {
433440
self.check_binding_is_irrefutable(pat, "local binding", scrut, Some(span));
434441
} else if let Ok(Irrefutable) = self.is_let_irrefutable(pat, scrut) {
435-
self.lint_single_let(span);
442+
self.lint_single_let(span, else_span);
436443
}
437444
}
438445

@@ -540,8 +547,15 @@ impl<'p, 'tcx> MatchVisitor<'p, 'tcx> {
540547
}
541548

542549
#[instrument(level = "trace", skip(self))]
543-
fn lint_single_let(&mut self, let_span: Span) {
544-
report_irrefutable_let_patterns(self.tcx, self.hir_source, self.let_source, 1, let_span);
550+
fn lint_single_let(&mut self, let_span: Span, else_span: Option<Span>) {
551+
report_irrefutable_let_patterns(
552+
self.tcx,
553+
self.hir_source,
554+
self.let_source,
555+
1,
556+
let_span,
557+
else_span,
558+
);
545559
}
546560

547561
fn analyze_binding(
@@ -836,6 +850,7 @@ fn report_irrefutable_let_patterns(
836850
source: LetSource,
837851
count: usize,
838852
span: Span,
853+
else_span: Option<Span>,
839854
) {
840855
macro_rules! emit_diag {
841856
($lint:tt) => {{
@@ -847,7 +862,14 @@ fn report_irrefutable_let_patterns(
847862
LetSource::None | LetSource::PlainLet | LetSource::Else => bug!(),
848863
LetSource::IfLet | LetSource::ElseIfLet => emit_diag!(IrrefutableLetPatternsIfLet),
849864
LetSource::IfLetGuard => emit_diag!(IrrefutableLetPatternsIfLetGuard),
850-
LetSource::LetElse => emit_diag!(IrrefutableLetPatternsLetElse),
865+
LetSource::LetElse => {
866+
tcx.emit_node_span_lint(
867+
IRREFUTABLE_LET_PATTERNS,
868+
id,
869+
span,
870+
IrrefutableLetPatternsLetElse { count, else_span },
871+
);
872+
}
851873
LetSource::WhileLet => emit_diag!(IrrefutableLetPatternsWhileLet),
852874
}
853875
}

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
//@ check-pass
2+
3+
// Regression test for https://github.com/rust-lang/rust/issues/152938
4+
// The irrefutable `let...else` diagnostic should explain that the pattern
5+
// always matches and point at the `else` block for removal.
6+
7+
pub fn say_hello(name: Option<String>) {
8+
let name_str = Some(name) else { return; };
9+
//~^ WARN irrefutable `let...else` pattern
10+
drop(name_str);
11+
}
12+
13+
fn main() {}

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
warning: irrefutable `let...else` pattern
2+
--> $DIR/let-else-irrefutable-152938.rs:8:5
3+
|
4+
LL | let name_str = Some(name) else { return; };
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^
6+
|
7+
= 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+
| ^^^^^^^^^^^
13+
= note: `#[warn(irrefutable_let_patterns)]` on by default
14+
15+
warning: 1 warning emitted
16+

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

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,12 @@ warning: irrefutable `let...else` pattern
44
LL | let x = 1 else { return };
55
| ^^^^^^^^^
66
|
7-
= note: this pattern will always match, so the `else` clause is useless
8-
= help: consider removing the `else` clause
7+
= 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+
| ^^^^^^^^^^
913
= note: `#[warn(irrefutable_let_patterns)]` on by default
1014

1115
warning: irrefutable `let...else` pattern
@@ -14,8 +18,16 @@ warning: irrefutable `let...else` pattern
1418
LL | let x = 1 else {
1519
| ^^^^^^^^^
1620
|
17-
= note: this pattern will always match, so the `else` clause is useless
18-
= help: consider removing the `else` clause
21+
= 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+
| |_____^
1931

2032
warning: 2 warnings emitted
2133

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

Lines changed: 42 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -268,8 +268,16 @@ LL | | 1
268268
LL | | } else {
269269
| |_____^
270270
|
271-
= note: this pattern will always match, so the `else` clause is useless
272-
= help: consider removing the `else` clause
271+
= note: this pattern always matches, so the else clause is unreachable
272+
help: remove this `else` block
273+
--> $DIR/bad-let-else-statement.rs:98:12
274+
|
275+
LL | } else {
276+
| ____________^
277+
LL | |
278+
LL | | return;
279+
LL | | };
280+
| |_____^
273281
= note: `#[warn(irrefutable_let_patterns)]` on by default
274282

275283
warning: irrefutable `let...else` pattern
@@ -281,26 +289,42 @@ LL | | x
281289
LL | | } else {
282290
| |_____^
283291
|
284-
= note: this pattern will always match, so the `else` clause is useless
285-
= help: consider removing the `else` clause
292+
= note: this pattern always matches, so the else clause is unreachable
293+
help: remove this `else` block
294+
--> $DIR/bad-let-else-statement.rs:163:12
295+
|
296+
LL | } else {
297+
| ____________^
298+
LL | |
299+
LL | | return;
300+
LL | | };
301+
| |_____^
286302

287303
warning: irrefutable `let...else` pattern
288304
--> $DIR/bad-let-else-statement.rs:170:5
289305
|
290306
LL | let ok = format_args!("") else { return; };
291307
| ^^^^^^^^^^^^^^^^^^^^^^^^^
292308
|
293-
= note: this pattern will always match, so the `else` clause is useless
294-
= help: consider removing the `else` clause
309+
= note: this pattern always matches, so the else clause is unreachable
310+
help: remove this `else` block
311+
--> $DIR/bad-let-else-statement.rs:170:36
312+
|
313+
LL | let ok = format_args!("") else { return; };
314+
| ^^^^^^^^^^^
295315

296316
warning: irrefutable `let...else` pattern
297317
--> $DIR/bad-let-else-statement.rs:173:5
298318
|
299319
LL | let bad = format_args! {""} else { return; };
300320
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
301321
|
302-
= note: this pattern will always match, so the `else` clause is useless
303-
= help: consider removing the `else` clause
322+
= note: this pattern always matches, so the else clause is unreachable
323+
help: remove this `else` block
324+
--> $DIR/bad-let-else-statement.rs:173:38
325+
|
326+
LL | let bad = format_args! {""} else { return; };
327+
| ^^^^^^^^^^^
304328

305329
warning: irrefutable `let...else` pattern
306330
--> $DIR/bad-let-else-statement.rs:204:5
@@ -311,8 +335,16 @@ LL | | 8
311335
LL | | } else {
312336
| |_____^
313337
|
314-
= note: this pattern will always match, so the `else` clause is useless
315-
= help: consider removing the `else` clause
338+
= note: this pattern always matches, so the else clause is unreachable
339+
help: remove this `else` block
340+
--> $DIR/bad-let-else-statement.rs:207:12
341+
|
342+
LL | } else {
343+
| ____________^
344+
LL | |
345+
LL | | return;
346+
LL | | };
347+
| |_____^
316348

317349
error: aborting due to 19 previous errors; 5 warnings emitted
318350

‎tests/ui/span/let-offset-of.stderr‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,12 @@ warning: irrefutable `let...else` pattern
1414
LL | let x = offset_of!(Foo, field) else { return; };
1515
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
1616
|
17-
= note: this pattern will always match, so the `else` clause is useless
18-
= help: consider removing the `else` clause
17+
= note: this pattern always matches, so the else clause is unreachable
18+
help: remove this `else` block
19+
--> $DIR/let-offset-of.rs:17:41
20+
|
21+
LL | let x = offset_of!(Foo, field) else { return; };
22+
| ^^^^^^^^^^^
1923

2024
warning: 2 warnings emitted
2125

0 commit comments

Comments
 (0)