summary refs log tree commit diff
diff options
context:
space:
mode:
authorPhilipp Krones <hello@philkrones.com>2025-03-10 15:48:43 +0000
committerGitHub <noreply@github.com>2025-03-10 15:48:43 +0000
commita25cbd50ca170d59b187b2a8e56cb33470e524eb (patch)
tree05fa5fa58da8d7b9350eb5c9eeb81c9fa52807d3
parent819f3c7c6761cca808f110b84b6b6fc9964cf265 (diff)
parentc8e47f9a72b0c3009e17b4fb66667931ac335026 (diff)
downloadrust-a25cbd50ca170d59b187b2a8e56cb33470e524eb.tar.gz
rust-a25cbd50ca170d59b187b2a8e56cb33470e524eb.zip
Improve `needless_pass_by_value` suggestion (#13880)
Fixes https://github.com/rust-lang/rust-clippy/issues/13744.

A simple check to check if the type is an `Option` allows to improve the
suggestion.

changelog: Improve `needless_pass_by_value` suggestion
-rw-r--r--clippy_lints/src/needless_pass_by_value.rs10
-rw-r--r--tests/ui/needless_pass_by_value.rs7
-rw-r--r--tests/ui/needless_pass_by_value.stderr10
3 files changed, 24 insertions, 3 deletions
diff --git a/clippy_lints/src/needless_pass_by_value.rs b/clippy_lints/src/needless_pass_by_value.rs
index 380cc380ad0..f5652e7b832 100644
--- a/clippy_lints/src/needless_pass_by_value.rs
+++ b/clippy_lints/src/needless_pass_by_value.rs
@@ -279,10 +279,18 @@ impl<'tcx> LateLintPass<'tcx> for NeedlessPassByValue {
                         }
                     }
 
+                    let suggestion = if is_type_diagnostic_item(cx, ty, sym::Option)
+                        && let snip = snippet(cx, input.span, "_")
+                        && let Some((first, rest)) = snip.split_once('<')
+                    {
+                        format!("{first}<&{rest}")
+                    } else {
+                        format!("&{}", snippet(cx, input.span, "_"))
+                    };
                     diag.span_suggestion(
                         input.span,
                         "consider taking a reference instead",
-                        format!("&{}", snippet(cx, input.span, "_")),
+                        suggestion,
                         Applicability::MaybeIncorrect,
                     );
                 };
diff --git a/tests/ui/needless_pass_by_value.rs b/tests/ui/needless_pass_by_value.rs
index 885fb409417..adea373fd55 100644
--- a/tests/ui/needless_pass_by_value.rs
+++ b/tests/ui/needless_pass_by_value.rs
@@ -189,6 +189,13 @@ struct Obj(String);
 
 fn prefix_test(_unused_with_prefix: Obj) {}
 
+// Regression test for <https://github.com/rust-lang/rust-clippy/issues/13744>.
+// It's more idiomatic to write `Option<&T>` rather than `&Option<T>`.
+fn option_inner_ref(x: Option<String>) {
+    //~^ ERROR: this argument is passed by value, but not consumed in the function body
+    assert!(x.is_some());
+}
+
 fn main() {
     // This should not cause an ICE either
     // https://github.com/rust-lang/rust-clippy/issues/3144
diff --git a/tests/ui/needless_pass_by_value.stderr b/tests/ui/needless_pass_by_value.stderr
index 4ac4fdce972..987bfc4affc 100644
--- a/tests/ui/needless_pass_by_value.stderr
+++ b/tests/ui/needless_pass_by_value.stderr
@@ -29,7 +29,7 @@ error: this argument is passed by value, but not consumed in the function body
   --> tests/ui/needless_pass_by_value.rs:58:18
    |
 LL | fn test_match(x: Option<Option<String>>, y: Option<Option<String>>) {
-   |                  ^^^^^^^^^^^^^^^^^^^^^^ help: consider taking a reference instead: `&Option<Option<String>>`
+   |                  ^^^^^^^^^^^^^^^^^^^^^^ help: consider taking a reference instead: `Option<&Option<String>>`
 
 error: this argument is passed by value, but not consumed in the function body
   --> tests/ui/needless_pass_by_value.rs:73:24
@@ -179,5 +179,11 @@ error: this argument is passed by value, but not consumed in the function body
 LL | fn more_fun(items: impl Club<'static, i32>) {}
    |                    ^^^^^^^^^^^^^^^^^^^^^^^ help: consider taking a reference instead: `&impl Club<'static, i32>`
 
-error: aborting due to 22 previous errors
+error: this argument is passed by value, but not consumed in the function body
+  --> tests/ui/needless_pass_by_value.rs:194:24
+   |
+LL | fn option_inner_ref(x: Option<String>) {
+   |                        ^^^^^^^^^^^^^^ help: consider taking a reference instead: `Option<&String>`
+
+error: aborting due to 23 previous errors