From 7fa38f678764d7d5203cd4155eb39a355857e1ce Mon Sep 17 00:00:00 2001 From: mcarton Date: Tue, 21 Jun 2016 22:54:22 +0200 Subject: [PATCH] Fix FP with `mut_mut` and `for` loops --- clippy_lints/src/mut_mut.rs | 63 +++++++++++++++++++++++++---------- tests/compile-fail/mut_mut.rs | 27 +++++++++++++-- 2 files changed, 69 insertions(+), 21 deletions(-) diff --git a/clippy_lints/src/mut_mut.rs b/clippy_lints/src/mut_mut.rs index 0bed45b0b5b..8a5439fb11e 100644 --- a/clippy_lints/src/mut_mut.rs +++ b/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 @@ fn get_lints(&self) -> LintArray { } 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); } } diff --git a/tests/compile-fail/mut_mut.rs b/tests/compile-fail/mut_mut.rs index 21c0dcee511..edcc6906f08 100644 --- a/tests/compile-fail/mut_mut.rs +++ b/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,7 +20,6 @@ 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 @@ -29,15 +27,38 @@ fn main() { 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); + } +} -- 2.44.0