Skip to content

Commit

Permalink
feat: LSP signature help (#5725)
Browse files Browse the repository at this point in the history
# Description

## Problem

Resolves #1585

## Summary

While working on autocompletion I was always checking what Rust Analyzer
did, or how it worked, and I noticed that in Rust Analyzer when it
autocompletes a function a little popup with the parameter names and
types would show up, which is really useful! But it turned out that's a
separate feature: signature help.

This PR implements that. I think it makes autocompletion much better. 

Before:


![lsp-signature-help-before](https://github.com/user-attachments/assets/2480e9bf-ae17-47a6-be0a-bcc58f2b2b92)

After:


![lsp-signature-help-after](https://github.com/user-attachments/assets/e50573d0-17cb-4c3a-b50b-a4fed462acdf)

Note: in the latest version the popup text starts with "fn ", but I
didn't want to re-record all the gifs 😅 .

You basically don't have to remember what were the parameter types.

It also works with methods:


![lsp-signature-help-method](https://github.com/user-attachments/assets/72fe65ee-2c68-4463-aa75-fd304fcbe50c)

And if you have an `fn`:


![lsp-signature-help-fn](https://github.com/user-attachments/assets/d29001e1-5de4-47f1-aede-da01bc1a3c53)

And here it's working in an aztec project (I always check that it works
outside of small toy programs 😊):


![lsp-signature-help-aztec](https://github.com/user-attachments/assets/3aa1d395-09bc-4578-b56d-c0e90630c4da)

## Additional Context

I promise this will be the last big LSP PR I send 🤞 . Now that we have
most of the LSP features registered, next PRs should be much smaller,
just adding on top of what there is.

These PRs are also relatively big because they traverse the AST and that
code is kind of boilerplatey. For that reason here I decided to put the
boilerplatey code in a `traversal.rs` file, so that it's clear there's
no extra logic there other than traversing the AST. I might send a
follow-up PR doing the same for autocompletion.

One more thing: you can trigger this functionality as a VSCode command
(Trigger Parameter Hints). But then when offering autocompletion you can
also tell VSCode to execute a command, and in this PR we tell it to
execute exactly that command (Rust Analyzer does the same, though they
have a custom command for it, not sure why).

UPDATE: I managed to implement the visitor pattern to reduce the
boilerplate. [Here's a PR](#5727)
with that. I think it will greatly simplify things, and make it easier
to keep the code updated as we add more nodes. It takes inspiration in
[how it's done in the Crystal
compiler](https://github.com/crystal-lang/crystal/blob/master/src/compiler/crystal/syntax/visitor.cr),
which in turn is inspired by how it was done in the Java editor support
for Eclipse 😄

## Documentation\*

Check one:
- [x] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[For Experimental Features]** Documentation to be submitted in a
separate PR.

# PR Checklist\*

- [x] I have tested the changes locally.
- [x] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.

---------

Co-authored-by: jfecher <jake@aztecprotocol.com>
  • Loading branch information
asterite and jfecher authored Aug 15, 2024
1 parent 5730e67 commit 5a3d241
Show file tree
Hide file tree
Showing 8 changed files with 885 additions and 37 deletions.
5 changes: 3 additions & 2 deletions tooling/lsp/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ use fxhash::FxHashSet;
use lsp_types::{
request::{
Completion, DocumentSymbolRequest, HoverRequest, InlayHintRequest, PrepareRenameRequest,
References, Rename,
References, Rename, SignatureHelpRequest,
},
CodeLens,
};
Expand Down Expand Up @@ -55,7 +55,7 @@ use requests::{
on_goto_declaration_request, on_goto_definition_request, on_goto_type_definition_request,
on_hover_request, on_initialize, on_inlay_hint_request, on_prepare_rename_request,
on_profile_run_request, on_references_request, on_rename_request, on_shutdown,
on_test_run_request, on_tests_request, LspInitializationOptions,
on_signature_help_request, on_test_run_request, on_tests_request, LspInitializationOptions,
};
use serde_json::Value as JsonValue;
use thiserror::Error;
Expand Down Expand Up @@ -143,6 +143,7 @@ impl NargoLspService {
.request::<HoverRequest, _>(on_hover_request)
.request::<InlayHintRequest, _>(on_inlay_hint_request)
.request::<Completion, _>(on_completion_request)
.request::<SignatureHelpRequest, _>(on_signature_help_request)
.notification::<notification::Initialized>(on_initialized)
.notification::<notification::DidChangeConfiguration>(on_did_change_configuration)
.notification::<notification::DidOpenTextDocument>(on_did_open_text_document)
Expand Down
24 changes: 23 additions & 1 deletion tooling/lsp/src/requests/completion/completion_items.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
use lsp_types::{CompletionItem, CompletionItemKind, CompletionItemLabelDetails, InsertTextFormat};
use lsp_types::{
Command, CompletionItem, CompletionItemKind, CompletionItemLabelDetails, InsertTextFormat,
};
use noirc_frontend::{
hir_def::{function::FuncMeta, stmt::HirPattern},
macros_api::{ModuleDefId, StructId},
Expand Down Expand Up @@ -164,6 +166,13 @@ impl<'a> NodeFinder<'a> {
completion_item
};

let completion_item = match function_completion_kind {
FunctionCompletionKind::Name => completion_item,
FunctionCompletionKind::NameAndParameters => {
completion_item_with_trigger_parameter_hints_command(completion_item)
}
};

Some(completion_item)
}

Expand Down Expand Up @@ -342,3 +351,16 @@ pub(super) fn completion_item_with_sort_text(
) -> CompletionItem {
CompletionItem { sort_text: Some(sort_text), ..completion_item }
}

pub(super) fn completion_item_with_trigger_parameter_hints_command(
completion_item: CompletionItem,
) -> CompletionItem {
CompletionItem {
command: Some(Command {
title: "Trigger parameter hints".to_string(),
command: "editor.action.triggerParameterHints".to_string(),
arguments: None,
}),
..completion_item
}
}
79 changes: 47 additions & 32 deletions tooling/lsp/src/requests/completion/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,9 @@ mod completion_tests {
requests::{
completion::{
completion_items::{
completion_item_with_sort_text, crate_completion_item, module_completion_item,
simple_completion_item, snippet_completion_item,
completion_item_with_sort_text,
completion_item_with_trigger_parameter_hints_command, crate_completion_item,
module_completion_item, simple_completion_item, snippet_completion_item,
},
sort_text::self_mismatch_sort_text,
},
Expand Down Expand Up @@ -90,6 +91,20 @@ mod completion_tests {
assert_eq!(items, expected);
}

pub(super) fn function_completion_item(
label: impl Into<String>,
kind: CompletionItemKind,
insert_text: impl Into<String>,
description: impl Into<String>,
) -> CompletionItem {
completion_item_with_trigger_parameter_hints_command(snippet_completion_item(
label,
kind,
insert_text,
Some(description.into()),
))
}

#[test]
async fn test_use_first_segment() {
let src = r#"
Expand Down Expand Up @@ -408,11 +423,11 @@ mod completion_tests {
"#;
assert_completion(
src,
vec![snippet_completion_item(
vec![function_completion_item(
"hello()",
CompletionItemKind::FUNCTION,
"hello()",
Some("fn()".to_string()),
"fn()",
)],
)
.await;
Expand All @@ -429,11 +444,11 @@ mod completion_tests {
"#;
assert_completion(
src,
vec![snippet_completion_item(
vec![function_completion_item(
"hello(…)",
CompletionItemKind::FUNCTION,
"hello(${1:x}, ${2:y})",
Some("fn(i32, Field)".to_string()),
"fn(i32, Field)".to_string(),
)],
)
.await;
Expand All @@ -455,11 +470,11 @@ mod completion_tests {
"assert(${1:predicate})",
Some("fn(T)".to_string()),
),
snippet_completion_item(
function_completion_item(
"assert_constant(…)",
CompletionItemKind::FUNCTION,
"assert_constant(${1:x})",
Some("fn(T)".to_string()),
"fn(T)",
),
snippet_completion_item(
"assert_eq(…)",
Expand Down Expand Up @@ -1063,17 +1078,17 @@ mod completion_tests {
assert_completion(
src,
vec![
snippet_completion_item(
function_completion_item(
"foobar(…)",
CompletionItemKind::FUNCTION,
"foobar(${1:x})",
Some("fn(self, i32)".to_string()),
"fn(self, i32)".to_string(),
),
snippet_completion_item(
function_completion_item(
"foobar2(…)",
CompletionItemKind::FUNCTION,
"foobar2(${1:x})",
Some("fn(&mut self, i32)".to_string()),
"fn(&mut self, i32)".to_string(),
),
],
)
Expand Down Expand Up @@ -1102,11 +1117,11 @@ mod completion_tests {
"#;
assert_completion(
src,
vec![snippet_completion_item(
vec![function_completion_item(
"foobar(…)",
CompletionItemKind::FUNCTION,
"foobar(${1:x})",
Some("fn(self, i32)".to_string()),
"fn(self, i32)",
)],
)
.await;
Expand All @@ -1131,11 +1146,11 @@ mod completion_tests {
"#;
assert_completion(
src,
vec![snippet_completion_item(
vec![function_completion_item(
"foobar(…)",
CompletionItemKind::FUNCTION,
"foobar(${1:x})",
Some("fn(self, i32)".to_string()),
"fn(self, i32)".to_string(),
)],
)
.await;
Expand All @@ -1161,28 +1176,28 @@ mod completion_tests {
src,
vec![
completion_item_with_sort_text(
snippet_completion_item(
function_completion_item(
"foobar(…)",
CompletionItemKind::FUNCTION,
"foobar(${1:self}, ${2:x})",
Some("fn(self, i32)".to_string()),
"fn(self, i32)",
),
self_mismatch_sort_text(),
),
completion_item_with_sort_text(
snippet_completion_item(
function_completion_item(
"foobar2(…)",
CompletionItemKind::FUNCTION,
"foobar2(${1:self}, ${2:x})",
Some("fn(&mut self, i32)".to_string()),
"fn(&mut self, i32)",
),
self_mismatch_sort_text(),
),
snippet_completion_item(
function_completion_item(
"foobar3(…)",
CompletionItemKind::FUNCTION,
"foobar3(${1:y})",
Some("fn(i32)".to_string()),
"fn(i32)",
),
],
)
Expand All @@ -1207,11 +1222,11 @@ mod completion_tests {
"#;
assert_completion(
src,
vec![snippet_completion_item(
vec![function_completion_item(
"foobar(…)",
CompletionItemKind::FUNCTION,
"foobar(${1:x})",
Some("fn(self, i32)".to_string()),
"fn(self, i32)".to_string(),
)],
)
.await;
Expand Down Expand Up @@ -1239,28 +1254,28 @@ mod completion_tests {
src,
vec![
completion_item_with_sort_text(
snippet_completion_item(
function_completion_item(
"foobar(…)",
CompletionItemKind::FUNCTION,
"foobar(${1:self}, ${2:x})",
Some("fn(self, i32)".to_string()),
"fn(self, i32)",
),
self_mismatch_sort_text(),
),
completion_item_with_sort_text(
snippet_completion_item(
function_completion_item(
"foobar2(…)",
CompletionItemKind::FUNCTION,
"foobar2(${1:self}, ${2:x})",
Some("fn(&mut self, i32)".to_string()),
"fn(&mut self, i32)",
),
self_mismatch_sort_text(),
),
snippet_completion_item(
function_completion_item(
"foobar3(…)",
CompletionItemKind::FUNCTION,
"foobar3(${1:y})",
Some("fn(i32)".to_string()),
"fn(i32)",
),
],
)
Expand Down Expand Up @@ -1317,11 +1332,11 @@ mod completion_tests {
"#;
assert_completion(
src,
vec![snippet_completion_item(
vec![function_completion_item(
"foo()",
CompletionItemKind::FUNCTION,
"foo()",
Some("fn(self) -> Foo".to_string()),
"fn(self) -> Foo".to_string(),
)],
)
.await;
Expand Down
13 changes: 12 additions & 1 deletion tooling/lsp/src/requests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ mod inlay_hint;
mod profile_run;
mod references;
mod rename;
mod signature_help;
mod test_run;
mod tests;

Expand All @@ -56,7 +57,8 @@ pub(crate) use {
goto_definition::on_goto_type_definition_request, hover::on_hover_request,
inlay_hint::on_inlay_hint_request, profile_run::on_profile_run_request,
references::on_references_request, rename::on_prepare_rename_request,
rename::on_rename_request, test_run::on_test_run_request, tests::on_tests_request,
rename::on_rename_request, signature_help::on_signature_help_request,
test_run::on_test_run_request, tests::on_tests_request,
};

/// LSP client will send initialization request after the server has started.
Expand Down Expand Up @@ -241,6 +243,15 @@ pub(crate) fn on_initialize(
},
completion_item: None,
})),
signature_help_provider: Some(lsp_types::OneOf::Right(
lsp_types::SignatureHelpOptions {
trigger_characters: Some(vec!["(".to_string(), ",".to_string()]),
retrigger_characters: None,
work_done_progress_options: WorkDoneProgressOptions {
work_done_progress: None,
},
},
)),
},
server_info: None,
})
Expand Down
Loading

0 comments on commit 5a3d241

Please sign in to comment.