Skip to content

Commit 976850b

Browse files
borsbors
committed
Auto merge of #6538 - Jarcho:vec_init_then_push, r=llogiq
New lint: vec_init_then_push fixes: #1483 This will trigger on `new`, `default`, and `with_capacity` when the given capacity is less than or equal to the number of push calls. Is there anything else this should trigger on? changelog: Added lint: `vec_init_then_push`
2 parents 445eb99 + 7b5f549 commit 976850b

8 files changed

+257
-7
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2281,6 +2281,7 @@ Released 2018-09-13
22812281
[`useless_transmute`]: https://rust-lang.github.io/rust-clippy/master/index.html#useless_transmute
22822282
[`useless_vec`]: https://rust-lang.github.io/rust-clippy/master/index.html#useless_vec
22832283
[`vec_box`]: https://rust-lang.github.io/rust-clippy/master/index.html#vec_box
2284+
[`vec_init_then_push`]: https://rust-lang.github.io/rust-clippy/master/index.html#vec_init_then_push
22842285
[`vec_resize_to_zero`]: https://rust-lang.github.io/rust-clippy/master/index.html#vec_resize_to_zero
22852286
[`verbose_bit_mask`]: https://rust-lang.github.io/rust-clippy/master/index.html#verbose_bit_mask
22862287
[`verbose_file_reads`]: https://rust-lang.github.io/rust-clippy/master/index.html#verbose_file_reads

‎clippy_lints/src/lib.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -342,6 +342,7 @@ mod unwrap_in_result;
342342
mod use_self;
343343
mod useless_conversion;
344344
mod vec;
345+
mod vec_init_then_push;
345346
mod vec_resize_to_zero;
346347
mod verbose_file_reads;
347348
mod wildcard_dependencies;
@@ -938,6 +939,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
938939
&use_self::USE_SELF,
939940
&useless_conversion::USELESS_CONVERSION,
940941
&vec::USELESS_VEC,
942+
&vec_init_then_push::VEC_INIT_THEN_PUSH,
941943
&vec_resize_to_zero::VEC_RESIZE_TO_ZERO,
942944
&verbose_file_reads::VERBOSE_FILE_READS,
943945
&wildcard_dependencies::WILDCARD_DEPENDENCIES,
@@ -1219,6 +1221,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
12191221
store.register_late_pass(|| box strings::StrToString);
12201222
store.register_late_pass(|| box strings::StringToString);
12211223
store.register_late_pass(|| box zero_sized_map_values::ZeroSizedMapValues);
1224+
store.register_late_pass(|| box vec_init_then_push::VecInitThenPush::default());
12221225

12231226
store.register_group(true, "clippy::restriction", Some("clippy_restriction"), vec![
12241227
LintId::of(&arithmetic::FLOAT_ARITHMETIC),
@@ -1642,6 +1645,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
16421645
LintId::of(&unwrap::UNNECESSARY_UNWRAP),
16431646
LintId::of(&useless_conversion::USELESS_CONVERSION),
16441647
LintId::of(&vec::USELESS_VEC),
1648+
LintId::of(&vec_init_then_push::VEC_INIT_THEN_PUSH),
16451649
LintId::of(&vec_resize_to_zero::VEC_RESIZE_TO_ZERO),
16461650
LintId::of(&write::PRINTLN_EMPTY_STRING),
16471651
LintId::of(&write::PRINT_LITERAL),
@@ -1943,6 +1947,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
19431947
LintId::of(&types::BOX_VEC),
19441948
LintId::of(&types::REDUNDANT_ALLOCATION),
19451949
LintId::of(&vec::USELESS_VEC),
1950+
LintId::of(&vec_init_then_push::VEC_INIT_THEN_PUSH),
19461951
]);
19471952

19481953
store.register_group(true, "clippy::cargo", Some("clippy_cargo"), vec![

‎clippy_lints/src/vec_init_then_push.rs

Lines changed: 187 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,187 @@
1+
use crate::utils::{is_type_diagnostic_item, match_def_path, paths, snippet, span_lint_and_sugg};
2+
use if_chain::if_chain;
3+
use rustc_ast::ast::LitKind;
4+
use rustc_errors::Applicability;
5+
use rustc_hir::{BindingAnnotation, Block, Expr, ExprKind, Local, PatKind, QPath, Stmt, StmtKind};
6+
use rustc_lint::{LateContext, LateLintPass, LintContext};
7+
use rustc_middle::lint::in_external_macro;
8+
use rustc_session::{declare_tool_lint, impl_lint_pass};
9+
use rustc_span::{symbol::sym, Span, Symbol};
10+
use std::convert::TryInto;
11+
12+
declare_clippy_lint! {
13+
/// **What it does:** Checks for calls to `push` immediately after creating a new `Vec`.
14+
///
15+
/// **Why is this bad?** The `vec![]` macro is both more performant and easier to read than
16+
/// multiple `push` calls.
17+
///
18+
/// **Known problems:** None.
19+
///
20+
/// **Example:**
21+
///
22+
/// ```rust
23+
/// let mut v = Vec::new();
24+
/// v.push(0);
25+
/// ```
26+
/// Use instead:
27+
/// ```rust
28+
/// let v = vec![0];
29+
/// ```
30+
pub VEC_INIT_THEN_PUSH,
31+
perf,
32+
"`push` immediately after `Vec` creation"
33+
}
34+
35+
impl_lint_pass!(VecInitThenPush => [VEC_INIT_THEN_PUSH]);
36+
37+
#[derive(Default)]
38+
pub struct VecInitThenPush {
39+
searcher: Option<VecPushSearcher>,
40+
}
41+
42+
#[derive(Clone, Copy)]
43+
enum VecInitKind {
44+
New,
45+
WithCapacity(u64),
46+
}
47+
struct VecPushSearcher {
48+
init: VecInitKind,
49+
name: Symbol,
50+
lhs_is_local: bool,
51+
lhs_span: Span,
52+
err_span: Span,
53+
found: u64,
54+
}
55+
impl VecPushSearcher {
56+
fn display_err(&self, cx: &LateContext<'_>) {
57+
match self.init {
58+
_ if self.found == 0 => return,
59+
VecInitKind::WithCapacity(x) if x > self.found => return,
60+
_ => (),
61+
};
62+
63+
let mut s = if self.lhs_is_local {
64+
String::from("let ")
65+
} else {
66+
String::new()
67+
};
68+
s.push_str(&snippet(cx, self.lhs_span, ".."));
69+
s.push_str(" = vec![..];");
70+
71+
span_lint_and_sugg(
72+
cx,
73+
VEC_INIT_THEN_PUSH,
74+
self.err_span,
75+
"calls to `push` immediately after creation",
76+
"consider using the `vec![]` macro",
77+
s,
78+
Applicability::HasPlaceholders,
79+
);
80+
}
81+
}
82+
83+
impl LateLintPass<'_> for VecInitThenPush {
84+
fn check_local(&mut self, cx: &LateContext<'tcx>, local: &'tcx Local<'tcx>) {
85+
self.searcher = None;
86+
if_chain! {
87+
if !in_external_macro(cx.sess(), local.span);
88+
if let Some(init) = local.init;
89+
if let PatKind::Binding(BindingAnnotation::Mutable, _, ident, None) = local.pat.kind;
90+
if let Some(init_kind) = get_vec_init_kind(cx, init);
91+
then {
92+
self.searcher = Some(VecPushSearcher {
93+
init: init_kind,
94+
name: ident.name,
95+
lhs_is_local: true,
96+
lhs_span: local.ty.map_or(local.pat.span, |t| local.pat.span.to(t.span)),
97+
err_span: local.span,
98+
found: 0,
99+
});
100+
}
101+
}
102+
}
103+
104+
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
105+
if self.searcher.is_none() {
106+
if_chain! {
107+
if !in_external_macro(cx.sess(), expr.span);
108+
if let ExprKind::Assign(left, right, _) = expr.kind;
109+
if let ExprKind::Path(QPath::Resolved(_, path)) = left.kind;
110+
if let Some(name) = path.segments.get(0);
111+
if let Some(init_kind) = get_vec_init_kind(cx, right);
112+
then {
113+
self.searcher = Some(VecPushSearcher {
114+
init: init_kind,
115+
name: name.ident.name,
116+
lhs_is_local: false,
117+
lhs_span: left.span,
118+
err_span: expr.span,
119+
found: 0,
120+
});
121+
}
122+
}
123+
}
124+
}
125+
126+
fn check_stmt(&mut self, cx: &LateContext<'tcx>, stmt: &'tcx Stmt<'_>) {
127+
if let Some(searcher) = self.searcher.take() {
128+
if_chain! {
129+
if let StmtKind::Expr(expr) | StmtKind::Semi(expr) = stmt.kind;
130+
if let ExprKind::MethodCall(path, _, [self_arg, _], _) = expr.kind;
131+
if path.ident.name.as_str() == "push";
132+
if let ExprKind::Path(QPath::Resolved(_, self_path)) = self_arg.kind;
133+
if let [self_name] = self_path.segments;
134+
if self_name.ident.name == searcher.name;
135+
then {
136+
self.searcher = Some(VecPushSearcher {
137+
found: searcher.found + 1,
138+
err_span: searcher.err_span.to(stmt.span),
139+
.. searcher
140+
});
141+
} else {
142+
searcher.display_err(cx);
143+
}
144+
}
145+
}
146+
}
147+
148+
fn check_block_post(&mut self, cx: &LateContext<'tcx>, _: &'tcx Block<'tcx>) {
149+
if let Some(searcher) = self.searcher.take() {
150+
searcher.display_err(cx);
151+
}
152+
}
153+
}
154+
155+
fn get_vec_init_kind<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> Option<VecInitKind> {
156+
if let ExprKind::Call(func, args) = expr.kind {
157+
match func.kind {
158+
ExprKind::Path(QPath::TypeRelative(ty, name))
159+
if is_type_diagnostic_item(cx, cx.typeck_results().node_type(ty.hir_id), sym::vec_type) =>
160+
{
161+
if name.ident.name.as_str() == "new" {
162+
return Some(VecInitKind::New);
163+
} else if name.ident.name.as_str() == "with_capacity" {
164+
return args.get(0).and_then(|arg| {
165+
if_chain! {
166+
if let ExprKind::Lit(lit) = &arg.kind;
167+
if let LitKind::Int(num, _) = lit.node;
168+
then {
169+
Some(VecInitKind::WithCapacity(num.try_into().ok()?))
170+
} else {
171+
None
172+
}
173+
}
174+
});
175+
}
176+
}
177+
ExprKind::Path(QPath::Resolved(_, path))
178+
if match_def_path(cx, path.res.opt_def_id()?, &paths::DEFAULT_TRAIT_METHOD)
179+
&& is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(expr), sym::vec_type) =>
180+
{
181+
return Some(VecInitKind::New);
182+
}
183+
_ => (),
184+
}
185+
}
186+
None
187+
}

‎tests/ui/clone_on_copy.fixed

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,8 @@
55
clippy::redundant_clone,
66
clippy::deref_addrof,
77
clippy::no_effect,
8-
clippy::unnecessary_operation
8+
clippy::unnecessary_operation,
9+
clippy::vec_init_then_push
910
)]
1011

1112
use std::cell::RefCell;

‎tests/ui/clone_on_copy.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,8 @@
55
clippy::redundant_clone,
66
clippy::deref_addrof,
77
clippy::no_effect,
8-
clippy::unnecessary_operation
8+
clippy::unnecessary_operation,
9+
clippy::vec_init_then_push
910
)]
1011

1112
use std::cell::RefCell;

‎tests/ui/clone_on_copy.stderr

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,31 +1,31 @@
11
error: using `clone` on type `i32` which implements the `Copy` trait
2-
--> $DIR/clone_on_copy.rs:22:5
2+
--> $DIR/clone_on_copy.rs:23:5
33
|
44
LL | 42.clone();
55
| ^^^^^^^^^^ help: try removing the `clone` call: `42`
66
|
77
= note: `-D clippy::clone-on-copy` implied by `-D warnings`
88

99
error: using `clone` on type `i32` which implements the `Copy` trait
10-
--> $DIR/clone_on_copy.rs:26:5
10+
--> $DIR/clone_on_copy.rs:27:5
1111
|
1212
LL | (&42).clone();
1313
| ^^^^^^^^^^^^^ help: try dereferencing it: `*(&42)`
1414

1515
error: using `clone` on type `i32` which implements the `Copy` trait
16-
--> $DIR/clone_on_copy.rs:29:5
16+
--> $DIR/clone_on_copy.rs:30:5
1717
|
1818
LL | rc.borrow().clone();
1919
| ^^^^^^^^^^^^^^^^^^^ help: try dereferencing it: `*rc.borrow()`
2020

2121
error: using `clone` on type `char` which implements the `Copy` trait
22-
--> $DIR/clone_on_copy.rs:35:14
22+
--> $DIR/clone_on_copy.rs:36:14
2323
|
2424
LL | is_ascii('z'.clone());
2525
| ^^^^^^^^^^^ help: try removing the `clone` call: `'z'`
2626

2727
error: using `clone` on type `i32` which implements the `Copy` trait
28-
--> $DIR/clone_on_copy.rs:39:14
28+
--> $DIR/clone_on_copy.rs:40:14
2929
|
3030
LL | vec.push(42.clone());
3131
| ^^^^^^^^^^ help: try removing the `clone` call: `42`

‎tests/ui/vec_init_then_push.rs

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
#![allow(unused_variables)]
2+
#![warn(clippy::vec_init_then_push)]
3+
4+
fn main() {
5+
let mut def_err: Vec<u32> = Default::default();
6+
def_err.push(0);
7+
8+
let mut new_err = Vec::<u32>::new();
9+
new_err.push(1);
10+
11+
let mut cap_err = Vec::with_capacity(2);
12+
cap_err.push(0);
13+
cap_err.push(1);
14+
cap_err.push(2);
15+
16+
let mut cap_ok = Vec::with_capacity(10);
17+
cap_ok.push(0);
18+
19+
new_err = Vec::new();
20+
new_err.push(0);
21+
}

‎tests/ui/vec_init_then_push.stderr

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
error: calls to `push` immediately after creation
2+
--> $DIR/vec_init_then_push.rs:5:5
3+
|
4+
LL | / let mut def_err: Vec<u32> = Default::default();
5+
LL | | def_err.push(0);
6+
| |____________________^ help: consider using the `vec![]` macro: `let mut def_err: Vec<u32> = vec![..];`
7+
|
8+
= note: `-D clippy::vec-init-then-push` implied by `-D warnings`
9+
10+
error: calls to `push` immediately after creation
11+
--> $DIR/vec_init_then_push.rs:8:5
12+
|
13+
LL | / let mut new_err = Vec::<u32>::new();
14+
LL | | new_err.push(1);
15+
| |____________________^ help: consider using the `vec![]` macro: `let mut new_err = vec![..];`
16+
17+
error: calls to `push` immediately after creation
18+
--> $DIR/vec_init_then_push.rs:11:5
19+
|
20+
LL | / let mut cap_err = Vec::with_capacity(2);
21+
LL | | cap_err.push(0);
22+
LL | | cap_err.push(1);
23+
LL | | cap_err.push(2);
24+
| |____________________^ help: consider using the `vec![]` macro: `let mut cap_err = vec![..];`
25+
26+
error: calls to `push` immediately after creation
27+
--> $DIR/vec_init_then_push.rs:19:5
28+
|
29+
LL | / new_err = Vec::new();
30+
LL | | new_err.push(0);
31+
| |____________________^ help: consider using the `vec![]` macro: `new_err = vec![..];`
32+
33+
error: aborting due to 4 previous errors
34+

0 commit comments

Comments
 (0)