languages: Validate pylsp binary before returning from check_if_user_installed (#51034)

Kyle Kelley created 1 month ago

Run `pylsp --version` via `delegate.try_exec()` in both branches of
`PyLspAdapter::check_if_user_installed` before returning the binary. If
execution fails (broken shebang, missing interpreter, etc.), log a
warning and return None so the system falls through gracefully instead
of surfacing an error dialog.

This matches the existing validation pattern used by TyLspAdapter and
RuffLspAdapter in their `fetch_server_binary` implementations.

No idea if this closes an issue but it sure was annoying on a system
where I deleted the pylsp environment I had. It surprised me too since I
had pylsp disabled in settings.

Release Notes:

- Fixed detection of when `pylsp` is not installed properly on a user's
system so that it doesn't get launched as an LSP when it doesn't exist.

Change summary

crates/languages/src/python.rs | 27 ++++++++++++++++++++++++++-
1 file changed, 26 insertions(+), 1 deletion(-)

Detailed changes

crates/languages/src/python.rs

@@ -1846,6 +1846,17 @@ impl LspInstaller for PyLspAdapter {
     ) -> Option<LanguageServerBinary> {
         if let Some(pylsp_bin) = delegate.which(Self::SERVER_NAME.as_ref()).await {
             let env = delegate.shell_env().await;
+            delegate
+                .try_exec(LanguageServerBinary {
+                    path: pylsp_bin.clone(),
+                    arguments: vec!["--version".into()],
+                    env: Some(env.clone()),
+                })
+                .await
+                .inspect_err(|err| {
+                    log::warn!("failed to validate user-installed pylsp at {pylsp_bin:?}: {err:#}")
+                })
+                .ok()?;
             Some(LanguageServerBinary {
                 path: pylsp_bin,
                 env: Some(env),
@@ -1854,7 +1865,21 @@ impl LspInstaller for PyLspAdapter {
         } else {
             let toolchain = toolchain?;
             let pylsp_path = Path::new(toolchain.path.as_ref()).parent()?.join("pylsp");
-            pylsp_path.exists().then(|| LanguageServerBinary {
+            if !pylsp_path.exists() {
+                return None;
+            }
+            delegate
+                .try_exec(LanguageServerBinary {
+                    path: toolchain.path.to_string().into(),
+                    arguments: vec![pylsp_path.clone().into(), "--version".into()],
+                    env: None,
+                })
+                .await
+                .inspect_err(|err| {
+                    log::warn!("failed to validate toolchain pylsp at {pylsp_path:?}: {err:#}")
+                })
+                .ok()?;
+            Some(LanguageServerBinary {
                 path: toolchain.path.to_string().into(),
                 arguments: vec![pylsp_path.into()],
                 env: None,