Skip to content

Commit 7fbf808

Browse files
committed
Added new lint: reserve_after_initialization
1 parent fc1152a commit 7fbf808

File tree

7 files changed

+268
-0
lines changed

7 files changed

+268
-0
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5299,6 +5299,7 @@ Released 2018-09-13
52995299
[`regex_macro`]: https://rust-lang.github.io/rust-clippy/master/index.html#regex_macro
53005300
[`repeat_once`]: https://rust-lang.github.io/rust-clippy/master/index.html#repeat_once
53015301
[`replace_consts`]: https://rust-lang.github.io/rust-clippy/master/index.html#replace_consts
5302+
[`reserve_after_initialization`]: https://rust-lang.github.io/rust-clippy/master/index.html#reserve_after_initialization
53025303
[`rest_pat_in_fully_bound_structs`]: https://rust-lang.github.io/rust-clippy/master/index.html#rest_pat_in_fully_bound_structs
53035304
[`result_expect_used`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_expect_used
53045305
[`result_large_err`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_large_err

clippy_lints/src/declared_lints.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -579,6 +579,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
579579
crate::reference::DEREF_ADDROF_INFO,
580580
crate::regex::INVALID_REGEX_INFO,
581581
crate::regex::TRIVIAL_REGEX_INFO,
582+
crate::reserve_after_initialization::RESERVE_AFTER_INITIALIZATION_INFO,
582583
crate::return_self_not_must_use::RETURN_SELF_NOT_MUST_USE_INFO,
583584
crate::returns::LET_AND_RETURN_INFO,
584585
crate::returns::NEEDLESS_RETURN_INFO,

clippy_lints/src/lib.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -285,6 +285,7 @@ mod ref_option_ref;
285285
mod ref_patterns;
286286
mod reference;
287287
mod regex;
288+
mod reserve_after_initialization;
288289
mod return_self_not_must_use;
289290
mod returns;
290291
mod same_name_method;
@@ -1095,6 +1096,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
10951096
});
10961097
store.register_late_pass(|_| Box::new(redundant_locals::RedundantLocals));
10971098
store.register_late_pass(|_| Box::new(ignored_unit_patterns::IgnoredUnitPatterns));
1099+
store.register_late_pass(|_| Box::new(reserve_after_initialization::ReserveAfterInitialization::default()));
10981100
// add lints here, do not remove this comment, it's used in `new_lint`
10991101
}
11001102

Lines changed: 242 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,242 @@
1+
use clippy_utils::diagnostics::span_lint_and_sugg;
2+
use clippy_utils::higher::{get_vec_init_kind, VecInitKind};
3+
use clippy_utils::source::snippet;
4+
use clippy_utils::visitors::for_each_local_use_after_expr;
5+
use clippy_utils::{get_parent_expr, path_to_local_id};
6+
use core::ops::ControlFlow;
7+
use rustc_ast::LitKind;
8+
use rustc_errors::Applicability;
9+
use rustc_hir::def::Res;
10+
use rustc_hir::{
11+
BindingAnnotation, Block, Expr, ExprKind, HirId, Local, Mutability, PatKind, QPath, Stmt, StmtKind, UnOp,
12+
};
13+
use rustc_lint::{LateContext, LateLintPass, LintContext};
14+
use rustc_middle::lint::in_external_macro;
15+
use rustc_session::{declare_tool_lint, impl_lint_pass};
16+
use rustc_span::{Span, Symbol};
17+
18+
declare_clippy_lint! {
19+
/// ### What it does
20+
/// Informs the user about a more concise way to create a vector with a known capacity.
21+
///
22+
/// ### Why is this bad?
23+
/// The `Vec::with_capacity` constructor is easier to understand.
24+
///
25+
/// ### Example
26+
/// ```rust
27+
/// {
28+
/// let mut v = vec![];
29+
/// v.reserve(space_hint);
30+
/// v
31+
/// }
32+
/// ```
33+
/// Use instead:
34+
/// ```rust
35+
/// Vec::with_capacity(space_hint)
36+
/// ```
37+
#[clippy::version = "1.73.0"]
38+
pub RESERVE_AFTER_INITIALIZATION,
39+
complexity,
40+
"`reserve` called immediatly after `Vec` creation"
41+
}
42+
impl_lint_pass!(ReserveAfterInitialization => [RESERVE_AFTER_INITIALIZATION]);
43+
44+
/*impl<'tcx> LateLintPass<'tcx> for ReserveAfterInitialization {
45+
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx rustc_hir::Expr<'_>) {
46+
if let ExprKind::Assign(left, right, _) = expr.kind
47+
&& let ExprKind::Path(QPath::Resolved(None, path)) = left.kind
48+
&& let [name] = &path.segments
49+
&& let Res::Local(id) = path.res
50+
&& let Some(init) = get_vec_init_kind(cx, right)
51+
&& !matches!(init, VecInitKind::WithExprCapacity(_)) {
52+
span_lint_and_help(
53+
cx,
54+
RESERVE_AFTER_INITIALIZATION,
55+
expr.span,
56+
"`reserve` called just after the initialisation of the vector",
57+
None,
58+
"use `Vec::with_capacity(space_hint)` instead"
59+
);
60+
}
61+
}
62+
63+
/*fn check_block(&mut self, cx: &LateContext<'_>, block: &'_ rustc_hir::Block<'_>) {
64+
span_lint_and_help(
65+
cx,
66+
RESERVE_AFTER_INITIALIZATION,
67+
block.span,
68+
"`reserve` called just after the initialisation of the vector",
69+
None,
70+
"use `Vec::with_capacity(space_hint)` instead"
71+
);
72+
}*/
73+
}*/
74+
75+
#[derive(Default)]
76+
pub struct ReserveAfterInitialization {
77+
searcher: Option<VecReserveSearcher>,
78+
}
79+
80+
struct VecReserveSearcher {
81+
local_id: HirId,
82+
lhs_is_let: bool,
83+
let_ty_span: Option<Span>,
84+
name: Symbol,
85+
err_span: Span,
86+
last_reserve_expr: HirId,
87+
space_hint: usize,
88+
}
89+
impl VecReserveSearcher {
90+
fn display_err(&self, cx: &LateContext<'_>) {
91+
if self.space_hint == 0 {
92+
return;
93+
}
94+
95+
let mut needs_mut = false;
96+
let _res = for_each_local_use_after_expr(cx, self.local_id, self.last_reserve_expr, |e| {
97+
let Some(parent) = get_parent_expr(cx, e) else {
98+
return ControlFlow::Continue(());
99+
};
100+
let adjusted_ty = cx.typeck_results().expr_ty_adjusted(e);
101+
let adjusted_mut = adjusted_ty.ref_mutability().unwrap_or(Mutability::Not);
102+
needs_mut |= adjusted_mut == Mutability::Mut;
103+
match parent.kind {
104+
ExprKind::AddrOf(_, Mutability::Mut, _) => {
105+
needs_mut = true;
106+
return ControlFlow::Break(true);
107+
},
108+
ExprKind::Unary(UnOp::Deref, _) | ExprKind::Index(..) if !needs_mut => {
109+
let mut last_place = parent;
110+
while let Some(parent) = get_parent_expr(cx, last_place) {
111+
if matches!(parent.kind, ExprKind::Unary(UnOp::Deref, _) | ExprKind::Field(..))
112+
|| matches!(parent.kind, ExprKind::Index(e, _, _) if e.hir_id == last_place.hir_id)
113+
{
114+
last_place = parent;
115+
} else {
116+
break;
117+
}
118+
}
119+
needs_mut |= cx.typeck_results().expr_ty_adjusted(last_place).ref_mutability()
120+
== Some(Mutability::Mut)
121+
|| get_parent_expr(cx, last_place)
122+
.map_or(false, |e| matches!(e.kind, ExprKind::AddrOf(_, Mutability::Mut, _)));
123+
},
124+
ExprKind::MethodCall(_, recv, ..)
125+
if recv.hir_id == e.hir_id
126+
&& adjusted_mut == Mutability::Mut
127+
&& !adjusted_ty.peel_refs().is_slice() =>
128+
{
129+
// No need to set `needs_mut` to true. The receiver will be either explicitly borrowed, or it will
130+
// be implicitly borrowed via an adjustment. Both of these cases are already handled by this point.
131+
return ControlFlow::Break(true);
132+
},
133+
ExprKind::Assign(lhs, ..) if e.hir_id == lhs.hir_id => {
134+
needs_mut = true;
135+
return ControlFlow::Break(false);
136+
},
137+
_ => (),
138+
}
139+
ControlFlow::Continue(())
140+
});
141+
142+
let mut s = if self.lhs_is_let {
143+
String::from("let ")
144+
} else {
145+
String::new()
146+
};
147+
if needs_mut {
148+
s.push_str("mut ");
149+
}
150+
s.push_str(self.name.as_str());
151+
if let Some(span) = self.let_ty_span {
152+
s.push_str(": ");
153+
s.push_str(&snippet(cx, span, "_"));
154+
}
155+
s.push_str(format!(" = Vec::with_capacity({});", self.space_hint).as_str());
156+
157+
span_lint_and_sugg(
158+
cx,
159+
RESERVE_AFTER_INITIALIZATION,
160+
self.err_span,
161+
"calls to `reverse` immediately after creation",
162+
"consider using `Vec::with_capacity(space_hint)`",
163+
s,
164+
Applicability::HasPlaceholders,
165+
);
166+
}
167+
}
168+
169+
impl<'tcx> LateLintPass<'tcx> for ReserveAfterInitialization {
170+
fn check_block(&mut self, _: &LateContext<'tcx>, _: &'tcx Block<'tcx>) {
171+
self.searcher = None;
172+
}
173+
174+
fn check_local(&mut self, cx: &LateContext<'tcx>, local: &'tcx Local<'tcx>) {
175+
if let Some(init_expr) = local.init
176+
&& let PatKind::Binding(BindingAnnotation::MUT, id, name, None) = local.pat.kind
177+
&& !in_external_macro(cx.sess(), local.span)
178+
&& let Some(init) = get_vec_init_kind(cx, init_expr)
179+
&& !matches!(init, VecInitKind::WithExprCapacity(_))
180+
{
181+
self.searcher = Some(VecReserveSearcher {
182+
local_id: id,
183+
lhs_is_let: true,
184+
name: name.name,
185+
let_ty_span: local.ty.map(|ty| ty.span),
186+
err_span: local.span,
187+
last_reserve_expr: init_expr.hir_id,
188+
space_hint: 0
189+
});
190+
}
191+
}
192+
193+
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
194+
if self.searcher.is_none()
195+
&& let ExprKind::Assign(left, right, _) = expr.kind
196+
&& let ExprKind::Path(QPath::Resolved(None, path)) = left.kind
197+
&& let [name] = &path.segments
198+
&& let Res::Local(id) = path.res
199+
&& !in_external_macro(cx.sess(), expr.span)
200+
&& let Some(init) = get_vec_init_kind(cx, right)
201+
&& !matches!(init, VecInitKind::WithExprCapacity(_))
202+
{
203+
self.searcher = Some(VecReserveSearcher {
204+
local_id: id,
205+
lhs_is_let: false,
206+
let_ty_span: None,
207+
name: name.ident.name,
208+
err_span: expr.span,
209+
last_reserve_expr: expr.hir_id,
210+
space_hint: 0
211+
});
212+
}
213+
}
214+
215+
fn check_stmt(&mut self, cx: &LateContext<'tcx>, stmt: &'tcx Stmt<'_>) {
216+
if let Some(searcher) = self.searcher.take() {
217+
if let StmtKind::Expr(expr) | StmtKind::Semi(expr) = stmt.kind
218+
&& let ExprKind::MethodCall(name, self_arg, other_args, _) = expr.kind
219+
&& other_args.len() == 1
220+
&& let ExprKind::Lit(lit) = other_args[0].kind
221+
&& let LitKind::Int(space_hint, _) = lit.node
222+
&& path_to_local_id(self_arg, searcher.local_id)
223+
&& name.ident.as_str() == "reserve"
224+
{
225+
self.searcher = Some(VecReserveSearcher {
226+
err_span: searcher.err_span.to(stmt.span),
227+
last_reserve_expr: expr.hir_id,
228+
space_hint: space_hint as usize,
229+
.. searcher
230+
});
231+
} else {
232+
searcher.display_err(cx);
233+
}
234+
}
235+
}
236+
237+
fn check_block_post(&mut self, cx: &LateContext<'tcx>, _: &'tcx Block<'tcx>) {
238+
if let Some(searcher) = self.searcher.take() {
239+
searcher.display_err(cx);
240+
}
241+
}
242+
}
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
#![warn(clippy::reserve_after_initialization)]
2+
3+
fn main() {
4+
let v: Vec<usize> = Vec::with_capacity(10);
5+
}
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
#![warn(clippy::reserve_after_initialization)]
2+
3+
fn main() {
4+
let mut v: Vec<usize> = vec![];
5+
v.reserve(10);
6+
}
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
error: calls to `reverse` immediately after creation
2+
--> $DIR/reserve_after_initialization.rs:4:5
3+
|
4+
LL | / let mut v: Vec<usize> = vec![];
5+
LL | | v.reserve(10);
6+
| |__________________^ help: consider using `Vec::with_capacity(space_hint)`: `let v: Vec<usize> = Vec::with_capacity(10);`
7+
|
8+
= note: `-D clippy::reserve-after-initialization` implied by `-D warnings`
9+
10+
error: aborting due to previous error
11+

0 commit comments

Comments
 (0)