Merge pull request #160 from Manishearth/dogfood

Start dogfooding clippy

diff --git a/.travis.yml b/.travis.yml
index bd997a0c22efda62ae466251fa2c9a1919860644..7eaa61c557212e6a1597ca1ecc9790face5c47ce 100644 (file)
--- a/.travis.yml
+++ b/.travis.yml
@@ -5,3 +5,4 @@ sudo: false
 script:
  - python util/update_lints.py -c
  - cargo test
+ - bash util/dogfood.sh

diff --git a/src/collapsible_if.rs b/src/collapsible_if.rs
index f0f53622c47dbd34c65a18387b4a4c1584578f2e..be34458e0dc87b32a693fd8b232b13dda8c8c2a6 100644 (file)
--- a/src/collapsible_if.rs
+++ b/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 {} && {} {}",

diff --git a/src/eq_op.rs b/src/eq_op.rs
index 495696b810cf60b7ac15baaecf67511095d23284..0b7511e7dbd9a8d1b5ac529f2b3de9be12240fd2 100644 (file)
--- a/src/eq_op.rs
+++ b/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;

diff --git a/src/lib.rs b/src/lib.rs
index 7cb2877b2fdf05841ee4634d9d6508ef6b8f4fc1..9135ecaca6c93dfbecbbcc395210d71693132f63 100755 (executable)
--- a/src/lib.rs
+++ b/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;

diff --git a/src/lifetimes.rs b/src/lifetimes.rs
index 644537f9be173de9808f9cf94a806bf6b887cb6a..c3c915ea777e7ef2a28259bec21fa1a26561b27e 100644 (file)
--- a/src/lifetimes.rs
+++ b/src/lifetimes.rs
@@ -18,14 +18,23 @@ fn get_lints(&self) -> LintArray {
         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()
 }
 

diff --git a/src/loops.rs b/src/loops.rs
index bfb5ad6861daad31ccbe8a6774a8fb3be7178586..74015bdc6beddf676a7645d293aada3104d93912 100644 (file)
--- a/src/loops.rs
+++ b/src/loops.rs
@@ -31,7 +31,7 @@ fn check_expr(&mut self, cx: &Context, expr: &Expr) {
                     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 @@ fn check_expr(&mut self, cx: &Context, expr: &Expr) {
 
 /// 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,

diff --git a/src/misc.rs b/src/misc.rs
index 6c9a7d92ce8ce5d7d4a89ebfbec3c69f39ccd20d..7372bfed6c5455f9cca7067d5e4f19fec6a9f644 100644 (file)
--- a/src/misc.rs
+++ b/src/misc.rs
@@ -73,7 +73,7 @@ fn get_lints(&self) -> LintArray {
     }
 
     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,

diff --git a/src/utils.rs b/src/utils.rs
index 220dc6215fdbb17f1466559fc4bbe59c469eaa4f..c54a7f56f7e86781a7ea2dadab7b8c8b76661f81 100644 (file)
--- a/src/utils.rs
+++ b/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

diff --git a/tests/compile-fail/lifetimes.rs b/tests/compile-fail/lifetimes.rs
index 36daa69fb317b1c90d19df800a208c6a810fafe8..287a8199d2c148f31cd0359221e318135214e8b1 100755 (executable)
--- a/tests/compile-fail/lifetimes.rs
+++ b/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 @@ fn distinct_self_and_in<'s, 't>(&'s self, _x: &'t u8) { }
     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);
 }

diff --git a/util/dogfood.sh b/util/dogfood.sh
new file mode 100644 (file)
index 0000000..51dd465
--- /dev/null
+++ b/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
+