Auto merge of rust-lang#17587 - joshka:jm/edit-name-after-refactor, r=Veykril

bors · bors · commit 82d0187cf81a · 2024-07-15T09:53:39.000Z
Trigger VSCode to rename after extract variable assist is applied 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: rust-lang#17579 https://github.com/user-attachments/assets/4cf38740-ab22-4b94-b0f1-eddd51c26c29 I haven't yet looked at the module or function extraction assists yet.
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
@@ -197,6 +197,7 @@ pub(crate) fn extract_variable(acc: &mut Assists, ctx: &AssistContext<'_>) -> Op
                     block.indent(indent_to);
                 }
             }
+            edit.rename();
         },
     )
 }
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
@@ -1128,13 +1128,14 @@ pub struct WorkspaceSymbolConfig {
     /// How many items are returned at most.
     pub search_limit: usize,
 }
-
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
 pub struct ClientCommandsConfig {
     pub run_single: bool,
     pub debug_single: bool,
     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,9 +1336,14 @@ pub(crate) fn code_action(
         command: None,
     };
 
-    if assist.trigger_signature_help && snap.config.client_commands().trigger_parameter_hints {
-        res.command = Some(command::trigger_parameter_hints());
-    }
+    let commands = snap.config.client_commands();
+    res.command = match assist.command {
+        Some(assists::Command::TriggerSignatureHelp) if commands.trigger_parameter_hints => {
+            Some(command::trigger_parameter_hints())
+        }
+        Some(assists::Command::Rename) if commands.rename => Some(command::rename()),
+        _ => None,
+    };
 
     match (assist.source_change, resolve_data) {
         (Some(it), _) => res.edit = Some(snippet_workspace_edit(snap, it)?),
@@ -1715,6 +1720,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
@@ -190,6 +190,7 @@ function createCommands(): Record<string, CommandFactory> {
         runSingle: { enabled: commands.runSingle },
         showReferences: { enabled: commands.showReferences },
         triggerParameterHints: { enabled: commands.triggerParameterHints },
+        rename: { enabled: commands.rename },
         openLogs: { enabled: commands.openLogs },
         revealDependency: { enabled: commands.revealDependency },
     };

Original file line number	Diff line number	Diff line change
`@@ -197,6 +197,7 @@ pub(crate) fn extract_variable(acc: &mut Assists, ctx: &AssistContext<'_>) -> Op`
`197`	`197`	`block.indent(indent_to);`
`198`	`198`	`}`
`199`	`199`	`}`
	`200`	`+ edit.rename();`
`200`	`201`	`},`
`201`	`202`	`)`
`202`	`203`	`}`
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`