diff --git a/src/lib.rs b/src/lib.rs index 7d7eff355..69a373b21 100755 --- a/src/lib.rs +++ b/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, ]); } diff --git a/src/lifetimes.rs b/src/lifetimes.rs new file mode 100644 index 000000000..602eea1ae --- /dev/null +++ b/src/lifetimes.rs @@ -0,0 +1,126 @@ +use syntax::ast::*; +use rustc::lint::{Context, LintPass, LintArray, Lint}; +use syntax::codemap::Span; +use syntax::visit::{Visitor, FnKind, walk_ty}; +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 references, exactly one input reference with same LT + + let mut input_visitor = RefVisitor(Vec::new()); + let mut output_visitor = RefVisitor(Vec::new()); + + // extract lifetimes of input argument types + for arg in &func.inputs { + walk_ty(&mut input_visitor, &*arg.ty); + } + // extract lifetime of "self" argument for methods + if let FnKind::FkMethod(_, sig, _) = kind { + match sig.explicit_self.node { + SelfRegion(ref lt_opt, _, _) => + input_visitor.visit_opt_lifetime_ref(sig.explicit_self.span, lt_opt), + SelfExplicit(ref ty, _) => + walk_ty(&mut input_visitor, ty), + _ => { } + } + } + // extract lifetimes of 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 +} + +fn unique_lifetimes(lts: &Vec) -> usize { + let set: HashSet<&RefLt> = HashSet::from_iter(lts.iter()); + set.len() +} + +struct RefVisitor(Vec); + +impl RefVisitor { + fn into_vec(self) -> Vec { self.0 } +} + +impl<'v> Visitor<'v> for RefVisitor { + fn visit_opt_lifetime_ref(&mut self, _: Span, lifetime: &'v Option) { + 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); + } + } +} diff --git a/tests/compile-fail/lifetimes.rs b/tests/compile-fail/lifetimes.rs new file mode 100755 index 000000000..9f2a3ee99 --- /dev/null +++ b/tests/compile-fail/lifetimes.rs @@ -0,0 +1,62 @@ +#![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 + +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); + + 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); +}