diff --git a/crates/editor/src/code_context_menus.rs b/crates/editor/src/code_context_menus.rs index 83d84950e0..b931c836eb 100644 --- a/crates/editor/src/code_context_menus.rs +++ b/crates/editor/src/code_context_menus.rs @@ -500,7 +500,7 @@ impl CompletionsMenu { highlight.font_weight = None; if completion .source - .lsp_completion() + .lsp_completion(false) .and_then(|lsp_completion| lsp_completion.deprecated) .unwrap_or(false) { @@ -711,10 +711,12 @@ impl CompletionsMenu { let completion = &completions[mat.candidate_id]; let sort_key = completion.sort_key(); - let sort_text = completion - .source - .lsp_completion() - .and_then(|lsp_completion| lsp_completion.sort_text.as_deref()); + let sort_text = + if let CompletionSource::Lsp { lsp_completion, .. } = &completion.source { + lsp_completion.sort_text.as_deref() + } else { + None + }; let score = Reverse(OrderedFloat(mat.score)); if mat.score >= 0.2 { diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 1404ccf3a1..b937bac242 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -17017,6 +17017,7 @@ fn snippet_completions( sort_text: Some(char::MAX.to_string()), ..lsp::CompletionItem::default() }), + lsp_defaults: None, }, label: CodeLabel { text: matching_prefix.clone(), diff --git a/crates/editor/src/editor_tests.rs b/crates/editor/src/editor_tests.rs index 451f2ede88..8c05279443 100644 --- a/crates/editor/src/editor_tests.rs +++ b/crates/editor/src/editor_tests.rs @@ -12334,24 +12334,6 @@ async fn test_completions_default_resolve_data_handling(cx: &mut TestAppContext) }, }; - let item_0_out = lsp::CompletionItem { - commit_characters: Some(default_commit_characters.clone()), - insert_text_format: Some(default_insert_text_format), - ..item_0 - }; - let items_out = iter::once(item_0_out) - .chain(items[1..].iter().map(|item| lsp::CompletionItem { - commit_characters: Some(default_commit_characters.clone()), - data: Some(default_data.clone()), - insert_text_mode: Some(default_insert_text_mode), - text_edit: Some(lsp::CompletionTextEdit::Edit(lsp::TextEdit { - range: default_edit_range, - new_text: item.label.clone(), - })), - ..item.clone() - })) - .collect::>(); - let mut cx = EditorLspTestContext::new_rust( lsp::ServerCapabilities { completion_provider: Some(lsp::CompletionOptions { @@ -12370,10 +12352,11 @@ async fn test_completions_default_resolve_data_handling(cx: &mut TestAppContext) let completion_data = default_data.clone(); let completion_characters = default_commit_characters.clone(); + let completion_items = items.clone(); cx.handle_request::(move |_, _, _| { let default_data = completion_data.clone(); let default_commit_characters = completion_characters.clone(); - let items = items.clone(); + let items = completion_items.clone(); async move { Ok(Some(lsp::CompletionResponse::List(lsp::CompletionList { items, @@ -12422,7 +12405,7 @@ async fn test_completions_default_resolve_data_handling(cx: &mut TestAppContext) .iter() .map(|mat| mat.string.clone()) .collect::>(), - items_out + items .iter() .map(|completion| completion.label.clone()) .collect::>() @@ -12435,14 +12418,18 @@ async fn test_completions_default_resolve_data_handling(cx: &mut TestAppContext) // with 4 from the end. assert_eq!( *resolved_items.lock(), - [ - &items_out[0..16], - &items_out[items_out.len() - 4..items_out.len()] - ] - .concat() - .iter() - .cloned() - .collect::>() + [&items[0..16], &items[items.len() - 4..items.len()]] + .concat() + .iter() + .cloned() + .map(|mut item| { + if item.data.is_none() { + item.data = Some(default_data.clone()); + } + item + }) + .collect::>(), + "Items sent for resolve should be unchanged modulo resolve `data` filled with default if missing" ); resolved_items.lock().clear(); @@ -12453,9 +12440,15 @@ async fn test_completions_default_resolve_data_handling(cx: &mut TestAppContext) // Completions that have already been resolved are skipped. assert_eq!( *resolved_items.lock(), - items_out[items_out.len() - 16..items_out.len() - 4] + items[items.len() - 16..items.len() - 4] .iter() .cloned() + .map(|mut item| { + if item.data.is_none() { + item.data = Some(default_data.clone()); + } + item + }) .collect::>() ); resolved_items.lock().clear(); diff --git a/crates/project/src/lsp_command.rs b/crates/project/src/lsp_command.rs index 037ecacbb8..5eb16bc74c 100644 --- a/crates/project/src/lsp_command.rs +++ b/crates/project/src/lsp_command.rs @@ -1847,7 +1847,6 @@ impl LspCommand for GetCompletions { let mut completions = if let Some(completions) = completions { match completions { lsp::CompletionResponse::Array(completions) => completions, - lsp::CompletionResponse::List(mut list) => { let items = std::mem::take(&mut list.items); response_list = Some(list); @@ -1855,74 +1854,19 @@ impl LspCommand for GetCompletions { } } } else { - Default::default() + Vec::new() }; let language_server_adapter = lsp_store .update(&mut cx, |lsp_store, _| { lsp_store.language_server_adapter_for_id(server_id) })? - .ok_or_else(|| anyhow!("no such language server"))?; + .with_context(|| format!("no language server with id {server_id}"))?; - let item_defaults = response_list + let lsp_defaults = response_list .as_ref() - .and_then(|list| list.item_defaults.as_ref()); - - if let Some(item_defaults) = item_defaults { - let default_data = item_defaults.data.as_ref(); - let default_commit_characters = item_defaults.commit_characters.as_ref(); - let default_edit_range = item_defaults.edit_range.as_ref(); - let default_insert_text_format = item_defaults.insert_text_format.as_ref(); - let default_insert_text_mode = item_defaults.insert_text_mode.as_ref(); - - if default_data.is_some() - || default_commit_characters.is_some() - || default_edit_range.is_some() - || default_insert_text_format.is_some() - || default_insert_text_mode.is_some() - { - for item in completions.iter_mut() { - if item.data.is_none() && default_data.is_some() { - item.data = default_data.cloned() - } - if item.commit_characters.is_none() && default_commit_characters.is_some() { - item.commit_characters = default_commit_characters.cloned() - } - if item.text_edit.is_none() { - if let Some(default_edit_range) = default_edit_range { - match default_edit_range { - CompletionListItemDefaultsEditRange::Range(range) => { - item.text_edit = - Some(lsp::CompletionTextEdit::Edit(lsp::TextEdit { - range: *range, - new_text: item.label.clone(), - })) - } - CompletionListItemDefaultsEditRange::InsertAndReplace { - insert, - replace, - } => { - item.text_edit = - Some(lsp::CompletionTextEdit::InsertAndReplace( - lsp::InsertReplaceEdit { - new_text: item.label.clone(), - insert: *insert, - replace: *replace, - }, - )) - } - } - } - } - if item.insert_text_format.is_none() && default_insert_text_format.is_some() { - item.insert_text_format = default_insert_text_format.cloned() - } - if item.insert_text_mode.is_none() && default_insert_text_mode.is_some() { - item.insert_text_mode = default_insert_text_mode.cloned() - } - } - } - } + .and_then(|list| list.item_defaults.clone()) + .map(Arc::new); let mut completion_edits = Vec::new(); buffer.update(&mut cx, |buffer, _cx| { @@ -1930,12 +1874,34 @@ impl LspCommand for GetCompletions { let clipped_position = buffer.clip_point_utf16(Unclipped(self.position), Bias::Left); let mut range_for_token = None; - completions.retain_mut(|lsp_completion| { - let edit = match lsp_completion.text_edit.as_ref() { + completions.retain(|lsp_completion| { + let lsp_edit = lsp_completion.text_edit.clone().or_else(|| { + let default_text_edit = lsp_defaults.as_deref()?.edit_range.as_ref()?; + match default_text_edit { + CompletionListItemDefaultsEditRange::Range(range) => { + Some(lsp::CompletionTextEdit::Edit(lsp::TextEdit { + range: *range, + new_text: lsp_completion.label.clone(), + })) + } + CompletionListItemDefaultsEditRange::InsertAndReplace { + insert, + replace, + } => Some(lsp::CompletionTextEdit::InsertAndReplace( + lsp::InsertReplaceEdit { + new_text: lsp_completion.label.clone(), + insert: *insert, + replace: *replace, + }, + )), + } + }); + + let edit = match lsp_edit { // If the language server provides a range to overwrite, then // check that the range is valid. Some(completion_text_edit) => { - match parse_completion_text_edit(completion_text_edit, &snapshot) { + match parse_completion_text_edit(&completion_text_edit, &snapshot) { Some(edit) => edit, None => return false, } @@ -1949,14 +1915,15 @@ impl LspCommand for GetCompletions { return false; } - let default_edit_range = response_list - .as_ref() - .and_then(|list| list.item_defaults.as_ref()) - .and_then(|defaults| defaults.edit_range.as_ref()) - .and_then(|range| match range { - CompletionListItemDefaultsEditRange::Range(r) => Some(r), - _ => None, - }); + let default_edit_range = lsp_defaults.as_ref().and_then(|lsp_defaults| { + lsp_defaults + .edit_range + .as_ref() + .and_then(|range| match range { + CompletionListItemDefaultsEditRange::Range(r) => Some(r), + _ => None, + }) + }); let range = if let Some(range) = default_edit_range { let range = range_from_lsp(*range); @@ -2006,14 +1973,25 @@ impl LspCommand for GetCompletions { Ok(completions .into_iter() .zip(completion_edits) - .map(|(lsp_completion, (old_range, mut new_text))| { + .map(|(mut lsp_completion, (old_range, mut new_text))| { LineEnding::normalize(&mut new_text); + if lsp_completion.data.is_none() { + if let Some(default_data) = lsp_defaults + .as_ref() + .and_then(|item_defaults| item_defaults.data.clone()) + { + // Servers (e.g. JDTLS) prefer unchanged completions, when resolving the items later, + // so we do not insert the defaults here, but `data` is needed for resolving, so this is an exception. + lsp_completion.data = Some(default_data); + } + } CoreCompletion { old_range, new_text, source: CompletionSource::Lsp { server_id, lsp_completion: Box::new(lsp_completion), + lsp_defaults: lsp_defaults.clone(), resolved: false, }, } diff --git a/crates/project/src/lsp_store.rs b/crates/project/src/lsp_store.rs index 304a3662ca..b55d8f86dc 100644 --- a/crates/project/src/lsp_store.rs +++ b/crates/project/src/lsp_store.rs @@ -49,10 +49,9 @@ use lsp::{ notification::DidRenameFiles, CodeActionKind, CompletionContext, DiagnosticSeverity, DiagnosticTag, DidChangeWatchedFilesRegistrationOptions, Edit, FileOperationFilter, FileOperationPatternKind, FileOperationRegistrationOptions, FileRename, FileSystemWatcher, - InsertTextFormat, LanguageServer, LanguageServerBinary, LanguageServerBinaryOptions, - LanguageServerId, LanguageServerName, LspRequestFuture, MessageActionItem, MessageType, OneOf, - RenameFilesParams, SymbolKind, TextEdit, WillRenameFiles, WorkDoneProgressCancelParams, - WorkspaceFolder, + LanguageServer, LanguageServerBinary, LanguageServerBinaryOptions, LanguageServerId, + LanguageServerName, LspRequestFuture, MessageActionItem, MessageType, OneOf, RenameFilesParams, + SymbolKind, TextEdit, WillRenameFiles, WorkDoneProgressCancelParams, WorkspaceFolder, }; use node_runtime::read_package_installed_version; use parking_lot::Mutex; @@ -70,6 +69,7 @@ use smol::channel::Sender; use snippet::Snippet; use std::{ any::Any, + borrow::Cow, cell::RefCell, cmp::Ordering, convert::TryInto, @@ -4475,6 +4475,7 @@ impl LspStore { completions: Rc>>, completion_index: usize, ) -> Result<()> { + let server_id = server.server_id(); let can_resolve = server .capabilities() .completion_provider @@ -4491,19 +4492,24 @@ impl LspStore { CompletionSource::Lsp { lsp_completion, resolved, + server_id: completion_server_id, .. } => { if *resolved { return Ok(()); } + anyhow::ensure!( + server_id == *completion_server_id, + "server_id mismatch, querying completion resolve for {server_id} but completion server id is {completion_server_id}" + ); server.request::(*lsp_completion.clone()) } CompletionSource::Custom => return Ok(()), } }; - let completion_item = request.await?; + let resolved_completion = request.await?; - if let Some(text_edit) = completion_item.text_edit.as_ref() { + if let Some(text_edit) = resolved_completion.text_edit.as_ref() { // Technically we don't have to parse the whole `text_edit`, since the only // language server we currently use that does update `text_edit` in `completionItem/resolve` // is `typescript-language-server` and they only update `text_edit.new_text`. @@ -4520,24 +4526,26 @@ impl LspStore { completion.old_range = old_range; } } - if completion_item.insert_text_format == Some(InsertTextFormat::SNIPPET) { - // vtsls might change the type of completion after resolution. - let mut completions = completions.borrow_mut(); - let completion = &mut completions[completion_index]; - if let Some(lsp_completion) = completion.source.lsp_completion_mut() { - if completion_item.insert_text_format != lsp_completion.insert_text_format { - lsp_completion.insert_text_format = completion_item.insert_text_format; - } - } - } let mut completions = completions.borrow_mut(); let completion = &mut completions[completion_index]; - completion.source = CompletionSource::Lsp { - lsp_completion: Box::new(completion_item), - resolved: true, - server_id: server.server_id(), - }; + if let CompletionSource::Lsp { + lsp_completion, + resolved, + server_id: completion_server_id, + .. + } = &mut completion.source + { + if *resolved { + return Ok(()); + } + anyhow::ensure!( + server_id == *completion_server_id, + "server_id mismatch, applying completion resolve for {server_id} but completion server id is {completion_server_id}" + ); + *lsp_completion = Box::new(resolved_completion); + *resolved = true; + } Ok(()) } @@ -4549,8 +4557,8 @@ impl LspStore { ) -> Result<()> { let completion_item = completions.borrow()[completion_index] .source - .lsp_completion() - .cloned(); + .lsp_completion(true) + .map(Cow::into_owned); if let Some(lsp_documentation) = completion_item .as_ref() .and_then(|completion_item| completion_item.documentation.clone()) @@ -4626,8 +4634,13 @@ impl LspStore { CompletionSource::Lsp { lsp_completion, resolved, + server_id: completion_server_id, .. } => { + anyhow::ensure!( + server_id == *completion_server_id, + "remote server_id mismatch, querying completion resolve for {server_id} but completion server id is {completion_server_id}" + ); if *resolved { return Ok(()); } @@ -4647,7 +4660,7 @@ impl LspStore { .request(request) .await .context("completion documentation resolve proto request")?; - let lsp_completion = serde_json::from_slice(&response.lsp_completion)?; + let resolved_lsp_completion = serde_json::from_slice(&response.lsp_completion)?; let documentation = if response.documentation.is_empty() { CompletionDocumentation::Undocumented @@ -4662,11 +4675,23 @@ impl LspStore { let mut completions = completions.borrow_mut(); let completion = &mut completions[completion_index]; completion.documentation = Some(documentation); - completion.source = CompletionSource::Lsp { - server_id, + if let CompletionSource::Lsp { lsp_completion, - resolved: true, - }; + resolved, + server_id: completion_server_id, + lsp_defaults: _, + } = &mut completion.source + { + if *resolved { + return Ok(()); + } + anyhow::ensure!( + server_id == *completion_server_id, + "remote server_id mismatch, applying completion resolve for {server_id} but completion server id is {completion_server_id}" + ); + *lsp_completion = Box::new(resolved_lsp_completion); + *resolved = true; + } let old_range = response .old_start @@ -4750,7 +4775,7 @@ impl LspStore { let completion = completions.borrow()[completion_index].clone(); let additional_text_edits = completion .source - .lsp_completion() + .lsp_completion(true) .as_ref() .and_then(|lsp_completion| lsp_completion.additional_text_edits.clone()); if let Some(edits) = additional_text_edits { @@ -8153,21 +8178,26 @@ impl LspStore { } pub(crate) fn serialize_completion(completion: &CoreCompletion) -> proto::Completion { - let (source, server_id, lsp_completion, resolved) = match &completion.source { + let (source, server_id, lsp_completion, lsp_defaults, resolved) = match &completion.source { CompletionSource::Lsp { server_id, lsp_completion, + lsp_defaults, resolved, } => ( proto::completion::Source::Lsp as i32, server_id.0 as u64, serde_json::to_vec(lsp_completion).unwrap(), + lsp_defaults + .as_deref() + .map(|lsp_defaults| serde_json::to_vec(lsp_defaults).unwrap()), *resolved, ), CompletionSource::Custom => ( proto::completion::Source::Custom as i32, 0, Vec::new(), + None, true, ), }; @@ -8178,6 +8208,7 @@ impl LspStore { new_text: completion.new_text.clone(), server_id, lsp_completion, + lsp_defaults, resolved, source, } @@ -8200,6 +8231,11 @@ impl LspStore { Some(proto::completion::Source::Lsp) => CompletionSource::Lsp { server_id: LanguageServerId::from_proto(completion.server_id), lsp_completion: serde_json::from_slice(&completion.lsp_completion)?, + lsp_defaults: completion + .lsp_defaults + .as_deref() + .map(serde_json::from_slice) + .transpose()?, resolved: completion.resolved, }, _ => anyhow::bail!("Unexpected completion source {}", completion.source), @@ -8288,8 +8324,8 @@ async fn populate_labels_for_completions( let lsp_completions = new_completions .iter() .filter_map(|new_completion| { - if let CompletionSource::Lsp { lsp_completion, .. } = &new_completion.source { - Some(*lsp_completion.clone()) + if let Some(lsp_completion) = new_completion.source.lsp_completion(true) { + Some(lsp_completion.into_owned()) } else { None } @@ -8309,8 +8345,8 @@ async fn populate_labels_for_completions( .fuse(); for completion in new_completions { - match &completion.source { - CompletionSource::Lsp { lsp_completion, .. } => { + match completion.source.lsp_completion(true) { + Some(lsp_completion) => { let documentation = if let Some(docs) = lsp_completion.documentation.clone() { Some(docs.into()) } else { @@ -8328,9 +8364,9 @@ async fn populate_labels_for_completions( new_text: completion.new_text, source: completion.source, confirm: None, - }) + }); } - CompletionSource::Custom => { + None => { let mut label = CodeLabel::plain(completion.new_text.clone(), None); ensure_uniform_list_compatible_label(&mut label); completions.push(Completion { @@ -8340,7 +8376,7 @@ async fn populate_labels_for_completions( new_text: completion.new_text, source: completion.source, confirm: None, - }) + }); } } } diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index 2db83dcb3f..42d9986349 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -382,6 +382,8 @@ pub enum CompletionSource { server_id: LanguageServerId, /// The raw completion provided by the language server. lsp_completion: Box, + /// A set of defaults for this completion item. + lsp_defaults: Option>, /// Whether this completion has been resolved, to ensure it happens once per completion. resolved: bool, }, @@ -397,17 +399,76 @@ impl CompletionSource { } } - pub fn lsp_completion(&self) -> Option<&lsp::CompletionItem> { - if let Self::Lsp { lsp_completion, .. } = self { - Some(lsp_completion) - } else { - None - } - } + pub fn lsp_completion(&self, apply_defaults: bool) -> Option> { + if let Self::Lsp { + lsp_completion, + lsp_defaults, + .. + } = self + { + if apply_defaults { + if let Some(lsp_defaults) = lsp_defaults { + let mut completion_with_defaults = *lsp_completion.clone(); + let default_commit_characters = lsp_defaults.commit_characters.as_ref(); + let default_edit_range = lsp_defaults.edit_range.as_ref(); + let default_insert_text_format = lsp_defaults.insert_text_format.as_ref(); + let default_insert_text_mode = lsp_defaults.insert_text_mode.as_ref(); - fn lsp_completion_mut(&mut self) -> Option<&mut lsp::CompletionItem> { - if let Self::Lsp { lsp_completion, .. } = self { - Some(lsp_completion) + if default_commit_characters.is_some() + || default_edit_range.is_some() + || default_insert_text_format.is_some() + || default_insert_text_mode.is_some() + { + if completion_with_defaults.commit_characters.is_none() + && default_commit_characters.is_some() + { + completion_with_defaults.commit_characters = + default_commit_characters.cloned() + } + if completion_with_defaults.text_edit.is_none() { + match default_edit_range { + Some(lsp::CompletionListItemDefaultsEditRange::Range(range)) => { + completion_with_defaults.text_edit = + Some(lsp::CompletionTextEdit::Edit(lsp::TextEdit { + range: *range, + new_text: completion_with_defaults.label.clone(), + })) + } + Some( + lsp::CompletionListItemDefaultsEditRange::InsertAndReplace { + insert, + replace, + }, + ) => { + completion_with_defaults.text_edit = + Some(lsp::CompletionTextEdit::InsertAndReplace( + lsp::InsertReplaceEdit { + new_text: completion_with_defaults.label.clone(), + insert: *insert, + replace: *replace, + }, + )) + } + None => {} + } + } + if completion_with_defaults.insert_text_format.is_none() + && default_insert_text_format.is_some() + { + completion_with_defaults.insert_text_format = + default_insert_text_format.cloned() + } + if completion_with_defaults.insert_text_mode.is_none() + && default_insert_text_mode.is_some() + { + completion_with_defaults.insert_text_mode = + default_insert_text_mode.cloned() + } + } + return Some(Cow::Owned(completion_with_defaults)); + } + } + Some(Cow::Borrowed(lsp_completion)) } else { None } @@ -4640,7 +4701,8 @@ impl Completion { const DEFAULT_KIND_KEY: usize = 2; let kind_key = self .source - .lsp_completion() + // `lsp::CompletionListItemDefaults` has no `kind` field + .lsp_completion(false) .and_then(|lsp_completion| lsp_completion.kind) .and_then(|lsp_completion_kind| match lsp_completion_kind { lsp::CompletionItemKind::KEYWORD => Some(0), @@ -4654,7 +4716,8 @@ impl Completion { /// Whether this completion is a snippet. pub fn is_snippet(&self) -> bool { self.source - .lsp_completion() + // `lsp::CompletionListItemDefaults` has `insert_text_format` field + .lsp_completion(true) .map_or(false, |lsp_completion| { lsp_completion.insert_text_format == Some(lsp::InsertTextFormat::SNIPPET) }) @@ -4664,9 +4727,10 @@ impl Completion { /// /// Will return `None` if this completion's kind is not [`CompletionItemKind::COLOR`]. pub fn color(&self) -> Option { - let lsp_completion = self.source.lsp_completion()?; + // `lsp::CompletionListItemDefaults` has no `kind` field + let lsp_completion = self.source.lsp_completion(false)?; if lsp_completion.kind? == CompletionItemKind::COLOR { - return color_extractor::extract_color(lsp_completion); + return color_extractor::extract_color(&lsp_completion); } None } diff --git a/crates/proto/proto/zed.proto b/crates/proto/proto/zed.proto index ae16516b7f..6cebbe0bb3 100644 --- a/crates/proto/proto/zed.proto +++ b/crates/proto/proto/zed.proto @@ -1000,6 +1000,7 @@ message Completion { bytes lsp_completion = 5; bool resolved = 6; Source source = 7; + optional bytes lsp_defaults = 8; enum Source { Custom = 0;