Skip to content

Commit

Permalink
Trigger the lint iff exposure's body is ExprKind::Block.
Browse files Browse the repository at this point in the history
  • Loading branch information
Y-Nak committed Feb 9, 2021
1 parent 7fbdd8e commit 8f50346
Show file tree
Hide file tree
Showing 3 changed files with 115 additions and 58 deletions.
3 changes: 2 additions & 1 deletion clippy_lints/src/methods/excessive_for_each.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,9 @@ pub(super) fn lint(cx: &LateContext<'_>, expr: &'tcx Expr<'_>, args: &[&[Expr<'_
if match_trait_method(cx, expr, &paths::ITERATOR);
if is_target_ty(cx, cx.typeck_results().expr_ty(iter_receiver));
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 body = cx.tcx.hir().body(body_id);
let mut ret_span_collector = RetSpanCollector::new();
ret_span_collector.visit_expr(&body.value);

Expand Down
56 changes: 43 additions & 13 deletions tests/ui/excessive_for_each.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,45 +6,72 @@ use std::collections::*;
fn main() {
// Should trigger this lint: Vec.
let vec: Vec<i32> = Vec::new();
vec.iter().for_each(|v| println!("{}", v));
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| println!("{}", v));
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| println!("{}", v));
vec_deq.iter().for_each(|v| {
acc += v;
});

// Should trigger this lint: LinkedList.
let list: LinkedList<i32> = LinkedList::new();
list.iter().for_each(|v| println!("{}", v));
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)| println!("{}: {}", k, v));
hash_map.iter_mut().for_each(|(k, v)| println!("{}: {}", k, v));
hash_map.keys().for_each(|k| println!("{}", k));
hash_map.values().for_each(|v| println!("{}", v));
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| println!("{}", v));
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| println!("{}", v));
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| println!("{}", v));
binary_heap.iter().for_each(|v| {
acc += v;
});

// Should trigger this lint: Array.
let s = [1, 2, 3];
s.iter().for_each(|v| println!("{}", v));
s.iter().for_each(|v| {
acc += v;
});

// Should trigger this lint. Slice.
vec.as_slice().iter().for_each(|v| println!("{}", v));
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| {
Expand Down Expand Up @@ -83,6 +110,9 @@ fn main() {
// 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 {
Expand Down
114 changes: 70 additions & 44 deletions tests/ui/excessive_for_each.stderr
Original file line number Diff line number Diff line change
@@ -1,85 +1,111 @@
error: excessive use of `for_each`
--> $DIR/excessive_for_each.rs:9:5
--> $DIR/excessive_for_each.rs:10:5
|
LL | vec.iter().for_each(|v| println!("{}", v));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for v in vec.iter() { .. }`
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:13:5
--> $DIR/excessive_for_each.rs:16:5
|
LL | vec_ref.iter().for_each(|v| println!("{}", v));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for v in vec_ref.iter() { .. }`
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:17:5
--> $DIR/excessive_for_each.rs:22:5
|
LL | vec_deq.iter().for_each(|v| println!("{}", v));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for v in vec_deq.iter() { .. }`
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:21:5
--> $DIR/excessive_for_each.rs:28:5
|
LL | list.iter().for_each(|v| println!("{}", v));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for v in list.iter() { .. }`
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:25:5
--> $DIR/excessive_for_each.rs:34:5
|
LL | hash_map.iter().for_each(|(k, v)| println!("{}: {}", k, v));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for (k, v) in hash_map.iter() { .. }`
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:26:5
--> $DIR/excessive_for_each.rs:37:5
|
LL | hash_map.iter_mut().for_each(|(k, v)| println!("{}: {}", k, v));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for (k, v) in hash_map.iter_mut() { .. }`
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:27:5
--> $DIR/excessive_for_each.rs:40:5
|
LL | hash_map.keys().for_each(|k| println!("{}", k));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for k in hash_map.keys() { .. }`
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:28:5
--> $DIR/excessive_for_each.rs:43:5
|
LL | hash_map.values().for_each(|v| println!("{}", v));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for v in hash_map.values() { .. }`
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:32:5
--> $DIR/excessive_for_each.rs:49:5
|
LL | hash_set.iter().for_each(|v| println!("{}", v));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for v in hash_set.iter() { .. }`
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:36:5
--> $DIR/excessive_for_each.rs:55:5
|
LL | btree_set.iter().for_each(|v| println!("{}", v));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for v in btree_set.iter() { .. }`
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:40:5
--> $DIR/excessive_for_each.rs:61:5
|
LL | binary_heap.iter().for_each(|v| println!("{}", v));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for v in binary_heap.iter() { .. }`
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:44:5
--> $DIR/excessive_for_each.rs:67:5
|
LL | s.iter().for_each(|v| println!("{}", v));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for v in s.iter() { .. }`
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:47:5
--> $DIR/excessive_for_each.rs:72:5
|
LL | vec.as_slice().iter().for_each(|v| println!("{}", v));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for v in vec.as_slice().iter() { .. }`
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:50:5
--> $DIR/excessive_for_each.rs:77:5
|
LL | / vec.iter().for_each(|v| {
LL | | if *v == 10 {
Expand All @@ -91,13 +117,13 @@ LL | | });
| |______^ help: try: `for v in vec.iter() { .. }`
|
note: change `return` to `continue` in the loop body
--> $DIR/excessive_for_each.rs:52:13
--> $DIR/excessive_for_each.rs:79:13
|
LL | return;
| ^^^^^^

error: excessive use of `for_each`
--> $DIR/excessive_for_each.rs:59:5
--> $DIR/excessive_for_each.rs:86:5
|
LL | / vec.iter().for_each(|v| {
LL | | for i in 0..*v {
Expand All @@ -109,12 +135,12 @@ LL | | });
| |______^ help: try: `'outer: for v in vec.iter() { .. }`
|
note: change `return` to `continue 'outer` in the loop body
--> $DIR/excessive_for_each.rs:62:17
--> $DIR/excessive_for_each.rs:89:17
|
LL | return;
| ^^^^^^
note: change `return` to `continue` in the loop body
--> $DIR/excessive_for_each.rs:68:13
--> $DIR/excessive_for_each.rs:95:13
|
LL | return;
| ^^^^^^
Expand Down

0 comments on commit 8f50346

Please sign in to comment.