new lint: repeat_vec_with_capacity

CHANGELOG.md

@@ -5462,6 +5462,7 @@ Released 2018-09-13
 [`ref_patterns`]: https://rust-lang.github.io/rust-clippy/master/index.html#ref_patterns
 [`regex_macro`]: https://rust-lang.github.io/rust-clippy/master/index.html#regex_macro
 [`repeat_once`]: https://rust-lang.github.io/rust-clippy/master/index.html#repeat_once
+[`repeat_vec_with_capacity`]: https://rust-lang.github.io/rust-clippy/master/index.html#repeat_vec_with_capacity
 [`replace_consts`]: https://rust-lang.github.io/rust-clippy/master/index.html#replace_consts
 [`reserve_after_initialization`]: https://rust-lang.github.io/rust-clippy/master/index.html#reserve_after_initialization
 [`rest_pat_in_fully_bound_structs`]: https://rust-lang.github.io/rust-clippy/master/index.html#rest_pat_in_fully_bound_structs

clippy_lints/src/declared_lints.rs

@@ -598,6 +598,7 @@
     crate::reference::DEREF_ADDROF_INFO,
     crate::regex::INVALID_REGEX_INFO,
     crate::regex::TRIVIAL_REGEX_INFO,
+    crate::repeat_vec_with_capacity::REPEAT_VEC_WITH_CAPACITY_INFO,
     crate::reserve_after_initialization::RESERVE_AFTER_INITIALIZATION_INFO,
     crate::return_self_not_must_use::RETURN_SELF_NOT_MUST_USE_INFO,
     crate::returns::LET_AND_RETURN_INFO,

clippy_lints/src/lib.rs

@@ -289,6 +289,7 @@
 mod ref_patterns;
 mod reference;
 mod regex;
+mod repeat_vec_with_capacity;
 mod reserve_after_initialization;
 mod return_self_not_must_use;
 mod returns;
@@ -1069,6 +1070,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
     store.register_late_pass(|_| Box::new(iter_without_into_iter::IterWithoutIntoIter));
     store.register_late_pass(|_| Box::new(iter_over_hash_type::IterOverHashType));
     store.register_late_pass(|_| Box::new(impl_hash_with_borrow_str_and_bytes::ImplHashWithBorrowStrBytes));
+    store.register_late_pass(|_| Box::new(repeat_vec_with_capacity::RepeatVecWithCapacity));
     // add lints here, do not remove this comment, it's used in `new_lint`
 }
 

clippy_lints/src/repeat_vec_with_capacity.rs

@@ -0,0 +1,109 @@
+use clippy_utils::consts::{constant, Constant};
+use clippy_utils::diagnostics::span_lint_and_then;
+use clippy_utils::higher::VecArgs;
+use clippy_utils::macros::root_macro_call;
+use clippy_utils::source::snippet;
+use clippy_utils::{expr_or_init, fn_def_id, match_def_path, paths};
+use rustc_errors::Applicability;
+use rustc_hir::{Expr, ExprKind};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_session::{declare_lint_pass, declare_tool_lint};
+use rustc_span::{sym, Span};
+
+declare_clippy_lint! {
+    /// ### What it does
+    /// Looks for patterns such as `vec![Vec::with_capacity(x); n]` or `iter::repeat(Vec::with_capacity(x))`.
+    ///
+    /// ### Why is this bad?
+    /// These constructs work by cloning the element, but cloning a `Vec<_>` does not
+    /// respect the old vector's capacity and effectively discards it.
+    ///
+    /// This makes `iter::repeat(Vec::with_capacity(x))` especially suspicious because the user most certainly
+    /// expected that the yielded `Vec<_>` will have the requested capacity, otherwise one can simply write
+    /// `iter::repeat(Vec::new())` instead and it will have the same effect.
+    ///
+    /// Similarily for `vec![x; n]`, the element `x` is cloned to fill the vec.
+    /// Unlike `iter::repeat` however, the vec repeat macro does not have to clone the value `n` times
+    /// but just `n - 1` times, because it can reuse the passed value for the last slot.
+    /// That means that the last `Vec<_>` gets the requested capacity but all other ones do not.
+    ///
+    /// ### Example
+    /// ```rust
+    /// let _: Vec<Vec<u8>> = vec![Vec::with_capacity(42); 123];
+    /// ```
+    /// Use instead:
+    /// ```rust
+    /// let _: Vec<Vec<u8>> = (0..123).map(|_| Vec::with_capacity(42)).collect();
+    /// //                                 ^^^ this closure executes 123 times
+    /// //                                     and the vecs will have the expected capacity
+    /// ```
+    #[clippy::version = "1.74.0"]
+    pub REPEAT_VEC_WITH_CAPACITY,
+    suspicious,
+    "repeating a `Vec::with_capacity` expression which does not retain capacity"
+}
+
+declare_lint_pass!(RepeatVecWithCapacity => [REPEAT_VEC_WITH_CAPACITY]);
+
+fn emit_lint(cx: &LateContext<'_>, span: Span, kind: &str, note: &'static str, sugg_msg: &'static str, sugg: String) {
+    span_lint_and_then(
+        cx,
+        REPEAT_VEC_WITH_CAPACITY,
+        span,
+        &format!("repeating `Vec::with_capacity` using `{kind}`, which does not retain capacity"),
+        |diag| {
+            diag.note(note);
+            diag.span_suggestion_verbose(span, sugg_msg, sugg, Applicability::MaybeIncorrect);
+        },
+    );
+}
+
+/// Checks `vec![Vec::with_capacity(x); n]`
+fn check_vec_macro(cx: &LateContext<'_>, expr: &Expr<'_>) {
+    if let Some(mac_call) = root_macro_call(expr.span)
+        && cx.tcx.is_diagnostic_item(sym::vec_macro, mac_call.def_id)
+        && let Some(VecArgs::Repeat(repeat_expr, len_expr)) = VecArgs::hir(cx, expr)
+        && fn_def_id(cx, repeat_expr).is_some_and(|did| match_def_path(cx, did, &paths::VEC_WITH_CAPACITY))
+        && !len_expr.span.from_expansion()
+        && let Some(Constant::Int(2..)) = constant(cx, cx.typeck_results(), expr_or_init(cx, len_expr))
+    {
+        emit_lint(
+            cx,
+            expr.span.source_callsite(),
+            "vec![x; n]",
+            "only the last `Vec` will have the capacity",
+            "if you intended to initialize multiple `Vec`s with an initial capacity, try",
+            format!(
+                "(0..{}).map(|_| {}).collect::<Vec<_>>()",
+                snippet(cx, len_expr.span, ""),
+                snippet(cx, repeat_expr.span, "..")
+            ),
+        );
+    }
+}
+
+/// Checks `iter::repeat(Vec::with_capacity(x))`
+fn check_repeat_fn(cx: &LateContext<'_>, expr: &Expr<'_>) {
+    if !expr.span.from_expansion()
+        && fn_def_id(cx, expr).is_some_and(|did| cx.tcx.is_diagnostic_item(sym::iter_repeat, did))
+        && let ExprKind::Call(_, [repeat_expr]) = expr.kind
+        && fn_def_id(cx, repeat_expr).is_some_and(|did| match_def_path(cx, did, &paths::VEC_WITH_CAPACITY))
+        && !repeat_expr.span.from_expansion()
+    {
+        emit_lint(
+            cx,
+            expr.span,
+            "iter::repeat",
+            "none of the yielded `Vec`s will have the requested capacity",
+            "if you intended to create an iterator that yields `Vec`s with an initial capacity, try",
+            format!("std::iter::from_fn(|| Some({}))", snippet(cx, repeat_expr.span, "..")),
+        );
+    }
+}
+
+impl LateLintPass<'_> for RepeatVecWithCapacity {
+    fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) {
+        check_vec_macro(cx, expr);
+        check_repeat_fn(cx, expr);
+    }
+}

tests/ui/repeat_vec_with_capacity.fixed

@@ -0,0 +1,38 @@
+#![warn(clippy::repeat_vec_with_capacity)]
+
+fn main() {
+    {
+        (0..123).map(|_| Vec::<()>::with_capacity(42)).collect::<Vec<_>>();
+        //~^ ERROR: repeating `Vec::with_capacity` using `vec![x; n]`, which does not retain capacity
+    }
+
+    {
+        let n = 123;
+        (0..n).map(|_| Vec::<()>::with_capacity(42)).collect::<Vec<_>>();
+        //~^ ERROR: repeating `Vec::with_capacity` using `vec![x; n]`, which does not retain capacity
+    }
+
+    {
+        macro_rules! from_macro {
+            ($x:expr) => {
+                vec![$x; 123];
+            };
+        }
+        // vec expansion is from another macro, don't lint
+        from_macro!(Vec::<()>::with_capacity(42));
+    }
+
+    {
+        std::iter::from_fn(|| Some(Vec::<()>::with_capacity(42)));
+        //~^ ERROR: repeating `Vec::with_capacity` using `iter::repeat`, which does not retain capacity
+    }
+
+    {
+        macro_rules! from_macro {
+            ($x:expr) => {
+                std::iter::repeat($x)
+            };
+        }
+        from_macro!(Vec::<()>::with_capacity(42));
+    }
+}

tests/ui/repeat_vec_with_capacity.rs

@@ -0,0 +1,38 @@
+#![warn(clippy::repeat_vec_with_capacity)]
+
+fn main() {
+    {
+        vec![Vec::<()>::with_capacity(42); 123];
+        //~^ ERROR: repeating `Vec::with_capacity` using `vec![x; n]`, which does not retain capacity
+    }
+
+    {
+        let n = 123;
+        vec![Vec::<()>::with_capacity(42); n];
+        //~^ ERROR: repeating `Vec::with_capacity` using `vec![x; n]`, which does not retain capacity
+    }
+
+    {
+        macro_rules! from_macro {
+            ($x:expr) => {
+                vec![$x; 123];
+            };
+        }
+        // vec expansion is from another macro, don't lint
+        from_macro!(Vec::<()>::with_capacity(42));
+    }
+
+    {
+        std::iter::repeat(Vec::<()>::with_capacity(42));
+        //~^ ERROR: repeating `Vec::with_capacity` using `iter::repeat`, which does not retain capacity
+    }
+
+    {
+        macro_rules! from_macro {
+            ($x:expr) => {
+                std::iter::repeat($x)
+            };
+        }
+        from_macro!(Vec::<()>::with_capacity(42));
+    }
+}

tests/ui/repeat_vec_with_capacity.stderr

@@ -0,0 +1,40 @@
+error: repeating `Vec::with_capacity` using `vec![x; n]`, which does not retain capacity
+  --> $DIR/repeat_vec_with_capacity.rs:5:9
+   |
+LL |         vec![Vec::<()>::with_capacity(42); 123];
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: only the last `Vec` will have the capacity
+   = note: `-D clippy::repeat-vec-with-capacity` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::repeat_vec_with_capacity)]`
+help: if you intended to initialize multiple `Vec`s with an initial capacity, try
+   |
+LL |         (0..123).map(|_| Vec::<()>::with_capacity(42)).collect::<Vec<_>>();
+   |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+error: repeating `Vec::with_capacity` using `vec![x; n]`, which does not retain capacity
+  --> $DIR/repeat_vec_with_capacity.rs:11:9
+   |
+LL |         vec![Vec::<()>::with_capacity(42); n];
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: only the last `Vec` will have the capacity
+help: if you intended to initialize multiple `Vec`s with an initial capacity, try
+   |
+LL |         (0..n).map(|_| Vec::<()>::with_capacity(42)).collect::<Vec<_>>();
+   |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+error: repeating `Vec::with_capacity` using `iter::repeat`, which does not retain capacity
+  --> $DIR/repeat_vec_with_capacity.rs:26:9
+   |
+LL |         std::iter::repeat(Vec::<()>::with_capacity(42));
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: none of the yielded `Vec`s will have the requested capacity
+help: if you intended to create an iterator that yields `Vec`s with an initial capacity, try
+   |
+LL |         std::iter::from_fn(|| Some(Vec::<()>::with_capacity(42)));
+   |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+error: aborting due to 3 previous errors
+