Simplify check_for_loop_arg

Y-Nak · Y-Nak · commit 74bd806b05b1 · 2021-03-02T18:14:20.000+09:00
diff --git a/clippy_lints/src/loops/explicit_into_iter_loop.rs b/clippy_lints/src/loops/explicit_into_iter_loop.rs
@@ -3,10 +3,17 @@ use crate::utils::{snippet_with_applicability, span_lint_and_sugg};
 use rustc_errors::Applicability;
 use rustc_hir::Expr;
 use rustc_lint::LateContext;
+use rustc_middle::ty::TyS;
+
+pub(super) fn check(cx: &LateContext<'_>, args: &'hir [Expr<'hir>], arg: &Expr<'_>) {
+    let receiver_ty = cx.typeck_results().expr_ty(&args[0]);
+    let receiver_ty_adjusted = cx.typeck_results().expr_ty_adjusted(&args[0]);
+    if !TyS::same_type(receiver_ty, receiver_ty_adjusted) {
+        return;
+    }
 
-pub(super) fn check(cx: &LateContext<'_>, method_args: &'hir [Expr<'hir>], arg: &Expr<'_>) {
     let mut applicability = Applicability::MachineApplicable;
-    let object = snippet_with_applicability(cx, method_args[0].span, "_", &mut applicability);
+    let object = snippet_with_applicability(cx, args[0].span, "_", &mut applicability);
     span_lint_and_sugg(
         cx,
         EXPLICIT_INTO_ITER_LOOP,
diff --git a/clippy_lints/src/loops/explicit_iter_loop.rs b/clippy_lints/src/loops/explicit_iter_loop.rs
@@ -1,10 +1,35 @@
 use super::EXPLICIT_ITER_LOOP;
-use crate::utils::{snippet_with_applicability, span_lint_and_sugg};
+use crate::utils::{match_trait_method, snippet_with_applicability, span_lint_and_sugg};
 use rustc_errors::Applicability;
-use rustc_hir::Expr;
+use rustc_hir::{Expr, Mutability};
 use rustc_lint::LateContext;
+use rustc_middle::ty::{self, Ty, TyS};
+use rustc_span::symbol::sym;
+
+use crate::utils::{is_type_diagnostic_item, match_type, paths};
 
 pub(super) fn check(cx: &LateContext<'_>, args: &[Expr<'_>], arg: &Expr<'_>, method_name: &str) {
+    let should_lint = match method_name {
+        "iter" | "iter_mut" => is_ref_iterable_type(cx, &args[0]),
+        "into_iter" if match_trait_method(cx, arg, &paths::INTO_ITERATOR) => {
+            let receiver_ty = cx.typeck_results().expr_ty(&args[0]);
+            let receiver_ty_adjusted = cx.typeck_results().expr_ty_adjusted(&args[0]);
+            let ref_receiver_ty = cx.tcx.mk_ref(
+                cx.tcx.lifetimes.re_erased,
+                ty::TypeAndMut {
+                    ty: receiver_ty,
+                    mutbl: Mutability::Not,
+                },
+            );
+            TyS::same_type(receiver_ty_adjusted, ref_receiver_ty)
+        },
+        _ => false,
+    };
+
+    if !should_lint {
+        return;
+    }
+
     let mut applicability = Applicability::MachineApplicable;
     let object = snippet_with_applicability(cx, args[0].span, "_", &mut applicability);
     let muta = if method_name == "iter_mut" { "mut " } else { "" };
@@ -19,3 +44,31 @@ pub(super) fn check(cx: &LateContext<'_>, args: &[Expr<'_>], arg: &Expr<'_>, met
         applicability,
     )
 }
+
+/// Returns `true` if the type of expr is one that provides `IntoIterator` impls
+/// for `&T` and `&mut T`, such as `Vec`.
+#[rustfmt::skip]
+fn is_ref_iterable_type(cx: &LateContext<'_>, e: &Expr<'_>) -> bool {
+    // no walk_ptrs_ty: calling iter() on a reference can make sense because it
+    // will allow further borrows afterwards
+    let ty = cx.typeck_results().expr_ty(e);
+    is_iterable_array(ty, cx) ||
+    is_type_diagnostic_item(cx, ty, sym::vec_type) ||
+    match_type(cx, ty, &paths::LINKED_LIST) ||
+    is_type_diagnostic_item(cx, ty, sym!(hashmap_type)) ||
+    is_type_diagnostic_item(cx, ty, sym!(hashset_type)) ||
+    is_type_diagnostic_item(cx, ty, sym!(vecdeque_type)) ||
+    match_type(cx, ty, &paths::BINARY_HEAP) ||
+    match_type(cx, ty, &paths::BTREEMAP) ||
+    match_type(cx, ty, &paths::BTREESET)
+}
+
+fn is_iterable_array<'tcx>(ty: Ty<'tcx>, cx: &LateContext<'tcx>) -> bool {
+    // IntoIterator is currently only implemented for array sizes <= 32 in rustc
+    match ty.kind() {
+        ty::Array(_, n) => n
+            .try_eval_usize(cx.tcx, cx.param_env)
+            .map_or(false, |val| (0..=32).contains(&val)),
+        _ => false,
+    }
+}
diff --git a/clippy_lints/src/loops/iter_next_loop.rs b/clippy_lints/src/loops/iter_next_loop.rs
@@ -1,14 +1,19 @@
 use super::ITER_NEXT_LOOP;
-use crate::utils::span_lint;
+use crate::utils::{match_trait_method, paths, span_lint};
 use rustc_hir::Expr;
 use rustc_lint::LateContext;
 
-pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>) {
-    span_lint(
-        cx,
-        ITER_NEXT_LOOP,
-        expr.span,
-        "you are iterating over `Iterator::next()` which is an Option; this will compile but is \
-        probably not what you want",
-    );
+pub(super) fn check(cx: &LateContext<'_>, arg: &Expr<'_>, expr: &Expr<'_>) -> bool {
+    if match_trait_method(cx, arg, &paths::ITERATOR) {
+        span_lint(
+            cx,
+            ITER_NEXT_LOOP,
+            expr.span,
+            "you are iterating over `Iterator::next()` which is an Option; this will compile but is \
+            probably not what you want",
+        );
+        true
+    } else {
+        false
+    }
 }
diff --git a/clippy_lints/src/loops/mod.rs b/clippy_lints/src/loops/mod.rs
@@ -18,13 +18,11 @@ mod while_immutable_condition;
 mod while_let_loop;
 mod while_let_on_iterator;
 
-use crate::utils::{higher, is_type_diagnostic_item, match_trait_method, match_type, paths};
-use rustc_hir::{Expr, ExprKind, LoopSource, Mutability, Pat};
+use crate::utils::higher;
+use rustc_hir::{Expr, ExprKind, LoopSource, Pat};
 use rustc_lint::{LateContext, LateLintPass};
-use rustc_middle::ty::{self, Ty, TyS};
 use rustc_session::{declare_lint_pass, declare_tool_lint};
 use rustc_span::source_map::Span;
-use rustc_span::symbol::sym;
 use utils::{get_span_of_entire_for_loop, make_iterator_snippet, IncrementVisitor, InitializeVisitor};
 
 declare_clippy_lint! {
@@ -603,67 +601,27 @@ fn check_for_loop<'tcx>(
 
 fn check_for_loop_arg(cx: &LateContext<'_>, pat: &Pat<'_>, arg: &Expr<'_>, expr: &Expr<'_>) {
     let mut next_loop_linted = false; // whether or not ITER_NEXT_LOOP lint was used
+
     if let ExprKind::MethodCall(ref method, _, ref args, _) = arg.kind {
         // just the receiver, no arguments
         if args.len() == 1 {
             let method_name = &*method.ident.as_str();
             // check for looping over x.iter() or x.iter_mut(), could use &x or &mut x
-            if method_name == "iter" || method_name == "iter_mut" {
-                if is_ref_iterable_type(cx, &args[0]) {
+            match method_name {
+                "iter" | "iter_mut" => explicit_iter_loop::check(cx, args, arg, method_name),
+                "into_iter" => {
                     explicit_iter_loop::check(cx, args, arg, method_name);
-                }
-            } else if method_name == "into_iter" && match_trait_method(cx, arg, &paths::INTO_ITERATOR) {
-                let receiver_ty = cx.typeck_results().expr_ty(&args[0]);
-                let receiver_ty_adjusted = cx.typeck_results().expr_ty_adjusted(&args[0]);
-                if TyS::same_type(receiver_ty, receiver_ty_adjusted) {
                     explicit_into_iter_loop::check(cx, args, arg);
-                } else {
-                    let ref_receiver_ty = cx.tcx.mk_ref(
-                        cx.tcx.lifetimes.re_erased,
-                        ty::TypeAndMut {
-                            ty: receiver_ty,
-                            mutbl: Mutability::Not,
-                        },
-                    );
-                    if TyS::same_type(receiver_ty_adjusted, ref_receiver_ty) {
-                        explicit_iter_loop::check(cx, args, arg, method_name)
-                    }
-                }
-            } else if method_name == "next" && match_trait_method(cx, arg, &paths::ITERATOR) {
-                iter_next_loop::check(cx, expr);
-                next_loop_linted = true;
+                },
+                "next" => {
+                    next_loop_linted = iter_next_loop::check(cx, arg, expr);
+                },
+                _ => {},
             }
         }
     }
+
     if !next_loop_linted {
         for_loops_over_fallibles::check(cx, pat, arg);
     }
 }
-
-/// Returns `true` if the type of expr is one that provides `IntoIterator` impls
-/// for `&T` and `&mut T`, such as `Vec`.
-#[rustfmt::skip]
-fn is_ref_iterable_type(cx: &LateContext<'_>, e: &Expr<'_>) -> bool {
-    // no walk_ptrs_ty: calling iter() on a reference can make sense because it
-    // will allow further borrows afterwards
-    let ty = cx.typeck_results().expr_ty(e);
-    is_iterable_array(ty, cx) ||
-    is_type_diagnostic_item(cx, ty, sym::vec_type) ||
-    match_type(cx, ty, &paths::LINKED_LIST) ||
-    is_type_diagnostic_item(cx, ty, sym!(hashmap_type)) ||
-    is_type_diagnostic_item(cx, ty, sym!(hashset_type)) ||
-    is_type_diagnostic_item(cx, ty, sym!(vecdeque_type)) ||
-    match_type(cx, ty, &paths::BINARY_HEAP) ||
-    match_type(cx, ty, &paths::BTREEMAP) ||
-    match_type(cx, ty, &paths::BTREESET)
-}
-
-fn is_iterable_array<'tcx>(ty: Ty<'tcx>, cx: &LateContext<'tcx>) -> bool {
-    // IntoIterator is currently only implemented for array sizes <= 32 in rustc
-    match ty.kind() {
-        ty::Array(_, n) => n
-            .try_eval_usize(cx.tcx, cx.param_env)
-            .map_or(false, |val| (0..=32).contains(&val)),
-        _ => false,
-    }
-}
