Skip to content

Commit 3d30ef7

Browse files
giraffategiraffate
committed
Restrict same_item_push to suppress false positives
It emits a lint when the pushed item is a literal, a constant and an immutable binding that are initialized with those.
1 parent daad592 commit 3d30ef7

File tree

3 files changed

+97
-33
lines changed

3 files changed

+97
-33
lines changed

‎clippy_lints/src/loops.rs

Lines changed: 63 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1153,27 +1153,70 @@ fn detect_same_item_push<'tcx>(
11531153
let vec_str = snippet_with_macro_callsite(cx, vec.span, "");
11541154
let item_str = snippet_with_macro_callsite(cx, pushed_item.span, "");
11551155
if let ExprKind::Path(ref qpath) = pushed_item.kind {
1156-
if_chain! {
1157-
if let Res::Local(hir_id) = qpath_res(cx, qpath, pushed_item.hir_id);
1158-
let node = cx.tcx.hir().get(hir_id);
1159-
if let Node::Binding(pat) = node;
1160-
if let PatKind::Binding(bind_ann, ..) = pat.kind;
1161-
if !matches!(bind_ann, BindingAnnotation::RefMut | BindingAnnotation::Mutable);
1162-
then {
1163-
span_lint_and_help(
1164-
cx,
1165-
SAME_ITEM_PUSH,
1166-
vec.span,
1167-
"it looks like the same item is being pushed into this Vec",
1168-
None,
1169-
&format!(
1170-
"try using vec![{};SIZE] or {}.resize(NEW_SIZE, {})",
1171-
item_str, vec_str, item_str
1172-
),
1173-
)
1174-
}
1156+
match qpath_res(cx, qpath, pushed_item.hir_id) {
1157+
// immutable bindings that are initialized with literal or constant
1158+
Res::Local(hir_id) => {
1159+
if_chain! {
1160+
let node = cx.tcx.hir().get(hir_id);
1161+
if let Node::Binding(pat) = node;
1162+
if let PatKind::Binding(bind_ann, ..) = pat.kind;
1163+
if !matches!(bind_ann, BindingAnnotation::RefMut | BindingAnnotation::Mutable);
1164+
let parent_node = cx.tcx.hir().get_parent_node(hir_id);
1165+
if let Some(Node::Local(parent_let_expr)) = cx.tcx.hir().find(parent_node);
1166+
if let rustc_hir::Local { init: Some(init), .. } = parent_let_expr;
1167+
then {
1168+
match init.kind {
1169+
// immutable bindings that are initialized with literal
1170+
ExprKind::Lit(..) => {
1171+
span_lint_and_help(
1172+
cx,
1173+
SAME_ITEM_PUSH,
1174+
vec.span,
1175+
"it looks like the same item is being pushed into this Vec",
1176+
None,
1177+
&format!(
1178+
"try using vec![{};SIZE] or {}.resize(NEW_SIZE, {})",
1179+
item_str, vec_str, item_str
1180+
),
1181+
)
1182+
},
1183+
// immutable bindings that are initialized with constant
1184+
ExprKind::Path(ref path) => {
1185+
if let Res::Def(DefKind::Const, ..) = qpath_res(cx, path, init.hir_id) {
1186+
span_lint_and_help(
1187+
cx,
1188+
SAME_ITEM_PUSH,
1189+
vec.span,
1190+
"it looks like the same item is being pushed into this Vec",
1191+
None,
1192+
&format!(
1193+
"try using vec![{};SIZE] or {}.resize(NEW_SIZE, {})",
1194+
item_str, vec_str, item_str
1195+
),
1196+
)
1197+
}
1198+
}
1199+
_ => {},
1200+
}
1201+
}
1202+
}
1203+
},
1204+
// constant
1205+
Res::Def(DefKind::Const, ..) => span_lint_and_help(
1206+
cx,
1207+
SAME_ITEM_PUSH,
1208+
vec.span,
1209+
"it looks like the same item is being pushed into this Vec",
1210+
None,
1211+
&format!(
1212+
"try using vec![{};SIZE] or {}.resize(NEW_SIZE, {})",
1213+
item_str, vec_str, item_str
1214+
),
1215+
),
1216+
_ => {},
11751217
}
1176-
} else if mutated_variables(pushed_item, cx).map_or(false, |mutvars| mutvars.is_empty()) {
1218+
} else if let ExprKind::Lit(..) = pushed_item.kind {
1219+
// literal
11771220
span_lint_and_help(
11781221
cx,
11791222
SAME_ITEM_PUSH,

‎tests/ui/same_item_push.rs

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
#![warn(clippy::same_item_push)]
22

3+
const VALUE: u8 = 7;
4+
35
fn mutate_increment(x: &mut u8) -> u8 {
46
*x += 1;
57
*x
@@ -111,4 +113,15 @@ fn main() {
111113
for _ in 0..10 {
112114
vec15.push(Box::new(S {}));
113115
}
116+
117+
let mut vec16 = Vec::new();
118+
for _ in 0..20 {
119+
vec16.push(VALUE);
120+
}
121+
122+
let mut vec17 = Vec::new();
123+
let item = VALUE;
124+
for _ in 0..20 {
125+
vec17.push(item);
126+
}
114127
}

‎tests/ui/same_item_push.stderr

Lines changed: 21 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,35 +1,43 @@
11
error: it looks like the same item is being pushed into this Vec
2-
--> $DIR/same_item_push.rs:16:9
3-
|
4-
LL | spaces.push(vec![b' ']);
5-
| ^^^^^^
6-
|
7-
= note: `-D clippy::same-item-push` implied by `-D warnings`
8-
= help: try using vec![vec![b' '];SIZE] or spaces.resize(NEW_SIZE, vec![b' '])
9-
10-
error: it looks like the same item is being pushed into this Vec
11-
--> $DIR/same_item_push.rs:22:9
2+
--> $DIR/same_item_push.rs:24:9
123
|
134
LL | vec2.push(item);
145
| ^^^^
156
|
7+
= note: `-D clippy::same-item-push` implied by `-D warnings`
168
= help: try using vec![item;SIZE] or vec2.resize(NEW_SIZE, item)
179

1810
error: it looks like the same item is being pushed into this Vec
19-
--> $DIR/same_item_push.rs:28:9
11+
--> $DIR/same_item_push.rs:30:9
2012
|
2113
LL | vec3.push(item);
2214
| ^^^^
2315
|
2416
= help: try using vec![item;SIZE] or vec3.resize(NEW_SIZE, item)
2517

2618
error: it looks like the same item is being pushed into this Vec
27-
--> $DIR/same_item_push.rs:33:9
19+
--> $DIR/same_item_push.rs:35:9
2820
|
2921
LL | vec4.push(13);
3022
| ^^^^
3123
|
3224
= help: try using vec![13;SIZE] or vec4.resize(NEW_SIZE, 13)
3325

34-
error: aborting due to 4 previous errors
26+
error: it looks like the same item is being pushed into this Vec
27+
--> $DIR/same_item_push.rs:119:9
28+
|
29+
LL | vec16.push(VALUE);
30+
| ^^^^^
31+
|
32+
= help: try using vec![VALUE;SIZE] or vec16.resize(NEW_SIZE, VALUE)
33+
34+
error: it looks like the same item is being pushed into this Vec
35+
--> $DIR/same_item_push.rs:125:9
36+
|
37+
LL | vec17.push(item);
38+
| ^^^^^
39+
|
40+
= help: try using vec![item;SIZE] or vec17.resize(NEW_SIZE, item)
41+
42+
error: aborting due to 5 previous errors
3543

0 commit comments

Comments
 (0)