From f98ffa271d0112d04b482e1d61228d99bf006ccf Mon Sep 17 00:00:00 2001 From: Takayuki Nakata Date: Fri, 14 Aug 2020 22:54:12 +0900 Subject: [PATCH 1/2] Fix FP for `same_item_push` Don't emit a lint when `pushed_item` was declared as mutable variable. --- clippy_lints/src/loops.rs | 35 ++++++++++++++++++++++++++++++----- tests/ui/same_item_push.rs | 8 ++++++++ 2 files changed, 38 insertions(+), 5 deletions(-) diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs index 8352a8a3d2c6..f7db2563d2b1 100644 --- a/clippy_lints/src/loops.rs +++ b/clippy_lints/src/loops.rs @@ -1141,11 +1141,36 @@ fn detect_same_item_push<'tcx>( if same_item_push_visitor.should_lint { if let Some((vec, pushed_item)) = same_item_push_visitor.vec_push { // Make sure that the push does not involve possibly mutating values - if mutated_variables(pushed_item, cx).map_or(false, |mutvars| mutvars.is_empty()) { - if let PatKind::Wild = pat.kind { - let vec_str = snippet_with_macro_callsite(cx, vec.span, ""); - let item_str = snippet_with_macro_callsite(cx, pushed_item.span, ""); - + if let PatKind::Wild = pat.kind { + 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 { + if let Res::Local(hir_id) = qpath_res(cx, qpath, pushed_item.hir_id) { + let node = cx.tcx.hir().get(hir_id); + if_chain! { + if let Node::Binding(pat) = node; + if let PatKind::Binding(bind_ann, ..) = pat.kind; + then { + match bind_ann { + BindingAnnotation::RefMut | BindingAnnotation::Mutable => {}, + _ => { + 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()) { span_lint_and_help( cx, SAME_ITEM_PUSH, diff --git a/tests/ui/same_item_push.rs b/tests/ui/same_item_push.rs index ff1088f86f64..bfe27e020445 100644 --- a/tests/ui/same_item_push.rs +++ b/tests/ui/same_item_push.rs @@ -86,4 +86,12 @@ fn main() { for a in vec_a { vec12.push(2u8.pow(a.kind)); } + + // Fix #5902 + let mut vec13: Vec = Vec::new(); + let mut item = 0; + for _ in 0..10 { + vec13.push(item); + item += 10; + } } From 99ba290a14c6654164cea8339b827d8da0ff600d Mon Sep 17 00:00:00 2001 From: Takayuki Nakata Date: Mon, 17 Aug 2020 08:36:02 +0900 Subject: [PATCH 2/2] Improve code style --- clippy_lints/src/loops.rs | 39 +++++++++++++++++---------------------- 1 file changed, 17 insertions(+), 22 deletions(-) diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs index f7db2563d2b1..df06031d999e 100644 --- a/clippy_lints/src/loops.rs +++ b/clippy_lints/src/loops.rs @@ -1145,29 +1145,24 @@ 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 { - if let Res::Local(hir_id) = qpath_res(cx, qpath, pushed_item.hir_id) { + if_chain! { + if let Res::Local(hir_id) = qpath_res(cx, qpath, pushed_item.hir_id); let node = cx.tcx.hir().get(hir_id); - if_chain! { - if let Node::Binding(pat) = node; - if let PatKind::Binding(bind_ann, ..) = pat.kind; - then { - match bind_ann { - BindingAnnotation::RefMut | BindingAnnotation::Mutable => {}, - _ => { - 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 - ), - ) - } - } - } + 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 + ), + ) } } } else if mutated_variables(pushed_item, cx).map_or(false, |mutvars| mutvars.is_empty()) {