Lint needless_collect on non-std collection types

Author: Jarcho

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_expr_path_def_path(cx, repeat_fn, &paths::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();

clippy_lints/src/methods/mod.rs

@@ -79,7 +79,9 @@ use bind_instead_of_map::BindInsteadOfMap;
 use clippy_utils::consts::{constant, Constant};
 use clippy_utils::diagnostics::{span_lint, span_lint_and_help};
 use clippy_utils::ty::{contains_adt_constructor, contains_ty, implements_trait, is_copy, is_type_diagnostic_item};
-use clippy_utils::{contains_return, get_trait_def_id, iter_input_pats, meets_msrv, msrvs, paths, return_ty};
+use clippy_utils::{
+    contains_return, get_trait_def_id, is_trait_method, iter_input_pats, meets_msrv, msrvs, paths, return_ty,
+};
 use if_chain::if_chain;
 use rustc_hir as hir;
 use rustc_hir::def::Res;
@@ -2314,11 +2316,11 @@ impl_lint_pass!(Methods => [
 ]);
 
 /// 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>], Span)> {
-    if let ExprKind::MethodCall(path, args, _) = recv.kind {
+fn method_call<'tcx>(recv: &'tcx hir::Expr<'tcx>) -> Option<(&'tcx str, &'tcx [hir::Expr<'tcx>], Span, Span)> {
+    if let ExprKind::MethodCall(path, args, call_span) = recv.kind {
         if !args.iter().any(|e| e.span.from_expansion()) {
             let name = path.ident.name.as_str();
-            return Some((name, args, path.ident.span));
+            return Some((name, args, path.ident.span, call_span));
         }
     }
     None
@@ -2359,8 +2361,6 @@ impl<'tcx> LateLintPass<'tcx> for Methods {
             },
             _ => (),
         }
-
-        needless_collect::check(expr, cx);
     }
 
     #[allow(clippy::too_many_lines)]
@@ -2533,7 +2533,7 @@ impl<'tcx> LateLintPass<'tcx> for Methods {
 
 #[allow(clippy::too_many_lines)]
 fn check_methods<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, msrv: Option<&RustcVersion>) {
-    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);
@@ -2552,34 +2552,37 @@ fn check_methods<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, msrv: Optio
             ("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, 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(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(msrv, &msrvs::STR_REPEAT) {
+                            manual_str_repeat::check(cx, expr, recv, take_self_arg, take_arg);
+                        }
+                    },
+                    _ => {},
+                }
             },
             (name @ "count", args @ []) => match method_call(recv) {
-                Some(("cloned", [recv2], _)) => iter_overeager_cloned::check(cx, expr, recv2, name, args),
-                Some((name2 @ ("into_iter" | "iter" | "iter_mut"), [recv2], _)) => {
+                Some(("cloned", [recv2], _, _)) => iter_overeager_cloned::check(cx, expr, recv2, name, args),
+                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(("map", [_, arg], _, _)) => suspicious_map::check(cx, expr, recv, arg),
                 _ => {},
             },
             ("drain", [arg]) => {
                 iter_with_drain::check(cx, expr, recv, span, arg);
             },
             ("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, msrv, span, err_span),
+                Some(("ok", [recv], _, _)) => ok_expect::check(cx, expr, recv),
+                Some(("err", [recv], err_span, _)) => err_expect::check(cx, expr, recv, msrv, span, err_span),
                 _ => expect_used::check(cx, expr, recv),
             },
             ("extend", [arg]) => {
@@ -2598,13 +2601,13 @@ fn check_methods<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, msrv: Optio
                 flat_map_option::check(cx, expr, arg, span);
             },
             (name @ "flatten", args @ []) => 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, recv2, name, args),
+                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, recv2, name, args),
                 _ => {},
             },
             ("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);
                 }
             },
@@ -2614,19 +2617,19 @@ fn check_methods<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, msrv: Optio
             ("is_none", []) => check_is_some_is_none(cx, expr, recv, false),
             ("is_some", []) => check_is_some_is_none(cx, expr, recv, true),
             ("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", args @ []) | ("skip", args @ [_]) => {
-                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, recv2, name, args);
                     }
                 }
             },
             (name @ ("map" | "map_err"), [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, msrv),
                         ("as_ref", []) => option_as_ref_deref::check(cx, expr, recv2, m_arg, false, msrv),
@@ -2641,7 +2644,7 @@ fn check_methods<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, msrv: Optio
             },
             ("map_or", [def, map]) => option_map_or_none::check(cx, expr, recv, def, map),
             (name @ "next", args @ []) => {
-                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, recv2, name, args),
                         ("filter", [arg]) => filter_next::check(cx, expr, recv2, arg),
@@ -2654,10 +2657,10 @@ fn check_methods<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, msrv: Optio
                 }
             },
             ("nth", args @ [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, recv2, name, args),
-                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, recv2, name, args),
+                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"),
@@ -2679,7 +2682,7 @@ fn check_methods<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, msrv: Optio
             },
             ("step_by", [arg]) => iterator_step_by_zero::check(cx, expr, arg),
             ("take", args @ [_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, recv2, name, args);
                     }
@@ -2691,30 +2694,31 @@ fn check_methods<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, msrv: Optio
             },
             ("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);
                     },
                     _ => {},
                 }
                 unwrap_used::check(cx, expr, recv);
             },
             ("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);
                 },
                 _ => {},
             },
             ("unwrap_or_else", [u_arg]) => match method_call(recv) {
-                Some(("map", [recv, map_arg], _)) if map_unwrap_or::check(cx, expr, recv, map_arg, u_arg, msrv) => {},
+                Some(("map", [recv, map_arg], _, _)) if map_unwrap_or::check(cx, expr, recv, map_arg, u_arg, msrv) => {
+                },
                 _ => {
                     unwrap_or_else_default::check(cx, expr, recv, u_arg);
                     unnecessary_lazy_eval::check(cx, expr, recv, u_arg, "unwrap_or");
@@ -2726,7 +2730,7 @@ fn check_methods<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, msrv: Optio
 }
 
 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);
     }
 }