Auto merge of #11289 - lengyijun:filter_find, r=blyxyas

 [iter_overeager_cloned]: detect `.cloned().filter()` and `.cloned().find()`

changelog: [`iter_overeager_cloned`]: detect `.cloned().filter()` and `.cloned().find()`

Key idea:
```
// before
iter.cloned().filter(|x| unimplemented!() )
// after
iter.filter(|&x| unimplemented!() ).cloned()

// before
iter.cloned().filter( foo )
// after
// notice `iter` must be `Iterator<Item= &T>` (callee of `cloned()`)
// so the parameter in the closure of `filter` must be `&&T`
// so the deref is safe
iter.filter(|&x| foo(x) ).cloned()
```

2 files changed, 59 insertions, 18 deletions

diff --git a/clippy_lints/src/methods/iter_overeager_cloned.rs b/clippy_lints/src/methods/iter_overeager_cloned.rs
index 9f7ec19aa59..4b1ca0d9b9c 100644
--- a/clippy_lints/src/methods/iter_overeager_cloned.rs
+++ b/clippy_lints/src/methods/iter_overeager_cloned.rs
@@ -2,7 +2,7 @@ use clippy_utils::diagnostics::span_lint_and_then;
 use clippy_utils::source::snippet_opt;
 use clippy_utils::ty::{implements_trait, is_copy};
 use rustc_errors::Applicability;
-use rustc_hir::Expr;
+use rustc_hir::{Expr, ExprKind};
 use rustc_lint::LateContext;
 use rustc_middle::ty;
 use rustc_span::sym;
@@ -10,12 +10,28 @@ use rustc_span::sym;
 use super::ITER_OVEREAGER_CLONED;
 use crate::redundant_clone::REDUNDANT_CLONE;
 
+#[derive(Clone, Copy)]
+pub(super) enum Op<'a> {
+    // rm `.cloned()`
+    // e.g. `count`
+    RmCloned,
+
+    // later `.cloned()`
+    // and add `&` to the parameter of closure parameter
+    // e.g. `find` `filter`
+    FixClosure(&'a str, &'a Expr<'a>),
+
+    // later `.cloned()`
+    // e.g. `skip` `take`
+    LaterCloned,
+}
+
 pub(super) fn check<'tcx>(
     cx: &LateContext<'tcx>,
     expr: &'tcx Expr<'_>,
     cloned_call: &'tcx Expr<'_>,
     cloned_recv: &'tcx Expr<'_>,
-    is_count: bool,
+    op: Op<'tcx>,
     needs_into_iter: bool,
 ) {
     let typeck = cx.typeck_results();
@@ -35,10 +51,9 @@ pub(super) fn check<'tcx>(
             return;
         }
 
-        let (lint, msg, trailing_clone) = if is_count {
-            (REDUNDANT_CLONE, "unneeded cloning of iterator items", "")
-        } else {
-            (ITER_OVEREAGER_CLONED, "unnecessarily eager cloning of iterator items", ".cloned()")
+        let (lint, msg, trailing_clone) = match op {
+            Op::RmCloned => (REDUNDANT_CLONE, "unneeded cloning of iterator items", ""),
+            Op::LaterCloned | Op::FixClosure(_, _) => (ITER_OVEREAGER_CLONED, "unnecessarily eager cloning of iterator items", ".cloned()"),
         };
 
         span_lint_and_then(
@@ -47,11 +62,27 @@ pub(super) fn check<'tcx>(
             expr.span,
             msg,
             |diag| {
-                let method_span = expr.span.with_lo(cloned_call.span.hi());
-                if let Some(mut snip) = snippet_opt(cx, method_span) {
-                    snip.push_str(trailing_clone);
-                    let replace_span = expr.span.with_lo(cloned_recv.span.hi());
-                    diag.span_suggestion(replace_span, "try", snip, Applicability::MachineApplicable);
+                match op {
+                    Op::RmCloned | Op::LaterCloned => {
+                        let method_span = expr.span.with_lo(cloned_call.span.hi());
+                        if let Some(mut snip) = snippet_opt(cx, method_span) {
+                            snip.push_str(trailing_clone);
+                            let replace_span = expr.span.with_lo(cloned_recv.span.hi());
+                            diag.span_suggestion(replace_span, "try", snip, Applicability::MachineApplicable);
+                        }
+                    }
+                    Op::FixClosure(name, predicate_expr) => {
+                        if let Some(predicate) = snippet_opt(cx, predicate_expr.span) {
+                            let new_closure = if let ExprKind::Closure(_) = predicate_expr.kind {
+                                predicate.replacen('|', "|&", 1)
+                            } else {
+                                format!("|&x| {predicate}(x)")
+                            };
+                            let snip = format!(".{name}({new_closure}).cloned()" );
+                            let replace_span = expr.span.with_lo(cloned_recv.span.hi());
+                            diag.span_suggestion(replace_span, "try", snip, Applicability::MachineApplicable);
+                        }
+                    }
                 }
             }
         );
diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs
index 42756b27d01..729eda37af6 100644
--- a/clippy_lints/src/methods/mod.rs
+++ b/clippy_lints/src/methods/mod.rs
@@ -3919,7 +3919,7 @@ impl Methods {
                     }
                 },
                 ("count", []) if is_trait_method(cx, expr, sym::Iterator) => match method_call(recv) {
-                    Some(("cloned", recv2, [], _, _)) => iter_overeager_cloned::check(cx, expr, recv, recv2, true, false),
+                    Some(("cloned", recv2, [], _, _)) => iter_overeager_cloned::check(cx, expr, recv, recv2, iter_overeager_cloned::Op::RmCloned , false),
                     Some((name2 @ ("into_iter" | "iter" | "iter_mut"), recv2, [], _, _)) => {
                         iter_count::check(cx, expr, recv2, name2);
                     },
@@ -3973,6 +3973,13 @@ impl Methods {
                     string_extend_chars::check(cx, expr, recv, arg);
                     extend_with_drain::check(cx, expr, recv, arg);
                 },
+                (name @ ( "filter" | "find" ) , [arg]) => {
+                    if let Some(("cloned", recv2, [], _span2, _)) = method_call(recv) {
+                        // if `arg` has side-effect, the semantic will change
+                        iter_overeager_cloned::check(cx, expr, recv, recv2,
+                                iter_overeager_cloned::Op::FixClosure(name, arg), false);
+                    }
+                }
                 ("filter_map", [arg]) => {
                     unnecessary_filter_map::check(cx, expr, arg, name);
                     filter_map_bool_then::check(cx, expr, arg, call_span);
@@ -3987,7 +3994,7 @@ impl Methods {
                 },
                 ("flatten", []) => match method_call(recv) {
                     Some(("map", recv, [map_arg], map_span, _)) => map_flatten::check(cx, expr, recv, map_arg, map_span),
-                    Some(("cloned", recv2, [], _, _)) => iter_overeager_cloned::check(cx, expr, recv, recv2, false, true),
+                    Some(("cloned", recv2, [], _, _)) => iter_overeager_cloned::check(cx, expr, recv, recv2, iter_overeager_cloned::Op::LaterCloned , true),
                     _ => {},
                 },
                 ("fold", [init, acc]) => {
@@ -4021,7 +4028,8 @@ impl Methods {
                 },
                 ("last", []) => {
                     if let Some(("cloned", recv2, [], _span2, _)) = method_call(recv) {
-                        iter_overeager_cloned::check(cx, expr, recv, recv2, false, false);
+                        iter_overeager_cloned::check(cx, expr, recv, recv2,
+                                iter_overeager_cloned::Op::LaterCloned , false);
                     }
                 },
                 ("lock", []) => {
@@ -4058,7 +4066,7 @@ impl Methods {
                 ("next", []) => {
                     if let Some((name2, recv2, args2, _, _)) = method_call(recv) {
                         match (name2, args2) {
-                            ("cloned", []) => iter_overeager_cloned::check(cx, expr, recv, recv2, false, false),
+                            ("cloned", []) => iter_overeager_cloned::check(cx, expr, recv, recv2, iter_overeager_cloned::Op::LaterCloned, false),
                             ("filter", [arg]) => filter_next::check(cx, expr, recv2, arg),
                             ("filter_map", [arg]) => filter_map_next::check(cx, expr, recv2, arg, &self.msrv),
                             ("iter", []) => iter_next_slice::check(cx, expr, recv2),
@@ -4071,7 +4079,7 @@ impl Methods {
                 },
                 ("nth", [n_arg]) => match method_call(recv) {
                     Some(("bytes", recv2, [], _, _)) => bytes_nth::check(cx, expr, recv2, n_arg),
-                    Some(("cloned", recv2, [], _, _)) => iter_overeager_cloned::check(cx, expr, recv, recv2, false, false),
+                    Some(("cloned", recv2, [], _, _)) => iter_overeager_cloned::check(cx, expr, recv, recv2, iter_overeager_cloned::Op::LaterCloned , false),
                     Some(("iter", recv2, [], _, _)) => iter_nth::check(cx, expr, recv2, recv, n_arg, false),
                     Some(("iter_mut", recv2, [], _, _)) => iter_nth::check(cx, expr, recv2, recv, n_arg, true),
                     _ => iter_nth_zero::check(cx, expr, recv, n_arg),
@@ -4126,7 +4134,8 @@ impl Methods {
                     iter_skip_zero::check(cx, expr, arg);
 
                     if let Some(("cloned", recv2, [], _span2, _)) = method_call(recv) {
-                        iter_overeager_cloned::check(cx, expr, recv, recv2, false, false);
+                        iter_overeager_cloned::check(cx, expr, recv, recv2,
+                                iter_overeager_cloned::Op::LaterCloned , false);
                     }
                 }
                 ("sort", []) => {
@@ -4152,7 +4161,8 @@ impl Methods {
                 ("step_by", [arg]) => iterator_step_by_zero::check(cx, expr, arg),
                 ("take", [_arg]) => {
                     if let Some(("cloned", recv2, [], _span2, _)) = method_call(recv) {
-                        iter_overeager_cloned::check(cx, expr, recv, recv2, false, false);
+                        iter_overeager_cloned::check(cx, expr, recv, recv2,
+                                iter_overeager_cloned::Op::LaterCloned, false);
                     }
                 },
                 ("take", []) => needless_option_take::check(cx, expr, recv),