Merge commit 'd28211ddb6bec659acd523ef63966a5032a618b7' into beta

Author: flip1995

src/tools/clippy/clippy_lints/src/loops.rs

@@ -1131,6 +1131,27 @@ fn detect_same_item_push<'tcx>(
     body: &'tcx Expr<'_>,
     _: &'tcx Expr<'_>,
 ) {
+    fn emit_lint(cx: &LateContext<'_>, vec: &Expr<'_>, pushed_item: &Expr<'_>) {
+        let vec_str = snippet_with_macro_callsite(cx, vec.span, "");
+        let item_str = snippet_with_macro_callsite(cx, pushed_item.span, "");
+
+        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 !matches!(pat.kind, PatKind::Wild) {
+        return;
+    }
+
     // Determine whether it is safe to lint the body
     let mut same_item_push_visitor = SameItemPushVisitor {
         should_lint: true,
@@ -1140,23 +1161,50 @@ 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 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, "");
-
-                    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
+                match pushed_item.kind {
+                    ExprKind::Path(ref qpath) => {
+                        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 Some(init) = parent_let_expr.init;
+                                    then {
+                                        match init.kind {
+                                            // immutable bindings that are initialized with literal
+                                            ExprKind::Lit(..) => emit_lint(cx, vec, pushed_item),
+                                            // immutable bindings that are initialized with constant
+                                            ExprKind::Path(ref path) => {
+                                                if let Res::Def(DefKind::Const, ..) = qpath_res(cx, path, init.hir_id) {
+                                                    emit_lint(cx, vec, pushed_item);
+                                                }
+                                            }
+                                            _ => {},
+                                        }
+                                    }
+                                }
+                            },
+                            // constant
+                            Res::Def(DefKind::Const, ..) => emit_lint(cx, vec, pushed_item),
+                            _ => {},
+                        }
+                    },
+                    ExprKind::Lit(..) => emit_lint(cx, vec, pushed_item),
+                    _ => {},
                 }
             }
         }

src/tools/clippy/tests/ui/same_item_push.rs

@@ -1,5 +1,7 @@
 #![warn(clippy::same_item_push)]
 
+const VALUE: u8 = 7;
+
 fn mutate_increment(x: &mut u8) -> u8 {
     *x += 1;
     *x
@@ -9,65 +11,81 @@ fn increment(x: u8) -> u8 {
     x + 1
 }
 
-fn main() {
-    // Test for basic case
-    let mut spaces = Vec::with_capacity(10);
-    for _ in 0..10 {
-        spaces.push(vec![b' ']);
-    }
+fn fun() -> usize {
+    42
+}
 
-    let mut vec2: Vec<u8> = Vec::new();
+fn main() {
+    // ** linted cases **
+    let mut vec: Vec<u8> = Vec::new();
     let item = 2;
     for _ in 5..=20 {
-        vec2.push(item);
+        vec.push(item);
     }
 
-    let mut vec3: Vec<u8> = Vec::new();
+    let mut vec: Vec<u8> = Vec::new();
     for _ in 0..15 {
         let item = 2;
-        vec3.push(item);
+        vec.push(item);
     }
 
-    let mut vec4: Vec<u8> = Vec::new();
+    let mut vec: Vec<u8> = Vec::new();
     for _ in 0..15 {
-        vec4.push(13);
+        vec.push(13);
+    }
+
+    let mut vec = Vec::new();
+    for _ in 0..20 {
+        vec.push(VALUE);
+    }
+
+    let mut vec = Vec::new();
+    let item = VALUE;
+    for _ in 0..20 {
+        vec.push(item);
+    }
+
+    // ** non-linted cases **
+    let mut spaces = Vec::with_capacity(10);
+    for _ in 0..10 {
+        spaces.push(vec![b' ']);
     }
 
     // Suggestion should not be given as pushed variable can mutate
-    let mut vec5: Vec<u8> = Vec::new();
+    let mut vec: Vec<u8> = Vec::new();
     let mut item: u8 = 2;
     for _ in 0..30 {
-        vec5.push(mutate_increment(&mut item));
+        vec.push(mutate_increment(&mut item));
     }
 
-    let mut vec6: Vec<u8> = Vec::new();
+    let mut vec: Vec<u8> = Vec::new();
     let mut item: u8 = 2;
     let mut item2 = &mut mutate_increment(&mut item);
     for _ in 0..30 {
-        vec6.push(mutate_increment(item2));
+        vec.push(mutate_increment(item2));
     }
 
-    let mut vec7: Vec<usize> = Vec::new();
+    let mut vec: Vec<usize> = Vec::new();
     for (a, b) in [0, 1, 4, 9, 16].iter().enumerate() {
-        vec7.push(a);
+        vec.push(a);
     }
 
-    let mut vec8: Vec<u8> = Vec::new();
+    let mut vec: Vec<u8> = Vec::new();
     for i in 0..30 {
-        vec8.push(increment(i));
+        vec.push(increment(i));
     }
 
-    let mut vec9: Vec<u8> = Vec::new();
+    let mut vec: Vec<u8> = Vec::new();
     for i in 0..30 {
-        vec9.push(i + i * i);
+        vec.push(i + i * i);
     }
 
     // Suggestion should not be given as there are multiple pushes that are not the same
-    let mut vec10: Vec<u8> = Vec::new();
+    let mut vec: Vec<u8> = Vec::new();
     let item: u8 = 2;
     for _ in 0..30 {
-        vec10.push(item);
-        vec10.push(item * 2);
+        vec.push(item);
+        vec.push(item * 2);
     }
 
     // Suggestion should not be given as Vec is not involved
@@ -82,8 +100,52 @@ fn main() {
     for i in 0..30 {
         vec_a.push(A { kind: i });
     }
-    let mut vec12: Vec<u8> = Vec::new();
+    let mut vec: Vec<u8> = Vec::new();
     for a in vec_a {
-        vec12.push(2u8.pow(a.kind));
+        vec.push(2u8.pow(a.kind));
+    }
+
+    // Fix #5902
+    let mut vec: Vec<u8> = Vec::new();
+    let mut item = 0;
+    for _ in 0..10 {
+        vec.push(item);
+        item += 10;
+    }
+
+    // Fix #5979
+    let mut vec: Vec<std::fs::File> = Vec::new();
+    for _ in 0..10 {
+        vec.push(std::fs::File::open("foobar").unwrap());
+    }
+    // Fix #5979
+    #[derive(Clone)]
+    struct S {}
+
+    trait T {}
+    impl T for S {}
+
+    let mut vec: Vec<Box<dyn T>> = Vec::new();
+    for _ in 0..10 {
+        vec.push(Box::new(S {}));
+    }
+
+    // Fix #5985
+    let mut vec = Vec::new();
+    let item = 42;
+    let item = fun();
+    for _ in 0..20 {
+        vec.push(item);
+    }
+
+    // Fix #5985
+    let mut vec = Vec::new();
+    let key = 1;
+    for _ in 0..20 {
+        let item = match key {
+            1 => 10,
+            _ => 0,
+        };
+        vec.push(item);
     }
 }

src/tools/clippy/tests/ui/same_item_push.stderr

@@ -1,35 +1,43 @@
 error: it looks like the same item is being pushed into this Vec
-  --> $DIR/same_item_push.rs:16:9
+  --> $DIR/same_item_push.rs:23:9
    |
-LL |         spaces.push(vec![b' ']);
-   |         ^^^^^^
+LL |         vec.push(item);
+   |         ^^^
    |
    = note: `-D clippy::same-item-push` implied by `-D warnings`
-   = help: try using vec![vec![b' '];SIZE] or spaces.resize(NEW_SIZE, vec![b' '])
+   = help: try using vec![item;SIZE] or vec.resize(NEW_SIZE, item)
 
 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:29:9
    |
-LL |         vec2.push(item);
-   |         ^^^^
+LL |         vec.push(item);
+   |         ^^^
    |
-   = help: try using vec![item;SIZE] or vec2.resize(NEW_SIZE, item)
+   = help: try using vec![item;SIZE] or vec.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:34:9
    |
-LL |         vec3.push(item);
-   |         ^^^^
+LL |         vec.push(13);
+   |         ^^^
    |
-   = help: try using vec![item;SIZE] or vec3.resize(NEW_SIZE, item)
+   = help: try using vec![13;SIZE] or vec.resize(NEW_SIZE, 13)
 
 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:39:9
    |
-LL |         vec4.push(13);
-   |         ^^^^
+LL |         vec.push(VALUE);
+   |         ^^^
    |
-   = help: try using vec![13;SIZE] or vec4.resize(NEW_SIZE, 13)
+   = help: try using vec![VALUE;SIZE] or vec.resize(NEW_SIZE, VALUE)
 
-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:45:9
+   |
+LL |         vec.push(item);
+   |         ^^^
+   |
+   = help: try using vec![item;SIZE] or vec.resize(NEW_SIZE, item)
+
+error: aborting due to 5 previous errors