[Draft] [iter_overeager_cloned]: detect .cloned().map() and .cloned()…

….for_each()

key idea:
for `f` in `map` and `for_each`, if `f` never move the parameter, the `cloned` can be removed

clippy_lints/src/methods/iter_overeager_cloned.rs

@@ -1,9 +1,12 @@
 use clippy_utils::diagnostics::span_lint_and_then;
 use clippy_utils::source::snippet_opt;
 use clippy_utils::ty::{implements_trait, is_copy};
+use rustc_ast::BindingAnnotation;
 use rustc_errors::Applicability;
-use rustc_hir::Expr;
+use rustc_hir::{Body, Expr};
 use rustc_lint::LateContext;
+use rustc_middle::mir::visit::Visitor;
+use rustc_middle::mir::{self, Local, Location, Mutability, Operand, VarDebugInfoContents};
 use rustc_middle::ty;
 use rustc_span::sym;
 
@@ -16,6 +19,10 @@ pub(super) enum Op<'a> {
     // e.g. `count`
     RmCloned,
 
+    // rm `.cloned()`
+    // e.g. `map` `for_each`
+    NeedlessMove(&'a str, &'a Expr<'a>),
+
     // later `.cloned()`
     // and add `&` to the parameter of closure parameter
     // e.g. `find` `filter`
@@ -51,8 +58,32 @@ pub(super) fn check<'tcx>(
             return;
         }
 
+        if let Op::NeedlessMove(_, expr) = op {
+            if let rustc_hir::ExprKind::Closure(closure) = expr.kind
+               && let Body {params: [p], value: _, generator_kind: _ } = cx.tcx.hir().body(closure.body) {
+                    match p.pat.kind {
+                        rustc_hir::PatKind::Binding(BindingAnnotation(_, Mutability::Not), _, _, _) => {
+                            let body = cx.tcx.optimized_mir(closure.def_id);
+                            if let VarDebugInfoContents::Place(p) = &body.var_debug_info[0].value {
+                                let mut find_move = FindMove::new(p.local);
+                                find_move.visit_body(body);
+                                if find_move.found {
+                                    return;
+                                }
+                            } else {
+                                return;
+                            }
+                        }
+                        rustc_hir::PatKind::Wild => {}
+                        _ => return,
+                    }
+            } else {
+                return;
+            }
+        }
+
         let (lint, msg, trailing_clone) = match op {
-            Op::RmCloned => (REDUNDANT_CLONE, "unneeded cloning of iterator items", ""),
+            Op::RmCloned | Op::NeedlessMove(_, _) => (REDUNDANT_CLONE, "unneeded cloning of iterator items", ""),
             Op::LaterCloned | Op::FixClosure(_, _) => (ITER_OVEREAGER_CLONED, "unnecessarily eager cloning of iterator items", ".cloned()"),
         };
 
@@ -83,8 +114,44 @@ pub(super) fn check<'tcx>(
                             diag.span_suggestion(replace_span, "try", snip, Applicability::MachineApplicable);
                         }
                     }
+                    Op::NeedlessMove(_, _) => {
+                        let method_span = expr.span.with_lo(cloned_call.span.hi());
+                        if let Some(snip) = snippet_opt(cx, method_span) {
+                            let replace_span = expr.span.with_lo(cloned_recv.span.hi());
+                            // suggestion may not accurate: programmer need some manual fix
+                            diag.span_suggestion(replace_span, "try", snip, Applicability::MachineApplicable);
+                        }
+                    }
                 }
             }
         );
     }
 }
+
+struct FindMove {
+    target: Local,
+    found: bool,
+}
+
+impl FindMove {
+    fn new(target: Local) -> Self {
+        FindMove { target, found: false }
+    }
+}
+
+impl<'tcx> mir::visit::Visitor<'tcx> for FindMove {
+    fn visit_operand(&mut self, operand: &Operand<'tcx>, _: Location) {
+        if self.found {
+            return;
+        }
+
+        match &operand {
+            Operand::Move(p) | Operand::Copy(p) => {
+                if p.local == self.target {
+                    self.found = true;
+                }
+            },
+            Operand::Constant(_) => {},
+        }
+    }
+}

clippy_lints/src/methods/mod.rs

@@ -4001,9 +4001,11 @@ impl Methods {
                     manual_try_fold::check(cx, expr, init, acc, call_span, &self.msrv);
                     unnecessary_fold::check(cx, expr, init, acc, span);
                 },
-                ("for_each", [_]) => {
-                    if let Some(("inspect", _, [_], span2, _)) = method_call(recv) {
-                        inspect_for_each::check(cx, expr, span2);
+                ("for_each", [arg]) => {
+                    match method_call(recv) {
+                        Some(("inspect", _, [_], span2, _)) => inspect_for_each::check(cx, expr, span2),
+                        Some(("cloned", recv2, [], _span2, _)) => iter_overeager_cloned::check(cx, expr, recv, recv2, iter_overeager_cloned::Op::NeedlessMove(name, arg), false),
+                        _ => {}
                     }
                 },
                 ("get", [arg]) => {
@@ -4038,8 +4040,10 @@ impl Methods {
                 (name @ ("map" | "map_err"), [m_arg]) => {
                     if name == "map" {
                         map_clone::check(cx, expr, recv, m_arg, &self.msrv);
-                        if let Some((map_name @ ("iter" | "into_iter"), recv2, _, _, _)) = method_call(recv) {
-                            iter_kv_map::check(cx, map_name, expr, recv2, m_arg);
+                        match method_call(recv) {
+                            Some((map_name @ ("iter" | "into_iter"), recv2, _, _, _)) => iter_kv_map::check(cx, map_name, expr, recv2, m_arg),
+                            Some(("cloned", recv2, [], _span2, _)) => iter_overeager_cloned::check(cx, expr, recv, recv2, iter_overeager_cloned::Op::NeedlessMove(name, m_arg), false),
+                            _ => {}
                         }
                     } else {
                         map_err_ignore::check(cx, expr, m_arg);

tests/ui/iter_overeager_cloned.fixed

@@ -43,14 +43,12 @@ fn main() {
         let _ = vec.iter().filter(|&(x, y)| x == "2").cloned();
     }
 
-    // Not implemented yet
-    let _ = vec.iter().cloned().map(|x| x.len());
+    let _ = vec.iter().map(|x| x.len());
 
     // This would fail if changed.
     let _ = vec.iter().cloned().map(|x| x + "2");
 
-    // Not implemented yet
-    let _ = vec.iter().cloned().for_each(|x| assert!(!x.is_empty()));
+    let _ = vec.iter().for_each(|x| assert!(!x.is_empty()));
 
     // Not implemented yet
     let _ = vec.iter().cloned().all(|x| x.len() == 1);

tests/ui/iter_overeager_cloned.rs

@@ -44,13 +44,11 @@ fn main() {
         let _ = vec.iter().cloned().filter(|(x, y)| x == "2");
     }
 
-    // Not implemented yet
     let _ = vec.iter().cloned().map(|x| x.len());
 
     // This would fail if changed.
     let _ = vec.iter().cloned().map(|x| x + "2");
 
-    // Not implemented yet
     let _ = vec.iter().cloned().for_each(|x| assert!(!x.is_empty()));
 
     // Not implemented yet

tests/ui/iter_overeager_cloned.stderr

@@ -122,5 +122,21 @@ LL |         let _ = vec.iter().cloned().filter(|(x, y)| x == "2");
    |                           |
    |                           help: try: `.filter(|&(x, y)| x == "2").cloned()`
 
-error: aborting due to 14 previous errors
+error: unneeded cloning of iterator items
+  --> $DIR/iter_overeager_cloned.rs:47:13
+   |
+LL |     let _ = vec.iter().cloned().map(|x| x.len());
+   |             ^^^^^^^^^^--------------------------
+   |                       |
+   |                       help: try: `.map(|x| x.len())`
+
+error: unneeded cloning of iterator items
+  --> $DIR/iter_overeager_cloned.rs:52:13
+   |
+LL |     let _ = vec.iter().cloned().for_each(|x| assert!(!x.is_empty()));
+   |             ^^^^^^^^^^----------------------------------------------
+   |                       |
+   |                       help: try: `.for_each(|x| assert!(!x.is_empty()))`
+
+error: aborting due to 16 previous errors