fixup! Fix ptr_arg suggests changes when it's actually better not to bother

immersum · immersum · commit dbb0910db8fe · 2025-06-26T12:02:12.000+02:00
changelog: fix false positive: [`ptr_arg`] no longer triggers with
underscore binding to `&amp;T` or `&amp;mut T` argument
diff --git a/clippy_lints/src/ptr.rs b/clippy_lints/src/ptr.rs
@@ -586,9 +586,10 @@ fn check_ptr_arg_usage<'tcx>(cx: &LateContext<'tcx>, body: &Body<'tcx>, args: &[
                     // Only trace simple bindings. e.g `let x = y;`
                     if let PatKind::Binding(BindingMode::NONE, id, ident, None) = l.pat.kind
                         // Let's not lint for the current parameter. The user may still intend to mutate
-                        // the referenced value behind the parameter through this local let binding with
-                        // the underscore being temporary.
-                        && !(ident.name.as_str().starts_with('_') && args.mutability() == Mutability::Mut)
+                        // (or, if not mutate, then perhaps call a method that's not otherwise available
+                        // for) the referenced value behind the parameter through this local let binding
+                        // with the underscore being only temporary.
+                        && !ident.name.as_str().starts_with('_')
                     {
                         self.bindings.insert(id, args_idx);
                     } else {
@@ -656,10 +657,12 @@ fn check_ptr_arg_usage<'tcx>(cx: &LateContext<'tcx>, body: &Body<'tcx>, args: &[
                 let param = &body.params[arg.idx];
                 match param.pat.kind {
                     PatKind::Binding(BindingMode::NONE, id, ident, None)
+                        if !is_lint_allowed(cx, PTR_ARG, param.hir_id)
                         // Let's not lint for the current parameter. The user may still intend to mutate
-                        // the referenced value behind the parameter with the underscore being temporary.
-                        if !(ident.name.as_str().starts_with('_') && arg.mutability() == Mutability::Mut
-                            || is_lint_allowed(cx, PTR_ARG, param.hir_id)) =>
+                        // (or, if not mutate, then perhaps call a method that's not otherwise available
+                        // for) the referenced value behind the parameter with the underscore being only
+                        // temporary.
+                        && !ident.name.as_str().starts_with('_') =>
                     {
                         Some((id, i))
                     },
diff --git a/tests/ui/ptr_arg.rs b/tests/ui/ptr_arg.rs
@@ -123,7 +123,7 @@ fn test_cow_with_ref(c: &Cow<[i32]>) {}
 //~^ ptr_arg
 
 fn test_cow(c: Cow<[i32]>) {
-    let _c = c;
+    let d = c;
 }
 
 trait Foo2 {
@@ -141,36 +141,36 @@ mod issue_5644 {
     use std::path::PathBuf;
 
     fn allowed(
-        #[allow(clippy::ptr_arg)] _v: &Vec<u32>,
-        #[allow(clippy::ptr_arg)] _s: &String,
-        #[allow(clippy::ptr_arg)] _p: &PathBuf,
-        #[allow(clippy::ptr_arg)] _c: &Cow<[i32]>,
-        #[expect(clippy::ptr_arg)] _expect: &Cow<[i32]>,
+        #[allow(clippy::ptr_arg)] v: &Vec<u32>,
+        #[allow(clippy::ptr_arg)] s: &String,
+        #[allow(clippy::ptr_arg)] p: &PathBuf,
+        #[allow(clippy::ptr_arg)] c: &Cow<[i32]>,
+        #[expect(clippy::ptr_arg)] expect: &Cow<[i32]>,
     ) {
     }
 
-    fn some_allowed(#[allow(clippy::ptr_arg)] _v: &Vec<u32>, _s: &String) {}
+    fn some_allowed(#[allow(clippy::ptr_arg)] v: &Vec<u32>, s: &String) {}
     //~^ ptr_arg
 
     struct S;
     impl S {
         fn allowed(
-            #[allow(clippy::ptr_arg)] _v: &Vec<u32>,
-            #[allow(clippy::ptr_arg)] _s: &String,
-            #[allow(clippy::ptr_arg)] _p: &PathBuf,
-            #[allow(clippy::ptr_arg)] _c: &Cow<[i32]>,
-            #[expect(clippy::ptr_arg)] _expect: &Cow<[i32]>,
+            #[allow(clippy::ptr_arg)] v: &Vec<u32>,
+            #[allow(clippy::ptr_arg)] s: &String,
+            #[allow(clippy::ptr_arg)] p: &PathBuf,
+            #[allow(clippy::ptr_arg)] c: &Cow<[i32]>,
+            #[expect(clippy::ptr_arg)] expect: &Cow<[i32]>,
         ) {
         }
     }
 
     trait T {
         fn allowed(
-            #[allow(clippy::ptr_arg)] _v: &Vec<u32>,
-            #[allow(clippy::ptr_arg)] _s: &String,
-            #[allow(clippy::ptr_arg)] _p: &PathBuf,
-            #[allow(clippy::ptr_arg)] _c: &Cow<[i32]>,
-            #[expect(clippy::ptr_arg)] _expect: &Cow<[i32]>,
+            #[allow(clippy::ptr_arg)] v: &Vec<u32>,
+            #[allow(clippy::ptr_arg)] s: &String,
+            #[allow(clippy::ptr_arg)] p: &PathBuf,
+            #[allow(clippy::ptr_arg)] c: &Cow<[i32]>,
+            #[expect(clippy::ptr_arg)] expect: &Cow<[i32]>,
         ) {
         }
     }
@@ -182,22 +182,22 @@ mod issue6509 {
     fn foo_vec(vec: &Vec<u8>) {
         //~^ ptr_arg
 
-        let _ = vec.clone().pop();
-        let _ = vec.clone().clone();
+        let a = vec.clone().pop();
+        let b = vec.clone().clone();
     }
 
     fn foo_path(path: &PathBuf) {
         //~^ ptr_arg
 
-        let _ = path.clone().pop();
-        let _ = path.clone().clone();
+        let c = path.clone().pop();
+        let d = path.clone().clone();
     }
 
-    fn foo_str(str: &PathBuf) {
+    fn foo_str(str: &String) {
         //~^ ptr_arg
 
-        let _ = str.clone().pop();
-        let _ = str.clone().clone();
+        let e = str.clone().pop();
+        let f = str.clone().clone();
     }
 }
 
@@ -340,8 +340,8 @@ mod issue_13308 {
         ToOwned::clone_into(source, destination);
     }
 
-    fn h1(_: &<String as Deref>::Target) {}
-    fn h2<T: Deref>(_: T, _: &T::Target) {}
+    fn h1(x: &<String as Deref>::Target) {}
+    fn h2<T: Deref>(x: T, y: &T::Target) {}
 
     // Other cases that are still ok to lint and ideally shouldn't regress
     fn good(v1: &String, v2: &String) {
@@ -353,28 +353,28 @@ mod issue_13308 {
     }
 }
 
-mod issue_13489 {
+mod issue_13489_and_13728 {
     // This is a no-lint from now on.
-    fn foo(_x: &mut Vec<i32>) {
+    fn foo(_x: &Vec<i32>) {
         todo!();
     }
 
     // But this still gives us a lint.
-    fn foo_used(x: &mut Vec<i32>) {
+    fn foo_used(x: &Vec<i32>) {
         //~^ ptr_arg
 
         todo!();
     }
 
     // This is also a no-lint from now on.
-    fn foo_local(x: &mut Vec<i32>) {
+    fn foo_local(x: &Vec<i32>) {
         let _y = x;
 
         todo!();
     }
 
     // But this still gives us a lint.
-    fn foo_local_used(x: &mut Vec<i32>) {
+    fn foo_local_used(x: &Vec<i32>) {
         //~^ ptr_arg
 
         let y = x;
@@ -383,21 +383,21 @@ mod issue_13489 {
     }
 
     // This only lints once from now on.
-    fn foofoo(_x: &mut Vec<i32>, y: &mut String) {
+    fn foofoo(_x: &Vec<i32>, y: &String) {
         //~^ ptr_arg
 
         todo!();
     }
 
     // And this is also a no-lint from now on.
-    fn foofoo_local(_x: &mut Vec<i32>, y: &mut String) {
+    fn foofoo_local(_x: &Vec<i32>, y: &String) {
         let _z = y;
 
         todo!();
     }
 }
 
-mod issue_13728 {
+mod issue_13489_and_13728_mut {
     // This is a no-lint from now on.
     fn bar(_x: &mut Vec<u32>) {
         todo!()
diff --git a/tests/ui/ptr_arg.stderr b/tests/ui/ptr_arg.stderr
@@ -127,10 +127,10 @@ LL | fn test_cow_with_ref(c: &Cow<[i32]>) {}
    |                         ^^^^^^^^^^^ help: change this to: `&[i32]`
 
 error: writing `&String` instead of `&str` involves a new object where a slice will do
-  --> tests/ui/ptr_arg.rs:152:66
+  --> tests/ui/ptr_arg.rs:152:64
    |
-LL |     fn some_allowed(#[allow(clippy::ptr_arg)] _v: &Vec<u32>, _s: &String) {}
-   |                                                                  ^^^^^^^ help: change this to: `&str`
+LL |     fn some_allowed(#[allow(clippy::ptr_arg)] v: &Vec<u32>, s: &String) {}
+   |                                                                ^^^^^^^ help: change this to: `&str`
 
 error: writing `&Vec` instead of `&[_]` involves a new object where a slice will do
   --> tests/ui/ptr_arg.rs:182:21
@@ -143,8 +143,8 @@ help: change this to
 LL ~     fn foo_vec(vec: &[u8]) {
 LL |
 LL |
-LL ~         let _ = vec.to_owned().pop();
-LL ~         let _ = vec.to_owned().clone();
+LL ~         let a = vec.to_owned().pop();
+LL ~         let b = vec.to_owned().clone();
    |
 
 error: writing `&PathBuf` instead of `&Path` involves a new object where a slice will do
@@ -158,23 +158,23 @@ help: change this to
 LL ~     fn foo_path(path: &Path) {
 LL |
 LL |
-LL ~         let _ = path.to_path_buf().pop();
-LL ~         let _ = path.to_path_buf().clone();
+LL ~         let c = path.to_path_buf().pop();
+LL ~         let d = path.to_path_buf().clone();
    |
 
-error: writing `&PathBuf` instead of `&Path` involves a new object where a slice will do
+error: writing `&String` instead of `&str` involves a new object where a slice will do
   --> tests/ui/ptr_arg.rs:196:21
    |
-LL |     fn foo_str(str: &PathBuf) {
-   |                     ^^^^^^^^
+LL |     fn foo_str(str: &String) {
+   |                     ^^^^^^^
    |
 help: change this to
    |
-LL ~     fn foo_str(str: &Path) {
+LL ~     fn foo_str(str: &str) {
 LL |
 LL |
-LL ~         let _ = str.to_path_buf().pop();
-LL ~         let _ = str.to_path_buf().clone();
+LL ~         let e = str.to_owned().pop();
+LL ~         let f = str.to_owned().clone();
    |
 
 error: writing `&mut Vec` instead of `&mut [_]` involves a new object where a slice will do
@@ -231,23 +231,23 @@ error: writing `&String` instead of `&str` involves a new object where a slice w
 LL |     fn good(v1: &String, v2: &String) {
    |                              ^^^^^^^ help: change this to: `&str`
 
-error: writing `&mut Vec` instead of `&mut [_]` involves a new object where a slice will do
+error: writing `&Vec` instead of `&[_]` involves a new object where a slice will do
   --> tests/ui/ptr_arg.rs:363:20
    |
-LL |     fn foo_used(x: &mut Vec<i32>) {
-   |                    ^^^^^^^^^^^^^ help: change this to: `&mut [i32]`
+LL |     fn foo_used(x: &Vec<i32>) {
+   |                    ^^^^^^^^^ help: change this to: `&[i32]`
 
-error: writing `&mut Vec` instead of `&mut [_]` involves a new object where a slice will do
+error: writing `&Vec` instead of `&[_]` involves a new object where a slice will do
   --> tests/ui/ptr_arg.rs:377:26
    |
-LL |     fn foo_local_used(x: &mut Vec<i32>) {
-   |                          ^^^^^^^^^^^^^ help: change this to: `&mut [i32]`
+LL |     fn foo_local_used(x: &Vec<i32>) {
+   |                          ^^^^^^^^^ help: change this to: `&[i32]`
 
-error: writing `&mut String` instead of `&mut str` involves a new object where a slice will do
-  --> tests/ui/ptr_arg.rs:386:37
+error: writing `&String` instead of `&str` involves a new object where a slice will do
+  --> tests/ui/ptr_arg.rs:386:33
    |
-LL |     fn foofoo(_x: &mut Vec<i32>, y: &mut String) {
-   |                                     ^^^^^^^^^^^ help: change this to: `&mut str`
+LL |     fn foofoo(_x: &Vec<i32>, y: &String) {
+   |                                 ^^^^^^^ help: change this to: `&str`
 
 error: writing `&mut Vec` instead of `&mut [_]` involves a new object where a slice will do
   --> tests/ui/ptr_arg.rs:407:20
