From b834806dbccfbaaa84c35f4db039a99ec54ebd5e Mon Sep 17 00:00:00 2001
From: Pascal Kuthe <pascalkuthe@pm.me>
Date: Mon, 8 Apr 2024 02:46:32 +0200
Subject: [PATCH] use newtype parttern for langauge server id

---
 Cargo.lock                            |   1 +
 Cargo.toml                            |   1 +
 helix-core/Cargo.toml                 |   3 +-
 helix-core/src/diagnostic.rs          |  21 +++-
 helix-lsp/Cargo.toml                  |   1 +
 helix-lsp/src/client.rs               |  14 ++-
 helix-lsp/src/file_event.rs           |  16 +--
 helix-lsp/src/lib.rs                  | 141 +++++++++++++++-----------
 helix-lsp/src/transport.rs            |  14 +--
 helix-term/src/application.rs         |  11 +-
 helix-term/src/commands/lsp.rs        |  14 ++-
 helix-term/src/handlers/completion.rs |   2 +-
 helix-term/src/ui/completion.rs       |   5 +-
 helix-term/src/ui/spinner.rs          |   8 +-
 helix-view/src/document.rs            |  28 ++---
 helix-view/src/editor.rs              |  22 ++--
 helix-view/src/gutter.rs              |   2 +-
 17 files changed, 176 insertions(+), 128 deletions(-)

diff --git a/Cargo.lock b/Cargo.lock
index 6af72fc4..017a248a 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -1393,6 +1393,7 @@ dependencies = [
  "parking_lot",
  "serde",
  "serde_json",
+ "slotmap",
  "thiserror",
  "tokio",
  "tokio-stream",
diff --git a/Cargo.toml b/Cargo.toml
index 3bfc4b8e..6206281b 100644
--- a/Cargo.toml
+++ b/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"
diff --git a/helix-core/Cargo.toml b/helix-core/Cargo.toml
index 7482262e..16d6b457 100644
--- a/helix-core/Cargo.toml
+++ b/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"
diff --git a/helix-core/src/diagnostic.rs b/helix-core/src/diagnostic.rs
index 52d77a9a..d41119d3 100644
--- a/helix-core/src/diagnostic.rs
+++ b/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)
+    }
+}
diff --git a/helix-lsp/Cargo.toml b/helix-lsp/Cargo.toml
index 0b502f76..f87f15f2 100644
--- a/helix-lsp/Cargo.toml
+++ b/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
diff --git a/helix-lsp/src/client.rs b/helix-lsp/src/client.rs
index b2290e03..79d8adb3 100644
--- a/helix-lsp/src/client.rs
+++ b/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
     }
 
diff --git a/helix-lsp/src/file_event.rs b/helix-lsp/src/file_event.rs
index 93457fa5..8f4c1675 100644
--- a/helix-lsp/src/file_event.rs
+++ b/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 } => {
diff --git a/helix-lsp/src/lib.rs b/helix-lsp/src/lib.rs
index 262bc1b9..04f018bc 100644
--- a/helix-lsp/src/lib.rs
+++ b/helix-lsp/src/lib.rs
@@ -17,6 +17,7 @@ use helix_core::syntax::{
     LanguageConfiguration, LanguageServerConfiguration, LanguageServerFeatures,
 };
 use helix_stdx::path;
+use slotmap::SlotMap;
 use tokio::sync::mpsc::UnboundedReceiver;
 
 use std::{
@@ -28,8 +29,9 @@ use std::{
 use thiserror::Error;
 use tokio_stream::wrappers::UnboundedReceiverStream;
 
-pub type Result<T> = core::result::Result<T, Error>;
+pub type Result<T, E = Error> = core::result::Result<T, E>;
 pub type LanguageServerName = String;
+pub use helix_core::diagnostic::LanguageServerId;
 
 #[derive(Error, Debug)]
 pub enum Error {
@@ -651,38 +653,42 @@ impl Notification {
 
 #[derive(Debug)]
 pub struct Registry {
-    inner: HashMap<LanguageServerName, Vec<Arc<Client>>>,
+    inner: SlotMap<LanguageServerId, Arc<Client>>,
+    inner_by_name: HashMap<LanguageServerName, Vec<Arc<Client>>>,
     syn_loader: Arc<ArcSwap<helix_core::syntax::Loader>>,
-    counter: usize,
-    pub incoming: SelectAll<UnboundedReceiverStream<(usize, Call)>>,
+    pub incoming: SelectAll<UnboundedReceiverStream<(LanguageServerId, Call)>>,
     pub file_event_handler: file_event::Handler,
 }
 
 impl Registry {
     pub fn new(syn_loader: Arc<ArcSwap<helix_core::syntax::Loader>>) -> Self {
         Self {
-            inner: HashMap::new(),
+            inner: SlotMap::with_key(),
+            inner_by_name: HashMap::new(),
             syn_loader,
-            counter: 0,
             incoming: SelectAll::new(),
             file_event_handler: file_event::Handler::new(),
         }
     }
 
-    pub fn get_by_id(&self, id: usize) -> Option<&Client> {
-        self.inner
-            .values()
-            .flatten()
-            .find(|client| client.id() == id)
-            .map(|client| &**client)
+    pub fn get_by_id(&self, id: LanguageServerId) -> Option<&Arc<Client>> {
+        self.inner.get(id)
     }
 
-    pub fn remove_by_id(&mut self, id: usize) {
+    pub fn remove_by_id(&mut self, id: LanguageServerId) {
+        let Some(client) = self.inner.remove(id) else {
+            log::error!("client was already removed");
+            return
+        };
         self.file_event_handler.remove_client(id);
-        self.inner.retain(|_, language_servers| {
-            language_servers.retain(|ls| id != ls.id());
-            !language_servers.is_empty()
-        });
+        let instances = self
+            .inner_by_name
+            .get_mut(client.name())
+            .expect("inner and inner_by_name must be synced");
+        instances.retain(|ls| id != ls.id());
+        if instances.is_empty() {
+            self.inner_by_name.remove(client.name());
+        }
     }
 
     fn start_client(
@@ -692,28 +698,28 @@ impl Registry {
         doc_path: Option<&std::path::PathBuf>,
         root_dirs: &[PathBuf],
         enable_snippets: bool,
-    ) -> Result<Option<Arc<Client>>> {
+    ) -> Result<Arc<Client>, StartupError> {
         let syn_loader = self.syn_loader.load();
         let config = syn_loader
             .language_server_configs()
             .get(&name)
             .ok_or_else(|| anyhow::anyhow!("Language server '{name}' not defined"))?;
-        let id = self.counter;
-        self.counter += 1;
-        if let Some(NewClient(client, incoming)) = start_client(
-            id,
-            name,
-            ls_config,
-            config,
-            doc_path,
-            root_dirs,
-            enable_snippets,
-        )? {
-            self.incoming.push(UnboundedReceiverStream::new(incoming));
-            Ok(Some(client))
-        } else {
-            Ok(None)
-        }
+        let id = self.inner.try_insert_with_key(|id| {
+            start_client(
+                id,
+                name,
+                ls_config,
+                config,
+                doc_path,
+                root_dirs,
+                enable_snippets,
+            )
+            .map(|client| {
+                self.incoming.push(UnboundedReceiverStream::new(client.1));
+                client.0
+            })
+        })?;
+        Ok(self.inner[id].clone())
     }
 
     /// If this method is called, all documents that have a reference to language servers used by the language config have to refresh their language servers,
@@ -730,7 +736,7 @@ impl Registry {
             .language_servers
             .iter()
             .filter_map(|LanguageServerFeatures { name, .. }| {
-                if self.inner.contains_key(name) {
+                if self.inner_by_name.contains_key(name) {
                     let client = match self.start_client(
                         name.clone(),
                         language_config,
@@ -738,16 +744,18 @@ impl Registry {
                         root_dirs,
                         enable_snippets,
                     ) {
-                        Ok(client) => client?,
-                        Err(error) => return Some(Err(error)),
+                        Ok(client) => client,
+                        Err(StartupError::NoRequiredRootFound) => return None,
+                        Err(StartupError::Error(err)) => return Some(Err(err)),
                     };
                     let old_clients = self
-                        .inner
+                        .inner_by_name
                         .insert(name.clone(), vec![client.clone()])
                         .unwrap();
 
                     for old_client in old_clients {
                         self.file_event_handler.remove_client(old_client.id());
+                        self.inner.remove(client.id());
                         tokio::spawn(async move {
                             let _ = old_client.force_shutdown().await;
                         });
@@ -762,9 +770,10 @@ impl Registry {
     }
 
     pub fn stop(&mut self, name: &str) {
-        if let Some(clients) = self.inner.remove(name) {
+        if let Some(clients) = self.inner_by_name.remove(name) {
             for client in clients {
                 self.file_event_handler.remove_client(client.id());
+                self.inner.remove(client.id());
                 tokio::spawn(async move {
                     let _ = client.force_shutdown().await;
                 });
@@ -781,7 +790,7 @@ impl Registry {
     ) -> impl Iterator<Item = (LanguageServerName, Result<Arc<Client>>)> + 'a {
         language_config.language_servers.iter().filter_map(
             move |LanguageServerFeatures { name, .. }| {
-                if let Some(clients) = self.inner.get(name) {
+                if let Some(clients) = self.inner_by_name.get(name) {
                     if let Some((_, client)) = clients.iter().enumerate().find(|(i, client)| {
                         client.try_add_doc(&language_config.roots, root_dirs, doc_path, *i == 0)
                     }) {
@@ -796,21 +805,21 @@ impl Registry {
                     enable_snippets,
                 ) {
                     Ok(client) => {
-                        let client = client?;
-                        self.inner
+                        self.inner_by_name
                             .entry(name.to_owned())
                             .or_default()
                             .push(client.clone());
                         Some((name.clone(), Ok(client)))
                     }
-                    Err(err) => Some((name.to_owned(), Err(err))),
+                    Err(StartupError::NoRequiredRootFound) => None,
+                    Err(StartupError::Error(err)) => Some((name.to_owned(), Err(err))),
                 }
             },
         )
     }
 
     pub fn iter_clients(&self) -> impl Iterator<Item = &Arc<Client>> {
-        self.inner.values().flatten()
+        self.inner.values()
     }
 }
 
@@ -833,7 +842,7 @@ impl ProgressStatus {
 /// Acts as a container for progress reported by language servers. Each server
 /// has a unique id assigned at creation through [`Registry`]. This id is then used
 /// to store the progress in this map.
-pub struct LspProgressMap(HashMap<usize, HashMap<lsp::ProgressToken, ProgressStatus>>);
+pub struct LspProgressMap(HashMap<LanguageServerId, HashMap<lsp::ProgressToken, ProgressStatus>>);
 
 impl LspProgressMap {
     pub fn new() -> Self {
@@ -841,28 +850,35 @@ impl LspProgressMap {
     }
 
     /// Returns a map of all tokens corresponding to the language server with `id`.
-    pub fn progress_map(&self, id: usize) -> Option<&HashMap<lsp::ProgressToken, ProgressStatus>> {
+    pub fn progress_map(
+        &self,
+        id: LanguageServerId,
+    ) -> Option<&HashMap<lsp::ProgressToken, ProgressStatus>> {
         self.0.get(&id)
     }
 
-    pub fn is_progressing(&self, id: usize) -> bool {
+    pub fn is_progressing(&self, id: LanguageServerId) -> bool {
         self.0.get(&id).map(|it| !it.is_empty()).unwrap_or_default()
     }
 
     /// Returns last progress status for a given server with `id` and `token`.
-    pub fn progress(&self, id: usize, token: &lsp::ProgressToken) -> Option<&ProgressStatus> {
+    pub fn progress(
+        &self,
+        id: LanguageServerId,
+        token: &lsp::ProgressToken,
+    ) -> Option<&ProgressStatus> {
         self.0.get(&id).and_then(|values| values.get(token))
     }
 
     /// Checks if progress `token` for server with `id` is created.
-    pub fn is_created(&mut self, id: usize, token: &lsp::ProgressToken) -> bool {
+    pub fn is_created(&mut self, id: LanguageServerId, token: &lsp::ProgressToken) -> bool {
         self.0
             .get(&id)
             .map(|values| values.get(token).is_some())
             .unwrap_or_default()
     }
 
-    pub fn create(&mut self, id: usize, token: lsp::ProgressToken) {
+    pub fn create(&mut self, id: LanguageServerId, token: lsp::ProgressToken) {
         self.0
             .entry(id)
             .or_default()
@@ -872,7 +888,7 @@ impl LspProgressMap {
     /// Ends the progress by removing the `token` from server with `id`, if removed returns the value.
     pub fn end_progress(
         &mut self,
-        id: usize,
+        id: LanguageServerId,
         token: &lsp::ProgressToken,
     ) -> Option<ProgressStatus> {
         self.0.get_mut(&id).and_then(|vals| vals.remove(token))
@@ -881,7 +897,7 @@ impl LspProgressMap {
     /// Updates the progress of `token` for server with `id` to `status`, returns the value replaced or `None`.
     pub fn update(
         &mut self,
-        id: usize,
+        id: LanguageServerId,
         token: lsp::ProgressToken,
         status: lsp::WorkDoneProgress,
     ) -> Option<ProgressStatus> {
@@ -892,19 +908,30 @@ impl LspProgressMap {
     }
 }
 
-struct NewClient(Arc<Client>, UnboundedReceiver<(usize, Call)>);
+struct NewClient(Arc<Client>, UnboundedReceiver<(LanguageServerId, Call)>);
+
+enum StartupError {
+    NoRequiredRootFound,
+    Error(Error),
+}
+
+impl<T: Into<Error>> From<T> for StartupError {
+    fn from(value: T) -> Self {
+        StartupError::Error(value.into())
+    }
+}
 
 /// start_client takes both a LanguageConfiguration and a LanguageServerConfiguration to ensure that
 /// it is only called when it makes sense.
 fn start_client(
-    id: usize,
+    id: LanguageServerId,
     name: String,
     config: &LanguageConfiguration,
     ls_config: &LanguageServerConfiguration,
     doc_path: Option<&std::path::PathBuf>,
     root_dirs: &[PathBuf],
     enable_snippets: bool,
-) -> Result<Option<NewClient>> {
+) -> Result<NewClient, StartupError> {
     let (workspace, workspace_is_cwd) = helix_loader::find_workspace();
     let workspace = path::normalize(workspace);
     let root = find_lsp_workspace(
@@ -929,7 +956,7 @@ fn start_client(
             .map(|entry| entry.file_name())
             .any(|entry| globset.is_match(entry))
         {
-            return Ok(None);
+            return Err(StartupError::NoRequiredRootFound);
         }
     }
 
@@ -981,7 +1008,7 @@ fn start_client(
         initialize_notify.notify_one();
     });
 
-    Ok(Some(NewClient(client, incoming)))
+    Ok(NewClient(client, incoming))
 }
 
 /// Find an LSP workspace of a file using the following mechanism:
diff --git a/helix-lsp/src/transport.rs b/helix-lsp/src/transport.rs
index f2f35d6a..bd671abe 100644
--- a/helix-lsp/src/transport.rs
+++ b/helix-lsp/src/transport.rs
@@ -1,4 +1,4 @@
-use crate::{jsonrpc, Error, Result};
+use crate::{jsonrpc, Error, LanguageServerId, Result};
 use anyhow::Context;
 use log::{error, info};
 use serde::{Deserialize, Serialize};
@@ -37,7 +37,7 @@ enum ServerMessage {
 
 #[derive(Debug)]
 pub struct Transport {
-    id: usize,
+    id: LanguageServerId,
     name: String,
     pending_requests: Mutex<HashMap<jsonrpc::Id, Sender<Result<Value>>>>,
 }
@@ -47,10 +47,10 @@ impl Transport {
         server_stdout: BufReader<ChildStdout>,
         server_stdin: BufWriter<ChildStdin>,
         server_stderr: BufReader<ChildStderr>,
-        id: usize,
+        id: LanguageServerId,
         name: String,
     ) -> (
-        UnboundedReceiver<(usize, jsonrpc::Call)>,
+        UnboundedReceiver<(LanguageServerId, jsonrpc::Call)>,
         UnboundedSender<Payload>,
         Arc<Notify>,
     ) {
@@ -194,7 +194,7 @@ impl Transport {
 
     async fn process_server_message(
         &self,
-        client_tx: &UnboundedSender<(usize, jsonrpc::Call)>,
+        client_tx: &UnboundedSender<(LanguageServerId, jsonrpc::Call)>,
         msg: ServerMessage,
         language_server_name: &str,
     ) -> Result<()> {
@@ -251,7 +251,7 @@ impl Transport {
     async fn recv(
         transport: Arc<Self>,
         mut server_stdout: BufReader<ChildStdout>,
-        client_tx: UnboundedSender<(usize, jsonrpc::Call)>,
+        client_tx: UnboundedSender<(LanguageServerId, jsonrpc::Call)>,
     ) {
         let mut recv_buffer = String::new();
         loop {
@@ -329,7 +329,7 @@ impl Transport {
     async fn send(
         transport: Arc<Self>,
         mut server_stdin: BufWriter<ChildStdin>,
-        client_tx: UnboundedSender<(usize, jsonrpc::Call)>,
+        client_tx: UnboundedSender<(LanguageServerId, jsonrpc::Call)>,
         mut client_rx: UnboundedReceiver<Payload>,
         initialize_notify: Arc<Notify>,
     ) {
diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs
index 809393c7..6bdf60bc 100644
--- a/helix-term/src/application.rs
+++ b/helix-term/src/application.rs
@@ -4,7 +4,7 @@ use helix_core::{diagnostic::Severity, pos_at_coords, syntax, Selection};
 use helix_lsp::{
     lsp::{self, notification::Notification},
     util::lsp_range_to_range,
-    LspProgressMap,
+    LanguageServerId, LspProgressMap,
 };
 use helix_stdx::path::get_relative_path;
 use helix_view::{
@@ -655,7 +655,7 @@ impl Application {
     pub async fn handle_language_server_message(
         &mut self,
         call: helix_lsp::Call,
-        server_id: usize,
+        server_id: LanguageServerId,
     ) {
         use helix_lsp::{Call, MethodCall, Notification};
 
@@ -1030,12 +1030,7 @@ impl Application {
                         Ok(json!(result))
                     }
                     Ok(MethodCall::RegisterCapability(params)) => {
-                        if let Some(client) = self
-                            .editor
-                            .language_servers
-                            .iter_clients()
-                            .find(|client| client.id() == server_id)
-                        {
+                        if let Some(client) = self.editor.language_servers.get_by_id(server_id) {
                             for reg in params.registrations {
                                 match reg.method.as_str() {
                                     lsp::notification::DidChangeWatchedFiles::METHOD => {
diff --git a/helix-term/src/commands/lsp.rs b/helix-term/src/commands/lsp.rs
index 6a5ceae6..88bd0791 100644
--- a/helix-term/src/commands/lsp.rs
+++ b/helix-term/src/commands/lsp.rs
@@ -6,7 +6,7 @@ use helix_lsp::{
         NumberOrString,
     },
     util::{diagnostic_to_lsp_diagnostic, lsp_range_to_range, range_to_lsp_range},
-    Client, OffsetEncoding,
+    Client, LanguageServerId, OffsetEncoding,
 };
 use tokio_stream::StreamExt;
 use tui::{
@@ -266,7 +266,7 @@ enum DiagnosticsFormat {
 
 fn diag_picker(
     cx: &Context,
-    diagnostics: BTreeMap<PathBuf, Vec<(lsp::Diagnostic, usize)>>,
+    diagnostics: BTreeMap<PathBuf, Vec<(lsp::Diagnostic, LanguageServerId)>>,
     format: DiagnosticsFormat,
 ) -> Picker<PickerDiagnostic> {
     // TODO: drop current_path comparison and instead use workspace: bool flag?
@@ -497,7 +497,7 @@ pub fn workspace_diagnostics_picker(cx: &mut Context) {
 
 struct CodeActionOrCommandItem {
     lsp_item: lsp::CodeActionOrCommand,
-    language_server_id: usize,
+    language_server_id: LanguageServerId,
 }
 
 impl ui::menu::Item for CodeActionOrCommandItem {
@@ -757,7 +757,11 @@ impl ui::menu::Item for lsp::Command {
     }
 }
 
-pub fn execute_lsp_command(editor: &mut Editor, language_server_id: usize, cmd: lsp::Command) {
+pub fn execute_lsp_command(
+    editor: &mut Editor,
+    language_server_id: LanguageServerId,
+    cmd: lsp::Command,
+) {
     // the command is executed on the server and communicated back
     // to the client asynchronously using workspace edits
     let future = match editor
@@ -1034,7 +1038,7 @@ pub fn rename_symbol(cx: &mut Context) {
     fn create_rename_prompt(
         editor: &Editor,
         prefill: String,
-        language_server_id: Option<usize>,
+        language_server_id: Option<LanguageServerId>,
     ) -> Box<ui::Prompt> {
         let prompt = ui::Prompt::new(
             "rename-to:".into(),
diff --git a/helix-term/src/handlers/completion.rs b/helix-term/src/handlers/completion.rs
index 491ca563..8bd85ef6 100644
--- a/helix-term/src/handlers/completion.rs
+++ b/helix-term/src/handlers/completion.rs
@@ -251,7 +251,7 @@ fn request_completion(
                 .into_iter()
                 .map(|item| CompletionItem {
                     item,
-                    language_server_id,
+                    provider: language_server_id,
                     resolved: false,
                 })
                 .collect();
diff --git a/helix-term/src/ui/completion.rs b/helix-term/src/ui/completion.rs
index 1fcb8e54..2b3437e1 100644
--- a/helix-term/src/ui/completion.rs
+++ b/helix-term/src/ui/completion.rs
@@ -94,7 +94,7 @@ impl menu::Item for CompletionItem {
 #[derive(Debug, PartialEq, Default, Clone)]
 pub struct CompletionItem {
     pub item: lsp::CompletionItem,
-    pub language_server_id: usize,
+    pub provider: LanguageServerId,
     pub resolved: bool,
 }
 
@@ -224,7 +224,7 @@ impl Completion {
                 ($item:expr) => {
                     match editor
                         .language_servers
-                        .get_by_id($item.language_server_id)
+                        .get_by_id($item.provider)
                     {
                         Some(ls) => ls,
                         None => {
@@ -285,7 +285,6 @@ impl Completion {
                     let language_server = language_server!(item);
                     let offset_encoding = language_server.offset_encoding();
 
-                    // resolve item if not yet resolved
                     if !item.resolved {
                         if let Some(resolved) =
                             Self::resolve_completion_item(language_server, item.item.clone())
diff --git a/helix-term/src/ui/spinner.rs b/helix-term/src/ui/spinner.rs
index 379c4489..9ce61055 100644
--- a/helix-term/src/ui/spinner.rs
+++ b/helix-term/src/ui/spinner.rs
@@ -1,16 +1,18 @@
 use std::{collections::HashMap, time::Instant};
 
+use helix_lsp::LanguageServerId;
+
 #[derive(Default, Debug)]
 pub struct ProgressSpinners {
-    inner: HashMap<usize, Spinner>,
+    inner: HashMap<LanguageServerId, Spinner>,
 }
 
 impl ProgressSpinners {
-    pub fn get(&self, id: usize) -> Option<&Spinner> {
+    pub fn get(&self, id: LanguageServerId) -> Option<&Spinner> {
         self.inner.get(&id)
     }
 
-    pub fn get_or_create(&mut self, id: usize) -> &mut Spinner {
+    pub fn get_or_create(&mut self, id: LanguageServerId) -> &mut Spinner {
         self.inner.entry(id).or_default()
     }
 }
diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs
index d44b4240..3393fbed 100644
--- a/helix-view/src/document.rs
+++ b/helix-view/src/document.rs
@@ -624,7 +624,7 @@ where
     *mut_ref = f(mem::take(mut_ref));
 }
 
-use helix_lsp::{lsp, Client, LanguageServerName};
+use helix_lsp::{lsp, Client, LanguageServerId, LanguageServerName};
 use url::Url;
 
 impl Document {
@@ -1296,11 +1296,7 @@ impl Document {
             });
 
             self.diagnostics.sort_unstable_by_key(|diagnostic| {
-                (
-                    diagnostic.range,
-                    diagnostic.severity,
-                    diagnostic.language_server_id,
-                )
+                (diagnostic.range, diagnostic.severity, diagnostic.provider)
             });
 
             // Update the inlay hint annotations' positions, helping ensure they are displayed in the proper place
@@ -1644,7 +1640,7 @@ impl Document {
         })
     }
 
-    pub fn supports_language_server(&self, id: usize) -> bool {
+    pub fn supports_language_server(&self, id: LanguageServerId) -> bool {
         self.language_servers().any(|l| l.id() == id)
     }
 
@@ -1767,7 +1763,7 @@ impl Document {
         text: &Rope,
         language_config: Option<&LanguageConfiguration>,
         diagnostic: &helix_lsp::lsp::Diagnostic,
-        language_server_id: usize,
+        language_server_id: LanguageServerId,
         offset_encoding: helix_lsp::OffsetEncoding,
     ) -> Option<Diagnostic> {
         use helix_core::diagnostic::{Range, Severity::*};
@@ -1844,7 +1840,7 @@ impl Document {
             tags,
             source: diagnostic.source.clone(),
             data: diagnostic.data.clone(),
-            language_server_id,
+            provider: language_server_id,
         })
     }
 
@@ -1857,13 +1853,13 @@ impl Document {
         &mut self,
         diagnostics: impl IntoIterator<Item = Diagnostic>,
         unchanged_sources: &[String],
-        language_server_id: Option<usize>,
+        language_server_id: Option<LanguageServerId>,
     ) {
         if unchanged_sources.is_empty() {
             self.clear_diagnostics(language_server_id);
         } else {
             self.diagnostics.retain(|d| {
-                if language_server_id.map_or(false, |id| id != d.language_server_id) {
+                if language_server_id.map_or(false, |id| id != d.provider) {
                     return true;
                 }
 
@@ -1876,18 +1872,14 @@ impl Document {
         }
         self.diagnostics.extend(diagnostics);
         self.diagnostics.sort_unstable_by_key(|diagnostic| {
-            (
-                diagnostic.range,
-                diagnostic.severity,
-                diagnostic.language_server_id,
-            )
+            (diagnostic.range, diagnostic.severity, diagnostic.provider)
         });
     }
 
     /// clears diagnostics for a given language server id if set, otherwise all diagnostics are cleared
-    pub fn clear_diagnostics(&mut self, language_server_id: Option<usize>) {
+    pub fn clear_diagnostics(&mut self, language_server_id: Option<LanguageServerId>) {
         if let Some(id) = language_server_id {
-            self.diagnostics.retain(|d| d.language_server_id != id);
+            self.diagnostics.retain(|d| d.provider != id);
         } else {
             self.diagnostics.clear();
         }
diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs
index d7058d3e..2e04037a 100644
--- a/helix-view/src/editor.rs
+++ b/helix-view/src/editor.rs
@@ -16,7 +16,7 @@ use helix_vcs::DiffProviderRegistry;
 
 use futures_util::stream::select_all::SelectAll;
 use futures_util::{future, StreamExt};
-use helix_lsp::Call;
+use helix_lsp::{Call, LanguageServerId};
 use tokio_stream::wrappers::UnboundedReceiverStream;
 
 use std::{
@@ -960,7 +960,7 @@ pub struct Editor {
     pub macro_recording: Option<(char, Vec<KeyEvent>)>,
     pub macro_replaying: Vec<char>,
     pub language_servers: helix_lsp::Registry,
-    pub diagnostics: BTreeMap<PathBuf, Vec<(lsp::Diagnostic, usize)>>,
+    pub diagnostics: BTreeMap<PathBuf, Vec<(lsp::Diagnostic, LanguageServerId)>>,
     pub diff_providers: DiffProviderRegistry,
 
     pub debugger: Option<dap::Client>,
@@ -1020,7 +1020,7 @@ pub type Motion = Box<dyn Fn(&mut Editor)>;
 pub enum EditorEvent {
     DocumentSaved(DocumentSavedEventResult),
     ConfigEvent(ConfigEvent),
-    LanguageServerMessage((usize, Call)),
+    LanguageServerMessage((LanguageServerId, Call)),
     DebuggerEvent(dap::Payload),
     IdleTimer,
     Redraw,
@@ -1260,8 +1260,13 @@ impl Editor {
     }
 
     #[inline]
-    pub fn language_server_by_id(&self, language_server_id: usize) -> Option<&helix_lsp::Client> {
-        self.language_servers.get_by_id(language_server_id)
+    pub fn language_server_by_id(
+        &self,
+        language_server_id: LanguageServerId,
+    ) -> Option<&helix_lsp::Client> {
+        self.language_servers
+            .get_by_id(language_server_id)
+            .map(|client| &**client)
     }
 
     /// Refreshes the language server for a given document
@@ -1861,7 +1866,7 @@ impl Editor {
     /// Returns all supported diagnostics for the document
     pub fn doc_diagnostics<'a>(
         language_servers: &'a helix_lsp::Registry,
-        diagnostics: &'a BTreeMap<PathBuf, Vec<(lsp::Diagnostic, usize)>>,
+        diagnostics: &'a BTreeMap<PathBuf, Vec<(lsp::Diagnostic, LanguageServerId)>>,
         document: &Document,
     ) -> impl Iterator<Item = helix_core::Diagnostic> + 'a {
         Editor::doc_diagnostics_with_filter(language_servers, diagnostics, document, |_, _| true)
@@ -1871,10 +1876,9 @@ impl Editor {
     /// filtered by `filter` which is invocated with the raw `lsp::Diagnostic` and the language server id it came from
     pub fn doc_diagnostics_with_filter<'a>(
         language_servers: &'a helix_lsp::Registry,
-        diagnostics: &'a BTreeMap<PathBuf, Vec<(lsp::Diagnostic, usize)>>,
-
+        diagnostics: &'a BTreeMap<PathBuf, Vec<(lsp::Diagnostic, LanguageServerId)>>,
         document: &Document,
-        filter: impl Fn(&lsp::Diagnostic, usize) -> bool + 'a,
+        filter: impl Fn(&lsp::Diagnostic, LanguageServerId) -> bool + 'a,
     ) -> impl Iterator<Item = helix_core::Diagnostic> + 'a {
         let text = document.text().clone();
         let language_config = document.language.clone();
diff --git a/helix-view/src/gutter.rs b/helix-view/src/gutter.rs
index ebdac9e2..36f719f7 100644
--- a/helix-view/src/gutter.rs
+++ b/helix-view/src/gutter.rs
@@ -71,7 +71,7 @@ pub fn diagnostic<'doc>(
                     d.line == line
                         && doc
                             .language_servers_with_feature(LanguageServerFeature::Diagnostics)
-                            .any(|ls| ls.id() == d.language_server_id)
+                            .any(|ls| ls.id() == d.provider)
                 });
             diagnostics_on_line.max_by_key(|d| d.severity).map(|d| {
                 write!(out, "●").ok();