Unnamed repository; edit this file 'description' to name the repository.
LSP: Eagerly decode request results in the client
Previously the `call` helper (and its related functions) returned a
`serde_json::Value` which was then decoded either later in the client
(see signature help and hover) or by the client's caller. This led to
some unnecessary boilerplate in the client:
let resp = self.call::<MyRequest>(params);
Some(async move { Ok(serde_json::from_value(resp.await?)?) })
and in the caller. It also allowed for mistakes with the types. The
workspace symbol request's calling code for example mistakenly decoded a
`lsp::WorkspaceSymbolResponse` as `Vec<lsp::SymbolInformation>` - one of
the untagged enum members (so it parsed successfully) but not the
correct type.
With this change, the `call` helper eagerly decodes the response to a
request as the `lsp::request::Request::Result` trait item. This is
similar to the old helper `request` (which has become redundant and has
been eliminated) but all work is done within the same async block which
avoids some awkward lifetimes. The return types of functions like
`Client::text_document_range_inlay_hints` are now more verbose but it is
no longer possible to accidentally decode as an incorrect type.
Additionally `Client::resolve_code_action` now uses the `call_with_ref`
helper to avoid an unnecessary clone.
| -rw-r--r-- | helix-lsp/src/client.rs | 114 | ||||
| -rw-r--r-- | helix-term/src/commands.rs | 16 | ||||
| -rw-r--r-- | helix-term/src/commands/lsp.rs | 91 | ||||
| -rw-r--r-- | helix-term/src/handlers/completion/request.rs | 1 | ||||
| -rw-r--r-- | helix-view/src/document.rs | 11 | ||||
| -rw-r--r-- | helix-view/src/editor.rs | 2 |
6 files changed, 97 insertions, 138 deletions
diff --git a/helix-lsp/src/client.rs b/helix-lsp/src/client.rs index bf8a8695..e5a116d7 100644 --- a/helix-lsp/src/client.rs +++ b/helix-lsp/src/client.rs @@ -386,22 +386,10 @@ impl Client { } /// Execute a RPC request on the language server. - async fn request<R: lsp::request::Request>(&self, params: R::Params) -> Result<R::Result> - where - R::Params: serde::Serialize, - R::Result: core::fmt::Debug, // TODO: temporary - { - // a future that resolves into the response - let json = self.call::<R>(params).await?; - let response = serde_json::from_value(json)?; - Ok(response) - } - - /// Execute a RPC request on the language server. fn call<R: lsp::request::Request>( &self, params: R::Params, - ) -> impl Future<Output = Result<Value>> + ) -> impl Future<Output = Result<R::Result>> where R::Params: serde::Serialize, { @@ -411,7 +399,7 @@ impl Client { fn call_with_ref<R: lsp::request::Request>( &self, params: &R::Params, - ) -> impl Future<Output = Result<Value>> + ) -> impl Future<Output = Result<R::Result>> where R::Params: serde::Serialize, { @@ -422,7 +410,7 @@ impl Client { &self, params: &R::Params, timeout_secs: u64, - ) -> impl Future<Output = Result<Value>> + ) -> impl Future<Output = Result<R::Result>> where R::Params: serde::Serialize, { @@ -458,6 +446,7 @@ impl Client { .await .map_err(|_| Error::Timeout(id))? // return Timeout .ok_or(Error::StreamClosed)? + .and_then(|value| serde_json::from_value(value).map_err(Into::into)) } } @@ -697,11 +686,11 @@ impl Client { work_done_progress_params: lsp::WorkDoneProgressParams::default(), }; - self.request::<lsp::request::Initialize>(params).await + self.call::<lsp::request::Initialize>(params).await } pub async fn shutdown(&self) -> Result<()> { - self.request::<lsp::request::Shutdown>(()).await + self.call::<lsp::request::Shutdown>(()).await } pub fn exit(&self) { @@ -746,7 +735,7 @@ impl Client { old_path: &Path, new_path: &Path, is_dir: bool, - ) -> Option<impl Future<Output = Result<lsp::WorkspaceEdit>>> { + ) -> Option<impl Future<Output = Result<Option<lsp::WorkspaceEdit>>>> { let capabilities = self.file_operations_intests(); if !capabilities.will_rename.has_interest(old_path, is_dir) { return None; @@ -763,16 +752,10 @@ impl Client { old_uri: url_from_path(old_path)?, new_uri: url_from_path(new_path)?, }]; - let request = self.call_with_timeout::<lsp::request::WillRenameFiles>( + Some(self.call_with_timeout::<lsp::request::WillRenameFiles>( &lsp::RenameFilesParams { files }, 5, - ); - - Some(async move { - let json = request.await?; - let response: Option<lsp::WorkspaceEdit> = serde_json::from_value(json)?; - Ok(response.unwrap_or_default()) - }) + )) } pub fn did_rename(&self, old_path: &Path, new_path: &Path, is_dir: bool) -> Option<()> { @@ -1016,7 +999,7 @@ impl Client { position: lsp::Position, work_done_token: Option<lsp::ProgressToken>, context: lsp::CompletionContext, - ) -> Option<impl Future<Output = Result<Value>>> { + ) -> Option<impl Future<Output = Result<Option<lsp::CompletionResponse>>>> { let capabilities = self.capabilities.get().unwrap(); // Return early if the server does not support completion. @@ -1042,14 +1025,13 @@ impl Client { &self, completion_item: &lsp::CompletionItem, ) -> impl Future<Output = Result<lsp::CompletionItem>> { - let res = self.call_with_ref::<lsp::request::ResolveCompletionItem>(completion_item); - async move { Ok(serde_json::from_value(res.await?)?) } + self.call_with_ref::<lsp::request::ResolveCompletionItem>(completion_item) } pub fn resolve_code_action( &self, - code_action: lsp::CodeAction, - ) -> Option<impl Future<Output = Result<Value>>> { + code_action: &lsp::CodeAction, + ) -> Option<impl Future<Output = Result<lsp::CodeAction>>> { let capabilities = self.capabilities.get().unwrap(); // Return early if the server does not support resolving code actions. @@ -1061,7 +1043,7 @@ impl Client { _ => return None, } - Some(self.call::<lsp::request::CodeActionResolveRequest>(code_action)) + Some(self.call_with_ref::<lsp::request::CodeActionResolveRequest>(code_action)) } pub fn text_document_signature_help( @@ -1085,8 +1067,7 @@ impl Client { // lsp::SignatureHelpContext }; - let res = self.call::<lsp::request::SignatureHelpRequest>(params); - Some(async move { Ok(serde_json::from_value(res.await?)?) }) + Some(self.call::<lsp::request::SignatureHelpRequest>(params)) } pub fn text_document_range_inlay_hints( @@ -1094,7 +1075,7 @@ impl Client { text_document: lsp::TextDocumentIdentifier, range: lsp::Range, work_done_token: Option<lsp::ProgressToken>, - ) -> Option<impl Future<Output = Result<Value>>> { + ) -> Option<impl Future<Output = Result<Option<Vec<lsp::InlayHint>>>>> { let capabilities = self.capabilities.get().unwrap(); match capabilities.inlay_hint_provider { @@ -1140,8 +1121,7 @@ impl Client { // lsp::SignatureHelpContext }; - let res = self.call::<lsp::request::HoverRequest>(params); - Some(async move { Ok(serde_json::from_value(res.await?)?) }) + Some(self.call::<lsp::request::HoverRequest>(params)) } // formatting @@ -1151,7 +1131,7 @@ impl Client { text_document: lsp::TextDocumentIdentifier, options: lsp::FormattingOptions, work_done_token: Option<lsp::ProgressToken>, - ) -> Option<impl Future<Output = Result<Vec<lsp::TextEdit>>>> { + ) -> Option<impl Future<Output = Result<Option<Vec<lsp::TextEdit>>>>> { let capabilities = self.capabilities.get().unwrap(); // Return early if the server does not support formatting. @@ -1184,13 +1164,7 @@ impl Client { work_done_progress_params: lsp::WorkDoneProgressParams { work_done_token }, }; - let request = self.call::<lsp::request::Formatting>(params); - - Some(async move { - let json = request.await?; - let response: Option<Vec<lsp::TextEdit>> = serde_json::from_value(json)?; - Ok(response.unwrap_or_default()) - }) + Some(self.call::<lsp::request::Formatting>(params)) } pub fn text_document_range_formatting( @@ -1199,7 +1173,7 @@ impl Client { range: lsp::Range, options: lsp::FormattingOptions, work_done_token: Option<lsp::ProgressToken>, - ) -> Option<impl Future<Output = Result<Vec<lsp::TextEdit>>>> { + ) -> Option<impl Future<Output = Result<Option<Vec<lsp::TextEdit>>>>> { let capabilities = self.capabilities.get().unwrap(); // Return early if the server does not support range formatting. @@ -1215,13 +1189,7 @@ impl Client { work_done_progress_params: lsp::WorkDoneProgressParams { work_done_token }, }; - let request = self.call::<lsp::request::RangeFormatting>(params); - - Some(async move { - let json = request.await?; - let response: Option<Vec<lsp::TextEdit>> = serde_json::from_value(json)?; - Ok(response.unwrap_or_default()) - }) + Some(self.call::<lsp::request::RangeFormatting>(params)) } pub fn text_document_document_highlight( @@ -1229,7 +1197,7 @@ impl Client { text_document: lsp::TextDocumentIdentifier, position: lsp::Position, work_done_token: Option<lsp::ProgressToken>, - ) -> Option<impl Future<Output = Result<Value>>> { + ) -> Option<impl Future<Output = Result<Option<Vec<lsp::DocumentHighlight>>>>> { let capabilities = self.capabilities.get().unwrap(); // Return early if the server does not support document highlight. @@ -1262,7 +1230,7 @@ impl Client { text_document: lsp::TextDocumentIdentifier, position: lsp::Position, work_done_token: Option<lsp::ProgressToken>, - ) -> impl Future<Output = Result<Value>> { + ) -> impl Future<Output = Result<T::Result>> { let params = lsp::GotoDefinitionParams { text_document_position_params: lsp::TextDocumentPositionParams { text_document, @@ -1282,7 +1250,7 @@ impl Client { text_document: lsp::TextDocumentIdentifier, position: lsp::Position, work_done_token: Option<lsp::ProgressToken>, - ) -> Option<impl Future<Output = Result<Value>>> { + ) -> Option<impl Future<Output = Result<Option<lsp::GotoDefinitionResponse>>>> { let capabilities = self.capabilities.get().unwrap(); // Return early if the server does not support goto-definition. @@ -1303,7 +1271,7 @@ impl Client { text_document: lsp::TextDocumentIdentifier, position: lsp::Position, work_done_token: Option<lsp::ProgressToken>, - ) -> Option<impl Future<Output = Result<Value>>> { + ) -> Option<impl Future<Output = Result<Option<lsp::GotoDefinitionResponse>>>> { let capabilities = self.capabilities.get().unwrap(); // Return early if the server does not support goto-declaration. @@ -1328,7 +1296,7 @@ impl Client { text_document: lsp::TextDocumentIdentifier, position: lsp::Position, work_done_token: Option<lsp::ProgressToken>, - ) -> Option<impl Future<Output = Result<Value>>> { + ) -> Option<impl Future<Output = Result<Option<lsp::GotoDefinitionResponse>>>> { let capabilities = self.capabilities.get().unwrap(); // Return early if the server does not support goto-type-definition. @@ -1352,7 +1320,7 @@ impl Client { text_document: lsp::TextDocumentIdentifier, position: lsp::Position, work_done_token: Option<lsp::ProgressToken>, - ) -> Option<impl Future<Output = Result<Value>>> { + ) -> Option<impl Future<Output = Result<Option<lsp::GotoDefinitionResponse>>>> { let capabilities = self.capabilities.get().unwrap(); // Return early if the server does not support goto-definition. @@ -1377,7 +1345,7 @@ impl Client { position: lsp::Position, include_declaration: bool, work_done_token: Option<lsp::ProgressToken>, - ) -> Option<impl Future<Output = Result<Value>>> { + ) -> Option<impl Future<Output = Result<Option<Vec<lsp::Location>>>>> { let capabilities = self.capabilities.get().unwrap(); // Return early if the server does not support goto-reference. @@ -1406,7 +1374,7 @@ impl Client { pub fn document_symbols( &self, text_document: lsp::TextDocumentIdentifier, - ) -> Option<impl Future<Output = Result<Value>>> { + ) -> Option<impl Future<Output = Result<Option<lsp::DocumentSymbolResponse>>>> { let capabilities = self.capabilities.get().unwrap(); // Return early if the server does not support document symbols. @@ -1428,7 +1396,7 @@ impl Client { &self, text_document: lsp::TextDocumentIdentifier, position: lsp::Position, - ) -> Option<impl Future<Output = Result<Value>>> { + ) -> Option<impl Future<Output = Result<Option<lsp::PrepareRenameResponse>>>> { let capabilities = self.capabilities.get().unwrap(); match capabilities.rename_provider { @@ -1448,7 +1416,10 @@ impl Client { } // empty string to get all symbols - pub fn workspace_symbols(&self, query: String) -> Option<impl Future<Output = Result<Value>>> { + pub fn workspace_symbols( + &self, + query: String, + ) -> Option<impl Future<Output = Result<Option<lsp::WorkspaceSymbolResponse>>>> { let capabilities = self.capabilities.get().unwrap(); // Return early if the server does not support workspace symbols. @@ -1471,7 +1442,7 @@ impl Client { text_document: lsp::TextDocumentIdentifier, range: lsp::Range, context: lsp::CodeActionContext, - ) -> Option<impl Future<Output = Result<Value>>> { + ) -> Option<impl Future<Output = Result<Option<Vec<lsp::CodeActionOrCommand>>>>> { let capabilities = self.capabilities.get().unwrap(); // Return early if the server does not support code actions. @@ -1499,7 +1470,7 @@ impl Client { text_document: lsp::TextDocumentIdentifier, position: lsp::Position, new_name: String, - ) -> Option<impl Future<Output = Result<lsp::WorkspaceEdit>>> { + ) -> Option<impl Future<Output = Result<Option<lsp::WorkspaceEdit>>>> { if !self.supports_feature(LanguageServerFeature::RenameSymbol) { return None; } @@ -1515,16 +1486,13 @@ impl Client { }, }; - let request = self.call::<lsp::request::Rename>(params); - - Some(async move { - let json = request.await?; - let response: Option<lsp::WorkspaceEdit> = serde_json::from_value(json)?; - Ok(response.unwrap_or_default()) - }) + Some(self.call::<lsp::request::Rename>(params)) } - pub fn command(&self, command: lsp::Command) -> Option<impl Future<Output = Result<Value>>> { + pub fn command( + &self, + command: lsp::Command, + ) -> Option<impl Future<Output = Result<Option<Value>>>> { let capabilities = self.capabilities.get().unwrap(); // Return early if the language server does not support executing commands. diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index a197792e..2e15dcdc 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -146,10 +146,10 @@ impl Context<'_> { #[inline] pub fn callback<T, F>( &mut self, - call: impl Future<Output = helix_lsp::Result<serde_json::Value>> + 'static + Send, + call: impl Future<Output = helix_lsp::Result<T>> + 'static + Send, callback: F, ) where - T: for<'de> serde::Deserialize<'de> + Send + 'static, + T: Send + 'static, F: FnOnce(&mut Editor, &mut Compositor, T) + Send + 'static, { self.jobs.callback(make_job_callback(call, callback)); @@ -175,16 +175,15 @@ impl Context<'_> { #[inline] fn make_job_callback<T, F>( - call: impl Future<Output = helix_lsp::Result<serde_json::Value>> + 'static + Send, + call: impl Future<Output = helix_lsp::Result<T>> + 'static + Send, callback: F, ) -> std::pin::Pin<Box<impl Future<Output = Result<Callback, anyhow::Error>>>> where - T: for<'de> serde::Deserialize<'de> + Send + 'static, + T: Send + 'static, F: FnOnce(&mut Editor, &mut Compositor, T) + Send + 'static, { Box::pin(async move { - let json = call.await?; - let response = serde_json::from_value(json)?; + let response = call.await?; let call: job::Callback = Callback::EditorCompositor(Box::new( move |editor: &mut Editor, compositor: &mut Compositor| { callback(editor, compositor, response) @@ -4905,7 +4904,10 @@ fn format_selections(cx: &mut Context) { ) .unwrap(); - let edits = tokio::task::block_in_place(|| helix_lsp::block_on(future)).unwrap_or_default(); + let edits = tokio::task::block_in_place(|| helix_lsp::block_on(future)) + .ok() + .flatten() + .unwrap_or_default(); let transaction = helix_lsp::util::generate_transaction_from_edits(doc.text(), edits, offset_encoding); diff --git a/helix-term/src/commands/lsp.rs b/helix-term/src/commands/lsp.rs index 6d6b9744..8377f7c7 100644 --- a/helix-term/src/commands/lsp.rs +++ b/helix-term/src/commands/lsp.rs @@ -343,9 +343,7 @@ pub fn symbol_picker(cx: &mut Context) { .expect("docs with active language servers must be backed by paths"); async move { - let json = request.await?; - let response: Option<lsp::DocumentSymbolResponse> = serde_json::from_value(json)?; - let symbols = match response { + let symbols = match request.await? { Some(symbols) => symbols, None => return anyhow::Ok(vec![]), }; @@ -461,30 +459,34 @@ pub fn workspace_symbol_picker(cx: &mut Context) { .unwrap(); let offset_encoding = language_server.offset_encoding(); async move { - let json = request.await?; - - let response: Vec<_> = - serde_json::from_value::<Option<Vec<lsp::SymbolInformation>>>(json)? - .unwrap_or_default() - .into_iter() - .filter_map(|symbol| { - let uri = match Uri::try_from(&symbol.location.uri) { - Ok(uri) => uri, - Err(err) => { - log::warn!("discarding symbol with invalid URI: {err}"); - return None; - } - }; - Some(SymbolInformationItem { - location: Location { - uri, - range: symbol.location.range, - offset_encoding, - }, - symbol, - }) + let symbols = request + .await? + .and_then(|resp| match resp { + lsp::WorkspaceSymbolResponse::Flat(symbols) => Some(symbols), + lsp::WorkspaceSymbolResponse::Nested(_) => None, + }) + .unwrap_or_default(); + + let response: Vec<_> = symbols + .into_iter() + .filter_map(|symbol| { + let uri = match Uri::try_from(&symbol.location.uri) { + Ok(uri) => uri, + Err(err) => { + log::warn!("discarding symbol with invalid URI: {err}"); + return None; + } + }; + Some(SymbolInformationItem { + location: Location { + uri, + range: symbol.location.range, + offset_encoding, + }, + symbol, }) - .collect(); + }) + .collect(); anyhow::Ok(response) } @@ -676,11 +678,8 @@ pub fn code_action(cx: &mut Context) { Some((code_action_request, language_server_id)) }) .map(|(request, ls_id)| async move { - let json = request.await?; - let response: Option<lsp::CodeActionResponse> = serde_json::from_value(json)?; - let mut actions = match response { - Some(a) => a, - None => return anyhow::Ok(Vec::new()), + let Some(mut actions) = request.await? else { + return anyhow::Ok(Vec::new()); }; // remove disabled code actions @@ -782,15 +781,9 @@ pub fn code_action(cx: &mut Context) { // we support lsp "codeAction/resolve" for `edit` and `command` fields let mut resolved_code_action = None; if code_action.edit.is_none() || code_action.command.is_none() { - if let Some(future) = - language_server.resolve_code_action(code_action.clone()) - { - if let Ok(response) = helix_lsp::block_on(future) { - if let Ok(code_action) = - serde_json::from_value::<CodeAction>(response) - { - resolved_code_action = Some(code_action); - } + if let Some(future) = language_server.resolve_code_action(code_action) { + if let Ok(code_action) = helix_lsp::block_on(future) { + resolved_code_action = Some(code_action); } } } @@ -882,7 +875,7 @@ fn goto_impl(editor: &mut Editor, compositor: &mut Compositor, locations: Vec<Lo fn goto_single_impl<P, F>(cx: &mut Context, feature: LanguageServerFeature, request_provider: P) where P: Fn(&Client, lsp::Position, lsp::TextDocumentIdentifier) -> Option<F>, - F: Future<Output = helix_lsp::Result<serde_json::Value>> + 'static + Send, + F: Future<Output = helix_lsp::Result<Option<lsp::GotoDefinitionResponse>>> + 'static + Send, { let (view, doc) = current_ref!(cx.editor); let mut futures: FuturesOrdered<_> = doc @@ -891,11 +884,7 @@ where let offset_encoding = language_server.offset_encoding(); let pos = doc.position(view.id, offset_encoding); let future = request_provider(language_server, pos, doc.identifier()).unwrap(); - async move { - let json = future.await?; - let response: Option<lsp::GotoDefinitionResponse> = serde_json::from_value(json)?; - anyhow::Ok((response, offset_encoding)) - } + async move { anyhow::Ok((future.await?, offset_encoding)) } }) .collect(); @@ -992,11 +981,7 @@ pub fn goto_reference(cx: &mut Context) { None, ) .unwrap(); - async move { - let json = future.await?; - let locations: Option<Vec<lsp::Location>> = serde_json::from_value(json)?; - anyhow::Ok((locations, offset_encoding)) - } + async move { anyhow::Ok((future.await?, offset_encoding)) } }) .collect(); @@ -1158,7 +1143,9 @@ pub fn rename_symbol(cx: &mut Context) { match block_on(future) { Ok(edits) => { - let _ = cx.editor.apply_workspace_edit(offset_encoding, &edits); + let _ = cx + .editor + .apply_workspace_edit(offset_encoding, &edits.unwrap_or_default()); } Err(err) => cx.editor.set_error(err.to_string()), } diff --git a/helix-term/src/handlers/completion/request.rs b/helix-term/src/handlers/completion/request.rs index 3d2a158e..26f252a4 100644 --- a/helix-term/src/handlers/completion/request.rs +++ b/helix-term/src/handlers/completion/request.rs @@ -303,7 +303,6 @@ fn request_completions_from_language_server( async move { let response: Option<lsp::CompletionResponse> = completion_response .await - .and_then(|json| serde_json::from_value(json).map_err(helix_lsp::Error::Parse)) .inspect_err(|err| log::error!("completion request failed: {err}")) .ok() .flatten(); diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index 9b3b4548..b75aebe7 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -867,10 +867,13 @@ impl Document { )?; let fut = async move { - let edits = request.await.unwrap_or_else(|e| { - log::warn!("LSP formatting failed: {}", e); - Default::default() - }); + let edits = request + .await + .unwrap_or_else(|e| { + log::warn!("LSP formatting failed: {}", e); + Default::default() + }) + .unwrap_or_default(); Ok(helix_lsp::util::generate_transaction_from_edits( &text, edits, diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs index 723fb139..65976f2c 100644 --- a/helix-view/src/editor.rs +++ b/helix-view/src/editor.rs @@ -1410,7 +1410,7 @@ impl Editor { continue; }; let edit = match helix_lsp::block_on(request) { - Ok(edit) => edit, + Ok(edit) => edit.unwrap_or_default(), Err(err) => { log::error!("invalid willRename response: {err:?}"); continue; |