Auto merge of #8744 - Jarcho:needless_collect_fp, r=llogiq

Extend `needless_collect`

Extends when `is_empty` and `contains` are linted.

`is_empty` will be linted when `<IterTy as Iterator>::Item` is the same as `<CollectTy as IntoIterator>::Item`. This can be a false positive if the `FromIterator` implementation filters out items, but I don't know of any which do that also implement `IntoIterator` with a matching `Item` type.

`contains` will be linted when the argument type is `&<IterTy as Iterator>::Item`. It has the same false positives as `is_empty` with the same note that I know of nothing that actually causes that in practice.

changelog: Lint `needless_collect` when `is_empty` or `contains` is called on some non-std types

11 files changed, 433 insertions, 176 deletions

diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs
index 747636b7ec3..e9094ca3a09 100644
--- a/clippy_lints/src/declared_lints.rs
+++ b/clippy_lints/src/declared_lints.rs
@@ -237,7 +237,6 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
     crate::loops::MANUAL_MEMCPY_INFO,
     crate::loops::MISSING_SPIN_LOOP_INFO,
     crate::loops::MUT_RANGE_BOUND_INFO,
-    crate::loops::NEEDLESS_COLLECT_INFO,
     crate::loops::NEEDLESS_RANGE_LOOP_INFO,
     crate::loops::NEVER_LOOP_INFO,
     crate::loops::SAME_ITEM_PUSH_INFO,
@@ -346,6 +345,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
     crate::methods::MAP_UNWRAP_OR_INFO,
     crate::methods::MUT_MUTEX_LOCK_INFO,
     crate::methods::NAIVE_BYTECOUNT_INFO,
+    crate::methods::NEEDLESS_COLLECT_INFO,
     crate::methods::NEEDLESS_OPTION_AS_DEREF_INFO,
     crate::methods::NEEDLESS_OPTION_TAKE_INFO,
     crate::methods::NEEDLESS_SPLITN_INFO,
diff --git a/clippy_lints/src/loops/mod.rs b/clippy_lints/src/loops/mod.rs
index 821fe173023..8e52cac4323 100644
--- a/clippy_lints/src/loops/mod.rs
+++ b/clippy_lints/src/loops/mod.rs
@@ -9,7 +9,6 @@ mod manual_flatten;
 mod manual_memcpy;
 mod missing_spin_loop;
 mod mut_range_bound;
-mod needless_collect;
 mod needless_range_loop;
 mod never_loop;
 mod same_item_push;
@@ -207,28 +206,6 @@ declare_clippy_lint! {
 
 declare_clippy_lint! {
     /// ### What it does
-    /// Checks for functions collecting an iterator when collect
-    /// is not needed.
-    ///
-    /// ### Why is this bad?
-    /// `collect` causes the allocation of a new data structure,
-    /// when this allocation may not be needed.
-    ///
-    /// ### Example
-    /// ```rust
-    /// # let iterator = vec![1].into_iter();
-    /// let len = iterator.clone().collect::<Vec<_>>().len();
-    /// // should be
-    /// let len = iterator.count();
-    /// ```
-    #[clippy::version = "1.30.0"]
-    pub NEEDLESS_COLLECT,
-    nursery,
-    "collecting an iterator when collect is not needed"
-}
-
-declare_clippy_lint! {
-    /// ### What it does
     /// Checks `for` loops over slices with an explicit counter
     /// and suggests the use of `.enumerate()`.
     ///
@@ -605,7 +582,6 @@ declare_lint_pass!(Loops => [
     EXPLICIT_INTO_ITER_LOOP,
     ITER_NEXT_LOOP,
     WHILE_LET_LOOP,
-    NEEDLESS_COLLECT,
     EXPLICIT_COUNTER_LOOP,
     EMPTY_LOOP,
     WHILE_LET_ON_ITERATOR,
@@ -667,8 +643,6 @@ impl<'tcx> LateLintPass<'tcx> for Loops {
             while_immutable_condition::check(cx, condition, body);
             missing_spin_loop::check(cx, condition, body);
         }
-
-        needless_collect::check(expr, cx);
     }
 }
 
diff --git a/clippy_lints/src/methods/collapsible_str_replace.rs b/clippy_lints/src/methods/collapsible_str_replace.rs
index 501646863fe..ac61b437788 100644
--- a/clippy_lints/src/methods/collapsible_str_replace.rs
+++ b/clippy_lints/src/methods/collapsible_str_replace.rs
@@ -23,7 +23,7 @@ pub(super) fn check<'tcx>(
         // If the parent node's `to` argument is the same as the `to` argument
         // of the last replace call in the current chain, don't lint as it was already linted
         if let Some(parent) = get_parent_expr(cx, expr)
-            && let Some(("replace", _, [current_from, current_to], _)) = method_call(parent)
+            && let Some(("replace", _, [current_from, current_to], _, _)) = method_call(parent)
             && eq_expr_value(cx, to, current_to)
             && from_kind == cx.typeck_results().expr_ty(current_from).peel_refs().kind()
         {
@@ -48,7 +48,7 @@ fn collect_replace_calls<'tcx>(
     let mut from_args = VecDeque::new();
 
     let _: Option<()> = for_each_expr(expr, |e| {
-        if let Some(("replace", _, [from, to], _)) = method_call(e) {
+        if let Some(("replace", _, [from, to], _, _)) = method_call(e) {
             if eq_expr_value(cx, to_arg, to) && cx.typeck_results().expr_ty(from).peel_refs().is_char() {
                 methods.push_front(e);
                 from_args.push_front(from);
@@ -78,7 +78,7 @@ fn check_consecutive_replace_calls<'tcx>(
         .collect();
     let app = Applicability::MachineApplicable;
     let earliest_replace_call = replace_methods.methods.front().unwrap();
-    if let Some((_, _, [..], span_lo)) = method_call(earliest_replace_call) {
+    if let Some((_, _, [..], span_lo, _)) = method_call(earliest_replace_call) {
         span_lint_and_sugg(
             cx,
             COLLAPSIBLE_STR_REPLACE,
diff --git a/clippy_lints/src/methods/manual_str_repeat.rs b/clippy_lints/src/methods/manual_str_repeat.rs
index 8b6b8f1bf16..8b798fdb12f 100644
--- a/clippy_lints/src/methods/manual_str_repeat.rs
+++ b/clippy_lints/src/methods/manual_str_repeat.rs
@@ -59,10 +59,8 @@ pub(super) fn check(
         if let ExprKind::Call(repeat_fn, [repeat_arg]) = take_self_arg.kind;
         if is_path_diagnostic_item(cx, repeat_fn, sym::iter_repeat);
         if is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(collect_expr), sym::String);
-        if let Some(collect_id) = cx.typeck_results().type_dependent_def_id(collect_expr.hir_id);
         if let Some(take_id) = cx.typeck_results().type_dependent_def_id(take_expr.hir_id);
         if let Some(iter_trait_id) = cx.tcx.get_diagnostic_item(sym::Iterator);
-        if cx.tcx.trait_of_item(collect_id) == Some(iter_trait_id);
         if cx.tcx.trait_of_item(take_id) == Some(iter_trait_id);
         if let Some(repeat_kind) = parse_repeat_arg(cx, repeat_arg);
         let ctxt = collect_expr.span.ctxt();
diff --git a/clippy_lints/src/methods/map_collect_result_unit.rs b/clippy_lints/src/methods/map_collect_result_unit.rs
index d420f144eea..a0300d27870 100644
--- a/clippy_lints/src/methods/map_collect_result_unit.rs
+++ b/clippy_lints/src/methods/map_collect_result_unit.rs
@@ -1,5 +1,4 @@
 use clippy_utils::diagnostics::span_lint_and_sugg;
-use clippy_utils::is_trait_method;
 use clippy_utils::source::snippet;
 use clippy_utils::ty::is_type_diagnostic_item;
 use if_chain::if_chain;
@@ -11,18 +10,10 @@ use rustc_span::symbol::sym;
 
 use super::MAP_COLLECT_RESULT_UNIT;
 
-pub(super) fn check(
-    cx: &LateContext<'_>,
-    expr: &hir::Expr<'_>,
-    iter: &hir::Expr<'_>,
-    map_fn: &hir::Expr<'_>,
-    collect_recv: &hir::Expr<'_>,
-) {
+pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, iter: &hir::Expr<'_>, map_fn: &hir::Expr<'_>) {
+    // return of collect `Result<(),_>`
+    let collect_ret_ty = cx.typeck_results().expr_ty(expr);
     if_chain! {
-        // called on Iterator
-        if is_trait_method(cx, collect_recv, sym::Iterator);
-        // return of collect `Result<(),_>`
-        let collect_ret_ty = cx.typeck_results().expr_ty(expr);
         if is_type_diagnostic_item(cx, collect_ret_ty, sym::Result);
         if let ty::Adt(_, substs) = collect_ret_ty.kind();
         if let Some(result_t) = substs.types().next();
diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs
index 26834dc4fcc..1c0bbe086f7 100644
--- a/clippy_lints/src/methods/mod.rs
+++ b/clippy_lints/src/methods/mod.rs
@@ -54,6 +54,7 @@ mod map_flatten;
 mod map_identity;
 mod map_unwrap_or;
 mod mut_mutex_lock;
+mod needless_collect;
 mod needless_option_as_deref;
 mod needless_option_take;
 mod no_effect_replace;
@@ -3141,6 +3142,28 @@ declare_clippy_lint! {
     "jumping to the start of stream using `seek` method"
 }
 
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for functions collecting an iterator when collect
+    /// is not needed.
+    ///
+    /// ### Why is this bad?
+    /// `collect` causes the allocation of a new data structure,
+    /// when this allocation may not be needed.
+    ///
+    /// ### Example
+    /// ```rust
+    /// # let iterator = vec![1].into_iter();
+    /// let len = iterator.clone().collect::<Vec<_>>().len();
+    /// // should be
+    /// let len = iterator.count();
+    /// ```
+    #[clippy::version = "1.30.0"]
+    pub NEEDLESS_COLLECT,
+    nursery,
+    "collecting an iterator when collect is not needed"
+}
+
 pub struct Methods {
     avoid_breaking_exported_api: bool,
     msrv: Option<RustcVersion>,
@@ -3267,16 +3290,17 @@ impl_lint_pass!(Methods => [
     ITER_KV_MAP,
     SEEK_FROM_CURRENT,
     SEEK_TO_START_INSTEAD_OF_REWIND,
+    NEEDLESS_COLLECT,
 ]);
 
 /// Extracts a method call name, args, and `Span` of the method name.
 fn method_call<'tcx>(
     recv: &'tcx hir::Expr<'tcx>,
-) -> Option<(&'tcx str, &'tcx hir::Expr<'tcx>, &'tcx [hir::Expr<'tcx>], Span)> {
-    if let ExprKind::MethodCall(path, receiver, args, _) = recv.kind {
+) -> Option<(&'tcx str, &'tcx hir::Expr<'tcx>, &'tcx [hir::Expr<'tcx>], Span, Span)> {
+    if let ExprKind::MethodCall(path, receiver, args, call_span) = recv.kind {
         if !args.iter().any(|e| e.span.from_expansion()) && !receiver.span.from_expansion() {
             let name = path.ident.name.as_str();
-            return Some((name, receiver, args, path.ident.span));
+            return Some((name, receiver, args, path.ident.span, call_span));
         }
     }
     None
@@ -3462,7 +3486,7 @@ impl<'tcx> LateLintPass<'tcx> for Methods {
 impl Methods {
     #[allow(clippy::too_many_lines)]
     fn check_methods<'tcx>(&self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
-        if let Some((name, recv, args, span)) = method_call(expr) {
+        if let Some((name, recv, args, span, call_span)) = method_call(expr) {
             match (name, args) {
                 ("add" | "offset" | "sub" | "wrapping_offset" | "wrapping_add" | "wrapping_sub", [_arg]) => {
                     zst_offset::check(cx, expr, recv);
@@ -3481,28 +3505,31 @@ impl Methods {
                 ("as_ref", []) => useless_asref::check(cx, expr, "as_ref", recv),
                 ("assume_init", []) => uninit_assumed_init::check(cx, expr, recv),
                 ("cloned", []) => cloned_instead_of_copied::check(cx, expr, recv, span, self.msrv),
-                ("collect", []) => match method_call(recv) {
-                    Some((name @ ("cloned" | "copied"), recv2, [], _)) => {
-                        iter_cloned_collect::check(cx, name, expr, recv2);
-                    },
-                    Some(("map", m_recv, [m_arg], _)) => {
-                        map_collect_result_unit::check(cx, expr, m_recv, m_arg, recv);
-                    },
-                    Some(("take", take_self_arg, [take_arg], _)) => {
-                        if meets_msrv(self.msrv, msrvs::STR_REPEAT) {
-                            manual_str_repeat::check(cx, expr, recv, take_self_arg, take_arg);
-                        }
-                    },
-                    _ => {},
+                ("collect", []) if is_trait_method(cx, expr, sym::Iterator) => {
+                    needless_collect::check(cx, span, expr, recv, call_span);
+                    match method_call(recv) {
+                        Some((name @ ("cloned" | "copied"), recv2, [], _, _)) => {
+                            iter_cloned_collect::check(cx, name, expr, recv2);
+                        },
+                        Some(("map", m_recv, [m_arg], _, _)) => {
+                            map_collect_result_unit::check(cx, expr, m_recv, m_arg);
+                        },
+                        Some(("take", take_self_arg, [take_arg], _, _)) => {
+                            if meets_msrv(self.msrv, msrvs::STR_REPEAT) {
+                                manual_str_repeat::check(cx, expr, recv, take_self_arg, take_arg);
+                            }
+                        },
+                        _ => {},
+                    }
                 },
                 ("count", []) if is_trait_method(cx, expr, sym::Iterator) => match method_call(recv) {
-                    Some(("cloned", recv2, [], _)) => iter_overeager_cloned::check(cx, expr, recv, recv2, true, false),
-                    Some((name2 @ ("into_iter" | "iter" | "iter_mut"), recv2, [], _)) => {
+                    Some(("cloned", recv2, [], _, _)) => iter_overeager_cloned::check(cx, expr, recv, recv2, true, false),
+                    Some((name2 @ ("into_iter" | "iter" | "iter_mut"), recv2, [], _, _)) => {
                         iter_count::check(cx, expr, recv2, name2);
                     },
-                    Some(("map", _, [arg], _)) => suspicious_map::check(cx, expr, recv, arg),
-                    Some(("filter", recv2, [arg], _)) => bytecount::check(cx, expr, recv2, arg),
-                    Some(("bytes", recv2, [], _)) => bytes_count_to_len::check(cx, expr, recv, recv2),
+                    Some(("map", _, [arg], _, _)) => suspicious_map::check(cx, expr, recv, arg),
+                    Some(("filter", recv2, [arg], _, _)) => bytecount::check(cx, expr, recv2, arg),
+                    Some(("bytes", recv2, [], _, _)) => bytes_count_to_len::check(cx, expr, recv, recv2),
                     _ => {},
                 },
                 ("drain", [arg]) => {
@@ -3514,8 +3541,8 @@ impl Methods {
                     }
                 },
                 ("expect", [_]) => match method_call(recv) {
-                    Some(("ok", recv, [], _)) => ok_expect::check(cx, expr, recv),
-                    Some(("err", recv, [], err_span)) => err_expect::check(cx, expr, recv, self.msrv, span, err_span),
+                    Some(("ok", recv, [], _, _)) => ok_expect::check(cx, expr, recv),
+                    Some(("err", recv, [], err_span, _)) => err_expect::check(cx, expr, recv, self.msrv, span, err_span),
                     _ => expect_used::check(cx, expr, recv, false, self.allow_expect_in_tests),
                 },
                 ("expect_err", [_]) => expect_used::check(cx, expr, recv, true, self.allow_expect_in_tests),
@@ -3535,13 +3562,13 @@ impl Methods {
                     flat_map_option::check(cx, expr, arg, span);
                 },
                 ("flatten", []) => match method_call(recv) {
-                    Some(("map", recv, [map_arg], map_span)) => map_flatten::check(cx, expr, recv, map_arg, map_span),
-                    Some(("cloned", recv2, [], _)) => iter_overeager_cloned::check(cx, expr, recv, recv2, false, true),
+                    Some(("map", recv, [map_arg], map_span, _)) => map_flatten::check(cx, expr, recv, map_arg, map_span),
+                    Some(("cloned", recv2, [], _, _)) => iter_overeager_cloned::check(cx, expr, recv, recv2, false, true),
                     _ => {},
                 },
                 ("fold", [init, acc]) => unnecessary_fold::check(cx, expr, init, acc, span),
                 ("for_each", [_]) => {
-                    if let Some(("inspect", _, [_], span2)) = method_call(recv) {
+                    if let Some(("inspect", _, [_], span2, _)) = method_call(recv) {
                         inspect_for_each::check(cx, expr, span2);
                     }
                 },
@@ -3561,12 +3588,12 @@ impl Methods {
                     iter_on_single_or_empty_collections::check(cx, expr, name, recv);
                 },
                 ("join", [join_arg]) => {
-                    if let Some(("collect", _, _, span)) = method_call(recv) {
+                    if let Some(("collect", _, _, span, _)) = method_call(recv) {
                         unnecessary_join::check(cx, expr, recv, join_arg, span);
                     }
                 },
                 ("last", []) | ("skip", [_]) => {
-                    if let Some((name2, recv2, args2, _span2)) = method_call(recv) {
+                    if let Some((name2, recv2, args2, _span2, _)) = method_call(recv) {
                         if let ("cloned", []) = (name2, args2) {
                             iter_overeager_cloned::check(cx, expr, recv, recv2, false, false);
                         }
@@ -3578,13 +3605,13 @@ impl Methods {
                 (name @ ("map" | "map_err"), [m_arg]) => {
                     if name == "map" {
                         map_clone::check(cx, expr, recv, m_arg, self.msrv);
-                        if let Some((map_name @ ("iter" | "into_iter"), recv2, _, _)) = method_call(recv) {
+                        if let Some((map_name @ ("iter" | "into_iter"), recv2, _, _, _)) = method_call(recv) {
                             iter_kv_map::check(cx, map_name, expr, recv2, m_arg);
                         }
                     } else {
                         map_err_ignore::check(cx, expr, m_arg);
                     }
-                    if let Some((name, recv2, args, span2)) = method_call(recv) {
+                    if let Some((name, recv2, args, span2,_)) = method_call(recv) {
                         match (name, args) {
                             ("as_mut", []) => option_as_ref_deref::check(cx, expr, recv2, m_arg, true, self.msrv),
                             ("as_ref", []) => option_as_ref_deref::check(cx, expr, recv2, m_arg, false, self.msrv),
@@ -3604,7 +3631,7 @@ impl Methods {
                     manual_ok_or::check(cx, expr, recv, def, map);
                 },
                 ("next", []) => {
-                    if let Some((name2, recv2, args2, _)) = method_call(recv) {
+                    if let Some((name2, recv2, args2, _, _)) = method_call(recv) {
                         match (name2, args2) {
                             ("cloned", []) => iter_overeager_cloned::check(cx, expr, recv, recv2, false, false),
                             ("filter", [arg]) => filter_next::check(cx, expr, recv2, arg),
@@ -3617,10 +3644,10 @@ impl Methods {
                     }
                 },
                 ("nth", [n_arg]) => match method_call(recv) {
-                    Some(("bytes", recv2, [], _)) => bytes_nth::check(cx, expr, recv2, n_arg),
-                    Some(("cloned", recv2, [], _)) => iter_overeager_cloned::check(cx, expr, recv, recv2, false, false),
-                    Some(("iter", recv2, [], _)) => iter_nth::check(cx, expr, recv2, recv, n_arg, false),
-                    Some(("iter_mut", recv2, [], _)) => iter_nth::check(cx, expr, recv2, recv, n_arg, true),
+                    Some(("bytes", recv2, [], _, _)) => bytes_nth::check(cx, expr, recv2, n_arg),
+                    Some(("cloned", recv2, [], _, _)) => iter_overeager_cloned::check(cx, expr, recv, recv2, false, false),
+                    Some(("iter", recv2, [], _, _)) => iter_nth::check(cx, expr, recv2, recv, n_arg, false),
+                    Some(("iter_mut", recv2, [], _, _)) => iter_nth::check(cx, expr, recv2, recv, n_arg, true),
                     _ => iter_nth_zero::check(cx, expr, recv, n_arg),
                 },
                 ("ok_or_else", [arg]) => unnecessary_lazy_eval::check(cx, expr, recv, arg, "ok_or"),
@@ -3685,7 +3712,7 @@ impl Methods {
                 },
                 ("step_by", [arg]) => iterator_step_by_zero::check(cx, expr, arg),
                 ("take", [_arg]) => {
-                    if let Some((name2, recv2, args2, _span2)) = method_call(recv) {
+                    if let Some((name2, recv2, args2, _span2, _)) = method_call(recv) {
                         if let ("cloned", []) = (name2, args2) {
                             iter_overeager_cloned::check(cx, expr, recv, recv2, false, false);
                         }
@@ -3708,13 +3735,13 @@ impl Methods {
                 },
                 ("unwrap", []) => {
                     match method_call(recv) {
-                        Some(("get", recv, [get_arg], _)) => {
+                        Some(("get", recv, [get_arg], _, _)) => {
                             get_unwrap::check(cx, expr, recv, get_arg, false);
                         },
-                        Some(("get_mut", recv, [get_arg], _)) => {
+                        Some(("get_mut", recv, [get_arg], _, _)) => {
                             get_unwrap::check(cx, expr, recv, get_arg, true);
                         },
-                        Some(("or", recv, [or_arg], or_span)) => {
+                        Some(("or", recv, [or_arg], or_span, _)) => {
                             or_then_unwrap::check(cx, expr, recv, or_arg, or_span);
                         },
                         _ => {},
@@ -3723,19 +3750,19 @@ impl Methods {
                 },
                 ("unwrap_err", []) => unwrap_used::check(cx, expr, recv, true, self.allow_unwrap_in_tests),
                 ("unwrap_or", [u_arg]) => match method_call(recv) {
-                    Some((arith @ ("checked_add" | "checked_sub" | "checked_mul"), lhs, [rhs], _)) => {
+                    Some((arith @ ("checked_add" | "checked_sub" | "checked_mul"), lhs, [rhs], _, _)) => {
                         manual_saturating_arithmetic::check(cx, expr, lhs, rhs, u_arg, &arith["checked_".len()..]);
                     },
-                    Some(("map", m_recv, [m_arg], span)) => {
+                    Some(("map", m_recv, [m_arg], span, _)) => {
                         option_map_unwrap_or::check(cx, expr, m_recv, m_arg, recv, u_arg, span);
                     },
-                    Some(("then_some", t_recv, [t_arg], _)) => {
+                    Some(("then_some", t_recv, [t_arg], _, _)) => {
                         obfuscated_if_else::check(cx, expr, t_recv, t_arg, u_arg);
                     },
                     _ => {},
                 },
                 ("unwrap_or_else", [u_arg]) => match method_call(recv) {
-                    Some(("map", recv, [map_arg], _))
+                    Some(("map", recv, [map_arg], _, _))
                         if map_unwrap_or::check(cx, expr, recv, map_arg, u_arg, self.msrv) => {},
                     _ => {
                         unwrap_or_else_default::check(cx, expr, recv, u_arg);
@@ -3756,7 +3783,7 @@ impl Methods {
 }
 
 fn check_is_some_is_none(cx: &LateContext<'_>, expr: &Expr<'_>, recv: &Expr<'_>, is_some: bool) {
-    if let Some((name @ ("find" | "position" | "rposition"), f_recv, [arg], span)) = method_call(recv) {
+    if let Some((name @ ("find" | "position" | "rposition"), f_recv, [arg], span, _)) = method_call(recv) {
         search_is_some::check(cx, expr, name, is_some, f_recv, arg, recv, span);
     }
 }
diff --git a/clippy_lints/src/loops/needless_collect.rs b/clippy_lints/src/methods/needless_collect.rs
index 66f9e28596e..b088e642e0e 100644
--- a/clippy_lints/src/loops/needless_collect.rs
+++ b/clippy_lints/src/methods/needless_collect.rs
@@ -3,94 +3,99 @@ use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_hir_and_then};
 use clippy_utils::higher;
 use clippy_utils::source::{snippet, snippet_with_applicability};
 use clippy_utils::sugg::Sugg;
-use clippy_utils::ty::is_type_diagnostic_item;
-use clippy_utils::{can_move_expr_to_closure, is_trait_method, path_to_local, path_to_local_id, CaptureKind};
-use if_chain::if_chain;
+use clippy_utils::ty::{is_type_diagnostic_item, make_normalized_projection, make_projection};
+use clippy_utils::{
+    can_move_expr_to_closure, get_enclosing_block, get_parent_node, is_trait_method, path_to_local, path_to_local_id,
+    CaptureKind,
+};
 use rustc_data_structures::fx::FxHashMap;
 use rustc_errors::{Applicability, MultiSpan};
 use rustc_hir::intravisit::{walk_block, walk_expr, Visitor};
-use rustc_hir::{Block, Expr, ExprKind, HirId, HirIdSet, Local, Mutability, Node, PatKind, Stmt, StmtKind};
+use rustc_hir::{
+    BindingAnnotation, Block, Expr, ExprKind, HirId, HirIdSet, Local, Mutability, Node, PatKind, Stmt, StmtKind,
+};
 use rustc_lint::LateContext;
 use rustc_middle::hir::nested_filter;
-use rustc_middle::ty::subst::GenericArgKind;
-use rustc_middle::ty::{self, Ty};
-use rustc_span::sym;
-use rustc_span::Span;
+use rustc_middle::ty::{self, AssocKind, EarlyBinder, GenericArg, GenericArgKind, Ty};
+use rustc_span::symbol::Ident;
+use rustc_span::{sym, Span, Symbol};
 
 const NEEDLESS_COLLECT_MSG: &str = "avoid using `collect()` when not needed";
 
-pub(super) fn check<'tcx>(expr: &'tcx Expr<'_>, cx: &LateContext<'tcx>) {
-    check_needless_collect_direct_usage(expr, cx);
-    check_needless_collect_indirect_usage(expr, cx);
-}
-fn check_needless_collect_direct_usage<'tcx>(expr: &'tcx Expr<'_>, cx: &LateContext<'tcx>) {
-    if_chain! {
-        if let ExprKind::MethodCall(method, receiver, args, _) = expr.kind;
-        if let ExprKind::MethodCall(chain_method, ..) = receiver.kind;
-        if chain_method.ident.name == sym!(collect) && is_trait_method(cx, receiver, sym::Iterator);
-        then {
-            let ty = cx.typeck_results().expr_ty(receiver);
-            let mut applicability = Applicability::MaybeIncorrect;
-            let is_empty_sugg = "next().is_none()".to_string();
-            let method_name = method.ident.name.as_str();
-            let sugg = if is_type_diagnostic_item(cx, ty, sym::Vec) ||
-                        is_type_diagnostic_item(cx, ty, sym::VecDeque) ||
-                        is_type_diagnostic_item(cx, ty, sym::LinkedList) ||
-                        is_type_diagnostic_item(cx, ty, sym::BinaryHeap) {
-                match method_name {
-                    "len" => "count()".to_string(),
-                    "is_empty" => is_empty_sugg,
-                    "contains" => {
-                        let contains_arg = snippet_with_applicability(cx, args[0].span, "??", &mut applicability);
-                        let (arg, pred) = contains_arg
-                            .strip_prefix('&')
-                            .map_or(("&x", &*contains_arg), |s| ("x", s));
-                        format!("any(|{arg}| x == {pred})")
-                    }
-                    _ => return,
-                }
-            }
-            else if is_type_diagnostic_item(cx, ty, sym::BTreeMap) ||
-                is_type_diagnostic_item(cx, ty, sym::HashMap) {
-                match method_name {
-                    "is_empty" => is_empty_sugg,
-                    _ => return,
-                }
-            }
-            else {
-                return;
-            };
-            span_lint_and_sugg(
-                cx,
-                NEEDLESS_COLLECT,
-                chain_method.ident.span.with_hi(expr.span.hi()),
-                NEEDLESS_COLLECT_MSG,
-                "replace with",
-                sugg,
-                applicability,
-            );
-        }
-    }
-}
+pub(super) fn check<'tcx>(
+    cx: &LateContext<'tcx>,
+    name_span: Span,
+    collect_expr: &'tcx Expr<'_>,
+    iter_expr: &'tcx Expr<'tcx>,
+    call_span: Span,
+) {
+    if let Some(parent) = get_parent_node(cx.tcx, collect_expr.hir_id) {
+        match parent {
+            Node::Expr(parent) => {
+                if let ExprKind::MethodCall(name, _, args @ ([] | [_]), _) = parent.kind {
+                    let mut app = Applicability::MachineApplicable;
+                    let name = name.ident.as_str();
+                    let collect_ty = cx.typeck_results().expr_ty(collect_expr);
 
-fn check_needless_collect_indirect_usage<'tcx>(expr: &'tcx Expr<'_>, cx: &LateContext<'tcx>) {
-    if let ExprKind::Block(block, _) = expr.kind {
-        for stmt in block.stmts {
-            if_chain! {
-                if let StmtKind::Local(local) = stmt.kind;
-                if let PatKind::Binding(_, id, ..) = local.pat.kind;
-                if let Some(init_expr) = local.init;
-                if let ExprKind::MethodCall(method_name, iter_source, [], ..) = init_expr.kind;
-                if method_name.ident.name == sym!(collect) && is_trait_method(cx, init_expr, sym::Iterator);
-                let ty = cx.typeck_results().expr_ty(init_expr);
-                if is_type_diagnostic_item(cx, ty, sym::Vec) ||
-                    is_type_diagnostic_item(cx, ty, sym::VecDeque) ||
-                    is_type_diagnostic_item(cx, ty, sym::BinaryHeap) ||
-                    is_type_diagnostic_item(cx, ty, sym::LinkedList);
-                let iter_ty = cx.typeck_results().expr_ty(iter_source);
-                if let Some(iter_calls) = detect_iter_and_into_iters(block, id, cx, get_captured_ids(cx, iter_ty));
-                if let [iter_call] = &*iter_calls;
-                then {
+                    let sugg: String = match name {
+                        "len" => {
+                            if let Some(adt) = collect_ty.ty_adt_def()
+                                && matches!(
+                                    cx.tcx.get_diagnostic_name(adt.did()),
+                                    Some(sym::Vec | sym::VecDeque | sym::LinkedList | sym::BinaryHeap)
+                                )
+                            {
+                                "count()".into()
+                            } else {
+                                return;
+                            }
+                        },
+                        "is_empty"
+                            if is_is_empty_sig(cx, parent.hir_id)
+                                && iterates_same_ty(cx, cx.typeck_results().expr_ty(iter_expr), collect_ty) =>
+                        {
+                            "next().is_none()".into()
+                        },
+                        "contains" => {
+                            if is_contains_sig(cx, parent.hir_id, iter_expr)
+                                && let Some(arg) = args.first()
+                            {
+                                let (span, prefix) = if let ExprKind::AddrOf(_, _, arg) = arg.kind {
+                                    (arg.span, "")
+                                } else {
+                                    (arg.span, "*")
+                                };
+                                let snip = snippet_with_applicability(cx, span, "??", &mut app);
+                                format!("any(|x| x == {prefix}{snip})")
+                            } else {
+                                return;
+                            }
+                        },
+                        _ => return,
+                    };
+
+                    span_lint_and_sugg(
+                        cx,
+                        NEEDLESS_COLLECT,
+                        call_span.with_hi(parent.span.hi()),
+                        NEEDLESS_COLLECT_MSG,
+                        "replace with",
+                        sugg,
+                        app,
+                    );
+                }
+            },
+            Node::Local(l) => {
+                if let PatKind::Binding(BindingAnnotation::NONE | BindingAnnotation::MUT, id, _, None)
+                    = l.pat.kind
+                    && let ty = cx.typeck_results().expr_ty(collect_expr)
+                    && [sym::Vec, sym::VecDeque, sym::BinaryHeap, sym::LinkedList].into_iter()
+                        .any(|item| is_type_diagnostic_item(cx, ty, item))
+                    && let iter_ty = cx.typeck_results().expr_ty(iter_expr)
+                    && let Some(block) = get_enclosing_block(cx, l.hir_id)
+                    && let Some(iter_calls) = detect_iter_and_into_iters(block, id, cx, get_captured_ids(cx, iter_ty))
+                    && let [iter_call] = &*iter_calls
+                {
                     let mut used_count_visitor = UsedCountVisitor {
                         cx,
                         id,
@@ -102,20 +107,20 @@ fn check_needless_collect_indirect_usage<'tcx>(expr: &'tcx Expr<'_>, cx: &LateCo
                     }
 
                     // Suggest replacing iter_call with iter_replacement, and removing stmt
-                    let mut span = MultiSpan::from_span(method_name.ident.span);
+                    let mut span = MultiSpan::from_span(name_span);
                     span.push_span_label(iter_call.span, "the iterator could be used here instead");
                     span_lint_hir_and_then(
                         cx,
                         super::NEEDLESS_COLLECT,
-                        init_expr.hir_id,
+                        collect_expr.hir_id,
                         span,
                         NEEDLESS_COLLECT_MSG,
                         |diag| {
-                            let iter_replacement = format!("{}{}", Sugg::hir(cx, iter_source, ".."), iter_call.get_iter_method(cx));
+                            let iter_replacement = format!("{}{}", Sugg::hir(cx, iter_expr, ".."), iter_call.get_iter_method(cx));
                             diag.multipart_suggestion(
                                 iter_call.get_suggestion_text(),
                                 vec![
-                                    (stmt.span, String::new()),
+                                    (l.span, String::new()),
                                     (iter_call.span, iter_replacement)
                                 ],
                                 Applicability::MaybeIncorrect,
@@ -123,11 +128,61 @@ fn check_needless_collect_indirect_usage<'tcx>(expr: &'tcx Expr<'_>, cx: &LateCo
                         },
                     );
                 }
-            }
+            },
+            _ => (),
         }
     }
 }
 
+/// Checks if the given method call matches the expected signature of `([&[mut]] self) -> bool`
+fn is_is_empty_sig(cx: &LateContext<'_>, call_id: HirId) -> bool {
+    cx.typeck_results().type_dependent_def_id(call_id).map_or(false, |id| {
+        let sig = cx.tcx.fn_sig(id).skip_binder();
+        sig.inputs().len() == 1 && sig.output().is_bool()
+    })
+}
+
+/// Checks if `<iter_ty as Iterator>::Item` is the same as `<collect_ty as IntoIter>::Item`
+fn iterates_same_ty<'tcx>(cx: &LateContext<'tcx>, iter_ty: Ty<'tcx>, collect_ty: Ty<'tcx>) -> bool {
+    let item = Symbol::intern("Item");
+    if let Some(iter_trait) = cx.tcx.get_diagnostic_item(sym::Iterator)
+        && let Some(into_iter_trait) = cx.tcx.get_diagnostic_item(sym::IntoIterator)
+        && let Some(iter_item_ty) = make_normalized_projection(cx.tcx, cx.param_env, iter_trait, item, [iter_ty])
+        && let Some(into_iter_item_proj) = make_projection(cx.tcx, into_iter_trait, item, [collect_ty])
+        && let Ok(into_iter_item_ty) = cx.tcx.try_normalize_erasing_regions(
+            cx.param_env,
+            cx.tcx.mk_projection(into_iter_item_proj.item_def_id, into_iter_item_proj.substs)
+        )
+    {
+        iter_item_ty == into_iter_item_ty
+    } else {
+        false
+    }
+}
+
+/// Checks if the given method call matches the expected signature of
+/// `([&[mut]] self, &<iter_ty as Iterator>::Item) -> bool`
+fn is_contains_sig(cx: &LateContext<'_>, call_id: HirId, iter_expr: &Expr<'_>) -> bool {
+    let typeck = cx.typeck_results();
+    if let Some(id) = typeck.type_dependent_def_id(call_id)
+        && let sig = cx.tcx.fn_sig(id)
+        && sig.skip_binder().output().is_bool()
+        && let [_, search_ty] = *sig.skip_binder().inputs()
+        && let ty::Ref(_, search_ty, Mutability::Not) = *cx.tcx.erase_late_bound_regions(sig.rebind(search_ty)).kind()
+        && let Some(iter_trait) = cx.tcx.get_diagnostic_item(sym::Iterator)
+        && let Some(iter_item) = cx.tcx
+            .associated_items(iter_trait)
+            .find_by_name_and_kind(cx.tcx, Ident::with_dummy_span(Symbol::intern("Item")), AssocKind::Type, iter_trait)
+        && let substs = cx.tcx.mk_substs([GenericArg::from(typeck.expr_ty_adjusted(iter_expr))].into_iter())
+        && let proj_ty = cx.tcx.mk_projection(iter_item.def_id, substs)
+        && let Ok(item_ty) = cx.tcx.try_normalize_erasing_regions(cx.param_env, proj_ty)
+    {
+        item_ty == EarlyBinder(search_ty).subst(cx.tcx, cx.typeck_results().node_substs(call_id))
+    } else {
+        false
+    }
+}
+
 struct IterFunction {
     func: IterFunctionKind,
     span: Span,
diff --git a/clippy_utils/src/ty.rs b/clippy_utils/src/ty.rs
index 2dd8c826ab3..a022aac4bfe 100644
--- a/clippy_utils/src/ty.rs
+++ b/clippy_utils/src/ty.rs
@@ -13,8 +13,9 @@ use rustc_infer::infer::TyCtxtInferExt;
 use rustc_lint::LateContext;
 use rustc_middle::mir::interpret::{ConstValue, Scalar};
 use rustc_middle::ty::{
-    self, AdtDef, Binder, BoundRegion, DefIdTree, FnSig, IntTy, ParamEnv, Predicate, PredicateKind, ProjectionTy,
-    Region, RegionKind, Ty, TyCtxt, TypeSuperVisitable, TypeVisitable, TypeVisitor, UintTy, VariantDef, VariantDiscr,
+    self, AdtDef, AssocKind, Binder, BoundRegion, DefIdTree, FnSig, GenericParamDefKind, IntTy, ParamEnv, Predicate,
+    PredicateKind, ProjectionTy, Region, RegionKind, SubstsRef, Ty, TyCtxt, TypeSuperVisitable, TypeVisitable,
+    TypeVisitor, UintTy, VariantDef, VariantDiscr,
 };
 use rustc_middle::ty::{GenericArg, GenericArgKind};
 use rustc_span::symbol::Ident;
@@ -938,3 +939,132 @@ pub fn approx_ty_size<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> u64 {
         (Err(_), _) => 0,
     }
 }
+
+/// Makes the projection type for the named associated type in the given impl or trait impl.
+///
+/// This function is for associated types which are "known" to exist, and as such, will only return
+/// `None` when debug assertions are disabled in order to prevent ICE's. With debug assertions
+/// enabled this will check that the named associated type exists, the correct number of
+/// substitutions are given, and that the correct kinds of substitutions are given (lifetime,
+/// constant or type). This will not check if type normalization would succeed.
+pub fn make_projection<'tcx>(
+    tcx: TyCtxt<'tcx>,
+    container_id: DefId,
+    assoc_ty: Symbol,
+    substs: impl IntoIterator<Item = impl Into<GenericArg<'tcx>>>,
+) -> Option<ProjectionTy<'tcx>> {
+    fn helper<'tcx>(
+        tcx: TyCtxt<'tcx>,
+        container_id: DefId,
+        assoc_ty: Symbol,
+        substs: SubstsRef<'tcx>,
+    ) -> Option<ProjectionTy<'tcx>> {
+        let Some(assoc_item) = tcx
+            .associated_items(container_id)
+            .find_by_name_and_kind(tcx, Ident::with_dummy_span(assoc_ty), AssocKind::Type, container_id)
+        else {
+            debug_assert!(false, "type `{assoc_ty}` not found in `{container_id:?}`");
+            return None;
+        };
+        #[cfg(debug_assertions)]
+        {
+            let generics = tcx.generics_of(assoc_item.def_id);
+            let generic_count = generics.parent_count + generics.params.len();
+            let params = generics
+                .parent
+                .map_or([].as_slice(), |id| &*tcx.generics_of(id).params)
+                .iter()
+                .chain(&generics.params)
+                .map(|x| &x.kind);
+
+            debug_assert!(
+                generic_count == substs.len(),
+                "wrong number of substs for `{:?}`: found `{}` expected `{}`.\n\
+                    note: the expected parameters are: {:#?}\n\
+                    the given arguments are: `{:#?}`",
+                assoc_item.def_id,
+                substs.len(),
+                generic_count,
+                params.map(GenericParamDefKind::descr).collect::<Vec<_>>(),
+                substs,
+            );
+
+            if let Some((idx, (param, arg))) = params
+                .clone()
+                .zip(substs.iter().map(GenericArg::unpack))
+                .enumerate()
+                .find(|(_, (param, arg))| {
+                    !matches!(
+                        (param, arg),
+                        (GenericParamDefKind::Lifetime, GenericArgKind::Lifetime(_))
+                            | (GenericParamDefKind::Type { .. }, GenericArgKind::Type(_))
+                            | (GenericParamDefKind::Const { .. }, GenericArgKind::Const(_))
+                    )
+                })
+            {
+                debug_assert!(
+                    false,
+                    "mismatched subst type at index {}: expected a {}, found `{:?}`\n\
+                        note: the expected parameters are {:#?}\n\
+                        the given arguments are {:#?}",
+                    idx,
+                    param.descr(),
+                    arg,
+                    params.map(GenericParamDefKind::descr).collect::<Vec<_>>(),
+                    substs,
+                );
+            }
+        }
+
+        Some(ProjectionTy {
+            substs,
+            item_def_id: assoc_item.def_id,
+        })
+    }
+    helper(
+        tcx,
+        container_id,
+        assoc_ty,
+        tcx.mk_substs(substs.into_iter().map(Into::into)),
+    )
+}
+
+/// Normalizes the named associated type in the given impl or trait impl.
+///
+/// This function is for associated types which are "known" to be valid with the given
+/// substitutions, and as such, will only return `None` when debug assertions are disabled in order
+/// to prevent ICE's. With debug assertions enabled this will check that that type normalization
+/// succeeds as well as everything checked by `make_projection`.
+pub fn make_normalized_projection<'tcx>(
+    tcx: TyCtxt<'tcx>,
+    param_env: ParamEnv<'tcx>,
+    container_id: DefId,
+    assoc_ty: Symbol,
+    substs: impl IntoIterator<Item = impl Into<GenericArg<'tcx>>>,
+) -> Option<Ty<'tcx>> {
+    fn helper<'tcx>(tcx: TyCtxt<'tcx>, param_env: ParamEnv<'tcx>, ty: ProjectionTy<'tcx>) -> Option<Ty<'tcx>> {
+        #[cfg(debug_assertions)]
+        if let Some((i, subst)) = ty
+            .substs
+            .iter()
+            .enumerate()
+            .find(|(_, subst)| subst.has_late_bound_regions())
+        {
+            debug_assert!(
+                false,
+                "substs contain late-bound region at index `{i}` which can't be normalized.\n\
+                    use `TyCtxt::erase_late_bound_regions`\n\
+                    note: subst is `{subst:#?}`",
+            );
+            return None;
+        }
+        match tcx.try_normalize_erasing_regions(param_env, tcx.mk_projection(ty.item_def_id, ty.substs)) {
+            Ok(ty) => Some(ty),
+            Err(e) => {
+                debug_assert!(false, "failed to normalize type `{ty}`: {e:#?}");
+                None
+            },
+        }
+    }
+    helper(tcx, param_env, make_projection(tcx, container_id, assoc_ty, substs)?)
+}
diff --git a/tests/ui/needless_collect.fixed b/tests/ui/needless_collect.fixed
index 6ecbbcb6249..2659ad38488 100644
--- a/tests/ui/needless_collect.fixed
+++ b/tests/ui/needless_collect.fixed
@@ -33,4 +33,33 @@ fn main() {
     // `BinaryHeap` doesn't have `contains` method
     sample.iter().count();
     sample.iter().next().is_none();
+
+    // Don't lint string from str
+    let _ = ["", ""].into_iter().collect::<String>().is_empty();
+
+    let _ = sample.iter().next().is_none();
+    let _ = sample.iter().any(|x| x == &0);
+
+    struct VecWrapper<T>(Vec<T>);
+    impl<T> core::ops::Deref for VecWrapper<T> {
+        type Target = Vec<T>;
+        fn deref(&self) -> &Self::Target {
+            &self.0
+        }
+    }
+    impl<T> IntoIterator for VecWrapper<T> {
+        type IntoIter = <Vec<T> as IntoIterator>::IntoIter;
+        type Item = <Vec<T> as IntoIterator>::Item;
+        fn into_iter(self) -> Self::IntoIter {
+            self.0.into_iter()
+        }
+    }
+    impl<T> FromIterator<T> for VecWrapper<T> {
+        fn from_iter<I: IntoIterator<Item = T>>(iter: I) -> Self {
+            Self(Vec::from_iter(iter))
+        }
+    }
+
+    let _ = sample.iter().next().is_none();
+    let _ = sample.iter().any(|x| x == &0);
 }
diff --git a/tests/ui/needless_collect.rs b/tests/ui/needless_collect.rs
index 8dc69bcf5b3..535ec82982b 100644
--- a/tests/ui/needless_collect.rs
+++ b/tests/ui/needless_collect.rs
@@ -33,4 +33,33 @@ fn main() {
     // `BinaryHeap` doesn't have `contains` method
     sample.iter().collect::<BinaryHeap<_>>().len();
     sample.iter().collect::<BinaryHeap<_>>().is_empty();
+
+    // Don't lint string from str
+    let _ = ["", ""].into_iter().collect::<String>().is_empty();
+
+    let _ = sample.iter().collect::<HashSet<_>>().is_empty();
+    let _ = sample.iter().collect::<HashSet<_>>().contains(&&0);
+
+    struct VecWrapper<T>(Vec<T>);
+    impl<T> core::ops::Deref for VecWrapper<T> {
+        type Target = Vec<T>;
+        fn deref(&self) -> &Self::Target {
+            &self.0
+        }
+    }
+    impl<T> IntoIterator for VecWrapper<T> {
+        type IntoIter = <Vec<T> as IntoIterator>::IntoIter;
+        type Item = <Vec<T> as IntoIterator>::Item;
+        fn into_iter(self) -> Self::IntoIter {
+            self.0.into_iter()
+        }
+    }
+    impl<T> FromIterator<T> for VecWrapper<T> {
+        fn from_iter<I: IntoIterator<Item = T>>(iter: I) -> Self {
+            Self(Vec::from_iter(iter))
+        }
+    }
+
+    let _ = sample.iter().collect::<VecWrapper<_>>().is_empty();
+    let _ = sample.iter().collect::<VecWrapper<_>>().contains(&&0);
 }
diff --git a/tests/ui/needless_collect.stderr b/tests/ui/needless_collect.stderr
index 039091627a8..584d2a1d835 100644
--- a/tests/ui/needless_collect.stderr
+++ b/tests/ui/needless_collect.stderr
@@ -66,5 +66,29 @@ error: avoid using `collect()` when not needed
 LL |     sample.iter().collect::<BinaryHeap<_>>().is_empty();
    |                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `next().is_none()`
 
-error: aborting due to 11 previous errors
+error: avoid using `collect()` when not needed
+  --> $DIR/needless_collect.rs:40:27
+   |
+LL |     let _ = sample.iter().collect::<HashSet<_>>().is_empty();
+   |                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `next().is_none()`
+
+error: avoid using `collect()` when not needed
+  --> $DIR/needless_collect.rs:41:27
+   |
+LL |     let _ = sample.iter().collect::<HashSet<_>>().contains(&&0);
+   |                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `any(|x| x == &0)`
+
+error: avoid using `collect()` when not needed
+  --> $DIR/needless_collect.rs:63:27
+   |
+LL |     let _ = sample.iter().collect::<VecWrapper<_>>().is_empty();
+   |                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `next().is_none()`
+
+error: avoid using `collect()` when not needed
+  --> $DIR/needless_collect.rs:64:27
+   |
+LL |     let _ = sample.iter().collect::<VecWrapper<_>>().contains(&&0);
+   |                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `any(|x| x == &0)`
+
+error: aborting due to 15 previous errors