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

changelog: fix false positive: [`ptr_arg`] no longer triggers with
underscore binding to `&mut` argument

Fixes #13489
Fixes #13728

Author: immersum

clippy_lints/src/ptr.rs

@@ -584,7 +584,12 @@ 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
+                        // 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)
+                    {
                         self.bindings.insert(id, args_idx);
                     } else {
                         set_skip_flag();
@@ -650,7 +655,12 @@ 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)
+                        // 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)) =>
+                    {
                         Some((id, i))
                     },
                     _ => {

tests/ui/ptr_arg.rs

@@ -352,3 +352,91 @@ mod issue_13308 {
         h2(String::new(), v2);
     }
 }
+
+mod issue_13489 {
+    // This is a no-lint from now on.
+    fn foo(_x: &mut Vec<i32>) {
+        todo!();
+    }
+
+    // But this still gives us a lint.
+    fn foo_used(x: &mut Vec<i32>) {
+        //~^ ptr_arg
+
+        todo!();
+    }
+
+    // This is also a no-lint from now on.
+    fn foo_local(x: &mut Vec<i32>) {
+        let _y = x;
+
+        todo!();
+    }
+
+    // But this still gives us a lint.
+    fn foo_local_used(x: &mut Vec<i32>) {
+        //~^ ptr_arg
+
+        let y = x;
+
+        todo!();
+    }
+
+    // This only lints once from now on.
+    fn foofoo(_x: &mut Vec<i32>, y: &mut String) {
+        //~^ ptr_arg
+
+        todo!();
+    }
+
+    // And this is also a no-lint from now on.
+    fn foofoo_local(_x: &mut Vec<i32>, y: &mut String) {
+        let _z = y;
+
+        todo!();
+    }
+}
+
+mod issue_13728 {
+    // 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!()
+    }
+}

tests/ui/ptr_arg.stderr

@@ -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 `&mut Vec` instead of `&mut [_]` 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]`
+
+error: writing `&mut Vec` instead of `&mut [_]` 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]`
+
+error: writing `&mut String` instead of `&mut str` involves a new object where a slice will do
+  --> tests/ui/ptr_arg.rs:386:37
+   |
+LL |     fn foofoo(_x: &mut Vec<i32>, y: &mut String) {
+   |                                     ^^^^^^^^^^^ help: change this to: `&mut 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