Skip to content

Commit

Permalink
Fix FP in indirect needless_collect when used multiple times
Browse files Browse the repository at this point in the history
  • Loading branch information
giraffate committed Nov 9, 2020
1 parent d212c38 commit 50f3551
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 1 deletion.
34 changes: 33 additions & 1 deletion clippy_lints/src/loops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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(
Expand Down Expand Up @@ -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 occurences 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>> {
Expand Down
20 changes: 20 additions & 0 deletions tests/ui/needless_collect_indirect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<_>>();
}
}

0 comments on commit 50f3551

Please sign in to comment.