diff --git a/clippy_lints/src/loops/needless_range_loop.rs b/clippy_lints/src/loops/needless_range_loop.rs index 7837b18bcd36..f3111f5074b0 100644 --- a/clippy_lints/src/loops/needless_range_loop.rs +++ b/clippy_lints/src/loops/needless_range_loop.rs @@ -9,7 +9,7 @@ use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexMap}; use rustc_errors::Applicability; use rustc_hir::def::{DefKind, Res}; use rustc_hir::intravisit::{Visitor, walk_expr}; -use rustc_hir::{BinOpKind, BorrowKind, Closure, Expr, ExprKind, HirId, Mutability, Pat, PatKind, QPath}; +use rustc_hir::{BinOpKind, BorrowKind, Closure, Expr, ExprKind, HirId, Mutability, Node, Pat, PatKind, QPath}; use rustc_lint::LateContext; use rustc_middle::middle::region; use rustc_middle::ty::{self, Ty}; @@ -241,12 +241,18 @@ struct VarVisitor<'a, 'tcx> { } impl<'tcx> VarVisitor<'_, 'tcx> { - fn check(&mut self, idx: &'tcx Expr<'_>, seqexpr: &'tcx Expr<'_>, expr: &'tcx Expr<'_>) -> bool { + fn check<'a>(&mut self, idxs: &mut Vec<&'tcx Expr<'a>>, seqexpr: &'tcx Expr<'a>, expr: &'tcx Expr<'_>) -> bool { + // Handle multi-dimensional array + if let ExprKind::Index(seqexpr, idx, _) = seqexpr.kind { + idxs.push(idx); + // Recursive call + return self.check(idxs, seqexpr, expr); + } if let ExprKind::Path(ref seqpath) = seqexpr.kind // the indexed container is referenced by a name && let QPath::Resolved(None, seqvar) = *seqpath && seqvar.segments.len() == 1 - && is_local_used(self.cx, idx, self.var) + && let Some(idx) = idxs.iter().find(|idx| is_local_used(self.cx, **idx, self.var)) { if self.prefer_mutable { self.indexed_mut.insert(seqvar.segments[0].ident.name); @@ -302,16 +308,27 @@ impl<'tcx> Visitor<'tcx> for VarVisitor<'_, 'tcx> { .and_then(|def_id| self.cx.tcx.trait_of_item(def_id)) && ((meth.ident.name == sym::index && self.cx.tcx.lang_items().index_trait() == Some(trait_id)) || (meth.ident.name == sym::index_mut && self.cx.tcx.lang_items().index_mut_trait() == Some(trait_id))) - && !self.check(args_1, args_0, expr) { - return; + let mut idxs: Vec<&Expr<'_>> = vec![args_1]; + if !self.check(&mut idxs, args_0, expr) { + return; + } } - if let ExprKind::Index(seqexpr, idx, _) = expr.kind + if let ExprKind::Index(seqexpr, idx, _) = expr.kind { + // Only handle the top `Index` expr + if let Node::Expr(Expr { + kind: ExprKind::Index(_, _, _), + .. + }) = self.cx.tcx.parent_hir_node(expr.hir_id) + { + return; + } + let mut idxs: Vec<&Expr<'_>> = vec![idx]; // an index op - && !self.check(idx, seqexpr, expr) - { - return; + if !self.check(&mut idxs, seqexpr, expr) { + return; + } } if let ExprKind::Path(QPath::Resolved(None, path)) = expr.kind diff --git a/tests/ui/needless_range_loop.rs b/tests/ui/needless_range_loop.rs index 8a1c1be289cf..0402fd89affa 100644 --- a/tests/ui/needless_range_loop.rs +++ b/tests/ui/needless_range_loop.rs @@ -185,3 +185,23 @@ mod issue_2496 { unimplemented!() } } + +fn issue_15068() { + let a = vec![vec![0u8; MAX_LEN]; MAX_LEN]; + let b = vec![0u8; MAX_LEN]; + + for i in 0..MAX_LEN { + // no error + let _ = a[0][i]; + let _ = b[i]; + } + + // Could be rewrited to: + // ```no_run + // for in a[0].take(MAX_LEN) { + // ``` + for i in 0..MAX_LEN { + //~^ needless_range_loop + let _ = a[0][i]; + } +} diff --git a/tests/ui/needless_range_loop.stderr b/tests/ui/needless_range_loop.stderr index 23c085f9d3b2..f3f2b6ea4315 100644 --- a/tests/ui/needless_range_loop.stderr +++ b/tests/ui/needless_range_loop.stderr @@ -168,5 +168,17 @@ LL - for i in 0..vec.len() { LL + for (i, ) in vec.iter_mut().enumerate() { | -error: aborting due to 14 previous errors +error: the loop variable `i` is only used to index `a` + --> tests/ui/needless_range_loop.rs:203:14 + | +LL | for i in 0..MAX_LEN { + | ^^^^^^^^^^ + | +help: consider using an iterator + | +LL - for i in 0..MAX_LEN { +LL + for in a.iter().take(MAX_LEN) { + | + +error: aborting due to 15 previous errors