New line: cloned_next
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 8f4da9a..258a825 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -3049,6 +3049,7 @@
[`iter_not_returning_iterator`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_not_returning_iterator
[`iter_nth`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_nth
[`iter_nth_zero`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_nth_zero
+[`iter_overeager_cloned`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_overeager_cloned
[`iter_skip_next`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_skip_next
[`iterator_step_by_zero`]: https://rust-lang.github.io/rust-clippy/master/index.html#iterator_step_by_zero
[`just_underscores_and_digits`]: https://rust-lang.github.io/rust-clippy/master/index.html#just_underscores_and_digits
diff --git a/clippy_lints/src/lib.register_all.rs b/clippy_lints/src/lib.register_all.rs
index 26fb425..eba5109 100644
--- a/clippy_lints/src/lib.register_all.rs
+++ b/clippy_lints/src/lib.register_all.rs
@@ -156,6 +156,7 @@
LintId::of(methods::ITER_NEXT_SLICE),
LintId::of(methods::ITER_NTH),
LintId::of(methods::ITER_NTH_ZERO),
+ LintId::of(methods::ITER_OVEREAGER_CLONED),
LintId::of(methods::ITER_SKIP_NEXT),
LintId::of(methods::MANUAL_FILTER_MAP),
LintId::of(methods::MANUAL_FIND_MAP),
diff --git a/clippy_lints/src/lib.register_lints.rs b/clippy_lints/src/lib.register_lints.rs
index 746bdb1..56146a0 100644
--- a/clippy_lints/src/lib.register_lints.rs
+++ b/clippy_lints/src/lib.register_lints.rs
@@ -288,6 +288,7 @@
methods::ITER_NEXT_SLICE,
methods::ITER_NTH,
methods::ITER_NTH_ZERO,
+ methods::ITER_OVEREAGER_CLONED,
methods::ITER_SKIP_NEXT,
methods::MANUAL_FILTER_MAP,
methods::MANUAL_FIND_MAP,
diff --git a/clippy_lints/src/lib.register_perf.rs b/clippy_lints/src/lib.register_perf.rs
index f9ffd4c..c44ef12 100644
--- a/clippy_lints/src/lib.register_perf.rs
+++ b/clippy_lints/src/lib.register_perf.rs
@@ -14,6 +14,7 @@
LintId::of(methods::EXPECT_FUN_CALL),
LintId::of(methods::EXTEND_WITH_DRAIN),
LintId::of(methods::ITER_NTH),
+ LintId::of(methods::ITER_OVEREAGER_CLONED),
LintId::of(methods::MANUAL_STR_REPEAT),
LintId::of(methods::OR_FUN_CALL),
LintId::of(methods::SINGLE_CHAR_PATTERN),
diff --git a/clippy_lints/src/methods/iter_overeager_cloned.rs b/clippy_lints/src/methods/iter_overeager_cloned.rs
new file mode 100644
index 0000000..ca33bfc
--- /dev/null
+++ b/clippy_lints/src/methods/iter_overeager_cloned.rs
@@ -0,0 +1,62 @@
+use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::source::snippet;
+use clippy_utils::ty::{get_iterator_item_ty, is_copy};
+use itertools::Itertools;
+use rustc_errors::Applicability;
+use rustc_hir as hir;
+use rustc_lint::LateContext;
+use rustc_middle::ty;
+use std::ops::Not;
+
+use super::ITER_OVEREAGER_CLONED;
+use crate::redundant_clone::REDUNDANT_CLONE;
+
+/// lint overeager use of `cloned()` for `Iterator`s
+pub(super) fn check<'tcx>(
+ cx: &LateContext<'tcx>,
+ expr: &'tcx hir::Expr<'_>,
+ recv: &'tcx hir::Expr<'_>,
+ name: &str,
+ map_arg: &[hir::Expr<'_>],
+) {
+ // Check if it's iterator and get type associated with `Item`.
+ let inner_ty = match get_iterator_item_ty(cx, cx.typeck_results().expr_ty_adjusted(recv)) {
+ Some(ty) => ty,
+ _ => return,
+ };
+
+ match inner_ty.kind() {
+ ty::Ref(_, ty, _) if !is_copy(cx, ty) => {},
+ _ => return,
+ };
+
+ let (lint, preserve_cloned) = match name {
+ "count" => (REDUNDANT_CLONE, false),
+ _ => (ITER_OVEREAGER_CLONED, true),
+ };
+ let wildcard_params = map_arg.is_empty().not().then(|| "...").unwrap_or_default();
+ let msg = format!(
+ "called `cloned().{}({})` on an `Iterator`. It may be more efficient to call `{}({}){}` instead",
+ name,
+ wildcard_params,
+ name,
+ wildcard_params,
+ preserve_cloned.then(|| ".cloned()").unwrap_or_default(),
+ );
+
+ span_lint_and_sugg(
+ cx,
+ lint,
+ expr.span,
+ &msg,
+ "try this",
+ format!(
+ "{}.{}({}){}",
+ snippet(cx, recv.span, ".."),
+ name,
+ map_arg.iter().map(|a| snippet(cx, a.span, "..")).join(", "),
+ preserve_cloned.then(|| ".cloned()").unwrap_or_default(),
+ ),
+ Applicability::MachineApplicable,
+ );
+}
diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs
index a3cfdd3..cff6251 100644
--- a/clippy_lints/src/methods/mod.rs
+++ b/clippy_lints/src/methods/mod.rs
@@ -30,6 +30,7 @@
mod iter_next_slice;
mod iter_nth;
mod iter_nth_zero;
+mod iter_overeager_cloned;
mod iter_skip_next;
mod iterator_step_by_zero;
mod manual_saturating_arithmetic;
@@ -108,6 +109,41 @@
declare_clippy_lint! {
/// ### What it does
+ /// Checks for usage of `_.cloned().<func>()` where call to `.cloned()` can be postponed.
+ ///
+ /// ### Why is this bad?
+ /// It's often inefficient to clone all elements of an iterator, when eventually, only some
+ /// of them will be consumed.
+ ///
+ /// ### Examples
+ /// ```rust
+ /// # let vec = vec!["string".to_string()];
+ ///
+ /// // Bad
+ /// vec.iter().cloned().take(10);
+ ///
+ /// // Good
+ /// vec.iter().take(10).cloned();
+ ///
+ /// // Bad
+ /// vec.iter().cloned().last();
+ ///
+ /// // Good
+ /// vec.iter().last().cloned();
+ ///
+ /// ```
+ /// ### Known Problems
+ /// This `lint` removes the side of effect of cloning items in the iterator.
+ /// A code that relies on that side-effect could fail.
+ ///
+ #[clippy::version = "1.59.0"]
+ pub ITER_OVEREAGER_CLONED,
+ perf,
+ "using `cloned()` early with `Iterator::iter()` can lead to some performance inefficiencies"
+}
+
+declare_clippy_lint! {
+ /// ### What it does
/// Checks for usages of `Iterator::flat_map()` where `filter_map()` could be
/// used instead.
///
@@ -1950,6 +1986,7 @@
CLONE_ON_COPY,
CLONE_ON_REF_PTR,
CLONE_DOUBLE_REF,
+ ITER_OVEREAGER_CLONED,
CLONED_INSTEAD_OF_COPIED,
FLAT_MAP_OPTION,
INEFFICIENT_TO_STRING,
@@ -2243,9 +2280,10 @@
},
_ => {},
},
- ("count", []) => match method_call(recv) {
- Some((name @ ("into_iter" | "iter" | "iter_mut"), [recv2], _)) => {
- iter_count::check(cx, expr, recv2, name);
+ (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], _)) => {
+ iter_count::check(cx, expr, recv2, name2);
},
Some(("map", [_, arg], _)) => suspicious_map::check(cx, expr, recv, arg),
_ => {},
@@ -2266,10 +2304,10 @@
flat_map_identity::check(cx, expr, arg, span);
flat_map_option::check(cx, expr, arg, span);
},
- ("flatten", []) => {
- if let Some(("map", [recv, map_arg], _)) = method_call(recv) {
- map_flatten::check(cx, expr, recv, map_arg);
- }
+ (name @ "flatten", args @ []) => match method_call(recv) {
+ Some(("map", [recv, map_arg], _)) => map_flatten::check(cx, expr, recv, map_arg),
+ 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", [_]) => {
@@ -2281,6 +2319,13 @@
("is_file", []) => filetype_is_file::check(cx, expr, recv),
("is_none", []) => check_is_some_is_none(cx, expr, recv, false),
("is_some", []) => check_is_some_is_none(cx, expr, recv, true),
+ ("last", args @ []) => {
+ if let Some((name2, [recv2, args2 @ ..], _span2)) = method_call(recv) {
+ if let ("cloned", []) = (name2, args2) {
+ iter_overeager_cloned::check(cx, expr, recv2, name, args);
+ }
+ }
+ },
("map", [m_arg]) => {
if let Some((name, [recv2, args @ ..], span2)) = method_call(recv) {
match (name, args) {
@@ -2296,20 +2341,22 @@
map_identity::check(cx, expr, recv, m_arg, span);
},
("map_or", [def, map]) => option_map_or_none::check(cx, expr, recv, def, map),
- ("next", []) => {
- if let Some((name, [recv, args @ ..], _)) = method_call(recv) {
- match (name, args) {
- ("filter", [arg]) => filter_next::check(cx, expr, recv, arg),
- ("filter_map", [arg]) => filter_map_next::check(cx, expr, recv, arg, msrv),
- ("iter", []) => iter_next_slice::check(cx, expr, recv),
- ("skip", [arg]) => iter_skip_next::check(cx, expr, recv, arg),
+ (name @ "next", args @ []) => {
+ 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),
+ ("filter_map", [arg]) => filter_map_next::check(cx, expr, recv2, arg, msrv),
+ ("iter", []) => iter_next_slice::check(cx, expr, recv2),
+ ("skip", [arg]) => iter_skip_next::check(cx, expr, recv2, arg),
("skip_while", [_]) => skip_while_next::check(cx, expr),
_ => {},
}
}
},
- ("nth", [n_arg]) => match method_call(recv) {
+ ("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),
_ => iter_nth_zero::check(cx, expr, recv, n_arg),
@@ -2320,6 +2367,13 @@
unnecessary_lazy_eval::check(cx, expr, recv, arg, "or");
}
},
+ ("skip", args) => {
+ if let Some((name2, [recv2, args2 @ ..], _span2)) = method_call(recv) {
+ if let ("cloned", []) = (name2, args2) {
+ iter_overeager_cloned::check(cx, expr, recv2, name, args);
+ }
+ }
+ },
("splitn" | "rsplitn", [count_arg, pat_arg]) => {
if let Some((Constant::Int(count), _)) = constant(cx, cx.typeck_results(), count_arg) {
suspicious_splitn::check(cx, name, expr, recv, count);
@@ -2337,6 +2391,13 @@
}
},
("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 ("cloned", []) = (name2, args2) {
+ iter_overeager_cloned::check(cx, expr, recv2, name, args);
+ }
+ }
+ },
("to_os_string" | "to_owned" | "to_path_buf" | "to_vec", []) => {
implicit_clone::check(cx, name, expr, recv);
},
diff --git a/tests/ui/iter_overeager_cloned.fixed b/tests/ui/iter_overeager_cloned.fixed
new file mode 100644
index 0000000..a904167
--- /dev/null
+++ b/tests/ui/iter_overeager_cloned.fixed
@@ -0,0 +1,45 @@
+// run-rustfix
+#![warn(clippy::iter_overeager_cloned, clippy::redundant_clone, clippy::filter_next)]
+
+fn main() {
+ let vec = vec!["1".to_string(), "2".to_string(), "3".to_string()];
+
+ let _: Option<String> = vec.iter().last().cloned();
+
+ let _: Option<String> = vec.iter().chain(vec.iter()).next().cloned();
+
+ let _: usize = vec.iter().filter(|x| x == &"2").count();
+
+ let _: Vec<_> = vec.iter().take(2).cloned().collect();
+
+ let _: Vec<_> = vec.iter().skip(2).cloned().collect();
+
+ let _ = vec.iter().filter(|x| x == &"2").nth(2).cloned();
+
+ let _ = [Some(Some("str".to_string())), Some(Some("str".to_string()))]
+ .iter().flatten().cloned();
+
+ // Not implemented yet
+ let _ = vec.iter().cloned().filter(|x| x.starts_with('2'));
+
+ // Not implemented yet
+ let _ = vec.iter().cloned().map(|x| x.len());
+
+ // This would fail if changed.
+ let _ = vec.iter().cloned().map(|x| x + "2");
+
+ // Not implemented yet
+ let _ = vec.iter().cloned().find(|x| x == "2");
+
+ // Not implemented yet
+ let _ = vec.iter().cloned().for_each(|x| assert!(!x.is_empty()));
+
+ // Not implemented yet
+ let _ = vec.iter().cloned().all(|x| x.len() == 1);
+
+ // Not implemented yet
+ let _ = vec.iter().cloned().any(|x| x.len() == 1);
+
+ // Should probably stay as it is.
+ let _ = [0, 1, 2, 3, 4].iter().cloned().take(10);
+}
diff --git a/tests/ui/iter_overeager_cloned.rs b/tests/ui/iter_overeager_cloned.rs
new file mode 100644
index 0000000..dd04e33
--- /dev/null
+++ b/tests/ui/iter_overeager_cloned.rs
@@ -0,0 +1,47 @@
+// run-rustfix
+#![warn(clippy::iter_overeager_cloned, clippy::redundant_clone, clippy::filter_next)]
+
+fn main() {
+ let vec = vec!["1".to_string(), "2".to_string(), "3".to_string()];
+
+ let _: Option<String> = vec.iter().cloned().last();
+
+ let _: Option<String> = vec.iter().chain(vec.iter()).cloned().next();
+
+ let _: usize = vec.iter().filter(|x| x == &"2").cloned().count();
+
+ let _: Vec<_> = vec.iter().cloned().take(2).collect();
+
+ let _: Vec<_> = vec.iter().cloned().skip(2).collect();
+
+ let _ = vec.iter().filter(|x| x == &"2").cloned().nth(2);
+
+ let _ = [Some(Some("str".to_string())), Some(Some("str".to_string()))]
+ .iter()
+ .cloned()
+ .flatten();
+
+ // Not implemented yet
+ let _ = vec.iter().cloned().filter(|x| x.starts_with('2'));
+
+ // Not implemented yet
+ let _ = vec.iter().cloned().map(|x| x.len());
+
+ // This would fail if changed.
+ let _ = vec.iter().cloned().map(|x| x + "2");
+
+ // Not implemented yet
+ let _ = vec.iter().cloned().find(|x| x == "2");
+
+ // Not implemented yet
+ let _ = vec.iter().cloned().for_each(|x| assert!(!x.is_empty()));
+
+ // Not implemented yet
+ let _ = vec.iter().cloned().all(|x| x.len() == 1);
+
+ // Not implemented yet
+ let _ = vec.iter().cloned().any(|x| x.len() == 1);
+
+ // Should probably stay as it is.
+ let _ = [0, 1, 2, 3, 4].iter().cloned().take(10);
+}
diff --git a/tests/ui/iter_overeager_cloned.stderr b/tests/ui/iter_overeager_cloned.stderr
new file mode 100644
index 0000000..e36b0e3
--- /dev/null
+++ b/tests/ui/iter_overeager_cloned.stderr
@@ -0,0 +1,58 @@
+error: called `cloned().last()` on an `Iterator`. It may be more efficient to call `last().cloned()` instead
+ --> $DIR/iter_overeager_cloned.rs:7:29
+ |
+LL | let _: Option<String> = vec.iter().cloned().last();
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `vec.iter().last().cloned()`
+ |
+ = note: `-D clippy::iter-overeager-cloned` implied by `-D warnings`
+
+error: called `cloned().next()` on an `Iterator`. It may be more efficient to call `next().cloned()` instead
+ --> $DIR/iter_overeager_cloned.rs:9:29
+ |
+LL | let _: Option<String> = vec.iter().chain(vec.iter()).cloned().next();
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `vec.iter().chain(vec.iter()).next().cloned()`
+
+error: called `cloned().count()` on an `Iterator`. It may be more efficient to call `count()` instead
+ --> $DIR/iter_overeager_cloned.rs:11:20
+ |
+LL | let _: usize = vec.iter().filter(|x| x == &"2").cloned().count();
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `vec.iter().filter(|x| x == &"2").count()`
+ |
+ = note: `-D clippy::redundant-clone` implied by `-D warnings`
+
+error: called `cloned().take(...)` on an `Iterator`. It may be more efficient to call `take(...).cloned()` instead
+ --> $DIR/iter_overeager_cloned.rs:13:21
+ |
+LL | let _: Vec<_> = vec.iter().cloned().take(2).collect();
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `vec.iter().take(2).cloned()`
+
+error: called `cloned().skip(...)` on an `Iterator`. It may be more efficient to call `skip(...).cloned()` instead
+ --> $DIR/iter_overeager_cloned.rs:15:21
+ |
+LL | let _: Vec<_> = vec.iter().cloned().skip(2).collect();
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `vec.iter().skip(2).cloned()`
+
+error: called `cloned().nth(...)` on an `Iterator`. It may be more efficient to call `nth(...).cloned()` instead
+ --> $DIR/iter_overeager_cloned.rs:17:13
+ |
+LL | let _ = vec.iter().filter(|x| x == &"2").cloned().nth(2);
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `vec.iter().filter(|x| x == &"2").nth(2).cloned()`
+
+error: called `cloned().flatten()` on an `Iterator`. It may be more efficient to call `flatten().cloned()` instead
+ --> $DIR/iter_overeager_cloned.rs:19:13
+ |
+LL | let _ = [Some(Some("str".to_string())), Some(Some("str".to_string()))]
+ | _____________^
+LL | | .iter()
+LL | | .cloned()
+LL | | .flatten();
+ | |__________________^
+ |
+help: try this
+ |
+LL ~ let _ = [Some(Some("str".to_string())), Some(Some("str".to_string()))]
+LL ~ .iter().flatten().cloned();
+ |
+
+error: aborting due to 7 previous errors
+
