From 8f89108533690afe964799d8f514956ec8b72377 Mon Sep 17 00:00:00 2001 From: Takayuki Nakata Date: Mon, 9 Nov 2020 22:14:11 +0900 Subject: [PATCH] Fix FP in indirect `needless_collect` when used multiple times --- clippy_lints/src/loops.rs | 34 ++++++++++++++++++++++++++- tests/ui/needless_collect_indirect.rs | 20 ++++++++++++++++ 2 files changed, 53 insertions(+), 1 deletion(-) diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs index 0d31e9cfc..143cbea55 100644 --- a/clippy_lints/src/loops.rs +++ b/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 { + 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> { diff --git a/tests/ui/needless_collect_indirect.rs b/tests/ui/needless_collect_indirect.rs index 4f6e53577..0918a6868 100644 --- a/tests/ui/needless_collect_indirect.rs +++ b/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::>(); 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::>(); + 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::>(); + + // Fix #6297 + let sample = [1; 5]; + let multiple_indirect = sample.iter().collect::>(); + 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::>(); + } }