On irrefutable let pattern lint point only at pattern

estebank · estebank · commit cc85d1ff7fb6 · 2021-02-25T10:16:22.000-08:00
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`.
diff --git a/compiler/rustc_ast_lowering/src/expr.rs b/compiler/rustc_ast_lowering/src/expr.rs
@@ -88,9 +88,14 @@ impl<'hir> LoweringContext<'_, 'hir> {
                     self.lower_expr_let(e.span, pat, scrutinee)
                 }
                 ExprKind::If(ref cond, ref then, ref else_opt) => match cond.kind {
-                    ExprKind::Let(ref pat, ref scrutinee) => {
-                        self.lower_expr_if_let(e.span, pat, scrutinee, then, else_opt.as_deref())
-                    }
+                    ExprKind::Let(ref pat, ref scrutinee) => self.lower_expr_if_let(
+                        e.span,
+                        pat,
+                        scrutinee,
+                        then,
+                        else_opt.as_deref(),
+                        cond.span,
+                    ),
                     _ => self.lower_expr_if(cond, then, else_opt.as_deref()),
                 },
                 ExprKind::While(ref cond, ref body, opt_label) => self
@@ -366,6 +371,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
         scrutinee: &Expr,
         then: &Block,
         else_opt: Option<&Expr>,
+        let_span: Span,
     ) -> hir::ExprKind<'hir> {
         // FIXME(#53667): handle lowering of && and parens.
 
@@ -384,7 +390,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
         let then_expr = self.lower_block_expr(then);
         let then_arm = self.arm(then_pat, self.arena.alloc(then_expr));
 
-        let desugar = hir::MatchSource::IfLetDesugar { contains_else_clause };
+        let desugar = hir::MatchSource::IfLetDesugar { contains_else_clause, let_span };
         hir::ExprKind::Match(scrutinee, arena_vec![self; then_arm, else_arm], desugar)
     }
 
@@ -420,7 +426,12 @@ impl<'hir> LoweringContext<'_, 'hir> {
                 //   }
                 let scrutinee = self.with_loop_condition_scope(|t| t.lower_expr(scrutinee));
                 let pat = self.lower_pat(pat);
-                (pat, scrutinee, hir::MatchSource::WhileLetDesugar, hir::LoopSource::WhileLet)
+                (
+                    pat,
+                    scrutinee,
+                    hir::MatchSource::WhileLetDesugar { let_span: cond.span },
+                    hir::LoopSource::WhileLet,
+                )
             }
             _ => {
                 // We desugar: `'label: while $cond $body` into:
diff --git a/compiler/rustc_hir/src/hir.rs b/compiler/rustc_hir/src/hir.rs
@@ -1879,14 +1879,14 @@ pub enum MatchSource {
     /// A `match _ { .. }`.
     Normal,
     /// An `if let _ = _ { .. }` (optionally with `else { .. }`).
-    IfLetDesugar { contains_else_clause: bool },
+    IfLetDesugar { contains_else_clause: bool, let_span: Span },
     /// An `if let _ = _ => { .. }` match guard.
     IfLetGuardDesugar,
     /// A `while _ { .. }` (which was desugared to a `loop { match _ { .. } }`).
     WhileDesugar,
     /// A `while let _ = _ { .. }` (which was desugared to a
     /// `loop { match _ { .. } }`).
-    WhileLetDesugar,
+    WhileLetDesugar { let_span: Span },
     /// A desugared `for _ in _ { .. }` loop.
     ForLoopDesugar,
     /// A desugared `?` operator.
@@ -1901,7 +1901,7 @@ impl MatchSource {
         match self {
             Normal => "match",
             IfLetDesugar { .. } | IfLetGuardDesugar => "if",
-            WhileDesugar | WhileLetDesugar => "while",
+            WhileDesugar | WhileLetDesugar { .. } => "while",
             ForLoopDesugar => "for",
             TryDesugar => "?",
             AwaitDesugar => ".await",
diff --git a/compiler/rustc_mir_build/src/thir/pattern/check_match.rs b/compiler/rustc_mir_build/src/thir/pattern/check_match.rs
@@ -373,7 +373,7 @@ fn irrefutable_let_pattern(tcx: TyCtxt<'_>, span: Span, id: HirId, source: hir::
             diag.help("consider replacing the `if let` with a `let`");
             diag.emit()
         }
-        hir::MatchSource::WhileLetDesugar => {
+        hir::MatchSource::WhileLetDesugar { .. } => {
             let mut diag = lint.build("irrefutable `while let` pattern");
             diag.note("this pattern will always match, so the loop will never exit");
             diag.help("consider instead using a `loop { ... }` with a `let` inside it");
@@ -423,13 +423,14 @@ fn report_arm_reachability<'p, 'tcx>(
                 match source {
                     hir::MatchSource::WhileDesugar => bug!(),
 
-                    hir::MatchSource::IfLetDesugar { .. } | hir::MatchSource::WhileLetDesugar => {
+                    hir::MatchSource::IfLetDesugar { let_span, .. }
+                    | hir::MatchSource::WhileLetDesugar { let_span } => {
                         // Check which arm we're on.
                         match arm_index {
                             // The arm with the user-specified pattern.
-                            0 => unreachable_pattern(cx.tcx, arm.pat.span, arm.hir_id, None),
+                            0 => unreachable_pattern(cx.tcx, let_span, arm.hir_id, None),
                             // The arm with the wildcard pattern.
-                            1 => irrefutable_let_pattern(cx.tcx, arm.pat.span, arm.hir_id, source),
+                            1 => irrefutable_let_pattern(cx.tcx, let_span, arm.hir_id, source),
                             _ => bug!(),
                         }
                     }
diff --git a/compiler/rustc_passes/src/check_const.rs b/compiler/rustc_passes/src/check_const.rs
@@ -49,7 +49,9 @@ impl NonConstExpr {
 
             // All other expressions are allowed.
             Self::Loop(Loop | While | WhileLet)
-            | Self::Match(WhileDesugar | WhileLetDesugar | Normal | IfLetDesugar { .. }) => &[],
+            | Self::Match(WhileDesugar | WhileLetDesugar { .. } | Normal | IfLetDesugar { .. }) => {
+                &[]
+            }
         };
 
         Some(gates)
@@ -207,7 +209,7 @@ impl<'tcx> Visitor<'tcx> for CheckConstVisitor<'tcx> {
                 let non_const_expr = match source {
                     // These are handled by `ExprKind::Loop` above.
                     hir::MatchSource::WhileDesugar
-                    | hir::MatchSource::WhileLetDesugar
+                    | hir::MatchSource::WhileLetDesugar { .. }
                     | hir::MatchSource::ForLoopDesugar => None,
 
                     _ => Some(NonConstExpr::Match(*source)),
diff --git a/src/test/ui/closures/2229_closure_analysis/diagnostics/closure-origin-single-variant-diagnostics.stderr b/src/test/ui/closures/2229_closure_analysis/diagnostics/closure-origin-single-variant-diagnostics.stderr
@@ -8,13 +8,10 @@ LL | #![feature(capture_disjoint_fields)]
    = note: see issue #53488 <https://github.com/rust-lang/rust/issues/53488> for more information
 
 warning: irrefutable `if let` pattern
-  --> $DIR/closure-origin-single-variant-diagnostics.rs:18:9
+  --> $DIR/closure-origin-single-variant-diagnostics.rs:18:12
    |
-LL | /         if let SingleVariant::Point(ref mut x, _) = point {
-LL | |
-LL | |             *x += 1;
-LL | |         }
-   | |_________^
+LL |         if let SingleVariant::Point(ref mut x, _) = point {
+   |            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: `#[warn(irrefutable_let_patterns)]` on by default
    = note: this pattern will always match, so the `if let` is useless
diff --git a/src/test/ui/expr/if/if-let.stderr b/src/test/ui/expr/if/if-let.stderr
@@ -1,8 +1,8 @@
 warning: irrefutable `if let` pattern
-  --> $DIR/if-let.rs:6:13
+  --> $DIR/if-let.rs:6:16
    |
 LL |               if let $p = $e $b
-   |               ^^^^^^^^^^^^^^^^^
+   |                  ^^^
 ...
 LL | /     foo!(a, 1, {
 LL | |         println!("irrefutable pattern");
@@ -15,10 +15,10 @@ LL | |     });
    = note: this warning originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)
 
 warning: irrefutable `if let` pattern
-  --> $DIR/if-let.rs:6:13
+  --> $DIR/if-let.rs:6:16
    |
 LL |               if let $p = $e $b
-   |               ^^^^^^^^^^^^^^^^^
+   |                  ^^^
 ...
 LL | /     bar!(a, 1, {
 LL | |         println!("irrefutable pattern");
@@ -30,51 +30,37 @@ LL | |     });
    = note: this warning originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)
 
 warning: irrefutable `if let` pattern
-  --> $DIR/if-let.rs:26:5
+  --> $DIR/if-let.rs:26:8
    |
-LL | /     if let a = 1 {
-LL | |         println!("irrefutable pattern");
-LL | |     }
-   | |_____^
+LL |     if let a = 1 {
+   |        ^^^^^^^^^
    |
    = note: this pattern will always match, so the `if let` is useless
    = help: consider replacing the `if let` with a `let`
 
 warning: irrefutable `if let` pattern
-  --> $DIR/if-let.rs:30:5
+  --> $DIR/if-let.rs:30:8
    |
-LL | /     if let a = 1 {
-LL | |         println!("irrefutable pattern");
-LL | |     } else if true {
-LL | |         println!("else-if in irrefutable `if let`");
-LL | |     } else {
-LL | |         println!("else in irrefutable `if let`");
-LL | |     }
-   | |_____^
+LL |     if let a = 1 {
+   |        ^^^^^^^^^
    |
    = note: this pattern will always match, so the `if let` is useless
    = help: consider replacing the `if let` with a `let`
 
 warning: irrefutable `if let` pattern
-  --> $DIR/if-let.rs:40:12
+  --> $DIR/if-let.rs:40:15
    |
-LL |       } else if let a = 1 {
-   |  ____________^
-LL | |         println!("irrefutable pattern");
-LL | |     }
-   | |_____^
+LL |     } else if let a = 1 {
+   |               ^^^^^^^^^
    |
    = note: this pattern will always match, so the `if let` is useless
    = help: consider replacing the `if let` with a `let`
 
 warning: irrefutable `if let` pattern
-  --> $DIR/if-let.rs:46:12
+  --> $DIR/if-let.rs:46:15
    |
-LL |       } else if let a = 1 {
-   |  ____________^
-LL | |         println!("irrefutable pattern");
-LL | |     }
-   | |_____^
+LL |     } else if let a = 1 {
+   |               ^^^^^^^^^
    |
    = note: this pattern will always match, so the `if let` is useless
    = help: consider replacing the `if let` with a `let`
diff --git a/src/test/ui/pattern/usefulness/deny-irrefutable-let-patterns.stderr b/src/test/ui/pattern/usefulness/deny-irrefutable-let-patterns.stderr
@@ -1,8 +1,8 @@
 error: irrefutable `if let` pattern
-  --> $DIR/deny-irrefutable-let-patterns.rs:7:5
+  --> $DIR/deny-irrefutable-let-patterns.rs:7:8
    |
 LL |     if let _ = 5 {}
-   |     ^^^^^^^^^^^^^^^
+   |        ^^^^^^^^^
    |
 note: the lint level is defined here
   --> $DIR/deny-irrefutable-let-patterns.rs:4:9
@@ -13,12 +13,10 @@ LL | #![deny(irrefutable_let_patterns)]
    = help: consider replacing the `if let` with a `let`
 
 error: irrefutable `while let` pattern
-  --> $DIR/deny-irrefutable-let-patterns.rs:9:5
+  --> $DIR/deny-irrefutable-let-patterns.rs:9:11
    |
-LL | /     while let _ = 5 {
-LL | |         break;
-LL | |     }
-   | |_____^
+LL |     while let _ = 5 {
+   |           ^^^^^^^^^
    |
    = note: this pattern will always match, so the loop will never exit
    = help: consider instead using a `loop { ... }` with a `let` inside it
diff --git a/src/test/ui/rfc-2008-non-exhaustive/uninhabited/patterns_same_crate.stderr b/src/test/ui/rfc-2008-non-exhaustive/uninhabited/patterns_same_crate.stderr
@@ -17,22 +17,22 @@ LL |         Some(_x) => (),
    |         ^^^^^^^^
 
 error: unreachable pattern
-  --> $DIR/patterns_same_crate.rs:61:15
+  --> $DIR/patterns_same_crate.rs:61:11
    |
 LL |     while let PartiallyInhabitedVariants::Struct { x } = partially_inhabited_variant() {
-   |               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 error: unreachable pattern
-  --> $DIR/patterns_same_crate.rs:65:15
+  --> $DIR/patterns_same_crate.rs:65:11
    |
 LL |     while let Some(_x) = uninhabited_struct() {
-   |               ^^^^^^^^
+   |           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 error: unreachable pattern
-  --> $DIR/patterns_same_crate.rs:68:15
+  --> $DIR/patterns_same_crate.rs:68:11
    |
 LL |     while let Some(_x) = uninhabited_tuple_struct() {
-   |               ^^^^^^^^
+   |           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 error: aborting due to 5 previous errors
 
diff --git a/src/test/ui/uninhabited/uninhabited-patterns.stderr b/src/test/ui/uninhabited/uninhabited-patterns.stderr
@@ -29,10 +29,10 @@ LL |         Err(Ok(_y)) => (),
    |         ^^^^^^^^^^^
 
 error: unreachable pattern
-  --> $DIR/uninhabited-patterns.rs:44:15
+  --> $DIR/uninhabited-patterns.rs:44:11
    |
 LL |     while let Some(_y) = foo() {
-   |               ^^^^^^^^
+   |           ^^^^^^^^^^^^^^^^^^^^
 
 error: aborting due to 5 previous errors
 
diff --git a/src/test/ui/while-let.stderr b/src/test/ui/while-let.stderr
@@ -1,8 +1,8 @@
 warning: irrefutable `while let` pattern
-  --> $DIR/while-let.rs:7:13
+  --> $DIR/while-let.rs:7:19
    |
 LL |               while let $p = $e $b
-   |               ^^^^^^^^^^^^^^^^^^^^
+   |                     ^^^
 ...
 LL | /     foo!(_a, 1, {
 LL | |         println!("irrefutable pattern");
@@ -15,10 +15,10 @@ LL | |     });
    = note: this warning originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)
 
 warning: irrefutable `while let` pattern
-  --> $DIR/while-let.rs:7:13
+  --> $DIR/while-let.rs:7:19
    |
 LL |               while let $p = $e $b
-   |               ^^^^^^^^^^^^^^^^^^^^
+   |                     ^^^
 ...
 LL | /     bar!(_a, 1, {
 LL | |         println!("irrefutable pattern");
@@ -30,13 +30,10 @@ LL | |     });
    = note: this warning originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)
 
 warning: irrefutable `while let` pattern
-  --> $DIR/while-let.rs:27:5
+  --> $DIR/while-let.rs:27:11
    |
-LL | /     while let _a = 1 {
-LL | |         println!("irrefutable pattern");
-LL | |         break;
-LL | |     }
-   | |_____^
+LL |     while let _a = 1 {
+   |           ^^^^^^^^^^
    |
    = note: this pattern will always match, so the loop will never exit
    = help: consider instead using a `loop { ... }` with a `let` inside it
diff --git a/src/tools/clippy/clippy_lints/src/if_let_mutex.rs b/src/tools/clippy/clippy_lints/src/if_let_mutex.rs
@@ -57,6 +57,7 @@ impl<'tcx> LateLintPass<'tcx> for IfLetMutex {
             ref arms,
             MatchSource::IfLetDesugar {
                 contains_else_clause: true,
+                ..
             },
         ) = ex.kind
         {
diff --git a/src/tools/clippy/clippy_lints/src/implicit_return.rs b/src/tools/clippy/clippy_lints/src/implicit_return.rs
@@ -92,6 +92,7 @@ fn expr_match(cx: &LateContext<'_>, expr: &Expr<'_>) {
             let check_all_arms = match source {
                 MatchSource::IfLetDesugar {
                     contains_else_clause: has_else,
+                    ..
                 } => has_else,
                 _ => true,
             };
diff --git a/src/tools/clippy/clippy_lints/src/loops.rs b/src/tools/clippy/clippy_lints/src/loops.rs
@@ -629,7 +629,7 @@ impl<'tcx> LateLintPass<'tcx> for Loops {
                 }
             }
         }
-        if let ExprKind::Match(ref match_expr, ref arms, MatchSource::WhileLetDesugar) = expr.kind {
+        if let ExprKind::Match(ref match_expr, ref arms, MatchSource::WhileLetDesugar { .. }) = expr.kind {
             let pat = &arms[0].pat.kind;
             if let (
                 &PatKind::TupleStruct(ref qpath, ref pat_args, _),
diff --git a/src/tools/clippy/clippy_lints/src/matches.rs b/src/tools/clippy/clippy_lints/src/matches.rs
@@ -1627,7 +1627,7 @@ mod redundant_pattern_match {
             match match_source {
                 MatchSource::Normal => find_sugg_for_match(cx, expr, op, arms),
                 MatchSource::IfLetDesugar { .. } => find_sugg_for_if_let(cx, expr, op, arms, "if"),
-                MatchSource::WhileLetDesugar => find_sugg_for_if_let(cx, expr, op, arms, "while"),
+                MatchSource::WhileLetDesugar { .. } => find_sugg_for_if_let(cx, expr, op, arms, "while"),
                 _ => {},
             }
         }
diff --git a/src/tools/clippy/clippy_lints/src/option_if_let_else.rs b/src/tools/clippy/clippy_lints/src/option_if_let_else.rs
@@ -118,6 +118,7 @@ fn should_wrap_in_braces(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
                     arms,
                     MatchSource::IfLetDesugar {
                         contains_else_clause: true,
+                        ..
                     },
                 ),
             ..
@@ -159,7 +160,7 @@ fn detect_option_if_let_else<'tcx>(
 ) -> Option<OptionIfLetElseOccurence> {
     if_chain! {
         if !utils::in_macro(expr.span); // Don't lint macros, because it behaves weirdly
-        if let ExprKind::Match(cond_expr, arms, MatchSource::IfLetDesugar{contains_else_clause: true}) = &expr.kind;
+        if let ExprKind::Match(cond_expr, arms, MatchSource::IfLetDesugar{ contains_else_clause: true, .. }) = &expr.kind;
         if arms.len() == 2;
         if !is_result_ok(cx, cond_expr); // Don't lint on Result::ok because a different lint does it already
         if let PatKind::TupleStruct(struct_qpath, &[inner_pat], _) = &arms[0].pat.kind;
diff --git a/src/tools/clippy/clippy_lints/src/pattern_type_mismatch.rs b/src/tools/clippy/clippy_lints/src/pattern_type_mismatch.rs
@@ -105,7 +105,7 @@ impl<'tcx> LateLintPass<'tcx> for PatternTypeMismatch {
     fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
         if let ExprKind::Match(ref expr, arms, source) = expr.kind {
             match source {
-                MatchSource::Normal | MatchSource::IfLetDesugar { .. } | MatchSource::WhileLetDesugar => {
+                MatchSource::Normal | MatchSource::IfLetDesugar { .. } | MatchSource::WhileLetDesugar { .. }=> {
                     if let Some(expr_ty) = cx.typeck_results().node_type_opt(expr.hir_id) {
                         'pattern_checks: for arm in arms {
                             let pat = &arm.pat;
diff --git a/src/tools/clippy/clippy_lints/src/question_mark.rs b/src/tools/clippy/clippy_lints/src/question_mark.rs
diff --git a/src/tools/clippy/clippy_lints/src/returns.rs b/src/tools/clippy/clippy_lints/src/returns.rs
diff --git a/src/tools/clippy/clippy_lints/src/utils/author.rs b/src/tools/clippy/clippy_lints/src/utils/author.rs
