Repository: rust-lang/rust-analyzer
Pull request #12040 (2022-04-20 08:59:23)

fix: Improve parameter completion
fix https://github.com/rust-lang/rust-analyzer/issues/12016 and handles some extra cases. 


Patch:
diff --git a/crates/ide/src/ssr.rs b/crates/ide/src/ssr.rs
index ba9a7f62e6..6142a0eafd 100644
--- a/crates/ide/src/ssr.rs
+++ b/crates/ide/src/ssr.rs
@@ -45,7 +45,6 @@ pub(crate) fn ssr_assists(
             group: Some(GroupLabel("Apply SSR".into())),
             target: comment_range,
             source_change,
-            trigger_signature_help: false,
         };
 
         ssr_assists.push(assist);
@@ -141,7 +140,6 @@ mod tests {
                         is_snippet: false,
                     },
                 ),
-                trigger_signature_help: false,
             }
         "#]]
         .assert_debug_eq(&apply_in_file_assist);
@@ -188,7 +186,6 @@ mod tests {
                         is_snippet: false,
                     },
                 ),
-                trigger_signature_help: false,
             }
         "#]]
         .assert_debug_eq(&apply_in_workspace_assist);
@@ -228,7 +225,6 @@ mod tests {
                 ),
                 target: 10..21,
                 source_change: None,
-                trigger_signature_help: false,
             }
         "#]]
         .assert_debug_eq(&apply_in_file_assist);
@@ -248,7 +244,6 @@ mod tests {
                 ),
                 target: 10..21,
                 source_change: None,
-                trigger_signature_help: false,
             }
         "#]]
         .assert_debug_eq(&apply_in_workspace_assist);
diff --git a/crates/ide_assists/src/assist_context.rs b/crates/ide_assists/src/assist_context.rs
index ec85faaf46..fd46aa2479 100644
--- a/crates/ide_assists/src/assist_context.rs
+++ b/crates/ide_assists/src/assist_context.rs
@@ -193,11 +193,9 @@ impl Assists {
             return None;
         }
 
-        let mut trigger_signature_help = false;
         let source_change = if self.resolve.should_resolve(&id) {
             let mut builder = AssistBuilder::new(self.file);
             f(&mut builder);
-            trigger_signature_help = builder.trigger_signature_help;
             Some(builder.finish())
         } else {
             None
@@ -205,7 +203,7 @@ impl Assists {
 
         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 });
         Some(())
     }
 
@@ -221,7 +219,6 @@ pub(crate) struct AssistBuilder {
     edit: TextEditBuilder,
     file_id: FileId,
     source_change: SourceChange,
-    trigger_signature_help: bool,
 
     /// Maps the original, immutable `SyntaxNode` to a `clone_for_update` twin.
     mutated_tree: Option<TreeMutator>,
@@ -255,7 +252,6 @@ impl AssistBuilder {
             edit: TextEdit::builder(),
             file_id,
             source_change: SourceChange::default(),
-            trigger_signature_help: false,
             mutated_tree: None,
         }
     }
@@ -336,9 +332,6 @@ impl AssistBuilder {
         let file_system_edit = FileSystemEdit::MoveFile { src, dst };
         self.source_change.push_file_system_edit(file_system_edit);
     }
-    pub(crate) fn trigger_signature_help(&mut self) {
-        self.trigger_signature_help = true;
-    }
 
     fn finish(mut self) -> SourceChange {
         self.commit();
diff --git a/crates/ide_assists/src/handlers/add_turbo_fish.rs b/crates/ide_assists/src/handlers/add_turbo_fish.rs
index a82dca9db0..c3d27f7ea6 100644
--- a/crates/ide_assists/src/handlers/add_turbo_fish.rs
+++ b/crates/ide_assists/src/handlers/add_turbo_fish.rs
@@ -89,18 +89,15 @@ pub(crate) fn add_turbo_fish(acc: &mut Assists, ctx: &AssistContext) -> Option<(
         AssistId("add_turbo_fish", AssistKind::RefactorRewrite),
         "Add `::<>`",
         ident.text_range(),
-        |builder| {
-            builder.trigger_signature_help();
-            match ctx.config.snippet_cap {
-                Some(cap) => {
-                    let snip = format!("::<{}>", get_snippet_fish_head(number_of_arguments));
-                    builder.insert_snippet(cap, ident.text_range().end(), snip)
-                }
-                None => {
-                    let fish_head = std::iter::repeat("_").take(number_of_arguments).format(", ");
-                    let snip = format!("::<{}>", fish_head);
-                    builder.insert(ident.text_range().end(), snip);
-                }
+        |builder| match ctx.config.snippet_cap {
+            Some(cap) => {
+                let snip = format!("::<{}>", get_snippet_fish_head(number_of_arguments));
+                builder.insert_snippet(cap, ident.text_range().end(), snip)
+            }
+            None => {
+                let fish_head = std::iter::repeat("_").take(number_of_arguments).format(", ");
+                let snip = format!("::<{}>", fish_head);
+                builder.insert(ident.text_range().end(), snip);
             }
         },
     )
diff --git a/crates/ide_completion/src/completions/fn_param.rs b/crates/ide_completion/src/completions/fn_param.rs
index 7e9c1f49de..ae09339e95 100644
--- a/crates/ide_completion/src/completions/fn_param.rs
+++ b/crates/ide_completion/src/completions/fn_param.rs
@@ -5,7 +5,7 @@ use rustc_hash::FxHashMap;
 use syntax::{
     algo,
     ast::{self, HasModuleItem},
-    match_ast, AstNode, Direction, SyntaxKind,
+    match_ast, AstNode, Direction, SyntaxKind, TextRange,
 };
 
 use crate::{
@@ -27,14 +27,13 @@ pub(crate) fn complete_fn_param(acc: &mut Completions, ctx: &CompletionContext)
 
     let comma_wrapper = comma_wrapper(ctx);
     let mut add_new_item_to_acc = |label: &str, lookup: String| {
-        let mk_item = |label: &str| {
-            CompletionItem::new(CompletionItemKind::Binding, ctx.source_range(), label)
+        let mk_item = |label: &str, range: TextRange| {
+            CompletionItem::new(CompletionItemKind::Binding, range, label)
         };
-        let mut item = match &comma_wrapper {
-            Some(fmt) => mk_item(&fmt(label)),
-            None => mk_item(label),
+        let item = match &comma_wrapper {
+            Some((fmt, range, lookup)) => mk_item(&fmt(label), *range).lookup_by(lookup).to_owned(),
+            None => mk_item(label, ctx.source_range()).lookup_by(lookup).to_owned(),
         };
-        item.lookup_by(lookup);
         item.add_to(acc)
     };
 
@@ -162,14 +161,16 @@ fn should_add_self_completions(ctx: &CompletionContext, param_list: &ast::ParamL
     inside_impl && no_params
 }
 
-fn comma_wrapper(ctx: &CompletionContext) -> Option<impl Fn(&str) -> String> {
+fn comma_wrapper(ctx: &CompletionContext) -> Option<(impl Fn(&str) -> String, TextRange, String)> {
+    let param = ctx.token.ancestors().find(|node| node.kind() == SyntaxKind::PARAM)?;
+
     let next_token_kind = {
-        let t = ctx.token.next_token()?;
+        let t = param.last_token()?.next_token()?;
         let t = algo::skip_whitespace_token(t, Direction::Next)?;
         t.kind()
     };
     let prev_token_kind = {
-        let t = ctx.previous_token.clone()?;
+        let t = param.first_token()?.prev_token()?;
         let t = algo::skip_whitespace_token(t, Direction::Prev)?;
         t.kind()
     };
@@ -182,5 +183,9 @@ fn comma_wrapper(ctx: &CompletionContext) -> Option<impl Fn(&str) -> String> {
         matches!(prev_token_kind, SyntaxKind::COMMA | SyntaxKind::L_PAREN | SyntaxKind::PIPE);
     let leading = if has_leading_comma { "" } else { ", " };
 
-    Some(move |param: &_| format!("{}{}{}", leading, param, trailing))
+    Some((
+        move |label: &_| (format!("{}{}{}", leading, label, trailing)),
+        param.text_range(),
+        format!("{}{}", leading, param.text()),
+    ))
 }
diff --git a/crates/ide_completion/src/render/function.rs b/crates/ide_completion/src/render/function.rs
index 9e5f91db39..bd6833567c 100644
--- a/crates/ide_completion/src/render/function.rs
+++ b/crates/ide_completion/src/render/function.rs
@@ -562,6 +562,90 @@ fn qux(Foo { bar }: Foo) {
 fn main() {
   qux(${1:foo})$0
 }
+"#,
+        );
+    }
+
+    #[test]
+    fn complete_fn_param() {
+        // has mut kw
+        check_edit(
+            "mut ba",
+            r#"
+fn f(foo: (), mut bar: u32) {}
+fn g(foo: (), mut ba$0)
+"#,
+            r#"
+fn f(foo: (), mut bar: u32) {}
+fn g(foo: (), mut bar: u32)
+"#,
+        );
+
+        // has type param
+        check_edit(
+            "mut ba: u32",
+            r#"
+fn g(foo: (), mut ba$0: u32)
+fn f(foo: (), mut bar: u32) {}
+"#,
+            r#"
+fn g(foo: (), mut bar: u32)
+fn f(foo: (), mut bar: u32) {}
+"#,
+        );
+    }
+
+    #[test]
+    fn complete_fn_mut_param_add_comma() {
+        // add leading and trailing comma
+        check_edit(
+            ", mut ba",
+            r#"
+fn f(foo: (), mut bar: u32) {}
+fn g(foo: ()mut ba$0 baz: ())
+"#,
+            r#"
+fn f(foo: (), mut bar: u32) {}
+fn g(foo: (), mut bar: u32, baz: ())
+"#,
+        );
+    }
+
+    #[test]
+    fn complete_fn_mut_param_has_attribute() {
+        check_edit(
+            "mut ba",
+            r#"
+fn f(foo: (), #[baz = "qux"] mut bar: u32) {}
+fn g(foo: (), mut ba$0)
+"#,
+            r#"
+fn f(foo: (), #[baz = "qux"] mut bar: u32) {}
+fn g(foo: (), #[baz = "qux"] mut bar: u32)
+"#,
+        );
+
+        check_edit(
+            r#"#[baz = "qux"] mut ba"#,
+            r#"
+fn f(foo: (), #[baz = "qux"] mut bar: u32) {}
+fn g(foo: (), #[baz = "qux"] mut ba$0)
+"#,
+            r#"
+fn f(foo: (), #[baz = "qux"] mut bar: u32) {}
+fn g(foo: (), #[baz = "qux"] mut bar: u32)
+"#,
+        );
+
+        check_edit(
+            r#", #[baz = "qux"] mut ba"#,
+            r#"
+fn f(foo: (), #[baz = "qux"] mut bar: u32) {}
+fn g(foo: ()#[baz = "qux"] mut ba$0)
+"#,
+            r#"
+fn f(foo: (), #[baz = "qux"] mut bar: u32) {}
+fn g(foo: (), #[baz = "qux"] mut bar: u32)
 "#,
         );
     }
diff --git a/crates/ide_db/src/assists.rs b/crates/ide_db/src/assists.rs
index da23763dc2..be5f063f0f 100644
--- a/crates/ide_db/src/assists.rs
+++ b/crates/ide_db/src/assists.rs
@@ -29,7 +29,6 @@ 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,
 }
 
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
diff --git a/crates/ide_diagnostics/src/handlers/unresolved_module.rs b/crates/ide_diagnostics/src/handlers/unresolved_module.rs
index b8f2a9e94a..587a3ee26d 100644
--- a/crates/ide_diagnostics/src/handlers/unresolved_module.rs
+++ b/crates/ide_diagnostics/src/handlers/unresolved_module.rs
@@ -117,7 +117,6 @@ mod baz {}
                                             is_snippet: false,
                                         },
                                     ),
-                                    trigger_signature_help: false,
                                 },
                                 Assist {
                                     id: AssistId(
@@ -144,7 +143,6 @@ mod baz {}
                                             is_snippet: false,
                                         },
                                     ),
-                                    trigger_signature_help: false,
                                 },
                             ],
                         ),
diff --git a/crates/ide_diagnostics/src/lib.rs b/crates/ide_diagnostics/src/lib.rs
index b04da2e092..13ac0a2450 100644
--- a/crates/ide_diagnostics/src/lib.rs
+++ b/crates/ide_diagnostics/src/lib.rs
@@ -238,6 +238,5 @@ fn unresolved_fix(id: &'static str, label: &str, target: TextRange) -> Assist {
         group: None,
         target,
         source_change: None,
-        trigger_signature_help: false,
     }
 }
diff --git a/crates/rust-analyzer/src/diagnostics/to_proto.rs b/crates/rust-analyzer/src/diagnostics/to_proto.rs
index e9a192cdfb..45e46c1a06 100644
--- a/crates/rust-analyzer/src/diagnostics/to_proto.rs
+++ b/crates/rust-analyzer/src/diagnostics/to_proto.rs
@@ -204,7 +204,6 @@ fn map_rust_child_diagnostic(
                     }),
                     is_preferred: Some(true),
                     data: None,
-                    command: None,
                 },
             }),
         })
diff --git a/crates/rust-analyzer/src/handlers.rs b/crates/rust-analyzer/src/handlers.rs
index a1b71ab143..fb64eeea4f 100644
--- a/crates/rust-analyzer/src/handlers.rs
+++ b/crates/rust-analyzer/src/handlers.rs
@@ -1160,9 +1160,8 @@ pub(crate) fn handle_code_action_resolve(
         ))
         .into());
     }
-    let ca = to_proto::code_action(&snap, assist.clone(), None)?;
-    code_action.edit = ca.edit;
-    code_action.command = ca.command;
+    let edit = to_proto::code_action(&snap, assist.clone(), None)?.edit;
+    code_action.edit = edit;
     Ok(code_action)
 }
 
diff --git a/crates/rust-analyzer/src/lsp_ext.rs b/crates/rust-analyzer/src/lsp_ext.rs
index 1cfaa13327..489a233a0a 100644
--- a/crates/rust-analyzer/src/lsp_ext.rs
+++ b/crates/rust-analyzer/src/lsp_ext.rs
@@ -311,8 +311,9 @@ pub struct CodeAction {
     pub group: Option<String>,
     #[serde(skip_serializing_if = "Option::is_none")]
     pub kind: Option<CodeActionKind>,
-    #[serde(skip_serializing_if = "Option::is_none")]
-    pub command: Option<lsp_types::Command>,
+    // We don't handle commands on the client-side
+    // #[serde(skip_serializing_if = "Option::is_none")]
+    // pub command: Option<lsp_types::Command>,
     #[serde(skip_serializing_if = "Option::is_none")]
     pub edit: Option<SnippetWorkspaceEdit>,
     #[serde(skip_serializing_if = "Option::is_none")]
diff --git a/crates/rust-analyzer/src/to_proto.rs b/crates/rust-analyzer/src/to_proto.rs
index 57b0e93765..deea54cf7d 100644
--- a/crates/rust-analyzer/src/to_proto.rs
+++ b/crates/rust-analyzer/src/to_proto.rs
@@ -1018,13 +1018,7 @@ pub(crate) fn code_action(
         edit: None,
         is_preferred: None,
         data: None,
-        command: None,
     };
-
-    if assist.trigger_signature_help && snap.config.client_commands().trigger_parameter_hints {
-        res.command = Some(command::trigger_parameter_hints());
-    }
-
     match (assist.source_change, resolve_data) {
         (Some(it), _) => res.edit = Some(snippet_workspace_edit(snap, it)?),
         (None, Some((index, code_action_params))) => {
diff --git a/docs/dev/lsp-extensions.md b/docs/dev/lsp-extensions.md
index 2e1cee8955..46193a6ff3 100644
--- a/docs/dev/lsp-extensions.md
+++ b/docs/dev/lsp-extensions.md
@@ -1,5 +1,5 @@
 <!---
-lsp_ext.rs hash: 7a34bc3f38e2a7d8
+lsp_ext.rs hash: 326ad62235135223
 
 If you need to change the above hash to make the test pass, please check if you
 need to adjust this doc as well and ping this issue:
diff --git a/editors/code/package.json b/editors/code/package.json
index cd0ad92d9a..60c827e31a 100644
--- a/editors/code/package.json
+++ b/editors/code/package.json
@@ -293,6 +293,11 @@
                     "default": null,
                     "markdownDescription": "Environment variables passed to the runnable launched using `Test` or `Debug` lens or `rust-analyzer.run` command."
                 },
+                "rust-analyzer.inlayHints.enable": {
+                    "type": "boolean",
+                    "default": true,
+                    "description": "Whether to show inlay hints."
+                },
                 "rust-analyzer.server.path": {
                     "type": [
                         "null",
diff --git a/editors/code/src/commands.ts b/editors/code/src/commands.ts
index 1e89938c05..18f5b3aa10 100644
--- a/editors/code/src/commands.ts
+++ b/editors/code/src/commands.ts
@@ -300,9 +300,10 @@ export function serverVersion(ctx: Ctx): Cmd {
 
 export function toggleInlayHints(_ctx: Ctx): Cmd {
     return async () => {
-        const config = vscode.workspace.getConfiguration("editor.inlayHints", { languageId: "rust" });
+        const scope = vscode.ConfigurationTarget.Global;
+        const config = vscode.workspace.getConfiguration("editor.inlayHints");
         const value = !config.get("enabled");
-        await config.update('enabled', value, vscode.ConfigurationTarget.Global);
+        await config.update('enabled', value, scope);
     };
 }
 
@@ -734,9 +735,6 @@ export function resolveCodeAction(ctx: Ctx): Cmd {
         const fileSystemEdit = await client.protocol2CodeConverter.asWorkspaceEdit(lcFileSystemEdit);
         await vscode.workspace.applyEdit(fileSystemEdit);
         await applySnippetWorkspaceEdit(edit);
-        if (item.command != null) {
-            await vscode.commands.executeCommand(item.command.command, item.command.arguments);
-        }
     };
 }
 


Test patch:
diff --git a/crates/ide_assists/src/tests.rs b/crates/ide_assists/src/tests.rs
index 6b57f9962c..b3a0bd2773 100644
--- a/crates/ide_assists/src/tests.rs
+++ b/crates/ide_assists/src/tests.rs
@@ -341,7 +341,6 @@ pub fn test_some_range(a: int) -> bool {
                 group: None,
                 target: 59..60,
                 source_change: None,
-                trigger_signature_help: false,
             }
         "#]]
         .assert_debug_eq(&extract_into_variable_assist);
@@ -357,7 +356,6 @@ pub fn test_some_range(a: int) -> bool {
                 group: None,
                 target: 59..60,
                 source_change: None,
-                trigger_signature_help: false,
             }
         "#]]
         .assert_debug_eq(&extract_into_function_assist);
@@ -387,7 +385,6 @@ pub fn test_some_range(a: int) -> bool {
                 group: None,
                 target: 59..60,
                 source_change: None,
-                trigger_signature_help: false,
             }
         "#]]
         .assert_debug_eq(&extract_into_variable_assist);
@@ -403,7 +400,6 @@ pub fn test_some_range(a: int) -> bool {
                 group: None,
                 target: 59..60,
                 source_change: None,
-                trigger_signature_help: false,
             }
         "#]]
         .assert_debug_eq(&extract_into_function_assist);
@@ -454,7 +450,6 @@ pub fn test_some_range(a: int) -> bool {
                         is_snippet: true,
                     },
                 ),
-                trigger_signature_help: false,
             }
         "#]]
         .assert_debug_eq(&extract_into_variable_assist);
@@ -470,7 +465,6 @@ pub fn test_some_range(a: int) -> bool {
                 group: None,
                 target: 59..60,
                 source_change: None,
-                trigger_signature_help: false,
             }
         "#]]
         .assert_debug_eq(&extract_into_function_assist);
@@ -513,7 +507,6 @@ pub fn test_some_range(a: int) -> bool {
                         is_snippet: true,
                     },
                 ),
-                trigger_signature_help: false,
             }
         "#]]
         .assert_debug_eq(&extract_into_variable_assist);
@@ -550,7 +543,6 @@ pub fn test_some_range(a: int) -> bool {
                         is_snippet: true,
                     },
                 ),
-                trigger_signature_help: false,
             }
         "#]]
         .assert_debug_eq(&extract_into_function_assist);
diff --git a/crates/ide_completion/src/tests/fn_param.rs b/crates/ide_completion/src/tests/fn_param.rs
index 779ec0c3a7..678c99aa7a 100644
--- a/crates/ide_completion/src/tests/fn_param.rs
+++ b/crates/ide_completion/src/tests/fn_param.rs
@@ -232,3 +232,16 @@ fn bar(bar$0) {}
         "#]],
     )
 }
+
+#[test]
+fn completes_for_params_with_attributes() {
+    check(
+        r#"
+fn f(foo: (), #[baz = "qux"] mut bar: u32) {}
+fn g(foo: (), #[baz = "qux"] mut ba$0)
+"#,
+        expect![[r##"
+            bn #[baz = "qux"] mut bar: u32
+        "##]],
+    )
+}
diff --git a/crates/rust-analyzer/src/diagnostics/test_data/clippy_pass_by_ref.txt b/crates/rust-analyzer/src/diagnostics/test_data/clippy_pass_by_ref.txt
index 41c509452a..4610984acb 100644
--- a/crates/rust-analyzer/src/diagnostics/test_data/clippy_pass_by_ref.txt
+++ b/crates/rust-analyzer/src/diagnostics/test_data/clippy_pass_by_ref.txt
@@ -318,7 +318,6 @@
                             "quickfix",
                         ),
                     ),
-                    command: None,
                     edit: Some(
                         SnippetWorkspaceEdit {
                             changes: Some(
diff --git a/crates/rust-analyzer/src/diagnostics/test_data/rustc_unused_variable.txt b/crates/rust-analyzer/src/diagnostics/test_data/rustc_unused_variable.txt
index 1c5c336223..46d44192c5 100644
--- a/crates/rust-analyzer/src/diagnostics/test_data/rustc_unused_variable.txt
+++ b/crates/rust-analyzer/src/diagnostics/test_data/rustc_unused_variable.txt
@@ -165,7 +165,6 @@
                             "quickfix",
                         ),
                     ),
-                    command: None,
                     edit: Some(
                         SnippetWorkspaceEdit {
                             changes: Some(
diff --git a/crates/rust-analyzer/src/diagnostics/test_data/rustc_unused_variable_as_hint.txt b/crates/rust-analyzer/src/diagnostics/test_data/rustc_unused_variable_as_hint.txt
index 3ab3412d97..b595487e8f 100644
--- a/crates/rust-analyzer/src/diagnostics/test_data/rustc_unused_variable_as_hint.txt
+++ b/crates/rust-analyzer/src/diagnostics/test_data/rustc_unused_variable_as_hint.txt
@@ -165,7 +165,6 @@
                             "quickfix",
                         ),
                     ),
-                    command: None,
                     edit: Some(
                         SnippetWorkspaceEdit {
                             changes: Some(
diff --git a/crates/rust-analyzer/src/diagnostics/test_data/rustc_unused_variable_as_info.txt b/crates/rust-analyzer/src/diagnostics/test_data/rustc_unused_variable_as_info.txt
index 0702420aa5..3ee50392a7 100644
--- a/crates/rust-analyzer/src/diagnostics/test_data/rustc_unused_variable_as_info.txt
+++ b/crates/rust-analyzer/src/diagnostics/test_data/rustc_unused_variable_as_info.txt
@@ -165,7 +165,6 @@
                             "quickfix",
                         ),
                     ),
-                    command: None,
                     edit: Some(
                         SnippetWorkspaceEdit {
                             changes: Some(
diff --git a/crates/rust-analyzer/src/diagnostics/test_data/snap_multi_line_fix.txt b/crates/rust-analyzer/src/diagnostics/test_data/snap_multi_line_fix.txt
index 4365e450df..d7987f65ed 100644
--- a/crates/rust-analyzer/src/diagnostics/test_data/snap_multi_line_fix.txt
+++ b/crates/rust-analyzer/src/diagnostics/test_data/snap_multi_line_fix.txt
@@ -328,7 +328,6 @@
                             "quickfix",
                         ),
                     ),
-                    command: None,
                     edit: Some(
                         SnippetWorkspaceEdit {
                             changes: Some(