nordic-dev.net/rust
nordic-dev.net/rust
2
0
Fork 0
mirror of https://github.com/rust-lang/rust.git synced 2024-11-01 23:12:02 +00:00
Code Issues Packages Projects Releases Wiki Activity

Refactor excessive_for_each

Browse Source
This commit is contained in:
Yoshitomo Nakanishi 2021-03-13 00:42:43 +09:00
parent 5543c34699
commit 527fbbef48
13 changed files with 525 additions and 406 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
CHANGELOG.md
View File

@ -2192,7 +2192,6 @@ Released 2018-09-13
[`eq_op`]: https://rust-lang.github.io/rust-clippy/master/index.html#eq_op
[`erasing_op`]: https://rust-lang.github.io/rust-clippy/master/index.html#erasing_op
[`eval_order_dependence`]: https://rust-lang.github.io/rust-clippy/master/index.html#eval_order_dependence
[`excessive_for_each`]: https://rust-lang.github.io/rust-clippy/master/index.html#excessive_for_each
[`excessive_precision`]: https://rust-lang.github.io/rust-clippy/master/index.html#excessive_precision
[`exhaustive_enums`]: https://rust-lang.github.io/rust-clippy/master/index.html#exhaustive_enums
[`exhaustive_structs`]: https://rust-lang.github.io/rust-clippy/master/index.html#exhaustive_structs
@ -2370,6 +2369,7 @@ Released 2018-09-13
[`needless_collect`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_collect
[`needless_continue`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_continue
[`needless_doctest_main`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_doctest_main
[`needless_for_each`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_for_each
[`needless_lifetimes`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_lifetimes
[`needless_pass_by_value`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_pass_by_value
[`needless_question_mark`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_question_mark

0
clippy_lints/src/iter_for_each.rs
View File

6
clippy_lints/src/lib.rs
View File

@ -291,6 +291,7 @@ mod needless_bool;
mod needless_borrow;
mod needless_borrowed_ref;
mod needless_continue;
mod needless_for_each;
mod needless_pass_by_value;
mod needless_question_mark;
mod needless_update;
@ -781,7 +782,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
&methods::CLONE_DOUBLE_REF,
&methods::CLONE_ON_COPY,
&methods::CLONE_ON_REF_PTR,
&methods::EXCESSIVE_FOR_EACH,
&methods::EXPECT_FUN_CALL,
&methods::EXPECT_USED,
&methods::FILETYPE_IS_FILE,
@ -868,6 +868,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
&needless_borrow::NEEDLESS_BORROW,
&needless_borrowed_ref::NEEDLESS_BORROWED_REFERENCE,
&needless_continue::NEEDLESS_CONTINUE,
&needless_for_each::NEEDLESS_FOR_EACH,
&needless_pass_by_value::NEEDLESS_PASS_BY_VALUE,
&needless_question_mark::NEEDLESS_QUESTION_MARK,
&needless_update::NEEDLESS_UPDATE,
@ -1046,6 +1047,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_late_pass(|| box ptr_eq::PtrEq);
store.register_late_pass(|| box needless_bool::NeedlessBool);
store.register_late_pass(|| box needless_bool::BoolComparison);
store.register_late_pass(|| box needless_for_each::NeedlessForEach);
store.register_late_pass(|| box approx_const::ApproxConstant);
store.register_late_pass(|| box misc::MiscLints);
store.register_late_pass(|| box eta_reduction::EtaReduction);
@ -1314,7 +1316,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&matches::WILDCARD_ENUM_MATCH_ARM),
LintId::of(&mem_forget::MEM_FORGET),
LintId::of(&methods::CLONE_ON_REF_PTR),
LintId::of(&methods::EXCESSIVE_FOR_EACH),
LintId::of(&methods::EXPECT_USED),
LintId::of(&methods::FILETYPE_IS_FILE),
LintId::of(&methods::GET_UNWRAP),
@ -1325,6 +1326,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&missing_doc::MISSING_DOCS_IN_PRIVATE_ITEMS),
LintId::of(&missing_inline::MISSING_INLINE_IN_PUBLIC_ITEMS),
LintId::of(&modulo_arithmetic::MODULO_ARITHMETIC),
LintId::of(&needless_for_each::NEEDLESS_FOR_EACH),
LintId::of(&panic_in_result_fn::PANIC_IN_RESULT_FN),
LintId::of(&panic_unimplemented::PANIC),
LintId::of(&panic_unimplemented::TODO),

122
clippy_lints/src/methods/excessive_for_each.rs
View File

@ -1,122 +0,0 @@
use rustc_errors::Applicability;
use rustc_hir::{
intravisit::{walk_expr, NestedVisitorMap, Visitor},
Expr, ExprKind,
};
use rustc_lint::LateContext;
use rustc_middle::hir::map::Map;
use rustc_span::source_map::Span;
use if_chain::if_chain;
use crate::utils::{has_iter_method, match_trait_method, paths, snippet, span_lint_and_then};
use super::EXCESSIVE_FOR_EACH;
pub(super) fn lint(cx: &LateContext<'_>, expr: &'tcx Expr<'_>, args: &[&[Expr<'_>]]) {
if args.len() < 2 {
return;
}
let for_each_args = args[0];
if for_each_args.len() < 2 {
return;
}
let for_each_receiver = &for_each_args[0];
let for_each_arg = &for_each_args[1];
let iter_receiver = &args[1][0];
if_chain! {
if has_iter_method(cx, cx.typeck_results().expr_ty(iter_receiver)).is_some();
if match_trait_method(cx, expr, &paths::ITERATOR);
if let ExprKind::Closure(_, _, body_id, ..) = for_each_arg.kind;
let body = cx.tcx.hir().body(body_id);
if let ExprKind::Block(..) = body.value.kind;
then {
let mut ret_collector = RetCollector::new();
ret_collector.visit_expr(&body.value);
// Skip the lint if `return` is used in `Loop` in order not to suggest using `'label`.
if ret_collector.ret_in_loop {
return;
}
let sugg = format!(
"for {} in {} {{ .. }}",
snippet(cx, body.params[0].pat.span, ".."),
snippet(cx, for_each_receiver.span, "..")
);
span_lint_and_then(
cx,
EXCESSIVE_FOR_EACH,
expr.span,
"excessive use of `for_each`",
|diag| {
diag.span_suggestion(expr.span, "try", sugg, Applicability::HasPlaceholders);
for span in ret_collector.spans {
diag.span_note(span, "change `return` to `continue` in the loop body");
}
}
)
}
}
}
/// This type plays two roles.
/// 1. Collect spans of `return` in the closure body.
/// 2. Detect use of `return` in `Loop` in the closure body.
///
/// NOTE: The functionality of this type is similar to
/// [`crate::utilts::visitors::find_all_ret_expressions`], but we can't use
/// `find_all_ret_expressions` instead of this type. The reasons are:
/// 1. `find_all_ret_expressions` passes the argument of `ExprKind::Ret` to a callback, but what we
/// need here is `ExprKind::Ret` itself.
/// 2. We can't trace current loop depth with `find_all_ret_expressions`.
struct RetCollector {
spans: Vec<Span>,
ret_in_loop: bool,
loop_depth: u16,
}
impl RetCollector {
fn new() -> Self {
Self {
spans: Vec::new(),
ret_in_loop: false,
loop_depth: 0,
}
}
}
impl<'tcx> Visitor<'tcx> for RetCollector {
type Map = Map<'tcx>;
fn visit_expr(&mut self, expr: &Expr<'_>) {
match expr.kind {
ExprKind::Ret(..) => {
if self.loop_depth > 0 && !self.ret_in_loop {
self.ret_in_loop = true
}
self.spans.push(expr.span)
},
ExprKind::Loop(..) => {
self.loop_depth += 1;
walk_expr(self, expr);
self.loop_depth -= 1;
return;
},
_ => {},
}
walk_expr(self, expr);
}
fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
NestedVisitorMap::None
}
}

29
clippy_lints/src/methods/mod.rs
View File

@ -974,33 +974,6 @@ declare_clippy_lint! {
"using `.skip(x).next()` on an iterator"
}
declare_clippy_lint! {
/// **What it does:** Checks for use of `.method(..).for_each(closure)` if the reciever of `.method(..)` doesn't
/// implement `Iterator` and the return type of `.method(..)` implements `Iterator`.
///
/// **Why is this bad?** Excessive use of `for_each` reduces redability, using `for` loop is
/// clearer and more concise.
///
/// **Known problems:** None.
///
/// **Example:**
///
/// ```rust
/// let v = vec![0, 1, 2];
/// v.iter().for_each(|elem| println!("{}", elem));
/// ```
/// Use instead:
/// ```rust
/// let v = vec![0, 1, 2];
/// for elem in v.iter() {
/// println!("{}", elem);
/// }
/// ```
pub EXCESSIVE_FOR_EACH,
restriction,
"using `.iter().for_each(|x| {..})` when using `for` loop would work instead"
}
declare_clippy_lint! {
/// **What it does:** Checks for use of `.get().unwrap()` (or
/// `.get_mut().unwrap`) on a standard library type which implements `Index`
@ -1688,7 +1661,6 @@ impl_lint_pass!(Methods => [
ITER_NTH_ZERO,
BYTES_NTH,
ITER_SKIP_NEXT,
EXCESSIVE_FOR_EACH,
GET_UNWRAP,
STRING_EXTEND_CHARS,
ITER_CLONED_COLLECT,
@ -1835,7 +1807,6 @@ impl<'tcx> LateLintPass<'tcx> for Methods {
["to_os_string", ..] => implicit_clone::check(cx, expr, sym::OsStr),
["to_path_buf", ..] => implicit_clone::check(cx, expr, sym::Path),
["to_vec", ..] => implicit_clone::check(cx, expr, sym::slice),
["for_each", ..] => excessive_for_each::lint(cx, expr, &arg_lists),
_ => {},
}

170
clippy_lints/src/needless_for_each.rs Normal file
View File

@ -0,0 +1,170 @@
use rustc_errors::Applicability;
use rustc_hir::{
intravisit::{walk_expr, NestedVisitorMap, Visitor},
Expr, ExprKind, Stmt, StmtKind,
};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::hir::map::Map;
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::{source_map::Span, sym, Symbol};
use if_chain::if_chain;
use crate::utils::{
has_iter_method, is_diagnostic_assoc_item, method_calls, snippet_with_applicability, span_lint_and_then,
};
declare_clippy_lint! {
/// **What it does:** Checks for usage of `for_each` that would be more simply written as a
/// `for` loop.
///
/// **Why is this bad?** `for_each` may be used after applying iterator transformers like
/// `filter` for better readability and performance. It may also be used to fit a simple
/// operation on one line.
/// But when none of these apply, a simple `for` loop is more idiomatic.
///
/// **Known problems:** None.
///
/// **Example:**
///
/// ```rust
/// let v = vec![0, 1, 2];
/// v.iter().for_each(|elem| {
/// println!("{}", elem);
/// })
/// ```
/// Use instead:
/// ```rust
/// let v = vec![0, 1, 2];
/// for elem in v.iter() {
/// println!("{}", elem);
/// }
/// ```
pub NEEDLESS_FOR_EACH,
restriction,
"using `for_each` where a `for` loop would be simpler"
}
declare_lint_pass!(NeedlessForEach => [NEEDLESS_FOR_EACH]);
impl LateLintPass<'_> for NeedlessForEach {
fn check_stmt(&mut self, cx: &LateContext<'tcx>, stmt: &'tcx Stmt<'_>) {
let expr = match stmt.kind {
StmtKind::Expr(expr) | StmtKind::Semi(expr) => expr,
StmtKind::Local(local) if local.init.is_some() => local.init.unwrap(),
_ => return,
};
// Max depth is set to 3 because we need to check the method chain length is just two.
let (method_names, arg_lists, _) = method_calls(expr, 3);
if_chain! {
// This assures the length of this method chain is two.
if let [for_each_args, iter_args] = arg_lists.as_slice();
if let Some(for_each_sym) = method_names.first();
if *for_each_sym == Symbol::intern("for_each");
if let Some(did) = cx.typeck_results().type_dependent_def_id(expr.hir_id);
if is_diagnostic_assoc_item(cx, did, sym::Iterator);
// Checks the type of the first method receiver is NOT a user defined type.
if has_iter_method(cx, cx.typeck_results().expr_ty(&iter_args[0])).is_some();
if let ExprKind::Closure(_, _, body_id, ..) = for_each_args[1].kind;
let body = cx.tcx.hir().body(body_id);
// Skip the lint if the body is not block because this is simpler than `for` loop.
// e.g. `v.iter().for_each(f)` is simpler and clearer than using `for` loop.
if let ExprKind::Block(..) = body.value.kind;
then {
let mut ret_collector = RetCollector::default();
ret_collector.visit_expr(&body.value);
// Skip the lint if `return` is used in `Loop` in order not to suggest using `'label`.
if ret_collector.ret_in_loop {
return;
}
// We can't use `Applicability::MachineApplicable` when the closure contains `return`
// because `Diagnostic::multipart_suggestion` doesn't work with multiple overlapped
// spans.
let mut applicability = if ret_collector.spans.is_empty() {
Applicability::MachineApplicable
} else {
Applicability::MaybeIncorrect
};
let mut suggs = vec![];
suggs.push((stmt.span, format!(
"for {} in {} {}",
snippet_with_applicability(cx, body.params[0].pat.span, "..", &mut applicability),
snippet_with_applicability(cx, for_each_args[0].span, "..", &mut applicability),
snippet_with_applicability(cx, body.value.span, "..", &mut applicability),
)));
for span in &ret_collector.spans {
suggs.push((*span, "return".to_string()));
}
span_lint_and_then(
cx,
NEEDLESS_FOR_EACH,
stmt.span,
"needless use of `for_each`",
|diag| {
diag.multipart_suggestion("try", suggs, applicability);
// `Diagnostic::multipart_suggestion` ignores the second and subsequent overlapped spans,
// so `span_note` is needed here even though `suggs` includes the replacements.
for span in ret_collector.spans {
diag.span_note(span, "replace `return` with `continue`");
}
}
)
}
}
}
}
/// This type plays two roles.
/// 1. Collect spans of `return` in the closure body.
/// 2. Detect use of `return` in `Loop` in the closure body.
///
/// NOTE: The functionality of this type is similar to
/// [`crate::utilts::visitors::find_all_ret_expressions`], but we can't use
/// `find_all_ret_expressions` instead of this type. The reasons are:
/// 1. `find_all_ret_expressions` passes the argument of `ExprKind::Ret` to a callback, but what we
/// need here is `ExprKind::Ret` itself.
/// 2. We can't trace current loop depth with `find_all_ret_expressions`.
#[derive(Default)]
struct RetCollector {
spans: Vec<Span>,
ret_in_loop: bool,
loop_depth: u16,
}
impl<'tcx> Visitor<'tcx> for RetCollector {
type Map = Map<'tcx>;
fn visit_expr(&mut self, expr: &Expr<'_>) {
match expr.kind {
ExprKind::Ret(..) => {
if self.loop_depth > 0 && !self.ret_in_loop {
self.ret_in_loop = true
}
self.spans.push(expr.span)
},
ExprKind::Loop(..) => {
self.loop_depth += 1;
walk_expr(self, expr);
self.loop_depth -= 1;
return;
},
_ => {},
}
walk_expr(self, expr);
}
fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
NestedVisitorMap::None
}
}

126
tests/ui/excessive_for_each.rs
View File

@ -1,126 +0,0 @@
#![warn(clippy::excessive_for_each)]
#![allow(clippy::needless_return)]
use std::collections::*;
fn main() {
// Should trigger this lint: Vec.
let vec: Vec<i32> = Vec::new();
let mut acc = 0;
vec.iter().for_each(|v| {
acc += v;
});
// Should trigger this lint: &Vec.
let vec_ref = &vec;
vec_ref.iter().for_each(|v| {
acc += v;
});
// Should trigger this lint: VecDeque.
let vec_deq: VecDeque<i32> = VecDeque::new();
vec_deq.iter().for_each(|v| {
acc += v;
});
// Should trigger this lint: LinkedList.
let list: LinkedList<i32> = LinkedList::new();
list.iter().for_each(|v| {
acc += v;
});
// Should trigger this lint: HashMap.
let mut hash_map: HashMap<i32, i32> = HashMap::new();
hash_map.iter().for_each(|(k, v)| {
acc += k + v;
});
hash_map.iter_mut().for_each(|(k, v)| {
acc += *k + *v;
});
hash_map.keys().for_each(|k| {
acc += k;
});
hash_map.values().for_each(|v| {
acc += v;
});
// Should trigger this lint: HashSet.
let hash_set: HashSet<i32> = HashSet::new();
hash_set.iter().for_each(|v| {
acc += v;
});
// Should trigger this lint: BTreeSet.
let btree_set: BTreeSet<i32> = BTreeSet::new();
btree_set.iter().for_each(|v| {
acc += v;
});
// Should trigger this lint: BinaryHeap.
let binary_heap: BinaryHeap<i32> = BinaryHeap::new();
binary_heap.iter().for_each(|v| {
acc += v;
});
// Should trigger this lint: Array.
let s = [1, 2, 3];
s.iter().for_each(|v| {
acc += v;
});
// Should trigger this lint. Slice.
vec.as_slice().iter().for_each(|v| {
acc += v;
});
// Should trigger this lint with notes that say "change `return` to `continue`".
vec.iter().for_each(|v| {
if *v == 10 {
return;
} else {
println!("{}", v);
}
});
// Should NOT trigger this lint in case `return` is used in `Loop` of the closure.
vec.iter().for_each(|v| {
for i in 0..*v {
if i == 10 {
return;
} else {
println!("{}", v);
}
}
if *v == 20 {
return;
} else {
println!("{}", v);
}
});
// Should NOT trigger this lint in case `for_each` follows long iterator chain.
vec.iter().chain(vec.iter()).for_each(|v| println!("{}", v));
// Should NOT trigger this lint in case a `for_each` argument is not closure.
fn print(x: &i32) {
println!("{}", x);
}
vec.iter().for_each(print);
// Should NOT trigger this lint in case the receiver of `iter` is a user defined type.
let my_collection = MyCollection { v: vec![] };
my_collection.iter().for_each(|v| println!("{}", v));
// Should NOT trigger this lint in case the closure body is not a `ExprKind::Block`.
vec.iter().for_each(|x| acc += x);
}
struct MyCollection {
v: Vec<i32>,
}
impl MyCollection {
fn iter(&self) -> impl Iterator<Item = &i32> {
self.v.iter()
}
}

126
tests/ui/excessive_for_each.stderr
View File

@ -1,126 +0,0 @@
error: excessive use of `for_each`
--> $DIR/excessive_for_each.rs:10:5
|
LL | / vec.iter().for_each(|v| {
LL | | acc += v;
LL | | });
| |______^ help: try: `for v in vec.iter() { .. }`
|
= note: `-D clippy::excessive-for-each` implied by `-D warnings`
error: excessive use of `for_each`
--> $DIR/excessive_for_each.rs:16:5
|
LL | / vec_ref.iter().for_each(|v| {
LL | | acc += v;
LL | | });
| |______^ help: try: `for v in vec_ref.iter() { .. }`
error: excessive use of `for_each`
--> $DIR/excessive_for_each.rs:22:5
|
LL | / vec_deq.iter().for_each(|v| {
LL | | acc += v;
LL | | });
| |______^ help: try: `for v in vec_deq.iter() { .. }`
error: excessive use of `for_each`
--> $DIR/excessive_for_each.rs:28:5
|
LL | / list.iter().for_each(|v| {
LL | | acc += v;
LL | | });
| |______^ help: try: `for v in list.iter() { .. }`
error: excessive use of `for_each`
--> $DIR/excessive_for_each.rs:34:5
|
LL | / hash_map.iter().for_each(|(k, v)| {
LL | | acc += k + v;
LL | | });
| |______^ help: try: `for (k, v) in hash_map.iter() { .. }`
error: excessive use of `for_each`
--> $DIR/excessive_for_each.rs:37:5
|
LL | / hash_map.iter_mut().for_each(|(k, v)| {
LL | | acc += *k + *v;
LL | | });
| |______^ help: try: `for (k, v) in hash_map.iter_mut() { .. }`
error: excessive use of `for_each`
--> $DIR/excessive_for_each.rs:40:5
|
LL | / hash_map.keys().for_each(|k| {
LL | | acc += k;
LL | | });
| |______^ help: try: `for k in hash_map.keys() { .. }`
error: excessive use of `for_each`
--> $DIR/excessive_for_each.rs:43:5
|
LL | / hash_map.values().for_each(|v| {
LL | | acc += v;
LL | | });
| |______^ help: try: `for v in hash_map.values() { .. }`
error: excessive use of `for_each`
--> $DIR/excessive_for_each.rs:49:5
|
LL | / hash_set.iter().for_each(|v| {
LL | | acc += v;
LL | | });
| |______^ help: try: `for v in hash_set.iter() { .. }`
error: excessive use of `for_each`
--> $DIR/excessive_for_each.rs:55:5
|
LL | / btree_set.iter().for_each(|v| {
LL | | acc += v;
LL | | });
| |______^ help: try: `for v in btree_set.iter() { .. }`
error: excessive use of `for_each`
--> $DIR/excessive_for_each.rs:61:5
|
LL | / binary_heap.iter().for_each(|v| {
LL | | acc += v;
LL | | });
| |______^ help: try: `for v in binary_heap.iter() { .. }`
error: excessive use of `for_each`
--> $DIR/excessive_for_each.rs:67:5
|
LL | / s.iter().for_each(|v| {
LL | | acc += v;
LL | | });
| |______^ help: try: `for v in s.iter() { .. }`
error: excessive use of `for_each`
--> $DIR/excessive_for_each.rs:72:5
|
LL | / vec.as_slice().iter().for_each(|v| {
LL | | acc += v;
LL | | });
| |______^ help: try: `for v in vec.as_slice().iter() { .. }`
error: excessive use of `for_each`
--> $DIR/excessive_for_each.rs:77:5
|
LL | / vec.iter().for_each(|v| {
LL | | if *v == 10 {
LL | | return;
LL | | } else {
LL | | println!("{}", v);
LL | | }
LL | | });
| |______^ help: try: `for v in vec.iter() { .. }`
|
note: change `return` to `continue` in the loop body
--> $DIR/excessive_for_each.rs:79:13
|
LL | return;
| ^^^^^^
error: aborting due to 14 previous errors

99
tests/ui/needless_for_each_fixable.fixed Normal file
View File

@ -0,0 +1,99 @@
// run-rustfix
#![warn(clippy::needless_for_each)]
#![allow(unused, clippy::needless_return, clippy::match_single_binding)]
use std::collections::HashMap;
fn should_lint() {
let v: Vec<i32> = Vec::new();
let mut acc = 0;
for elem in v.iter() {
acc += elem;
}
for elem in v.into_iter() {
acc += elem;
}
let mut hash_map: HashMap<i32, i32> = HashMap::new();
for (k, v) in hash_map.iter() {
acc += k + v;
}
for (k, v) in hash_map.iter_mut() {
acc += *k + *v;
}
for k in hash_map.keys() {
acc += k;
}
for v in hash_map.values() {
acc += v;
}
fn my_vec() -> Vec<i32> {
Vec::new()
}
for elem in my_vec().iter() {
acc += elem;
}
}
fn should_not_lint() {
let v: Vec<i32> = Vec::new();
let mut acc = 0;
// `for_each` argument is not closure.
fn print(x: &i32) {
println!("{}", x);
}
v.iter().for_each(print);
// `for_each` follows long iterator chain.
v.iter().chain(v.iter()).for_each(|v| println!("{}", v));
v.as_slice().iter().for_each(|v| {
acc += v;
});
// `return` is used in `Loop` of the closure.
v.iter().for_each(|v| {
for i in 0..*v {
if i == 10 {
return;
} else {
println!("{}", v);
}
}
if *v == 20 {
return;
} else {
println!("{}", v);
}
});
// User defined type.
struct MyStruct {
v: Vec<i32>,
}
impl MyStruct {
fn iter(&self) -> impl Iterator<Item = &i32> {
self.v.iter()
}
}
let s = MyStruct { v: Vec::new() };
s.iter().for_each(|elem| {
acc += elem;
});
// Previously transformed iterator variable.
let it = v.iter();
it.chain(v.iter()).for_each(|elem| {
acc += elem;
});
// `for_each` is not directly in a statement.
match 1 {
_ => v.iter().for_each(|elem| {
acc += elem;
}),
}
}
fn main() {}

99
tests/ui/needless_for_each_fixable.rs Normal file
View File

@ -0,0 +1,99 @@
// run-rustfix
#![warn(clippy::needless_for_each)]
#![allow(unused, clippy::needless_return, clippy::match_single_binding)]
use std::collections::HashMap;
fn should_lint() {
let v: Vec<i32> = Vec::new();
let mut acc = 0;
v.iter().for_each(|elem| {
acc += elem;
});
v.into_iter().for_each(|elem| {
acc += elem;
});
let mut hash_map: HashMap<i32, i32> = HashMap::new();
hash_map.iter().for_each(|(k, v)| {
acc += k + v;
});
hash_map.iter_mut().for_each(|(k, v)| {
acc += *k + *v;
});
hash_map.keys().for_each(|k| {
acc += k;
});
hash_map.values().for_each(|v| {
acc += v;
});
fn my_vec() -> Vec<i32> {
Vec::new()
}
my_vec().iter().for_each(|elem| {
acc += elem;
});
}
fn should_not_lint() {
let v: Vec<i32> = Vec::new();
let mut acc = 0;
// `for_each` argument is not closure.
fn print(x: &i32) {
println!("{}", x);
}
v.iter().for_each(print);
// `for_each` follows long iterator chain.
v.iter().chain(v.iter()).for_each(|v| println!("{}", v));
v.as_slice().iter().for_each(|v| {
acc += v;
});
// `return` is used in `Loop` of the closure.
v.iter().for_each(|v| {
for i in 0..*v {
if i == 10 {
return;
} else {
println!("{}", v);
}
}
if *v == 20 {
return;
} else {
println!("{}", v);
}
});
// User defined type.
struct MyStruct {
v: Vec<i32>,
}
impl MyStruct {
fn iter(&self) -> impl Iterator<Item = &i32> {
self.v.iter()
}
}
let s = MyStruct { v: Vec::new() };
s.iter().for_each(|elem| {
acc += elem;
});
// Previously transformed iterator variable.
let it = v.iter();
it.chain(v.iter()).for_each(|elem| {
acc += elem;
});
// `for_each` is not directly in a statement.
match 1 {
_ => v.iter().for_each(|elem| {
acc += elem;
}),
}
}
fn main() {}

108
tests/ui/needless_for_each_fixable.stderr Normal file
View File

@ -0,0 +1,108 @@
error: needless use of `for_each`
--> $DIR/needless_for_each_fixable.rs:10:5
|
LL | / v.iter().for_each(|elem| {
LL | | acc += elem;
LL | | });
| |_______^
|
= note: `-D clippy::needless-for-each` implied by `-D warnings`
help: try
|
LL | for elem in v.iter() {
LL | acc += elem;
LL | }
|
error: needless use of `for_each`
--> $DIR/needless_for_each_fixable.rs:13:5
|
LL | / v.into_iter().for_each(|elem| {
LL | | acc += elem;
LL | | });
| |_______^
|
help: try
|
LL | for elem in v.into_iter() {
LL | acc += elem;
LL | }
|
error: needless use of `for_each`
--> $DIR/needless_for_each_fixable.rs:18:5
|
LL | / hash_map.iter().for_each(|(k, v)| {
LL | | acc += k + v;
LL | | });
| |_______^
|
help: try
|
LL | for (k, v) in hash_map.iter() {
LL | acc += k + v;
LL | }
|
error: needless use of `for_each`
--> $DIR/needless_for_each_fixable.rs:21:5
|
LL | / hash_map.iter_mut().for_each(|(k, v)| {
LL | | acc += *k + *v;
LL | | });
| |_______^
|
help: try
|
LL | for (k, v) in hash_map.iter_mut() {
LL | acc += *k + *v;
LL | }
|
error: needless use of `for_each`
--> $DIR/needless_for_each_fixable.rs:24:5
|
LL | / hash_map.keys().for_each(|k| {
LL | | acc += k;
LL | | });
| |_______^
|
help: try
|
LL | for k in hash_map.keys() {
LL | acc += k;
LL | }
|
error: needless use of `for_each`
--> $DIR/needless_for_each_fixable.rs:27:5
|
LL | / hash_map.values().for_each(|v| {
LL | | acc += v;
LL | | });
| |_______^
|
help: try
|
LL | for v in hash_map.values() {
LL | acc += v;
LL | }
|
error: needless use of `for_each`
--> $DIR/needless_for_each_fixable.rs:34:5
|
LL | / my_vec().iter().for_each(|elem| {
LL | | acc += elem;
LL | | });
| |_______^
|
help: try
|
LL | for elem in my_vec().iter() {
LL | acc += elem;
LL | }
|
error: aborting due to 7 previous errors

14
tests/ui/needless_for_each_unfixable.rs Normal file
View File

@ -0,0 +1,14 @@
#![warn(clippy::needless_for_each)]
#![allow(clippy::needless_return)]
fn main() {
let v: Vec<i32> = Vec::new();
// This is unfixable because the closure includes `return`.
v.iter().for_each(|v| {
if *v == 10 {
return;
} else {
println!("{}", v);
}
});
}

30
tests/ui/needless_for_each_unfixable.stderr Normal file
View File

@ -0,0 +1,30 @@
error: needless use of `for_each`
--> $DIR/needless_for_each_unfixable.rs:7:5
|
LL | / v.iter().for_each(|v| {
LL | | if *v == 10 {
LL | | return;
LL | | } else {
LL | | println!("{}", v);
LL | | }
LL | | });
| |_______^
|
= note: `-D clippy::needless-for-each` implied by `-D warnings`
note: replace `return` with `continue`
--> $DIR/needless_for_each_unfixable.rs:9:13
|
LL | return;
| ^^^^^^
help: try
|
LL | for v in v.iter() {
LL | if *v == 10 {
LL | return;
LL | } else {
LL | println!("{}", v);
LL | }
...
error: aborting due to previous error