needless_collect: avoid warning if non-iterator methods are used (rust-lang#14147)

blyxyas · web-flow · commit a9c61ec1e144 · 2025-03-02T22:40:07.000Z
changelog: [`needless_collect`]: avoid warning if non-`Iterator` methods are called on the result of `into_iter` Fixes rust-lang#13430
diff --git a/clippy_lints/src/methods/needless_collect.rs b/clippy_lints/src/methods/needless_collect.rs
@@ -1,3 +1,5 @@
+use std::ops::ControlFlow;
+
 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};
@@ -9,9 +11,9 @@ use clippy_utils::{
 };
 use rustc_data_structures::fx::FxHashMap;
 use rustc_errors::{Applicability, MultiSpan};
-use rustc_hir::intravisit::{Visitor, walk_block, walk_expr};
+use rustc_hir::intravisit::{Visitor, walk_block, walk_expr, walk_stmt};
 use rustc_hir::{
-    BindingMode, Block, Expr, ExprKind, HirId, HirIdSet, LetStmt, Mutability, Node, PatKind, Stmt, StmtKind,
+    BindingMode, Block, Expr, ExprKind, HirId, HirIdSet, LetStmt, Mutability, Node, Pat, PatKind, Stmt, StmtKind,
 };
 use rustc_lint::LateContext;
 use rustc_middle::hir::nested_filter;
@@ -103,6 +105,12 @@ pub(super) fn check<'tcx>(
                     return;
                 }
 
+                if let IterFunctionKind::IntoIter(hir_id) = iter_call.func
+                    && !check_iter_expr_used_only_as_iterator(cx, hir_id, block)
+                {
+                    return;
+                }
+
                 // Suggest replacing iter_call with iter_replacement, and removing stmt
                 let mut span = MultiSpan::from_span(name_span);
                 span.push_span_label(iter_call.span, "the iterator could be used here instead");
@@ -253,7 +261,7 @@ struct IterFunction {
 impl IterFunction {
     fn get_iter_method(&self, cx: &LateContext<'_>) -> String {
         match &self.func {
-            IterFunctionKind::IntoIter => String::new(),
+            IterFunctionKind::IntoIter(_) => String::new(),
             IterFunctionKind::Len => String::from(".count()"),
             IterFunctionKind::IsEmpty => String::from(".next().is_none()"),
             IterFunctionKind::Contains(span) => {
@@ -268,7 +276,7 @@ impl IterFunction {
     }
     fn get_suggestion_text(&self) -> &'static str {
         match &self.func {
-            IterFunctionKind::IntoIter => {
+            IterFunctionKind::IntoIter(_) => {
                 "use the original Iterator instead of collecting it and then producing a new one"
             },
             IterFunctionKind::Len => {
@@ -284,7 +292,7 @@ impl IterFunction {
     }
 }
 enum IterFunctionKind {
-    IntoIter,
+    IntoIter(HirId),
     Len,
     IsEmpty,
     Contains(Span),
@@ -343,7 +351,7 @@ impl<'tcx> Visitor<'tcx> for IterFunctionVisitor<'_, 'tcx> {
                     }
                     match method_name.ident.name.as_str() {
                         "into_iter" => self.uses.push(Some(IterFunction {
-                            func: IterFunctionKind::IntoIter,
+                            func: IterFunctionKind::IntoIter(expr.hir_id),
                             span: expr.span,
                         })),
                         "len" => self.uses.push(Some(IterFunction {
@@ -520,3 +528,61 @@ fn get_captured_ids(cx: &LateContext<'_>, ty: Ty<'_>) -> HirIdSet {
 
     set
 }
+
+struct IteratorMethodCheckVisitor<'a, 'tcx> {
+    cx: &'a LateContext<'tcx>,
+    hir_id_of_expr: HirId,
+    hir_id_of_let_binding: Option<HirId>,
+}
+
+impl<'tcx> Visitor<'tcx> for IteratorMethodCheckVisitor<'_, 'tcx> {
+    type Result = ControlFlow<()>;
+    fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) -> ControlFlow<()> {
+        if let ExprKind::MethodCall(_method_name, recv, _args, _) = &expr.kind
+            && (recv.hir_id == self.hir_id_of_expr
+                || self
+                    .hir_id_of_let_binding
+                    .is_some_and(|hid| path_to_local_id(recv, hid)))
+            && !is_trait_method(self.cx, expr, sym::Iterator)
+        {
+            return ControlFlow::Break(());
+        } else if let ExprKind::Assign(place, value, _span) = &expr.kind
+            && value.hir_id == self.hir_id_of_expr
+            && let Some(id) = path_to_local(place)
+        {
+            // our iterator was directly assigned to a variable
+            self.hir_id_of_let_binding = Some(id);
+        }
+        walk_expr(self, expr)
+    }
+    fn visit_stmt(&mut self, stmt: &'tcx Stmt<'tcx>) -> ControlFlow<()> {
+        if let StmtKind::Let(LetStmt {
+            init: Some(expr),
+            pat:
+                Pat {
+                    kind: PatKind::Binding(BindingMode::NONE | BindingMode::MUT, id, _, None),
+                    ..
+                },
+            ..
+        }) = &stmt.kind
+            && expr.hir_id == self.hir_id_of_expr
+        {
+            // our iterator was directly assigned to a variable
+            self.hir_id_of_let_binding = Some(*id);
+        }
+        walk_stmt(self, stmt)
+    }
+}
+
+fn check_iter_expr_used_only_as_iterator<'tcx>(
+    cx: &LateContext<'tcx>,
+    hir_id_of_expr: HirId,
+    block: &'tcx Block<'tcx>,
+) -> bool {
+    let mut visitor = IteratorMethodCheckVisitor {
+        cx,
+        hir_id_of_expr,
+        hir_id_of_let_binding: None,
+    };
+    visitor.visit_block(block).is_continue()
+}
diff --git a/tests/ui/needless_collect.fixed b/tests/ui/needless_collect.fixed
@@ -98,6 +98,29 @@ fn main() {
     let mut out = vec![];
     values.iter().cloned().map(|x| out.push(x)).collect::<Vec<_>>();
     let _y = values.iter().cloned().map(|x| out.push(x)).collect::<Vec<_>>(); // this is fine
+
+    // Don't write a warning if we call `clone()` on the iterator
+    // https://github.com/rust-lang/rust-clippy/issues/13430
+    let my_collection: Vec<()> = vec![()].into_iter().map(|()| {}).collect();
+    let _cloned = my_collection.into_iter().clone();
+    let my_collection: Vec<()> = vec![()].into_iter().map(|()| {}).collect();
+    let my_iter = my_collection.into_iter();
+    let _cloned = my_iter.clone();
+    // Same for `as_slice()`, for same reason.
+    let my_collection: Vec<()> = vec![()].into_iter().map(|()| {}).collect();
+    let _sliced = my_collection.into_iter().as_slice();
+    let my_collection: Vec<()> = vec![()].into_iter().map(|()| {}).collect();
+    let my_iter = my_collection.into_iter();
+    let _sliced = my_iter.as_slice();
+    // Assignment outside of main scope
+    {
+        let x;
+        {
+            let xxx: Vec<()> = vec![()].into_iter().map(|()| {}).collect();
+            x = xxx.into_iter();
+            for i in x.as_slice() {}
+        }
+    }
 }
 
 fn foo(_: impl IntoIterator<Item = usize>) {}
diff --git a/tests/ui/needless_collect.rs b/tests/ui/needless_collect.rs
@@ -98,6 +98,29 @@ fn main() {
     let mut out = vec![];
     values.iter().cloned().map(|x| out.push(x)).collect::<Vec<_>>();
     let _y = values.iter().cloned().map(|x| out.push(x)).collect::<Vec<_>>(); // this is fine
+
+    // Don't write a warning if we call `clone()` on the iterator
+    // https://github.com/rust-lang/rust-clippy/issues/13430
+    let my_collection: Vec<()> = vec![()].into_iter().map(|()| {}).collect();
+    let _cloned = my_collection.into_iter().clone();
+    let my_collection: Vec<()> = vec![()].into_iter().map(|()| {}).collect();
+    let my_iter = my_collection.into_iter();
+    let _cloned = my_iter.clone();
+    // Same for `as_slice()`, for same reason.
+    let my_collection: Vec<()> = vec![()].into_iter().map(|()| {}).collect();
+    let _sliced = my_collection.into_iter().as_slice();
+    let my_collection: Vec<()> = vec![()].into_iter().map(|()| {}).collect();
+    let my_iter = my_collection.into_iter();
+    let _sliced = my_iter.as_slice();
+    // Assignment outside of main scope
+    {
+        let x;
+        {
+            let xxx: Vec<()> = vec![()].into_iter().map(|()| {}).collect();
+            x = xxx.into_iter();
+            for i in x.as_slice() {}
+        }
+    }
 }
 
 fn foo(_: impl IntoIterator<Item = usize>) {}
