Skip to content

impl Completions: Auto add function call parenthesis #2207 #2254

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
asukaminato0721 wants to merge 6 commits into from
+158 −14
Closed

impl Completions: Auto add function call parenthesis #2207 #2254

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
7f41c8e
impl bracket
asukaminato0721 Jan 29, 2026
ca1044c
update
asukaminato0721 Jan 29, 2026
3292228
Merge branch 'main' into 2207
asukaminato0721 Jan 31, 2026
ed1189c
fix
asukaminato0721 Jan 31, 2026
9a44fc0
Merge branch 'main' into 2207
asukaminato0721 Feb 2, 2026
8411d25
update
asukaminato0721 Feb 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions lsp/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
19 changes: 16 additions & 3 deletions pyrefly/lib/lsp/non_wasm/server.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -3067,7 +3069,7 @@ impl Server {
params: CompletionParams,
) -> anyhow::Result<CompletionResponse> {
let uri = &params.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 => {
Expand All @@ -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)
Expand All @@ -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();
Expand Down
1 change: 1 addition & 0 deletions pyrefly/lib/lsp/non_wasm/workspace.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,7 @@ pub struct LspAnalysisConfig {
#[allow(dead_code)]
pub diagnostic_mode: Option<DiagnosticMode>,
pub import_format: Option<ImportFormat>,
pub complete_function_parens: Option<bool>,
pub inlay_hints: Option<InlayHintConfig>,
// TODO: this is not a pylance setting. it should be in pyrefly settings
#[serde(default)]
Expand Down
64 changes: 63 additions & 1 deletion pyrefly/lib/lsp/wasm/completion.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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(
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -444,10 +490,16 @@ impl Transaction<'_> {
handle: &Handle,
position: TextSize,
import_format: ImportFormat,
supports_completion_item_details: bool,
options: CompletionOptions,
) -> (Vec<CompletionItem>, 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) {
Expand Down Expand Up @@ -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)
{
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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
Expand Down
20 changes: 10 additions & 10 deletions pyrefly/lib/state/lsp.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand All @@ -2597,7 +2598,10 @@ impl<'a> Transaction<'a> {
handle,
position,
import_format,
supports_completion_item_details,
CompletionOptions {
supports_completion_item_details,
..Default::default()
},
)
.0
}
Expand All @@ -2608,7 +2612,7 @@ impl<'a> Transaction<'a> {
handle: &Handle,
position: TextSize,
import_format: ImportFormat,
supports_completion_item_details: bool,
options: CompletionOptions,
) -> (Vec<CompletionItem>, bool) {
// Check if position is in a disabled range (comments)
if let Some(module) = self.get_module_info(handle) {
Expand All @@ -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
Expand Down
62 changes: 62 additions & 0 deletions pyrefly/lib/test/lsp/lsp_interaction/completion.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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::<DidChangeTextDocument>(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)
})
Comment on lines +110 to +114
Copy link

Copilot AI Jan 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new behavior has two branches (snippet vs. non-snippet clients), but this test only asserts the snippet path. Consider adding a companion test (or an additional assertion) with snippetSupport: false to verify the plain-text insertion uses spam() and does not set insertTextFormat to SNIPPET.

Copilot uses AI. Check for mistakes.
})
.unwrap();

interaction.shutdown().unwrap();
}

#[test]
fn test_completion_sorted_in_sorttext_order() {
let root = get_test_files_root();
Expand Down
Loading