From 5029dc805fa1b8c58988cad119a45a6d51bcdaad Mon Sep 17 00:00:00 2001
From: Yoshitomo Nakanishi <yurayura.rounin.3@gmail.com>
Date: Tue, 9 Feb 2021 00:24:23 +0900
Subject: [PATCH 01/14] New Lint: excessive_for_each

---
 CHANGELOG.md                                  |   1 +
 clippy_lints/src/iter_for_each.rs             |   0
 clippy_lints/src/lib.rs                       |   3 +
 .../src/methods/excessive_for_each.rs         | 149 ++++++++++++++++++
 clippy_lints/src/methods/mod.rs               |  29 ++++
 tests/ui/excessive_for_each.rs                |  96 +++++++++++
 tests/ui/excessive_for_each.stderr            | 123 +++++++++++++++
 7 files changed, 401 insertions(+)
 create mode 100644 clippy_lints/src/iter_for_each.rs
 create mode 100644 clippy_lints/src/methods/excessive_for_each.rs
 create mode 100644 tests/ui/excessive_for_each.rs
 create mode 100644 tests/ui/excessive_for_each.stderr

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 681ecd6832a..c35ab6c94bf 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2192,6 +2192,7 @@ 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
diff --git a/clippy_lints/src/iter_for_each.rs b/clippy_lints/src/iter_for_each.rs
new file mode 100644
index 00000000000..e69de29bb2d
diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
index d37e229fb57..8fb3bfdafc9 100644
--- a/clippy_lints/src/lib.rs
+++ b/clippy_lints/src/lib.rs
@@ -781,6 +781,7 @@ 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,
@@ -1581,6 +1582,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&methods::CHARS_NEXT_CMP),
         LintId::of(&methods::CLONE_DOUBLE_REF),
         LintId::of(&methods::CLONE_ON_COPY),
+        LintId::of(&methods::EXCESSIVE_FOR_EACH),
         LintId::of(&methods::EXPECT_FUN_CALL),
         LintId::of(&methods::FILTER_MAP_IDENTITY),
         LintId::of(&methods::FILTER_NEXT),
@@ -1797,6 +1799,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&methods::BYTES_NTH),
         LintId::of(&methods::CHARS_LAST_CMP),
         LintId::of(&methods::CHARS_NEXT_CMP),
+        LintId::of(&methods::EXCESSIVE_FOR_EACH),
         LintId::of(&methods::FROM_ITER_INSTEAD_OF_COLLECT),
         LintId::of(&methods::INTO_ITER_ON_REF),
         LintId::of(&methods::ITER_CLONED_COLLECT),
diff --git a/clippy_lints/src/methods/excessive_for_each.rs b/clippy_lints/src/methods/excessive_for_each.rs
new file mode 100644
index 00000000000..36f92d5b95f
--- /dev/null
+++ b/clippy_lints/src/methods/excessive_for_each.rs
@@ -0,0 +1,149 @@
+use rustc_errors::Applicability;
+use rustc_hir::{
+    intravisit::{walk_expr, NestedVisitorMap, Visitor},
+    Expr, ExprKind,
+};
+use rustc_lint::LateContext;
+use rustc_middle::{hir::map::Map, ty, ty::Ty};
+use rustc_span::source_map::Span;
+
+use crate::utils::{match_trait_method, match_type, paths, snippet, span_lint_and_then};
+
+use if_chain::if_chain;
+
+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 match_trait_method(cx, expr, &paths::ITERATOR);
+        if !match_trait_method(cx, for_each_receiver, &paths::ITERATOR);
+        if is_target_ty(cx, cx.typeck_results().expr_ty(iter_receiver));
+        if let ExprKind::Closure(_, _, body_id, ..) = for_each_arg.kind;
+        then {
+            let body = cx.tcx.hir().body(body_id);
+            let mut ret_span_collector = RetSpanCollector::new();
+            ret_span_collector.visit_expr(&body.value);
+
+            let label = "'outer";
+            let loop_label = if ret_span_collector.need_label {
+                format!("{}: ", label)
+            } else {
+                "".to_string()
+            };
+            let sugg =
+                format!("{}for {} in {} {{ .. }}", loop_label, snippet(cx, body.params[0].pat.span, ""), snippet(cx, for_each_receiver.span, ""));
+
+            let mut notes = vec![];
+            for (span, need_label) in ret_span_collector.spans {
+                let cont_label = if need_label {
+                    format!(" {}", label)
+                } else {
+                    "".to_string()
+                };
+                let note = format!("change `return` to `continue{}` in the loop body", cont_label);
+                notes.push((span, note));
+            }
+
+            span_lint_and_then(cx,
+                      super::EXCESSIVE_FOR_EACH,
+                      expr.span,
+                      "excessive use of `for_each`",
+                      |diag| {
+                          diag.span_suggestion(expr.span, "try", sugg, Applicability::HasPlaceholders);
+                          for note in notes {
+                              diag.span_note(note.0, &note.1);
+                          }
+                      }
+                );
+        }
+    }
+}
+
+type PathSegment = &'static [&'static str];
+
+const TARGET_ITER_RECEIVER_TY: &[PathSegment] = &[
+    &paths::VEC,
+    &paths::VEC_DEQUE,
+    &paths::LINKED_LIST,
+    &paths::HASHMAP,
+    &paths::BTREEMAP,
+    &paths::HASHSET,
+    &paths::BTREESET,
+    &paths::BINARY_HEAP,
+];
+
+fn is_target_ty(cx: &LateContext<'_>, expr_ty: Ty<'_>) -> bool {
+    let expr_ty = expr_ty.peel_refs();
+    for target in TARGET_ITER_RECEIVER_TY {
+        if match_type(cx, expr_ty, target) {
+            return true;
+        }
+    }
+
+    if_chain! {
+        if matches!(expr_ty.kind(), ty::Slice(_) | ty::Array(..));
+        then {
+            return true;
+        }
+    }
+
+    false
+}
+
+/// Collect spans of `return` in the closure body.
+struct RetSpanCollector {
+    spans: Vec<(Span, bool)>,
+    loop_depth: u16,
+    need_label: bool,
+}
+
+impl RetSpanCollector {
+    fn new() -> Self {
+        Self {
+            spans: Vec::new(),
+            loop_depth: 0,
+            need_label: false,
+        }
+    }
+}
+
+impl<'tcx> Visitor<'tcx> for RetSpanCollector {
+    type Map = Map<'tcx>;
+
+    fn visit_expr(&mut self, expr: &Expr<'_>) {
+        match expr.kind {
+            ExprKind::Ret(..) => {
+                if self.loop_depth > 0 && !self.need_label {
+                    self.need_label = true
+                }
+
+                self.spans.push((expr.span, self.loop_depth > 0))
+            },
+
+            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
+    }
+}
diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs
index fccdee07877..058140fddb8 100644
--- a/clippy_lints/src/methods/mod.rs
+++ b/clippy_lints/src/methods/mod.rs
@@ -974,6 +974,33 @@ declare_clippy_lint! {
     "using `.skip(x).next()` on an iterator"
 }
 
+declare_clippy_lint! {
+    /// **What it does:** Checks for use of `obj.method().for_each(closure)` if obj doesn't
+    /// implelement `Iterator` and `method()` returns `Impl Iterator` type.
+    ///
+    /// **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,
+    style,
+    "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`
@@ -1661,6 +1688,7 @@ impl_lint_pass!(Methods => [
     ITER_NTH_ZERO,
     BYTES_NTH,
     ITER_SKIP_NEXT,
+    EXCESSIVE_FOR_EACH,
     GET_UNWRAP,
     STRING_EXTEND_CHARS,
     ITER_CLONED_COLLECT,
@@ -1807,6 +1835,7 @@ 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),
             _ => {},
         }
 
diff --git a/tests/ui/excessive_for_each.rs b/tests/ui/excessive_for_each.rs
new file mode 100644
index 00000000000..12c87782d97
--- /dev/null
+++ b/tests/ui/excessive_for_each.rs
@@ -0,0 +1,96 @@
+#![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();
+    vec.iter().for_each(|v| println!("{}", v));
+
+    // Should trigger this lint: &Vec.
+    let vec_ref = &vec;
+    vec_ref.iter().for_each(|v| println!("{}", v));
+
+    // Should trigger this lint: VecDeque.
+    let vec_deq: VecDeque<i32> = VecDeque::new();
+    vec_deq.iter().for_each(|v| println!("{}", v));
+
+    // Should trigger this lint: LinkedList.
+    let list: LinkedList<i32> = LinkedList::new();
+    list.iter().for_each(|v| println!("{}", 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));
+
+    // Should trigger this lint: HashSet.
+    let hash_set: HashSet<i32> = HashSet::new();
+    hash_set.iter().for_each(|v| println!("{}", v));
+
+    // Should trigger this lint: BTreeSet.
+    let btree_set: BTreeSet<i32> = BTreeSet::new();
+    btree_set.iter().for_each(|v| println!("{}", v));
+
+    // Should trigger this lint: BinaryHeap.
+    let binary_heap: BinaryHeap<i32> = BinaryHeap::new();
+    binary_heap.iter().for_each(|v| println!("{}", v));
+
+    // Should trigger this lint: Array.
+    let s = [1, 2, 3];
+    s.iter().for_each(|v| println!("{}", v));
+
+    // Should trigger this lint. Slice.
+    vec.as_slice().iter().for_each(|v| println!("{}", 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 trigger this lint with notes that say "change `return` to `continue 'outer`".
+    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));
+}
+
+struct MyCollection {
+    v: Vec<i32>,
+}
+
+impl MyCollection {
+    fn iter(&self) -> impl Iterator<Item = &i32> {
+        self.v.iter()
+    }
+}
diff --git a/tests/ui/excessive_for_each.stderr b/tests/ui/excessive_for_each.stderr
new file mode 100644
index 00000000000..026b14a5899
--- /dev/null
+++ b/tests/ui/excessive_for_each.stderr
@@ -0,0 +1,123 @@
+error: excessive use of `for_each`
+  --> $DIR/excessive_for_each.rs:9:5
+   |
+LL |     vec.iter().for_each(|v| println!("{}", v));
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ 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
+   |
+LL |     vec_ref.iter().for_each(|v| println!("{}", v));
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for v in vec_ref.iter() { .. }`
+
+error: excessive use of `for_each`
+  --> $DIR/excessive_for_each.rs:17:5
+   |
+LL |     vec_deq.iter().for_each(|v| println!("{}", v));
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for v in vec_deq.iter() { .. }`
+
+error: excessive use of `for_each`
+  --> $DIR/excessive_for_each.rs:21:5
+   |
+LL |     list.iter().for_each(|v| println!("{}", v));
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for v in list.iter() { .. }`
+
+error: excessive use of `for_each`
+  --> $DIR/excessive_for_each.rs:25:5
+   |
+LL |     hash_map.iter().for_each(|(k, v)| println!("{}: {}", k, v));
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for (k, v) in hash_map.iter() { .. }`
+
+error: excessive use of `for_each`
+  --> $DIR/excessive_for_each.rs:26:5
+   |
+LL |     hash_map.iter_mut().for_each(|(k, v)| println!("{}: {}", k, v));
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for (k, v) in hash_map.iter_mut() { .. }`
+
+error: excessive use of `for_each`
+  --> $DIR/excessive_for_each.rs:27:5
+   |
+LL |     hash_map.keys().for_each(|k| println!("{}", k));
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for k in hash_map.keys() { .. }`
+
+error: excessive use of `for_each`
+  --> $DIR/excessive_for_each.rs:28:5
+   |
+LL |     hash_map.values().for_each(|v| println!("{}", v));
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for v in hash_map.values() { .. }`
+
+error: excessive use of `for_each`
+  --> $DIR/excessive_for_each.rs:32:5
+   |
+LL |     hash_set.iter().for_each(|v| println!("{}", v));
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for v in hash_set.iter() { .. }`
+
+error: excessive use of `for_each`
+  --> $DIR/excessive_for_each.rs:36:5
+   |
+LL |     btree_set.iter().for_each(|v| println!("{}", v));
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for v in btree_set.iter() { .. }`
+
+error: excessive use of `for_each`
+  --> $DIR/excessive_for_each.rs:40:5
+   |
+LL |     binary_heap.iter().for_each(|v| println!("{}", v));
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for v in binary_heap.iter() { .. }`
+
+error: excessive use of `for_each`
+  --> $DIR/excessive_for_each.rs:44:5
+   |
+LL |     s.iter().for_each(|v| println!("{}", v));
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for v in s.iter() { .. }`
+
+error: excessive use of `for_each`
+  --> $DIR/excessive_for_each.rs:47:5
+   |
+LL |     vec.as_slice().iter().for_each(|v| println!("{}", v));
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for v in vec.as_slice().iter() { .. }`
+
+error: excessive use of `for_each`
+  --> $DIR/excessive_for_each.rs:50: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:52:13
+   |
+LL |             return;
+   |             ^^^^^^
+
+error: excessive use of `for_each`
+  --> $DIR/excessive_for_each.rs:59:5
+   |
+LL | /     vec.iter().for_each(|v| {
+LL | |         for i in 0..*v {
+LL | |             if i == 10 {
+LL | |                 return;
+...  |
+LL | |         }
+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
+   |
+LL |                 return;
+   |                 ^^^^^^
+note: change `return` to `continue` in the loop body
+  --> $DIR/excessive_for_each.rs:68:13
+   |
+LL |             return;
+   |             ^^^^^^
+
+error: aborting due to 15 previous errors
+

From 30952530c502acbfc40853f0392853469ece8024 Mon Sep 17 00:00:00 2001
From: Yoshitomo Nakanishi <yurayura.rounin.3@gmail.com>
Date: Tue, 9 Feb 2021 21:20:42 +0900
Subject: [PATCH 02/14] Fix codes that fails dogfood

---
 clippy_lints/src/matches.rs | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/clippy_lints/src/matches.rs b/clippy_lints/src/matches.rs
index 425124b78f4..8c6c30a1881 100644
--- a/clippy_lints/src/matches.rs
+++ b/clippy_lints/src/matches.rs
@@ -924,7 +924,7 @@ fn check_wild_err_arm<'tcx>(cx: &LateContext<'tcx>, ex: &Expr<'tcx>, arms: &[Arm
                     let mut ident_bind_name = String::from("_");
                     if !matching_wild {
                         // Looking for unused bindings (i.e.: `_e`)
-                        inner.iter().for_each(|pat| {
+                        for pat in inner.iter() {
                             if let PatKind::Binding(_, id, ident, None) = pat.kind {
                                 if ident.as_str().starts_with('_')
                                     && !LocalUsedVisitor::new(cx, id).check_expr(arm.body)
@@ -933,7 +933,7 @@ fn check_wild_err_arm<'tcx>(cx: &LateContext<'tcx>, ex: &Expr<'tcx>, arms: &[Arm
                                     matching_wild = true;
                                 }
                             }
-                        });
+                        }
                     }
                     if_chain! {
                         if matching_wild;

From 54a04711edb15ad0ce5e19f833a5172f651bb4c0 Mon Sep 17 00:00:00 2001
From: Yoshitomo Nakanishi <yurayura.rounin.3@gmail.com>
Date: Tue, 9 Feb 2021 23:36:20 +0900
Subject: [PATCH 03/14] Change a category of excessive_for_each: Style ->
 Restriction

---
 clippy_lints/src/lib.rs                        | 3 +--
 clippy_lints/src/methods/excessive_for_each.rs | 1 -
 clippy_lints/src/methods/mod.rs                | 2 +-
 3 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
index 8fb3bfdafc9..2f2ccdb310a 100644
--- a/clippy_lints/src/lib.rs
+++ b/clippy_lints/src/lib.rs
@@ -1314,6 +1314,7 @@ 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),
@@ -1582,7 +1583,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&methods::CHARS_NEXT_CMP),
         LintId::of(&methods::CLONE_DOUBLE_REF),
         LintId::of(&methods::CLONE_ON_COPY),
-        LintId::of(&methods::EXCESSIVE_FOR_EACH),
         LintId::of(&methods::EXPECT_FUN_CALL),
         LintId::of(&methods::FILTER_MAP_IDENTITY),
         LintId::of(&methods::FILTER_NEXT),
@@ -1799,7 +1799,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&methods::BYTES_NTH),
         LintId::of(&methods::CHARS_LAST_CMP),
         LintId::of(&methods::CHARS_NEXT_CMP),
-        LintId::of(&methods::EXCESSIVE_FOR_EACH),
         LintId::of(&methods::FROM_ITER_INSTEAD_OF_COLLECT),
         LintId::of(&methods::INTO_ITER_ON_REF),
         LintId::of(&methods::ITER_CLONED_COLLECT),
diff --git a/clippy_lints/src/methods/excessive_for_each.rs b/clippy_lints/src/methods/excessive_for_each.rs
index 36f92d5b95f..6b3a11044f0 100644
--- a/clippy_lints/src/methods/excessive_for_each.rs
+++ b/clippy_lints/src/methods/excessive_for_each.rs
@@ -26,7 +26,6 @@ pub(super) fn lint(cx: &LateContext<'_>, expr: &'tcx Expr<'_>, args: &[&[Expr<'_
 
     if_chain! {
         if match_trait_method(cx, expr, &paths::ITERATOR);
-        if !match_trait_method(cx, for_each_receiver, &paths::ITERATOR);
         if is_target_ty(cx, cx.typeck_results().expr_ty(iter_receiver));
         if let ExprKind::Closure(_, _, body_id, ..) = for_each_arg.kind;
         then {
diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs
index 058140fddb8..344c7f1bd9a 100644
--- a/clippy_lints/src/methods/mod.rs
+++ b/clippy_lints/src/methods/mod.rs
@@ -997,7 +997,7 @@ declare_clippy_lint! {
     /// }
     /// ```
     pub EXCESSIVE_FOR_EACH,
-    style,
+    restriction,
     "using `.iter().for_each(|x| {..})` when using `for` loop would work instead"
 }
 

From 5bb0f16552e8eab45025c7f4581b8ae5edca3219 Mon Sep 17 00:00:00 2001
From: Yoshitomo Nakanishi <yurayura.rounin.3@gmail.com>
Date: Wed, 10 Feb 2021 00:53:53 +0900
Subject: [PATCH 04/14] 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            | 134 +++++++++++-------
 3 files changed, 125 insertions(+), 68 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 @@ 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| {
@@ -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 {
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() { .. }`
-
-error: excessive use of `for_each`
-  --> $DIR/excessive_for_each.rs:21:5
-   |
-LL |     list.iter().for_each(|v| println!("{}", v));
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for v in list.iter() { .. }`
-
-error: excessive use of `for_each`
-  --> $DIR/excessive_for_each.rs:25:5
-   |
-LL |     hash_map.iter().for_each(|(k, v)| println!("{}: {}", k, v));
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for (k, v) in hash_map.iter() { .. }`
-
-error: excessive use of `for_each`
-  --> $DIR/excessive_for_each.rs:26:5
-   |
-LL |     hash_map.iter_mut().for_each(|(k, v)| println!("{}: {}", k, v));
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for (k, v) in hash_map.iter_mut() { .. }`
-
-error: excessive use of `for_each`
-  --> $DIR/excessive_for_each.rs:27:5
-   |
-LL |     hash_map.keys().for_each(|k| println!("{}", k));
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for k in hash_map.keys() { .. }`
+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 |     hash_map.values().for_each(|v| println!("{}", v));
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for v in hash_map.values() { .. }`
+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:32:5
+  --> $DIR/excessive_for_each.rs:34:5
    |
-LL |     hash_set.iter().for_each(|v| println!("{}", v));
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for v in hash_set.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:36:5
+  --> $DIR/excessive_for_each.rs:37:5
    |
-LL |     btree_set.iter().for_each(|v| println!("{}", v));
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for v in btree_set.iter() { .. }`
+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 |     binary_heap.iter().for_each(|v| println!("{}", v));
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for v in binary_heap.iter() { .. }`
+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:44:5
+  --> $DIR/excessive_for_each.rs:43:5
    |
-LL |     s.iter().for_each(|v| println!("{}", v));
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for v in s.iter() { .. }`
+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:47:5
+  --> $DIR/excessive_for_each.rs:49:5
    |
-LL |     vec.as_slice().iter().for_each(|v| println!("{}", v));
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for v in vec.as_slice().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:50:5
+  --> $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 {
@@ -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;
    |             ^^^^^^

From 90cbbb2da3989a437a36c075396331f81a0e3b30 Mon Sep 17 00:00:00 2001
From: Yoshitomo Nakanishi <yurayura.rounin.3@gmail.com>
Date: Thu, 11 Feb 2021 12:50:20 +0900
Subject: [PATCH 05/14] Avoid to suggest using label

---
 .../src/methods/excessive_for_each.rs         | 49 +++++++++----------
 tests/ui/excessive_for_each.rs                |  2 +-
 tests/ui/excessive_for_each.stderr            | 25 +---------
 3 files changed, 25 insertions(+), 51 deletions(-)

diff --git a/clippy_lints/src/methods/excessive_for_each.rs b/clippy_lints/src/methods/excessive_for_each.rs
index f3e9e2400f1..e1440e68327 100644
--- a/clippy_lints/src/methods/excessive_for_each.rs
+++ b/clippy_lints/src/methods/excessive_for_each.rs
@@ -31,26 +31,20 @@ pub(super) fn lint(cx: &LateContext<'_>, expr: &'tcx Expr<'_>, args: &[&[Expr<'_
         let body = cx.tcx.hir().body(body_id);
         if let ExprKind::Block(..) = body.value.kind;
         then {
-            let mut ret_span_collector = RetSpanCollector::new();
-            ret_span_collector.visit_expr(&body.value);
+            let mut ret_collector = RetCollector::new();
+            ret_collector.visit_expr(&body.value);
+
+            // Skip the lint if `return` is used in `Loop` to avoid a suggest using `'label`.
+            if ret_collector.ret_in_loop {
+                return;
+            }
 
-            let label = "'outer";
-            let loop_label = if ret_span_collector.need_label {
-                format!("{}: ", label)
-            } else {
-                "".to_string()
-            };
             let sugg =
-                format!("{}for {} in {} {{ .. }}", loop_label, snippet(cx, body.params[0].pat.span, ""), snippet(cx, for_each_receiver.span, ""));
+                format!("for {} in {} {{ .. }}", snippet(cx, body.params[0].pat.span, ""), snippet(cx, for_each_receiver.span, ""));
 
             let mut notes = vec![];
-            for (span, need_label) in ret_span_collector.spans {
-                let cont_label = if need_label {
-                    format!(" {}", label)
-                } else {
-                    "".to_string()
-                };
-                let note = format!("change `return` to `continue{}` in the loop body", cont_label);
+            for span in ret_collector.spans {
+                let note = format!("change `return` to `continue` in the loop body");
                 notes.push((span, note));
             }
 
@@ -100,34 +94,37 @@ fn is_target_ty(cx: &LateContext<'_>, expr_ty: Ty<'_>) -> bool {
     false
 }
 
-/// Collect spans of `return` in the closure body.
-struct RetSpanCollector {
-    spans: Vec<(Span, bool)>,
+/// 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.
+struct RetCollector {
+    spans: Vec<Span>,
+    ret_in_loop: bool,
+
     loop_depth: u16,
-    need_label: bool,
 }
 
-impl RetSpanCollector {
+impl RetCollector {
     fn new() -> Self {
         Self {
             spans: Vec::new(),
+            ret_in_loop: false,
             loop_depth: 0,
-            need_label: false,
         }
     }
 }
 
-impl<'tcx> Visitor<'tcx> for RetSpanCollector {
+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.need_label {
-                    self.need_label = true
+                if self.loop_depth > 0 && !self.ret_in_loop {
+                    self.ret_in_loop = true
                 }
 
-                self.spans.push((expr.span, self.loop_depth > 0))
+                self.spans.push(expr.span)
             },
 
             ExprKind::Loop(..) => {
diff --git a/tests/ui/excessive_for_each.rs b/tests/ui/excessive_for_each.rs
index 1c8e450398e..0800bef71e9 100644
--- a/tests/ui/excessive_for_each.rs
+++ b/tests/ui/excessive_for_each.rs
@@ -82,7 +82,7 @@ fn main() {
         }
     });
 
-    // Should trigger this lint with notes that say "change `return` to `continue 'outer`".
+    // 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 {
diff --git a/tests/ui/excessive_for_each.stderr b/tests/ui/excessive_for_each.stderr
index c4b66e3a034..f5799484e03 100644
--- a/tests/ui/excessive_for_each.stderr
+++ b/tests/ui/excessive_for_each.stderr
@@ -122,28 +122,5 @@ note: change `return` to `continue` in the loop body
 LL |             return;
    |             ^^^^^^
 
-error: excessive use of `for_each`
-  --> $DIR/excessive_for_each.rs:86:5
-   |
-LL | /     vec.iter().for_each(|v| {
-LL | |         for i in 0..*v {
-LL | |             if i == 10 {
-LL | |                 return;
-...  |
-LL | |         }
-LL | |     });
-   | |______^ help: try: `'outer: for v in vec.iter() { .. }`
-   |
-note: change `return` to `continue 'outer` in the loop body
-  --> $DIR/excessive_for_each.rs:89:17
-   |
-LL |                 return;
-   |                 ^^^^^^
-note: change `return` to `continue` in the loop body
-  --> $DIR/excessive_for_each.rs:95:13
-   |
-LL |             return;
-   |             ^^^^^^
-
-error: aborting due to 15 previous errors
+error: aborting due to 14 previous errors
 

From ccd7a600233ead22a77e024503edf4ef679c1751 Mon Sep 17 00:00:00 2001
From: Yoshitomo Nakanishi <yurayura.rounin.3@gmail.com>
Date: Mon, 22 Feb 2021 21:41:38 +0900
Subject: [PATCH 06/14] Refactor: Remove duplicated codes from
 excessive_for_each

---
 .../src/methods/excessive_for_each.rs         | 79 ++++++-------------
 1 file changed, 24 insertions(+), 55 deletions(-)

diff --git a/clippy_lints/src/methods/excessive_for_each.rs b/clippy_lints/src/methods/excessive_for_each.rs
index e1440e68327..14aef0e99d5 100644
--- a/clippy_lints/src/methods/excessive_for_each.rs
+++ b/clippy_lints/src/methods/excessive_for_each.rs
@@ -4,13 +4,15 @@ use rustc_hir::{
     Expr, ExprKind,
 };
 use rustc_lint::LateContext;
-use rustc_middle::{hir::map::Map, ty, ty::Ty};
+use rustc_middle::hir::map::Map;
 use rustc_span::source_map::Span;
 
-use crate::utils::{match_trait_method, match_type, paths, snippet, span_lint_and_then};
-
 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;
@@ -25,8 +27,8 @@ pub(super) fn lint(cx: &LateContext<'_>, expr: &'tcx Expr<'_>, args: &[&[Expr<'_
     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 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;
@@ -34,66 +36,33 @@ pub(super) fn lint(cx: &LateContext<'_>, expr: &'tcx Expr<'_>, args: &[&[Expr<'_
             let mut ret_collector = RetCollector::new();
             ret_collector.visit_expr(&body.value);
 
-            // Skip the lint if `return` is used in `Loop` to avoid a suggest using `'label`.
+            // 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, ""));
+            let sugg = format!(
+                "for {} in {} {{ .. }}",
+                snippet(cx, body.params[0].pat.span, ""),
+                snippet(cx, for_each_receiver.span, "")
+            );
 
-            let mut notes = vec![];
-            for span in ret_collector.spans {
-                let note = format!("change `return` to `continue` in the loop body");
-                notes.push((span, note));
-            }
-
-            span_lint_and_then(cx,
-                      super::EXCESSIVE_FOR_EACH,
-                      expr.span,
-                      "excessive use of `for_each`",
-                      |diag| {
-                          diag.span_suggestion(expr.span, "try", sugg, Applicability::HasPlaceholders);
-                          for note in notes {
-                              diag.span_note(note.0, &note.1);
-                          }
-                      }
-                );
+            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");
+                    }
+                }
+            )
         }
     }
 }
 
-type PathSegment = &'static [&'static str];
-
-const TARGET_ITER_RECEIVER_TY: &[PathSegment] = &[
-    &paths::VEC,
-    &paths::VEC_DEQUE,
-    &paths::LINKED_LIST,
-    &paths::HASHMAP,
-    &paths::BTREEMAP,
-    &paths::HASHSET,
-    &paths::BTREESET,
-    &paths::BINARY_HEAP,
-];
-
-fn is_target_ty(cx: &LateContext<'_>, expr_ty: Ty<'_>) -> bool {
-    let expr_ty = expr_ty.peel_refs();
-    for target in TARGET_ITER_RECEIVER_TY {
-        if match_type(cx, expr_ty, target) {
-            return true;
-        }
-    }
-
-    if_chain! {
-        if matches!(expr_ty.kind(), ty::Slice(_) | ty::Array(..));
-        then {
-            return true;
-        }
-    }
-
-    false
-}
-
 /// 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.

From 25d8b94cecf5f7d17fc1d3d6140b62ae2f910b56 Mon Sep 17 00:00:00 2001
From: Yoshitomo Nakanishi <yurayura.rounin.3@gmail.com>
Date: Tue, 23 Feb 2021 21:55:00 +0900
Subject: [PATCH 07/14] Add comments to clarify why RetCollector is needed

---
 clippy_lints/src/methods/excessive_for_each.rs | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/clippy_lints/src/methods/excessive_for_each.rs b/clippy_lints/src/methods/excessive_for_each.rs
index 14aef0e99d5..470f4ea2204 100644
--- a/clippy_lints/src/methods/excessive_for_each.rs
+++ b/clippy_lints/src/methods/excessive_for_each.rs
@@ -66,6 +66,13 @@ pub(super) fn lint(cx: &LateContext<'_>, expr: &'tcx Expr<'_>, args: &[&[Expr<'_
 /// 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,

From 38431712998affe21621174faa2e490525d6b221 Mon Sep 17 00:00:00 2001
From: Yoshitomo Nakanishi <yurayura.rounin.3@gmail.com>
Date: Sat, 27 Feb 2021 13:50:22 +0900
Subject: [PATCH 08/14] Improve the document of excessive_for_each

---
 clippy_lints/src/methods/mod.rs | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs
index 344c7f1bd9a..f0c99528fe1 100644
--- a/clippy_lints/src/methods/mod.rs
+++ b/clippy_lints/src/methods/mod.rs
@@ -975,8 +975,8 @@ declare_clippy_lint! {
 }
 
 declare_clippy_lint! {
-    /// **What it does:** Checks for use of `obj.method().for_each(closure)` if obj doesn't
-    /// implelement `Iterator` and `method()` returns `Impl Iterator` type.
+    /// **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.

From 5543c34699ea2723565c1d517b3d3d98cb9fd8d4 Mon Sep 17 00:00:00 2001
From: Yoshitomo Nakanishi <yurayura.rounin.3@gmail.com>
Date: Sat, 27 Feb 2021 13:58:41 +0900
Subject: [PATCH 09/14] Use ".." as default value of snippet in
 excessive_for_each

---
 clippy_lints/src/methods/excessive_for_each.rs | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/clippy_lints/src/methods/excessive_for_each.rs b/clippy_lints/src/methods/excessive_for_each.rs
index 470f4ea2204..bcb7cf1491f 100644
--- a/clippy_lints/src/methods/excessive_for_each.rs
+++ b/clippy_lints/src/methods/excessive_for_each.rs
@@ -43,8 +43,8 @@ pub(super) fn lint(cx: &LateContext<'_>, expr: &'tcx Expr<'_>, args: &[&[Expr<'_
 
             let sugg = format!(
                 "for {} in {} {{ .. }}",
-                snippet(cx, body.params[0].pat.span, ""),
-                snippet(cx, for_each_receiver.span, "")
+                snippet(cx, body.params[0].pat.span, ".."),
+                snippet(cx, for_each_receiver.span, "..")
             );
 
             span_lint_and_then(

From 527fbbef486bab32a95209b638b097a14ff41db0 Mon Sep 17 00:00:00 2001
From: Yoshitomo Nakanishi <yurayura.rounin.3@gmail.com>
Date: Sat, 13 Mar 2021 00:42:43 +0900
Subject: [PATCH 10/14] Refactor excessive_for_each

---
 CHANGELOG.md                                  |   2 +-
 clippy_lints/src/iter_for_each.rs             |   0
 clippy_lints/src/lib.rs                       |   6 +-
 .../src/methods/excessive_for_each.rs         | 122 -------------
 clippy_lints/src/methods/mod.rs               |  29 ---
 clippy_lints/src/needless_for_each.rs         | 170 ++++++++++++++++++
 tests/ui/excessive_for_each.rs                | 126 -------------
 tests/ui/excessive_for_each.stderr            | 126 -------------
 tests/ui/needless_for_each_fixable.fixed      |  99 ++++++++++
 tests/ui/needless_for_each_fixable.rs         |  99 ++++++++++
 tests/ui/needless_for_each_fixable.stderr     | 108 +++++++++++
 tests/ui/needless_for_each_unfixable.rs       |  14 ++
 tests/ui/needless_for_each_unfixable.stderr   |  30 ++++
 13 files changed, 525 insertions(+), 406 deletions(-)
 delete mode 100644 clippy_lints/src/iter_for_each.rs
 delete mode 100644 clippy_lints/src/methods/excessive_for_each.rs
 create mode 100644 clippy_lints/src/needless_for_each.rs
 delete mode 100644 tests/ui/excessive_for_each.rs
 delete mode 100644 tests/ui/excessive_for_each.stderr
 create mode 100644 tests/ui/needless_for_each_fixable.fixed
 create mode 100644 tests/ui/needless_for_each_fixable.rs
 create mode 100644 tests/ui/needless_for_each_fixable.stderr
 create mode 100644 tests/ui/needless_for_each_unfixable.rs
 create mode 100644 tests/ui/needless_for_each_unfixable.stderr

diff --git a/CHANGELOG.md b/CHANGELOG.md
index c35ab6c94bf..a2193f0eab9 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -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
diff --git a/clippy_lints/src/iter_for_each.rs b/clippy_lints/src/iter_for_each.rs
deleted file mode 100644
index e69de29bb2d..00000000000
diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
index 2f2ccdb310a..8c9247d9781 100644
--- a/clippy_lints/src/lib.rs
+++ b/clippy_lints/src/lib.rs
@@ -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),
diff --git a/clippy_lints/src/methods/excessive_for_each.rs b/clippy_lints/src/methods/excessive_for_each.rs
deleted file mode 100644
index bcb7cf1491f..00000000000
--- a/clippy_lints/src/methods/excessive_for_each.rs
+++ /dev/null
@@ -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
-    }
-}
diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs
index f0c99528fe1..fccdee07877 100644
--- a/clippy_lints/src/methods/mod.rs
+++ b/clippy_lints/src/methods/mod.rs
@@ -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),
             _ => {},
         }
 
diff --git a/clippy_lints/src/needless_for_each.rs b/clippy_lints/src/needless_for_each.rs
new file mode 100644
index 00000000000..a7b0a1ca082
--- /dev/null
+++ b/clippy_lints/src/needless_for_each.rs
@@ -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
+    }
+}
diff --git a/tests/ui/excessive_for_each.rs b/tests/ui/excessive_for_each.rs
deleted file mode 100644
index 0800bef71e9..00000000000
--- a/tests/ui/excessive_for_each.rs
+++ /dev/null
@@ -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()
-    }
-}
diff --git a/tests/ui/excessive_for_each.stderr b/tests/ui/excessive_for_each.stderr
deleted file mode 100644
index f5799484e03..00000000000
--- a/tests/ui/excessive_for_each.stderr
+++ /dev/null
@@ -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
-
diff --git a/tests/ui/needless_for_each_fixable.fixed b/tests/ui/needless_for_each_fixable.fixed
new file mode 100644
index 00000000000..0caa95a9f53
--- /dev/null
+++ b/tests/ui/needless_for_each_fixable.fixed
@@ -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() {}
diff --git a/tests/ui/needless_for_each_fixable.rs b/tests/ui/needless_for_each_fixable.rs
new file mode 100644
index 00000000000..a04243de27a
--- /dev/null
+++ b/tests/ui/needless_for_each_fixable.rs
@@ -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() {}
diff --git a/tests/ui/needless_for_each_fixable.stderr b/tests/ui/needless_for_each_fixable.stderr
new file mode 100644
index 00000000000..214e357a208
--- /dev/null
+++ b/tests/ui/needless_for_each_fixable.stderr
@@ -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
+
diff --git a/tests/ui/needless_for_each_unfixable.rs b/tests/ui/needless_for_each_unfixable.rs
new file mode 100644
index 00000000000..d765d7dab65
--- /dev/null
+++ b/tests/ui/needless_for_each_unfixable.rs
@@ -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);
+        }
+    });
+}
diff --git a/tests/ui/needless_for_each_unfixable.stderr b/tests/ui/needless_for_each_unfixable.stderr
new file mode 100644
index 00000000000..58d107062bc
--- /dev/null
+++ b/tests/ui/needless_for_each_unfixable.stderr
@@ -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
+

From f2cc995bcfdc86a564b4040585f97f012be9454b Mon Sep 17 00:00:00 2001
From: Yoshitomo Nakanishi <yurayura.rounin.3@gmail.com>
Date: Sat, 13 Mar 2021 15:11:39 +0900
Subject: [PATCH 11/14] Remove method_calls

---
 clippy_lints/src/lib.rs                   |  2 +-
 clippy_lints/src/needless_for_each.rs     | 40 +++++++++++++----------
 tests/ui/needless_for_each_fixable.fixed  | 39 +++++++++++++---------
 tests/ui/needless_for_each_fixable.rs     | 39 +++++++++++++---------
 tests/ui/needless_for_each_fixable.stderr | 27 +++++++++++----
 5 files changed, 92 insertions(+), 55 deletions(-)

diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
index 8c9247d9781..11716afe11c 100644
--- a/clippy_lints/src/lib.rs
+++ b/clippy_lints/src/lib.rs
@@ -1326,7 +1326,6 @@ 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),
@@ -1409,6 +1408,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&misc_early::UNSEPARATED_LITERAL_SUFFIX),
         LintId::of(&mut_mut::MUT_MUT),
         LintId::of(&needless_continue::NEEDLESS_CONTINUE),
+        LintId::of(&needless_for_each::NEEDLESS_FOR_EACH),
         LintId::of(&needless_pass_by_value::NEEDLESS_PASS_BY_VALUE),
         LintId::of(&non_expressive_names::SIMILAR_NAMES),
         LintId::of(&option_if_let_else::OPTION_IF_LET_ELSE),
diff --git a/clippy_lints/src/needless_for_each.rs b/clippy_lints/src/needless_for_each.rs
index a7b0a1ca082..f60b09898ab 100644
--- a/clippy_lints/src/needless_for_each.rs
+++ b/clippy_lints/src/needless_for_each.rs
@@ -10,9 +10,7 @@ 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,
-};
+use crate::utils::{has_iter_method, is_trait_method, 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
@@ -41,7 +39,7 @@ declare_clippy_lint! {
     /// }
     /// ```
     pub NEEDLESS_FOR_EACH,
-    restriction,
+    pedantic,
     "using `for_each` where a `for` loop would be simpler"
 }
 
@@ -55,22 +53,28 @@ impl LateLintPass<'_> for NeedlessForEach {
             _ => 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);
+            // Check the method name is `for_each`.
+            if let ExprKind::MethodCall(method_name, _, for_each_args, _) = expr.kind;
+            if method_name.ident.name == Symbol::intern("for_each");
+            // Check `for_each` is an associated function of `Iterator`.
+            if is_trait_method(cx, expr, sym::Iterator);
+            // Checks the receiver of `for_each` is also a method call.
+            if let Some(for_each_receiver) = for_each_args.get(0);
+            if let ExprKind::MethodCall(_, _, iter_args, _) = for_each_receiver.kind;
+            // Skip the lint if the call chain is too long. e.g. `v.field.iter().for_each()` or
+            // `v.foo().iter().for_each()` must be skipped.
+            if let Some(iter_receiver) = iter_args.get(0);
+            if matches!(
+                iter_receiver.kind,
+                ExprKind::Array(..) | ExprKind::Call(..) | ExprKind::Path(..)
+            );
+            // Checks the type of the `iter` method receiver is NOT a user defined type.
+            if has_iter_method(cx, cx.typeck_results().expr_ty(&iter_receiver)).is_some();
             // 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::Closure(_, _, body_id, ..) = for_each_args[1].kind;
+            let body = cx.tcx.hir().body(body_id);
             if let ExprKind::Block(..) = body.value.kind;
             then {
                 let mut ret_collector = RetCollector::default();
@@ -99,7 +103,7 @@ impl LateLintPass<'_> for NeedlessForEach {
                 )));
 
                 for span in &ret_collector.spans {
-                    suggs.push((*span, "return".to_string()));
+                    suggs.push((*span, "continue".to_string()));
                 }
 
                 span_lint_and_then(
diff --git a/tests/ui/needless_for_each_fixable.fixed b/tests/ui/needless_for_each_fixable.fixed
index 0caa95a9f53..a4d4937a19a 100644
--- a/tests/ui/needless_for_each_fixable.fixed
+++ b/tests/ui/needless_for_each_fixable.fixed
@@ -14,6 +14,10 @@ fn should_lint() {
         acc += elem;
     }
 
+    for elem in [1, 2, 3].iter() {
+        acc += elem;
+    }
+
     let mut hash_map: HashMap<i32, i32> = HashMap::new();
     for (k, v) in hash_map.iter() {
         acc += k + v;
@@ -46,11 +50,30 @@ fn should_not_lint() {
     }
     v.iter().for_each(print);
 
+    // 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;
+    });
+
     // `for_each` follows long iterator chain.
-    v.iter().chain(v.iter()).for_each(|v| println!("{}", v));
+    v.iter().chain(v.iter()).for_each(|v| {
+        acc += v;
+    });
     v.as_slice().iter().for_each(|v| {
         acc += v;
     });
+    s.v.iter().for_each(|v| {
+        acc += v;
+    });
 
     // `return` is used in `Loop` of the closure.
     v.iter().for_each(|v| {
@@ -68,20 +91,6 @@ fn should_not_lint() {
         }
     });
 
-    // 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| {
diff --git a/tests/ui/needless_for_each_fixable.rs b/tests/ui/needless_for_each_fixable.rs
index a04243de27a..b374128f253 100644
--- a/tests/ui/needless_for_each_fixable.rs
+++ b/tests/ui/needless_for_each_fixable.rs
@@ -14,6 +14,10 @@ fn should_lint() {
         acc += elem;
     });
 
+    [1, 2, 3].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;
@@ -46,11 +50,30 @@ fn should_not_lint() {
     }
     v.iter().for_each(print);
 
+    // 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;
+    });
+
     // `for_each` follows long iterator chain.
-    v.iter().chain(v.iter()).for_each(|v| println!("{}", v));
+    v.iter().chain(v.iter()).for_each(|v| {
+        acc += v;
+    });
     v.as_slice().iter().for_each(|v| {
         acc += v;
     });
+    s.v.iter().for_each(|v| {
+        acc += v;
+    });
 
     // `return` is used in `Loop` of the closure.
     v.iter().for_each(|v| {
@@ -68,20 +91,6 @@ fn should_not_lint() {
         }
     });
 
-    // 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| {
diff --git a/tests/ui/needless_for_each_fixable.stderr b/tests/ui/needless_for_each_fixable.stderr
index 214e357a208..483a5e6d61d 100644
--- a/tests/ui/needless_for_each_fixable.stderr
+++ b/tests/ui/needless_for_each_fixable.stderr
@@ -30,7 +30,22 @@ LL |     }
    |
 
 error: needless use of `for_each`
-  --> $DIR/needless_for_each_fixable.rs:18:5
+  --> $DIR/needless_for_each_fixable.rs:17:5
+   |
+LL | /     [1, 2, 3].iter().for_each(|elem| {
+LL | |         acc += elem;
+LL | |     });
+   | |_______^
+   |
+help: try
+   |
+LL |     for elem in [1, 2, 3].iter() {
+LL |         acc += elem;
+LL |     }
+   |
+
+error: needless use of `for_each`
+  --> $DIR/needless_for_each_fixable.rs:22:5
    |
 LL | /     hash_map.iter().for_each(|(k, v)| {
 LL | |         acc += k + v;
@@ -45,7 +60,7 @@ LL |     }
    |
 
 error: needless use of `for_each`
-  --> $DIR/needless_for_each_fixable.rs:21:5
+  --> $DIR/needless_for_each_fixable.rs:25:5
    |
 LL | /     hash_map.iter_mut().for_each(|(k, v)| {
 LL | |         acc += *k + *v;
@@ -60,7 +75,7 @@ LL |     }
    |
 
 error: needless use of `for_each`
-  --> $DIR/needless_for_each_fixable.rs:24:5
+  --> $DIR/needless_for_each_fixable.rs:28:5
    |
 LL | /     hash_map.keys().for_each(|k| {
 LL | |         acc += k;
@@ -75,7 +90,7 @@ LL |     }
    |
 
 error: needless use of `for_each`
-  --> $DIR/needless_for_each_fixable.rs:27:5
+  --> $DIR/needless_for_each_fixable.rs:31:5
    |
 LL | /     hash_map.values().for_each(|v| {
 LL | |         acc += v;
@@ -90,7 +105,7 @@ LL |     }
    |
 
 error: needless use of `for_each`
-  --> $DIR/needless_for_each_fixable.rs:34:5
+  --> $DIR/needless_for_each_fixable.rs:38:5
    |
 LL | /     my_vec().iter().for_each(|elem| {
 LL | |         acc += elem;
@@ -104,5 +119,5 @@ LL |         acc += elem;
 LL |     }
    |
 
-error: aborting due to 7 previous errors
+error: aborting due to 8 previous errors
 

From 1109dc8838b8af0ad7e2b8eb3a7039c907188082 Mon Sep 17 00:00:00 2001
From: Yoshitomo Nakanishi <yurayura.rounin.3@gmail.com>
Date: Sat, 13 Mar 2021 15:23:57 +0900
Subject: [PATCH 12/14] Fix codes that make dogfood fail

---
 tests/lint_message_convention.rs | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/lint_message_convention.rs b/tests/lint_message_convention.rs
index 3f754c255b7..2f8989c8e11 100644
--- a/tests/lint_message_convention.rs
+++ b/tests/lint_message_convention.rs
@@ -89,14 +89,14 @@ fn lint_message_convention() {
         .filter(|message| !message.bad_lines.is_empty())
         .collect();
 
-    bad_tests.iter().for_each(|message| {
+    for message in &bad_tests {
         eprintln!(
             "error: the test '{}' contained the following nonconforming lines :",
             message.path.display()
         );
         message.bad_lines.iter().for_each(|line| eprintln!("{}", line));
         eprintln!("\n\n");
-    });
+    }
 
     eprintln!(
         "\n\n\nLint message should not start with a capital letter and should not have punctuation at the end of the message unless multiple sentences are needed."

From bf1e3f7a9f53dd783ddf500ee45e8c0629dd5e67 Mon Sep 17 00:00:00 2001
From: Yoshitomo Nakanishi <yurayura.rounin.3@gmail.com>
Date: Sun, 14 Mar 2021 10:22:28 +0900
Subject: [PATCH 13/14] Skip needless_for_each if an input stmt is local

---
 clippy_lints/src/needless_for_each.rs       | 50 ++++++++++-----------
 tests/ui/needless_for_each_fixable.fixed    |  5 +++
 tests/ui/needless_for_each_fixable.rs       |  5 +++
 tests/ui/needless_for_each_unfixable.stderr |  9 ++--
 4 files changed, 37 insertions(+), 32 deletions(-)

diff --git a/clippy_lints/src/needless_for_each.rs b/clippy_lints/src/needless_for_each.rs
index f60b09898ab..727937354d6 100644
--- a/clippy_lints/src/needless_for_each.rs
+++ b/clippy_lints/src/needless_for_each.rs
@@ -49,31 +49,28 @@ 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,
         };
 
         if_chain! {
             // Check the method name is `for_each`.
-            if let ExprKind::MethodCall(method_name, _, for_each_args, _) = expr.kind;
+            if let ExprKind::MethodCall(method_name, _, [for_each_recv, for_each_arg], _) = expr.kind;
             if method_name.ident.name == Symbol::intern("for_each");
             // Check `for_each` is an associated function of `Iterator`.
             if is_trait_method(cx, expr, sym::Iterator);
             // Checks the receiver of `for_each` is also a method call.
-            if let Some(for_each_receiver) = for_each_args.get(0);
-            if let ExprKind::MethodCall(_, _, iter_args, _) = for_each_receiver.kind;
+            if let ExprKind::MethodCall(_, _, [iter_recv], _) = for_each_recv.kind;
             // Skip the lint if the call chain is too long. e.g. `v.field.iter().for_each()` or
             // `v.foo().iter().for_each()` must be skipped.
-            if let Some(iter_receiver) = iter_args.get(0);
             if matches!(
-                iter_receiver.kind,
+                iter_recv.kind,
                 ExprKind::Array(..) | ExprKind::Call(..) | ExprKind::Path(..)
             );
             // Checks the type of the `iter` method receiver is NOT a user defined type.
-            if has_iter_method(cx, cx.typeck_results().expr_ty(&iter_receiver)).is_some();
+            if has_iter_method(cx, cx.typeck_results().expr_ty(&iter_recv)).is_some();
             // 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::Closure(_, _, body_id, ..) = for_each_args[1].kind;
+            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 {
@@ -85,26 +82,27 @@ impl LateLintPass<'_> for NeedlessForEach {
                     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
+                let (mut applicability, ret_suggs) = if ret_collector.spans.is_empty() {
+                    (Applicability::MachineApplicable, None)
                 } else {
-                    Applicability::MaybeIncorrect
+                    (
+                        Applicability::MaybeIncorrect,
+                        Some(
+                            ret_collector
+                                .spans
+                                .into_iter()
+                                .map(|span| (span, "continue".to_string()))
+                                .collect(),
+                        ),
+                    )
                 };
 
-                let mut suggs = vec![];
-                suggs.push((stmt.span, format!(
+                let sugg = 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, for_each_recv.span, "..", &mut applicability),
                     snippet_with_applicability(cx, body.value.span, "..", &mut applicability),
-                )));
-
-                for span in &ret_collector.spans {
-                    suggs.push((*span, "continue".to_string()));
-                }
+                );
 
                 span_lint_and_then(
                     cx,
@@ -112,11 +110,9 @@ impl LateLintPass<'_> for NeedlessForEach {
                     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`");
+                        diag.span_suggestion(stmt.span, "try", sugg, applicability);
+                        if let Some(ret_suggs) = ret_suggs {
+                            diag.multipart_suggestion("try replacing `return` with `continue`", ret_suggs, applicability);
                         }
                     }
                 )
diff --git a/tests/ui/needless_for_each_fixable.fixed b/tests/ui/needless_for_each_fixable.fixed
index a4d4937a19a..f00f9ee4c33 100644
--- a/tests/ui/needless_for_each_fixable.fixed
+++ b/tests/ui/needless_for_each_fixable.fixed
@@ -103,6 +103,11 @@ fn should_not_lint() {
             acc += elem;
         }),
     }
+
+    // `for_each` is in a let bingind.
+    let _ = v.iter().for_each(|elem| {
+        acc += elem;
+    });
 }
 
 fn main() {}
diff --git a/tests/ui/needless_for_each_fixable.rs b/tests/ui/needless_for_each_fixable.rs
index b374128f253..1bd400d348b 100644
--- a/tests/ui/needless_for_each_fixable.rs
+++ b/tests/ui/needless_for_each_fixable.rs
@@ -103,6 +103,11 @@ fn should_not_lint() {
             acc += elem;
         }),
     }
+
+    // `for_each` is in a let bingind.
+    let _ = v.iter().for_each(|elem| {
+        acc += elem;
+    });
 }
 
 fn main() {}
diff --git a/tests/ui/needless_for_each_unfixable.stderr b/tests/ui/needless_for_each_unfixable.stderr
index 58d107062bc..bbb63fd8deb 100644
--- a/tests/ui/needless_for_each_unfixable.stderr
+++ b/tests/ui/needless_for_each_unfixable.stderr
@@ -11,11 +11,6 @@ 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() {
@@ -25,6 +20,10 @@ LL |         } else {
 LL |             println!("{}", v);
 LL |         }
  ...
+help: try replacing `return` with `continue`
+   |
+LL |             continue;
+   |             ^^^^^^^^
 
 error: aborting due to previous error
 

From e61f9782c805f246394f870902140bf32c897b7e Mon Sep 17 00:00:00 2001
From: Yoshitomo Nakanishi <yurayura.rounin.3@gmail.com>
Date: Mon, 15 Mar 2021 12:01:39 +0900
Subject: [PATCH 14/14] Tweak a suggestion message of needless_for_each

---
 clippy_lints/src/needless_for_each.rs       | 21 +++++++++------------
 tests/ui/needless_for_each_unfixable.stderr |  2 +-
 2 files changed, 10 insertions(+), 13 deletions(-)

diff --git a/clippy_lints/src/needless_for_each.rs b/clippy_lints/src/needless_for_each.rs
index 727937354d6..2ea871990f1 100644
--- a/clippy_lints/src/needless_for_each.rs
+++ b/clippy_lints/src/needless_for_each.rs
@@ -10,7 +10,10 @@ use rustc_span::{source_map::Span, sym, Symbol};
 
 use if_chain::if_chain;
 
-use crate::utils::{has_iter_method, is_trait_method, snippet_with_applicability, span_lint_and_then};
+use clippy_utils::diagnostics::span_lint_and_then;
+use clippy_utils::is_trait_method;
+use clippy_utils::source::snippet_with_applicability;
+use clippy_utils::ty::has_iter_method;
 
 declare_clippy_lint! {
     /// **What it does:** Checks for usage of `for_each` that would be more simply written as a
@@ -104,18 +107,12 @@ impl LateLintPass<'_> for NeedlessForEach {
                     snippet_with_applicability(cx, body.value.span, "..", &mut applicability),
                 );
 
-                span_lint_and_then(
-                    cx,
-                    NEEDLESS_FOR_EACH,
-                    stmt.span,
-                    "needless use of `for_each`",
-                    |diag| {
-                        diag.span_suggestion(stmt.span, "try", sugg, applicability);
-                        if let Some(ret_suggs) = ret_suggs {
-                            diag.multipart_suggestion("try replacing `return` with `continue`", ret_suggs, applicability);
-                        }
+                span_lint_and_then(cx, NEEDLESS_FOR_EACH, stmt.span, "needless use of `for_each`", |diag| {
+                    diag.span_suggestion(stmt.span, "try", sugg, applicability);
+                    if let Some(ret_suggs) = ret_suggs {
+                        diag.multipart_suggestion("...and replace `return` with `continue`", ret_suggs, applicability);
                     }
-                )
+                })
             }
         }
     }
diff --git a/tests/ui/needless_for_each_unfixable.stderr b/tests/ui/needless_for_each_unfixable.stderr
index bbb63fd8deb..8c4507d2328 100644
--- a/tests/ui/needless_for_each_unfixable.stderr
+++ b/tests/ui/needless_for_each_unfixable.stderr
@@ -20,7 +20,7 @@ LL |         } else {
 LL |             println!("{}", v);
 LL |         }
  ...
-help: try replacing `return` with `continue`
+help: ...and replace `return` with `continue`
    |
 LL |             continue;
    |             ^^^^^^^^