Merge pull request #589 from mcarton/vec

Add a lint to warn about &vec![_] if &[_] would do

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

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

src/utils.rs

@@ -20,6 +20,7 @@ pub type MethodArgs = HirVec<P<Expr>>;
 
 // 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 {

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<Expr>, &'a P<Expr>),
+    /// `vec![a, b, c]`
+    Vec(&'a [P<Expr>]),
+}
+
+/// 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<VecArgs<'e>> {
+    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
+}

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<u8>) {}
+
+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]);
+}