new lint for needless lifetimes (fixes #115)

This commit is contained in:
Georg Brandl 2015-08-12 13:16:09 +02:00
parent dbd396db91
commit b349f9e88d
3 changed files with 191 additions and 0 deletions

View File

@ -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,
]);
}

126
src/lifetimes.rs Normal file
View File

@ -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<RefLt>) -> usize {
let set: HashSet<&RefLt> = HashSet::from_iter(lts.iter());
set.len()
}
struct RefVisitor(Vec<RefLt>);
impl RefVisitor {
fn into_vec(self) -> Vec<RefLt> { self.0 }
}
impl<'v> Visitor<'v> for RefVisitor {
fn visit_opt_lifetime_ref(&mut self, _: Span, lifetime: &'v 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);
}
}
}

62
tests/compile-fail/lifetimes.rs Executable file
View File

@ -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);
}