Skip to content

Commit 3f7dc80

Browse files
committed
new lint: repeat_vec_with_capacity
1 parent abf01e4 commit 3f7dc80

7 files changed

+229
-0
lines changed

Diff for: CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -5400,6 +5400,7 @@ Released 2018-09-13
54005400
[`ref_patterns`]: https://rust-lang.github.io/rust-clippy/master/index.html#ref_patterns
54015401
[`regex_macro`]: https://rust-lang.github.io/rust-clippy/master/index.html#regex_macro
54025402
[`repeat_once`]: https://rust-lang.github.io/rust-clippy/master/index.html#repeat_once
5403+
[`repeat_vec_with_capacity`]: https://rust-lang.github.io/rust-clippy/master/index.html#repeat_vec_with_capacity
54035404
[`replace_consts`]: https://rust-lang.github.io/rust-clippy/master/index.html#replace_consts
54045405
[`reserve_after_initialization`]: https://rust-lang.github.io/rust-clippy/master/index.html#reserve_after_initialization
54055406
[`rest_pat_in_fully_bound_structs`]: https://rust-lang.github.io/rust-clippy/master/index.html#rest_pat_in_fully_bound_structs

Diff for: clippy_lints/src/declared_lints.rs

+1
Original file line numberDiff line numberDiff line change
@@ -594,6 +594,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
594594
crate::reference::DEREF_ADDROF_INFO,
595595
crate::regex::INVALID_REGEX_INFO,
596596
crate::regex::TRIVIAL_REGEX_INFO,
597+
crate::repeat_vec_with_capacity::REPEAT_VEC_WITH_CAPACITY_INFO,
597598
crate::reserve_after_initialization::RESERVE_AFTER_INITIALIZATION_INFO,
598599
crate::return_self_not_must_use::RETURN_SELF_NOT_MUST_USE_INFO,
599600
crate::returns::LET_AND_RETURN_INFO,

Diff for: clippy_lints/src/lib.rs

+2
Original file line numberDiff line numberDiff line change
@@ -288,6 +288,7 @@ mod ref_option_ref;
288288
mod ref_patterns;
289289
mod reference;
290290
mod regex;
291+
mod repeat_vec_with_capacity;
291292
mod reserve_after_initialization;
292293
mod return_self_not_must_use;
293294
mod returns;
@@ -1066,6 +1067,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
10661067
store.register_late_pass(move |_| Box::new(manual_hash_one::ManualHashOne::new(msrv())));
10671068
store.register_late_pass(|_| Box::new(iter_without_into_iter::IterWithoutIntoIter));
10681069
store.register_late_pass(|_| Box::new(iter_over_hash_type::IterOverHashType));
1070+
store.register_late_pass(|_| Box::new(repeat_vec_with_capacity::RepeatVecWithCapacity));
10691071
// add lints here, do not remove this comment, it's used in `new_lint`
10701072
}
10711073

Diff for: clippy_lints/src/repeat_vec_with_capacity.rs

+109
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
use clippy_utils::consts::{constant, Constant};
2+
use clippy_utils::diagnostics::span_lint_and_then;
3+
use clippy_utils::higher::VecArgs;
4+
use clippy_utils::macros::root_macro_call;
5+
use clippy_utils::source::snippet;
6+
use clippy_utils::{expr_or_init, fn_def_id, match_def_path, paths};
7+
use rustc_errors::Applicability;
8+
use rustc_hir::{Expr, ExprKind};
9+
use rustc_lint::{LateContext, LateLintPass};
10+
use rustc_session::{declare_lint_pass, declare_tool_lint};
11+
use rustc_span::{sym, Span};
12+
13+
declare_clippy_lint! {
14+
/// ### What it does
15+
/// Looks for patterns such as `vec![Vec::with_capacity(x); n]` or `iter::repeat(Vec::with_capacity(x))`.
16+
///
17+
/// ### Why is this bad?
18+
/// These constructs work by cloning the element, but cloning a `Vec<_>` does not
19+
/// respect the old vector's capacity and effectively discards it.
20+
///
21+
/// This makes `iter::repeat(Vec::with_capacity(x))` especially suspicious because the user most certainly
22+
/// expected that the yielded `Vec<_>` will have the requested capacity, otherwise one can simply write
23+
/// `iter::repeat(Vec::new())` instead and it will have the same effect.
24+
///
25+
/// Similarily for `vec![x; n]`, the element `x` is cloned to fill the vec.
26+
/// Unlike `iter::repeat` however, the vec repeat macro does not have to clone the value `n` times
27+
/// but just `n - 1` times, because it can reuse the passed value for the last slot.
28+
/// That means that the last `Vec<_>` gets the requested capacity but all other ones do not.
29+
///
30+
/// ### Example
31+
/// ```rust
32+
/// let _: Vec<Vec<u8>> = vec![Vec::with_capacity(42); 123];
33+
/// ```
34+
/// Use instead:
35+
/// ```rust
36+
/// let _: Vec<Vec<u8>> = (0..123).map(|_| Vec::with_capacity(42)).collect();
37+
/// // ^^^ this closure executes 123 times
38+
/// // and the vecs will have the expected capacity
39+
/// ```
40+
#[clippy::version = "1.74.0"]
41+
pub REPEAT_VEC_WITH_CAPACITY,
42+
suspicious,
43+
"repeating a `Vec::with_capacity` expression which does not retain capacity"
44+
}
45+
46+
declare_lint_pass!(RepeatVecWithCapacity => [REPEAT_VEC_WITH_CAPACITY]);
47+
48+
fn emit_lint(cx: &LateContext<'_>, span: Span, kind: &str, note: &'static str, sugg_msg: &'static str, sugg: String) {
49+
span_lint_and_then(
50+
cx,
51+
REPEAT_VEC_WITH_CAPACITY,
52+
span,
53+
&format!("repeating `Vec::with_capacity` using `{kind}`, which does not retain capacity"),
54+
|diag| {
55+
diag.note(note);
56+
diag.span_suggestion_verbose(span, sugg_msg, sugg, Applicability::MaybeIncorrect);
57+
},
58+
);
59+
}
60+
61+
/// Checks `vec![Vec::with_capacity(x); n]`
62+
fn check_vec_macro(cx: &LateContext<'_>, expr: &Expr<'_>) {
63+
if let Some(mac_call) = root_macro_call(expr.span)
64+
&& cx.tcx.is_diagnostic_item(sym::vec_macro, mac_call.def_id)
65+
&& let Some(VecArgs::Repeat(repeat_expr, len_expr)) = VecArgs::hir(cx, expr)
66+
&& fn_def_id(cx, repeat_expr).is_some_and(|did| match_def_path(cx, did, &paths::VEC_WITH_CAPACITY))
67+
&& !len_expr.span.from_expansion()
68+
&& let Some(Constant::Int(2..)) = constant(cx, cx.typeck_results(), expr_or_init(cx, len_expr))
69+
{
70+
emit_lint(
71+
cx,
72+
expr.span.source_callsite(),
73+
"vec![x; n]",
74+
"only the last `Vec` will have the capacity",
75+
"if you intended to initialize multiple `Vec`s with an initial capacity, try",
76+
format!(
77+
"(0..{}).map(|_| {}).collect::<Vec<_>>()",
78+
snippet(cx, len_expr.span, ""),
79+
snippet(cx, repeat_expr.span, "..")
80+
),
81+
);
82+
}
83+
}
84+
85+
/// Checks `iter::repeat(Vec::with_capacity(x))`
86+
fn check_repeat_fn(cx: &LateContext<'_>, expr: &Expr<'_>) {
87+
if !expr.span.from_expansion()
88+
&& fn_def_id(cx, expr).is_some_and(|did| cx.tcx.is_diagnostic_item(sym::iter_repeat, did))
89+
&& let ExprKind::Call(_, [repeat_expr]) = expr.kind
90+
&& fn_def_id(cx, repeat_expr).is_some_and(|did| match_def_path(cx, did, &paths::VEC_WITH_CAPACITY))
91+
&& !repeat_expr.span.from_expansion()
92+
{
93+
emit_lint(
94+
cx,
95+
expr.span,
96+
"iter::repeat",
97+
"none of the yielded `Vec`s will have the requested capacity",
98+
"if you intended to create an iterator that yields `Vec`s with an initial capacity, try",
99+
format!("std::iter::from_fn(|| Some({}))", snippet(cx, repeat_expr.span, "..")),
100+
);
101+
}
102+
}
103+
104+
impl LateLintPass<'_> for RepeatVecWithCapacity {
105+
fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) {
106+
check_vec_macro(cx, expr);
107+
check_repeat_fn(cx, expr);
108+
}
109+
}

Diff for: tests/ui/repeat_vec_with_capacity.fixed

+38
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
#![warn(clippy::repeat_vec_with_capacity)]
2+
3+
fn main() {
4+
{
5+
(0..123).map(|_| Vec::<()>::with_capacity(42)).collect::<Vec<_>>();
6+
//~^ ERROR: repeating `Vec::with_capacity` using `vec![x; n]`, which does not retain capacity
7+
}
8+
9+
{
10+
let n = 123;
11+
(0..n).map(|_| Vec::<()>::with_capacity(42)).collect::<Vec<_>>();
12+
//~^ ERROR: repeating `Vec::with_capacity` using `vec![x; n]`, which does not retain capacity
13+
}
14+
15+
{
16+
macro_rules! from_macro {
17+
($x:expr) => {
18+
vec![$x; 123];
19+
};
20+
}
21+
// vec expansion is from another macro, don't lint
22+
from_macro!(Vec::<()>::with_capacity(42));
23+
}
24+
25+
{
26+
std::iter::from_fn(|| Some(Vec::<()>::with_capacity(42)));
27+
//~^ ERROR: repeating `Vec::with_capacity` using `iter::repeat`, which does not retain capacity
28+
}
29+
30+
{
31+
macro_rules! from_macro {
32+
($x:expr) => {
33+
std::iter::repeat($x)
34+
};
35+
}
36+
from_macro!(Vec::<()>::with_capacity(42));
37+
}
38+
}

Diff for: tests/ui/repeat_vec_with_capacity.rs

+38
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
#![warn(clippy::repeat_vec_with_capacity)]
2+
3+
fn main() {
4+
{
5+
vec![Vec::<()>::with_capacity(42); 123];
6+
//~^ ERROR: repeating `Vec::with_capacity` using `vec![x; n]`, which does not retain capacity
7+
}
8+
9+
{
10+
let n = 123;
11+
vec![Vec::<()>::with_capacity(42); n];
12+
//~^ ERROR: repeating `Vec::with_capacity` using `vec![x; n]`, which does not retain capacity
13+
}
14+
15+
{
16+
macro_rules! from_macro {
17+
($x:expr) => {
18+
vec![$x; 123];
19+
};
20+
}
21+
// vec expansion is from another macro, don't lint
22+
from_macro!(Vec::<()>::with_capacity(42));
23+
}
24+
25+
{
26+
std::iter::repeat(Vec::<()>::with_capacity(42));
27+
//~^ ERROR: repeating `Vec::with_capacity` using `iter::repeat`, which does not retain capacity
28+
}
29+
30+
{
31+
macro_rules! from_macro {
32+
($x:expr) => {
33+
std::iter::repeat($x)
34+
};
35+
}
36+
from_macro!(Vec::<()>::with_capacity(42));
37+
}
38+
}

Diff for: tests/ui/repeat_vec_with_capacity.stderr

+40
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
error: repeating `Vec::with_capacity` using `vec![x; n]`, which does not retain capacity
2+
--> $DIR/repeat_vec_with_capacity.rs:5:9
3+
|
4+
LL | vec![Vec::<()>::with_capacity(42); 123];
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
6+
|
7+
= note: only the last `Vec` will have the capacity
8+
= note: `-D clippy::repeat-vec-with-capacity` implied by `-D warnings`
9+
= help: to override `-D warnings` add `#[allow(clippy::repeat_vec_with_capacity)]`
10+
help: if you intended to initialize multiple `Vec`s with an initial capacity, try
11+
|
12+
LL | (0..123).map(|_| Vec::<()>::with_capacity(42)).collect::<Vec<_>>();
13+
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
14+
15+
error: repeating `Vec::with_capacity` using `vec![x; n]`, which does not retain capacity
16+
--> $DIR/repeat_vec_with_capacity.rs:11:9
17+
|
18+
LL | vec![Vec::<()>::with_capacity(42); n];
19+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
20+
|
21+
= note: only the last `Vec` will have the capacity
22+
help: if you intended to initialize multiple `Vec`s with an initial capacity, try
23+
|
24+
LL | (0..n).map(|_| Vec::<()>::with_capacity(42)).collect::<Vec<_>>();
25+
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
26+
27+
error: repeating `Vec::with_capacity` using `iter::repeat`, which does not retain capacity
28+
--> $DIR/repeat_vec_with_capacity.rs:26:9
29+
|
30+
LL | std::iter::repeat(Vec::<()>::with_capacity(42));
31+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
32+
|
33+
= note: none of the yielded `Vec`s will have the requested capacity
34+
help: if you intended to create an iterator that yields `Vec`s with an initial capacity, try
35+
|
36+
LL | std::iter::from_fn(|| Some(Vec::<()>::with_capacity(42)));
37+
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
38+
39+
error: aborting due to 3 previous errors
40+

0 commit comments

Comments
 (0)