Auto merge of #6313 - giraffate:fix_fp_needless_collect, r=ebroto

Fix FP in indirect `needless_collect` when used multiple times

Fix #5991
Fix #6297

changelog: Fix FP in indirect `needless_collect` when used multiple times

Author: bors

clippy_lints/src/loops.rs

@@ -2950,7 +2950,7 @@ fn check_needless_collect_indirect_usage<'tcx>(expr: &'tcx Expr<'_>, cx: &LateCo
         for ref stmt in block.stmts {
             if_chain! {
                 if let StmtKind::Local(
-                    Local { pat: Pat { kind: PatKind::Binding(_, _, ident, .. ), .. },
+                    Local { pat: Pat { hir_id: pat_id, kind: PatKind::Binding(_, _, ident, .. ), .. },
                     init: Some(ref init_expr), .. }
                 ) = stmt.kind;
                 if let ExprKind::MethodCall(ref method_name, _, &[ref iter_source], ..) = init_expr.kind;
@@ -2964,6 +2964,16 @@ fn check_needless_collect_indirect_usage<'tcx>(expr: &'tcx Expr<'_>, cx: &LateCo
                 if let Some(iter_calls) = detect_iter_and_into_iters(block, *ident);
                 if iter_calls.len() == 1;
                 then {
+                    let mut used_count_visitor = UsedCountVisitor {
+                        cx,
+                        id: *pat_id,
+                        count: 0,
+                    };
+                    walk_block(&mut used_count_visitor, block);
+                    if used_count_visitor.count > 1 {
+                        return;
+                    }
+
                     // Suggest replacing iter_call with iter_replacement, and removing stmt
                     let iter_call = &iter_calls[0];
                     span_lint_and_then(
@@ -3087,6 +3097,28 @@ impl<'tcx> Visitor<'tcx> for IterFunctionVisitor {
     }
 }
 
+struct UsedCountVisitor<'a, 'tcx> {
+    cx: &'a LateContext<'tcx>,
+    id: HirId,
+    count: usize,
+}
+
+impl<'a, 'tcx> Visitor<'tcx> for UsedCountVisitor<'a, 'tcx> {
+    type Map = Map<'tcx>;
+
+    fn visit_expr(&mut self, expr: &'tcx Expr<'_>) {
+        if same_var(self.cx, expr, self.id) {
+            self.count += 1;
+        } else {
+            walk_expr(self, expr);
+        }
+    }
+
+    fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
+        NestedVisitorMap::OnlyBodies(self.cx.tcx.hir())
+    }
+}
+
 /// Detect the occurrences of calls to `iter` or `into_iter` for the
 /// given identifier
 fn detect_iter_and_into_iters<'tcx>(block: &'tcx Block<'tcx>, identifier: Ident) -> Option<Vec<IterFunction>> {

tests/ui/needless_collect_indirect.rs

@@ -22,4 +22,24 @@ fn main() {
     let sample = vec![a.clone(), "b".to_string(), "c".to_string()];
     let non_copy_contains = sample.into_iter().collect::<Vec<_>>();
     non_copy_contains.contains(&a);
+
+    // Fix #5991
+    let vec_a = vec![1, 2, 3, 4, 5, 6, 7, 8, 9, 10];
+    let vec_b = vec_a.iter().collect::<Vec<_>>();
+    if vec_b.len() > 3 {}
+    let other_vec = vec![1, 3, 12, 4, 16, 2];
+    let we_got_the_same_numbers = other_vec.iter().filter(|item| vec_b.contains(item)).collect::<Vec<_>>();
+
+    // Fix #6297
+    let sample = [1; 5];
+    let multiple_indirect = sample.iter().collect::<Vec<_>>();
+    let sample2 = vec![2, 3];
+    if multiple_indirect.is_empty() {
+        // do something
+    } else {
+        let found = sample2
+            .iter()
+            .filter(|i| multiple_indirect.iter().any(|s| **s % **i == 0))
+            .collect::<Vec<_>>();
+    }
 }