add [unnecessary_reserve] lint

CHANGELOG.md

@@ -4127,6 +4127,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_self_imports`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_self_imports
 [`unnecessary_sort_by`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_sort_by
 [`unnecessary_to_owned`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_to_owned

clippy_lints/src/lib.register_lints.rs

@@ -563,6 +563,7 @@ store.register_lints(&[
     unnamed_address::FN_ADDRESS_COMPARISONS,
     unnamed_address::VTABLE_ADDRESS_COMPARISONS,
     unnecessary_owned_empty_strings::UNNECESSARY_OWNED_EMPTY_STRINGS,
+    unnecessary_reserve::UNNECESSARY_RESERVE,
     unnecessary_self_imports::UNNECESSARY_SELF_IMPORTS,
     unnecessary_sort_by::UNNECESSARY_SORT_BY,
     unnecessary_wraps::UNNECESSARY_WRAPS,

clippy_lints/src/lib.register_pedantic.rs

@@ -91,6 +91,7 @@ store.register_group(true, "clippy::pedantic", Some("clippy_pedantic"), vec![
     LintId::of(types::LINKEDLIST),
     LintId::of(types::OPTION_OPTION),
     LintId::of(unicode::UNICODE_NOT_NFC),
+    LintId::of(unnecessary_reserve::UNNECESSARY_RESERVE),
     LintId::of(unnecessary_wraps::UNNECESSARY_WRAPS),
     LintId::of(unnested_or_patterns::UNNESTED_OR_PATTERNS),
     LintId::of(unused_async::UNUSED_ASYNC),

clippy_lints/src/lib.rs

@@ -390,6 +390,7 @@ mod unit_return_expecting_ord;
 mod unit_types;
 mod unnamed_address;
 mod unnecessary_owned_empty_strings;
+mod unnecessary_reserve;
 mod unnecessary_self_imports;
 mod unnecessary_sort_by;
 mod unnecessary_wraps;
@@ -933,6 +934,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     store.register_late_pass(|| Box::new(std_instead_of_core::StdReexports::default()));
     store.register_late_pass(|| Box::new(manual_instant_elapsed::ManualInstantElapsed));
     store.register_late_pass(|| Box::new(partialeq_to_none::PartialeqToNone));
+    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,151 @@
+use clippy_utils::diagnostics::span_lint_and_then;
+use clippy_utils::{match_def_path, meets_msrv, msrvs, paths, visitors::expr_visitor_no_bodies};
+use rustc_errors::Applicability;
+use rustc_hir::{intravisit::Visitor, Block, ExprKind, QPath, StmtKind};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_middle::ty;
+use rustc_semver::RustcVersion;
+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"
+}
+
+pub struct UnnecessaryReserve {
+    msrv: Option<RustcVersion>,
+}
+
+impl UnnecessaryReserve {
+    #[must_use]
+    pub fn new(msrv: Option<RustcVersion>) -> Self {
+        Self { msrv }
+    }
+}
+
+impl_lint_pass!(UnnecessaryReserve => [UNNECESSARY_RESERVE]);
+
+impl<'tcx> LateLintPass<'tcx> for UnnecessaryReserve {
+    fn check_block(&mut self, cx: &LateContext<'tcx>, block: &Block<'tcx>) {
+        if !meets_msrv(self.msrv, msrvs::UNNECESSARY_RESERVE) {
+            return;
+        }
+
+        for (idx, stmt) in block.stmts.iter().enumerate() {
+            if let StmtKind::Semi(semi_expr) = stmt.kind
+                && let ExprKind::MethodCall(_, [struct_calling_on, _], _) = semi_expr.kind
+                && let Some(expr_def_id) = cx.typeck_results().type_dependent_def_id(semi_expr.hir_id)
+                && (match_def_path(cx, expr_def_id, &paths::VEC_RESERVE) ||
+                    match_def_path(cx, expr_def_id, &paths::VEC_DEQUE_RESERVE))
+                && acceptable_type(cx, struct_calling_on)
+                && let Some(next_stmt_span) = check_extend_method(cx, block, idx, struct_calling_on)
+                && !next_stmt_span.from_expansion()
+            {
+                span_lint_and_then(
+                    cx,
+                    UNNECESSARY_RESERVE,
+                    next_stmt_span,
+                    "unnecessary call to `reserve`",
+                    |diag| {
+                        diag.span_suggestion(
+                            semi_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<'_>,
+    idx: usize,
+    struct_expr: &rustc_hir::Expr<'_>,
+) -> Option<rustc_span::Span> {
+    let mut read_found = false;
+    let next_stmt_span;
+
+    let mut visitor = expr_visitor_no_bodies(|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)
+            && match_def_path(cx, expr_def_id, &paths::ITER_EXTEND)
+            && acceptable_type(cx, struct_calling_on)
+            && equal_ident(struct_calling_on, struct_expr)
+        {
+            read_found = true;
+        }
+        !read_found
+    });
+
+    if idx == block.stmts.len() - 1 {
+        if let Some(e) = block.expr {
+            visitor.visit_expr(e);
+            next_stmt_span = e.span;
+        } else {
+            return None;
+        }
+    } else {
+        let next_stmt = &block.stmts[idx + 1];
+        visitor.visit_stmt(next_stmt);
+        next_stmt_span = next_stmt.span;
+    }
+    drop(visitor);
+
+    if read_found {
+        return Some(next_stmt_span);
+    }
+
+    None
+}
+
+#[must_use]
+fn equal_ident(left: &rustc_hir::Expr<'_>, right: &rustc_hir::Expr<'_>) -> bool {
+    fn ident_name(expr: &rustc_hir::Expr<'_>) -> Option<rustc_span::Symbol> {
+        if let ExprKind::Path(QPath::Resolved(None, inner_path)) = expr.kind
+            && let [inner_seg] = inner_path.segments
+        {
+            return Some(inner_seg.ident.name);
+        }
+        None
+    }
+
+    ident_name(left) == ident_name(right)
+}

clippy_utils/src/msrvs.rs

@@ -12,7 +12,7 @@ macro_rules! msrv_aliases {
 
 // names may refer to stabilized feature flags or library items
 msrv_aliases! {
-    1,62,0 { BOOL_THEN_SOME }
+    1,62,0 { BOOL_THEN_SOME, UNNECESSARY_RESERVE }
     1,53,0 { OR_PATTERNS, MANUAL_BITS, BTREE_MAP_RETAIN, BTREE_SET_RETAIN }
     1,52,0 { STR_SPLIT_ONCE, REM_EUCLID_CONST }
     1,51,0 { BORROW_AS_PTR, UNSIGNED_ABS }

clippy_utils/src/paths.rs

@@ -70,6 +70,7 @@ pub const IPADDR_V4: [&str; 5] = ["std", "net", "ip", "IpAddr", "V4"];
 pub const IPADDR_V6: [&str; 5] = ["std", "net", "ip", "IpAddr", "V6"];
 pub const ITER_COUNT: [&str; 6] = ["core", "iter", "traits", "iterator", "Iterator", "count"];
 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 ITER_REPEAT: [&str; 5] = ["core", "iter", "sources", "repeat", "repeat"];
 #[expect(clippy::invalid_paths)] // internal lints do not know about all external crates
 pub const ITERTOOLS_NEXT_TUPLE: [&str; 3] = ["itertools", "Itertools", "next_tuple"];
@@ -191,6 +192,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 lint
+    vec.reserve(1);
+    vec.extend([1]);
+
+    // do lint
+    vec.reserve(array.len());
+    vec.extend(array);
+
+    // do lint
+    {
+        vec.reserve(1);
+        vec.extend([1])
+    };
+
+    // 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 lint
+    vec_deque.reserve(1);
+    vec_deque.extend([1]);
+
+    // do lint
+    vec_deque.reserve(array.len());
+    vec_deque.extend(array);
+
+    // do 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,52 @@
+error: unnecessary call to `reserve`
+  --> $DIR/unnecessary_reserve.rs:20:5
+   |
+LL |     vec.reserve(1);
+   |     -------------- help: remove this line
+LL |     vec.extend([1]);
+   |     ^^^^^^^^^^^^^^^^
+   |
+   = note: `-D clippy::unnecessary-reserve` implied by `-D warnings`
+
+error: unnecessary call to `reserve`
+  --> $DIR/unnecessary_reserve.rs:24:5
+   |
+LL |     vec.reserve(array.len());
+   |     ------------------------ help: remove this line
+LL |     vec.extend(array);
+   |     ^^^^^^^^^^^^^^^^^^
+
+error: unnecessary call to `reserve`
+  --> $DIR/unnecessary_reserve.rs:29:9
+   |
+LL |         vec.reserve(1);
+   |         -------------- help: remove this line
+LL |         vec.extend([1])
+   |         ^^^^^^^^^^^^^^^
+
+error: unnecessary call to `reserve`
+  --> $DIR/unnecessary_reserve.rs:49:5
+   |
+LL |     vec_deque.reserve(1);
+   |     -------------------- help: remove this line
+LL |     vec_deque.extend([1]);
+   |     ^^^^^^^^^^^^^^^^^^^^^^
+
+error: unnecessary call to `reserve`
+  --> $DIR/unnecessary_reserve.rs:53:5
+   |
+LL |     vec_deque.reserve(array.len());
+   |     ------------------------------ help: remove this line
+LL |     vec_deque.extend(array);
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: unnecessary call to `reserve`
+  --> $DIR/unnecessary_reserve.rs:58:9
+   |
+LL |         vec_deque.reserve(1);
+   |         -------------------- help: remove this line
+LL |         vec_deque.extend([1])
+   |         ^^^^^^^^^^^^^^^^^^^^^
+
+error: aborting due to 6 previous errors
+