Fix FP with mut_mut and for loops

Author: mcarton

clippy_lints/src/mut_mut.rs

@@ -1,7 +1,8 @@
+use rustc::hir;
+use rustc::hir::intravisit;
 use rustc::lint::*;
 use rustc::ty::{TypeAndMut, TyRef};
-use rustc::hir::*;
-use utils::{in_external_macro, span_lint};
+use utils::{in_external_macro, recover_for_loop, span_lint};
 
 /// **What it does:** This lint checks for instances of `mut mut` references.
 ///
@@ -27,30 +28,56 @@ impl LintPass for MutMut {
 }
 
 impl LateLintPass for MutMut {
-    fn check_expr(&mut self, cx: &LateContext, expr: &Expr) {
-        if in_external_macro(cx, expr.span) {
+    fn check_block(&mut self, cx: &LateContext, block: &hir::Block) {
+        intravisit::walk_block(&mut MutVisitor { cx: cx }, block);
+    }
+
+    fn check_ty(&mut self, cx: &LateContext, ty: &hir::Ty) {
+        use rustc::hir::intravisit::Visitor;
+
+        MutVisitor { cx: cx }.visit_ty(ty);
+    }
+}
+
+pub struct MutVisitor<'a, 'tcx: 'a> {
+    cx: &'a LateContext<'a, 'tcx>,
+}
+
+impl<'a, 'tcx, 'v> intravisit::Visitor<'v> for MutVisitor<'a, 'tcx> {
+    fn visit_expr(&mut self, expr: &'v hir::Expr) {
+        if in_external_macro(self.cx, expr.span) {
             return;
         }
 
-        if let ExprAddrOf(MutMutable, ref e) = expr.node {
-            if let ExprAddrOf(MutMutable, _) = e.node {
-                span_lint(cx, MUT_MUT, expr.span, "generally you want to avoid `&mut &mut _` if possible");
-            } else {
-                if let TyRef(_, TypeAndMut { mutbl: MutMutable, .. }) = cx.tcx.expr_ty(e).sty {
-                    span_lint(cx,
-                              MUT_MUT,
-                              expr.span,
-                              "this expression mutably borrows a mutable reference. Consider reborrowing");
-                }
+        if let Some((_, arg, body)) = recover_for_loop(expr) {
+            // A `for` loop lowers to:
+            // ```rust
+            // match ::std::iter::Iterator::next(&mut iter) {
+            // //                                ^^^^
+            // ```
+            // Let's ignore the generated code.
+            intravisit::walk_expr(self, arg);
+            intravisit::walk_expr(self, body);
+        } else if let hir::ExprAddrOf(hir::MutMutable, ref e) = expr.node {
+            if let hir::ExprAddrOf(hir::MutMutable, _) = e.node {
+                span_lint(self.cx, MUT_MUT, expr.span, "generally you want to avoid `&mut &mut _` if possible");
+            } else if let TyRef(_, TypeAndMut { mutbl: hir::MutMutable, .. }) = self.cx.tcx.expr_ty(e).sty {
+                span_lint(self.cx,
+                          MUT_MUT,
+                          expr.span,
+                          "this expression mutably borrows a mutable reference. Consider reborrowing");
             }
         }
     }
 
-    fn check_ty(&mut self, cx: &LateContext, ty: &Ty) {
-        if let TyRptr(_, MutTy { ty: ref pty, mutbl: MutMutable }) = ty.node {
-            if let TyRptr(_, MutTy { mutbl: MutMutable, .. }) = pty.node {
-                span_lint(cx, MUT_MUT, ty.span, "generally you want to avoid `&mut &mut _` if possible");
+    fn visit_ty(&mut self, ty: &hir::Ty) {
+        if let hir::TyRptr(_, hir::MutTy { ty: ref pty, mutbl: hir::MutMutable }) = ty.node {
+            if let hir::TyRptr(_, hir::MutTy { mutbl: hir::MutMutable, .. }) = pty.node {
+                span_lint(self.cx, MUT_MUT, ty.span, "generally you want to avoid `&mut &mut _` if possible");
             }
+
         }
+
+        intravisit::walk_ty(self, ty);
     }
 }

tests/compile-fail/mut_mut.rs

@@ -2,16 +2,15 @@
 #![plugin(clippy)]
 
 #![allow(unused, no_effect, unnecessary_operation)]
+#![deny(mut_mut)]
 
 //#![plugin(regex_macros)]
 //extern crate regex;
 
-#[deny(mut_mut)]
 fn fun(x : &mut &mut u32) -> bool { //~ERROR generally you want to avoid `&mut &mut
     **x > 0
 }
 
-#[deny(mut_mut)]
 fn less_fun(x : *mut *mut u32) {
   let y = x;
 }
@@ -21,23 +20,45 @@ macro_rules! mut_ptr {
     //~^ ERROR generally you want to avoid `&mut &mut
 }
 
-#[deny(mut_mut)]
 #[allow(unused_mut, unused_variables)]
 fn main() {
     let mut x = &mut &mut 1u32; //~ERROR generally you want to avoid `&mut &mut
     {
         let mut y = &mut x; //~ERROR this expression mutably borrows a mutable reference
     }
 
+    if fun(x) {
+        let y : &mut &mut u32 = &mut &mut 2;
+        //~^ ERROR generally you want to avoid `&mut &mut
+        //~| ERROR generally you want to avoid `&mut &mut
+        //~| ERROR generally you want to avoid `&mut &mut
+        **y + **x;
+    }
+
     if fun(x) {
         let y : &mut &mut &mut u32 = &mut &mut &mut 2;
         //~^ ERROR generally you want to avoid `&mut &mut
         //~| ERROR generally you want to avoid `&mut &mut
         //~| ERROR generally you want to avoid `&mut &mut
         //~| ERROR generally you want to avoid `&mut &mut
+        //~| ERROR generally you want to avoid `&mut &mut
+        //~| ERROR generally you want to avoid `&mut &mut
         ***y + **x;
     }
 
     let mut z = mut_ptr!(&mut 3u32);
     //~^ NOTE in this expansion of mut_ptr!
 }
+
+fn issue939() {
+    let array = [5, 6, 7, 8, 9];
+    let mut args = array.iter().skip(2);
+    for &arg in &mut args {
+        println!("{}", arg);
+    }
+
+    let args = &mut args;
+    for arg in args {
+        println!(":{}", arg);
+    }
+}