Auto merge of #13176 - shenyifu:master, r=Manishearth

Fix fix under loop may dropping loop label when applying fix.

changelog: [`explicit_counter_loop`]: fix label drop
changelog: [`for_kv_map`]: add label drop test

Author: bors

clippy_lints/src/loops/explicit_counter_loop.rs

@@ -2,6 +2,7 @@ use super::{make_iterator_snippet, IncrementVisitor, InitializeVisitor, EXPLICIT
 use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then};
 use clippy_utils::source::snippet_with_applicability;
 use clippy_utils::{get_enclosing_block, is_integer_const};
+use rustc_ast::Label;
 use rustc_errors::Applicability;
 use rustc_hir::intravisit::{walk_block, walk_expr};
 use rustc_hir::{Expr, Pat};
@@ -17,6 +18,7 @@ pub(super) fn check<'tcx>(
     arg: &'tcx Expr<'_>,
     body: &'tcx Expr<'_>,
     expr: &'tcx Expr<'_>,
+    label: Option<Label>,
 ) {
     // Look for variables that are incremented once per loop iteration.
     let mut increment_visitor = IncrementVisitor::new(cx);
@@ -34,7 +36,7 @@ pub(super) fn check<'tcx>(
             {
                 let mut applicability = Applicability::MaybeIncorrect;
                 let span = expr.span.with_hi(arg.span.hi());
-
+                let loop_label = label.map_or(String::new(), |l| format!("{}: ", l.ident.name));
                 let int_name = match ty.map(Ty::kind) {
                     // usize or inferred
                     Some(ty::Uint(UintTy::Usize)) | None => {
@@ -45,7 +47,7 @@ pub(super) fn check<'tcx>(
                             format!("the variable `{name}` is used as a loop counter"),
                             "consider using",
                             format!(
-                                "for ({name}, {}) in {}.enumerate()",
+                                "{loop_label}for ({name}, {}) in {}.enumerate()",
                                 snippet_with_applicability(cx, pat.span, "item", &mut applicability),
                                 make_iterator_snippet(cx, arg, &mut applicability),
                             ),
@@ -68,7 +70,7 @@ pub(super) fn check<'tcx>(
                             span,
                             "consider using",
                             format!(
-                                "for ({name}, {}) in (0_{int_name}..).zip({})",
+                                "{loop_label}for ({name}, {}) in (0_{int_name}..).zip({})",
                                 snippet_with_applicability(cx, pat.span, "item", &mut applicability),
                                 make_iterator_snippet(cx, arg, &mut applicability),
                             ),

clippy_lints/src/loops/mod.rs

@@ -25,6 +25,7 @@ mod while_let_on_iterator;
 use clippy_config::msrvs::Msrv;
 use clippy_config::Conf;
 use clippy_utils::higher;
+use rustc_ast::Label;
 use rustc_hir::{Expr, ExprKind, LoopSource, Pat};
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_session::impl_lint_pass;
@@ -760,6 +761,7 @@ impl<'tcx> LateLintPass<'tcx> for Loops {
             body,
             loop_id,
             span,
+            label,
         }) = for_loop
         {
             // we don't want to check expanded macros
@@ -768,7 +770,7 @@ impl<'tcx> LateLintPass<'tcx> for Loops {
             if body.span.from_expansion() {
                 return;
             }
-            self.check_for_loop(cx, pat, arg, body, expr, span);
+            self.check_for_loop(cx, pat, arg, body, expr, span, label);
             if let ExprKind::Block(block, _) = body.kind {
                 never_loop::check(cx, block, loop_id, span, for_loop.as_ref());
             }
@@ -808,6 +810,7 @@ impl<'tcx> LateLintPass<'tcx> for Loops {
 }
 
 impl Loops {
+    #[allow(clippy::too_many_arguments)]
     fn check_for_loop<'tcx>(
         &self,
         cx: &LateContext<'tcx>,
@@ -816,11 +819,12 @@ impl Loops {
         body: &'tcx Expr<'_>,
         expr: &'tcx Expr<'_>,
         span: Span,
+        label: Option<Label>,
     ) {
         let is_manual_memcpy_triggered = manual_memcpy::check(cx, pat, arg, body, expr);
         if !is_manual_memcpy_triggered {
             needless_range_loop::check(cx, pat, arg, body, expr);
-            explicit_counter_loop::check(cx, pat, arg, body, expr);
+            explicit_counter_loop::check(cx, pat, arg, body, expr, label);
         }
         self.check_for_loop_arg(cx, pat, arg);
         for_kv_map::check(cx, pat, arg, body);

clippy_utils/src/higher.rs

@@ -25,6 +25,8 @@ pub struct ForLoop<'tcx> {
     pub loop_id: HirId,
     /// entire `for` loop span
     pub span: Span,
+    /// label
+    pub label: Option<ast::Label>,
 }
 
 impl<'tcx> ForLoop<'tcx> {
@@ -33,7 +35,7 @@ impl<'tcx> ForLoop<'tcx> {
         if let ExprKind::DropTemps(e) = expr.kind
             && let ExprKind::Match(iterexpr, [arm], MatchSource::ForLoopDesugar) = e.kind
             && let ExprKind::Call(_, [arg]) = iterexpr.kind
-            && let ExprKind::Loop(block, ..) = arm.body.kind
+            && let ExprKind::Loop(block, label, ..) = arm.body.kind
             && let [stmt] = block.stmts
             && let hir::StmtKind::Expr(e) = stmt.kind
             && let ExprKind::Match(_, [_, some_arm], _) = e.kind
@@ -45,6 +47,7 @@ impl<'tcx> ForLoop<'tcx> {
                 body: some_arm.body,
                 loop_id: arm.body.hir_id,
                 span: expr.span.ctxt().outer_expn_data().call_site,
+                label,
             });
         }
         None

tests/ui/explicit_counter_loop.rs

@@ -278,3 +278,16 @@ mod issue_10058 {
         }
     }
 }
+
+mod issue_13123 {
+    pub fn test() {
+        let mut vec = vec![1, 2, 3, 4];
+        let mut _index = 0;
+        'label: for v in vec {
+            _index += 1;
+            if v == 1 {
+                break 'label;
+            }
+        }
+    }
+}

tests/ui/explicit_counter_loop.stderr

@@ -57,5 +57,11 @@ LL |         for _item in slice {
    |
    = note: `idx_u32` is of type `u32`, making it ineligible for `Iterator::enumerate`
 
-error: aborting due to 9 previous errors
+error: the variable `_index` is used as a loop counter
+  --> tests/ui/explicit_counter_loop.rs:286:9
+   |
+LL |         'label: for v in vec {
+   |         ^^^^^^^^^^^^^^^^^^^^ help: consider using: `'label: for (_index, v) in vec.into_iter().enumerate()`
+
+error: aborting due to 10 previous errors
 

tests/ui/for_kv_map.fixed

@@ -40,6 +40,16 @@ fn main() {
         let _k = k;
     }
 
+    let m: HashMap<u64, u64> = HashMap::new();
+    let rm = &m;
+    'label: for k in rm.keys() {
+        //~^ ERROR: you seem to want to iterate on a map's keys
+        let _k = k;
+        if *k == 0u64 {
+            break 'label;
+        }
+    }
+
     // The following should not produce warnings.
 
     let m: HashMap<u64, u64> = HashMap::new();

tests/ui/for_kv_map.rs

@@ -40,6 +40,16 @@ fn main() {
         let _k = k;
     }
 
+    let m: HashMap<u64, u64> = HashMap::new();
+    let rm = &m;
+    'label: for (k, _value) in rm {
+        //~^ ERROR: you seem to want to iterate on a map's keys
+        let _k = k;
+        if *k == 0u64 {
+            break 'label;
+        }
+    }
+
     // The following should not produce warnings.
 
     let m: HashMap<u64, u64> = HashMap::new();

tests/ui/for_kv_map.stderr

@@ -55,5 +55,16 @@ help: use the corresponding method
 LL |     for k in rm.keys() {
    |         ~    ~~~~~~~~~
 
-error: aborting due to 5 previous errors
+error: you seem to want to iterate on a map's keys
+  --> tests/ui/for_kv_map.rs:45:32
+   |
+LL |     'label: for (k, _value) in rm {
+   |                                ^^
+   |
+help: use the corresponding method
+   |
+LL |     'label: for k in rm.keys() {
+   |                 ~    ~~~~~~~~~
+
+error: aborting due to 6 previous errors