Trigger VSCode to rename after extract variable assist is applied

bors · joshka · commit 377a0a7696bb · 2024-07-12T19:06:19.000-07:00
When the user applies the "Extract Variable" assist, the cursor is positioned at the newly inserted variable. This commit adds a command to the assist that triggers the rename action in VSCode. This way, the user can quickly rename the variable after applying the assist. Fixes part of: #17579
diff --git a/src/tools/rust-analyzer/crates/ide-assists/src/assist_context.rs b/src/tools/rust-analyzer/crates/ide-assists/src/assist_context.rs
@@ -185,19 +185,19 @@ impl Assists {
             return None;
         }
 
-        let mut trigger_signature_help = false;
+        let mut command = None;
         let source_change = if self.resolve.should_resolve(&id) {
             let mut builder = SourceChangeBuilder::new(self.file);
             f(&mut builder);
-            trigger_signature_help = builder.trigger_signature_help;
+            command = builder.command.take();
             Some(builder.finish())
         } else {
             None
         };
 
         let label = Label::new(label);
         let group = group.cloned();
-        self.buf.push(Assist { id, label, group, target, source_change, trigger_signature_help });
+        self.buf.push(Assist { id, label, group, target, source_change, command });
         Some(())
     }
 
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
@@ -135,6 +135,7 @@ pub(crate) fn extract_variable(acc: &mut Assists, ctx: &AssistContext<'_>) -> Op
                             }
                         }
                     }
+                    edit.rename();
                 }
                 Anchor::Replace(stmt) => {
                     cov_mark::hit!(test_extract_var_expr_stmt);
@@ -149,6 +150,7 @@ pub(crate) fn extract_variable(acc: &mut Assists, ctx: &AssistContext<'_>) -> Op
                             }
                         }
                     }
+                    edit.rename();
                 }
                 Anchor::WrapInBlock(to_wrap) => {
                     let indent_to = to_wrap.indent_level();
@@ -192,6 +194,7 @@ pub(crate) fn extract_variable(acc: &mut Assists, ctx: &AssistContext<'_>) -> Op
                             }
                         }
                     }
+                    edit.rename();
 
                     // fixup indentation of block
                     block.indent(indent_to);
diff --git a/src/tools/rust-analyzer/crates/ide-assists/src/tests.rs b/src/tools/rust-analyzer/crates/ide-assists/src/tests.rs
@@ -454,7 +454,7 @@ pub fn test_some_range(a: int) -> bool {
                 group: None,
                 target: 59..60,
                 source_change: None,
-                trigger_signature_help: false,
+                command: None,
             }
         "#]]
         .assert_debug_eq(&extract_into_variable_assist);
@@ -470,7 +470,7 @@ pub fn test_some_range(a: int) -> bool {
                 group: None,
                 target: 59..60,
                 source_change: None,
-                trigger_signature_help: false,
+                command: None,
             }
         "#]]
         .assert_debug_eq(&extract_into_function_assist);
@@ -500,7 +500,7 @@ pub fn test_some_range(a: int) -> bool {
                 group: None,
                 target: 59..60,
                 source_change: None,
-                trigger_signature_help: false,
+                command: None,
             }
         "#]]
         .assert_debug_eq(&extract_into_variable_assist);
@@ -516,7 +516,7 @@ pub fn test_some_range(a: int) -> bool {
                 group: None,
                 target: 59..60,
                 source_change: None,
-                trigger_signature_help: false,
+                command: None,
             }
         "#]]
         .assert_debug_eq(&extract_into_function_assist);
@@ -587,7 +587,9 @@ pub fn test_some_range(a: int) -> bool {
                         is_snippet: true,
                     },
                 ),
-                trigger_signature_help: false,
+                command: Some(
+                    Rename,
+                ),
             }
         "#]]
         .assert_debug_eq(&extract_into_variable_assist);
@@ -603,7 +605,7 @@ pub fn test_some_range(a: int) -> bool {
                 group: None,
                 target: 59..60,
                 source_change: None,
-                trigger_signature_help: false,
+                command: None,
             }
         "#]]
         .assert_debug_eq(&extract_into_function_assist);
@@ -666,7 +668,9 @@ pub fn test_some_range(a: int) -> bool {
                         is_snippet: true,
                     },
                 ),
-                trigger_signature_help: false,
+                command: Some(
+                    Rename,
+                ),
             }
         "#]]
         .assert_debug_eq(&extract_into_variable_assist);
@@ -715,7 +719,7 @@ pub fn test_some_range(a: int) -> bool {
                         is_snippet: true,
                     },
                 ),
-                trigger_signature_help: false,
+                command: None,
             }
         "#]]
         .assert_debug_eq(&extract_into_function_assist);
diff --git a/src/tools/rust-analyzer/crates/ide-db/src/assists.rs b/src/tools/rust-analyzer/crates/ide-db/src/assists.rs
@@ -29,7 +29,16 @@ pub struct Assist {
     /// cumbersome, especially if you want to embed an assist into another data
     /// structure, such as a diagnostic.
     pub source_change: Option<SourceChange>,
-    pub trigger_signature_help: bool,
+    /// The command to execute after the assist is applied.
+    pub command: Option<Command>,
+}
+
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub enum Command {
+    /// Show the parameter hints popup.
+    TriggerSignatureHelp,
+    /// Rename the just inserted item.
+    Rename,
 }
 
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
diff --git a/src/tools/rust-analyzer/crates/ide-db/src/source_change.rs b/src/tools/rust-analyzer/crates/ide-db/src/source_change.rs
@@ -5,7 +5,7 @@
 
 use std::{collections::hash_map::Entry, iter, mem};
 
-use crate::SnippetCap;
+use crate::{assists::Command, SnippetCap};
 use base_db::{AnchoredPathBuf, FileId};
 use itertools::Itertools;
 use nohash_hasher::IntMap;
@@ -194,7 +194,7 @@ pub struct SourceChangeBuilder {
     pub edit: TextEditBuilder,
     pub file_id: FileId,
     pub source_change: SourceChange,
-    pub trigger_signature_help: bool,
+    pub command: Option<Command>,
 
     /// Maps the original, immutable `SyntaxNode` to a `clone_for_update` twin.
     pub mutated_tree: Option<TreeMutator>,
@@ -236,7 +236,7 @@ impl SourceChangeBuilder {
             edit: TextEdit::builder(),
             file_id,
             source_change: SourceChange::default(),
-            trigger_signature_help: false,
+            command: None,
             mutated_tree: None,
             snippet_builder: None,
         }
@@ -304,8 +304,15 @@ impl SourceChangeBuilder {
         let file_system_edit = FileSystemEdit::MoveFile { src, dst };
         self.source_change.push_file_system_edit(file_system_edit);
     }
+
+    /// Triggers the parameter hint popup after the assist is applied
     pub fn trigger_signature_help(&mut self) {
-        self.trigger_signature_help = true;
+        self.command = Some(Command::TriggerSignatureHelp);
+    }
+
+    /// Renames the item at the cursor position after the assist is applied
+    pub fn rename(&mut self) {
+        self.command = Some(Command::Rename);
     }
 
     /// Adds a tabstop snippet to place the cursor before `node`
diff --git a/src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/trait_impl_redundant_assoc_item.rs b/src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/trait_impl_redundant_assoc_item.rs
@@ -98,7 +98,7 @@ fn quickfix_for_redundant_assoc_item(
         group: None,
         target: range,
         source_change: Some(source_change_builder.finish()),
-        trigger_signature_help: false,
+        command: None,
     }])
 }
 
diff --git a/src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/typed_hole.rs b/src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/typed_hole.rs
@@ -82,7 +82,7 @@ fn fixes(ctx: &DiagnosticsContext<'_>, d: &hir::TypedHole) -> Option<Vec<Assist>
                 original_range.file_id,
                 TextEdit::replace(original_range.range, code),
             )),
-            trigger_signature_help: false,
+            command: None,
         })
         .collect();
 
diff --git a/src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/unresolved_field.rs b/src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/unresolved_field.rs
@@ -130,7 +130,7 @@ fn add_variant_to_union(
         group: None,
         target: error_range.range,
         source_change: Some(src_change_builder.finish()),
-        trigger_signature_help: false,
+        command: None,
     })
 }
 
@@ -173,7 +173,7 @@ fn add_field_to_struct_fix(
                 group: None,
                 target: error_range.range,
                 source_change: Some(src_change_builder.finish()),
-                trigger_signature_help: false,
+                command: None,
             })
         }
         None => {
@@ -204,7 +204,7 @@ fn add_field_to_struct_fix(
                 group: None,
                 target: error_range.range,
                 source_change: Some(src_change_builder.finish()),
-                trigger_signature_help: false,
+                command: None,
             })
         }
         Some(FieldList::TupleFieldList(_tuple)) => {
@@ -266,7 +266,7 @@ fn method_fix(
             file_id,
             TextEdit::insert(range.end(), "()".to_owned()),
         )),
-        trigger_signature_help: false,
+        command: None,
     })
 }
 #[cfg(test)]
diff --git a/src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/unresolved_method.rs b/src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/unresolved_method.rs
@@ -108,7 +108,7 @@ fn field_fix(
             (file_id, TextEdit::insert(range.start(), "(".to_owned())),
             (file_id, TextEdit::insert(range.end(), ")".to_owned())),
         ])),
-        trigger_signature_help: false,
+        command: None,
     })
 }
 
@@ -191,7 +191,7 @@ fn assoc_func_fix(ctx: &DiagnosticsContext<'_>, d: &hir::UnresolvedMethodCall) -
                 file_id,
                 TextEdit::replace(range, assoc_func_call_expr_string),
             )),
-            trigger_signature_help: false,
+            command: None,
         })
     } else {
         None
diff --git a/src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/unused_variables.rs b/src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/unused_variables.rs
@@ -73,7 +73,7 @@ fn fixes(
             diagnostic_range.file_id,
             TextEdit::replace(name_range, format!("_{}", var_name.display(db))),
         )),
-        trigger_signature_help: false,
+        command: None,
     }])
 }
 
diff --git a/src/tools/rust-analyzer/crates/ide-diagnostics/src/lib.rs b/src/tools/rust-analyzer/crates/ide-diagnostics/src/lib.rs
@@ -613,7 +613,7 @@ fn unresolved_fix(id: &'static str, label: &str, target: TextRange) -> Assist {
         group: None,
         target,
         source_change: None,
-        trigger_signature_help: false,
+        command: None,
     }
 }
 
diff --git a/src/tools/rust-analyzer/crates/ide/src/ssr.rs b/src/tools/rust-analyzer/crates/ide/src/ssr.rs
@@ -45,7 +45,7 @@ pub(crate) fn ssr_assists(
             group: Some(GroupLabel("Apply SSR".into())),
             target: comment_range,
             source_change,
-            trigger_signature_help: false,
+            command: None,
         };
 
         ssr_assists.push(assist);
@@ -143,7 +143,7 @@ mod tests {
                         is_snippet: false,
                     },
                 ),
-                trigger_signature_help: false,
+                command: None,
             }
         "#]]
         .assert_debug_eq(&apply_in_file_assist);
@@ -196,7 +196,7 @@ mod tests {
                         is_snippet: false,
                     },
                 ),
-                trigger_signature_help: false,
+                command: None,
             }
         "#]]
         .assert_debug_eq(&apply_in_workspace_assist);
@@ -236,7 +236,7 @@ mod tests {
                 ),
                 target: 10..21,
                 source_change: None,
-                trigger_signature_help: false,
+                command: None,
             }
         "#]]
         .assert_debug_eq(&apply_in_file_assist);
@@ -256,7 +256,7 @@ mod tests {
                 ),
                 target: 10..21,
                 source_change: None,
-                trigger_signature_help: false,
+                command: None,
             }
         "#]]
         .assert_debug_eq(&apply_in_workspace_assist);
diff --git a/src/tools/rust-analyzer/crates/rust-analyzer/src/config.rs b/src/tools/rust-analyzer/crates/rust-analyzer/src/config.rs
@@ -1135,6 +1135,7 @@ pub struct ClientCommandsConfig {
     pub show_reference: bool,
     pub goto_location: bool,
     pub trigger_parameter_hints: bool,
+    pub rename: bool,
 }
 
 #[derive(Debug)]
@@ -1901,6 +1902,7 @@ impl Config {
             show_reference: get("rust-analyzer.showReferences"),
             goto_location: get("rust-analyzer.gotoLocation"),
             trigger_parameter_hints: get("editor.action.triggerParameterHints"),
+            rename: get("editor.action.rename"),
         }
     }
 
diff --git a/src/tools/rust-analyzer/crates/rust-analyzer/src/lsp/to_proto.rs b/src/tools/rust-analyzer/crates/rust-analyzer/src/lsp/to_proto.rs
@@ -13,7 +13,7 @@ use ide::{
     NavigationTarget, ReferenceCategory, RenameError, Runnable, Severity, SignatureHelp,
     SnippetEdit, SourceChange, StructureNodeKind, SymbolKind, TextEdit, TextRange, TextSize,
 };
-use ide_db::{rust_doc::format_docs, FxHasher};
+use ide_db::{assists, rust_doc::format_docs, FxHasher};
 use itertools::Itertools;
 use paths::{Utf8Component, Utf8Prefix};
 use semver::VersionReq;
@@ -1336,8 +1336,14 @@ pub(crate) fn code_action(
         command: None,
     };
 
-    if assist.trigger_signature_help && snap.config.client_commands().trigger_parameter_hints {
+    if assist.command == Some(assists::Command::TriggerSignatureHelp)
+        && snap.config.client_commands().trigger_parameter_hints
+    {
         res.command = Some(command::trigger_parameter_hints());
+    } else if assist.command == Some(assists::Command::Rename)
+        && snap.config.client_commands().rename
+    {
+        res.command = Some(command::rename());
     }
 
     match (assist.source_change, resolve_data) {
@@ -1715,6 +1721,14 @@ pub(crate) mod command {
             arguments: None,
         }
     }
+
+    pub(crate) fn rename() -> lsp_types::Command {
+        lsp_types::Command {
+            title: "rename".into(),
+            command: "rust-analyzer.rename".into(),
+            arguments: None,
+        }
+    }
 }
 
 pub(crate) fn implementation_title(count: usize) -> String {
diff --git a/src/tools/rust-analyzer/editors/code/src/client.ts b/src/tools/rust-analyzer/editors/code/src/client.ts
@@ -348,6 +348,7 @@ class ExperimentalFeatures implements lc.StaticFeature {
                     "rust-analyzer.showReferences",
                     "rust-analyzer.gotoLocation",
                     "editor.action.triggerParameterHints",
+                    "editor.action.rename",
                 ],
             },
             ...capabilities.experimental,
diff --git a/src/tools/rust-analyzer/editors/code/src/commands.ts b/src/tools/rust-analyzer/editors/code/src/commands.ts
@@ -118,6 +118,12 @@ export function triggerParameterHints(_: CtxInit): Cmd {
     };
 }
 
+export function rename(_: CtxInit): Cmd {
+    return async () => {
+        await vscode.commands.executeCommand("editor.action.rename");
+    };
+}
+
 export function openLogs(ctx: CtxInit): Cmd {
     return async () => {
         if (ctx.client.outputChannel) {
diff --git a/src/tools/rust-analyzer/editors/code/src/main.ts b/src/tools/rust-analyzer/editors/code/src/main.ts

Original file line number	Diff line number	Diff line change
`@@ -135,6 +135,7 @@ pub(crate) fn extract_variable(acc: &mut Assists, ctx: &AssistContext<'_>) -> Op`
`135`	`135`	`}`
`136`	`136`	`}`
`137`	`137`	`}`
	`138`	`+ edit.rename();`
`138`	`139`	`}`
`139`	`140`	`Anchor::Replace(stmt) => {`
`140`	`141`	`cov_mark::hit!(test_extract_var_expr_stmt);`
`@@ -149,6 +150,7 @@ pub(crate) fn extract_variable(acc: &mut Assists, ctx: &AssistContext<'_>) -> Op`
`149`	`150`	`}`
`150`	`151`	`}`
`151`	`152`	`}`
	`153`	`+ edit.rename();`
`152`	`154`	`}`
`153`	`155`	`Anchor::WrapInBlock(to_wrap) => {`
`154`	`156`	`let indent_to = to_wrap.indent_level();`
`@@ -192,6 +194,7 @@ pub(crate) fn extract_variable(acc: &mut Assists, ctx: &AssistContext<'_>) -> Op`
`192`	`194`	`}`
`193`	`195`	`}`
`194`	`196`	`}`
	`197`	`+ edit.rename();`
`195`	`198`
`196`	`199`	`// fixup indentation of block`
`197`	`200`	`block.indent(indent_to);`
Original file line number	Diff line number	Diff line change
`@@ -98,7 +98,7 @@ fn quickfix_for_redundant_assoc_item(`
`98`	`98`	`group: None,`
`99`	`99`	`target: range,`
`100`	`100`	`source_change: Some(source_change_builder.finish()),`
`101`		`- trigger_signature_help: false,`
	`101`	`+ command: None,`
`102`	`102`	`}])`
`103`	`103`	`}`
`104`	`104`
Original file line number	Diff line number	Diff line change
`@@ -73,7 +73,7 @@ fn fixes(`
`73`	`73`	`diagnostic_range.file_id,`
`74`	`74`	`TextEdit::replace(name_range, format!("_{}", var_name.display(db))),`
`75`	`75`	`)),`
`76`		`- trigger_signature_help: false,`
	`76`	`+ command: None,`
`77`	`77`	`}])`
`78`	`78`	`}`
`79`	`79`