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] 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 f3e9e2400f16..e1440e683275 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 1c8e450398e1..0800bef71e9c 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 c4b66e3a0346..f5799484e034 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