about summary refs log tree commit diff
diff options
context:
space:
mode:
authorChayim Refael Friedman <chayimfr@gmail.com>2024-08-29 00:10:26 +0300
committerChayim Refael Friedman <chayimfr@gmail.com>2024-08-29 00:10:26 +0300
commitfe5f91ed8e54eeac1cc81930982d01483d745a65 (patch)
tree3efa56d8782a28f0b508c4b85fb089882e5744e7
parent2a7ec0b0ad0b9d7aea147694c23b5f52b6dc6fa9 (diff)
downloadrust-fe5f91ed8e54eeac1cc81930982d01483d745a65.tar.gz
rust-fe5f91ed8e54eeac1cc81930982d01483d745a65.zip
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.
-rw-r--r--src/tools/rust-analyzer/crates/ide-assists/src/handlers/extract_variable.rs43
1 files changed, 42 insertions, 1 deletions
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)
                 }
@@ -1506,4 +1521,30 @@ fn foo() {
 }"#,
         );
     }
+
+    #[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();
+}"#,
+        );
+    }
 }