Fix Clippy with changed for loop desugar

8 files changed, 56 insertions, 59 deletions

diff --git a/clippy_lints/src/loops/explicit_counter_loop.rs b/clippy_lints/src/loops/explicit_counter_loop.rs
index 98e60f7ed85..6f213d7a699 100644
--- a/clippy_lints/src/loops/explicit_counter_loop.rs
+++ b/clippy_lints/src/loops/explicit_counter_loop.rs
@@ -1,6 +1,4 @@
-use super::{
-    get_span_of_entire_for_loop, make_iterator_snippet, IncrementVisitor, InitializeVisitor, EXPLICIT_COUNTER_LOOP,
-};
+use super::{make_iterator_snippet, IncrementVisitor, InitializeVisitor, EXPLICIT_COUNTER_LOOP};
 use clippy_utils::diagnostics::span_lint_and_sugg;
 use clippy_utils::source::snippet_with_applicability;
 use clippy_utils::{get_enclosing_block, is_integer_const};
@@ -37,12 +35,10 @@ pub(super) fn check<'tcx>(
                 then {
                     let mut applicability = Applicability::MachineApplicable;
 
-                    let for_span = get_span_of_entire_for_loop(expr);
-
                     span_lint_and_sugg(
                         cx,
                         EXPLICIT_COUNTER_LOOP,
-                        for_span.with_hi(arg.span.hi()),
+                        expr.span.with_hi(arg.span.hi()),
                         &format!("the variable `{}` is used as a loop counter", name),
                         "consider using",
                         format!(
diff --git a/clippy_lints/src/loops/manual_memcpy.rs b/clippy_lints/src/loops/manual_memcpy.rs
index 72027a163af..2362b4b2067 100644
--- a/clippy_lints/src/loops/manual_memcpy.rs
+++ b/clippy_lints/src/loops/manual_memcpy.rs
@@ -1,4 +1,4 @@
-use super::{get_span_of_entire_for_loop, IncrementVisitor, InitializeVisitor, MANUAL_MEMCPY};
+use super::{IncrementVisitor, InitializeVisitor, MANUAL_MEMCPY};
 use clippy_utils::diagnostics::span_lint_and_sugg;
 use clippy_utils::source::snippet;
 use clippy_utils::sugg::Sugg;
@@ -86,7 +86,7 @@ pub(super) fn check<'tcx>(
                 span_lint_and_sugg(
                     cx,
                     MANUAL_MEMCPY,
-                    get_span_of_entire_for_loop(expr),
+                    expr.span,
                     "it looks like you're manually copying between slices",
                     "try replacing the loop by",
                     big_sugg,
diff --git a/clippy_lints/src/loops/mod.rs b/clippy_lints/src/loops/mod.rs
index 5df1b796401..fd4881b2947 100644
--- a/clippy_lints/src/loops/mod.rs
+++ b/clippy_lints/src/loops/mod.rs
@@ -23,7 +23,7 @@ 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};
+use utils::{make_iterator_snippet, IncrementVisitor, InitializeVisitor};
 
 declare_clippy_lint! {
     /// ### What it does
@@ -566,7 +566,15 @@ declare_lint_pass!(Loops => [
 impl<'tcx> LateLintPass<'tcx> for Loops {
     #[allow(clippy::too_many_lines)]
     fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
-        if let Some(higher::ForLoop { pat, arg, body, span }) = higher::ForLoop::hir(expr) {
+        let for_loop = higher::ForLoop::hir(expr);
+        if let Some(higher::ForLoop {
+            pat,
+            arg,
+            body,
+            loop_id,
+            span,
+        }) = for_loop
+        {
             // we don't want to check expanded macros
             // this check is not at the top of the function
             // since higher::for_loop expressions are marked as expansions
@@ -574,6 +582,9 @@ impl<'tcx> LateLintPass<'tcx> for Loops {
                 return;
             }
             check_for_loop(cx, pat, arg, body, expr, span);
+            if let ExprKind::Block(block, _) = body.kind {
+                never_loop::check(cx, block, loop_id, span, for_loop.as_ref());
+            }
         }
 
         // we don't want to check expanded macros
@@ -582,7 +593,9 @@ impl<'tcx> LateLintPass<'tcx> for Loops {
         }
 
         // check for never_loop
-        never_loop::check(cx, expr);
+        if let ExprKind::Loop(block, ..) = expr.kind {
+            never_loop::check(cx, block, expr.hir_id, expr.span, None);
+        }
 
         // check for `loop { if let {} else break }` that could be `while let`
         // (also matches an explicit "match" instead of "if let")
diff --git a/clippy_lints/src/loops/never_loop.rs b/clippy_lints/src/loops/never_loop.rs
index c0fde5e5166..86b7d6d989a 100644
--- a/clippy_lints/src/loops/never_loop.rs
+++ b/clippy_lints/src/loops/never_loop.rs
@@ -4,35 +4,41 @@ use clippy_utils::diagnostics::span_lint_and_then;
 use clippy_utils::higher::ForLoop;
 use clippy_utils::source::snippet;
 use rustc_errors::Applicability;
-use rustc_hir::{Block, Expr, ExprKind, HirId, InlineAsmOperand, LoopSource, Node, Pat, Stmt, StmtKind};
+use rustc_hir::{Block, Expr, ExprKind, HirId, InlineAsmOperand, Pat, Stmt, StmtKind};
 use rustc_lint::LateContext;
+use rustc_span::Span;
 use std::iter::{once, Iterator};
 
-pub(super) fn check(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
-    if let ExprKind::Loop(block, _, source, _) = expr.kind {
-        match never_loop_block(block, expr.hir_id) {
-            NeverLoopResult::AlwaysBreak => {
-                span_lint_and_then(cx, NEVER_LOOP, expr.span, "this loop never actually loops", |diag| {
-                    if_chain! {
-                        if source == LoopSource::ForLoop;
-                        if let Some((_, Node::Expr(parent_match))) = cx.tcx.hir().parent_iter(expr.hir_id).nth(1);
-                        if let Some(ForLoop { arg: iterator, pat, span: for_span, .. }) = ForLoop::hir(parent_match);
-                        then {
-                            // Suggests using an `if let` instead. This is `Unspecified` because the
-                            // loop may (probably) contain `break` statements which would be invalid
-                            // in an `if let`.
-                            diag.span_suggestion_verbose(
-                                for_span.with_hi(iterator.span.hi()),
-                                "if you need the first element of the iterator, try writing",
-                                for_to_if_let_sugg(cx, iterator, pat),
-                                Applicability::Unspecified,
-                            );
-                        }
-                    };
-                });
-            },
-            NeverLoopResult::MayContinueMainLoop | NeverLoopResult::Otherwise => (),
-        }
+pub(super) fn check(
+    cx: &LateContext<'tcx>,
+    block: &'tcx Block<'_>,
+    loop_id: HirId,
+    span: Span,
+    for_loop: Option<&ForLoop<'_>>,
+) {
+    match never_loop_block(block, loop_id) {
+        NeverLoopResult::AlwaysBreak => {
+            span_lint_and_then(cx, NEVER_LOOP, span, "this loop never actually loops", |diag| {
+                if let Some(ForLoop {
+                    arg: iterator,
+                    pat,
+                    span: for_span,
+                    ..
+                }) = for_loop
+                {
+                    // Suggests using an `if let` instead. This is `Unspecified` because the
+                    // loop may (probably) contain `break` statements which would be invalid
+                    // in an `if let`.
+                    diag.span_suggestion_verbose(
+                        for_span.with_hi(iterator.span.hi()),
+                        "if you need the first element of the iterator, try writing",
+                        for_to_if_let_sugg(cx, iterator, pat),
+                        Applicability::Unspecified,
+                    );
+                }
+            });
+        },
+        NeverLoopResult::MayContinueMainLoop | NeverLoopResult::Otherwise => (),
     }
 }
 
diff --git a/clippy_lints/src/loops/single_element_loop.rs b/clippy_lints/src/loops/single_element_loop.rs
index 0fd09ff7197..e39605f3e7d 100644
--- a/clippy_lints/src/loops/single_element_loop.rs
+++ b/clippy_lints/src/loops/single_element_loop.rs
@@ -1,4 +1,4 @@
-use super::{get_span_of_entire_for_loop, SINGLE_ELEMENT_LOOP};
+use super::SINGLE_ELEMENT_LOOP;
 use clippy_utils::diagnostics::span_lint_and_sugg;
 use clippy_utils::single_segment_path;
 use clippy_utils::source::{indent_of, snippet};
@@ -30,7 +30,6 @@ pub(super) fn check<'tcx>(
         if !block.stmts.is_empty();
 
         then {
-            let for_span = get_span_of_entire_for_loop(expr);
             let mut block_str = snippet(cx, block.span, "..").into_owned();
             block_str.remove(0);
             block_str.pop();
@@ -39,7 +38,7 @@ pub(super) fn check<'tcx>(
             span_lint_and_sugg(
                 cx,
                 SINGLE_ELEMENT_LOOP,
-                for_span,
+                expr.span,
                 "for loop over a single element",
                 "try",
                 format!("{{\n{}let {} = &{};{}}}", " ".repeat(indent_of(cx, block.stmts[0].span).unwrap_or(0)), target.name, list_item_name, block_str),
diff --git a/clippy_lints/src/loops/utils.rs b/clippy_lints/src/loops/utils.rs
index f9f515cc40a..c3939a66c6a 100644
--- a/clippy_lints/src/loops/utils.rs
+++ b/clippy_lints/src/loops/utils.rs
@@ -7,7 +7,6 @@ use rustc_hir::HirIdMap;
 use rustc_hir::{BinOpKind, BorrowKind, Expr, ExprKind, HirId, Mutability, Pat, PatKind, Stmt, StmtKind};
 use rustc_lint::LateContext;
 use rustc_middle::hir::map::Map;
-use rustc_span::source_map::Span;
 use rustc_span::symbol::{sym, Symbol};
 use std::iter::Iterator;
 
@@ -300,17 +299,6 @@ impl<'tcx> Visitor<'tcx> for LoopNestVisitor {
     }
 }
 
-// this function assumes the given expression is a `for` loop.
-pub(super) fn get_span_of_entire_for_loop(expr: &Expr<'_>) -> Span {
-    // for some reason this is the only way to get the `Span`
-    // of the entire `for` loop
-    if let ExprKind::Match(_, arms, _) = &expr.kind {
-        arms[0].body.span
-    } else {
-        unreachable!()
-    }
-}
-
 /// If `arg` was the argument to a `for` loop, return the "cleanest" way of writing the
 /// actual `Iterator` that the loop uses.
 pub(super) fn make_iterator_snippet(cx: &LateContext<'_>, arg: &Expr<'_>, applic_ref: &mut Applicability) -> String {
diff --git a/clippy_lints/src/misc.rs b/clippy_lints/src/misc.rs
index 0f32cd9164e..78183add9cc 100644
--- a/clippy_lints/src/misc.rs
+++ b/clippy_lints/src/misc.rs
@@ -20,8 +20,8 @@ use rustc_span::symbol::sym;
 use clippy_utils::consts::{constant, Constant};
 use clippy_utils::sugg::Sugg;
 use clippy_utils::{
-    expr_path_res, get_item_name, get_parent_expr, higher, in_constant, is_diag_trait_item, is_integer_const,
-    iter_input_pats, last_path_segment, match_any_def_paths, paths, unsext, SpanlessEq,
+    expr_path_res, get_item_name, get_parent_expr, in_constant, is_diag_trait_item, is_integer_const, iter_input_pats,
+    last_path_segment, match_any_def_paths, paths, unsext, SpanlessEq,
 };
 
 declare_clippy_lint! {
@@ -312,7 +312,6 @@ impl<'tcx> LateLintPass<'tcx> for MiscLints {
             if let StmtKind::Local(local) = stmt.kind;
             if let PatKind::Binding(an, .., name, None) = local.pat.kind;
             if let Some(init) = local.init;
-            if !higher::is_from_for_desugar(local);
             if an == BindingAnnotation::Ref || an == BindingAnnotation::RefMut;
             then {
                 // use the macro callsite when the init span (but not the whole local span)
diff --git a/clippy_lints/src/unit_types/let_unit_value.rs b/clippy_lints/src/unit_types/let_unit_value.rs
index fad647dfb26..b25a6e3375b 100644
--- a/clippy_lints/src/unit_types/let_unit_value.rs
+++ b/clippy_lints/src/unit_types/let_unit_value.rs
@@ -1,5 +1,4 @@
 use clippy_utils::diagnostics::span_lint_and_then;
-use clippy_utils::higher;
 use clippy_utils::source::snippet_with_macro_callsite;
 use rustc_errors::Applicability;
 use rustc_hir::{Stmt, StmtKind};
@@ -14,9 +13,6 @@ pub(super) fn check(cx: &LateContext<'_>, stmt: &Stmt<'_>) {
             if in_external_macro(cx.sess(), stmt.span) || local.pat.span.from_expansion() {
                 return;
             }
-            if higher::is_from_for_desugar(local) {
-                return;
-            }
             span_lint_and_then(
                 cx,
                 LET_UNIT_VALUE,