Add unnecessary_reserve lint

CHANGELOG.md

@@ -4990,6 +4990,7 @@ Released 2018-09-13
 [`unnecessary_mut_passed`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_mut_passed
 [`unnecessary_operation`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_operation
 [`unnecessary_owned_empty_strings`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_owned_empty_strings
+[`unnecessary_reserve`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_reserve
 [`unnecessary_safety_comment`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_safety_comment
 [`unnecessary_safety_doc`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_safety_doc
 [`unnecessary_self_imports`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_self_imports

clippy_lints/src/declared_lints.rs

@@ -624,6 +624,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
     crate::unnamed_address::VTABLE_ADDRESS_COMPARISONS_INFO,
     crate::unnecessary_box_returns::UNNECESSARY_BOX_RETURNS_INFO,
     crate::unnecessary_owned_empty_strings::UNNECESSARY_OWNED_EMPTY_STRINGS_INFO,
+    crate::unnecessary_reserve::UNNECESSARY_RESERVE_INFO,
     crate::unnecessary_self_imports::UNNECESSARY_SELF_IMPORTS_INFO,
     crate::unnecessary_struct_initialization::UNNECESSARY_STRUCT_INITIALIZATION_INFO,
     crate::unnecessary_wraps::UNNECESSARY_WRAPS_INFO,

clippy_lints/src/lib.rs

@@ -305,6 +305,7 @@ mod unit_types;
 mod unnamed_address;
 mod unnecessary_box_returns;
 mod unnecessary_owned_empty_strings;
+mod unnecessary_reserve;
 mod unnecessary_self_imports;
 mod unnecessary_struct_initialization;
 mod unnecessary_wraps;
@@ -959,6 +960,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     store.register_late_pass(|_| Box::new(tests_outside_test_module::TestsOutsideTestModule));
     store.register_late_pass(|_| Box::new(manual_slice_size_calculation::ManualSliceSizeCalculation));
     store.register_early_pass(|| Box::new(suspicious_doc_comments::SuspiciousDocComments));
+    store.register_late_pass(move |_| Box::new(unnecessary_reserve::UnnecessaryReserve::new(msrv())));
     // add lints here, do not remove this comment, it's used in `new_lint`
 }
 

clippy_lints/src/unnecessary_reserve.rs

@@ -0,0 +1,121 @@
+use clippy_utils::diagnostics::span_lint_and_then;
+use clippy_utils::msrvs::{self, Msrv};
+use clippy_utils::{get_enclosing_block, match_def_path, paths, visitors::for_each_expr, SpanlessEq};
+use core::ops::ControlFlow;
+use rustc_errors::Applicability;
+use rustc_hir::{Block, Expr, ExprKind, PathSegment};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_middle::ty;
+use rustc_session::{declare_tool_lint, impl_lint_pass};
+use rustc_span::sym;
+
+declare_clippy_lint! {
+    /// ### What it does
+    ///
+    /// This lint checks for a call to `reserve` before `extend` on a `Vec` or `VecDeque`.
+    /// ### Why is this bad?
+    /// Since Rust 1.62, `extend` implicitly calls `reserve`
+    ///
+    /// ### Example
+    /// ```rust
+    /// let mut vec: Vec<usize> = vec![];
+    /// let array: &[usize] = &[1, 2];
+    /// vec.reserve(array.len());
+    /// vec.extend(array);
+    /// ```
+    /// Use instead:
+    /// ```rust
+    /// let mut vec: Vec<usize> = vec![];
+    /// let array: &[usize] = &[1, 2];
+    /// vec.extend(array);
+    /// ```
+    #[clippy::version = "1.64.0"]
+    pub UNNECESSARY_RESERVE,
+    pedantic,
+    "calling `reserve` before `extend` on a `Vec` or `VecDeque`, when it will be called implicitly"
+}
+
+impl_lint_pass!(UnnecessaryReserve => [UNNECESSARY_RESERVE]);
+
+pub struct UnnecessaryReserve {
+    msrv: Msrv,
+}
+impl UnnecessaryReserve {
+    pub fn new(msrv: Msrv) -> Self {
+        Self { msrv }
+    }
+}
+
+impl<'tcx> LateLintPass<'tcx> for UnnecessaryReserve {
+    fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &Expr<'tcx>) {
+        if !self.msrv.meets(msrvs::EXTEND_IMPLICIT_RESERVE) {
+            return;
+        }
+
+        if let ExprKind::MethodCall(PathSegment { ident: method, .. }, struct_calling_on, args_a, _) = expr.kind
+                && method.name.as_str() == "reserve"
+                && acceptable_type(cx, struct_calling_on)
+                && let Some(block) = get_enclosing_block(cx, expr.hir_id)
+                && let Some(next_stmt_span) = check_extend_method(cx, block, struct_calling_on, &args_a[0])
+                && !next_stmt_span.from_expansion()
+            {
+                span_lint_and_then(
+                    cx,
+                    UNNECESSARY_RESERVE,
+                    next_stmt_span,
+                    "unnecessary call to `reserve`",
+                    |diag| {
+                        diag.span_suggestion(
+                            expr.span,
+                            "remove this line",
+                            String::new(),
+                            Applicability::MaybeIncorrect,
+                        );
+                    }
+                );
+            }
+    }
+
+    extract_msrv_attr!(LateContext);
+}
+
+#[must_use]
+fn acceptable_type(cx: &LateContext<'_>, struct_calling_on: &rustc_hir::Expr<'_>) -> bool {
+    let acceptable_types = [sym::Vec, sym::VecDeque];
+    acceptable_types.iter().any(|&acceptable_ty| {
+        match cx.typeck_results().expr_ty(struct_calling_on).peel_refs().kind() {
+            ty::Adt(def, _) => cx.tcx.is_diagnostic_item(acceptable_ty, def.did()),
+            _ => false,
+        }
+    })
+}
+
+#[must_use]
+fn check_extend_method(
+    cx: &LateContext<'_>,
+    block: &Block<'_>,
+    struct_expr: &rustc_hir::Expr<'_>,
+    args_a: &rustc_hir::Expr<'_>,
+) -> Option<rustc_span::Span> {
+    let args_a_kind = &args_a.kind;
+    let mut read_found = false;
+    let mut spanless_eq = SpanlessEq::new(cx);
+
+    let _: Option<!> = for_each_expr(block, |expr| {
+        if let ExprKind::MethodCall(_, struct_calling_on, _,_) = expr.kind
+            && let Some(expr_def_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id)
+            && let ExprKind::MethodCall(PathSegment { ident: method_call_a, .. },..) = args_a_kind
+            && method_call_a.name == rustc_span::sym::len
+            && match_def_path(cx, expr_def_id, &paths::ITER_EXTEND)
+            && acceptable_type(cx, struct_calling_on)
+            // Check that both expr are equal
+            && spanless_eq.eq_expr(struct_calling_on, struct_expr)
+        {
+            read_found = true;
+        }
+        let _: bool = !read_found;
+        ControlFlow::Continue(())
+    });
+
+    if read_found { Some(block.span) } else { None }
+}

clippy_utils/src/msrvs.rs

@@ -21,7 +21,7 @@ macro_rules! msrv_aliases {
 msrv_aliases! {
     1,68,0 { PATH_MAIN_SEPARATOR_STR }
     1,65,0 { LET_ELSE }
-    1,62,0 { BOOL_THEN_SOME, DEFAULT_ENUM_ATTRIBUTE }
+    1,62,0 { BOOL_THEN_SOME, DEFAULT_ENUM_ATTRIBUTE, EXTEND_IMPLICIT_RESERVE }
     1,58,0 { FORMAT_ARGS_CAPTURE, PATTERN_TRAIT_CHAR_ARRAY }
     1,55,0 { SEEK_REWIND }
     1,53,0 { OR_PATTERNS, MANUAL_BITS, BTREE_MAP_RETAIN, BTREE_SET_RETAIN, ARRAY_INTO_ITERATOR }

clippy_utils/src/paths.rs

@@ -49,6 +49,7 @@ pub const IDENT: [&str; 3] = ["rustc_span", "symbol", "Ident"];
 pub const IDENT_AS_STR: [&str; 4] = ["rustc_span", "symbol", "Ident", "as_str"];
 pub const INSERT_STR: [&str; 4] = ["alloc", "string", "String", "insert_str"];
 pub const ITER_EMPTY: [&str; 5] = ["core", "iter", "sources", "empty", "Empty"];
+pub const ITER_EXTEND: [&str; 6] = ["core", "iter", "traits", "collect", "Extend", "extend"];
 pub const ITERTOOLS_NEXT_TUPLE: [&str; 3] = ["itertools", "Itertools", "next_tuple"];
 #[cfg(feature = "internal")]
 pub const KW_MODULE: [&str; 3] = ["rustc_span", "symbol", "kw"];
@@ -154,6 +155,8 @@ pub const VEC_DEQUE_ITER: [&str; 5] = ["alloc", "collections", "vec_deque", "Vec
 pub const VEC_FROM_ELEM: [&str; 3] = ["alloc", "vec", "from_elem"];
 pub const VEC_NEW: [&str; 4] = ["alloc", "vec", "Vec", "new"];
 pub const VEC_RESIZE: [&str; 4] = ["alloc", "vec", "Vec", "resize"];
+pub const VEC_RESERVE: [&str; 4] = ["alloc", "vec", "Vec", "reserve"];
+pub const VEC_DEQUE_RESERVE: [&str; 5] = ["alloc", "collections", "vec_deque", "VecDeque", "reserve"];
 pub const WEAK_ARC: [&str; 3] = ["alloc", "sync", "Weak"];
 pub const WEAK_RC: [&str; 3] = ["alloc", "rc", "Weak"];
 pub const PTR_NON_NULL: [&str; 4] = ["core", "ptr", "non_null", "NonNull"];

tests/ui/unnecessary_reserve.rs

@@ -0,0 +1,95 @@
+#![warn(clippy::unnecessary_reserve)]
+#![feature(custom_inner_attributes)]
+
+use std::collections::HashMap;
+use std::collections::VecDeque;
+
+fn main() {
+    vec_reserve();
+    vec_deque_reserve();
+    hash_map_reserve();
+    msrv_1_62();
+}
+
+fn vec_reserve() {
+    let mut vec: Vec<usize> = vec![];
+    let array: &[usize] = &[1, 2];
+
+    // do not lint
+    vec.reserve(1);
+    vec.extend([1]);
+
+    //// do lint
+    vec.reserve(array.len());
+    vec.extend(array);
+
+    // do lint
+    {
+        vec.reserve(array.len());
+        vec.extend(array)
+    };
+
+    // do not lint
+    vec.reserve(array.len());
+    vec.push(1);
+    vec.extend(array);
+
+    // do not lint
+    let mut other_vec: Vec<usize> = vec![];
+    other_vec.reserve(1);
+    vec.extend([1])
+}
+
+fn vec_deque_reserve() {
+    let mut vec_deque: VecDeque<usize> = [1].into();
+    let array: &[usize] = &[1, 2];
+
+    // do not lint
+    vec_deque.reserve(1);
+    vec_deque.extend([1]);
+
+    // do lint
+    vec_deque.reserve(array.len());
+    vec_deque.extend(array);
+
+    // do not lint
+    {
+        vec_deque.reserve(1);
+        vec_deque.extend([1])
+    };
+
+    // do not lint
+    vec_deque.reserve(array.len() + 1);
+    vec_deque.push_back(1);
+    vec_deque.extend(array);
+
+    // do not lint
+    let mut other_vec_deque: VecDeque<usize> = [1].into();
+    other_vec_deque.reserve(1);
+    vec_deque.extend([1])
+}
+
+fn hash_map_reserve() {
+    let mut map: HashMap<usize, usize> = HashMap::new();
+    let mut other_map: HashMap<usize, usize> = HashMap::new();
+    // do not lint
+    map.reserve(other_map.len());
+    map.extend(other_map);
+}
+
+fn msrv_1_62() {
+    #![clippy::msrv = "1.61"]
+    let mut vec: Vec<usize> = vec![];
+    let array: &[usize] = &[1, 2];
+
+    // do not lint
+    vec.reserve(1);
+    vec.extend([1]);
+
+    let mut vec_deque: VecDeque<usize> = [1].into();
+    let array: &[usize] = &[1, 2];
+
+    // do not lint
+    vec_deque.reserve(1);
+    vec_deque.extend([1]);
+}

tests/ui/unnecessary_reserve.stderr

@@ -0,0 +1,62 @@
+error: unnecessary call to `reserve`
+  --> $DIR/unnecessary_reserve.rs:14:18
+   |
+LL |   fn vec_reserve() {
+   |  __________________^
+LL | |     let mut vec: Vec<usize> = vec![];
+LL | |     let array: &[usize] = &[1, 2];
+LL | |
+...  |
+LL | |     vec.reserve(array.len());
+   | |     ------------------------ help: remove this line
+...  |
+LL | |     vec.extend([1])
+LL | | }
+   | |_^
+   |
+   = note: `-D clippy::unnecessary-reserve` implied by `-D warnings`
+
+error: unnecessary call to `reserve`
+  --> $DIR/unnecessary_reserve.rs:27:5
+   |
+LL | /     {
+LL | |         vec.reserve(array.len());
+   | |         ------------------------ help: remove this line
+LL | |         vec.extend(array)
+LL | |     };
+   | |_____^
+
+error: unnecessary call to `reserve`
+  --> $DIR/unnecessary_reserve.rs:14:18
+   |
+LL |   fn vec_reserve() {
+   |  __________________^
+LL | |     let mut vec: Vec<usize> = vec![];
+LL | |     let array: &[usize] = &[1, 2];
+LL | |
+...  |
+LL | |     vec.reserve(array.len());
+   | |     ------------------------ help: remove this line
+...  |
+LL | |     vec.extend([1])
+LL | | }
+   | |_^
+
+error: unnecessary call to `reserve`
+  --> $DIR/unnecessary_reserve.rs:43:24
+   |
+LL |   fn vec_deque_reserve() {
+   |  ________________________^
+LL | |     let mut vec_deque: VecDeque<usize> = [1].into();
+LL | |     let array: &[usize] = &[1, 2];
+LL | |
+...  |
+LL | |     vec_deque.reserve(array.len());
+   | |     ------------------------------ help: remove this line
+...  |
+LL | |     vec_deque.extend([1])
+LL | | }
+   | |_^
+
+error: aborting due to 4 previous errors
+