new lint: unnecessary_reserve

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_config/src/conf.rs

@@ -647,6 +647,7 @@ define_Conf! {
         unchecked_duration_subtraction,
         uninlined_format_args,
         unnecessary_lazy_evaluations,
+        unnecessary_reserve,
         unnested_or_patterns,
         unused_trait_names,
         use_self,

clippy_lints/src/declared_lints.rs

@@ -758,6 +758,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
     crate::unnecessary_literal_bound::UNNECESSARY_LITERAL_BOUND_INFO,
     crate::unnecessary_map_on_constructor::UNNECESSARY_MAP_ON_CONSTRUCTOR_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_semicolon::UNNECESSARY_SEMICOLON_INFO,
     crate::unnecessary_struct_initialization::UNNECESSARY_STRUCT_INITIALIZATION_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,133 @@
+use clippy_config::Conf;
+use clippy_utils::diagnostics::span_lint_and_then;
+use clippy_utils::msrvs::{self, Msrv};
+use clippy_utils::ty::adt_def_id;
+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_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?
+    /// `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.86.0"]
+    pub UNNECESSARY_RESERVE,
+    complexity,
+    "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);
+}
+
+fn acceptable_type(cx: &LateContext<'_>, struct_calling_on: &Expr<'_>) -> bool {
+    if let Some(did) = adt_def_id(cx.typeck_results().expr_ty_adjusted(struct_calling_on)) {
+        matches!(cx.tcx.get_diagnostic_name(did), Some(sym::Vec | sym::VecDeque))
+    } else {
+        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 mut found_reserve = false;
+    let mut read_found = false;
+    let mut spanless_eq = SpanlessEq::new(cx);
+
+    let _: Option<!> = for_each_expr(cx, block, |expr: &Expr<'tcx>| {
+        if !found_reserve {
+            if expr.hir_id == args_a.hir_id {
+                found_reserve = true;
+            }
+            return ControlFlow::Continue(());
+        }
+
+        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"];

clippy_utils/src/ty/mod.rs

@@ -1352,3 +1352,8 @@ pub fn option_arg_ty<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> Option<Ty<'t
         _ => None,
     }
 }
+
+/// Check if `ty` with references peeled is an `Adt` and return its `DefId`.
+pub fn adt_def_id(ty: Ty<'_>) -> Option<DefId> {
+    ty.peel_refs().ty_adt_def().map(AdtDef::did)
+}

clippy_utils/src/ty/type_certainty/mod.rs

@@ -11,13 +11,14 @@
 //! As a heuristic, `expr_type_is_certain` may produce false negatives, but a false positive should
 //! be considered a bug.
 
+use super::adt_def_id;
 use crate::def_path_res;
 use rustc_hir::def::{DefKind, Res};
 use rustc_hir::def_id::DefId;
 use rustc_hir::intravisit::{InferKind, Visitor, VisitorExt, walk_qpath, walk_ty};
 use rustc_hir::{self as hir, AmbigArg, Expr, ExprKind, GenericArgs, HirId, Node, PathSegment, QPath, TyKind};
 use rustc_lint::LateContext;
-use rustc_middle::ty::{self, AdtDef, GenericArgKind, Ty};
+use rustc_middle::ty::{self, GenericArgKind, Ty};
 use rustc_span::{Span, Symbol};
 
 mod certainty;
@@ -313,10 +314,6 @@ fn self_ty<'tcx>(cx: &LateContext<'tcx>, method_def_id: DefId) -> Ty<'tcx> {
     cx.tcx.fn_sig(method_def_id).skip_binder().inputs().skip_binder()[0]
 }
 
-fn adt_def_id(ty: Ty<'_>) -> Option<DefId> {
-    ty.peel_refs().ty_adt_def().map(AdtDef::did)
-}
-
 fn contains_param(ty: Ty<'_>, index: u32) -> bool {
     ty.walk()
         .any(|arg| matches!(arg.unpack(), GenericArgKind::Type(ty) if ty.is_param(index)))

tests/ui/unnecessary_reserve.fixed

@@ -0,0 +1,109 @@
+#![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();
+    box_vec_reserve();
+}
+
+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]);
+}
+
+fn box_vec_reserve() {
+    let mut vec: Box<Vec<usize>> = Box::default();
+    let array: &[usize] = &[1, 2];
+
+    // do lint
+    ;
+    vec.extend(array);
+}