From 781dfa438e337348ca586e97745df7f67cf1a8c5 Mon Sep 17 00:00:00 2001 From: harehare Date: Tue, 14 Jul 2026 21:16:55 +0900 Subject: [PATCH] =?UTF-8?q?=E2=9C=A8=20feat(mq-lsp):=20add=20signature=20h?= =?UTF-8?q?elp=20support?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implement the textDocument/signatureHelp handler so the client gets inline argument-position hints while typing a function or macro call. mq_hir::Symbol ranges are too coarse for this (a Call symbol's range covers only its name token, and punctuation isn't lowered into HIR at all), so signature_help.rs re-parses the document into a CST and walks it directly to find the enclosing call, its parens, and its commas, then resolves the callee's Params through the existing Hir lookup. --- crates/mq-lsp/README.md | 1 + crates/mq-lsp/src/capabilities.rs | 9 +- crates/mq-lsp/src/lib.rs | 1 + crates/mq-lsp/src/main.rs | 1 + crates/mq-lsp/src/server.rs | 66 +++++++- crates/mq-lsp/src/signature_help.rs | 236 ++++++++++++++++++++++++++++ 6 files changed, 311 insertions(+), 3 deletions(-) create mode 100644 crates/mq-lsp/src/signature_help.rs diff --git a/crates/mq-lsp/README.md b/crates/mq-lsp/README.md index b1c045a4c..4880639e0 100644 --- a/crates/mq-lsp/README.md +++ b/crates/mq-lsp/README.md @@ -8,6 +8,7 @@ Language Server Protocol (LSP) implementation for the [mq](https://mqlang.org/) - ๐Ÿงน **Linting**: Optional `mq-lint` diagnostics (correctness, style, complexity, selector, and module rules), toggled with `--enable-lint` - ๐Ÿ’ก **Code Completion**: Intelligent suggestions for selectors, functions, and variables - ๐Ÿ“– **Hover Information**: Inline documentation and type information +- โœ๏ธ **Signature Help**: Inline parameter hints while typing a function or macro call - ๐ŸŽฏ **Go To Definition**: Navigate to symbol definitions with a single click - ๐Ÿ”— **Find References**: Locate all usages of a symbol across your workspace - ๐Ÿ—‚๏ธ **Document Symbols**: Outline view of all symbols in the current file diff --git a/crates/mq-lsp/src/capabilities.rs b/crates/mq-lsp/src/capabilities.rs index 9b208b03a..8c930c36c 100644 --- a/crates/mq-lsp/src/capabilities.rs +++ b/crates/mq-lsp/src/capabilities.rs @@ -2,8 +2,8 @@ use tower_lsp_server::ls_types::{ CodeActionKind, CodeActionOptions, CodeActionProviderCapability, CompletionOptions, DocumentFormattingOptions, DocumentRangeFormattingOptions, ExecuteCommandOptions, HoverProviderCapability, InlayHintOptions, InlayHintServerCapabilities, OneOf, RenameOptions, SemanticTokensFullOptions, SemanticTokensLegend, - SemanticTokensOptions, SemanticTokensServerCapabilities, ServerCapabilities, TextDocumentSyncCapability, - TextDocumentSyncKind, + SemanticTokensOptions, SemanticTokensServerCapabilities, ServerCapabilities, SignatureHelpOptions, + TextDocumentSyncCapability, TextDocumentSyncKind, }; use crate::semantic_tokens; @@ -12,6 +12,11 @@ pub(crate) fn server_capabilities() -> ServerCapabilities { ServerCapabilities { text_document_sync: Some(TextDocumentSyncCapability::Kind(TextDocumentSyncKind::FULL)), hover_provider: Some(HoverProviderCapability::Simple(true)), + signature_help_provider: Some(SignatureHelpOptions { + trigger_characters: Some(vec!["(".to_string(), ",".to_string()]), + retrigger_characters: None, + work_done_progress_options: Default::default(), + }), completion_provider: Some(CompletionOptions { trigger_characters: Some(vec!["|".to_string(), ":".to_string(), ".".to_string()]), ..Default::default() diff --git a/crates/mq-lsp/src/lib.rs b/crates/mq-lsp/src/lib.rs index 29b0c5583..1dbb46320 100644 --- a/crates/mq-lsp/src/lib.rs +++ b/crates/mq-lsp/src/lib.rs @@ -31,6 +31,7 @@ pub mod references; pub mod rename; pub mod semantic_tokens; pub mod server; +pub mod signature_help; pub mod workspace_symbol; pub use server::start; diff --git a/crates/mq-lsp/src/main.rs b/crates/mq-lsp/src/main.rs index b9af6332a..37999f70a 100644 --- a/crates/mq-lsp/src/main.rs +++ b/crates/mq-lsp/src/main.rs @@ -17,6 +17,7 @@ pub mod references; pub mod rename; pub mod semantic_tokens; pub mod server; +pub mod signature_help; pub mod workspace_symbol; #[derive(Parser, Debug)] diff --git a/crates/mq-lsp/src/server.rs b/crates/mq-lsp/src/server.rs index dd97e697a..0eda29e5c 100644 --- a/crates/mq-lsp/src/server.rs +++ b/crates/mq-lsp/src/server.rs @@ -11,7 +11,7 @@ use url::Url; use crate::error::LspError; use crate::{ capabilities, code_action, completions, document_symbol, execute_command, goto_definition, hover, inlay_hints, - references, rename, semantic_tokens, workspace_symbol, + references, rename, semantic_tokens, signature_help, workspace_symbol, }; use tower_lsp_server::{Client, LanguageServer, LspService, Server, jsonrpc, ls_types}; @@ -172,6 +172,22 @@ impl LanguageServer for Backend { Ok(hover::response(Arc::clone(&self.hir), to_url(&url), type_env, position)) } + async fn signature_help( + &self, + params: ls_types::SignatureHelpParams, + ) -> jsonrpc::Result> { + let url = params.text_document_position_params.text_document.uri; + let position = params.text_document_position_params.position; + let source_text = self.text_map.get(&url.to_string()).map(|text| Arc::clone(text.value())); + + Ok(signature_help::response( + Arc::clone(&self.hir), + to_url(&url), + position, + source_text.as_deref().map(String::as_str), + )) + } + async fn inlay_hint(&self, params: ls_types::InlayHintParams) -> jsonrpc::Result>> { if !self.config.enable_type_checking { return Ok(None); @@ -764,6 +780,53 @@ mod tests { assert!(result.is_ok()); } + #[tokio::test] + async fn test_signature_help() { + let (service, _) = LspService::new(|client| Backend { + client, + hir: Arc::new(RwLock::new(mq_hir::Hir::default())), + source_map: RwLock::new(BiMap::new()), + type_env_map: DashMap::new(), + error_map: DashMap::new(), + text_map: DashMap::new(), + config: LspConfig::default(), + }); + + let backend = service.inner(); + let uri = Url::parse("file:///test.mq").unwrap(); + let code = "def foo(a, b): a + b; | foo(1, 2)"; + + backend + .did_open(ls_types::DidOpenTextDocumentParams { + text_document: ls_types::TextDocumentItem { + uri: to_uri(&uri), + language_id: "mq".to_string(), + version: 1, + text: code.to_string(), + }, + }) + .await; + + // Cursor right before the `2` argument. + let result = backend + .signature_help(ls_types::SignatureHelpParams { + text_document_position_params: ls_types::TextDocumentPositionParams { + text_document: ls_types::TextDocumentIdentifier { uri: to_uri(&uri) }, + position: ls_types::Position::new(0, 31), + }, + work_done_progress_params: Default::default(), + context: None, + }) + .await; + + assert!(result.is_ok()); + let help = result.unwrap().unwrap(); + + assert_eq!(help.signatures.len(), 1); + assert_eq!(help.signatures[0].label, "foo(a, b)"); + assert_eq!(help.active_parameter, Some(1)); + } + #[tokio::test] async fn test_semantic_tokens() { let (service, _) = LspService::new(|client| Backend { @@ -1449,6 +1512,7 @@ mod tests { assert!(capabilities.rename_provider.is_some()); assert!(capabilities.document_symbol_provider.is_some()); assert!(capabilities.workspace_symbol_provider.is_some()); + assert!(capabilities.signature_help_provider.is_some()); // Test shutdown let shutdown_result = backend.shutdown().await; diff --git a/crates/mq-lsp/src/signature_help.rs b/crates/mq-lsp/src/signature_help.rs new file mode 100644 index 000000000..6028fd958 --- /dev/null +++ b/crates/mq-lsp/src/signature_help.rs @@ -0,0 +1,236 @@ +use std::sync::{Arc, RwLock}; + +use mq_lang::{CstNode, Shared, TokenKind}; +use tower_lsp_server::ls_types::{ParameterInformation, ParameterLabel, Position, SignatureHelp, SignatureInformation}; +use url::Url; + +/// Signature help needs exact call/paren/comma boundaries, which `mq_hir::Symbol` ranges +/// don't preserve (a `Call` symbol's range is only its name token; punctuation isn't +/// lowered into HIR at all). So this re-parses the document into a CST โ€” cheap for the +/// small scripts mq targets โ€” and walks it directly instead of going through the HIR. +pub(crate) fn response( + hir: Arc>, + url: Url, + position: Position, + source_text: Option<&str>, +) -> Option { + let source_text = source_text?; + let hir_guard = hir.read().unwrap(); + let source_id = hir_guard.source_by_url(&url)?; + let cursor = mq_lang::Position::new(position.line + 1, (position.character + 1) as usize); + + let (nodes, _) = mq_lang::parse_recovery(source_text); + let call = find_enclosing_call(&nodes, cursor)?; + + let (_, target) = hir_guard.find_symbol_in_position(source_id, call.range().start)?; + let params = match &target.kind { + mq_hir::SymbolKind::Function(params) | mq_hir::SymbolKind::Macro(params) => params.clone(), + _ => return None, + }; + + let name = target.value.as_deref().unwrap_or_default(); + let param_labels = params.iter().map(|p| p.to_string()).collect::>(); + let label = format!("{}({})", name, param_labels.join(", ")); + let active_parameter = active_parameter_index(&call, cursor, params.len()); + + Some(SignatureHelp { + signatures: vec![SignatureInformation { + label, + documentation: None, + parameters: Some( + param_labels + .into_iter() + .map(|label| ParameterInformation { + label: ParameterLabel::Simple(label), + documentation: None, + }) + .collect(), + ), + active_parameter, + }], + active_signature: Some(0), + active_parameter, + }) +} + +/// Finds the innermost `Call`/`CallDynamic` node whose full span (name through closing +/// paren, via `node_range()`) contains `position`. Recursing into children after recording +/// a match means a nested call (e.g. `outer(inner(1))`) naturally overwrites the outer one +/// once the cursor is confirmed to be inside it. +fn find_enclosing_call(nodes: &[Shared], position: mq_lang::Position) -> Option> { + let mut best = None; + for node in nodes { + visit(node, position, &mut best); + } + best +} + +fn visit(node: &Shared, position: mq_lang::Position, best: &mut Option>) { + if !node.node_range().contains(&position) { + return; + } + if matches!( + node.kind, + mq_lang::CstNodeKind::Call | mq_lang::CstNodeKind::CallDynamic + ) { + *best = Some(node.clone()); + } + for child in &node.children { + visit(child, position, best); + } +} + +/// Counts the call's own top-level `,` children that fall before `position`. Only direct +/// children are considered, so commas belonging to a nested call or array argument aren't +/// mistaken for this call's own argument separators. +fn active_parameter_index(call: &Shared, position: mq_lang::Position, param_count: usize) -> Option { + if param_count == 0 { + return None; + } + + let commas_before_cursor = call + .children + .iter() + .filter(|child| { + child.is_token() + && child + .token + .as_deref() + .is_some_and(|token| matches!(token.kind, TokenKind::Comma)) + }) + .filter(|child| child.range().start < position) + .count(); + + Some(commas_before_cursor.min(param_count - 1) as u32) +} + +#[cfg(test)] +mod tests { + use super::*; + use mq_hir::Hir; + + #[test] + fn test_no_call_at_position() { + let mut hir = Hir::default(); + let url = Url::parse("file:///test.mq").unwrap(); + let code = "let x = 1"; + hir.add_code(Some(url.clone()), code); + + let result = response(Arc::new(RwLock::new(hir)), url, Position::new(0, 5), Some(code)); + assert!(result.is_none()); + } + + #[test] + fn test_no_source_text() { + let mut hir = Hir::default(); + let url = Url::parse("file:///test.mq").unwrap(); + let code = "def foo(a): a; | foo(1)"; + hir.add_code(Some(url.clone()), code); + + let result = response(Arc::new(RwLock::new(hir)), url, Position::new(0, 21), None); + assert!(result.is_none()); + } + + #[test] + fn test_invalid_url() { + let hir = Hir::default(); + let url = Url::parse("file:///nonexistent.mq").unwrap(); + + let result = response(Arc::new(RwLock::new(hir)), url, Position::new(0, 0), Some("")); + assert!(result.is_none()); + } + + #[test] + fn test_signature_help_first_parameter() { + let mut hir = Hir::default(); + let url = Url::parse("file:///test.mq").unwrap(); + let code = "def foo(a, b): a + b; | foo(1, 2)"; + hir.add_code(Some(url.clone()), code); + + // Cursor right before the `1` argument. + let result = response(Arc::new(RwLock::new(hir)), url, Position::new(0, 28), Some(code)); + assert!(result.is_some()); + let help = result.unwrap(); + + assert_eq!(help.signatures.len(), 1); + assert_eq!(help.signatures[0].label, "foo(a, b)"); + assert_eq!(help.active_parameter, Some(0)); + } + + #[test] + fn test_signature_help_second_parameter() { + let mut hir = Hir::default(); + let url = Url::parse("file:///test.mq").unwrap(); + let code = "def foo(a, b): a + b; | foo(1, 2)"; + hir.add_code(Some(url.clone()), code); + + // Cursor right before the `2` argument. + let result = response(Arc::new(RwLock::new(hir)), url, Position::new(0, 31), Some(code)); + assert!(result.is_some()); + let help = result.unwrap(); + + assert_eq!(help.active_parameter, Some(1)); + } + + #[test] + fn test_signature_help_no_parameters() { + let mut hir = Hir::default(); + let url = Url::parse("file:///test.mq").unwrap(); + let code = "def noop(): 1; | noop()"; + hir.add_code(Some(url.clone()), code); + + // Cursor inside the empty argument list. + let result = response(Arc::new(RwLock::new(hir)), url, Position::new(0, 22), Some(code)); + assert!(result.is_some()); + let help = result.unwrap(); + + assert_eq!(help.signatures[0].label, "noop()"); + assert_eq!(help.active_parameter, None); + } + + #[test] + fn test_signature_help_nested_call_uses_innermost() { + let mut hir = Hir::default(); + let url = Url::parse("file:///test.mq").unwrap(); + let code = "def inner(x): x; | def outer(y): y; | outer(inner(1))"; + hir.add_code(Some(url.clone()), code); + + // Cursor on the `1` inside `inner(1)`. + let result = response(Arc::new(RwLock::new(hir)), url, Position::new(0, 51), Some(code)); + assert!(result.is_some()); + let help = result.unwrap(); + + assert_eq!(help.signatures[0].label, "inner(x)"); + } + + #[test] + fn test_signature_help_outer_call_when_between_calls() { + let mut hir = Hir::default(); + let url = Url::parse("file:///test.mq").unwrap(); + let code = "def inner(x): x; | def outer(y): y; | outer(inner(1))"; + hir.add_code(Some(url.clone()), code); + + // Cursor right on `outer`'s opening paren, before `inner(1)` starts. + let result = response(Arc::new(RwLock::new(hir)), url, Position::new(0, 43), Some(code)); + assert!(result.is_some()); + let help = result.unwrap(); + + assert_eq!(help.signatures[0].label, "outer(y)"); + } + + #[test] + fn test_signature_help_variadic_clamps_active_parameter() { + let mut hir = Hir::default(); + let url = Url::parse("file:///test.mq").unwrap(); + let code = "def foo(*rest): rest; | foo(1, 2, 3)"; + hir.add_code(Some(url.clone()), code); + + // Cursor on the third argument. + let result = response(Arc::new(RwLock::new(hir)), url, Position::new(0, 34), Some(code)); + assert!(result.is_some()); + let help = result.unwrap(); + + assert_eq!(help.signatures[0].label, "foo(*rest)"); + assert_eq!(help.active_parameter, Some(0)); + } +}