Fix FP of `needless_range_loop` when meeting multidimensional array (#15486)

Fixes  rust-lang/rust-clippy#15068

changelog: Fix [`needless_range_loop`] false positive and false negative
when meeting multidimensional array

3 files changed, 86 insertions, 5 deletions

diff --git a/clippy_lints/src/loops/needless_range_loop.rs b/clippy_lints/src/loops/needless_range_loop.rs
index 7bb684d65bb..11edb929d70 100644
--- a/clippy_lints/src/loops/needless_range_loop.rs
+++ b/clippy_lints/src/loops/needless_range_loop.rs
@@ -3,7 +3,7 @@ use clippy_utils::diagnostics::span_lint_and_then;
 use clippy_utils::source::snippet;
 use clippy_utils::ty::has_iter_method;
 use clippy_utils::visitors::is_local_used;
-use clippy_utils::{SpanlessEq, contains_name, higher, is_integer_const, sugg};
+use clippy_utils::{SpanlessEq, contains_name, higher, is_integer_const, peel_hir_expr_while, sugg};
 use rustc_ast::ast;
 use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexMap};
 use rustc_errors::Applicability;
@@ -253,12 +253,38 @@ struct VarVisitor<'a, 'tcx> {
 
 impl<'tcx> VarVisitor<'_, 'tcx> {
     fn check(&mut self, idx: &'tcx Expr<'_>, seqexpr: &'tcx Expr<'_>, expr: &'tcx Expr<'_>) -> bool {
-        let index_used_directly = matches!(idx.kind, ExprKind::Path(_));
+        let mut used_cnt = 0;
+        // It is `true` if all indices are direct
+        let mut index_used_directly = true;
+
+        // Handle initial index
+        if is_local_used(self.cx, idx, self.var) {
+            used_cnt += 1;
+            index_used_directly &= matches!(idx.kind, ExprKind::Path(_));
+        }
+        // Handle nested indices
+        let seqexpr = peel_hir_expr_while(seqexpr, |e| {
+            if let ExprKind::Index(e, idx, _) = e.kind {
+                if is_local_used(self.cx, idx, self.var) {
+                    used_cnt += 1;
+                    index_used_directly &= matches!(idx.kind, ExprKind::Path(_));
+                }
+                Some(e)
+            } else {
+                None
+            }
+        });
+
+        match used_cnt {
+            0 => return true,
+            n if n > 1 => self.nonindex = true, // Optimize code like `a[i][i]`
+            _ => {},
+        }
+
         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)
         {
             if self.prefer_mutable {
                 self.indexed_mut.insert(seqvar.segments[0].ident.name);
@@ -312,7 +338,6 @@ impl<'tcx> VarVisitor<'_, 'tcx> {
 impl<'tcx> Visitor<'tcx> for VarVisitor<'_, 'tcx> {
     fn visit_expr(&mut self, expr: &'tcx Expr<'_>) {
         if let ExprKind::MethodCall(meth, args_0, [args_1, ..], _) = &expr.kind
-            // a range index op
             && let Some(trait_id) = self
                 .cx
                 .typeck_results()
diff --git a/tests/ui/needless_range_loop.rs b/tests/ui/needless_range_loop.rs
index 70cf9fa7369..ea4591d8b71 100644
--- a/tests/ui/needless_range_loop.rs
+++ b/tests/ui/needless_range_loop.rs
@@ -210,3 +210,35 @@ fn needless_loop() {
         black_box([1, 2, 3, 4, 5, 6, 7, 8][i]);
     }
 }
+
+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];
+    }
+
+    for i in 0..MAX_LEN {
+        // no error
+        let _ = a[i][0];
+        let _ = b[i];
+    }
+
+    for i in 0..MAX_LEN {
+        // no error
+        let _ = a[i][b[i] as usize];
+    }
+
+    for i in 0..MAX_LEN {
+        //~^ needless_range_loop
+        let _ = a[i][i];
+    }
+
+    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 23c085f9d3b..33a519d8a80 100644
--- a/tests/ui/needless_range_loop.stderr
+++ b/tests/ui/needless_range_loop.stderr
@@ -168,5 +168,29 @@ LL -     for i in 0..vec.len() {
 LL +     for (i, <item>) in vec.iter_mut().enumerate() {
    |
 
-error: aborting due to 14 previous errors
+error: the loop variable `i` is used to index `a`
+  --> tests/ui/needless_range_loop.rs:235:14
+   |
+LL |     for i in 0..MAX_LEN {
+   |              ^^^^^^^^^^
+   |
+help: consider using an iterator and enumerate()
+   |
+LL -     for i in 0..MAX_LEN {
+LL +     for (i, <item>) in a.iter().enumerate().take(MAX_LEN) {
+   |
+
+error: the loop variable `i` is only used to index `a`
+  --> tests/ui/needless_range_loop.rs:240:14
+   |
+LL |     for i in 0..MAX_LEN {
+   |              ^^^^^^^^^^
+   |
+help: consider using an iterator
+   |
+LL -     for i in 0..MAX_LEN {
+LL +     for <item> in a.iter().take(MAX_LEN) {
+   |
+
+error: aborting due to 16 previous errors