From fc72aa6abec475c50106f24ed1bc01c7516cab7c Mon Sep 17 00:00:00 2001 From: zc he Date: Mon, 3 Mar 2025 20:54:42 +0800 Subject: [PATCH] feat(lsp): signature help (manually triggered) (#15233) # Description To check for missing parameters image # User-Facing Changes For other languages, the help request can be triggered by the `(` character of the function call. Editors like nvim refuse to set the trigger character to space, and space is probably way too common for that. So this kind of request has to be triggered manually for now. example of nvim config: ```lua vim.api.nvim_create_autocmd("FileType", { pattern = "nu", callback = function(event) vim.bo[event.buf].commentstring = "# %s" vim.api.nvim_buf_set_keymap(event.buf, "i", "", "", { callback = function() vim.lsp.buf.signature_help() end, }) end, }) ``` # Tests + Formatting +2 # After Submitting --- crates/nu-cli/src/completions/completer.rs | 4 +- crates/nu-lsp/src/lib.rs | 39 ++- crates/nu-lsp/src/signature.rs | 339 +++++++++++++++++++++ tests/fixtures/lsp/hints/signature.nu | 17 ++ 4 files changed, 375 insertions(+), 24 deletions(-) create mode 100644 crates/nu-lsp/src/signature.rs create mode 100644 tests/fixtures/lsp/hints/signature.nu diff --git a/crates/nu-cli/src/completions/completer.rs b/crates/nu-cli/src/completions/completer.rs index 0540e83610..40231dabf7 100644 --- a/crates/nu-cli/src/completions/completer.rs +++ b/crates/nu-cli/src/completions/completer.rs @@ -325,8 +325,8 @@ impl NuCompleter { // unfinished argument completion for commands match &element_expression.expr { Expr::Call(call) => { - // TODO: the argument to complete won't necessarily be the last one in the future - // for lsp completion, we won't trim the text, + // NOTE: the argument to complete is not necessarily the last one + // for lsp completion, we don't trim the text, // so that `def`s after pos can be completed for arg in call.arguments.iter() { let span = arg.span(); diff --git a/crates/nu-lsp/src/lib.rs b/crates/nu-lsp/src/lib.rs index d72e625b33..9d05d44516 100644 --- a/crates/nu-lsp/src/lib.rs +++ b/crates/nu-lsp/src/lib.rs @@ -6,8 +6,8 @@ use lsp_types::{ Hover, HoverContents, HoverParams, InlayHint, MarkupContent, MarkupKind, OneOf, Position, Range, ReferencesOptions, RenameOptions, SemanticToken, SemanticTokenType, SemanticTokensLegend, SemanticTokensOptions, SemanticTokensServerCapabilities, - ServerCapabilities, TextDocumentSyncKind, Uri, WorkDoneProgressOptions, WorkspaceFolder, - WorkspaceFoldersServerCapabilities, WorkspaceServerCapabilities, + ServerCapabilities, SignatureHelpOptions, TextDocumentSyncKind, Uri, WorkDoneProgressOptions, + WorkspaceFolder, WorkspaceFoldersServerCapabilities, WorkspaceServerCapabilities, }; use miette::{miette, IntoDiagnostic, Result}; use nu_protocol::{ @@ -33,6 +33,7 @@ mod goto; mod hints; mod notification; mod semantic_tokens; +mod signature; mod symbols; mod workspace; @@ -156,6 +157,7 @@ impl LanguageServer { ..Default::default() }), ), + signature_help_provider: Some(SignatureHelpOptions::default()), ..Default::default() }) .expect("Must be serializable"); @@ -206,6 +208,9 @@ impl LanguageServer { self.document_highlight(params) }) } + request::DocumentSymbolRequest::METHOD => { + Self::handle_lsp_request(request, |params| self.document_symbol(params)) + } request::GotoDefinition::METHOD => { Self::handle_lsp_request(request, |params| self.goto_definition(params)) } @@ -215,11 +220,6 @@ impl LanguageServer { request::InlayHintRequest::METHOD => { Self::handle_lsp_request(request, |params| self.get_inlay_hints(params)) } - request::SemanticTokensFullRequest::METHOD => { - Self::handle_lsp_request(request, |params| { - self.get_semantic_tokens(params) - }) - } request::PrepareRenameRequest::METHOD => { let id = request.id.clone(); if let Err(e) = self.prepare_rename(request) { @@ -235,8 +235,15 @@ impl LanguageServer { request::Rename::METHOD => { Self::handle_lsp_request(request, |params| self.rename(params)) } - request::DocumentSymbolRequest::METHOD => { - Self::handle_lsp_request(request, |params| self.document_symbol(params)) + request::SemanticTokensFullRequest::METHOD => { + Self::handle_lsp_request(request, |params| { + self.get_semantic_tokens(params) + }) + } + request::SignatureHelpRequest::METHOD => { + Self::handle_lsp_request(request, |params| { + self.get_signature_help(params) + }) } request::WorkspaceSymbolRequest::METHOD => { Self::handle_lsp_request(request, |params| { @@ -433,19 +440,7 @@ impl LanguageServer { // Usage description.push_str("---\n### Usage \n```nu\n"); let signature = decl.signature(); - description.push_str(&format!(" {}", signature.name)); - if !signature.named.is_empty() { - description.push_str(" {flags}"); - } - for required_arg in &signature.required_positional { - description.push_str(&format!(" <{}>", required_arg.name)); - } - for optional_arg in &signature.optional_positional { - description.push_str(&format!(" <{}?>", optional_arg.name)); - } - if let Some(arg) = &signature.rest_positional { - description.push_str(&format!(" <...{}>", arg.name)); - } + description.push_str(&Self::get_signature_label(&signature)); description.push_str("\n```\n"); // Flags diff --git a/crates/nu-lsp/src/signature.rs b/crates/nu-lsp/src/signature.rs new file mode 100644 index 0000000000..263a29dacb --- /dev/null +++ b/crates/nu-lsp/src/signature.rs @@ -0,0 +1,339 @@ +use lsp_types::{ + Documentation, MarkupContent, MarkupKind, ParameterInformation, SignatureHelp, + SignatureHelpParams, SignatureInformation, +}; +use nu_protocol::{ + ast::{Argument, Call, Expr, Expression, FindMapResult, Traverse}, + engine::StateWorkingSet, + PositionalArg, Signature, +}; + +use crate::{uri_to_path, LanguageServer}; + +fn find_active_internal_call<'a>( + expr: &'a Expression, + working_set: &'a StateWorkingSet, + pos: usize, +) -> FindMapResult<&'a Call> { + if !expr.span.contains(pos) { + return FindMapResult::Stop; + } + let closure = |e| find_active_internal_call(e, working_set, pos); + match &expr.expr { + Expr::Call(call) => { + if call.head.contains(pos) { + return FindMapResult::Stop; + } + call.arguments + .iter() + .find_map(|arg| arg.expr().and_then(|e| e.find_map(working_set, &closure))) + .or(Some(call.as_ref())) + .map(FindMapResult::Found) + .unwrap_or_default() + } + _ => FindMapResult::Continue, + } +} + +impl LanguageServer { + pub(crate) fn get_signature_label(signature: &Signature) -> String { + let mut label = String::new(); + label.push_str(&format!(" {}", signature.name)); + if !signature.named.is_empty() { + label.push_str(" {flags}"); + } + for required_arg in &signature.required_positional { + label.push_str(&format!(" <{}>", required_arg.name)); + } + for optional_arg in &signature.optional_positional { + let value_info = if let Some(value) = optional_arg + .default_value + .as_ref() + .and_then(|v| v.coerce_str().ok()) + { + format!("={}", value) + } else { + String::new() + }; + label.push_str(&format!(" <{}?{}>", optional_arg.name, value_info)); + } + if let Some(arg) = &signature.rest_positional { + label.push_str(&format!(" <...{}>", arg.name)); + } + label + } + + pub(crate) fn get_signature_help( + &mut self, + params: &SignatureHelpParams, + ) -> Option { + let path_uri = params + .text_document_position_params + .text_document + .uri + .to_owned(); + let docs = self.docs.lock().ok()?; + let file = docs.get_document(&path_uri)?; + let location = file.offset_at(params.text_document_position_params.position) as usize; + let file_text = file.get_content(None).to_owned(); + drop(docs); + + let engine_state = self.new_engine_state(); + let mut working_set = StateWorkingSet::new(&engine_state); + + // NOTE: in case the cursor is at the end of the call expression + let need_placeholder = location == 0 + || file_text + .get(location - 1..location) + .is_some_and(|s| s.chars().all(|c| c.is_whitespace())); + let file_path = uri_to_path(&path_uri); + let filename = if need_placeholder { + "lsp_signature_helper_temp_file" + } else { + file_path.to_str()? + }; + + let block = if need_placeholder { + nu_parser::parse( + &mut working_set, + Some(filename), + format!( + "{}a{}", + file_text.get(..location).unwrap_or_default(), + file_text.get(location..).unwrap_or_default() + ) + .as_bytes(), + false, + ) + } else { + nu_parser::parse( + &mut working_set, + Some(filename), + file_text.as_bytes(), + false, + ) + }; + let span = working_set.get_span_for_filename(filename)?; + + let pos_to_search = location.saturating_add(span.start).saturating_sub(1); + let active_call = block.find_map(&working_set, &|expr: &Expression| { + find_active_internal_call(expr, &working_set, pos_to_search) + })?; + let active_signature = working_set.get_decl(active_call.decl_id).signature(); + + let mut param_num_before_pos = 0; + for arg in active_call.arguments.iter() { + // skip flags + if matches!(arg, Argument::Named(_)) { + continue; + } + if arg.span().end <= pos_to_search { + param_num_before_pos += 1; + } else { + break; + } + } + let str_to_doc = |s: String| { + Some(Documentation::MarkupContent(MarkupContent { + kind: MarkupKind::Markdown, + value: s, + })) + }; + let arg_to_param_info = |arg: &PositionalArg| ParameterInformation { + label: lsp_types::ParameterLabel::Simple(arg.name.to_owned()), + documentation: str_to_doc(format!( + ": `<{}>` - {}", + arg.shape.to_type(), + arg.desc.to_owned() + )), + }; + let mut parameters: Vec = active_signature + .required_positional + .iter() + .map(arg_to_param_info) + .chain( + active_signature + .optional_positional + .iter() + .map(arg_to_param_info), + ) + .collect(); + if let Some(rest_arg) = &active_signature.rest_positional { + parameters.push(arg_to_param_info(rest_arg)); + } + let max_idx = parameters.len().saturating_sub(1) as u32; + Some(SignatureHelp { + signatures: vec![SignatureInformation { + label: Self::get_signature_label(&active_signature), + documentation: str_to_doc(active_signature.description), + parameters: Some(parameters), + active_parameter: Some(std::cmp::min(param_num_before_pos, max_idx)), + }], + active_signature: Some(0), + active_parameter: None, + }) + } +} + +#[cfg(test)] +mod tests { + use crate::path_to_uri; + use crate::tests::{initialize_language_server, open_unchecked, result_from_message}; + use assert_json_diff::assert_json_include; + use lsp_server::{Connection, Message}; + use lsp_types::{ + request::{Request, SignatureHelpRequest}, + TextDocumentIdentifier, Uri, WorkDoneProgressParams, + }; + use lsp_types::{Position, SignatureHelpParams, TextDocumentPositionParams}; + use nu_test_support::fs::fixtures; + + fn send_signature_help_request( + client_connection: &Connection, + uri: Uri, + line: u32, + character: u32, + ) -> Message { + client_connection + .sender + .send(Message::Request(lsp_server::Request { + id: 1.into(), + method: SignatureHelpRequest::METHOD.to_string(), + params: serde_json::to_value(SignatureHelpParams { + text_document_position_params: TextDocumentPositionParams { + text_document: TextDocumentIdentifier { uri }, + position: Position { line, character }, + }, + work_done_progress_params: WorkDoneProgressParams::default(), + context: None, + }) + .unwrap(), + })) + .unwrap(); + client_connection + .receiver + .recv_timeout(std::time::Duration::from_secs(2)) + .unwrap() + } + + #[test] + fn signature_help_on_builtins() { + let (client_connection, _recv) = initialize_language_server(None, None); + + let mut script = fixtures(); + script.push("lsp"); + script.push("hints"); + script.push("signature.nu"); + let script = path_to_uri(&script); + + open_unchecked(&client_connection, script.clone()); + let resp = send_signature_help_request(&client_connection, script.clone(), 0, 15); + assert_json_include!( + actual: result_from_message(resp), + expected: serde_json::json!({ + "signatures": [{ + "label": " str substring {flags} <...rest>", + "parameters": [ ], + "activeParameter": 0 + }], + "activeSignature": 0 + }) + ); + + let resp = send_signature_help_request(&client_connection, script.clone(), 0, 17); + assert_json_include!( + actual: result_from_message(resp), + expected: serde_json::json!({ "signatures": [{ "activeParameter": 0 }], "activeSignature": 0 }) + ); + + let resp = send_signature_help_request(&client_connection, script.clone(), 0, 18); + assert_json_include!( + actual: result_from_message(resp), + expected: serde_json::json!({ "signatures": [{ "activeParameter": 1 }], "activeSignature": 0 }) + ); + + let resp = send_signature_help_request(&client_connection, script.clone(), 0, 22); + assert_json_include!( + actual: result_from_message(resp), + expected: serde_json::json!({ "signatures": [{ "activeParameter": 1 }], "activeSignature": 0 }) + ); + + let resp = send_signature_help_request(&client_connection, script.clone(), 7, 0); + assert_json_include!( + actual: result_from_message(resp), + expected: serde_json::json!({ "signatures": [{ + "label": " str substring {flags} <...rest>", + "activeParameter": 1 + }]}) + ); + + let resp = send_signature_help_request(&client_connection, script.clone(), 4, 0); + assert_json_include!( + actual: result_from_message(resp), + expected: serde_json::json!({ "signatures": [{ + "label": " str substring {flags} <...rest>", + "activeParameter": 0 + }]}) + ); + + let resp = send_signature_help_request(&client_connection, script, 16, 6); + assert_json_include!( + actual: result_from_message(resp), + expected: serde_json::json!({ "signatures": [{ + "label": " echo {flags} <...rest>", + "activeParameter": 0 + }]}) + ); + } + + #[test] + fn signature_help_on_custom_commands() { + let config_str = r#"export def "foo bar" [ + p1: int + p2: string, + p3?: int = 1 # doc +] {}"#; + let (client_connection, _recv) = initialize_language_server(Some(config_str), None); + + let mut script = fixtures(); + script.push("lsp"); + script.push("hints"); + script.push("signature.nu"); + let script = path_to_uri(&script); + + open_unchecked(&client_connection, script.clone()); + let resp = send_signature_help_request(&client_connection, script.clone(), 9, 11); + assert_json_include!( + actual: result_from_message(resp), + expected: serde_json::json!({ + "signatures": [{ + "label": " foo bar {flags} ", + "parameters": [ + {"label": "p1", "documentation": {"value": ": `` - "}}, + {"label": "p2", "documentation": {"value": ": `` - "}}, + {"label": "p3", "documentation": {"value": ": `` - doc"}}, + ], + "activeParameter": 1 + }], + "activeSignature": 0 + }) + ); + + let resp = send_signature_help_request(&client_connection, script, 10, 15); + assert_json_include!( + actual: result_from_message(resp), + expected: serde_json::json!({ + "signatures": [{ + "label": " foo baz {flags} ", + "parameters": [ + {"label": "p1", "documentation": {"value": ": `` - "}}, + {"label": "p2", "documentation": {"value": ": `` - "}}, + {"label": "p3", "documentation": {"value": ": `` - doc"}}, + ], + "activeParameter": 2 + }], + "activeSignature": 0 + }) + ); + } +} diff --git a/tests/fixtures/lsp/hints/signature.nu b/tests/fixtures/lsp/hints/signature.nu new file mode 100644 index 0000000000..65ee6732f3 --- /dev/null +++ b/tests/fixtures/lsp/hints/signature.nu @@ -0,0 +1,17 @@ +str substring ..2 -g foo bar b +( + # inside parenthesis + str substring + ..2 -b + foo + b + +) +foo bar 1 2 3 +foo baz 1 2 3 +def "foo baz" [ + p1: int + p2: string, + p3?: int = 1 # doc +] {} +echo