Fix lang servers status set to Downloading when checking version (#22292)

Thorsten Ball created 

This message has confused me many times too: we printed the status as
"Downloading" when we were only checking whether we need to install a
given version of a language server.

This fixes the issue for Node-based language servers where we had the
same check in all implementations.

Closes  #22241

Release Notes:

- Fixed some language servers reporting status as "Downloading..." when
only a version check was being done.

Change summary

crates/language/src/language.rs    | 32 ++++++++++++---
crates/languages/src/css.rs        | 43 ++++++++++++++++-----
crates/languages/src/json.rs       | 44 +++++++++++++++++-----
crates/languages/src/python.rs     | 44 ++++++++++++++++------
crates/languages/src/tailwind.rs   | 45 +++++++++++++++++------
crates/languages/src/typescript.rs | 62 +++++++++++++++++++++----------
crates/languages/src/yaml.rs       | 43 ++++++++++++++++-----
7 files changed, 228 insertions(+), 85 deletions(-)

Detailed changes

crates/language/src/language.rs 🔗 

@@ -385,6 +385,15 @@ pub trait LspAdapter: 'static + Send + Sync {
         None
     }
 
+    async fn check_if_version_installed(
+        &self,
+        _version: &(dyn 'static + Send + Any),
+        _container_dir: &PathBuf,
+        _delegate: &dyn LspAdapterDelegate,
+    ) -> Option<LanguageServerBinary> {
+        None
+    }
+
     async fn fetch_server_binary(
         &self,
         latest_version: Box<dyn 'static + Send + Any>,
@@ -516,14 +525,23 @@ async fn try_fetch_server_binary<L: LspAdapter + 'static + Send + Sync + ?Sized>
         .fetch_latest_server_version(delegate.as_ref())
         .await?;
 
-    log::info!("downloading language server {:?}", name.0);
-    delegate.update_status(adapter.name(), LanguageServerBinaryStatus::Downloading);
-    let binary = adapter
-        .fetch_server_binary(latest_version, container_dir, delegate.as_ref())
-        .await;
+    if let Some(binary) = adapter
+        .check_if_version_installed(latest_version.as_ref(), &container_dir, delegate.as_ref())
+        .await
+    {
+        log::info!("language server {:?} is already installed", name.0);
+        delegate.update_status(name.clone(), LanguageServerBinaryStatus::None);
+        Ok(binary)
+    } else {
+        log::info!("downloading language server {:?}", name.0);
+        delegate.update_status(adapter.name(), LanguageServerBinaryStatus::Downloading);
+        let binary = adapter
+            .fetch_server_binary(latest_version, container_dir, delegate.as_ref())
+            .await;
 
-    delegate.update_status(name.clone(), LanguageServerBinaryStatus::None);
-    binary
+        delegate.update_status(name.clone(), LanguageServerBinaryStatus::None);
+        binary
+    }
 }
 
 #[derive(Clone, Debug, Default, PartialEq, Eq)]

crates/languages/src/css.rs 🔗 

@@ -26,6 +26,7 @@ pub struct CssLspAdapter {
 }
 
 impl CssLspAdapter {
+    const PACKAGE_NAME: &str = "vscode-langservers-extracted";
     pub fn new(node: NodeRuntime) -> Self {
         CssLspAdapter { node }
     }
@@ -56,18 +57,13 @@ impl LspAdapter for CssLspAdapter {
     ) -> Result<LanguageServerBinary> {
         let latest_version = latest_version.downcast::<String>().unwrap();
         let server_path = container_dir.join(SERVER_PATH);
-        let package_name = "vscode-langservers-extracted";
 
-        let should_install_language_server = self
-            .node
-            .should_install_npm_package(package_name, &server_path, &container_dir, &latest_version)
-            .await;
-
-        if should_install_language_server {
-            self.node
-                .npm_install_packages(&container_dir, &[(package_name, latest_version.as_str())])
-                .await?;
-        }
+        self.node
+            .npm_install_packages(
+                &container_dir,
+                &[(Self::PACKAGE_NAME, latest_version.as_str())],
+            )
+            .await?;
 
         Ok(LanguageServerBinary {
             path: self.node.binary_path().await?,
@@ -76,6 +72,31 @@ impl LspAdapter for CssLspAdapter {
         })
     }
 
+    async fn check_if_version_installed(
+        &self,
+        version: &(dyn 'static + Send + Any),
+        container_dir: &PathBuf,
+        _: &dyn LspAdapterDelegate,
+    ) -> Option<LanguageServerBinary> {
+        let version = version.downcast_ref::<String>().unwrap();
+        let server_path = container_dir.join(SERVER_PATH);
+
+        let should_install_language_server = self
+            .node
+            .should_install_npm_package(Self::PACKAGE_NAME, &server_path, &container_dir, &version)
+            .await;
+
+        if should_install_language_server {
+            None
+        } else {
+            Some(LanguageServerBinary {
+                path: self.node.binary_path().await.ok()?,
+                env: None,
+                arguments: server_binary_arguments(&server_path),
+            })
+        }
+    }
+
     async fn cached_server_binary(
         &self,
         container_dir: PathBuf,

crates/languages/src/json.rs 🔗 

@@ -64,6 +64,8 @@ pub struct JsonLspAdapter {
 }
 
 impl JsonLspAdapter {
+    const PACKAGE_NAME: &str = "vscode-langservers-extracted";
+
     pub fn new(node: NodeRuntime, languages: Arc<LanguageRegistry>) -> Self {
         Self {
             node,
@@ -142,31 +144,51 @@ impl LspAdapter for JsonLspAdapter {
     ) -> Result<Box<dyn 'static + Send + Any>> {
         Ok(Box::new(
             self.node
-                .npm_package_latest_version("vscode-langservers-extracted")
+                .npm_package_latest_version(Self::PACKAGE_NAME)
                 .await?,
         ) as Box<_>)
     }
 
-    async fn fetch_server_binary(
+    async fn check_if_version_installed(
         &self,
-        latest_version: Box<dyn 'static + Send + Any>,
-        container_dir: PathBuf,
+        version: &(dyn 'static + Send + Any),
+        container_dir: &PathBuf,
         _: &dyn LspAdapterDelegate,
-    ) -> Result<LanguageServerBinary> {
-        let latest_version = latest_version.downcast::<String>().unwrap();
+    ) -> Option<LanguageServerBinary> {
+        let version = version.downcast_ref::<String>().unwrap();
         let server_path = container_dir.join(SERVER_PATH);
-        let package_name = "vscode-langservers-extracted";
 
         let should_install_language_server = self
             .node
-            .should_install_npm_package(package_name, &server_path, &container_dir, &latest_version)
+            .should_install_npm_package(Self::PACKAGE_NAME, &server_path, &container_dir, &version)
             .await;
 
         if should_install_language_server {
-            self.node
-                .npm_install_packages(&container_dir, &[(package_name, latest_version.as_str())])
-                .await?;
+            None
+        } else {
+            Some(LanguageServerBinary {
+                path: self.node.binary_path().await.ok()?,
+                env: None,
+                arguments: server_binary_arguments(&server_path),
+            })
         }
+    }
+
+    async fn fetch_server_binary(
+        &self,
+        latest_version: Box<dyn 'static + Send + Any>,
+        container_dir: PathBuf,
+        _: &dyn LspAdapterDelegate,
+    ) -> Result<LanguageServerBinary> {
+        let latest_version = latest_version.downcast::<String>().unwrap();
+        let server_path = container_dir.join(SERVER_PATH);
+
+        self.node
+            .npm_install_packages(
+                &container_dir,
+                &[(Self::PACKAGE_NAME, latest_version.as_str())],
+            )
+            .await?;
 
         Ok(LanguageServerBinary {
             path: self.node.binary_path().await?,

crates/languages/src/python.rs 🔗 

@@ -117,30 +117,48 @@ impl LspAdapter for PythonLspAdapter {
         let latest_version = latest_version.downcast::<String>().unwrap();
         let server_path = container_dir.join(SERVER_PATH);
 
+        self.node
+            .npm_install_packages(
+                &container_dir,
+                &[(Self::SERVER_NAME.as_ref(), latest_version.as_str())],
+            )
+            .await?;
+
+        Ok(LanguageServerBinary {
+            path: self.node.binary_path().await?,
+            env: None,
+            arguments: server_binary_arguments(&server_path),
+        })
+    }
+
+    async fn check_if_version_installed(
+        &self,
+        version: &(dyn 'static + Send + Any),
+        container_dir: &PathBuf,
+        _: &dyn LspAdapterDelegate,
+    ) -> Option<LanguageServerBinary> {
+        let version = version.downcast_ref::<String>().unwrap();
+        let server_path = container_dir.join(SERVER_PATH);
+
         let should_install_language_server = self
             .node
             .should_install_npm_package(
                 Self::SERVER_NAME.as_ref(),
                 &server_path,
                 &container_dir,
-                &latest_version,
+                &version,
             )
             .await;
 
         if should_install_language_server {
-            self.node
-                .npm_install_packages(
-                    &container_dir,
-                    &[(Self::SERVER_NAME.as_ref(), latest_version.as_str())],
-                )
-                .await?;
+            None
+        } else {
+            Some(LanguageServerBinary {
+                path: self.node.binary_path().await.ok()?,
+                env: None,
+                arguments: server_binary_arguments(&server_path),
+            })
         }
-
-        Ok(LanguageServerBinary {
-            path: self.node.binary_path().await?,
-            env: None,
-            arguments: server_binary_arguments(&server_path),
-        })
     }
 
     async fn cached_server_binary(

crates/languages/src/tailwind.rs 🔗 

@@ -34,6 +34,7 @@ pub struct TailwindLspAdapter {
 impl TailwindLspAdapter {
     const SERVER_NAME: LanguageServerName =
         LanguageServerName::new_static("tailwindcss-language-server");
+    const PACKAGE_NAME: &str = "@tailwindcss/language-server";
 
     pub fn new(node: NodeRuntime) -> Self {
         TailwindLspAdapter { node }
@@ -52,7 +53,7 @@ impl LspAdapter for TailwindLspAdapter {
     ) -> Result<Box<dyn 'static + Any + Send>> {
         Ok(Box::new(
             self.node
-                .npm_package_latest_version("@tailwindcss/language-server")
+                .npm_package_latest_version(Self::PACKAGE_NAME)
                 .await?,
         ) as Box<_>)
     }
@@ -65,18 +66,13 @@ impl LspAdapter for TailwindLspAdapter {
     ) -> Result<LanguageServerBinary> {
         let latest_version = latest_version.downcast::<String>().unwrap();
         let server_path = container_dir.join(SERVER_PATH);
-        let package_name = "@tailwindcss/language-server";
 
-        let should_install_language_server = self
-            .node
-            .should_install_npm_package(package_name, &server_path, &container_dir, &latest_version)
-            .await;
-
-        if should_install_language_server {
-            self.node
-                .npm_install_packages(&container_dir, &[(package_name, latest_version.as_str())])
-                .await?;
-        }
+        self.node
+            .npm_install_packages(
+                &container_dir,
+                &[(Self::PACKAGE_NAME, latest_version.as_str())],
+            )
+            .await?;
 
         Ok(LanguageServerBinary {
             path: self.node.binary_path().await?,
@@ -85,6 +81,31 @@ impl LspAdapter for TailwindLspAdapter {
         })
     }
 
+    async fn check_if_version_installed(
+        &self,
+        version: &(dyn 'static + Send + Any),
+        container_dir: &PathBuf,
+        _: &dyn LspAdapterDelegate,
+    ) -> Option<LanguageServerBinary> {
+        let version = version.downcast_ref::<String>().unwrap();
+        let server_path = container_dir.join(SERVER_PATH);
+
+        let should_install_language_server = self
+            .node
+            .should_install_npm_package(Self::PACKAGE_NAME, &server_path, &container_dir, &version)
+            .await;
+
+        if should_install_language_server {
+            None
+        } else {
+            Some(LanguageServerBinary {
+                path: self.node.binary_path().await.ok()?,
+                env: None,
+                arguments: server_binary_arguments(&server_path),
+            })
+        }
+    }
+
     async fn cached_server_binary(
         &self,
         container_dir: PathBuf,

crates/languages/src/typescript.rs 🔗 

@@ -73,6 +73,7 @@ impl TypeScriptLspAdapter {
     const NEW_SERVER_PATH: &'static str = "node_modules/typescript-language-server/lib/cli.mjs";
     const SERVER_NAME: LanguageServerName =
         LanguageServerName::new_static("typescript-language-server");
+    const PACKAGE_NAME: &str = "typescript";
     pub fn new(node: NodeRuntime) -> Self {
         TypeScriptLspAdapter { node }
     }
@@ -114,40 +115,61 @@ impl LspAdapter for TypeScriptLspAdapter {
         }) as Box<_>)
     }
 
-    async fn fetch_server_binary(
+    async fn check_if_version_installed(
         &self,
-        latest_version: Box<dyn 'static + Send + Any>,
-        container_dir: PathBuf,
+        version: &(dyn 'static + Send + Any),
+        container_dir: &PathBuf,
         _: &dyn LspAdapterDelegate,
-    ) -> Result<LanguageServerBinary> {
-        let latest_version = latest_version.downcast::<TypeScriptVersions>().unwrap();
+    ) -> Option<LanguageServerBinary> {
+        println!("{:?}", version.type_id());
+        let version = version.downcast_ref::<TypeScriptVersions>().unwrap();
         let server_path = container_dir.join(Self::NEW_SERVER_PATH);
-        let package_name = "typescript";
 
         let should_install_language_server = self
             .node
             .should_install_npm_package(
-                package_name,
+                Self::PACKAGE_NAME,
                 &server_path,
                 &container_dir,
-                latest_version.typescript_version.as_str(),
+                version.typescript_version.as_str(),
             )
             .await;
 
         if should_install_language_server {
-            self.node
-                .npm_install_packages(
-                    &container_dir,
-                    &[
-                        (package_name, latest_version.typescript_version.as_str()),
-                        (
-                            "typescript-language-server",
-                            latest_version.server_version.as_str(),
-                        ),
-                    ],
-                )
-                .await?;
+            None
+        } else {
+            Some(LanguageServerBinary {
+                path: self.node.binary_path().await.ok()?,
+                env: None,
+                arguments: typescript_server_binary_arguments(&server_path),
+            })
         }
+    }
+
+    async fn fetch_server_binary(
+        &self,
+        latest_version: Box<dyn 'static + Send + Any>,
+        container_dir: PathBuf,
+        _: &dyn LspAdapterDelegate,
+    ) -> Result<LanguageServerBinary> {
+        let latest_version = latest_version.downcast::<TypeScriptVersions>().unwrap();
+        let server_path = container_dir.join(Self::NEW_SERVER_PATH);
+
+        self.node
+            .npm_install_packages(
+                &container_dir,
+                &[
+                    (
+                        Self::PACKAGE_NAME,
+                        latest_version.typescript_version.as_str(),
+                    ),
+                    (
+                        "typescript-language-server",
+                        latest_version.server_version.as_str(),
+                    ),
+                ],
+            )
+            .await?;
 
         Ok(LanguageServerBinary {
             path: self.node.binary_path().await?,

crates/languages/src/yaml.rs 🔗 

@@ -31,6 +31,7 @@ pub struct YamlLspAdapter {
 
 impl YamlLspAdapter {
     const SERVER_NAME: LanguageServerName = LanguageServerName::new_static("yaml-language-server");
+    const PACKAGE_NAME: &str = "yaml-language-server";
     pub fn new(node: NodeRuntime) -> Self {
         YamlLspAdapter { node }
     }
@@ -61,18 +62,13 @@ impl LspAdapter for YamlLspAdapter {
     ) -> Result<LanguageServerBinary> {
         let latest_version = latest_version.downcast::<String>().unwrap();
         let server_path = container_dir.join(SERVER_PATH);
-        let package_name = "yaml-language-server";
 
-        let should_install_language_server = self
-            .node
-            .should_install_npm_package(package_name, &server_path, &container_dir, &latest_version)
-            .await;
-
-        if should_install_language_server {
-            self.node
-                .npm_install_packages(&container_dir, &[(package_name, latest_version.as_str())])
-                .await?;
-        }
+        self.node
+            .npm_install_packages(
+                &container_dir,
+                &[(Self::PACKAGE_NAME, latest_version.as_str())],
+            )
+            .await?;
 
         Ok(LanguageServerBinary {
             path: self.node.binary_path().await?,
@@ -81,6 +77,31 @@ impl LspAdapter for YamlLspAdapter {
         })
     }
 
+    async fn check_if_version_installed(
+        &self,
+        version: &(dyn 'static + Send + Any),
+        container_dir: &PathBuf,
+        _: &dyn LspAdapterDelegate,
+    ) -> Option<LanguageServerBinary> {
+        let version = version.downcast_ref::<String>().unwrap();
+        let server_path = container_dir.join(SERVER_PATH);
+
+        let should_install_language_server = self
+            .node
+            .should_install_npm_package(Self::PACKAGE_NAME, &server_path, &container_dir, &version)
+            .await;
+
+        if should_install_language_server {
+            None
+        } else {
+            Some(LanguageServerBinary {
+                path: self.node.binary_path().await.ok()?,
+                env: None,
+                arguments: server_binary_arguments(&server_path),
+            })
+        }
+    }
+
     async fn cached_server_binary(
         &self,
         container_dir: PathBuf,