From 5bb0f16552e8eab45025c7f4581b8ae5edca3219 Mon Sep 17 00:00:00 2001 From: Yoshitomo Nakanishi Date: Wed, 10 Feb 2021 00:53:53 +0900 Subject: [PATCH] Trigger the lint iff exposure's body is ExprKind::Block. --- .../src/methods/excessive_for_each.rs | 3 +- tests/ui/excessive_for_each.rs | 56 +++++++-- tests/ui/excessive_for_each.stderr | 114 +++++++++++------- 3 files changed, 115 insertions(+), 58 deletions(-) diff --git a/clippy_lints/src/methods/excessive_for_each.rs b/clippy_lints/src/methods/excessive_for_each.rs index 6b3a11044f0..f3e9e2400f1 100644 --- a/clippy_lints/src/methods/excessive_for_each.rs +++ b/clippy_lints/src/methods/excessive_for_each.rs @@ -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); diff --git a/tests/ui/excessive_for_each.rs b/tests/ui/excessive_for_each.rs index 12c87782d97..1c8e450398e 100644 --- a/tests/ui/excessive_for_each.rs +++ b/tests/ui/excessive_for_each.rs @@ -6,45 +6,72 @@ fn main() { // Should trigger this lint: Vec. let vec: Vec = 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 = 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 = 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 = 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 = 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 = 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 = 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| { @@ -83,6 +110,9 @@ fn print(x: &i32) { // 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 { diff --git a/tests/ui/excessive_for_each.stderr b/tests/ui/excessive_for_each.stderr index 026b14a5899..c4b66e3a034 100644 --- a/tests/ui/excessive_for_each.stderr +++ b/tests/ui/excessive_for_each.stderr @@ -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 { @@ -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 { @@ -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; | ^^^^^^ -- 2.44.0