diff --git a/README.md b/README.md index fbd6f4ebf..212d868c6 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@ A collection of lints to catch common mistakes and improve your Rust code. [Jump to usage instructions](#usage) ##Lints -There are 101 lints included in this crate: +There are 102 lints included in this crate: name | default | meaning ---------------------------------------------------------------------------------------------------------------|---------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ @@ -105,6 +105,7 @@ name [unused_lifetimes](https://github.com/Manishearth/rust-clippy/wiki#unused_lifetimes) | warn | unused lifetimes in function definitions [used_underscore_binding](https://github.com/Manishearth/rust-clippy/wiki#used_underscore_binding) | warn | using a binding which is prefixed with an underscore [useless_transmute](https://github.com/Manishearth/rust-clippy/wiki#useless_transmute) | warn | transmutes that have the same to and from types +[useless_vec](https://github.com/Manishearth/rust-clippy/wiki#useless_vec) | warn | useless `vec!` [while_let_loop](https://github.com/Manishearth/rust-clippy/wiki#while_let_loop) | warn | `loop { if let { ... } else break }` can be written as a `while let` loop [while_let_on_iterator](https://github.com/Manishearth/rust-clippy/wiki#while_let_on_iterator) | warn | using a while-let loop instead of a for loop on an iterator [wrong_pub_self_convention](https://github.com/Manishearth/rust-clippy/wiki#wrong_pub_self_convention) | allow | defining a public method named with an established prefix (like "into_") that takes `self` with the wrong convention diff --git a/src/lib.rs b/src/lib.rs index 8b115a304..90625e4bb 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -79,6 +79,7 @@ pub mod array_indexing; pub mod panic; pub mod derive; pub mod print; +pub mod vec; mod reexport { pub use syntax::ast::{Name, NodeId}; @@ -144,6 +145,7 @@ pub fn plugin_registrar(reg: &mut Registry) { reg.register_late_lint_pass(box derive::Derive); reg.register_late_lint_pass(box types::CharLitAsU8); reg.register_late_lint_pass(box print::PrintLint); + reg.register_late_lint_pass(box vec::UselessVec); reg.register_lint_group("clippy_pedantic", vec![ @@ -250,6 +252,7 @@ pub fn plugin_registrar(reg: &mut Registry) { types::TYPE_COMPLEXITY, types::UNIT_CMP, unicode::ZERO_WIDTH_SPACE, + vec::USELESS_VEC, zero_div_zero::ZERO_DIVIDED_BY_ZERO, ]); } diff --git a/src/utils.rs b/src/utils.rs index c59e35c5c..a6dbcfd9e 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -20,6 +20,7 @@ pub type MethodArgs = HirVec>; // module DefPaths for certain structs/enums we check for pub const BEGIN_UNWIND: [&'static str; 3] = ["std", "rt", "begin_unwind"]; +pub const BOX_NEW_PATH: [&'static str; 4] = ["std", "boxed", "Box", "new"]; pub const BTREEMAP_ENTRY_PATH: [&'static str; 4] = ["collections", "btree", "map", "Entry"]; pub const BTREEMAP_PATH: [&'static str; 4] = ["collections", "btree", "map", "BTreeMap"]; pub const CLONE_PATH: [&'static str; 3] = ["clone", "Clone", "clone"]; @@ -36,6 +37,7 @@ pub const OPEN_OPTIONS_PATH: [&'static str; 3] = ["std", "fs", "OpenOptions"]; pub const OPTION_PATH: [&'static str; 3] = ["core", "option", "Option"]; pub const RESULT_PATH: [&'static str; 3] = ["core", "result", "Result"]; pub const STRING_PATH: [&'static str; 3] = ["collections", "string", "String"]; +pub const VEC_FROM_ELEM_PATH: [&'static str; 3] = ["std", "vec", "from_elem"]; pub const VEC_PATH: [&'static str; 3] = ["collections", "vec", "Vec"]; /// Produce a nested chain of if-lets and ifs from the patterns: @@ -487,7 +489,7 @@ pub fn span_note_and_lint<'a, T: LintContext>(cx: &'a T, lint: &'static Lint, sp pub fn span_lint_and_then<'a, T: LintContext, F>(cx: &'a T, lint: &'static Lint, sp: Span, msg: &str, f: F) -> DiagnosticWrapper<'a> - where F: Fn(&mut DiagnosticWrapper) + where F: FnOnce(&mut DiagnosticWrapper) { let mut db = DiagnosticWrapper(cx.struct_span_lint(lint, sp, msg)); if cx.current_level(lint) != Level::Allow { diff --git a/src/vec.rs b/src/vec.rs new file mode 100644 index 000000000..b46795a3c --- /dev/null +++ b/src/vec.rs @@ -0,0 +1,111 @@ +use rustc::lint::*; +use rustc::middle::ty::TypeVariants; +use rustc_front::hir::*; +use syntax::codemap::Span; +use syntax::ptr::P; +use utils::{BOX_NEW_PATH, VEC_FROM_ELEM_PATH}; +use utils::{is_expn_of, match_path, snippet, span_lint_and_then}; + +/// **What it does:** This lint warns about using `&vec![..]` when using `&[..]` would be possible. +/// It is `Warn` by default. +/// +/// **Why is this bad?** This is less efficient. +/// +/// **Known problems:** None. +/// +/// **Example:** +/// ```rust, ignore +/// foo(&vec![1, 2]) +/// ``` +declare_lint! { + pub USELESS_VEC, + Warn, + "useless `vec!`" +} + +#[derive(Copy, Clone, Debug)] +pub struct UselessVec; + +impl LintPass for UselessVec { + fn get_lints(&self) -> LintArray { + lint_array!(USELESS_VEC) + } +} + +impl LateLintPass for UselessVec { + fn check_expr(&mut self, cx: &LateContext, expr: &Expr) { + unexpand_vec(cx, expr); + + // search for `&!vec[_]` expressions where the adjusted type is `&[_]` + if_let_chain!{[ + let TypeVariants::TyRef(_, ref ty) = cx.tcx.expr_ty_adjusted(expr).sty, + let TypeVariants::TySlice(..) = ty.ty.sty, + let ExprAddrOf(_, ref addressee) = expr.node, + let Some(vec_args) = unexpand_vec(cx, addressee) + ], { + let snippet = match vec_args { + VecArgs::Repeat(elem, len) => { + format!("&[{}; {}]", snippet(cx, elem.span, "elem"), snippet(cx, len.span, "len")).into() + } + VecArgs::Vec(args) => { + if let Some(last) = args.iter().last() { + let span = Span { + lo: args[0].span.lo, + hi: last.span.hi, + expn_id: args[0].span.expn_id, + }; + + format!("&[{}]", snippet(cx, span, "..")).into() + } + else { + "&[]".into() + } + } + }; + + span_lint_and_then(cx, USELESS_VEC, expr.span, "useless use of `vec!`", |db| { + db.span_suggestion(expr.span, "you can use a slice directly", snippet); + }); + }} + } +} + +/// Represent the pre-expansion arguments of a `vec!` invocation. +pub enum VecArgs<'a> { + /// `vec![elem, len]` + Repeat(&'a P, &'a P), + /// `vec![a, b, c]` + Vec(&'a [P]), +} + +/// Returns the arguments of the `vec!` macro if this expression was expanded from `vec!`. +pub fn unexpand_vec<'e>(cx: &LateContext, expr: &'e Expr) -> Option> { + if_let_chain!{[ + let ExprCall(ref fun, ref args) = expr.node, + let ExprPath(_, ref path) = fun.node, + is_expn_of(cx, fun.span, "vec").is_some() + ], { + return if match_path(path, &VEC_FROM_ELEM_PATH) && args.len() == 2 { + // `vec![elem; size]` case + Some(VecArgs::Repeat(&args[0], &args[1])) + } + else if match_path(path, &["into_vec"]) && args.len() == 1 { + // `vec![a, b, c]` case + if_let_chain!{[ + let ExprCall(ref fun, ref args) = args[0].node, + let ExprPath(_, ref path) = fun.node, + match_path(path, &BOX_NEW_PATH) && args.len() == 1, + let ExprVec(ref args) = args[0].node + ], { + return Some(VecArgs::Vec(&*args)); + }} + + None + } + else { + None + }; + }} + + None +} diff --git a/tests/compile-fail/vec.rs b/tests/compile-fail/vec.rs new file mode 100755 index 000000000..b4f52ecad --- /dev/null +++ b/tests/compile-fail/vec.rs @@ -0,0 +1,44 @@ +#![feature(plugin)] +#![plugin(clippy)] + +#![deny(useless_vec)] + +fn on_slice(_: &[u8]) {} +#[allow(ptr_arg)] +fn on_vec(_: &Vec) {} + +fn main() { + on_slice(&vec![]); + //~^ ERROR useless use of `vec!` + //~| HELP you can use + //~| SUGGESTION on_slice(&[]) + on_slice(&[]); + + on_slice(&vec![1, 2]); + //~^ ERROR useless use of `vec!` + //~| HELP you can use + //~| SUGGESTION on_slice(&[1, 2]) + on_slice(&[1, 2]); + + on_slice(&vec ![1, 2]); + //~^ ERROR useless use of `vec!` + //~| HELP you can use + //~| SUGGESTION on_slice(&[1, 2]) + on_slice(&[1, 2]); + + on_slice(&vec!(1, 2)); + //~^ ERROR useless use of `vec!` + //~| HELP you can use + //~| SUGGESTION on_slice(&[1, 2]) + on_slice(&[1, 2]); + + on_slice(&vec![1; 2]); + //~^ ERROR useless use of `vec!` + //~| HELP you can use + //~| SUGGESTION on_slice(&[1; 2]) + on_slice(&[1; 2]); + + on_vec(&vec![]); + on_vec(&vec![1, 2]); + on_vec(&vec![1; 2]); +}