Extend explicit_iter_loop to all types

Author: Jarcho

clippy_lints/src/lib.rs

@@ -679,7 +679,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     });
     store.register_late_pass(|_| Box::<shadow::Shadow>::default());
     store.register_late_pass(|_| Box::new(unit_types::UnitTypes));
-    store.register_late_pass(|_| Box::new(loops::Loops));
+    store.register_late_pass(move |_| Box::new(loops::Loops::new(msrv())));
     store.register_late_pass(|_| Box::<main_recursion::MainRecursion>::default());
     store.register_late_pass(|_| Box::new(lifetimes::Lifetimes));
     store.register_late_pass(|_| Box::new(entry::HashMapPass));

clippy_lints/src/loops/explicit_iter_loop.rs

@@ -1,18 +1,37 @@
 use super::EXPLICIT_ITER_LOOP;
 use clippy_utils::diagnostics::span_lint_and_sugg;
 use clippy_utils::is_trait_method;
+use clippy_utils::msrvs::{self, Msrv};
 use clippy_utils::source::snippet_with_applicability;
-use clippy_utils::ty::is_type_diagnostic_item;
+use clippy_utils::ty::{implements_trait_with_env, make_normalized_projection_with_regions, normalize_with_regions,
+    make_normalized_projection, implements_trait, is_copy};
 use rustc_errors::Applicability;
 use rustc_hir::{Expr, Mutability};
 use rustc_lint::LateContext;
-use rustc_middle::ty::{self, Ty};
+use rustc_middle::ty::adjustment::{Adjust, Adjustment, AutoBorrow, AutoBorrowMutability};
+use rustc_middle::ty::{self, EarlyBinder, TypeAndMut, Ty};
 use rustc_span::sym;
 
-pub(super) fn check(cx: &LateContext<'_>, self_arg: &Expr<'_>, arg: &Expr<'_>, method_name: &str) {
-    let should_lint = match method_name {
-        "iter" | "iter_mut" => is_ref_iterable_type(cx, self_arg),
-        "into_iter" if is_trait_method(cx, arg, sym::IntoIterator) => {
+pub(super) fn check(cx: &LateContext<'_>, self_arg: &Expr<'_>, call_expr: &Expr<'_>, method_name: &str, msrv: &Msrv) {
+    let borrow_kind = match method_name {
+        "iter" | "iter_mut" => match is_ref_iterable(cx, self_arg, call_expr) {
+            Some((kind, ty)) => {
+                if let ty::Array(_, count) = *ty.peel_refs().kind() {
+                    if !ty.is_ref() {
+                        if !msrv.meets(msrvs::ARRAY_INTO_ITERATOR) {
+                            return;
+                        }
+                    } else if count.try_eval_target_usize(cx.tcx, cx.param_env).map_or(true, |x| x > 32)
+                        && !msrv.meets(msrvs::ARRAY_IMPL_ANY_LEN)
+                    {
+                        return
+                    }
+                }
+                kind
+            },
+            None => return,
+        },
+        "into_iter" if is_trait_method(cx, call_expr, sym::IntoIterator) => {
             let receiver_ty = cx.typeck_results().expr_ty(self_arg);
             let receiver_ty_adjusted = cx.typeck_results().expr_ty_adjusted(self_arg);
             let ref_receiver_ty = cx.tcx.mk_ref(
@@ -22,54 +41,159 @@ pub(super) fn check(cx: &LateContext<'_>, self_arg: &Expr<'_>, arg: &Expr<'_>, m
                     mutbl: Mutability::Not,
                 },
             );
-            receiver_ty_adjusted == ref_receiver_ty
+            if receiver_ty_adjusted == ref_receiver_ty {
+                AdjustKind::None
+            } else {
+                return;
+            }
         },
-        _ => false,
+        _ => return,
     };
 
-    if !should_lint {
-        return;
-    }
-
     let mut applicability = Applicability::MachineApplicable;
     let object = snippet_with_applicability(cx, self_arg.span, "_", &mut applicability);
-    let muta = if method_name == "iter_mut" { "mut " } else { "" };
+    let prefix = match borrow_kind {
+        AdjustKind::None => "",
+        AdjustKind::Borrow => "&",
+        AdjustKind::BorrowMut => "&mut ",
+        AdjustKind::Deref => "*",
+        AdjustKind::Reborrow => "&*",
+        AdjustKind::ReborrowMut => "&mut *",
+    };
     span_lint_and_sugg(
         cx,
         EXPLICIT_ITER_LOOP,
-        arg.span,
+        call_expr.span,
         "it is more concise to loop over references to containers instead of using explicit \
          iteration methods",
         "to write this more concisely, try",
-        format!("&{muta}{object}"),
+        format!("{prefix}{object}"),
         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) ||
-    is_type_diagnostic_item(cx, ty, sym::LinkedList) ||
-    is_type_diagnostic_item(cx, ty, sym::HashMap) ||
-    is_type_diagnostic_item(cx, ty, sym::HashSet) ||
-    is_type_diagnostic_item(cx, ty, sym::VecDeque) ||
-    is_type_diagnostic_item(cx, ty, sym::BinaryHeap) ||
-    is_type_diagnostic_item(cx, ty, sym::BTreeMap) ||
-    is_type_diagnostic_item(cx, ty, sym::BTreeSet)
+enum AdjustKind {
+    None,
+    Borrow,
+    BorrowMut,
+    Deref,
+    Reborrow,
+    ReborrowMut,
+}
+impl AdjustKind {
+    fn borrow(mutbl: Mutability) -> Self {
+        match mutbl {
+            Mutability::Not => Self::Borrow,
+            Mutability::Mut => Self::BorrowMut,
+        }
+    }
+
+    fn auto_borrow(mutbl: AutoBorrowMutability) -> Self {
+        match mutbl {
+            AutoBorrowMutability::Not => Self::Borrow,
+            AutoBorrowMutability::Mut { .. } => Self::BorrowMut,
+        }
+    }
+
+    fn reborrow(mutbl: AutoBorrowMutability) -> Self {
+        match mutbl {
+            AutoBorrowMutability::Not => Self::Reborrow,
+            AutoBorrowMutability::Mut { .. } => Self::ReborrowMut,
+        }
+    }
 }
 
-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_target_usize(cx.tcx, cx.param_env)
-            .map_or(false, |val| (0..=32).contains(&val)),
-        _ => false,
+/// Checks if an `iter` or `iter_mut` call returns `IntoIterator::IntoIter`. Returns how the
+/// argument needs to be adjusted.
+fn is_ref_iterable<'tcx>(cx: &LateContext<'tcx>, self_arg: &Expr<'_>, call_expr: &Expr<'_>) -> Option<(AdjustKind, Ty<'tcx>)> {
+    let typeck = cx.typeck_results();
+    if let Some(trait_id) = cx.tcx.get_diagnostic_item(sym::IntoIterator)
+        && let Some(fn_id) = typeck.type_dependent_def_id(call_expr.hir_id)
+        && let sig = cx.tcx.liberate_late_bound_regions(fn_id, cx.tcx.fn_sig(fn_id).skip_binder())
+        && let &[req_self_ty, req_res_ty] = &**sig.inputs_and_output
+        && let param_env = cx.tcx.param_env(fn_id)
+        && implements_trait_with_env(cx.tcx, param_env, req_self_ty, trait_id, [])
+        && let Some(into_iter_ty) =
+            make_normalized_projection_with_regions(cx.tcx, param_env, trait_id, sym!(IntoIter), [req_self_ty])
+        && let req_res_ty = normalize_with_regions(cx.tcx, param_env, req_res_ty)
+        && into_iter_ty == req_res_ty
+    {
+        let adjustments = typeck.expr_adjustments(self_arg);
+        let self_ty = typeck.expr_ty(self_arg);
+        let self_is_copy = is_copy(cx, self_ty);
+
+        if adjustments.is_empty() && self_is_copy {
+            return Some((AdjustKind::None, self_ty));
+        }
+
+        let res_ty = cx.tcx.erase_regions(EarlyBinder::bind(req_res_ty).subst(cx.tcx, typeck.node_substs(call_expr.hir_id)));
+        if !adjustments.is_empty() && self_is_copy {
+            if implements_trait(cx, self_ty, trait_id, &[])
+                && let Some(ty) = make_normalized_projection(cx.tcx, cx.param_env, trait_id, sym!(IntoIter), [self_ty])
+                && ty == res_ty
+            {
+                return Some((AdjustKind::None, self_ty));
+            }
+        }
+
+        let mutbl = if let ty::Ref(_, _, mutbl) = *req_self_ty.kind() {
+            Some(mutbl)
+        } else {
+            None
+        };
+        if let Some(mutbl) = mutbl
+            && !self_ty.is_ref()
+        {
+            let self_ty = cx.tcx.mk_ref(cx.tcx.lifetimes.re_erased, TypeAndMut {
+                ty: self_ty,
+                mutbl,
+            });
+            if implements_trait(cx, self_ty, trait_id, &[])
+                && let Some(ty) = make_normalized_projection(cx.tcx, cx.param_env, trait_id, sym!(IntoIter), [self_ty])
+                && ty == res_ty
+            {
+                return Some((AdjustKind::borrow(mutbl), self_ty));
+            }
+        }
+
+        match adjustments {
+            [] => Some((AdjustKind::None, self_ty)),
+            &[Adjustment { kind: Adjust::Deref(_), ..}, Adjustment { kind: Adjust::Borrow(AutoBorrow::Ref(_, mutbl)), target }, ..] => {
+                if target != self_ty
+                    && implements_trait(cx, target, trait_id, &[])
+                    && let Some(ty) = make_normalized_projection(cx.tcx, cx.param_env, trait_id, sym!(IntoIter), [target])
+                    && ty == res_ty
+                {
+                    Some((AdjustKind::reborrow(mutbl), target))
+                } else {
+                    None
+                }
+            }
+            &[Adjustment { kind: Adjust::Deref(_), target }, ..] => {
+                if is_copy(cx, target)
+                    && implements_trait(cx, target, trait_id, &[])
+                    && let Some(ty) = make_normalized_projection(cx.tcx, cx.param_env, trait_id, sym!(IntoIter), [target])
+                    && ty == res_ty
+                {
+                    Some((AdjustKind::Deref, target))
+                } else {
+                    None
+                }
+            }
+            &[Adjustment { kind: Adjust::Borrow(AutoBorrow::Ref(_, mutbl)), target }, ..] => {
+                if self_ty.is_ref()
+                    && implements_trait(cx, target, trait_id, &[])
+                    && let Some(ty) = make_normalized_projection(cx.tcx, cx.param_env, trait_id, sym!(IntoIter), [target])
+                    && ty == res_ty
+                {
+                    Some((AdjustKind::auto_borrow(mutbl), target))
+                } else {
+                    None
+                }
+            }
+            _ => None,
+        }
+    } else {
+        None
     }
 }

clippy_lints/src/loops/mod.rs

@@ -20,9 +20,10 @@ mod while_let_loop;
 mod while_let_on_iterator;
 
 use clippy_utils::higher;
+use clippy_utils::msrvs::Msrv;
 use rustc_hir::{Expr, ExprKind, LoopSource, Pat};
 use rustc_lint::{LateContext, LateLintPass};
-use rustc_session::{declare_lint_pass, declare_tool_lint};
+use rustc_session::{declare_tool_lint, impl_lint_pass};
 use rustc_span::source_map::Span;
 use utils::{make_iterator_snippet, IncrementVisitor, InitializeVisitor};
 
@@ -606,7 +607,15 @@ declare_clippy_lint! {
     "checking for emptiness of a `Vec` in the loop condition and popping an element in the body"
 }
 
-declare_lint_pass!(Loops => [
+pub struct Loops {
+    msrv: Msrv,
+}
+impl Loops {
+    pub fn new(msrv: Msrv) -> Self {
+        Self { msrv }
+    }
+}
+impl_lint_pass!(Loops => [
     MANUAL_MEMCPY,
     MANUAL_FLATTEN,
     NEEDLESS_RANGE_LOOP,
@@ -645,7 +654,7 @@ impl<'tcx> LateLintPass<'tcx> for Loops {
             if body.span.from_expansion() {
                 return;
             }
-            check_for_loop(cx, pat, arg, body, expr, span);
+            self.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());
             }
@@ -680,44 +689,47 @@ impl<'tcx> LateLintPass<'tcx> for Loops {
     }
 }
 
-fn check_for_loop<'tcx>(
-    cx: &LateContext<'tcx>,
-    pat: &'tcx Pat<'_>,
-    arg: &'tcx Expr<'_>,
-    body: &'tcx Expr<'_>,
-    expr: &'tcx Expr<'_>,
-    span: Span,
-) {
-    let is_manual_memcpy_triggered = manual_memcpy::check(cx, pat, arg, body, expr);
-    if !is_manual_memcpy_triggered {
-        needless_range_loop::check(cx, pat, arg, body, expr);
-        explicit_counter_loop::check(cx, pat, arg, body, expr);
+impl Loops {
+    fn check_for_loop<'tcx>(
+        &self,
+        cx: &LateContext<'tcx>,
+        pat: &'tcx Pat<'_>,
+        arg: &'tcx Expr<'_>,
+        body: &'tcx Expr<'_>,
+        expr: &'tcx Expr<'_>,
+        span: Span,
+    ) {
+        let is_manual_memcpy_triggered = manual_memcpy::check(cx, pat, arg, body, expr);
+        if !is_manual_memcpy_triggered {
+            needless_range_loop::check(cx, pat, arg, body, expr);
+            explicit_counter_loop::check(cx, pat, arg, body, expr);
+        }
+        self.check_for_loop_arg(cx, pat, arg);
+        for_kv_map::check(cx, pat, arg, body);
+        mut_range_bound::check(cx, arg, body);
+        single_element_loop::check(cx, pat, arg, body, expr);
+        same_item_push::check(cx, pat, arg, body, expr);
+        manual_flatten::check(cx, pat, arg, body, span);
+        manual_find::check(cx, pat, arg, body, span, expr);
     }
-    check_for_loop_arg(cx, pat, arg);
-    for_kv_map::check(cx, pat, arg, body);
-    mut_range_bound::check(cx, arg, body);
-    single_element_loop::check(cx, pat, arg, body, expr);
-    same_item_push::check(cx, pat, arg, body, expr);
-    manual_flatten::check(cx, pat, arg, body, span);
-    manual_find::check(cx, pat, arg, body, span, expr);
-}
 
-fn check_for_loop_arg(cx: &LateContext<'_>, _: &Pat<'_>, arg: &Expr<'_>) {
-    if let ExprKind::MethodCall(method, self_arg, [], _) = arg.kind {
-        let method_name = method.ident.as_str();
-        // check for looping over x.iter() or x.iter_mut(), could use &x or &mut x
-        match method_name {
-            "iter" | "iter_mut" => {
-                explicit_iter_loop::check(cx, self_arg, arg, method_name);
-            },
-            "into_iter" => {
-                explicit_iter_loop::check(cx, self_arg, arg, method_name);
-                explicit_into_iter_loop::check(cx, self_arg, arg);
-            },
-            "next" => {
-                iter_next_loop::check(cx, arg);
-            },
-            _ => {},
+    fn check_for_loop_arg(&self, cx: &LateContext<'_>, _: &Pat<'_>, arg: &Expr<'_>) {
+        if let ExprKind::MethodCall(method, self_arg, [], _) = arg.kind {
+            let method_name = method.ident.as_str();
+            // check for looping over x.iter() or x.iter_mut(), could use &x or &mut x
+            match method_name {
+                "iter" | "iter_mut" => {
+                    explicit_iter_loop::check(cx, self_arg, arg, method_name, &self.msrv);
+                },
+                "into_iter" => {
+                    explicit_iter_loop::check(cx, self_arg, arg, method_name, &self.msrv);
+                    explicit_into_iter_loop::check(cx, self_arg, arg);
+                },
+                "next" => {
+                    iter_next_loop::check(cx, arg);
+                },
+                _ => {},
+            }
         }
     }
 }

clippy_utils/src/msrvs.rs

@@ -28,7 +28,7 @@ msrv_aliases! {
     1,52,0 { STR_SPLIT_ONCE, REM_EUCLID_CONST }
     1,51,0 { BORROW_AS_PTR, SEEK_FROM_CURRENT, UNSIGNED_ABS }
     1,50,0 { BOOL_THEN, CLAMP }
-    1,47,0 { TAU, IS_ASCII_DIGIT_CONST }
+    1,47,0 { TAU, IS_ASCII_DIGIT_CONST, ARRAY_IMPL_ANY_LEN }
     1,46,0 { CONST_IF_MATCH }
     1,45,0 { STR_STRIP_PREFIX }
     1,43,0 { LOG2_10, LOG10_2 }