Skip to content

Commit

Permalink
typescript: Complete function calls with snippets (#11157)
Browse files Browse the repository at this point in the history
This allows function call (i.e. snippet) completion with
`typescript-language-server`. So far that didn't work, because
`typescript-language-server` doesn't respond with `insertText` when
getting the completions, but only when then sending
`completionItem/resolve` requests. See:
hrsh7th/nvim-cmp#646 (comment)

What this PR does is to support text edits in the response to
`completionItem/resolve`, which means updating the completion item.

It then enables this feature by default for
`typescript-language-server`.


TODOs:

- [x] Make this work over collab
- [x] Test that this doesn't break existing language server support
- [x] Refactor duplicated code

Release Notes:

- Added support for function call completion when using
`typescript-language-server`. This will result in parameters being
added, which can then be changed and navigated with `<tab>`. For this to
work with `typescript-language-server`, the documentation for a given
completion item needs to be resolved, meaning that if one types very
quickly and accepts completion before `typescript-language-server` could
respond with the documentation, no full function completion is used.

Demo:


https://github.com/zed-industries/zed/assets/1185253/c23ebe12-5902-4b50-888c-d9b8cd32965d
  • Loading branch information
mrnugget committed May 2, 2024
1 parent d8ca153 commit c812304
Show file tree
Hide file tree
Showing 7 changed files with 256 additions and 50 deletions.
91 changes: 89 additions & 2 deletions crates/collab/src/tests/editor_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ use call::ActiveCall;
use collections::HashMap;
use editor::{
actions::{
ConfirmCodeAction, ConfirmCompletion, ConfirmRename, Redo, Rename, RevertSelectedHunks,
ToggleCodeActions, Undo,
ConfirmCodeAction, ConfirmCompletion, ConfirmRename, ContextMenuFirst, Redo, Rename,
RevertSelectedHunks, ToggleCodeActions, Undo,
},
test::{
editor_hunks,
Expand Down Expand Up @@ -444,6 +444,93 @@ async fn test_collaborating_with_completion(cx_a: &mut TestAppContext, cx_b: &mu
"use d::SomeTrait;\nfn main() { a.first_method() }"
);
});

// Now we do a second completion, this time to ensure that documentation/snippets are
// resolved
editor_b.update(cx_b, |editor, cx| {
editor.change_selections(None, cx, |s| s.select_ranges([46..46]));
editor.handle_input("; a", cx);
editor.handle_input(".", cx);
});

buffer_b.read_with(cx_b, |buffer, _| {
assert_eq!(
buffer.text(),
"use d::SomeTrait;\nfn main() { a.first_method(); a. }"
);
});

let mut completion_response = fake_language_server
.handle_request::<lsp::request::Completion, _, _>(|params, _| async move {
assert_eq!(
params.text_document_position.text_document.uri,
lsp::Url::from_file_path("/a/main.rs").unwrap(),
);
assert_eq!(
params.text_document_position.position,
lsp::Position::new(1, 32),
);

Ok(Some(lsp::CompletionResponse::Array(vec![
lsp::CompletionItem {
label: "third_method(…)".into(),
detail: Some("fn(&mut self, B, C, D) -> E".into()),
text_edit: Some(lsp::CompletionTextEdit::Edit(lsp::TextEdit {
// no snippet placehodlers
new_text: "third_method".to_string(),
range: lsp::Range::new(
lsp::Position::new(1, 32),
lsp::Position::new(1, 32),
),
})),
insert_text_format: Some(lsp::InsertTextFormat::SNIPPET),
documentation: None,
..Default::default()
},
])))
});

// The completion now gets a new `text_edit.new_text` when resolving the completion item
let mut resolve_completion_response = fake_language_server
.handle_request::<lsp::request::ResolveCompletionItem, _, _>(|params, _| async move {
assert_eq!(params.label, "third_method(…)");
Ok(lsp::CompletionItem {
label: "third_method(…)".into(),
detail: Some("fn(&mut self, B, C, D) -> E".into()),
text_edit: Some(lsp::CompletionTextEdit::Edit(lsp::TextEdit {
// Now it's a snippet
new_text: "third_method($1, $2, $3)".to_string(),
range: lsp::Range::new(lsp::Position::new(1, 32), lsp::Position::new(1, 32)),
})),
insert_text_format: Some(lsp::InsertTextFormat::SNIPPET),
documentation: Some(lsp::Documentation::String(
"this is the documentation".into(),
)),
..Default::default()
})
});

cx_b.executor().run_until_parked();

completion_response.next().await.unwrap();

editor_b.update(cx_b, |editor, cx| {
assert!(editor.context_menu_visible());
editor.context_menu_first(&ContextMenuFirst {}, cx);
});

resolve_completion_response.next().await.unwrap();
cx_b.executor().run_until_parked();

// When accepting the completion, the snippet is insert.
editor_b.update(cx_b, |editor, cx| {
assert!(editor.context_menu_visible());
editor.confirm_completion(&ConfirmCompletion { item_ix: Some(0) }, cx);
assert_eq!(
editor.text(cx),
"use d::SomeTrait;\nfn main() { a.first_method(); a.third_method(, , ) }"
);
});
}

#[gpui::test(iterations = 10)]
Expand Down
1 change: 1 addition & 0 deletions crates/collab_ui/src/chat_panel/message_editor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ impl CompletionProvider for MessageEditorCompletionProvider {

fn resolve_completions(
&self,
_buffer: Model<Buffer>,
_completion_indices: Vec<usize>,
_completions: Arc<RwLock<Box<[Completion]>>>,
_cx: &mut ViewContext<Editor>,
Expand Down
16 changes: 13 additions & 3 deletions crates/editor/src/editor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -837,6 +837,7 @@ impl CompletionsMenu {
}

fn pre_resolve_completion_documentation(
buffer: Model<Buffer>,
completions: Arc<RwLock<Box<[Completion]>>>,
matches: Arc<[StringMatch]>,
editor: &Editor,
Expand All @@ -852,6 +853,7 @@ impl CompletionsMenu {
};

let resolve_task = provider.resolve_completions(
buffer,
matches.iter().map(|m| m.candidate_id).collect(),
completions.clone(),
cx,
Expand Down Expand Up @@ -880,7 +882,12 @@ impl CompletionsMenu {
};

let resolve_task = project.update(cx, |project, cx| {
project.resolve_completions(vec![completion_index], self.completions.clone(), cx)
project.resolve_completions(
self.buffer.clone(),
vec![completion_index],
self.completions.clone(),
cx,
)
});

let delay_ms =
Expand Down Expand Up @@ -3463,7 +3470,7 @@ impl Editor {
)
})
.collect(),
buffer,
buffer: buffer.clone(),
completions: Arc::new(RwLock::new(completions.into())),
matches: Vec::new().into(),
selected_item: 0,
Expand All @@ -3490,6 +3497,7 @@ impl Editor {
.completion_documentation_pre_resolve_debounce
.fire_new(delay, cx, |editor, cx| {
CompletionsMenu::pre_resolve_completion_documentation(
buffer,
completions,
matches,
editor,
Expand Down Expand Up @@ -10213,6 +10221,7 @@ pub trait CompletionProvider {

fn resolve_completions(
&self,
buffer: Model<Buffer>,
completion_indices: Vec<usize>,
completions: Arc<RwLock<Box<[Completion]>>>,
cx: &mut ViewContext<Editor>,
Expand Down Expand Up @@ -10241,12 +10250,13 @@ impl CompletionProvider for Model<Project> {

fn resolve_completions(
&self,
buffer: Model<Buffer>,
completion_indices: Vec<usize>,
completions: Arc<RwLock<Box<[Completion]>>>,
cx: &mut ViewContext<Editor>,
) -> Task<Result<bool>> {
self.update(cx, |project, cx| {
project.resolve_completions(completion_indices, completions, cx)
project.resolve_completions(buffer, completion_indices, completions, cx)
})
}

Expand Down
12 changes: 12 additions & 0 deletions crates/languages/src/typescript.rs
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,18 @@ impl LspAdapter for TypeScriptLspAdapter {
})))
}

async fn workspace_configuration(
self: Arc<Self>,
_: &Arc<dyn LspAdapterDelegate>,
_cx: &mut AsyncAppContext,
) -> Result<Value> {
Ok(json!({
"completions": {
"completeFunctionCalls": true
}
}))
}

fn language_ids(&self) -> HashMap<String, String> {
HashMap::from_iter([
("TypeScript".into(), "typescript".into()),
Expand Down
68 changes: 42 additions & 26 deletions crates/project/src/lsp_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1505,18 +1505,11 @@ impl LspCommand for GetCompletions {
let edit = match lsp_completion.text_edit.as_ref() {
// If the language server provides a range to overwrite, then
// check that the range is valid.
Some(lsp::CompletionTextEdit::Edit(edit)) => {
let range = range_from_lsp(edit.range);
let start = snapshot.clip_point_utf16(range.start, Bias::Left);
let end = snapshot.clip_point_utf16(range.end, Bias::Left);
if start != range.start.0 || end != range.end.0 {
log::info!("completion out of expected range");
return false;
Some(completion_text_edit) => {
match parse_completion_text_edit(completion_text_edit, &snapshot) {
Some(edit) => edit,
None => return false,
}
(
snapshot.anchor_before(start)..snapshot.anchor_after(end),
edit.new_text.clone(),
)
}

// If the language server does not provide a range, then infer
Expand Down Expand Up @@ -1570,21 +1563,6 @@ impl LspCommand for GetCompletions {
.clone();
(range, text)
}

Some(lsp::CompletionTextEdit::InsertAndReplace(edit)) => {
let range = range_from_lsp(edit.insert);

let start = snapshot.clip_point_utf16(range.start, Bias::Left);
let end = snapshot.clip_point_utf16(range.end, Bias::Left);
if start != range.start.0 || end != range.end.0 {
log::info!("completion out of expected range");
return false;
}
(
snapshot.anchor_before(start)..snapshot.anchor_after(end),
edit.new_text.clone(),
)
}
};

completion_edits.push(edit);
Expand Down Expand Up @@ -1684,6 +1662,44 @@ impl LspCommand for GetCompletions {
}
}

pub(crate) fn parse_completion_text_edit(
edit: &lsp::CompletionTextEdit,
snapshot: &BufferSnapshot,
) -> Option<(Range<Anchor>, String)> {
match edit {
lsp::CompletionTextEdit::Edit(edit) => {
let range = range_from_lsp(edit.range);
let start = snapshot.clip_point_utf16(range.start, Bias::Left);
let end = snapshot.clip_point_utf16(range.end, Bias::Left);
if start != range.start.0 || end != range.end.0 {
log::info!("completion out of expected range");
None
} else {
Some((
snapshot.anchor_before(start)..snapshot.anchor_after(end),
edit.new_text.clone(),
))
}
}

lsp::CompletionTextEdit::InsertAndReplace(edit) => {
let range = range_from_lsp(edit.insert);

let start = snapshot.clip_point_utf16(range.start, Bias::Left);
let end = snapshot.clip_point_utf16(range.end, Bias::Left);
if start != range.start.0 || end != range.end.0 {
log::info!("completion out of expected range");
None
} else {
Some((
snapshot.anchor_before(start)..snapshot.anchor_after(end),
edit.new_text.clone(),
))
}
}
}
}

#[async_trait(?Send)]
impl LspCommand for GetCodeActions {
type Response = Vec<CodeAction>;
Expand Down
Loading

0 comments on commit c812304

Please sign in to comment.