node_runtime: Use semver::Version to represent package versions (#44342)

Closes #ISSUE

This PR is rather a nice to have change than anything critical, so
review priority should remain low.

Switch to using `semver::Version` for representing node binary and npm
package versions. This is in an effort to root out implicit behavior and
improve type safety when interacting with the `node_runtime` crate by
catching invalid versions where they appear. Currently Zed may
implicitly assume the current version is correct, or always install the
newest version when a invalid version is passed. `semver::Version` also
doesn't require the heap, which is probably more of a fun fact than
anything useful.

`npm_install_packages` still takes versions as a `&str`, because
`latest` can be used to fetch the latest version on npm. This could
likely be made into an enum as well, but would make the PR even larger.

I tested changes with some node based language servers and external
agents, which all worked fine. It would be nice to have some e2e tests
for node. To be safe I'd put it on nightly after a Wednesday release.

Release Notes:

- N/A *or* Added/Fixed/Improved ...

crates/node_runtime/src/node_runtime.rs

@@ -34,9 +34,9 @@ pub struct NodeBinaryOptions {
 
 pub enum VersionStrategy<'a> {
     /// Install if current version doesn't match pinned version
-    Pin(&'a str),
+    Pin(&'a Version),
     /// Install if current version is older than latest version
-    Latest(&'a str),
+    Latest(&'a Version),
 }
 
 #[derive(Clone)]
@@ -230,14 +230,14 @@ impl NodeRuntime {
         &self,
         local_package_directory: &Path,
         name: &str,
-    ) -> Result<Option<String>> {
+    ) -> Result<Option<Version>> {
         self.instance()
             .await
             .npm_package_installed_version(local_package_directory, name)
             .await
     }
 
-    pub async fn npm_package_latest_version(&self, name: &str) -> Result<String> {
+    pub async fn npm_package_latest_version(&self, name: &str) -> Result<Version> {
         let http = self.0.lock().await.http.clone();
         let output = self
             .instance()
@@ -280,16 +280,19 @@ impl NodeRuntime {
             .map(|(name, version)| format!("{name}@{version}"))
             .collect();
 
-        let mut arguments: Vec<_> = packages.iter().map(|p| p.as_str()).collect();
-        arguments.extend_from_slice(&[
-            "--save-exact",
-            "--fetch-retry-mintimeout",
-            "2000",
-            "--fetch-retry-maxtimeout",
-            "5000",
-            "--fetch-timeout",
-            "5000",
-        ]);
+        let arguments: Vec<_> = packages
+            .iter()
+            .map(|p| p.as_str())
+            .chain([
+                "--save-exact",
+                "--fetch-retry-mintimeout",
+                "2000",
+                "--fetch-retry-maxtimeout",
+                "5000",
+                "--fetch-timeout",
+                "5000",
+            ])
+            .collect();
 
         // This is also wrong because the directory is wrong.
         self.run_npm_subcommand(Some(directory), "install", &arguments)
@@ -320,23 +323,9 @@ impl NodeRuntime {
             return true;
         };
 
-        let Some(installed_version) = Version::parse(&installed_version).log_err() else {
-            return true;
-        };
-
         match version_strategy {
-            VersionStrategy::Pin(pinned_version) => {
-                let Some(pinned_version) = Version::parse(pinned_version).log_err() else {
-                    return true;
-                };
-                installed_version != pinned_version
-            }
-            VersionStrategy::Latest(latest_version) => {
-                let Some(latest_version) = Version::parse(latest_version).log_err() else {
-                    return true;
-                };
-                installed_version < latest_version
-            }
+            VersionStrategy::Pin(pinned_version) => &installed_version != pinned_version,
+            VersionStrategy::Latest(latest_version) => &installed_version < latest_version,
         }
     }
 }
@@ -351,12 +340,12 @@ enum ArchiveType {
 pub struct NpmInfo {
     #[serde(default)]
     dist_tags: NpmInfoDistTags,
-    versions: Vec<String>,
+    versions: Vec<Version>,
 }
 
 #[derive(Debug, Deserialize, Default)]
 pub struct NpmInfoDistTags {
-    latest: Option<String>,
+    latest: Option<Version>,
 }
 
 #[async_trait::async_trait]
@@ -376,7 +365,7 @@ trait NodeRuntimeTrait: Send + Sync {
         &self,
         local_package_directory: &Path,
         name: &str,
-    ) -> Result<Option<String>>;
+    ) -> Result<Option<Version>>;
 }
 
 #[derive(Clone)]
@@ -610,7 +599,7 @@ impl NodeRuntimeTrait for ManagedNodeRuntime {
         &self,
         local_package_directory: &Path,
         name: &str,
-    ) -> Result<Option<String>> {
+    ) -> Result<Option<Version>> {
         read_package_installed_version(local_package_directory.join("node_modules"), name).await
     }
 }
@@ -735,7 +724,7 @@ impl NodeRuntimeTrait for SystemNodeRuntime {
         &self,
         local_package_directory: &Path,
         name: &str,
-    ) -> Result<Option<String>> {
+    ) -> Result<Option<Version>> {
         read_package_installed_version(local_package_directory.join("node_modules"), name).await
         // todo: allow returning a globally installed version (requires callers not to hard-code the path)
     }
@@ -744,7 +733,7 @@ impl NodeRuntimeTrait for SystemNodeRuntime {
 pub async fn read_package_installed_version(
     node_module_directory: PathBuf,
     name: &str,
-) -> Result<Option<String>> {
+) -> Result<Option<Version>> {
     let package_json_path = node_module_directory.join(name).join("package.json");
 
     let mut file = match fs::File::open(package_json_path).await {
@@ -760,7 +749,7 @@ pub async fn read_package_installed_version(
 
     #[derive(Deserialize)]
     struct PackageJson {
-        version: String,
+        version: Version,
     }
 
     let mut contents = String::new();
@@ -797,7 +786,7 @@ impl NodeRuntimeTrait for UnavailableNodeRuntime {
         &self,
         _local_package_directory: &Path,
         _: &str,
-    ) -> Result<Option<String>> {
+    ) -> Result<Option<Version>> {
         bail!("{}", self.error_message)
     }
 }