new lint for needless lifetimes (fixes #115)

src/lib.rs

@@ -32,6 +32,7 @@ pub mod unicode;
 pub mod strings;
 pub mod methods;
 pub mod returns;
+pub mod lifetimes;
 
 #[plugin_registrar]
 pub fn plugin_registrar(reg: &mut Registry) {
@@ -58,6 +59,7 @@ pub fn plugin_registrar(reg: &mut Registry) {
     reg.register_lint_pass(box strings::StringAdd as LintPassObject);
     reg.register_lint_pass(box returns::ReturnPass as LintPassObject);
     reg.register_lint_pass(box methods::MethodsPass as LintPassObject);
+    reg.register_lint_pass(box lifetimes::LifetimePass as LintPassObject);
 
     reg.register_lint_group("clippy", vec![types::BOX_VEC, types::LINKEDLIST,
                                            misc::SINGLE_MATCH,
@@ -83,5 +85,6 @@ pub fn plugin_registrar(reg: &mut Registry) {
                                            methods::OPTION_UNWRAP_USED,
                                            methods::RESULT_UNWRAP_USED,
                                            methods::STR_TO_STRING,
+                                           lifetimes::NEEDLESS_LIFETIMES,
                                            ]);
 }

src/lifetimes.rs

@@ -0,0 +1,97 @@
+use syntax::ast::*;
+use rustc::lint::{Context, LintPass, LintArray, Lint};
+use syntax::codemap::Span;
+use syntax::visit::FnKind;
+use utils::{in_macro, span_lint};
+use std::collections::HashSet;
+use std::iter::FromIterator;
+
+declare_lint!(pub NEEDLESS_LIFETIMES, Warn,
+              "Warn on explicit lifetimes when elision rules would apply");
+
+#[derive(Copy,Clone)]
+pub struct LifetimePass;
+
+impl LintPass for LifetimePass {
+    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 cx.sess().codemap().with_expn_info(span.expn_id, |info| in_macro(cx, info)) {
+            return;
+        }
+        if could_use_elision(kind, decl) {
+            span_lint(cx, NEEDLESS_LIFETIMES, span,
+                      "explicit lifetimes given where they could be inferred");
+        }
+    }
+}
+
+#[derive(PartialEq, Eq, Hash, Debug)]
+enum RefLt {
+    Unnamed,
+    Static,
+    Named(Name),
+}
+use self::RefLt::*;
+
+fn could_use_elision(kind: FnKind, func: &FnDecl) -> bool {
+    // There are two scenarios where elision works:
+    // * no output references, all input references have different LT
+    // * output reference, exactly one input reference with same LT
+
+    // extract lifetime of input arguments
+    let mut input_lts = func.inputs.iter().filter_map(|arg| extract_lifetime(&*arg.ty))
+                                          .collect::<Vec<_>>();
+    // extract lifetime of "self" for methods
+    if let FnKind::FkMethod(_, sig, _) = kind {
+        if let SelfRegion(ref lt_opt, _, _) = sig.explicit_self.node {
+            input_lts.push(match *lt_opt {
+                None     => Unnamed,
+                Some(lt) => Named(lt.name),
+            });
+        }
+    }
+    // no input arguments? easy case!
+    if input_lts.is_empty() {
+        return false;
+    }
+    let output_lt = match func.output {
+        Return(ref ty) => extract_lifetime(ty),
+        _              => None,
+    };
+    if let Some(output_lt) = output_lt {
+        // we have an output reference, so we need one input reference
+        if input_lts.len() == 1 {
+            match (&input_lts[0], output_lt) {
+                (&Named(n1), Named(n2)) if n1 == n2 => { return true; }
+                (&Named(_), Unnamed) => { return true; }
+                (&Unnamed, Named(_)) => { return true; }
+                _ => { } // already elided, different named lifetimes
+                         // or something static going on
+            }
+        }
+    } else {
+        // only one reference with unnamed lifetime, ok
+        if input_lts.len() == 1 && input_lts[0] == Unnamed {
+            return false;
+        }
+        // we have no output reference, so we only need all distinct lifetimes
+        let s: HashSet<&RefLt> = HashSet::from_iter(input_lts.iter());
+        if s.len() == input_lts.len()  {
+            return true;
+        }
+    }
+    false
+}
+
+fn extract_lifetime(ty: &Ty) -> Option<RefLt> {
+    match ty.node {
+        TyRptr(None, _) => Some(Unnamed),
+        TyRptr(Some(lt), _) if lt.name.as_str() == "'static" => Some(Static),
+        TyRptr(Some(lt), _) => Some(Named(lt.name)),
+        _ => None,
+    }
+}

tests/compile-fail/lifetimes.rs

@@ -0,0 +1,52 @@
+#![feature(plugin)]
+#![plugin(clippy)]
+
+#![deny(needless_lifetimes)]
+
+fn test1a<'a, 'b>(_x: &'a u8, _y: &'b u8, _z: u8) { } //~ERROR
+
+fn test1b<'a, 'b>(_x: &'a u8, _y: &'b u8, _z: &'static u8) { } //~ERROR
+
+fn test1c<'a>(_x: &'a u8, _y: &'a u8) { } // no error, same lifetime on two params
+
+fn test1d(_x: &u8, _y: &u8, _z: &'static u8) { } // no error, static involved
+
+fn test2a<'a>(x: &'a u8, _y: u8) -> &'a u8 { x } //~ERROR
+
+fn test2b<'a>(x: &'a u8, _y: &'a u8) -> &'a u8 { x } // no warning, multiple input refs
+
+fn test2c<'a, 'b>(x: &'a u8, _y: &'b u8) -> &'a u8 { x } // no warning, multiple input refs
+
+fn test2d<'a>(x: &'a u8, _y: &'static u8) -> &'a u8 { x } // no warning, static involved
+
+struct X {
+    x: u8,
+}
+
+impl X {
+    fn test3a<'s>(&'s self) -> &'s u8 { &self.x } //~ERROR
+
+    fn test3b<'s, 't>(&'s self, _x: &'t u8) -> &'s u8 { &self.x } // no error, multiple input refs
+
+    fn test3c<'s, 't>(&'s self, _x: &'t u8) { } //~ERROR
+
+    fn test3d<'s>(&'s self, _x: &'s u8) { } // no error, same lifetimes on two params
+}
+
+static STATIC: u8 = 1;
+
+fn main() {
+    test1a(&1, &2, 3);
+    test1b(&1, &2, &STATIC);
+    test1c(&1, &2);
+    test1d(&1, &2, &STATIC);
+    test2a(&1, 2);
+    test2b(&1, &2);
+    test2c(&1, &2);
+    test2d(&1, &STATIC);
+    let foo = X { x: 1 };
+    foo.test3a();
+    foo.test3b(&1);
+    foo.test3c(&1);
+    foo.test3d(&1);
+}