Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Restrict same_item_push to suppress false positives #6016

Merged
merged 6 commits into from
Sep 8, 2020
Merged

Restrict same_item_push to suppress false positives #6016

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
3d30ef7
Restrict `same_item_push` to suppress false positives
giraffate Sep 7, 2020
619ca76
Refactoring: use inner function
giraffate Sep 7, 2020
5d085ad
Some refactoring
giraffate Sep 7, 2020
2c9f82e
Add some tests to `same_item_push`
giraffate Sep 8, 2020
952c046
Refactoring: tests in `same_item_push`
giraffate Sep 8, 2020
1d8ae3a
Address `items_after_statement`
giraffate Sep 8, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
83 changes: 63 additions & 20 deletions clippy_lints/src/loops.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1153,27 +1153,70 @@ fn detect_same_item_push<'tcx>(
let vec_str = snippet_with_macro_callsite(cx, vec.span, "");
let item_str = snippet_with_macro_callsite(cx, pushed_item.span, "");
if let ExprKind::Path(ref qpath) = pushed_item.kind {
ebroto marked this conversation as resolved.
Show resolved Hide resolved
if_chain! {
ebroto marked this conversation as resolved.
Show resolved Hide resolved
if let Res::Local(hir_id) = qpath_res(cx, qpath, pushed_item.hir_id);
let node = cx.tcx.hir().get(hir_id);
if let Node::Binding(pat) = node;
if let PatKind::Binding(bind_ann, ..) = pat.kind;
if !matches!(bind_ann, BindingAnnotation::RefMut | BindingAnnotation::Mutable);
then {
span_lint_and_help(
cx,
SAME_ITEM_PUSH,
vec.span,
"it looks like the same item is being pushed into this Vec",
None,
&format!(
"try using vec![{};SIZE] or {}.resize(NEW_SIZE, {})",
item_str, vec_str, item_str
),
)
}
match qpath_res(cx, qpath, pushed_item.hir_id) {
// immutable bindings that are initialized with literal or constant
Res::Local(hir_id) => {
if_chain! {
let node = cx.tcx.hir().get(hir_id);
if let Node::Binding(pat) = node;
if let PatKind::Binding(bind_ann, ..) = pat.kind;
if !matches!(bind_ann, BindingAnnotation::RefMut | BindingAnnotation::Mutable);
let parent_node = cx.tcx.hir().get_parent_node(hir_id);
if let Some(Node::Local(parent_let_expr)) = cx.tcx.hir().find(parent_node);
if let rustc_hir::Local { init: Some(init), .. } = parent_let_expr;
ebroto marked this conversation as resolved.
Show resolved Hide resolved
then {
match init.kind {
// immutable bindings that are initialized with literal
ExprKind::Lit(..) => {
span_lint_and_help(
cx,
SAME_ITEM_PUSH,
vec.span,
"it looks like the same item is being pushed into this Vec",
None,
&format!(
"try using vec![{};SIZE] or {}.resize(NEW_SIZE, {})",
item_str, vec_str, item_str
),
)
ebroto marked this conversation as resolved.
Show resolved Hide resolved
},
// immutable bindings that are initialized with constant
ExprKind::Path(ref path) => {
if let Res::Def(DefKind::Const, ..) = qpath_res(cx, path, init.hir_id) {
span_lint_and_help(
cx,
SAME_ITEM_PUSH,
vec.span,
"it looks like the same item is being pushed into this Vec",
None,
&format!(
"try using vec![{};SIZE] or {}.resize(NEW_SIZE, {})",
item_str, vec_str, item_str
),
)
}
}
_ => {},
}
}
}
},
// constant
Res::Def(DefKind::Const, ..) => span_lint_and_help(
cx,
SAME_ITEM_PUSH,
vec.span,
"it looks like the same item is being pushed into this Vec",
None,
&format!(
"try using vec![{};SIZE] or {}.resize(NEW_SIZE, {})",
item_str, vec_str, item_str
),
),
_ => {},
}
} else if mutated_variables(pushed_item, cx).map_or(false, |mutvars| mutvars.is_empty()) {
} else if let ExprKind::Lit(..) = pushed_item.kind {
// literal
span_lint_and_help(
cx,
SAME_ITEM_PUSH,
Expand Down
13 changes: 13 additions & 0 deletions tests/ui/same_item_push.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
#![warn(clippy::same_item_push)]

const VALUE: u8 = 7;

fn mutate_increment(x: &mut u8) -> u8 {
*x += 1;
*x
Expand Down Expand Up @@ -111,4 +113,15 @@ fn main() {
for _ in 0..10 {
vec15.push(Box::new(S {}));
}

ebroto marked this conversation as resolved.
Show resolved Hide resolved
let mut vec16 = Vec::new();
for _ in 0..20 {
vec16.push(VALUE);
}

let mut vec17 = Vec::new();
let item = VALUE;
for _ in 0..20 {
vec17.push(item);
ebroto marked this conversation as resolved.
Show resolved Hide resolved
}
ebroto marked this conversation as resolved.
Show resolved Hide resolved
}
34 changes: 21 additions & 13 deletions tests/ui/same_item_push.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,35 +1,43 @@
error: it looks like the same item is being pushed into this Vec
--> $DIR/same_item_push.rs:16:9
|
LL | spaces.push(vec![b' ']);
| ^^^^^^
|
= note: `-D clippy::same-item-push` implied by `-D warnings`
= help: try using vec![vec![b' '];SIZE] or spaces.resize(NEW_SIZE, vec![b' '])

error: it looks like the same item is being pushed into this Vec
--> $DIR/same_item_push.rs:22:9
--> $DIR/same_item_push.rs:24:9
|
LL | vec2.push(item);
| ^^^^
|
= note: `-D clippy::same-item-push` implied by `-D warnings`
= help: try using vec![item;SIZE] or vec2.resize(NEW_SIZE, item)

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

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

error: aborting due to 4 previous errors
error: it looks like the same item is being pushed into this Vec
--> $DIR/same_item_push.rs:119:9
|
LL | vec16.push(VALUE);
| ^^^^^
|
= help: try using vec![VALUE;SIZE] or vec16.resize(NEW_SIZE, VALUE)

error: it looks like the same item is being pushed into this Vec
--> $DIR/same_item_push.rs:125:9
|
LL | vec17.push(item);
| ^^^^^
|
= help: try using vec![item;SIZE] or vec17.resize(NEW_SIZE, item)

error: aborting due to 5 previous errors