Auto merge of #7270 - Valentine-Mario:vec_extend_to_append, r=flip1995

Vec extend to append

This PR adds a check to suggest changes of vector from

```
vec.extend(other_vec.drain(..))
```

could be written as

```
vec![].append(&mut vec![]);
```

changelog: Add vec_extend_to_append lint
issue: #7209

CHANGELOG.md

@@ -2295,6 +2295,7 @@ Released 2018-09-13
 <!-- begin autogenerated links to lint list -->
 [`absurd_extreme_comparisons`]: https://rust-lang.github.io/rust-clippy/master/index.html#absurd_extreme_comparisons
 [`almost_swapped`]: https://rust-lang.github.io/rust-clippy/master/index.html#almost_swapped
+[`append_instead_of_extend`]: https://rust-lang.github.io/rust-clippy/master/index.html#append_instead_of_extend
 [`approx_constant`]: https://rust-lang.github.io/rust-clippy/master/index.html#approx_constant
 [`as_conversions`]: https://rust-lang.github.io/rust-clippy/master/index.html#as_conversions
 [`assertions_on_constants`]: https://rust-lang.github.io/rust-clippy/master/index.html#assertions_on_constants

clippy_lints/src/lib.rs

@@ -730,6 +730,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         mem_replace::MEM_REPLACE_OPTION_WITH_NONE,
         mem_replace::MEM_REPLACE_WITH_DEFAULT,
         mem_replace::MEM_REPLACE_WITH_UNINIT,
+        methods::APPEND_INSTEAD_OF_EXTEND,
         methods::BIND_INSTEAD_OF_MAP,
         methods::BYTES_NTH,
         methods::CHARS_LAST_CMP,
@@ -1276,6 +1277,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(mem_replace::MEM_REPLACE_OPTION_WITH_NONE),
         LintId::of(mem_replace::MEM_REPLACE_WITH_DEFAULT),
         LintId::of(mem_replace::MEM_REPLACE_WITH_UNINIT),
+        LintId::of(methods::APPEND_INSTEAD_OF_EXTEND),
         LintId::of(methods::BIND_INSTEAD_OF_MAP),
         LintId::of(methods::BYTES_NTH),
         LintId::of(methods::CHARS_LAST_CMP),
@@ -1736,6 +1738,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(large_enum_variant::LARGE_ENUM_VARIANT),
         LintId::of(loops::MANUAL_MEMCPY),
         LintId::of(loops::NEEDLESS_COLLECT),
+        LintId::of(methods::APPEND_INSTEAD_OF_EXTEND),
         LintId::of(methods::EXPECT_FUN_CALL),
         LintId::of(methods::ITER_NTH),
         LintId::of(methods::MANUAL_STR_REPEAT),

clippy_lints/src/methods/append_instead_of_extend.rs

@@ -0,0 +1,41 @@
+use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::source::snippet_with_applicability;
+use clippy_utils::ty::{is_type_diagnostic_item, is_type_lang_item};
+use if_chain::if_chain;
+use rustc_errors::Applicability;
+use rustc_hir::{Expr, ExprKind, LangItem};
+use rustc_lint::LateContext;
+use rustc_span::symbol::sym;
+
+use super::APPEND_INSTEAD_OF_EXTEND;
+
+pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, recv: &Expr<'_>, arg: &Expr<'_>) {
+    let ty = cx.typeck_results().expr_ty(recv).peel_refs();
+    if_chain! {
+        if is_type_diagnostic_item(cx, ty, sym::vec_type);
+        //check source object
+        if let ExprKind::MethodCall(src_method, _, [drain_vec, drain_arg], _) = &arg.kind;
+        if src_method.ident.as_str() == "drain";
+        if let src_ty = cx.typeck_results().expr_ty(drain_vec).peel_refs();
+        if is_type_diagnostic_item(cx, src_ty, sym::vec_type);
+        //check drain range
+        if let src_ty_range = cx.typeck_results().expr_ty(drain_arg).peel_refs();
+        if is_type_lang_item(cx, src_ty_range, LangItem::RangeFull);
+        then {
+            let mut applicability = Applicability::MachineApplicable;
+            span_lint_and_sugg(
+                cx,
+                APPEND_INSTEAD_OF_EXTEND,
+                expr.span,
+                "use of `extend` instead of `append` for adding the full range of a second vector",
+                "try this",
+                format!(
+                    "{}.append(&mut {})",
+                    snippet_with_applicability(cx, recv.span, "..", &mut applicability),
+                    snippet_with_applicability(cx, drain_vec.span, "..", &mut applicability)
+                ),
+                applicability,
+            );
+        }
+    }
+}

clippy_lints/src/methods/mod.rs

@@ -1,3 +1,4 @@
+mod append_instead_of_extend;
 mod bind_instead_of_map;
 mod bytes_nth;
 mod chars_cmp;
@@ -1032,6 +1033,30 @@ declare_clippy_lint! {
     "using `.get().unwrap()` or `.get_mut().unwrap()` when using `[]` would work instead"
 }
 
+declare_clippy_lint! {
+    /// **What it does:** Checks for occurrences where one vector gets extended instead of append
+    ///
+    /// **Why is this bad?** Using `append` instead of `extend` is more concise and faster
+    ///
+    /// **Known problems:** None.
+    ///
+    /// **Example:**
+    ///
+    /// ```rust
+    /// let mut a = vec![1, 2, 3];
+    /// let mut b = vec![4, 5, 6];
+    ///
+    /// // Bad
+    /// a.extend(b.drain(..));
+    ///
+    /// // Good
+    /// a.append(&mut b);
+    /// ```
+    pub APPEND_INSTEAD_OF_EXTEND,
+    perf,
+    "using vec.append(&mut vec) to move the full range of a vecor to another"
+}
+
 declare_clippy_lint! {
     /// **What it does:** Checks for the use of `.extend(s.chars())` where s is a
     /// `&str` or `String`.
@@ -1785,7 +1810,8 @@ impl_lint_pass!(Methods => [
     INSPECT_FOR_EACH,
     IMPLICIT_CLONE,
     SUSPICIOUS_SPLITN,
-    MANUAL_STR_REPEAT
+    MANUAL_STR_REPEAT,
+    APPEND_INSTEAD_OF_EXTEND
 ]);
 
 /// Extracts a method call name, args, and `Span` of the method name.
@@ -2047,7 +2073,10 @@ fn check_methods<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, msrv: Optio
                 Some(("ok", [recv], _)) => ok_expect::check(cx, expr, recv),
                 _ => expect_used::check(cx, expr, recv),
             },
-            ("extend", [arg]) => string_extend_chars::check(cx, expr, recv, arg),
+            ("extend", [arg]) => {
+                string_extend_chars::check(cx, expr, recv, arg);
+                append_instead_of_extend::check(cx, expr, recv, arg);
+            },
             ("filter_map", [arg]) => {
                 unnecessary_filter_map::check(cx, expr, arg);
                 filter_map_identity::check(cx, expr, arg, span);

tests/ui/append_instead_of_extend.fixed

@@ -0,0 +1,55 @@
+// run-rustfix
+#![warn(clippy::append_instead_of_extend)]
+use std::collections::BinaryHeap;
+fn main() {
+    //gets linted
+    let mut vec1 = vec![0u8; 1024];
+    let mut vec2: std::vec::Vec<u8> = Vec::new();
+
+    vec2.append(&mut vec1);
+
+    let mut vec3 = vec![0u8; 1024];
+    let mut vec4: std::vec::Vec<u8> = Vec::new();
+
+    vec4.append(&mut vec3);
+
+    let mut vec11: std::vec::Vec<u8> = Vec::new();
+
+    vec11.append(&mut return_vector());
+
+    //won't get linted it dosen't move the entire content of a vec into another
+    let mut test1 = vec![0u8, 10];
+    let mut test2: std::vec::Vec<u8> = Vec::new();
+
+    test2.extend(test1.drain(4..10));
+
+    let mut vec3 = vec![0u8; 104];
+    let mut vec7: std::vec::Vec<u8> = Vec::new();
+
+    vec3.append(&mut vec7);
+
+    let mut vec5 = vec![0u8; 1024];
+    let mut vec6: std::vec::Vec<u8> = Vec::new();
+
+    vec5.extend(vec6.drain(..4));
+
+    let mut vec9: std::vec::Vec<u8> = Vec::new();
+
+    return_vector().append(&mut vec9);
+
+    //won't get linted because it is not a vec
+
+    let mut heap = BinaryHeap::from(vec![1, 3]);
+    let mut heap2 = BinaryHeap::from(vec![]);
+    heap2.extend(heap.drain())
+}
+
+fn return_vector() -> Vec<u8> {
+    let mut new_vector = vec![];
+
+    for i in 1..10 {
+        new_vector.push(i)
+    }
+
+    new_vector
+}

tests/ui/append_instead_of_extend.rs

@@ -0,0 +1,55 @@
+// run-rustfix
+#![warn(clippy::append_instead_of_extend)]
+use std::collections::BinaryHeap;
+fn main() {
+    //gets linted
+    let mut vec1 = vec![0u8; 1024];
+    let mut vec2: std::vec::Vec<u8> = Vec::new();
+
+    vec2.extend(vec1.drain(..));
+
+    let mut vec3 = vec![0u8; 1024];
+    let mut vec4: std::vec::Vec<u8> = Vec::new();
+
+    vec4.extend(vec3.drain(..));
+
+    let mut vec11: std::vec::Vec<u8> = Vec::new();
+
+    vec11.extend(return_vector().drain(..));
+
+    //won't get linted it dosen't move the entire content of a vec into another
+    let mut test1 = vec![0u8, 10];
+    let mut test2: std::vec::Vec<u8> = Vec::new();
+
+    test2.extend(test1.drain(4..10));
+
+    let mut vec3 = vec![0u8; 104];
+    let mut vec7: std::vec::Vec<u8> = Vec::new();
+
+    vec3.append(&mut vec7);
+
+    let mut vec5 = vec![0u8; 1024];
+    let mut vec6: std::vec::Vec<u8> = Vec::new();
+
+    vec5.extend(vec6.drain(..4));
+
+    let mut vec9: std::vec::Vec<u8> = Vec::new();
+
+    return_vector().append(&mut vec9);
+
+    //won't get linted because it is not a vec
+
+    let mut heap = BinaryHeap::from(vec![1, 3]);
+    let mut heap2 = BinaryHeap::from(vec![]);
+    heap2.extend(heap.drain())
+}
+
+fn return_vector() -> Vec<u8> {
+    let mut new_vector = vec![];
+
+    for i in 1..10 {
+        new_vector.push(i)
+    }
+
+    new_vector
+}

tests/ui/append_instead_of_extend.stderr

@@ -0,0 +1,22 @@
+error: use of `extend` instead of `append` for adding the full range of a second vector
+  --> $DIR/append_instead_of_extend.rs:9:5
+   |
+LL |     vec2.extend(vec1.drain(..));
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `vec2.append(&mut vec1)`
+   |
+   = note: `-D clippy::append-instead-of-extend` implied by `-D warnings`
+
+error: use of `extend` instead of `append` for adding the full range of a second vector
+  --> $DIR/append_instead_of_extend.rs:14:5
+   |
+LL |     vec4.extend(vec3.drain(..));
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `vec4.append(&mut vec3)`
+
+error: use of `extend` instead of `append` for adding the full range of a second vector
+  --> $DIR/append_instead_of_extend.rs:18:5
+   |
+LL |     vec11.extend(return_vector().drain(..));
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `vec11.append(&mut return_vector())`
+
+error: aborting due to 3 previous errors
+