Switch LspAdapter to struct and revert some async/await

Isaac Clayton created

Change summary

crates/language/src/language.rs | 138 ++++++++++++++++++++++++++--------
crates/project/src/project.rs   |  50 +++++-------
2 files changed, 125 insertions(+), 63 deletions(-)

Detailed changes

crates/language/src/language.rs 🔗

@@ -7,6 +7,7 @@ pub mod proto;
 mod tests;
 
 use anyhow::{anyhow, Context, Result};
+use async_trait::async_trait;
 use client::http::HttpClient;
 use collections::HashMap;
 use futures::{
@@ -43,7 +44,7 @@ pub use outline::{Outline, OutlineItem};
 pub use tree_sitter::{Parser, Tree};
 
 thread_local! {
-    static PARSER: RefCell<Parser>  = RefCell::new(Parser::new());
+    static PARSER: RefCell<Parser> = RefCell::new(Parser::new());
 }
 
 lazy_static! {
@@ -63,38 +64,103 @@ pub trait ToLspPosition {
 #[derive(Clone, Debug, PartialEq, Eq, Hash)]
 pub struct LanguageServerName(pub Arc<str>);
 
-use async_trait::async_trait;
+/// Represents a Language Server, with certain cached sync properties.
+/// Uses [`LspAdapterTrait`] under the hood, but calls all 'static' methods
+/// once at startup, and caches the results.
+pub struct LspAdapter {
+    pub name: LanguageServerName,
+    pub server_args: Vec<String>,
+    pub initialization_options: Option<Value>,
+    pub disk_based_diagnostic_sources: Vec<String>,
+    pub disk_based_diagnostics_progress_token: Option<String>,
+    pub id_for_language: Option<String>,
+    pub adapter: Box<dyn LspAdapterTrait>,
+}
 
-// pub struct LspAdapter {
-//     name: LanguageServerName,
-//     adapter: Arc<dyn LspAdapter>,
-// }
+impl LspAdapter {
+    pub async fn new(adapter: impl LspAdapterTrait) -> Arc<Self> {
+        let adapter = Box::new(adapter);
+        let name = adapter.name().await;
+        let server_args = adapter.server_args().await;
+        let initialization_options = adapter.initialization_options().await;
+        let disk_based_diagnostic_sources = adapter.disk_based_diagnostic_sources().await;
+        let disk_based_diagnostics_progress_token =
+            adapter.disk_based_diagnostics_progress_token().await;
+        let id_for_language = adapter.id_for_language(name.0.as_ref()).await;
+
+        Arc::new(LspAdapter {
+            name,
+            server_args,
+            initialization_options,
+            disk_based_diagnostic_sources,
+            disk_based_diagnostics_progress_token,
+            id_for_language,
+            adapter,
+        })
+    }
 
-// impl LspAdapter {
-//     async fn new(adapter: Arc<dyn LspAdapter>) -> Self {
-//         let name = adapter.name().await;
+    pub async fn fetch_latest_server_version(
+        &self,
+        http: Arc<dyn HttpClient>,
+    ) -> Result<Box<dyn 'static + Send + Any>> {
+        self.adapter.fetch_latest_server_version(http).await
+    }
 
-//         LspAdapter { name, adapter }
-//     }
+    pub async fn fetch_server_binary(
+        &self,
+        version: Box<dyn 'static + Send + Any>,
+        http: Arc<dyn HttpClient>,
+        container_dir: PathBuf,
+    ) -> Result<PathBuf> {
+        self.adapter
+            .fetch_server_binary(version, http, container_dir)
+            .await
+    }
 
-//     fn name(&self) -> LanguageServerName {
-//         self.name
-//     }
-// }
+    pub async fn cached_server_binary(&self, container_dir: PathBuf) -> Option<PathBuf> {
+        self.adapter.cached_server_binary(container_dir).await
+    }
+
+    pub async fn process_diagnostics(&self, params: &mut lsp::PublishDiagnosticsParams) {
+        self.adapter.process_diagnostics(params).await
+    }
+
+    pub async fn label_for_completion(
+        &self,
+        completion_item: &lsp::CompletionItem,
+        language: &Language,
+    ) -> Option<CodeLabel> {
+        self.adapter
+            .label_for_completion(completion_item, language)
+            .await
+    }
+
+    pub async fn label_for_symbol(
+        &self,
+        name: &str,
+        kind: lsp::SymbolKind,
+        language: &Language,
+    ) -> Option<CodeLabel> {
+        self.adapter.label_for_symbol(name, kind, language).await
+    }
+}
 
 #[async_trait]
-pub trait LspAdapter: 'static + Send + Sync {
+pub trait LspAdapterTrait: 'static + Send + Sync {
     async fn name(&self) -> LanguageServerName;
+
     async fn fetch_latest_server_version(
         &self,
         http: Arc<dyn HttpClient>,
     ) -> Result<Box<dyn 'static + Send + Any>>;
+
     async fn fetch_server_binary(
         &self,
         version: Box<dyn 'static + Send + Any>,
         http: Arc<dyn HttpClient>,
         container_dir: PathBuf,
     ) -> Result<PathBuf>;
+
     async fn cached_server_binary(&self, container_dir: PathBuf) -> Option<PathBuf>;
 
     async fn process_diagnostics(&self, _: &mut lsp::PublishDiagnosticsParams) {}
@@ -189,7 +255,9 @@ fn deserialize_regex<'de, D: Deserializer<'de>>(d: D) -> Result<Option<Regex>, D
 }
 
 #[cfg(any(test, feature = "test-support"))]
-pub struct FakeLspAdapter {
+pub type FakeLspAdapter = Arc<FakeLspAdapterInner>;
+
+pub struct FakeLspAdapterInner {
     pub name: &'static str,
     pub capabilities: lsp::ServerCapabilities,
     pub initializer: Option<Box<dyn 'static + Send + Sync + Fn(&mut lsp::FakeLanguageServer)>>,
@@ -208,12 +276,12 @@ pub struct BracketPair {
 pub struct Language {
     pub(crate) config: LanguageConfig,
     pub(crate) grammar: Option<Arc<Grammar>>,
-    pub(crate) adapter: Option<Arc<dyn LspAdapter>>,
+    pub(crate) adapter: Option<Arc<LspAdapter>>,
 
     #[cfg(any(test, feature = "test-support"))]
     fake_adapter: Option<(
         mpsc::UnboundedSender<lsp::FakeLanguageServer>,
-        Arc<FakeLspAdapter>,
+        FakeLspAdapter,
     )>,
 }
 
@@ -373,7 +441,7 @@ impl LanguageRegistry {
             let server_binary_path = this
                 .lsp_binary_paths
                 .lock()
-                .entry(adapter.name().await)
+                .entry(adapter.name.clone())
                 .or_insert_with(|| {
                     get_server_binary_path(
                         adapter.clone(),
@@ -390,7 +458,7 @@ impl LanguageRegistry {
                 .map_err(|e| anyhow!(e));
 
             let server_binary_path = server_binary_path.await?;
-            let server_args = adapter.server_args().await;
+            let server_args = &adapter.server_args;
             let server = lsp::LanguageServer::new(
                 server_id,
                 &server_binary_path,
@@ -410,13 +478,13 @@ impl LanguageRegistry {
 }
 
 async fn get_server_binary_path(
-    adapter: Arc<dyn LspAdapter>,
+    adapter: Arc<LspAdapter>,
     language: Arc<Language>,
     http_client: Arc<dyn HttpClient>,
     download_dir: Arc<Path>,
     statuses: async_broadcast::Sender<(Arc<Language>, LanguageServerBinaryStatus)>,
 ) -> Result<PathBuf> {
-    let container_dir = download_dir.join(adapter.name().await.0.as_ref());
+    let container_dir = download_dir.join(adapter.name.0.as_ref());
     if !container_dir.exists() {
         smol::fs::create_dir_all(&container_dir)
             .await
@@ -452,7 +520,7 @@ async fn get_server_binary_path(
 }
 
 async fn fetch_latest_server_binary_path(
-    adapter: Arc<dyn LspAdapter>,
+    adapter: Arc<LspAdapter>,
     language: Arc<Language>,
     http_client: Arc<dyn HttpClient>,
     container_dir: &Path,
@@ -501,7 +569,7 @@ impl Language {
         }
     }
 
-    pub fn lsp_adapter(&self) -> Option<Arc<dyn LspAdapter>> {
+    pub fn lsp_adapter(&self) -> Option<Arc<LspAdapter>> {
         self.adapter.clone()
     }
 
@@ -533,7 +601,7 @@ impl Language {
         Arc::get_mut(self.grammar.as_mut().unwrap()).unwrap()
     }
 
-    pub fn with_lsp_adapter(mut self, lsp_adapter: Arc<dyn LspAdapter>) -> Self {
+    pub fn with_lsp_adapter(mut self, lsp_adapter: Arc<LspAdapter>) -> Self {
         self.adapter = Some(lsp_adapter);
         self
     }
@@ -544,8 +612,8 @@ impl Language {
         fake_lsp_adapter: FakeLspAdapter,
     ) -> mpsc::UnboundedReceiver<lsp::FakeLanguageServer> {
         let (servers_tx, servers_rx) = mpsc::unbounded();
-        let adapter = Arc::new(fake_lsp_adapter);
-        self.fake_adapter = Some((servers_tx, adapter.clone()));
+        self.fake_adapter = Some((servers_tx, fake_lsp_adapter.clone()));
+        let adapter = smol::block_on(LspAdapter::new(fake_lsp_adapter));
         self.adapter = Some(adapter);
         servers_rx
     }
@@ -558,16 +626,16 @@ impl Language {
         self.config.line_comment.as_deref()
     }
 
-    pub async fn disk_based_diagnostic_sources(&self) -> Vec<String> {
+    pub async fn disk_based_diagnostic_sources(&self) -> &[String] {
         match self.adapter.as_ref() {
-            Some(adapter) => adapter.disk_based_diagnostic_sources().await,
-            None => Vec::new(),
+            Some(adapter) => &adapter.disk_based_diagnostic_sources,
+            None => &[],
         }
     }
 
-    pub async fn disk_based_diagnostics_progress_token(&self) -> Option<String> {
+    pub async fn disk_based_diagnostics_progress_token(&self) -> Option<&str> {
         if let Some(adapter) = self.adapter.as_ref() {
-            adapter.disk_based_diagnostics_progress_token().await
+            adapter.disk_based_diagnostics_progress_token.as_deref()
         } else {
             None
         }
@@ -695,7 +763,7 @@ impl CodeLabel {
 }
 
 #[cfg(any(test, feature = "test-support"))]
-impl Default for FakeLspAdapter {
+impl Default for FakeLspAdapterInner {
     fn default() -> Self {
         Self {
             name: "the-fake-language-server",
@@ -709,7 +777,7 @@ impl Default for FakeLspAdapter {
 
 #[cfg(any(test, feature = "test-support"))]
 #[async_trait]
-impl LspAdapter for FakeLspAdapter {
+impl LspAdapterTrait for FakeLspAdapter {
     async fn name(&self) -> LanguageServerName {
         LanguageServerName(self.name.into())
     }

crates/project/src/project.rs 🔗

@@ -740,7 +740,7 @@ impl Project {
             for language in self.languages.to_vec() {
                 if let Some(lsp_adapter) = language.lsp_adapter() {
                     if !settings.enable_language_server(Some(&language.name())) {
-                        let lsp_name = lsp_adapter.name().await;
+                        let lsp_name = lsp_adapter.name;
                         for (worktree_id, started_lsp_name) in self.started_language_servers.keys()
                         {
                             if lsp_name == *started_lsp_name {
@@ -1663,7 +1663,7 @@ impl Project {
                         this.create_local_worktree(&abs_path, false, cx)
                     })
                     .await?;
-                let name = lsp_adapter.name().await;
+                let name = lsp_adapter.name;
                 this.update(&mut cx, |this, cx| {
                     this.language_servers
                         .insert((worktree.read(cx).id(), name), (lsp_adapter, lsp_server));
@@ -1816,7 +1816,7 @@ impl Project {
         Ok(())
     }
 
-    async fn register_buffer_with_language_server(
+    fn register_buffer_with_language_server(
         &mut self,
         buffer_handle: &ModelHandle<Buffer>,
         cx: &mut ModelContext<'_, Self>,
@@ -1833,10 +1833,10 @@ impl Project {
                 if let Some(language) = buffer.language() {
                     let worktree_id = file.worktree_id(cx);
                     if let Some(adapter) = language.lsp_adapter() {
-                        language_id = adapter.id_for_language(language.name().as_ref()).await;
+                        language_id = adapter.id_for_language.clone();
                         language_server = self
                             .language_server_ids
-                            .get(&(worktree_id, adapter.name()))
+                            .get(&(worktree_id, adapter.name.clone()))
                             .and_then(|id| self.language_servers.get(&id))
                             .and_then(|server_state| {
                                 if let LanguageServerState::Running { server, .. } = server_state {
@@ -2000,11 +2000,7 @@ impl Project {
                 // that don't support a disk-based progress token.
                 let (lsp_adapter, language_server) =
                     self.language_server_for_buffer(buffer.read(cx), cx)?;
-                if lsp_adapter
-                    .disk_based_diagnostics_progress_token()
-                    .await
-                    .is_none()
-                {
+                if lsp_adapter.disk_based_diagnostics_progress_token.is_none() {
                     let server_id = language_server.server_id();
                     self.disk_based_diagnostics_finished(server_id, cx);
                     self.broadcast_language_server_update(
@@ -2024,10 +2020,9 @@ impl Project {
     fn language_servers_for_worktree(
         &self,
         worktree_id: WorktreeId,
-    ) -> impl Iterator<Item = (&Arc<dyn LspAdapter>, &Arc<LanguageServer>)> {
-        self.language_server_ids
-            .iter()
-            .filter_map(move |((language_server_worktree_id, _), id)| {
+    ) -> impl Iterator<Item = &(Arc<LspAdapter>, Arc<LanguageServer>)> {
+        self.language_servers.iter().filter_map(
+            move |((language_server_worktree_id, _), server)| {
                 if *language_server_worktree_id == worktree_id {
                     if let Some(LanguageServerState::Running { adapter, server }) =
                         self.language_servers.get(&id)
@@ -2080,8 +2075,8 @@ impl Project {
         } else {
             return;
         };
-        let key = (worktree_id, adapter.name());
 
+        let key = (worktree_id, adapter.name);
         self.language_server_ids
             .entry(key.clone())
             .or_insert_with(|| {
@@ -2115,7 +2110,7 @@ impl Project {
                                         this.update(&mut cx, |this, cx| {
                                             this.on_lsp_diagnostics_published(
                                                 server_id, params, &adapter, cx,
-                                            );
+                                            )
                                         });
                                     }
                                 }
@@ -2427,8 +2422,8 @@ impl Project {
         } else {
             return;
         };
-        let server_name = adapter.name();
-        let stop = self.stop_language_server(worktree_id, server_name.clone(), cx);
+
+        let stop = self.stop_language_server(worktree_id, adapter.name, cx);
         cx.spawn_weak(|this, mut cx| async move {
             let (original_root_path, orphaned_worktrees) = stop.await;
             if let Some(this) = this.upgrade(&cx) {
@@ -2464,14 +2459,14 @@ impl Project {
         &mut self,
         server_id: usize,
         mut params: lsp::PublishDiagnosticsParams,
-        adapter: &Arc<dyn LspAdapter>,
+        adapter: &Arc<LspAdapter>,
         cx: &mut ModelContext<'_, Self>,
     ) {
         adapter.process_diagnostics(&mut params).await;
         self.update_diagnostics(
             server_id,
             params,
-            adapter.disk_based_diagnostic_sources().await,
+            &adapter.disk_based_diagnostic_sources,
             cx,
         )
         .log_err();
@@ -2645,7 +2640,7 @@ impl Project {
         this: WeakModelHandle<Self>,
         params: lsp::ApplyWorkspaceEditParams,
         server_id: usize,
-        adapter: Arc<dyn LspAdapter>,
+        adapter: Arc<LspAdapter>,
         language_server: Arc<LanguageServer>,
         mut cx: AsyncAppContext,
     ) -> Result<lsp::ApplyWorkspaceEditResponse> {
@@ -2716,7 +2711,7 @@ impl Project {
         &mut self,
         language_server_id: usize,
         params: lsp::PublishDiagnosticsParams,
-        disk_based_sources: Vec<String>,
+        disk_based_sources: &[String],
         cx: &mut ModelContext<Self>,
     ) -> Result<()> {
         let abs_path = params
@@ -3314,7 +3309,7 @@ impl Project {
                 struct PartialSymbol {
                     source_worktree_id: WorktreeId,
                     worktree_id: WorktreeId,
-                    adapter: Arc<dyn LspAdapter>,
+                    adapter: Arc<LspAdapter>,
                     path: PathBuf,
                     language: Option<Arc<Language>>,
                     name: String,
@@ -3372,7 +3367,7 @@ impl Project {
                         }
                         .unwrap_or_else(|| CodeLabel::plain(ps.name.clone(), None));
 
-                        let language_server_name = ps.adapter.name().await;
+                        let language_server_name = ps.adapter.name;
 
                         Symbol {
                             source_worktree_id: ps.source_worktree_id,
@@ -4019,7 +4014,7 @@ impl Project {
         this: ModelHandle<Self>,
         edit: lsp::WorkspaceEdit,
         push_to_history: bool,
-        lsp_adapter: Arc<dyn LspAdapter>,
+        lsp_adapter: Arc<LspAdapter>,
         language_server: Arc<LanguageServer>,
         cx: &mut AsyncAppContext,
     ) -> Result<ProjectTransaction> {
@@ -6029,10 +6024,9 @@ impl Project {
         &self,
         buffer: &Buffer,
         cx: &AppContext,
-    ) -> Option<(&Arc<dyn LspAdapter>, &Arc<LanguageServer>)> {
+    ) -> Option<&(Arc<LspAdapter>, Arc<LanguageServer>)> {
         if let Some((file, language)) = File::from_dyn(buffer.file()).zip(buffer.language()) {
-            // TODO(isaac): this is not a good idea
-            let name = cx.background().block(language.lsp_adapter()?.name());
+            let name = language.lsp_adapter()?.name;
             let worktree_id = file.worktree_id(cx);
             let key = (worktree_id, language.lsp_adapter()?.name());