Fix ptr_arg suggests changes when it's actually better not to bother (#15105)

Jarcho · web-flow · commit 786330e02c37 · 2025-07-16T15:37:28.000Z
No longer suggests `&[i32]` or `&mut [i32]` instead of `&Vec<i32>` or `&mut Vec<i32>` (also: `Path` and `PathBuf`, etc.) for the parameter type when the parameter name starts with an underscore (or, if that does not start with one, then a local `let` binding in the function body, pointing to the same value, does) – (Fixes #13489, fixes #13728) ~changelog: fix false positive: [`ptr_arg`] no longer triggers with underscore binding to `&mut` argument~ changelog: fix false positive: [`ptr_arg`] no longer triggers with underscore binding to `&T` or `&mut T` argument *Edit:* This change has been extended to all references, not just mutable ones. See [discussion below](#issuecomment-3006386877).
diff --git a/clippy_lints/src/ptr.rs b/clippy_lints/src/ptr.rs
@@ -584,7 +584,13 @@ fn check_ptr_arg_usage<'tcx>(cx: &LateContext<'tcx>, body: &Body<'tcx>, args: &[
                 Some((Node::Stmt(_), _)) => (),
                 Some((Node::LetStmt(l), _)) => {
                     // Only trace simple bindings. e.g `let x = y;`
-                    if let PatKind::Binding(BindingMode::NONE, id, _, None) = l.pat.kind {
+                    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
+                        // (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 {
                         set_skip_flag();
@@ -650,7 +656,14 @@ fn check_ptr_arg_usage<'tcx>(cx: &LateContext<'tcx>, body: &Body<'tcx>, args: &[
             .filter_map(|(i, arg)| {
                 let param = &body.params[arg.idx];
                 match param.pat.kind {
-                    PatKind::Binding(BindingMode::NONE, id, _, None) if !is_lint_allowed(cx, PTR_ARG, param.hir_id) => {
+                    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
+                        // (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) {
@@ -352,3 +352,91 @@ mod issue_13308 {
         h2(String::new(), v2);
     }
 }
+
+mod issue_13489_and_13728 {
+    // This is a no-lint from now on.
+    fn foo(_x: &Vec<i32>) {
+        todo!();
+    }
+
+    // But this still gives us a lint.
+    fn foo_used(x: &Vec<i32>) {
+        //~^ ptr_arg
+
+        todo!();
+    }
+
+    // This is also a no-lint from now on.
+    fn foo_local(x: &Vec<i32>) {
+        let _y = x;
+
+        todo!();
+    }
+
+    // But this still gives us a lint.
+    fn foo_local_used(x: &Vec<i32>) {
+        //~^ ptr_arg
+
+        let y = x;
+
+        todo!();
+    }
+
+    // This only lints once from now on.
+    fn foofoo(_x: &Vec<i32>, y: &String) {
+        //~^ ptr_arg
+
+        todo!();
+    }
+
+    // And this is also a no-lint from now on.
+    fn foofoo_local(_x: &Vec<i32>, y: &String) {
+        let _z = y;
+
+        todo!();
+    }
+}
+
+mod issue_13489_and_13728_mut {
+    // This is a no-lint from now on.
+    fn bar(_x: &mut Vec<u32>) {
+        todo!()
+    }
+
+    // But this still gives us a lint.
+    fn bar_used(x: &mut Vec<u32>) {
+        //~^ ptr_arg
+
+        todo!()
+    }
+
+    // This is also a no-lint from now on.
+    fn bar_local(x: &mut Vec<u32>) {
+        let _y = x;
+
+        todo!()
+    }
+
+    // But this still gives us a lint.
+    fn bar_local_used(x: &mut Vec<u32>) {
+        //~^ ptr_arg
+
+        let y = x;
+
+        todo!()
+    }
+
+    // This only lints once from now on.
+    fn barbar(_x: &mut Vec<u32>, y: &mut String) {
+        //~^ ptr_arg
+
+        todo!()
+    }
+
+    // And this is also a no-lint from now on.
+    fn barbar_local(_x: &mut Vec<u32>, y: &mut String) {
+        let _z = y;
+
+        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,6 +231,42 @@ 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 `&Vec` instead of `&[_]` involves a new object where a slice will do
+  --> tests/ui/ptr_arg.rs:363:20
+   |
+LL |     fn foo_used(x: &Vec<i32>) {
+   |                    ^^^^^^^^^ help: change this to: `&[i32]`
+
+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: &Vec<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:386:33
+   |
+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
+   |
+LL |     fn bar_used(x: &mut Vec<u32>) {
+   |                    ^^^^^^^^^^^^^ help: change this to: `&mut [u32]`
+
+error: writing `&mut Vec` instead of `&mut [_]` involves a new object where a slice will do
+  --> tests/ui/ptr_arg.rs:421:26
+   |
+LL |     fn bar_local_used(x: &mut Vec<u32>) {
+   |                          ^^^^^^^^^^^^^ help: change this to: `&mut [u32]`
+
+error: writing `&mut String` instead of `&mut str` involves a new object where a slice will do
+  --> tests/ui/ptr_arg.rs:430:37
+   |
+LL |     fn barbar(_x: &mut Vec<u32>, y: &mut String) {
+   |                                     ^^^^^^^^^^^ help: change this to: `&mut str`
+
 error: lifetime flowing from input to output with different syntax can be confusing
   --> tests/ui/ptr_arg.rs:314:36
    |
@@ -247,5 +283,5 @@ help: one option is to consistently use `'a`
 LL |     fn cow_good_ret_ty<'a>(input: &'a Cow<'a, str>) -> &'a str {
    |                                                         ++
 
-error: aborting due to 27 previous errors
+error: aborting due to 33 previous errors
 
