diff options
| -rw-r--r-- | clippy_lints/src/loops/explicit_counter_loop.rs | 8 | ||||
| -rw-r--r-- | clippy_lints/src/loops/mod.rs | 8 | ||||
| -rw-r--r-- | clippy_utils/src/higher.rs | 5 | ||||
| -rw-r--r-- | tests/ui/explicit_counter_loop.rs | 13 | ||||
| -rw-r--r-- | tests/ui/explicit_counter_loop.stderr | 8 | ||||
| -rw-r--r-- | tests/ui/for_kv_map.fixed | 10 | ||||
| -rw-r--r-- | tests/ui/for_kv_map.rs | 10 | ||||
| -rw-r--r-- | tests/ui/for_kv_map.stderr | 13 |
8 files changed, 67 insertions, 8 deletions
diff --git a/clippy_lints/src/loops/explicit_counter_loop.rs b/clippy_lints/src/loops/explicit_counter_loop.rs index f0ee64d714e..73a23615c2d 100644 --- a/clippy_lints/src/loops/explicit_counter_loop.rs +++ b/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), ), diff --git a/clippy_lints/src/loops/mod.rs b/clippy_lints/src/loops/mod.rs index be34bfaa463..92ccc0cc0a1 100644 --- a/clippy_lints/src/loops/mod.rs +++ b/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); diff --git a/clippy_utils/src/higher.rs b/clippy_utils/src/higher.rs index 7ce9bde1a73..33c88efa0cd 100644 --- a/clippy_utils/src/higher.rs +++ b/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 diff --git a/tests/ui/explicit_counter_loop.rs b/tests/ui/explicit_counter_loop.rs index c25e79a3617..28b477b6921 100644 --- a/tests/ui/explicit_counter_loop.rs +++ b/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; + } + } + } +} diff --git a/tests/ui/explicit_counter_loop.stderr b/tests/ui/explicit_counter_loop.stderr index e28f8783f9c..1b2d1f8570a 100644 --- a/tests/ui/explicit_counter_loop.stderr +++ b/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 diff --git a/tests/ui/for_kv_map.fixed b/tests/ui/for_kv_map.fixed index a2112d7b730..1733b29128f 100644 --- a/tests/ui/for_kv_map.fixed +++ b/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(); diff --git a/tests/ui/for_kv_map.rs b/tests/ui/for_kv_map.rs index 1b7959b8f92..de465a7c8e6 100644 --- a/tests/ui/for_kv_map.rs +++ b/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(); diff --git a/tests/ui/for_kv_map.stderr b/tests/ui/for_kv_map.stderr index f4ce473d095..adcc3ab8fdb 100644 --- a/tests/ui/for_kv_map.stderr +++ b/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 |
