avoid overloading ls with completion resolve requests

Cargo.toml

@@ -39,6 +39,7 @@ package.helix-term.opt-level = 2
 [workspace.dependencies]
 tree-sitter = { version = "0.22" }
 nucleo = "0.2.0"
+slotmap = "1.0.7"
 
 [workspace.package]
 version = "24.3.0"

helix-core/Cargo.toml

@@ -25,8 +25,7 @@ smartstring = "1.0.1"
 unicode-segmentation = "1.11"
 unicode-width = "0.1"
 unicode-general-category = "0.6"
-# slab = "0.4.2"
-slotmap = "1.0"
+slotmap.workspace = true
 tree-sitter.workspace = true
 once_cell = "1.19"
 arc-swap = "1"

helix-core/src/diagnostic.rs

@@ -1,4 +1,6 @@
 //! LSP diagnostic utility types.
+use std::fmt;
+
 use serde::{Deserialize, Serialize};
 
 /// Describes the severity level of a [`Diagnostic`].
@@ -47,8 +49,25 @@ pub struct Diagnostic {
     pub message: String,
     pub severity: Option<Severity>,
     pub code: Option<NumberOrString>,
-    pub language_server_id: usize,
+    pub provider: DiagnosticProvider,
     pub tags: Vec<DiagnosticTag>,
     pub source: Option<String>,
     pub data: Option<serde_json::Value>,
 }
+
+// TODO turn this into an enum + feature flag when lsp becomes optional
+pub type DiagnosticProvider = LanguageServerId;
+
+// while I would prefer having this in helix-lsp that necessitates a bunch of
+// conversions I would rather not add. I think its fine since this just a very
+// trivial newtype wrapper and we would need something similar once we define
+// completions in core
+slotmap::new_key_type! {
+    pub struct LanguageServerId;
+}
+
+impl fmt::Display for LanguageServerId {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(f, "{:?}", self.0)
+    }
+}

helix-lsp/Cargo.toml

@@ -31,3 +31,4 @@ tokio = { version = "1.37", features = ["rt", "rt-multi-thread", "io-util", "io-
 tokio-stream = "0.1.15"
 parking_lot = "0.12.1"
 arc-swap = "1"
+slotmap.workspace = true

helix-lsp/src/client.rs

@@ -2,7 +2,7 @@ use crate::{
     file_operations::FileOperationsInterest,
     find_lsp_workspace, jsonrpc,
     transport::{Payload, Transport},
-    Call, Error, OffsetEncoding, Result,
+    Call, Error, LanguageServerId, OffsetEncoding, Result,
 };
 
 use helix_core::{find_workspace, syntax::LanguageServerFeature, ChangeSet, Rope};
@@ -46,7 +46,7 @@ fn workspace_for_uri(uri: lsp::Url) -> WorkspaceFolder {
 
 #[derive(Debug)]
 pub struct Client {
-    id: usize,
+    id: LanguageServerId,
     name: String,
     _process: Child,
     server_tx: UnboundedSender<Payload>,
@@ -179,10 +179,14 @@ impl Client {
         server_environment: HashMap<String, String>,
         root_path: PathBuf,
         root_uri: Option<lsp::Url>,
-        id: usize,
+        id: LanguageServerId,
         name: String,
         req_timeout: u64,
-    ) -> Result<(Self, UnboundedReceiver<(usize, Call)>, Arc<Notify>)> {
+    ) -> Result<(
+        Self,
+        UnboundedReceiver<(LanguageServerId, Call)>,
+        Arc<Notify>,
+    )> {
         // Resolve path to the binary
         let cmd = helix_stdx::env::which(cmd)?;
 
@@ -234,7 +238,7 @@ impl Client {
         &self.name
     }
 
-    pub fn id(&self) -> usize {
+    pub fn id(&self) -> LanguageServerId {
         self.id
     }
 
@@ -393,6 +397,16 @@ impl Client {
         &self,
         params: R::Params,
     ) -> impl Future<Output = Result<Value>>
+    where
+        R::Params: serde::Serialize,
+    {
+        self.call_with_ref::<R>(&params)
+    }
+
+    fn call_with_ref<R: lsp::request::Request>(
+        &self,
+        params: &R::Params,
+    ) -> impl Future<Output = Result<Value>>
     where
         R::Params: serde::Serialize,
     {
@@ -401,7 +415,7 @@ impl Client {
 
     fn call_with_timeout<R: lsp::request::Request>(
         &self,
-        params: R::Params,
+        params: &R::Params,
         timeout_secs: u64,
     ) -> impl Future<Output = Result<Value>>
     where
@@ -410,17 +424,16 @@ impl Client {
         let server_tx = self.server_tx.clone();
         let id = self.next_request_id();
 
+        let params = serde_json::to_value(params);
         async move {
             use std::time::Duration;
             use tokio::time::timeout;
 
-            let params = serde_json::to_value(params)?;
-
             let request = jsonrpc::MethodCall {
                 jsonrpc: Some(jsonrpc::Version::V2),
                 id: id.clone(),
                 method: R::METHOD.to_string(),
-                params: Self::value_into_params(params),
+                params: Self::value_into_params(params?),
             };
 
             let (tx, mut rx) = channel::<Result<Value>>(1);
@@ -737,7 +750,7 @@ impl Client {
             new_uri: url_from_path(new_path)?,
         }];
         let request = self.call_with_timeout::<lsp::request::WillRenameFiles>(
-            lsp::RenameFilesParams { files },
+            &lsp::RenameFilesParams { files },
             5,
         );
 
@@ -1022,21 +1035,10 @@ impl Client {
 
     pub fn resolve_completion_item(
         &self,
-        completion_item: lsp::CompletionItem,
-    ) -> Option<impl Future<Output = Result<lsp::CompletionItem>>> {
-        let capabilities = self.capabilities.get().unwrap();
-
-        // Return early if the server does not support resolving completion items.
-        match capabilities.completion_provider {
-            Some(lsp::CompletionOptions {
-                resolve_provider: Some(true),
-                ..
-            }) => (),
-            _ => return None,
-        }
-
-        let res = self.call::<lsp::request::ResolveCompletionItem>(completion_item);
-        Some(async move { Ok(serde_json::from_value(res.await?)?) })
+        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?)?) }
     }
 
     pub fn resolve_code_action(

helix-lsp/src/file_event.rs

@@ -3,24 +3,24 @@ use std::{collections::HashMap, path::PathBuf, sync::Weak};
 use globset::{GlobBuilder, GlobSetBuilder};
 use tokio::sync::mpsc;
 
-use crate::{lsp, Client};
+use crate::{lsp, Client, LanguageServerId};
 
 enum Event {
     FileChanged {
         path: PathBuf,
     },
     Register {
-        client_id: usize,
+        client_id: LanguageServerId,
         client: Weak<Client>,
         registration_id: String,
         options: lsp::DidChangeWatchedFilesRegistrationOptions,
     },
     Unregister {
-        client_id: usize,
+        client_id: LanguageServerId,
         registration_id: String,
     },
     RemoveClient {
-        client_id: usize,
+        client_id: LanguageServerId,
     },
 }
 
@@ -59,7 +59,7 @@ impl Handler {
 
     pub fn register(
         &self,
-        client_id: usize,
+        client_id: LanguageServerId,
         client: Weak<Client>,
         registration_id: String,
         options: lsp::DidChangeWatchedFilesRegistrationOptions,
@@ -72,7 +72,7 @@ impl Handler {
         });
     }
 
-    pub fn unregister(&self, client_id: usize, registration_id: String) {
+    pub fn unregister(&self, client_id: LanguageServerId, registration_id: String) {
         let _ = self.tx.send(Event::Unregister {
             client_id,
             registration_id,
@@ -83,12 +83,12 @@ impl Handler {
         let _ = self.tx.send(Event::FileChanged { path });
     }
 
-    pub fn remove_client(&self, client_id: usize) {
+    pub fn remove_client(&self, client_id: LanguageServerId) {
         let _ = self.tx.send(Event::RemoveClient { client_id });
     }
 
     async fn run(mut rx: mpsc::UnboundedReceiver<Event>) {
-        let mut state: HashMap<usize, ClientState> = HashMap::new();
+        let mut state: HashMap<LanguageServerId, ClientState> = HashMap::new();
         while let Some(event) = rx.recv().await {
             match event {
                 Event::FileChanged { path } => {