Refactor while let loop to its own module

2 files changed, 92 insertions, 84 deletions

diff --git a/clippy_lints/src/loops/mod.rs b/clippy_lints/src/loops/mod.rs
index d205f053679..9076e48584a 100644
--- a/clippy_lints/src/loops/mod.rs
+++ b/clippy_lints/src/loops/mod.rs
@@ -12,14 +12,13 @@ mod needless_collect;
 mod never_loop;
 mod same_item_push;
 mod utils;
+mod while_let_loop;
 mod while_let_on_iterator;
 
 use crate::utils::sugg::Sugg;
-use crate::utils::{higher, snippet_with_applicability, span_lint_and_sugg, sugg};
-use rustc_errors::Applicability;
-use rustc_hir::{Block, Expr, ExprKind, LoopSource, MatchSource, Pat, StmtKind};
-use rustc_lint::{LateContext, LateLintPass, LintContext};
-use rustc_middle::lint::in_external_macro;
+use crate::utils::{higher, sugg};
+use rustc_hir::{Expr, ExprKind, LoopSource, Pat};
+use rustc_lint::{LateContext, LateLintPass};
 use rustc_session::{declare_lint_pass, declare_tool_lint};
 use rustc_span::source_map::Span;
 use utils::{get_span_of_entire_for_loop, make_iterator_snippet, IncrementVisitor, InitializeVisitor};
@@ -564,49 +563,7 @@ impl<'tcx> LateLintPass<'tcx> for Loops {
         if let ExprKind::Loop(ref block, _, LoopSource::Loop, _) = expr.kind {
             // also check for empty `loop {}` statements, skipping those in #[panic_handler]
             empty_loop::check_empty_loop(cx, expr, block);
-
-            // extract the expression from the first statement (if any) in a block
-            let inner_stmt_expr = extract_expr_from_first_stmt(block);
-            // or extract the first expression (if any) from the block
-            if let Some(inner) = inner_stmt_expr.or_else(|| extract_first_expr(block)) {
-                if let ExprKind::Match(ref matchexpr, ref 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;
-                                }
-
-                                // 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,
-                                );
-                            }
-                        },
-                        _ => (),
-                    }
-                }
-            }
+            while_let_loop::check_while_let_loop(cx, expr, block);
         }
 
         while_let_on_iterator::check_while_let_on_iterator(cx, expr);
@@ -709,39 +666,3 @@ impl std::ops::Sub<&MinifyingSugg<'static>> for MinifyingSugg<'static> {
         }
     }
 }
-
-/// 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(ref local) = block.stmts[0].kind {
-        local.init //.map(|expr| expr)
-    } else {
-        None
-    }
-}
-
-/// If a block begins with an expression (with or without semicolon), return it.
-fn extract_first_expr<'tcx>(block: &Block<'tcx>) -> Option<&'tcx Expr<'tcx>> {
-    match block.expr {
-        Some(ref expr) if block.stmts.is_empty() => Some(expr),
-        None if !block.stmts.is_empty() => match block.stmts[0].kind {
-            StmtKind::Expr(ref expr) | StmtKind::Semi(ref expr) => Some(expr),
-            StmtKind::Local(..) | StmtKind::Item(..) => None,
-        },
-        _ => None,
-    }
-}
-
-/// Returns `true` if expr contains a single break expr without destination label
-/// and
-/// passed expression. The expression may be within a block.
-fn is_simple_break_expr(expr: &Expr<'_>) -> bool {
-    match expr.kind {
-        ExprKind::Break(dest, ref passed_expr) if dest.label.is_none() && passed_expr.is_none() => true,
-        ExprKind::Block(ref b, _) => extract_first_expr(b).map_or(false, |subexpr| is_simple_break_expr(subexpr)),
-        _ => false,
-    }
-}
diff --git a/clippy_lints/src/loops/while_let_loop.rs b/clippy_lints/src/loops/while_let_loop.rs
new file mode 100644
index 00000000000..0c957986964
--- /dev/null
+++ b/clippy_lints/src/loops/while_let_loop.rs
@@ -0,0 +1,87 @@
+use super::WHILE_LET_LOOP;
+use crate::utils::{snippet_with_applicability, span_lint_and_sugg};
+use rustc_errors::Applicability;
+use rustc_hir::{Block, Expr, ExprKind, MatchSource, StmtKind};
+use rustc_lint::{LateContext, LintContext};
+use rustc_middle::lint::in_external_macro;
+
+pub(super) fn check_while_let_loop(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, loop_block: &'tcx Block<'_>) {
+    // extract the expression from the first statement (if any) in a 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(ref matchexpr, ref 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;
+                        }
+
+                        // 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 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(ref local) = block.stmts[0].kind {
+        local.init //.map(|expr| expr)
+    } else {
+        None
+    }
+}
+
+/// If a block begins with an expression (with or without semicolon), return it.
+fn extract_first_expr<'tcx>(block: &Block<'tcx>) -> Option<&'tcx Expr<'tcx>> {
+    match block.expr {
+        Some(ref expr) if block.stmts.is_empty() => Some(expr),
+        None if !block.stmts.is_empty() => match block.stmts[0].kind {
+            StmtKind::Expr(ref expr) | StmtKind::Semi(ref expr) => Some(expr),
+            StmtKind::Local(..) | StmtKind::Item(..) => None,
+        },
+        _ => None,
+    }
+}
+
+/// Returns `true` if expr contains a single break expr without destination label
+/// and
+/// passed expression. The expression may be within a block.
+fn is_simple_break_expr(expr: &Expr<'_>) -> bool {
+    match expr.kind {
+        ExprKind::Break(dest, ref passed_expr) if dest.label.is_none() && passed_expr.is_none() => true,
+        ExprKind::Block(ref b, _) => extract_first_expr(b).map_or(false, |subexpr| is_simple_break_expr(subexpr)),
+        _ => false,
+    }
+}