]> git.lizzy.rs Git - rust.git/blobdiff - clippy_lints/src/methods/mod.rs
Auto merge of #9698 - kraktus:xc_bool, r=xFrednet
[rust.git] / clippy_lints / src / methods / mod.rs
index 996d4cb412344ae7e9ea01154ec3791cf6ffc0c7..5c8add5f3022612433100379cc5006297e91e434 100644 (file)
@@ -54,6 +54,7 @@
 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;
     /// ```
     /// Use instead:
     /// ```rust
-    /// let hello = "hesuo worpd".replace(&['s', 'u', 'p'], "l");
+    /// let hello = "hesuo worpd".replace(['s', 'u', 'p'], "l");
     /// ```
-    #[clippy::version = "1.64.0"]
+    #[clippy::version = "1.65.0"]
     pub COLLAPSIBLE_STR_REPLACE,
     perf,
     "collapse consecutive calls to str::replace (2 or more) into a single call"
 
 declare_clippy_lint! {
     /// ### What it does
-    /// Checks for usage of `_.as_ref().map(Deref::deref)` or it's aliases (such as String::as_str).
+    /// Checks for usage of `_.as_ref().map(Deref::deref)` or its aliases (such as String::as_str).
     ///
     /// ### Why is this bad?
     /// Readability, this can be written more concisely as
     /// let s = "Hello world!";
     /// let cow = Cow::Borrowed(s);
     ///
-    /// let data = cow.into_owned();
-    /// assert!(matches!(data, String))
+    /// let _data: String = cow.into_owned();
     /// ```
     #[clippy::version = "1.65.0"]
     pub SUSPICIOUS_TO_OWNED,
     /// ### Known problems
     ///
     /// The type of the resulting iterator might become incompatible with its usage
-    #[clippy::version = "1.64.0"]
+    #[clippy::version = "1.65.0"]
     pub ITER_ON_SINGLE_ITEMS,
     nursery,
     "Iterator for array of length 1"
     /// ### Known problems
     ///
     /// The type of the resulting iterator might become incompatible with its usage
-    #[clippy::version = "1.64.0"]
+    #[clippy::version = "1.65.0"]
     pub ITER_ON_EMPTY_COLLECTIONS,
     nursery,
     "Iterator for empty array"
     "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 +3289,17 @@ pub fn new(
     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 +3485,7 @@ fn check_trait_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx TraitItem<'_>
 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 +3504,31 @@ fn check_methods<'tcx>(&self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
                 ("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 +3540,8 @@ fn check_methods<'tcx>(&self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
                     }
                 },
                 ("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 +3561,13 @@ fn check_methods<'tcx>(&self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
                     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 +3587,12 @@ fn check_methods<'tcx>(&self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
                     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 +3604,13 @@ fn check_methods<'tcx>(&self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
                 (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 +3630,7 @@ fn check_methods<'tcx>(&self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
                     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 +3643,10 @@ fn check_methods<'tcx>(&self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
                     }
                 },
                 ("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 +3711,7 @@ fn check_methods<'tcx>(&self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
                 },
                 ("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 +3734,13 @@ fn check_methods<'tcx>(&self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
                 },
                 ("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 +3749,19 @@ fn check_methods<'tcx>(&self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
                 },
                 ("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 +3782,7 @@ fn check_methods<'tcx>(&self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
 }
 
 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);
     }
 }