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

Author: bors

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,

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;
@@ -205,28 +204,6 @@ declare_clippy_lint! {
     "`loop { if let { ... } else break }`, which can be written as a `while let` loop"
 }
 
-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
@@ -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);
     }
 }
 

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,

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();

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();