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) {
@@ -59,6 +60,7 @@ pub fn plugin_registrar(reg: &mut Registry) {
     reg.register_lint_pass(box returns::ReturnPass as LintPassObject);
     reg.register_lint_pass(box methods::MethodsPass as LintPassObject);
     reg.register_lint_pass(box types::LetPass 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,
@@ -87,5 +89,6 @@ pub fn plugin_registrar(reg: &mut Registry) {
                                            methods::STR_TO_STRING,
                                            methods::STRING_TO_STRING,
                                            types::LET_UNIT_VALUE,
+                                           lifetimes::NEEDLESS_LIFETIMES,
                                            ]);
 }

src/lifetimes.rs

@@ -0,0 +1,143 @@
+use syntax::ast::*;
+use rustc::lint::{Context, LintPass, LintArray, Lint};
+use syntax::codemap::Span;
+use syntax::visit::{Visitor, FnKind, walk_ty};
+use utils::{in_external_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 in_external_macro(cx, span) {
+            return;
+        }
+        if could_use_elision(kind, decl) {
+            span_lint(cx, NEEDLESS_LIFETIMES, span,
+                      "explicit lifetimes given in parameter types where they could be elided");
+        }
+    }
+}
+
+/// The lifetime of a &-reference.
+#[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 references, exactly one input reference with same LT
+
+    // these will collect all the lifetimes for references in arg/return types
+    let mut input_visitor = RefVisitor(Vec::new());
+    let mut output_visitor = RefVisitor(Vec::new());
+
+    // 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 {
+            SelfRegion(ref opt_lt, _, _) => input_visitor.record(opt_lt),
+            SelfExplicit(ref ty, _) => walk_ty(&mut input_visitor, ty),
+            _ => { }
+        }
+    }
+    // extract lifetimes in input argument types
+    for arg in &func.inputs {
+        walk_ty(&mut input_visitor, &*arg.ty);
+    }
+    // extract lifetimes in output type
+    if let Return(ref ty) = func.output {
+        walk_ty(&mut output_visitor, ty);
+    }
+
+    let input_lts = input_visitor.into_vec();
+    let output_lts = output_visitor.into_vec();
+
+    // no input lifetimes? easy case!
+    if input_lts.is_empty() {
+        return false;
+    } else if output_lts.is_empty() {
+        // no output lifetimes, check distinctness of input lifetimes
+
+        // 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
+        if input_lts.len() == unique_lifetimes(&input_lts) {
+            return true;
+        }
+    } else {
+        // we have output references, so we need one input reference,
+        // and all output lifetimes must be the same
+        if unique_lifetimes(&output_lts) > 1 {
+            return false;
+        }
+        if input_lts.len() == 1 {
+            match (&input_lts[0], &output_lts[0]) {
+                (&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
+            }
+        }
+    }
+    false
+}
+
+/// Number of unique lifetimes in the given vector.
+fn unique_lifetimes(lts: &Vec<RefLt>) -> usize {
+    lts.iter().collect::<HashSet<_>>().len()
+}
+
+/// A visitor usable for syntax::visit::walk_ty().
+struct RefVisitor(Vec<RefLt>);
+
+impl RefVisitor {
+    fn record(&mut self, lifetime: &Option<Lifetime>) {
+        if let &Some(ref lt) = lifetime {
+            if lt.name.as_str() == "'static" {
+                self.0.push(Static);
+            } else {
+                self.0.push(Named(lt.name));
+            }
+        } else {
+            self.0.push(Unnamed);
+        }
+    }
+
+    fn into_vec(self) -> Vec<RefLt> {
+        self.0
+    }
+}
+
+impl<'v> Visitor<'v> for RefVisitor {
+    // for lifetimes of references
+    fn visit_opt_lifetime_ref(&mut self, _: Span, lifetime: &'v Option<Lifetime>) {
+        self.record(lifetime);
+    }
+
+    // for lifetimes as parameters of generics
+    fn visit_lifetime_ref(&mut self, lifetime: &'v Lifetime) {
+        self.record(&Some(*lifetime));
+    }
+
+    // for lifetime bounds; the default impl calls visit_lifetime_ref
+    fn visit_lifetime_bound(&mut self, _: &'v Lifetime) { }
+}

tests/compile-fail/lifetimes.rs

@@ -0,0 +1,70 @@
+#![feature(plugin)]
+#![plugin(clippy)]
+
+#![deny(needless_lifetimes)]
+
+fn distinct_lifetimes<'a, 'b>(_x: &'a u8, _y: &'b u8, _z: u8) { } //~ERROR
+
+fn distinct_and_static<'a, 'b>(_x: &'a u8, _y: &'b u8, _z: &'static u8) { } //~ERROR
+
+fn same_lifetime_on_input<'a>(_x: &'a u8, _y: &'a u8) { } // no error, same lifetime on two params
+
+fn only_static_on_input(_x: &u8, _y: &u8, _z: &'static u8) { } // no error, static involved
+
+fn in_and_out<'a>(x: &'a u8, _y: u8) -> &'a u8 { x } //~ERROR
+
+fn multiple_in_and_out_1<'a>(x: &'a u8, _y: &'a u8) -> &'a u8 { x } // no error, multiple input refs
+
+fn multiple_in_and_out_2<'a, 'b>(x: &'a u8, _y: &'b u8) -> &'a u8 { x } // no error, multiple input refs
+
+fn in_static_and_out<'a>(x: &'a u8, _y: &'static u8) -> &'a u8 { x } // no error, static involved
+
+fn deep_reference_1<'a, 'b>(x: &'a u8, _y: &'b u8) -> Result<&'a u8, ()> { Ok(x) } // no error
+
+fn deep_reference_2<'a>(x: Result<&'a u8, &'a u8>) -> &'a u8 { x.unwrap() } // no error, two input refs
+
+fn deep_reference_3<'a>(x: &'a u8, _y: u8) -> Result<&'a u8, ()> { Ok(x) } //~ERROR
+
+type Ref<'r> = &'r u8;
+
+fn lifetime_param_1<'a>(_x: Ref<'a>, _y: &'a u8) { }
+
+fn lifetime_param_2<'a, 'b: 'a>(_x: Ref<'a>, _y: &'b u8) { } //~ERROR
+
+struct X {
+    x: u8,
+}
+
+impl X {
+    fn self_and_out<'s>(&'s self) -> &'s u8 { &self.x } //~ERROR
+
+    fn self_and_in_out<'s, 't>(&'s self, _x: &'t u8) -> &'s u8 { &self.x } // no error, multiple input refs
+
+    fn distinct_self_and_in<'s, 't>(&'s self, _x: &'t u8) { } //~ERROR
+
+    fn self_and_same_in<'s>(&'s self, _x: &'s u8) { } // no error, same lifetimes on two params
+}
+
+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);
+}