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.

helix-lsp/src/client.rs

@@ -385,23 +385,11 @@ impl Client {
         self.workspace_folders.lock()
     }
 
-    /// 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.

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);

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 symbols = request
+                        .await?
+                        .and_then(|resp| match resp {
+                            lsp::WorkspaceSymbolResponse::Flat(symbols) => Some(symbols),
+                            lsp::WorkspaceSymbolResponse::Nested(_) => None,
+                        })
+                        .unwrap_or_default();
 
-                    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 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()),
                 }

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();

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,

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;