Introduce hir::ExprKind::Let - Take 2

1 files changed, 56 insertions, 42 deletions

diff --git a/clippy_lints/src/loops/while_let_loop.rs b/clippy_lints/src/loops/while_let_loop.rs
index 9c172079852..6be410ca8e3 100644
--- a/clippy_lints/src/loops/while_let_loop.rs
+++ b/clippy_lints/src/loops/while_let_loop.rs
@@ -1,8 +1,9 @@
 use super::WHILE_LET_LOOP;
 use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::higher;
 use clippy_utils::source::snippet_with_applicability;
 use rustc_errors::Applicability;
-use rustc_hir::{Block, Expr, ExprKind, MatchSource, StmtKind};
+use rustc_hir::{Block, Expr, ExprKind, MatchSource, Pat, StmtKind};
 use rustc_lint::{LateContext, LintContext};
 use rustc_middle::lint::in_external_macro;
 
@@ -11,41 +12,25 @@ pub(super) fn check(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, loop_block: &'
     let inner_stmt_expr = extract_expr_from_first_stmt(loop_block);
     // or extract the first expression (if any) from the block
     if let Some(inner) = inner_stmt_expr.or_else(|| extract_first_expr(loop_block)) {
-        if let ExprKind::Match(matchexpr, arms, ref source) = inner.kind {
-            // ensure "if let" compatible match structure
-            match *source {
-                MatchSource::Normal | MatchSource::IfLetDesugar { .. } => {
-                    if arms.len() == 2
-                        && arms[0].guard.is_none()
-                        && arms[1].guard.is_none()
-                        && is_simple_break_expr(arms[1].body)
-                    {
-                        if in_external_macro(cx.sess(), expr.span) {
-                            return;
-                        }
+        if let Some(higher::IfLet {
+            let_pat,
+            let_expr,
+            if_else: Some(if_else),
+            ..
+        }) = higher::IfLet::hir(inner)
+        {
+            if is_simple_break_expr(if_else) {
+                could_be_while_let(cx, expr, let_pat, let_expr);
+            }
+        }
 
-                        // NOTE: we used to build a body here instead of using
-                        // ellipsis, this was removed because:
-                        // 1) it was ugly with big bodies;
-                        // 2) it was not indented properly;
-                        // 3) it wasn’t very smart (see #675).
-                        let mut applicability = Applicability::HasPlaceholders;
-                        span_lint_and_sugg(
-                            cx,
-                            WHILE_LET_LOOP,
-                            expr.span,
-                            "this loop could be written as a `while let` loop",
-                            "try",
-                            format!(
-                                "while let {} = {} {{ .. }}",
-                                snippet_with_applicability(cx, arms[0].pat.span, "..", &mut applicability),
-                                snippet_with_applicability(cx, matchexpr.span, "..", &mut applicability),
-                            ),
-                            applicability,
-                        );
-                    }
-                },
-                _ => (),
+        if let ExprKind::Match(ref matchexpr, ref arms, MatchSource::Normal) = inner.kind {
+            if arms.len() == 2
+                && arms[0].guard.is_none()
+                && arms[1].guard.is_none()
+                && is_simple_break_expr(&arms[1].body)
+            {
+                could_be_while_let(cx, expr, &arms[0].pat, matchexpr);
             }
         }
     }
@@ -54,14 +39,12 @@ pub(super) fn check(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, loop_block: &'
 /// If a block begins with a statement (possibly a `let` binding) and has an
 /// expression, return it.
 fn extract_expr_from_first_stmt<'tcx>(block: &Block<'tcx>) -> Option<&'tcx Expr<'tcx>> {
-    if block.stmts.is_empty() {
-        return None;
-    }
-    if let StmtKind::Local(local) = block.stmts[0].kind {
-        local.init //.map(|expr| expr)
-    } else {
-        None
+    if let Some(first_stmt) = block.stmts.get(0) {
+        if let StmtKind::Local(local) = first_stmt.kind {
+            return local.init;
+        }
     }
+    None
 }
 
 /// If a block begins with an expression (with or without semicolon), return it.
@@ -86,3 +69,34 @@ fn is_simple_break_expr(expr: &Expr<'_>) -> bool {
         _ => false,
     }
 }
+
+fn could_be_while_let<'tcx>(
+    cx: &LateContext<'tcx>,
+    expr: &'tcx Expr<'_>,
+    let_pat: &'tcx Pat<'_>,
+    let_expr: &'tcx Expr<'_>,
+) {
+    if in_external_macro(cx.sess(), expr.span) {
+        return;
+    }
+
+    // NOTE: we used to build a body here instead of using
+    // ellipsis, this was removed because:
+    // 1) it was ugly with big bodies;
+    // 2) it was not indented properly;
+    // 3) it wasn’t very smart (see #675).
+    let mut applicability = Applicability::HasPlaceholders;
+    span_lint_and_sugg(
+        cx,
+        WHILE_LET_LOOP,
+        expr.span,
+        "this loop could be written as a `while let` loop",
+        "try",
+        format!(
+            "while let {} = {} {{ .. }}",
+            snippet_with_applicability(cx, let_pat.span, "..", &mut applicability),
+            snippet_with_applicability(cx, let_expr.span, "..", &mut applicability),
+        ),
+        applicability,
+    );
+}