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()
```

5 files changed, 193 insertions, 28 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),
diff --git a/tests/ui/iter_overeager_cloned.fixed b/tests/ui/iter_overeager_cloned.fixed
index ba26fe33d89..9605b449b40 100644
--- a/tests/ui/iter_overeager_cloned.fixed
+++ b/tests/ui/iter_overeager_cloned.fixed
@@ -20,8 +20,41 @@ fn main() {
         .iter()
         .flatten().cloned();
 
-    // Not implemented yet
-    let _ = vec.iter().cloned().filter(|x| x.starts_with('2'));
+    let _ = vec.iter().filter(|&x| x.starts_with('2')).cloned();
+
+    let _ = vec.iter().find(|&x| x == "2").cloned();
+
+    {
+        let f = |x: &String| x.starts_with('2');
+        let _ = vec.iter().filter(|&x| f(x)).cloned();
+        let _ = vec.iter().find(|&x| f(x)).cloned();
+    }
+
+    {
+        let vec: Vec<(String, String)> = vec![];
+        let f = move |x: &(String, String)| x.0.starts_with('2');
+        let _ = vec.iter().filter(|&x| f(x)).cloned();
+        let _ = vec.iter().find(|&x| f(x)).cloned();
+    }
+
+    fn test_move<'a>(
+        iter: impl Iterator<Item = &'a (&'a u32, String)> + 'a,
+        target: String,
+    ) -> impl Iterator<Item = (&'a u32, String)> + 'a {
+        iter.filter(move |&(&a, b)| a == 1 && b == &target).cloned()
+    }
+
+    {
+        #[derive(Clone)]
+        struct S<'a> {
+            a: &'a u32,
+            b: String,
+        }
+
+        fn bar<'a>(iter: impl Iterator<Item = &'a S<'a>> + 'a, target: String) -> impl Iterator<Item = S<'a>> + 'a {
+            iter.filter(move |&S { a, b }| **a == 1 && b == &target).cloned()
+        }
+    }
 
     // Not implemented yet
     let _ = vec.iter().cloned().map(|x| x.len());
@@ -30,9 +63,6 @@ fn main() {
     let _ = vec.iter().cloned().map(|x| x + "2");
 
     // Not implemented yet
-    let _ = vec.iter().cloned().find(|x| x == "2");
-
-    // Not implemented yet
     let _ = vec.iter().cloned().for_each(|x| assert!(!x.is_empty()));
 
     // Not implemented yet
diff --git a/tests/ui/iter_overeager_cloned.rs b/tests/ui/iter_overeager_cloned.rs
index 2d3c4ea7c20..8d75f039d44 100644
--- a/tests/ui/iter_overeager_cloned.rs
+++ b/tests/ui/iter_overeager_cloned.rs
@@ -21,9 +21,42 @@ fn main() {
         .cloned()
         .flatten();
 
-    // Not implemented yet
     let _ = vec.iter().cloned().filter(|x| x.starts_with('2'));
 
+    let _ = vec.iter().cloned().find(|x| x == "2");
+
+    {
+        let f = |x: &String| x.starts_with('2');
+        let _ = vec.iter().cloned().filter(f);
+        let _ = vec.iter().cloned().find(f);
+    }
+
+    {
+        let vec: Vec<(String, String)> = vec![];
+        let f = move |x: &(String, String)| x.0.starts_with('2');
+        let _ = vec.iter().cloned().filter(f);
+        let _ = vec.iter().cloned().find(f);
+    }
+
+    fn test_move<'a>(
+        iter: impl Iterator<Item = &'a (&'a u32, String)> + 'a,
+        target: String,
+    ) -> impl Iterator<Item = (&'a u32, String)> + 'a {
+        iter.cloned().filter(move |(&a, b)| a == 1 && b == &target)
+    }
+
+    {
+        #[derive(Clone)]
+        struct S<'a> {
+            a: &'a u32,
+            b: String,
+        }
+
+        fn bar<'a>(iter: impl Iterator<Item = &'a S<'a>> + 'a, target: String) -> impl Iterator<Item = S<'a>> + 'a {
+            iter.cloned().filter(move |S { a, b }| **a == 1 && b == &target)
+        }
+    }
+
     // Not implemented yet
     let _ = vec.iter().cloned().map(|x| x.len());
 
@@ -31,9 +64,6 @@ fn main() {
     let _ = vec.iter().cloned().map(|x| x + "2");
 
     // Not implemented yet
-    let _ = vec.iter().cloned().find(|x| x == "2");
-
-    // Not implemented yet
     let _ = vec.iter().cloned().for_each(|x| assert!(!x.is_empty()));
 
     // Not implemented yet
diff --git a/tests/ui/iter_overeager_cloned.stderr b/tests/ui/iter_overeager_cloned.stderr
index 2be2b7e07a4..bdbb04d7f08 100644
--- a/tests/ui/iter_overeager_cloned.stderr
+++ b/tests/ui/iter_overeager_cloned.stderr
@@ -66,5 +66,69 @@ LL ~         .iter()
 LL ~         .flatten().cloned();
    |
 
-error: aborting due to 7 previous errors
+error: unnecessarily eager cloning of iterator items
+  --> $DIR/iter_overeager_cloned.rs:24:13
+   |
+LL |     let _ = vec.iter().cloned().filter(|x| x.starts_with('2'));
+   |             ^^^^^^^^^^----------------------------------------
+   |                       |
+   |                       help: try: `.filter(|&x| x.starts_with('2')).cloned()`
+
+error: unnecessarily eager cloning of iterator items
+  --> $DIR/iter_overeager_cloned.rs:26:13
+   |
+LL |     let _ = vec.iter().cloned().find(|x| x == "2");
+   |             ^^^^^^^^^^----------------------------
+   |                       |
+   |                       help: try: `.find(|&x| x == "2").cloned()`
+
+error: unnecessarily eager cloning of iterator items
+  --> $DIR/iter_overeager_cloned.rs:30:17
+   |
+LL |         let _ = vec.iter().cloned().filter(f);
+   |                 ^^^^^^^^^^-------------------
+   |                           |
+   |                           help: try: `.filter(|&x| f(x)).cloned()`
+
+error: unnecessarily eager cloning of iterator items
+  --> $DIR/iter_overeager_cloned.rs:31:17
+   |
+LL |         let _ = vec.iter().cloned().find(f);
+   |                 ^^^^^^^^^^-----------------
+   |                           |
+   |                           help: try: `.find(|&x| f(x)).cloned()`
+
+error: unnecessarily eager cloning of iterator items
+  --> $DIR/iter_overeager_cloned.rs:37:17
+   |
+LL |         let _ = vec.iter().cloned().filter(f);
+   |                 ^^^^^^^^^^-------------------
+   |                           |
+   |                           help: try: `.filter(|&x| f(x)).cloned()`
+
+error: unnecessarily eager cloning of iterator items
+  --> $DIR/iter_overeager_cloned.rs:38:17
+   |
+LL |         let _ = vec.iter().cloned().find(f);
+   |                 ^^^^^^^^^^-----------------
+   |                           |
+   |                           help: try: `.find(|&x| f(x)).cloned()`
+
+error: unnecessarily eager cloning of iterator items
+  --> $DIR/iter_overeager_cloned.rs:45:9
+   |
+LL |         iter.cloned().filter(move |(&a, b)| a == 1 && b == &target)
+   |         ^^^^-------------------------------------------------------
+   |             |
+   |             help: try: `.filter(move |&(&a, b)| a == 1 && b == &target).cloned()`
+
+error: unnecessarily eager cloning of iterator items
+  --> $DIR/iter_overeager_cloned.rs:56:13
+   |
+LL |             iter.cloned().filter(move |S { a, b }| **a == 1 && b == &target)
+   |             ^^^^------------------------------------------------------------
+   |                 |
+   |                 help: try: `.filter(move |&S { a, b }| **a == 1 && b == &target).cloned()`
+
+error: aborting due to 15 previous errors