Merge pull request #582 from Manishearth/extend

extend_from_slice lint

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 99 lints included in this crate:
+There are 100 lints included in this crate:
 
 name                                                                                                           | default | meaning
 ---------------------------------------------------------------------------------------------------------------|---------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
@@ -34,6 +34,7 @@ name
 [expl_impl_clone_on_copy](https://github.com/Manishearth/rust-clippy/wiki#expl_impl_clone_on_copy)             | warn    | implementing `Clone` explicitly on `Copy` types
 [explicit_counter_loop](https://github.com/Manishearth/rust-clippy/wiki#explicit_counter_loop)                 | warn    | for-looping with an explicit counter when `_.enumerate()` would do
 [explicit_iter_loop](https://github.com/Manishearth/rust-clippy/wiki#explicit_iter_loop)                       | warn    | for-looping over `_.iter()` or `_.iter_mut()` when `&_` or `&mut _` would do
+[extend_from_slice](https://github.com/Manishearth/rust-clippy/wiki#extend_from_slice)                         | warn    | `.extend_from_slice(_)` is a faster way to extend a Vec by a slice
 [filter_next](https://github.com/Manishearth/rust-clippy/wiki#filter_next)                                     | warn    | using `filter(p).next()`, which is more succinctly expressed as `.find(p)`
 [float_cmp](https://github.com/Manishearth/rust-clippy/wiki#float_cmp)                                         | warn    | using `==` or `!=` on float values (as floating-point operations usually involve rounding errors, it is always better to check for approximate equality within small bounds)
 [identity_op](https://github.com/Manishearth/rust-clippy/wiki#identity_op)                                     | warn    | using identity operations, e.g. `x + 0` or `y / 1`

src/lib.rs

@@ -200,6 +200,7 @@ pub fn plugin_registrar(reg: &mut Registry) {
         matches::MATCH_REF_PATS,
         matches::SINGLE_MATCH,
         methods::CHARS_NEXT_CMP,
+        methods::EXTEND_FROM_SLICE,
         methods::FILTER_NEXT,
         methods::OK_EXPECT,
         methods::OPTION_MAP_UNWRAP_OR,

src/methods.rs

@@ -14,7 +14,7 @@ use utils::{
 };
 use utils::{
     BTREEMAP_ENTRY_PATH, DEFAULT_TRAIT_PATH, HASHMAP_ENTRY_PATH, OPTION_PATH, RESULT_PATH,
-    STRING_PATH
+    STRING_PATH, VEC_PATH,
 };
 use utils::MethodArgs;
 use rustc::middle::cstore::CrateStore;
@@ -212,9 +212,20 @@ declare_lint!(pub CHARS_NEXT_CMP, Warn,
 declare_lint!(pub OR_FUN_CALL, Warn,
               "using any `*or` method when the `*or_else` would do");
 
+/// **What it does:** This lint `Warn`s on using `.extend(s)` on a `vec` to extend the vec by a slice.
+///
+/// **Why is this bad?** Since Rust 1.6, the `extend_from_slice(_)` method is stable and at least for now faster.
+///
+/// **Known problems:** None.
+///
+/// **Example:** `my_vec.extend(&xs)`
+declare_lint!(pub EXTEND_FROM_SLICE, Warn,
+              "`.extend_from_slice(_)` is a faster way to extend a Vec by a slice");
+
 impl LintPass for MethodsPass {
     fn get_lints(&self) -> LintArray {
-        lint_array!(OPTION_UNWRAP_USED,
+        lint_array!(EXTEND_FROM_SLICE,
+                    OPTION_UNWRAP_USED,
                     RESULT_UNWRAP_USED,
                     STR_TO_STRING,
                     STRING_TO_STRING,
@@ -256,6 +267,8 @@ impl LateLintPass for MethodsPass {
                     lint_search_is_some(cx, expr, "position", arglists[0], arglists[1]);
                 } else if let Some(arglists) = method_chain_args(expr, &["rposition", "is_some"]) {
                     lint_search_is_some(cx, expr, "rposition", arglists[0], arglists[1]);
+                } else if let Some(arglists) = method_chain_args(expr, &["extend"]) {
+                    lint_extend(cx, expr, arglists[0]);
                 }
 
                 lint_or_fun_call(cx, expr, &name.node.as_str(), &args);
@@ -427,6 +440,53 @@ fn lint_or_fun_call(cx: &LateContext, expr: &Expr, name: &str, args: &[P<Expr>])
     }
 }
 
+fn lint_extend(cx: &LateContext, expr: &Expr, args: &MethodArgs) {
+    let (obj_ty, _) = walk_ptrs_ty_depth(cx.tcx.expr_ty(&args[0]));
+    if !match_type(cx, obj_ty, &VEC_PATH) {
+        return;
+    }
+    let arg_ty = cx.tcx.expr_ty(&args[1]);
+    if let Some((span, r)) = derefs_to_slice(cx, &args[1], &arg_ty) {
+        span_lint(cx, EXTEND_FROM_SLICE, expr.span,
+                  &format!("use of `extend` to extend a Vec by a slice"))
+            .span_suggestion(expr.span, "try this",
+                             format!("{}.extend_from_slice({}{})",
+                                     snippet(cx, args[0].span, "_"),
+                                     r, snippet(cx, span, "_")));
+    }
+}
+
+fn derefs_to_slice(cx: &LateContext, expr: &Expr, ty: &ty::Ty) 
+   -> Option<(Span, &'static str)> {
+    fn may_slice(cx: &LateContext, ty: &ty::Ty) -> bool {
+        match ty.sty {
+            ty::TySlice(_) => true,            
+            ty::TyStruct(..) => match_type(cx, ty, &VEC_PATH),
+            ty::TyArray(_, size) => size < 32,
+            ty::TyRef(_, ty::TypeAndMut { ty: ref inner, .. }) |
+            ty::TyBox(ref inner) => may_slice(cx, inner),
+            _ => false
+        }
+    }
+    if let ExprMethodCall(name, _, ref args) = expr.node {
+        if &name.node.as_str() == &"iter" && 
+               may_slice(cx, &cx.tcx.expr_ty(&args[0])) {
+            Some((args[0].span, "&"))
+        } else {
+            None
+        }
+    } else {
+        match ty.sty {
+            ty::TySlice(_) => Some((expr.span, "")),
+            ty::TyRef(_, ty::TypeAndMut { ty: ref inner, .. }) |
+            ty::TyBox(ref inner) => if may_slice(cx, inner) { 
+                Some((expr.span, ""))
+            } else { None },
+            _ => None
+        }
+    }
+}
+
 #[allow(ptr_arg)]
 // Type of MethodArgs is potentially a Vec
 /// lint use of `unwrap()` for `Option`s and `Result`s

tests/compile-fail/methods.rs

@@ -304,3 +304,22 @@ fn starts_with() {
     //~| HELP like this
     //~| SUGGESTION !"".starts_with(' ')
 }
+
+fn use_extend_from_slice() {
+    let mut v : Vec<&'static str> = vec![];
+    v.extend(&["Hello", "World"]); //~ERROR use of `extend`
+    v.extend(&vec!["Some", "more"]);
+    //~^ERROR use of `extend`
+    //~| HELP try this
+    //~| SUGGESTION v.extend_from_slice(&vec!["Some", "more"]);
+    
+    v.extend(vec!["And", "even", "more"].iter()); //~ERROR use of `extend`
+    let o : Option<&'static str> = None;
+    v.extend(o);
+    v.extend(Some("Bye"));
+    v.extend(vec!["Not", "like", "this"]);
+    v.extend(["But", "this"].iter()); 
+    //~^ERROR use of `extend
+    //~| HELP try this
+    //~| SUGGESTION v.extend_from_slice(&["But", "this"]);
+}