diff --git a/lsp/package.json b/lsp/package.json index b0c229d8d3..afd04d64e1 100644 --- a/lsp/package.json +++ b/lsp/package.json @@ -207,6 +207,12 @@ "description": "Controls whether hover tooltips include 'Go to definition' and 'Go to type definition' navigation links.", "scope": "resource" }, + "python.analysis.completeFunctionParens": { + "type": "boolean", + "default": false, + "description": "Automatically insert parentheses when completing a function or method.", + "scope": "resource" + }, "python.pyrefly.syncNotebooks": { "type": "boolean", "default": true, diff --git a/pyrefly/lib/lsp/non_wasm/server.rs b/pyrefly/lib/lsp/non_wasm/server.rs index cf5d8a1977..b73e52bf0a 100644 --- a/pyrefly/lib/lsp/non_wasm/server.rs +++ b/pyrefly/lib/lsp/non_wasm/server.rs @@ -250,6 +250,8 @@ use crate::lsp::non_wasm::will_rename_files::will_rename_files; use crate::lsp::non_wasm::workspace::LspAnalysisConfig; use crate::lsp::non_wasm::workspace::Workspace; use crate::lsp::non_wasm::workspace::Workspaces; +use crate::lsp::wasm::completion::CompletionOptions as CompletionRequestOptions; +use crate::lsp::wasm::completion::supports_snippet_completions; use crate::lsp::wasm::hover::get_hover; use crate::lsp::wasm::notebook::DidChangeNotebookDocument; use crate::lsp::wasm::notebook::DidChangeNotebookDocumentParams; @@ -3067,7 +3069,7 @@ impl Server { params: CompletionParams, ) -> anyhow::Result { let uri = ¶ms.text_document_position.text_document.uri; - let (handle, import_format) = match self + let (handle, lsp_config) = match self .make_handle_with_lsp_analysis_config_if_enabled(uri, Some(Completion::METHOD)) { None => { @@ -3076,7 +3078,18 @@ impl Server { items: Vec::new(), })); } - Some((x, config)) => (x, config.and_then(|c| c.import_format).unwrap_or_default()), + Some((x, config)) => (x, config), + }; + let import_format = lsp_config.and_then(|c| c.import_format).unwrap_or_default(); + let complete_function_parens = lsp_config + .and_then(|c| c.complete_function_parens) + .unwrap_or(false); + let completion_options = CompletionRequestOptions { + supports_completion_item_details: self.supports_completion_item_details(), + complete_function_parens, + supports_snippet_completions: supports_snippet_completions( + &self.initialize_params.capabilities, + ), }; let (items, is_incomplete) = transaction .get_module_info(&handle) @@ -3085,7 +3098,7 @@ impl Server { &handle, self.from_lsp_position(uri, &info, params.text_document_position.position), import_format, - self.supports_completion_item_details(), + completion_options, ) }) .unwrap_or_default(); diff --git a/pyrefly/lib/lsp/non_wasm/workspace.rs b/pyrefly/lib/lsp/non_wasm/workspace.rs index 366f7d98b5..b6dd2e32f2 100644 --- a/pyrefly/lib/lsp/non_wasm/workspace.rs +++ b/pyrefly/lib/lsp/non_wasm/workspace.rs @@ -258,6 +258,7 @@ pub struct LspAnalysisConfig { #[allow(dead_code)] pub diagnostic_mode: Option, pub import_format: Option, + pub complete_function_parens: Option, pub inlay_hints: Option, // TODO: this is not a pylance setting. it should be in pyrefly settings #[serde(default)] diff --git a/pyrefly/lib/lsp/wasm/completion.rs b/pyrefly/lib/lsp/wasm/completion.rs index f20df84290..ca7704672f 100644 --- a/pyrefly/lib/lsp/wasm/completion.rs +++ b/pyrefly/lib/lsp/wasm/completion.rs @@ -12,6 +12,7 @@ use lsp_types::CompletionItem; use lsp_types::CompletionItemKind; use lsp_types::CompletionItemLabelDetails; use lsp_types::CompletionItemTag; +use lsp_types::InsertTextFormat; use lsp_types::TextEdit; use pyrefly_build::handle::Handle; use pyrefly_python::ast::Ast; @@ -23,6 +24,7 @@ use pyrefly_types::display::LspDisplayMode; use pyrefly_types::literal::Lit; use pyrefly_types::types::Union; use ruff_python_ast::AnyNodeRef; +use ruff_python_ast::ExprContext; use ruff_python_ast::Identifier; use ruff_python_ast::name::Name; use ruff_text_size::Ranged; @@ -44,6 +46,25 @@ use crate::state::state::Transaction; use crate::types::callable::Param; use crate::types::types::Type; +/// Options that influence completion item formatting and behavior. +#[derive(Clone, Copy, Debug, Default)] +pub(crate) struct CompletionOptions { + pub supports_completion_item_details: bool, + pub complete_function_parens: bool, + pub supports_snippet_completions: bool, +} + +/// Returns true if the client supports snippet completions in completion items. +pub(crate) fn supports_snippet_completions(capabilities: &lsp_types::ClientCapabilities) -> bool { + capabilities + .text_document + .as_ref() + .and_then(|t| t.completion.as_ref()) + .and_then(|c| c.completion_item.as_ref()) + .and_then(|ci| ci.snippet_support) + .unwrap_or(false) +} + impl Transaction<'_> { /// Adds completion items for literal types (e.g., `Literal["foo", "bar"]`). pub(crate) fn add_literal_completions_from_type( @@ -114,6 +135,31 @@ impl Transaction<'_> { }); } + /// Adds function/method completion inserts with parentheses, using snippets when supported. + pub(crate) fn add_function_call_parens( + completions: &mut [CompletionItem], + supports_snippets: bool, + ) { + for item in completions { + if item.insert_text.is_some() || item.text_edit.is_some() { + continue; + } + if !matches!( + item.kind, + Some(CompletionItemKind::FUNCTION | CompletionItemKind::METHOD) + ) { + continue; + } + + if supports_snippets { + item.insert_text = Some(format!("{}($0)", item.label)); + item.insert_text_format = Some(InsertTextFormat::SNIPPET); + } else { + item.insert_text = Some(format!("{}()", item.label)); + } + } + } + /// Retrieves documentation for an export to display in completion items. pub(crate) fn get_documentation_from_export( &self, @@ -444,10 +490,16 @@ impl Transaction<'_> { handle: &Handle, position: TextSize, import_format: ImportFormat, - supports_completion_item_details: bool, + options: CompletionOptions, ) -> (Vec, bool) { + let CompletionOptions { + supports_completion_item_details, + complete_function_parens, + supports_snippet_completions, + } = options; let mut result = Vec::new(); let mut is_incomplete = false; + let mut allow_function_call_parens = false; // Because of parser error recovery, `from x impo...` looks like `from x import impo...` // If the user might be typing the `import` keyword, add that as an autocomplete option. match self.identifier_at(handle, position) { @@ -506,6 +558,7 @@ impl Transaction<'_> { identifier: _, context: IdentifierContext::Attribute { base_range, .. }, }) => { + allow_function_call_parens = true; if let Some(answers) = self.get_answers(handle) && let Some(base_type) = answers.get_type_trace(base_range) { @@ -552,6 +605,12 @@ impl Transaction<'_> { identifier, context, }) => { + if matches!( + context, + IdentifierContext::Expr(ExprContext::Load | ExprContext::Invalid) + ) { + allow_function_call_parens = true; + } if matches!(context, IdentifierContext::MethodDef { .. }) { Self::add_magic_method_completions(&identifier, &mut result); } @@ -616,6 +675,9 @@ impl Transaction<'_> { } } } + if complete_function_parens && allow_function_call_parens { + Self::add_function_call_parens(&mut result, supports_snippet_completions); + } for item in &mut result { let sort_text = if item .tags diff --git a/pyrefly/lib/state/lsp.rs b/pyrefly/lib/state/lsp.rs index 5810fed89f..65aaaa6eba 100644 --- a/pyrefly/lib/state/lsp.rs +++ b/pyrefly/lib/state/lsp.rs @@ -65,6 +65,7 @@ use crate::config::error_kind::ErrorKind; use crate::export::exports::Export; use crate::export::exports::ExportLocation; use crate::lsp::module_helpers::collect_symbol_def_paths; +use crate::lsp::wasm::completion::CompletionOptions; use crate::state::ide::IntermediateDefinition; use crate::state::ide::import_regular_import_edit; use crate::state::ide::insert_import_edit; @@ -2584,8 +2585,8 @@ impl<'a> Transaction<'a> { Some(references) } - // Kept for backwards compatibility - used by external callers (lsp/server.rs, playground.rs) - // who don't need the is_incomplete flag + // Kept for backwards compatibility - used by external callers who don't need the + // is_incomplete flag. pub fn completion( &self, handle: &Handle, @@ -2597,7 +2598,10 @@ impl<'a> Transaction<'a> { handle, position, import_format, - supports_completion_item_details, + CompletionOptions { + supports_completion_item_details, + ..Default::default() + }, ) .0 } @@ -2608,7 +2612,7 @@ impl<'a> Transaction<'a> { handle: &Handle, position: TextSize, import_format: ImportFormat, - supports_completion_item_details: bool, + options: CompletionOptions, ) -> (Vec, bool) { // Check if position is in a disabled range (comments) if let Some(module) = self.get_module_info(handle) { @@ -2618,12 +2622,8 @@ impl<'a> Transaction<'a> { } } - let (mut results, is_incomplete) = self.completion_sorted_opt_with_incomplete( - handle, - position, - import_format, - supports_completion_item_details, - ); + let (mut results, is_incomplete) = + self.completion_sorted_opt_with_incomplete(handle, position, import_format, options); results.sort_by(|item1, item2| { item1 .sort_text diff --git a/pyrefly/lib/test/lsp/lsp_interaction/completion.rs b/pyrefly/lib/test/lsp/lsp_interaction/completion.rs index d713bcdb04..378b33f5c4 100644 --- a/pyrefly/lib/test/lsp/lsp_interaction/completion.rs +++ b/pyrefly/lib/test/lsp/lsp_interaction/completion.rs @@ -8,6 +8,7 @@ use itertools::Itertools; use lsp_types::CompletionItemKind; use lsp_types::CompletionResponse; +use lsp_types::InsertTextFormat; use lsp_types::Url; use lsp_types::notification::DidChangeTextDocument; use lsp_types::request::Completion; @@ -56,6 +57,67 @@ fn test_completion_basic() { interaction.shutdown().unwrap(); } +#[test] +fn test_completion_function_parens_snippet() { + let root = get_test_files_root(); + let mut interaction = LspInteraction::new(); + interaction.set_root(root.path().join("basic")); + interaction + .initialize(InitializeSettings { + configuration: Some(Some(json!([{ + "analysis": { + "completeFunctionParens": true + } + }]))), + capabilities: Some(json!({ + "textDocument": { + "completion": { + "completionItem": { + "snippetSupport": true + } + } + } + })), + ..Default::default() + }) + .unwrap(); + + interaction.client.did_open("foo.py"); + + let root_path = root.path().join("basic"); + let foo_path = root_path.join("foo.py"); + interaction + .client + .send_notification::(json!({ + "textDocument": { + "uri": Url::from_file_path(&foo_path).unwrap().to_string(), + "languageId": "python", + "version": 2 + }, + "contentChanges": [{ + "range": { + "start": {"line": 0, "character": 0}, + "end": {"line": 0, "character": 0} + }, + "text": "def spam(x: int) -> None:\n pass\n\nsp\n" + }], + })); + + interaction + .client + .completion("foo.py", 3, 2) + .expect_completion_response_with(|list| { + list.items.iter().any(|item| { + item.label == "spam" + && item.insert_text.as_deref() == Some("spam($0)") + && item.insert_text_format == Some(InsertTextFormat::SNIPPET) + }) + }) + .unwrap(); + + interaction.shutdown().unwrap(); +} + #[test] fn test_completion_sorted_in_sorttext_order() { let root = get_test_files_root();