From fe5f91ed8e54eeac1cc81930982d01483d745a65 Mon Sep 17 00:00:00 2001
From: Chayim Refael Friedman <chayimfr@gmail.com>
Date: Thu, 29 Aug 2024 00:10:26 +0300
Subject: [PATCH] Don't add reference when it isn't needed for the "Extract
 variable" assist

I.e. don't generate `let var_name = &foo()`.

Anything that creates a new value don't need a reference. That excludes mostly field accesses and indexing.

I had a thought that we can also not generate a reference for fields and indexing as long as the type is `Copy`, but sometimes people impl `Copy` even when they don't want to copy the values (e.g. a large type), so I didn't do that.
---
 .../src/handlers/extract_variable.rs          | 43 ++++++++++++++++++-
 1 file changed, 42 insertions(+), 1 deletion(-)

diff --git a/src/tools/rust-analyzer/crates/ide-assists/src/handlers/extract_variable.rs b/src/tools/rust-analyzer/crates/ide-assists/src/handlers/extract_variable.rs
index 33cd5252813..e6d1662e687 100644
--- a/src/tools/rust-analyzer/crates/ide-assists/src/handlers/extract_variable.rs
+++ b/src/tools/rust-analyzer/crates/ide-assists/src/handlers/extract_variable.rs
@@ -67,6 +67,15 @@ pub(crate) fn extract_variable(acc: &mut Assists, ctx: &AssistContext<'_>) -> Op
         ast::Expr::IndexExpr(index) if index.base().as_ref() == Some(&to_extract) => true,
         _ => false,
     });
+    let needs_ref = needs_adjust
+        && matches!(
+            to_extract,
+            ast::Expr::FieldExpr(_)
+                | ast::Expr::IndexExpr(_)
+                | ast::Expr::MacroExpr(_)
+                | ast::Expr::ParenExpr(_)
+                | ast::Expr::PathExpr(_)
+        );
 
     let anchor = Anchor::from(&to_extract)?;
     let target = to_extract.syntax().text_range();
@@ -93,10 +102,16 @@ pub(crate) fn extract_variable(acc: &mut Assists, ctx: &AssistContext<'_>) -> Op
                 Some(ast::Expr::RefExpr(expr)) if expr.mut_token().is_some() => {
                     make::ident_pat(false, true, make::name(&var_name))
                 }
+                _ if needs_adjust
+                    && !needs_ref
+                    && ty.as_ref().is_some_and(|ty| ty.is_mutable_reference()) =>
+                {
+                    make::ident_pat(false, true, make::name(&var_name))
+                }
                 _ => make::ident_pat(false, false, make::name(&var_name)),
             };
 
-            let to_extract = match ty.as_ref().filter(|_| needs_adjust) {
+            let to_extract = match ty.as_ref().filter(|_| needs_ref) {
                 Some(receiver_type) if receiver_type.is_mutable_reference() => {
                     make::expr_ref(to_extract, true)
                 }
@@ -1503,6 +1518,32 @@ fn foo() {
 fn foo() {
     let mut $0var_name = 0;
     let v = &mut var_name;
+}"#,
+        );
+    }
+
+    #[test]
+    fn generates_no_ref_on_calls() {
+        check_assist(
+            extract_variable,
+            r#"
+struct S;
+impl S {
+    fn do_work(&mut self) {}
+}
+fn bar() -> S { S }
+fn foo() {
+    $0bar()$0.do_work();
+}"#,
+            r#"
+struct S;
+impl S {
+    fn do_work(&mut self) {}
+}
+fn bar() -> S { S }
+fn foo() {
+    let mut $0bar = bar();
+    bar.do_work();
 }"#,
         );
     }