Skip to content

Commit cc85d1f

Browse files
estebankestebank
committed
On irrefutable let pattern lint point only at pattern
Add `Span` to let desugaring `MatchSource` variants to point only at the `let` pattern when linting against irrefutable patterns in `if let` and `while let`.
1 parent 0ab7c1d commit cc85d1f

File tree

19 files changed

+78
-82
lines changed

19 files changed

+78
-82
lines changed

‎compiler/rustc_ast_lowering/src/expr.rs

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -88,9 +88,14 @@ impl<'hir> LoweringContext<'_, 'hir> {
8888
self.lower_expr_let(e.span, pat, scrutinee)
8989
}
9090
ExprKind::If(ref cond, ref then, ref else_opt) => match cond.kind {
91-
ExprKind::Let(ref pat, ref scrutinee) => {
92-
self.lower_expr_if_let(e.span, pat, scrutinee, then, else_opt.as_deref())
93-
}
91+
ExprKind::Let(ref pat, ref scrutinee) => self.lower_expr_if_let(
92+
e.span,
93+
pat,
94+
scrutinee,
95+
then,
96+
else_opt.as_deref(),
97+
cond.span,
98+
),
9499
_ => self.lower_expr_if(cond, then, else_opt.as_deref()),
95100
},
96101
ExprKind::While(ref cond, ref body, opt_label) => self
@@ -366,6 +371,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
366371
scrutinee: &Expr,
367372
then: &Block,
368373
else_opt: Option<&Expr>,
374+
let_span: Span,
369375
) -> hir::ExprKind<'hir> {
370376
// FIXME(#53667): handle lowering of && and parens.
371377

@@ -384,7 +390,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
384390
let then_expr = self.lower_block_expr(then);
385391
let then_arm = self.arm(then_pat, self.arena.alloc(then_expr));
386392

387-
let desugar = hir::MatchSource::IfLetDesugar { contains_else_clause };
393+
let desugar = hir::MatchSource::IfLetDesugar { contains_else_clause, let_span };
388394
hir::ExprKind::Match(scrutinee, arena_vec![self; then_arm, else_arm], desugar)
389395
}
390396

@@ -420,7 +426,12 @@ impl<'hir> LoweringContext<'_, 'hir> {
420426
// }
421427
let scrutinee = self.with_loop_condition_scope(|t| t.lower_expr(scrutinee));
422428
let pat = self.lower_pat(pat);
423-
(pat, scrutinee, hir::MatchSource::WhileLetDesugar, hir::LoopSource::WhileLet)
429+
(
430+
pat,
431+
scrutinee,
432+
hir::MatchSource::WhileLetDesugar { let_span: cond.span },
433+
hir::LoopSource::WhileLet,
434+
)
424435
}
425436
_ => {
426437
// We desugar: `'label: while $cond $body` into:

‎compiler/rustc_hir/src/hir.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1879,14 +1879,14 @@ pub enum MatchSource {
18791879
/// A `match _ { .. }`.
18801880
Normal,
18811881
/// An `if let _ = _ { .. }` (optionally with `else { .. }`).
1882-
IfLetDesugar { contains_else_clause: bool },
1882+
IfLetDesugar { contains_else_clause: bool, let_span: Span },
18831883
/// An `if let _ = _ => { .. }` match guard.
18841884
IfLetGuardDesugar,
18851885
/// A `while _ { .. }` (which was desugared to a `loop { match _ { .. } }`).
18861886
WhileDesugar,
18871887
/// A `while let _ = _ { .. }` (which was desugared to a
18881888
/// `loop { match _ { .. } }`).
1889-
WhileLetDesugar,
1889+
WhileLetDesugar { let_span: Span },
18901890
/// A desugared `for _ in _ { .. }` loop.
18911891
ForLoopDesugar,
18921892
/// A desugared `?` operator.
@@ -1901,7 +1901,7 @@ impl MatchSource {
19011901
match self {
19021902
Normal => "match",
19031903
IfLetDesugar { .. } | IfLetGuardDesugar => "if",
1904-
WhileDesugar | WhileLetDesugar => "while",
1904+
WhileDesugar | WhileLetDesugar { .. } => "while",
19051905
ForLoopDesugar => "for",
19061906
TryDesugar => "?",
19071907
AwaitDesugar => ".await",

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

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -373,7 +373,7 @@ fn irrefutable_let_pattern(tcx: TyCtxt<'_>, span: Span, id: HirId, source: hir::
373373
diag.help("consider replacing the `if let` with a `let`");
374374
diag.emit()
375375
}
376-
hir::MatchSource::WhileLetDesugar => {
376+
hir::MatchSource::WhileLetDesugar { .. } => {
377377
let mut diag = lint.build("irrefutable `while let` pattern");
378378
diag.note("this pattern will always match, so the loop will never exit");
379379
diag.help("consider instead using a `loop { ... }` with a `let` inside it");
@@ -423,13 +423,14 @@ fn report_arm_reachability<'p, 'tcx>(
423423
match source {
424424
hir::MatchSource::WhileDesugar => bug!(),
425425

426-
hir::MatchSource::IfLetDesugar { .. } | hir::MatchSource::WhileLetDesugar => {
426+
hir::MatchSource::IfLetDesugar { let_span, .. }
427+
| hir::MatchSource::WhileLetDesugar { let_span } => {
427428
// Check which arm we're on.
428429
match arm_index {
429430
// The arm with the user-specified pattern.
430-
0 => unreachable_pattern(cx.tcx, arm.pat.span, arm.hir_id, None),
431+
0 => unreachable_pattern(cx.tcx, let_span, arm.hir_id, None),
431432
// The arm with the wildcard pattern.
432-
1 => irrefutable_let_pattern(cx.tcx, arm.pat.span, arm.hir_id, source),
433+
1 => irrefutable_let_pattern(cx.tcx, let_span, arm.hir_id, source),
433434
_ => bug!(),
434435
}
435436
}

‎compiler/rustc_passes/src/check_const.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,9 @@ impl NonConstExpr {
4949

5050
// All other expressions are allowed.
5151
Self::Loop(Loop | While | WhileLet)
52-
| Self::Match(WhileDesugar | WhileLetDesugar | Normal | IfLetDesugar { .. }) => &[],
52+
| Self::Match(WhileDesugar | WhileLetDesugar { .. } | Normal | IfLetDesugar { .. }) => {
53+
&[]
54+
}
5355
};
5456

5557
Some(gates)
@@ -207,7 +209,7 @@ impl<'tcx> Visitor<'tcx> for CheckConstVisitor<'tcx> {
207209
let non_const_expr = match source {
208210
// These are handled by `ExprKind::Loop` above.
209211
hir::MatchSource::WhileDesugar
210-
| hir::MatchSource::WhileLetDesugar
212+
| hir::MatchSource::WhileLetDesugar { .. }
211213
| hir::MatchSource::ForLoopDesugar => None,
212214

213215
_ => Some(NonConstExpr::Match(*source)),

‎src/test/ui/closures/2229_closure_analysis/diagnostics/closure-origin-single-variant-diagnostics.stderr

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,10 @@ LL | #![feature(capture_disjoint_fields)]
88
= note: see issue #53488 <https://github.com/rust-lang/rust/issues/53488> for more information
99

1010
warning: irrefutable `if let` pattern
11-
--> $DIR/closure-origin-single-variant-diagnostics.rs:18:9
11+
--> $DIR/closure-origin-single-variant-diagnostics.rs:18:12
1212
|
13-
LL | / if let SingleVariant::Point(ref mut x, _) = point {
14-
LL | |
15-
LL | | *x += 1;
16-
LL | | }
17-
| |_________^
13+
LL | if let SingleVariant::Point(ref mut x, _) = point {
14+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
1815
|
1916
= note: `#[warn(irrefutable_let_patterns)]` on by default
2017
= note: this pattern will always match, so the `if let` is useless

‎src/test/ui/expr/if/if-let.stderr

Lines changed: 16 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
warning: irrefutable `if let` pattern
2-
--> $DIR/if-let.rs:6:13
2+
--> $DIR/if-let.rs:6:16
33
|
44
LL | if let $p = $e $b
5-
| ^^^^^^^^^^^^^^^^^
5+
| ^^^
66
...
77
LL | / foo!(a, 1, {
88
LL | | println!("irrefutable pattern");
@@ -15,10 +15,10 @@ LL | | });
1515
= note: this warning originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)
1616

1717
warning: irrefutable `if let` pattern
18-
--> $DIR/if-let.rs:6:13
18+
--> $DIR/if-let.rs:6:16
1919
|
2020
LL | if let $p = $e $b
21-
| ^^^^^^^^^^^^^^^^^
21+
| ^^^
2222
...
2323
LL | / bar!(a, 1, {
2424
LL | | println!("irrefutable pattern");
@@ -30,51 +30,37 @@ LL | | });
3030
= note: this warning originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)
3131

3232
warning: irrefutable `if let` pattern
33-
--> $DIR/if-let.rs:26:5
33+
--> $DIR/if-let.rs:26:8
3434
|
35-
LL | / if let a = 1 {
36-
LL | | println!("irrefutable pattern");
37-
LL | | }
38-
| |_____^
35+
LL | if let a = 1 {
36+
| ^^^^^^^^^
3937
|
4038
= note: this pattern will always match, so the `if let` is useless
4139
= help: consider replacing the `if let` with a `let`
4240

4341
warning: irrefutable `if let` pattern
44-
--> $DIR/if-let.rs:30:5
42+
--> $DIR/if-let.rs:30:8
4543
|
46-
LL | / if let a = 1 {
47-
LL | | println!("irrefutable pattern");
48-
LL | | } else if true {
49-
LL | | println!("else-if in irrefutable `if let`");
50-
LL | | } else {
51-
LL | | println!("else in irrefutable `if let`");
52-
LL | | }
53-
| |_____^
44+
LL | if let a = 1 {
45+
| ^^^^^^^^^
5446
|
5547
= note: this pattern will always match, so the `if let` is useless
5648
= help: consider replacing the `if let` with a `let`
5749

5850
warning: irrefutable `if let` pattern
59-
--> $DIR/if-let.rs:40:12
51+
--> $DIR/if-let.rs:40:15
6052
|
61-
LL | } else if let a = 1 {
62-
| ____________^
63-
LL | | println!("irrefutable pattern");
64-
LL | | }
65-
| |_____^
53+
LL | } else if let a = 1 {
54+
| ^^^^^^^^^
6655
|
6756
= note: this pattern will always match, so the `if let` is useless
6857
= help: consider replacing the `if let` with a `let`
6958

7059
warning: irrefutable `if let` pattern
71-
--> $DIR/if-let.rs:46:12
60+
--> $DIR/if-let.rs:46:15
7261
|
73-
LL | } else if let a = 1 {
74-
| ____________^
75-
LL | | println!("irrefutable pattern");
76-
LL | | }
77-
| |_____^
62+
LL | } else if let a = 1 {
63+
| ^^^^^^^^^
7864
|
7965
= note: this pattern will always match, so the `if let` is useless
8066
= help: consider replacing the `if let` with a `let`

‎src/test/ui/pattern/usefulness/deny-irrefutable-let-patterns.stderr

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
error: irrefutable `if let` pattern
2-
--> $DIR/deny-irrefutable-let-patterns.rs:7:5
2+
--> $DIR/deny-irrefutable-let-patterns.rs:7:8
33
|
44
LL | if let _ = 5 {}
5-
| ^^^^^^^^^^^^^^^
5+
| ^^^^^^^^^
66
|
77
note: the lint level is defined here
88
--> $DIR/deny-irrefutable-let-patterns.rs:4:9
@@ -13,12 +13,10 @@ LL | #![deny(irrefutable_let_patterns)]
1313
= help: consider replacing the `if let` with a `let`
1414

1515
error: irrefutable `while let` pattern
16-
--> $DIR/deny-irrefutable-let-patterns.rs:9:5
16+
--> $DIR/deny-irrefutable-let-patterns.rs:9:11
1717
|
18-
LL | / while let _ = 5 {
19-
LL | | break;
20-
LL | | }
21-
| |_____^
18+
LL | while let _ = 5 {
19+
| ^^^^^^^^^
2220
|
2321
= note: this pattern will always match, so the loop will never exit
2422
= help: consider instead using a `loop { ... }` with a `let` inside it

‎src/test/ui/rfc-2008-non-exhaustive/uninhabited/patterns_same_crate.stderr

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,22 +17,22 @@ LL | Some(_x) => (),
1717
| ^^^^^^^^
1818

1919
error: unreachable pattern
20-
--> $DIR/patterns_same_crate.rs:61:15
20+
--> $DIR/patterns_same_crate.rs:61:11
2121
|
2222
LL | while let PartiallyInhabitedVariants::Struct { x } = partially_inhabited_variant() {
23-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
23+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
2424

2525
error: unreachable pattern
26-
--> $DIR/patterns_same_crate.rs:65:15
26+
--> $DIR/patterns_same_crate.rs:65:11
2727
|
2828
LL | while let Some(_x) = uninhabited_struct() {
29-
| ^^^^^^^^
29+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
3030

3131
error: unreachable pattern
32-
--> $DIR/patterns_same_crate.rs:68:15
32+
--> $DIR/patterns_same_crate.rs:68:11
3333
|
3434
LL | while let Some(_x) = uninhabited_tuple_struct() {
35-
| ^^^^^^^^
35+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
3636

3737
error: aborting due to 5 previous errors
3838

‎src/test/ui/uninhabited/uninhabited-patterns.stderr

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,10 +29,10 @@ LL | Err(Ok(_y)) => (),
2929
| ^^^^^^^^^^^
3030

3131
error: unreachable pattern
32-
--> $DIR/uninhabited-patterns.rs:44:15
32+
--> $DIR/uninhabited-patterns.rs:44:11
3333
|
3434
LL | while let Some(_y) = foo() {
35-
| ^^^^^^^^
35+
| ^^^^^^^^^^^^^^^^^^^^
3636

3737
error: aborting due to 5 previous errors
3838

‎src/test/ui/while-let.stderr

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
warning: irrefutable `while let` pattern
2-
--> $DIR/while-let.rs:7:13
2+
--> $DIR/while-let.rs:7:19
33
|
44
LL | while let $p = $e $b
5-
| ^^^^^^^^^^^^^^^^^^^^
5+
| ^^^
66
...
77
LL | / foo!(_a, 1, {
88
LL | | println!("irrefutable pattern");
@@ -15,10 +15,10 @@ LL | | });
1515
= note: this warning originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)
1616

1717
warning: irrefutable `while let` pattern
18-
--> $DIR/while-let.rs:7:13
18+
--> $DIR/while-let.rs:7:19
1919
|
2020
LL | while let $p = $e $b
21-
| ^^^^^^^^^^^^^^^^^^^^
21+
| ^^^
2222
...
2323
LL | / bar!(_a, 1, {
2424
LL | | println!("irrefutable pattern");
@@ -30,13 +30,10 @@ LL | | });
3030
= note: this warning originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)
3131

3232
warning: irrefutable `while let` pattern
33-
--> $DIR/while-let.rs:27:5
33+
--> $DIR/while-let.rs:27:11
3434
|
35-
LL | / while let _a = 1 {
36-
LL | | println!("irrefutable pattern");
37-
LL | | break;
38-
LL | | }
39-
| |_____^
35+
LL | while let _a = 1 {
36+
| ^^^^^^^^^^
4037
|
4138
= note: this pattern will always match, so the loop will never exit
4239
= help: consider instead using a `loop { ... }` with a `let` inside it

‎src/tools/clippy/clippy_lints/src/if_let_mutex.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ impl<'tcx> LateLintPass<'tcx> for IfLetMutex {
5757
ref arms,
5858
MatchSource::IfLetDesugar {
5959
contains_else_clause: true,
60+
..
6061
},
6162
) = ex.kind
6263
{

‎src/tools/clippy/clippy_lints/src/implicit_return.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,7 @@ fn expr_match(cx: &LateContext<'_>, expr: &Expr<'_>) {
9292
let check_all_arms = match source {
9393
MatchSource::IfLetDesugar {
9494
contains_else_clause: has_else,
95+
..
9596
} => has_else,
9697
_ => true,
9798
};

‎src/tools/clippy/clippy_lints/src/loops.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -629,7 +629,7 @@ impl<'tcx> LateLintPass<'tcx> for Loops {
629629
}
630630
}
631631
}
632-
if let ExprKind::Match(ref match_expr, ref arms, MatchSource::WhileLetDesugar) = expr.kind {
632+
if let ExprKind::Match(ref match_expr, ref arms, MatchSource::WhileLetDesugar { .. }) = expr.kind {
633633
let pat = &arms[0].pat.kind;
634634
if let (
635635
&PatKind::TupleStruct(ref qpath, ref pat_args, _),

‎src/tools/clippy/clippy_lints/src/matches.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1627,7 +1627,7 @@ mod redundant_pattern_match {
16271627
match match_source {
16281628
MatchSource::Normal => find_sugg_for_match(cx, expr, op, arms),
16291629
MatchSource::IfLetDesugar { .. } => find_sugg_for_if_let(cx, expr, op, arms, "if"),
1630-
MatchSource::WhileLetDesugar => find_sugg_for_if_let(cx, expr, op, arms, "while"),
1630+
MatchSource::WhileLetDesugar { .. } => find_sugg_for_if_let(cx, expr, op, arms, "while"),
16311631
_ => {},
16321632
}
16331633
}

‎src/tools/clippy/clippy_lints/src/option_if_let_else.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,7 @@ fn should_wrap_in_braces(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
118118
arms,
119119
MatchSource::IfLetDesugar {
120120
contains_else_clause: true,
121+
..
121122
},
122123
),
123124
..
@@ -159,7 +160,7 @@ fn detect_option_if_let_else<'tcx>(
159160
) -> Option<OptionIfLetElseOccurence> {
160161
if_chain! {
161162
if !utils::in_macro(expr.span); // Don't lint macros, because it behaves weirdly
162-
if let ExprKind::Match(cond_expr, arms, MatchSource::IfLetDesugar{contains_else_clause: true}) = &expr.kind;
163+
if let ExprKind::Match(cond_expr, arms, MatchSource::IfLetDesugar{ contains_else_clause: true, .. }) = &expr.kind;
163164
if arms.len() == 2;
164165
if !is_result_ok(cx, cond_expr); // Don't lint on Result::ok because a different lint does it already
165166
if let PatKind::TupleStruct(struct_qpath, &[inner_pat], _) = &arms[0].pat.kind;

‎src/tools/clippy/clippy_lints/src/pattern_type_mismatch.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ impl<'tcx> LateLintPass<'tcx> for PatternTypeMismatch {
105105
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
106106
if let ExprKind::Match(ref expr, arms, source) = expr.kind {
107107
match source {
108-
MatchSource::Normal | MatchSource::IfLetDesugar { .. } | MatchSource::WhileLetDesugar => {
108+
MatchSource::Normal | MatchSource::IfLetDesugar { .. } | MatchSource::WhileLetDesugar { .. }=> {
109109
if let Some(expr_ty) = cx.typeck_results().node_type_opt(expr.hir_id) {
110110
'pattern_checks: for arm in arms {
111111
let pat = &arm.pat;

0 commit comments

Comments
 (0)