Auto merge of #12109 - GuillaumeGomez:map-clone-call, r=llogiq

Handle "calls" inside the closure as well in `map_clone` lint

Follow-up of #12104.

I just realized that I didn't handle the case where the `clone` method was made as a call and not a method call.

r? `@llogiq`

changelog: Handle "calls" inside the closure as well in `map_clone` lint

Author: bors

clippy_lints/src/methods/map_clone.rs

@@ -61,26 +61,39 @@ pub(super) fn check(cx: &LateContext<'_>, e: &hir::Expr<'_>, recv: &hir::Expr<'_
                                     }
                                 }
                             },
+                            hir::ExprKind::Call(call, [_]) => {
+                                if let hir::ExprKind::Path(qpath) = call.kind {
+                                    handle_path(cx, call, &qpath, e, recv);
+                                }
+                            },
                             _ => {},
                         }
                     },
                     _ => {},
                 }
             },
-            hir::ExprKind::Path(qpath) => {
-                if let Some(path_def_id) = cx.qpath_res(&qpath, arg.hir_id).opt_def_id()
-                    && match_def_path(cx, path_def_id, &paths::CLONE_TRAIT_METHOD)
-                {
-                    // FIXME: It would be better to infer the type to check if it's copyable or not
-                    // to suggest to use `.copied()` instead of `.cloned()` where applicable.
-                    lint_path(cx, e.span, recv.span);
-                }
-            },
+            hir::ExprKind::Path(qpath) => handle_path(cx, arg, &qpath, e, recv),
             _ => {},
         }
     }
 }
 
+fn handle_path(
+    cx: &LateContext<'_>,
+    arg: &hir::Expr<'_>,
+    qpath: &hir::QPath<'_>,
+    e: &hir::Expr<'_>,
+    recv: &hir::Expr<'_>,
+) {
+    if let Some(path_def_id) = cx.qpath_res(qpath, arg.hir_id).opt_def_id()
+        && match_def_path(cx, path_def_id, &paths::CLONE_TRAIT_METHOD)
+    {
+        // FIXME: It would be better to infer the type to check if it's copyable or not
+        // to suggest to use `.copied()` instead of `.cloned()` where applicable.
+        lint_path(cx, e.span, recv.span);
+    }
+}
+
 fn ident_eq(name: Ident, path: &hir::Expr<'_>) -> bool {
     if let hir::ExprKind::Path(hir::QPath::Resolved(None, path)) = path.kind {
         path.segments.len() == 1 && path.segments[0].ident == name

tests/ui/map_clone.fixed

@@ -4,6 +4,7 @@
     clippy::iter_cloned_collect,
     clippy::many_single_char_names,
     clippy::redundant_clone,
+    clippy::redundant_closure,
     clippy::useless_vec
 )]
 
@@ -60,4 +61,8 @@ fn main() {
 
         let _ = Some(RefCell::new(String::new()).borrow()).map(|s| s.clone());
     }
+
+    let x = Some(String::new());
+    let y = x.as_ref().cloned();
+    //~^ ERROR: you are explicitly cloning with `.map()`
 }

tests/ui/map_clone.rs

@@ -4,6 +4,7 @@
     clippy::iter_cloned_collect,
     clippy::many_single_char_names,
     clippy::redundant_clone,
+    clippy::redundant_closure,
     clippy::useless_vec
 )]
 
@@ -60,4 +61,8 @@ fn main() {
 
         let _ = Some(RefCell::new(String::new()).borrow()).map(|s| s.clone());
     }
+
+    let x = Some(String::new());
+    let y = x.as_ref().map(|x| String::clone(x));
+    //~^ ERROR: you are explicitly cloning with `.map()`
 }

tests/ui/map_clone.stderr

@@ -1,5 +1,5 @@
 error: you are using an explicit closure for copying elements
-  --> $DIR/map_clone.rs:11:22
+  --> $DIR/map_clone.rs:12:22
    |
 LL |     let _: Vec<i8> = vec![5_i8; 6].iter().map(|x| *x).collect();
    |                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling the dedicated `copied` method: `vec![5_i8; 6].iter().copied()`
@@ -8,34 +8,40 @@ LL |     let _: Vec<i8> = vec![5_i8; 6].iter().map(|x| *x).collect();
    = help: to override `-D warnings` add `#[allow(clippy::map_clone)]`
 
 error: you are using an explicit closure for cloning elements
-  --> $DIR/map_clone.rs:12:26
+  --> $DIR/map_clone.rs:13:26
    |
 LL |     let _: Vec<String> = vec![String::new()].iter().map(|x| x.clone()).collect();
    |                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling the dedicated `cloned` method: `vec![String::new()].iter().cloned()`
 
 error: you are using an explicit closure for copying elements
-  --> $DIR/map_clone.rs:13:23
+  --> $DIR/map_clone.rs:14:23
    |
 LL |     let _: Vec<u32> = vec![42, 43].iter().map(|&x| x).collect();
    |                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling the dedicated `copied` method: `vec![42, 43].iter().copied()`
 
 error: you are using an explicit closure for copying elements
-  --> $DIR/map_clone.rs:15:26
+  --> $DIR/map_clone.rs:16:26
    |
 LL |     let _: Option<u64> = Some(&16).map(|b| *b);
    |                          ^^^^^^^^^^^^^^^^^^^^^ help: consider calling the dedicated `copied` method: `Some(&16).copied()`
 
 error: you are using an explicit closure for copying elements
-  --> $DIR/map_clone.rs:16:25
+  --> $DIR/map_clone.rs:17:25
    |
 LL |     let _: Option<u8> = Some(&1).map(|x| x.clone());
    |                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling the dedicated `copied` method: `Some(&1).copied()`
 
 error: you are needlessly cloning iterator elements
-  --> $DIR/map_clone.rs:27:29
+  --> $DIR/map_clone.rs:28:29
    |
 LL |     let _ = std::env::args().map(|v| v.clone());
    |                             ^^^^^^^^^^^^^^^^^^^ help: remove the `map` call
 
-error: aborting due to 6 previous errors
+error: you are explicitly cloning with `.map()`
+  --> $DIR/map_clone.rs:66:13
+   |
+LL |     let y = x.as_ref().map(|x| String::clone(x));
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling the dedicated `cloned` method: `x.as_ref().cloned()`
+
+error: aborting due to 7 previous errors