new lint: unnecessary_reserve

Author: wowinter13

CHANGELOG.md

@@ -6173,6 +6173,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_result_map_or_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_result_map_or_else
 [`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

clippy_lints/src/declared_lints.rs

@@ -624,6 +624,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
     crate::pathbuf_init_then_push::PATHBUF_INIT_THEN_PUSH_INFO,
     crate::pattern_type_mismatch::PATTERN_TYPE_MISMATCH_INFO,
     crate::permissions_set_readonly_false::PERMISSIONS_SET_READONLY_FALSE_INFO,
+    crate::unnecessary_reserve::UNNECESSARY_RESERVE_INFO,
     crate::pointers_in_nomem_asm_block::POINTERS_IN_NOMEM_ASM_BLOCK_INFO,
     crate::precedence::PRECEDENCE_INFO,
     crate::ptr::CMP_NULL_INFO,

clippy_lints/src/lib.rs

@@ -372,6 +372,7 @@ mod unnecessary_box_returns;
 mod unnecessary_literal_bound;
 mod unnecessary_map_on_constructor;
 mod unnecessary_owned_empty_strings;
+mod unnecessary_reserve;
 mod unnecessary_self_imports;
 mod unnecessary_semicolon;
 mod unnecessary_struct_initialization;
@@ -956,6 +957,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
     store.register_late_pass(move |_| Box::new(assigning_clones::AssigningClones::new(conf)));
     store.register_late_pass(|_| Box::new(zero_repeat_side_effects::ZeroRepeatSideEffects));
     store.register_late_pass(|_| Box::new(manual_unwrap_or_default::ManualUnwrapOrDefault));
+    store.register_late_pass(move |_| Box::new(unnecessary_reserve::UnnecessaryReserve::new(conf)));
     store.register_late_pass(|_| Box::new(integer_division_remainder_used::IntegerDivisionRemainderUsed));
     store.register_late_pass(move |_| Box::new(macro_metavars_in_unsafe::ExprMetavarsInUnsafe::new(conf)));
     store.register_late_pass(move |_| Box::new(string_patterns::StringPatterns::new(conf)));

clippy_lints/src/unnecessary_reserve.rs

@@ -0,0 +1,129 @@
+use clippy_config::Conf;
+use clippy_utils::diagnostics::span_lint_and_then;
+use clippy_utils::msrvs::{self, Msrv};
+use clippy_utils::visitors::for_each_expr;
+use clippy_utils::{SpanlessEq, get_enclosing_block, match_def_path, paths};
+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::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(conf: &'static Conf) -> Self {
+        Self {
+            msrv: conf.msrv.clone(),
+        }
+    }
+}
+
+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<'tcx>(cx: &LateContext<'tcx>, struct_calling_on: &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<'tcx>(
+    cx: &LateContext<'tcx>,
+    block: &'tcx Block<'tcx>,
+    struct_expr: &Expr<'tcx>,
+    args_a: &Expr<'tcx>,
+) -> 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(cx, block, |expr: &Expr<'tcx>| {
+        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 == sym::len
+            && match_def_path(cx, expr_def_id, &paths::ITER_EXTEND)
+            && acceptable_type(cx, struct_calling_on)
+            && 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

@@ -31,7 +31,7 @@ msrv_aliases! {
     1,68,0 { PATH_MAIN_SEPARATOR_STR }
     1,65,0 { LET_ELSE, POINTER_CAST_CONSTNESS }
     1,63,0 { CLONE_INTO }
-    1,62,0 { BOOL_THEN_SOME, DEFAULT_ENUM_ATTRIBUTE, CONST_EXTERN_C_FN }
+    1,62,0 { BOOL_THEN_SOME, DEFAULT_ENUM_ATTRIBUTE, CONST_EXTERN_C_FN, EXTEND_IMPLICIT_RESERVE }
     1,59,0 { THREAD_LOCAL_CONST_INIT }
     1,58,0 { FORMAT_ARGS_CAPTURE, PATTERN_TRAIT_CHAR_ARRAY, CONST_RAW_PTR_DEREF }
     1,56,0 { CONST_FN_UNION }

clippy_utils/src/paths.rs

@@ -15,6 +15,7 @@ pub const APPLICABILITY_VALUES: [[&str; 3]; 4] = [
 pub const DIAG: [&str; 2] = ["rustc_errors", "Diag"];
 pub const EARLY_CONTEXT: [&str; 2] = ["rustc_lint", "EarlyContext"];
 pub const EARLY_LINT_PASS: [&str; 3] = ["rustc_lint", "passes", "EarlyLintPass"];
+pub const ITER_EXTEND: [&str; 6] = ["core", "iter", "traits", "collect", "Extend", "extend"];
 pub const IDENT: [&str; 3] = ["rustc_span", "symbol", "Ident"];
 pub const IDENT_AS_STR: [&str; 4] = ["rustc_span", "symbol", "Ident", "as_str"];
 pub const KW_MODULE: [&str; 3] = ["rustc_span", "symbol", "kw"];

tests/ui/unnecessary_reserve.fixed

@@ -0,0 +1,99 @@
+#![warn(clippy::unnecessary_reserve)]
+#![feature(custom_inner_attributes)]
+
+#![allow(clippy::unnecessary_operation)]
+use std::collections::{HashMap, VecDeque};
+
+fn main() {
+    vec_reserve();
+    vec_deque_reserve();
+    hash_map_reserve();
+    msrv_1_62();
+}
+
+fn vec_reserve() {
+    let mut vec: Vec<usize> = vec![];
+    let array1: &[usize] = &[1, 2];
+    let array2: &[usize] = &[3, 4];
+
+    // do not lint - different arrays
+    ;
+    vec.extend(array2);
+
+    // do not lint
+    vec.reserve(1);
+    vec.extend([1]);
+
+    //// do lint
+    ;
+    vec.extend(array1);
+
+    // do lint
+    {
+        ;
+        vec.extend(array1)
+    };
+
+    // do not lint
+    ;
+    vec.push(1);
+    vec.extend(array1);
+
+    // do not lint
+    let mut other_vec: Vec<usize> = Vec::with_capacity(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.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.rs

@@ -0,0 +1,100 @@
+#![warn(clippy::unnecessary_reserve)]
+#![feature(custom_inner_attributes)]
+
+#[allow(clippy::unnecessary_operation)]
+use std::collections::{HashMap, VecDeque};
+
+fn main() {
+    vec_reserve();
+    vec_deque_reserve();
+    hash_map_reserve();
+    msrv_1_62();
+}
+
+fn vec_reserve() {
+    let mut vec: Vec<usize> = vec![];
+    let array1: &[usize] = &[1, 2];
+    let array2: &[usize] = &[3, 4];
+
+    // do not lint - different arrays
+    vec.reserve(array1.len());
+    vec.extend(array2);
+
+    // do not lint
+    vec.reserve(1);
+    vec.extend([1]);
+
+    //// do lint
+    vec.reserve(array1.len());
+    vec.extend(array1);
+
+    // do lint
+    {
+        vec.reserve(array1.len());
+        vec.extend(array1)
+    };
+
+    // do not lint
+    vec.reserve(array1.len());
+    vec.push(1);
+    vec.extend(array1);
+
+    // 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]);
+}