Merge pull request #160 from Manishearth/dogfood

Start dogfooding clippy

.travis.yml

@@ -5,3 +5,4 @@ sudo: false
 script:
  - python util/update_lints.py -c
  - cargo test
+ - bash util/dogfood.sh

src/collapsible_if.rs

@@ -45,8 +45,12 @@ fn check_expr_expd(cx: &Context, e: &Expr, info: Option<&ExpnInfo>) {
     if in_macro(cx, info) { return; }
 
     if let ExprIf(ref check, ref then, None) = e.node {
-        if let Some(&Expr{ node: ExprIf(ref check_inner, ref content, None), ..}) =
+        if let Some(&Expr{ node: ExprIf(ref check_inner, ref content, None), span: sp, ..}) =
             single_stmt_of_block(then) {
+                if e.span.expn_id != sp.expn_id {
+                    return;
+                }
+                cx.sess().note(&format!("{:?} -- {:?}", e.span, sp));
                 span_help_and_lint(cx, COLLAPSIBLE_IF, e.span,
                     "this if statement can be collapsed",
                     &format!("try\nif {} && {} {}",

src/eq_op.rs

@@ -1,3 +1,4 @@
+#![allow(redundant_closure)] // FIXME (#116)
 use rustc::lint::*;
 use syntax::ast::*;
 use syntax::ast_util as ast_util;

src/lib.rs

@@ -1,6 +1,6 @@
 #![feature(plugin_registrar, box_syntax)]
 #![feature(rustc_private, collections)]
-#![allow(unused_imports)]
+#![allow(unused_imports, unknown_lints)]
 
 #[macro_use]
 extern crate syntax;

src/lifetimes.rs

@@ -18,14 +18,23 @@ impl LintPass for LifetimePass {
         lint_array!(NEEDLESS_LIFETIMES)
     }
 
-    fn check_fn(&mut self, cx: &Context, kind: FnKind, decl: &FnDecl,
-                _: &Block, span: Span, _: NodeId) {
-        if in_external_macro(cx, span) {
-            return;
+    fn check_item(&mut self, cx: &Context, item: &Item) {
+        if let ItemFn(ref decl, _, _, _, ref generics, _) = item.node {
+            check_fn_inner(cx, decl, None, &generics.lifetimes, item.span);
         }
-        if could_use_elision(kind, decl) {
-            span_lint(cx, NEEDLESS_LIFETIMES, span,
-                      "explicit lifetimes given in parameter types where they could be elided");
+    }
+
+    fn check_impl_item(&mut self, cx: &Context, item: &ImplItem) {
+        if let MethodImplItem(ref sig, _) = item.node {
+            check_fn_inner(cx, &*sig.decl, Some(&sig.explicit_self),
+                           &sig.generics.lifetimes, item.span);
+        }
+    }
+
+    fn check_trait_item(&mut self, cx: &Context, item: &TraitItem) {
+        if let MethodTraitItem(ref sig, _) = item.node {
+            check_fn_inner(cx, &*sig.decl, Some(&sig.explicit_self),
+                           &sig.generics.lifetimes, item.span);
         }
     }
 }
@@ -39,10 +48,34 @@ enum RefLt {
 }
 use self::RefLt::*;
 
-fn could_use_elision(kind: FnKind, func: &FnDecl) -> bool {
+fn check_fn_inner(cx: &Context, decl: &FnDecl, slf: Option<&ExplicitSelf>,
+                  named_lts: &[LifetimeDef], span: Span) {
+    if in_external_macro(cx, span) {
+        return;
+    }
+    if could_use_elision(decl, slf, named_lts) {
+        span_lint(cx, NEEDLESS_LIFETIMES, span,
+                  "explicit lifetimes given in parameter types where they could be elided");
+    }
+}
+
+fn could_use_elision(func: &FnDecl, slf: Option<&ExplicitSelf>,
+                     named_lts: &[LifetimeDef]) -> bool {
     // There are two scenarios where elision works:
     // * no output references, all input references have different LT
     // * output references, exactly one input reference with same LT
+    // All lifetimes must be unnamed, 'static or defined without bounds on the
+    // level of the current item.
+
+    // check named LTs
+    let mut allowed_lts = HashSet::new();
+    for lt in named_lts {
+        if lt.bounds.is_empty() {
+            allowed_lts.insert(Named(lt.lifetime.name));
+        }
+    }
+    allowed_lts.insert(Unnamed);
+    allowed_lts.insert(Static);
 
     // these will collect all the lifetimes for references in arg/return types
     let mut input_visitor = RefVisitor(Vec::new());
@@ -50,8 +83,8 @@ fn could_use_elision(kind: FnKind, func: &FnDecl) -> bool {
 
     // extract lifetime in "self" argument for methods (there is a "self" argument
     // in func.inputs, but its type is TyInfer)
-    if let FnKind::FkMethod(_, sig, _) = kind {
-        match sig.explicit_self.node {
+    if let Some(slf) = slf {
+        match slf.node {
             SelfRegion(ref opt_lt, _, _) => input_visitor.record(opt_lt),
             SelfExplicit(ref ty, _) => walk_ty(&mut input_visitor, ty),
             _ => { }
@@ -69,6 +102,13 @@ fn could_use_elision(kind: FnKind, func: &FnDecl) -> bool {
     let input_lts = input_visitor.into_vec();
     let output_lts = output_visitor.into_vec();
 
+    // check for lifetimes from higher scopes
+    for lt in input_lts.iter().chain(output_lts.iter()) {
+        if !allowed_lts.contains(lt) {
+            return false;
+        }
+    }
+
     // no input lifetimes? easy case!
     if input_lts.is_empty() {
         return false;
@@ -103,7 +143,7 @@ fn could_use_elision(kind: FnKind, func: &FnDecl) -> bool {
 }
 
 /// Number of unique lifetimes in the given vector.
-fn unique_lifetimes(lts: &Vec<RefLt>) -> usize {
+fn unique_lifetimes(lts: &[RefLt]) -> usize {
     lts.iter().collect::<HashSet<_>>().len()
 }
 

src/loops.rs

@@ -31,7 +31,7 @@ impl LintPass for LoopsPass {
                     walk_expr(&mut visitor, body);
                     // linting condition: we only indexed one variable
                     if visitor.indexed.len() == 1 {
-                        let indexed = visitor.indexed.into_iter().next().unwrap();
+                        let indexed = visitor.indexed.into_iter().next().expect("Len was nonzero, but no contents found");
                         if visitor.nonindex {
                             span_lint(cx, NEEDLESS_RANGE_LOOP, expr.span, &format!(
                                 "the loop variable `{}` is used to index `{}`. Consider using \
@@ -72,13 +72,12 @@ impl LintPass for LoopsPass {
 
 /// Recover the essential nodes of a desugared for loop:
 /// `for pat in arg { body }` becomes `(pat, arg, body)`.
-fn recover_for_loop<'a>(expr: &'a Expr) -> Option<(&'a Pat, &'a Expr, &'a Expr)> {
+fn recover_for_loop(expr: &Expr) -> Option<(&Pat, &Expr, &Expr)> {
     if_let_chain! {
         [
             let ExprMatch(ref iterexpr, ref arms, _) = expr.node,
             let ExprCall(_, ref iterargs) = iterexpr.node,
-            iterargs.len() == 1,
-            arms.len() == 1 && arms[0].guard.is_none(),
+            iterargs.len() == 1 && arms.len() == 1 && arms[0].guard.is_none(),
             let ExprLoop(ref block, _) = arms[0].body.node,
             block.stmts.is_empty(),
             let Some(ref loopexpr) = block.expr,

src/misc.rs

@@ -73,7 +73,7 @@ impl LintPass for TopLevelRefPass {
     }
 
     fn check_fn(&mut self, cx: &Context, _: FnKind, decl: &FnDecl, _: &Block, _: Span, _: NodeId) {
-        for ref arg in decl.inputs.iter() {
+        for ref arg in &decl.inputs {
             if let PatIdent(BindByRef(_), _, _) = arg.pat.node {
                 span_lint(cx,
                     TOPLEVEL_REF_ARG,

src/utils.rs

@@ -86,7 +86,7 @@ pub fn span_help_and_lint(cx: &Context, lint: &'static Lint, span: Span,
 }
 
 /// return the base type for references and raw pointers
-pub fn walk_ptrs_ty<'t>(ty: ty::Ty<'t>) -> ty::Ty<'t> {
+pub fn walk_ptrs_ty(ty: ty::Ty) -> ty::Ty {
     match ty.sty {
         ty::TyRef(_, ref tm) | ty::TyRawPtr(ref tm) => walk_ptrs_ty(tm.ty),
         _ => ty

tests/compile-fail/lifetimes.rs

@@ -2,7 +2,7 @@
 #![plugin(clippy)]
 
 #![deny(needless_lifetimes)]
-
+#![allow(dead_code)]
 fn distinct_lifetimes<'a, 'b>(_x: &'a u8, _y: &'b u8, _z: u8) { }
 //~^ERROR explicit lifetimes given
 
@@ -31,11 +31,13 @@ fn deep_reference_3<'a>(x: &'a u8, _y: u8) -> Result<&'a u8, ()> { Ok(x) }
 
 type Ref<'r> = &'r u8;
 
-fn lifetime_param_1<'a>(_x: Ref<'a>, _y: &'a u8) { }
+fn lifetime_param_1<'a>(_x: Ref<'a>, _y: &'a u8) { } // no error, same lifetime on two params
 
-fn lifetime_param_2<'a, 'b: 'a>(_x: Ref<'a>, _y: &'b u8) { }
+fn lifetime_param_2<'a, 'b>(_x: Ref<'a>, _y: &'b u8) { }
 //~^ERROR explicit lifetimes given
 
+fn lifetime_param_3<'a, 'b: 'a>(_x: Ref<'a>, _y: &'b u8) { } // no error, bounded lifetime
+
 struct X {
     x: u8,
 }
@@ -52,26 +54,13 @@ impl X {
     fn self_and_same_in<'s>(&'s self, _x: &'s u8) { } // no error, same lifetimes on two params
 }
 
+struct Foo<'a>(&'a u8);
+
+impl<'a> Foo<'a> {
+    fn self_shared_lifetime(&self, _: &'a u8) {} // no error, lifetime 'a not defined in method
+    fn self_bound_lifetime<'b: 'a>(&self, _: &'b u8) {} // no error, bounds exist
+}
 static STATIC: u8 = 1;
 
 fn main() {
-    distinct_lifetimes(&1, &2, 3);
-    distinct_and_static(&1, &2, &STATIC);
-    same_lifetime_on_input(&1, &2);
-    only_static_on_input(&1, &2, &STATIC);
-    in_and_out(&1, 2);
-    multiple_in_and_out_1(&1, &2);
-    multiple_in_and_out_2(&1, &2);
-    in_static_and_out(&1, &STATIC);
-    let _ = deep_reference_1(&1, &2);
-    let _ = deep_reference_2(Ok(&1));
-    let _ = deep_reference_3(&1, 2);
-    lifetime_param_1(&1, &2);
-    lifetime_param_2(&1, &2);
-
-    let foo = X { x: 1 };
-    foo.self_and_out();
-    foo.self_and_in_out(&1);
-    foo.distinct_self_and_in(&1);
-    foo.self_and_same_in(&1);
 }

util/dogfood.sh

@@ -0,0 +1,4 @@
+rm -rf target*/*so
+cargo build --lib && cp -R target target_recur && cargo rustc -- -Zextra-plugins=clippy -Ltarget_recur/debug   -Dclippy || exit 1
+rm -rf target_recur
+