Auto merge of #3757 - mikerite:fix-2542, r=oli-obk

Fix `needless_range_loop` bad suggestion

Detect if the index variable is used inside a closure.

Fixes #2542

Author: bors

clippy_lints/src/loops.rs

@@ -1830,17 +1830,29 @@ impl<'a, 'tcx> Visitor<'tcx> for VarVisitor<'a, 'tcx> {
             if let ExprKind::Path(ref qpath) = expr.node;
             if let QPath::Resolved(None, ref path) = *qpath;
             if path.segments.len() == 1;
-            if let Def::Local(local_id) = self.cx.tables.qpath_def(qpath, expr.hir_id);
             then {
-                if local_id == self.var {
-                    // we are not indexing anything, record that
-                    self.nonindex = true;
-                } else {
-                    // not the correct variable, but still a variable
-                    self.referenced.insert(path.segments[0].ident.name);
+                match self.cx.tables.qpath_def(qpath, expr.hir_id) {
+                    Def::Upvar(local_id, ..) => {
+                        if local_id == self.var {
+                            // we are not indexing anything, record that
+                            self.nonindex = true;
+                        }
+                    }
+                    Def::Local(local_id) =>
+                    {
+
+                        if local_id == self.var {
+                            self.nonindex = true;
+                        } else {
+                            // not the correct variable, but still a variable
+                            self.referenced.insert(path.segments[0].ident.name);
+                        }
+                    }
+                    _ => {}
                 }
             }
         }
+
         let old = self.prefer_mutable;
         match expr.node {
             ExprKind::AssignOp(_, ref lhs, ref rhs) | ExprKind::Assign(ref lhs, ref rhs) => {
@@ -1880,6 +1892,10 @@ impl<'a, 'tcx> Visitor<'tcx> for VarVisitor<'a, 'tcx> {
                     self.visit_expr(expr);
                 }
             },
+            ExprKind::Closure(_, _, body_id, ..) => {
+                let body = self.cx.tcx.hir().body(body_id);
+                self.visit_expr(&body.value);
+            },
             _ => walk_expr(self, expr),
         }
         self.prefer_mutable = old;

tests/ui/needless_range_loop.rs

@@ -80,4 +80,9 @@ fn main() {
     for i in 1..3 {
         println!("{}", arr[i]);
     }
+
+    // #2542
+    for i in 0..vec.len() {
+        vec[i] = Some(1).unwrap_or_else(|| panic!("error on {}", i));
+    }
 }

tests/ui/needless_range_loop.stderr

@@ -80,5 +80,15 @@ help: consider using an iterator
 LL |     for <item> in arr.iter().skip(1) {
    |         ^^^^^^    ^^^^^^^^^^^^^^^^^^
 
-error: aborting due to 8 previous errors
+error: the loop variable `i` is used to index `vec`
+  --> $DIR/needless_range_loop.rs:85:14
+   |
+LL |     for i in 0..vec.len() {
+   |              ^^^^^^^^^^^^
+help: consider using an iterator
+   |
+LL |     for (i, <item>) in vec.iter_mut().enumerate() {
+   |         ^^^^^^^^^^^    ^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: aborting due to 9 previous errors