fix: needless_collect does not consider side effects

Author: profetia

clippy_lints/src/methods/needless_collect.rs

@@ -4,7 +4,9 @@ use super::NEEDLESS_COLLECT;
 use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_hir_and_then};
 use clippy_utils::source::{snippet, snippet_with_applicability};
 use clippy_utils::sugg::Sugg;
-use clippy_utils::ty::{get_type_diagnostic_name, make_normalized_projection, make_projection};
+use clippy_utils::ty::{
+    get_type_diagnostic_name, is_iter_with_side_effects, make_normalized_projection, make_projection,
+};
 use clippy_utils::{
     CaptureKind, can_move_expr_to_closure, fn_def_id, get_enclosing_block, higher, is_trait_method, path_to_local,
     path_to_local_id,
@@ -23,13 +25,19 @@ use rustc_span::{Span, sym};
 
 const NEEDLESS_COLLECT_MSG: &str = "avoid using `collect()` when not needed";
 
+#[expect(clippy::too_many_lines)]
 pub(super) fn check<'tcx>(
     cx: &LateContext<'tcx>,
     name_span: Span,
     collect_expr: &'tcx Expr<'_>,
     iter_expr: &'tcx Expr<'tcx>,
     call_span: Span,
 ) {
+    let iter_ty = cx.typeck_results().expr_ty(iter_expr);
+    if is_iter_with_side_effects(cx, iter_ty) {
+        return; // don't lint if the iterator has side effects
+    }
+
     match cx.tcx.parent_hir_node(collect_expr.hir_id) {
         Node::Expr(parent) => {
             check_collect_into_intoiterator(cx, parent, collect_expr, call_span, iter_expr);

tests/ui/needless_collect.fixed

@@ -126,3 +126,32 @@ fn main() {
 fn foo(_: impl IntoIterator<Item = usize>) {}
 fn bar<I: IntoIterator<Item = usize>>(_: Vec<usize>, _: I) {}
 fn baz<I: IntoIterator<Item = usize>>(_: I, _: (), _: impl IntoIterator<Item = char>) {}
+
+mod issue9191 {
+    use std::collections::HashSet;
+
+    fn foo(xs: Vec<i32>, mut ys: HashSet<i32>) {
+        if xs.iter().map(|x| ys.remove(x)).collect::<Vec<_>>().contains(&true) {
+            todo!()
+        }
+    }
+}
+
+pub fn issue8055(v: impl IntoIterator<Item = i32>) -> Result<impl Iterator<Item = i32>, usize> {
+    let mut zeros = 0;
+
+    let res: Vec<_> = v
+        .into_iter()
+        .filter(|i| {
+            if *i == 0 {
+                zeros += 1
+            };
+            *i != 0
+        })
+        .collect();
+
+    if zeros != 0 {
+        return Err(zeros);
+    }
+    Ok(res.into_iter())
+}

tests/ui/needless_collect.rs

@@ -126,3 +126,32 @@ fn main() {
 fn foo(_: impl IntoIterator<Item = usize>) {}
 fn bar<I: IntoIterator<Item = usize>>(_: Vec<usize>, _: I) {}
 fn baz<I: IntoIterator<Item = usize>>(_: I, _: (), _: impl IntoIterator<Item = char>) {}
+
+mod issue9191 {
+    use std::collections::HashSet;
+
+    fn foo(xs: Vec<i32>, mut ys: HashSet<i32>) {
+        if xs.iter().map(|x| ys.remove(x)).collect::<Vec<_>>().contains(&true) {
+            todo!()
+        }
+    }
+}
+
+pub fn issue8055(v: impl IntoIterator<Item = i32>) -> Result<impl Iterator<Item = i32>, usize> {
+    let mut zeros = 0;
+
+    let res: Vec<_> = v
+        .into_iter()
+        .filter(|i| {
+            if *i == 0 {
+                zeros += 1
+            };
+            *i != 0
+        })
+        .collect();
+
+    if zeros != 0 {
+        return Err(zeros);
+    }
+    Ok(res.into_iter())
+}