about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2024-03-14 08:08:35 +0000
committerbors <bors@rust-lang.org>2024-03-14 08:08:35 +0000
commita75e271a65ff36a71dd4372551653ea3aca6e85d (patch)
tree3dec52b6b2b81bdcabbd7c260c8a71c1c87c14d0
parent660b058ba250e4f32ef165aaf8f61479da5dbb87 (diff)
parent5f8d8f165606775fcf81dfb27ac9de19f60a7bee (diff)
downloadrust-a75e271a65ff36a71dd4372551653ea3aca6e85d.tar.gz
rust-a75e271a65ff36a71dd4372551653ea3aca6e85d.zip
Auto merge of #12282 - maekawatoshiki:fix, r=xFrednet
Handle false positive with `map_clone` lint

### Summary

- Fixes https://github.com/rust-lang/rust-clippy/issues/12271
- (This is my first contribution to clippy and any suggestion would be appreciated)

changelog: [`map_clone`]: Handle false positive with `map_clone` lint
-rw-r--r--clippy_lints/src/methods/map_clone.rs11
-rw-r--r--tests/ui/map_clone.fixed16
-rw-r--r--tests/ui/map_clone.rs16
-rw-r--r--tests/ui/map_clone.stderr30
4 files changed, 53 insertions, 20 deletions
diff --git a/clippy_lints/src/methods/map_clone.rs b/clippy_lints/src/methods/map_clone.rs
index 27e17b43b01..c3c7a3a0033 100644
--- a/clippy_lints/src/methods/map_clone.rs
+++ b/clippy_lints/src/methods/map_clone.rs
@@ -86,8 +86,11 @@ pub(super) fn check(cx: &LateContext<'_>, e: &hir::Expr<'_>, recv: &hir::Expr<'_
                                     }
                                 }
                             },
-                            hir::ExprKind::Call(call, [_]) => {
-                                if let hir::ExprKind::Path(qpath) = call.kind {
+                            hir::ExprKind::Call(call, args) => {
+                                if let hir::ExprKind::Path(qpath) = call.kind
+                                    && let [arg] = args
+                                    && ident_eq(name, arg)
+                                {
                                     handle_path(cx, call, &qpath, e, recv);
                                 }
                             },
@@ -118,7 +121,9 @@ fn handle_path(
         if let ty::Adt(_, args) = cx.typeck_results().expr_ty(recv).kind()
             && let args = args.as_slice()
             && let Some(ty) = args.iter().find_map(|generic_arg| generic_arg.as_type())
-            && ty.is_ref()
+            && let ty::Ref(_, ty, Mutability::Not) = ty.kind()
+            && let ty::FnDef(_, lst) = cx.typeck_results().expr_ty(arg).kind()
+            && lst.iter().all(|l| l.as_type() == Some(*ty))
         {
             lint_path(cx, e.span, recv.span, is_copy(cx, ty.peel_refs()));
         }
diff --git a/tests/ui/map_clone.fixed b/tests/ui/map_clone.fixed
index 395eea69294..e58b6b2f19e 100644
--- a/tests/ui/map_clone.fixed
+++ b/tests/ui/map_clone.fixed
@@ -6,7 +6,8 @@
     clippy::redundant_clone,
     clippy::redundant_closure,
     clippy::useless_asref,
-    clippy::useless_vec
+    clippy::useless_vec,
+    clippy::empty_loop
 )]
 
 fn main() {
@@ -117,4 +118,17 @@ fn main() {
     let y = x.as_ref().map(|x| String::clone(x));
     let x: Result<String, ()> = Ok(String::new());
     let y = x.as_ref().map(|x| String::clone(x));
+
+    // Issue #12271
+    {
+        // Don't lint these
+        let x: Option<&u8> = None;
+        let y = x.map(|x| String::clone(loop {}));
+        let x: Option<&u8> = None;
+        let y = x.map(|x| u8::clone(loop {}));
+        let x: Vec<&u8> = vec![];
+        let y = x.into_iter().map(|x| String::clone(loop {}));
+        let x: Vec<&u8> = vec![];
+        let y = x.into_iter().map(|x| u8::clone(loop {}));
+    }
 }
diff --git a/tests/ui/map_clone.rs b/tests/ui/map_clone.rs
index 82a103edf5a..e642e4046f8 100644
--- a/tests/ui/map_clone.rs
+++ b/tests/ui/map_clone.rs
@@ -6,7 +6,8 @@
     clippy::redundant_clone,
     clippy::redundant_closure,
     clippy::useless_asref,
-    clippy::useless_vec
+    clippy::useless_vec,
+    clippy::empty_loop
 )]
 
 fn main() {
@@ -117,4 +118,17 @@ fn main() {
     let y = x.as_ref().map(|x| String::clone(x));
     let x: Result<String, ()> = Ok(String::new());
     let y = x.as_ref().map(|x| String::clone(x));
+
+    // Issue #12271
+    {
+        // Don't lint these
+        let x: Option<&u8> = None;
+        let y = x.map(|x| String::clone(loop {}));
+        let x: Option<&u8> = None;
+        let y = x.map(|x| u8::clone(loop {}));
+        let x: Vec<&u8> = vec![];
+        let y = x.into_iter().map(|x| String::clone(loop {}));
+        let x: Vec<&u8> = vec![];
+        let y = x.into_iter().map(|x| u8::clone(loop {}));
+    }
 }
diff --git a/tests/ui/map_clone.stderr b/tests/ui/map_clone.stderr
index 1a26a26a4ca..d9e025de4ab 100644
--- a/tests/ui/map_clone.stderr
+++ b/tests/ui/map_clone.stderr
@@ -1,5 +1,5 @@
 error: you are using an explicit closure for copying elements
-  --> tests/ui/map_clone.rs:13:22
+  --> tests/ui/map_clone.rs:14: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,85 +8,85 @@ 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
-  --> tests/ui/map_clone.rs:14:26
+  --> tests/ui/map_clone.rs:15: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
-  --> tests/ui/map_clone.rs:15:23
+  --> tests/ui/map_clone.rs:16: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
-  --> tests/ui/map_clone.rs:17:26
+  --> tests/ui/map_clone.rs:18: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
-  --> tests/ui/map_clone.rs:18:25
+  --> tests/ui/map_clone.rs:19: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
-  --> tests/ui/map_clone.rs:29:29
+  --> tests/ui/map_clone.rs:30:29
    |
 LL |     let _ = std::env::args().map(|v| v.clone());
    |                             ^^^^^^^^^^^^^^^^^^^ help: remove the `map` call
 
 error: you are explicitly cloning with `.map()`
-  --> tests/ui/map_clone.rs:68:13
+  --> tests/ui/map_clone.rs:69:13
    |
 LL |     let y = x.map(|x| String::clone(x));
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling the dedicated `cloned` method: `x.cloned()`
 
 error: you are explicitly cloning with `.map()`
-  --> tests/ui/map_clone.rs:70:13
+  --> tests/ui/map_clone.rs:71:13
    |
 LL |     let y = x.map(Clone::clone);
    |             ^^^^^^^^^^^^^^^^^^^ help: consider calling the dedicated `cloned` method: `x.cloned()`
 
 error: you are explicitly cloning with `.map()`
-  --> tests/ui/map_clone.rs:73:13
+  --> tests/ui/map_clone.rs:74:13
    |
 LL |     let y = x.map(String::clone);
    |             ^^^^^^^^^^^^^^^^^^^^ help: consider calling the dedicated `cloned` method: `x.cloned()`
 
 error: you are explicitly cloning with `.map()`
-  --> tests/ui/map_clone.rs:79:13
+  --> tests/ui/map_clone.rs:80:13
    |
 LL |     let y = x.map(|x| u32::clone(x));
    |             ^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling the dedicated `copied` method: `x.copied()`
 
 error: you are explicitly cloning with `.map()`
-  --> tests/ui/map_clone.rs:82:13
+  --> tests/ui/map_clone.rs:83:13
    |
 LL |     let y = x.map(|x| Clone::clone(x));
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling the dedicated `copied` method: `x.copied()`
 
 error: you are explicitly cloning with `.map()`
-  --> tests/ui/map_clone.rs:94:13
+  --> tests/ui/map_clone.rs:95:13
    |
 LL |     let y = x.map(|x| String::clone(x));
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling the dedicated `cloned` method: `x.cloned()`
 
 error: you are explicitly cloning with `.map()`
-  --> tests/ui/map_clone.rs:97:13
+  --> tests/ui/map_clone.rs:98:13
    |
 LL |     let y = x.map(|x| Clone::clone(x));
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling the dedicated `cloned` method: `x.cloned()`
 
 error: you are explicitly cloning with `.map()`
-  --> tests/ui/map_clone.rs:103:13
+  --> tests/ui/map_clone.rs:104:13
    |
 LL |     let y = x.map(|x| u32::clone(x));
    |             ^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling the dedicated `copied` method: `x.copied()`
 
 error: you are explicitly cloning with `.map()`
-  --> tests/ui/map_clone.rs:106:13
+  --> tests/ui/map_clone.rs:107:13
    |
 LL |     let y = x.map(|x| Clone::clone(x));
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling the dedicated `copied` method: `x.copied()`