From afb7e6721797484c704a01ed94ae67c086cdc007 Mon Sep 17 00:00:00 2001
From: mcarton <cartonmartin+git@gmail.com>
Date: Fri, 29 Jan 2016 01:54:10 +0100
Subject: [PATCH] Add a lint to warn about &vec![_] if &[_] would do
---
README.md | 3 +-
src/lib.rs | 3 ++
src/utils.rs | 4 +-
src/vec.rs | 111 ++++++++++++++++++++++++++++++++++++++
tests/compile-fail/vec.rs | 44 +++++++++++++++
5 files changed, 163 insertions(+), 2 deletions(-)
create mode 100644 src/vec.rs
create mode 100755 tests/compile-fail/vec.rs
diff --git a/README.md b/README.md
index fbd6f4ebfe6..212d868c698 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 8b115a304e9..90625e4bb5c 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 c59e35c5c5b..a6dbcfd9e2f 100644
--- a/src/utils.rs
+++ b/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 {
diff --git a/src/vec.rs b/src/vec.rs
new file mode 100644
index 00000000000..b46795a3cdd
--- /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<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
+}
diff --git a/tests/compile-fail/vec.rs b/tests/compile-fail/vec.rs
new file mode 100755
index 00000000000..b4f52ecadc5
--- /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<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]);
+}