Auto merge of #5997 - giraffate:fix_fp_about_clone_in_same_item_push, r=ebroto

Fix FP in `same_item_push`

Don't emit a lint when the pushed item doesn't have Clone trait

Fix #5979

changelog: Fix FP in `same_item_push` not to emit a lint when the pushed item doesn't have Clone trait

Author: bors

clippy_lints/src/loops.rs

@@ -1140,43 +1140,52 @@ fn detect_same_item_push<'tcx>(
     walk_expr(&mut same_item_push_visitor, body);
     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 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_chain! {
-                        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
-                                ),
-                            )
+            let vec_ty = cx.typeck_results().expr_ty(vec);
+            let ty = vec_ty.walk().nth(1).unwrap().expect_ty();
+            if cx
+                .tcx
+                .lang_items()
+                .clone_trait()
+                .map_or(false, |id| implements_trait(cx, ty, id, &[]))
+            {
+                // Make sure that the push does not involve possibly mutating values
+                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_chain! {
+                            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
+                                    ),
+                                )
+                            }
                         }
+                    } else if mutated_variables(pushed_item, cx).map_or(false, |mutvars| mutvars.is_empty()) {
+                        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,
-                        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
-                        ),
-                    )
                 }
             }
         }

tests/ui/same_item_push.rs

@@ -94,4 +94,21 @@ fn main() {
         vec13.push(item);
         item += 10;
     }
+
+    // Fix #5979
+    let mut vec14: Vec<std::fs::File> = Vec::new();
+    for _ in 0..10 {
+        vec14.push(std::fs::File::open("foobar").unwrap());
+    }
+    // Fix #5979
+    #[derive(Clone)]
+    struct S {}
+
+    trait T {}
+    impl T for S {}
+
+    let mut vec15: Vec<Box<dyn T>> = Vec::new();
+    for _ in 0..10 {
+        vec15.push(Box::new(S {}));
+    }
 }