Move span to last in position; match HirId for pat and match_expr equ…

…ality; clean up

clippy_lints/src/loops.rs

@@ -549,14 +549,14 @@ 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((span, pat, arg, body)) = higher::for_loop(expr) {
+        if let Some((pat, arg, body, span)) = higher::for_loop(expr) {
             // 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
             if body.span.from_expansion() {
                 return;
             }
-            check_for_loop(cx, span, pat, arg, body, expr);
+            check_for_loop(cx, pat, arg, body, expr, span);
         }
 
         // we don't want to check expanded macros
@@ -847,11 +847,11 @@ fn never_loop_expr_branch<'a, T: Iterator<Item = &'a Expr<'a>>>(e: &mut T, main_
 
 fn check_for_loop<'tcx>(
     cx: &LateContext<'tcx>,
-    span: Span,
     pat: &'tcx Pat<'_>,
     arg: &'tcx Expr<'_>,
     body: &'tcx Expr<'_>,
     expr: &'tcx Expr<'_>,
+    span: Span,
 ) {
     let is_manual_memcpy_triggered = detect_manual_memcpy(cx, pat, arg, body, expr);
     if !is_manual_memcpy_triggered {
@@ -863,7 +863,7 @@ fn check_for_loop<'tcx>(
     check_for_mut_range_bound(cx, arg, body);
     check_for_single_element_loop(cx, pat, arg, body, expr);
     detect_same_item_push(cx, pat, arg, body, expr);
-    check_manual_flatten(cx, span, pat, arg, body);
+    check_manual_flatten(cx, pat, arg, body, span);
 }
 
 // this function assumes the given expression is a `for` loop.
@@ -1991,10 +1991,10 @@ fn check_for_single_element_loop<'tcx>(
 /// iterator element is used.
 fn check_manual_flatten<'tcx>(
     cx: &LateContext<'tcx>,
-    span: Span,
     pat: &'tcx Pat<'_>,
     arg: &'tcx Expr<'_>,
     body: &'tcx Expr<'_>,
+    span: Span,
 ) {
     if_chain! {
         // Ensure the `if let` statement is the only expression in the for-loop
@@ -2006,7 +2006,10 @@ fn check_manual_flatten<'tcx>(
         ) = inner_expr.kind;
         if !contains_else_clause;
         // Ensure match_expr in `if let` statement is the same as the pat from the for-loop
-        if snippet(cx, match_expr.span, "..") == snippet(cx, pat.span, "..");
+        if let PatKind::Binding(_, pat_hir_id, _, _) = pat.kind;
+        if let ExprKind::Path(QPath::Resolved(None, match_expr_path)) = match_expr.kind;
+        if let Res::Local(match_expr_path_id) = match_expr_path.res;
+        if pat_hir_id == match_expr_path_id;
         // Ensure the `if let` statement is for the `Some` variant of `Option` or the `Ok` variant of `Result`
         if let PatKind::TupleStruct(QPath::Resolved(None, path), _, _) = match_arms[0].pat.kind;
         if is_some_ctor(cx, path.res) || is_ok_ctor(cx, path.res);

clippy_lints/src/mut_mut.rs

@@ -52,7 +52,7 @@ impl<'a, 'tcx> intravisit::Visitor<'tcx> for MutVisitor<'a, 'tcx> {
             return;
         }
 
-        if let Some((_, _, arg, body)) = higher::for_loop(expr) {
+        if let Some((_, arg, body, _)) = higher::for_loop(expr) {
             // A `for` loop lowers to:
             // ```rust
             // match ::std::iter::Iterator::next(&mut iter) {

clippy_lints/src/ranges.rs

@@ -442,7 +442,7 @@ fn check_reversed_empty_range(cx: &LateContext<'_>, expr: &Expr<'_>) {
         let mut cur_expr = expr;
         while let Some(parent_expr) = get_parent_expr(cx, cur_expr) {
             match higher::for_loop(parent_expr) {
-                Some((_, _, args, _)) if args.hir_id == expr.hir_id => return true,
+                Some((_, args, _, _)) if args.hir_id == expr.hir_id => return true,
                 _ => cur_expr = parent_expr,
             }
         }

clippy_lints/src/utils/higher.rs

@@ -134,11 +134,11 @@ pub fn is_from_for_desugar(local: &hir::Local<'_>) -> bool {
     false
 }
 
-/// Recover the entire span and the essential nodes of a desugared for loop:
-/// `for pat in arg { body }` becomes `(pat, arg, body)`. Return `(span, pat, arg, body)`.
+/// Recover the essential nodes of a desugared for loop as well as the entire span:
+/// `for pat in arg { body }` becomes `(pat, arg, body)`. Return `(pat, arg, body, span)`.
 pub fn for_loop<'tcx>(
     expr: &'tcx hir::Expr<'tcx>,
-) -> Option<(Span, &hir::Pat<'_>, &'tcx hir::Expr<'tcx>, &'tcx hir::Expr<'tcx>)> {
+) -> Option<(&hir::Pat<'_>, &'tcx hir::Expr<'tcx>, &'tcx hir::Expr<'tcx>, Span)> {
     if_chain! {
         if let hir::ExprKind::Match(ref iterexpr, ref arms, hir::MatchSource::ForLoopDesugar) = expr.kind;
         if let hir::ExprKind::Call(_, ref iterargs) = iterexpr.kind;
@@ -149,7 +149,7 @@ pub fn for_loop<'tcx>(
         if let hir::StmtKind::Local(ref local) = let_stmt.kind;
         if let hir::StmtKind::Expr(ref expr) = body.kind;
         then {
-            return Some((arms[0].span, &*local.pat, &iterargs[0], expr));
+            return Some((&*local.pat, &iterargs[0], expr, arms[0].span));
         }
     }
     None

clippy_lints/src/vec.rs

@@ -55,7 +55,7 @@ impl<'tcx> LateLintPass<'tcx> for UselessVec {
 
         // search for `for _ in vec![…]`
         if_chain! {
-            if let Some((_, _, arg, _)) = higher::for_loop(expr);
+            if let Some((_, arg, _, _)) = higher::for_loop(expr);
             if let Some(vec_args) = higher::vec_macro(cx, arg);
             if is_copy(cx, vec_type(cx.typeck_results().expr_ty_adjusted(arg)));
             then {