Merge pull request #1786 from volta-cli/chriskrycho/better-reporting

Better error messages for unsupported `uninstall` arguments

Author: charlespierce

crates/volta-core/src/error/kind.rs

@@ -443,10 +443,10 @@ pub enum ErrorKind {
         name: String,
     },
 
-    /// Thrown when serializnig a bin config to JSON fails
+    /// Thrown when serializing a bin config to JSON fails
     StringifyBinConfigError,
 
-    /// Thrown when serializnig a package config to JSON fails
+    /// Thrown when serializing a package config to JSON fails
     StringifyPackageConfigError,
 
     /// Thrown when serializing the platform to JSON fails

src/command/uninstall.rs

@@ -1,4 +1,4 @@
-use volta_core::error::{ExitCode, Fallible};
+use volta_core::error::{ErrorKind, ExitCode, Fallible};
 use volta_core::session::{ActivityKind, Session};
 use volta_core::tool;
 use volta_core::version::VersionSpec;
@@ -15,8 +15,20 @@ impl Command for Uninstall {
     fn run(self, session: &mut Session) -> Fallible<ExitCode> {
         session.add_event_start(ActivityKind::Uninstall);
 
-        let version = VersionSpec::default();
-        let tool = tool::Spec::from_str_and_version(&self.tool, version);
+        let tool = tool::Spec::try_from_str(&self.tool)?;
+
+        // For packages, specifically report that we do not support uninstalling
+        // specific versions. For runtimes and package managers, we currently
+        // *intentionally* let this fall through to inform the user that we do
+        // not support uninstalling those *at all*.
+        if let tool::Spec::Package(_name, version) = &tool {
+            let VersionSpec::None = version else {
+                return Err(ErrorKind::Unimplemented {
+                    feature: "uninstalling specific versions of tools".into(),
+                }
+                .into());
+            };
+        }
 
         tool.uninstall()?;
 

tests/acceptance/direct_uninstall.rs

@@ -1,3 +1,7 @@
+//! Tests for `npm uninstall`, `npm uninstall --global`, `yarn remove`, and
+//! `yarn global remove`, which we support as alternatives to `volta uninstall`
+//! and which should use its logic.
+
 use crate::support::sandbox::{sandbox, DistroMetadata, NodeFixture, Sandbox, Yarn1Fixture};
 use hamcrest2::assert_that;
 use hamcrest2::prelude::*;

tests/acceptance/volta_uninstall.rs

@@ -1,3 +1,5 @@
+//! Tests for `volta uninstall`.
+
 use crate::support::sandbox::{sandbox, Sandbox};
 use hamcrest2::assert_that;
 use hamcrest2::prelude::*;
@@ -94,6 +96,30 @@ fn uninstall_package_basic() {
     assert!(!Sandbox::package_image_exists("cowsay"));
 }
 
+// The setup here is the same as the above, but here we check to make sure that
+// if the user supplies a version, we error correctly.
+#[test]
+fn uninstall_package_basic_with_version() {
+    // basic uninstall - everything exists, and everything except the cached
+    // inventory files should be deleted
+    let s = sandbox()
+        .package_config("cowsay", PKG_CONFIG_BASIC)
+        .binary_config("cowsay", &bin_config("cowsay"))
+        .binary_config("cowthink", &bin_config("cowthink"))
+        .shim("cowsay")
+        .shim("cowthink")
+        .package_image("cowsay", "1.4.0", None)
+        .env(VOLTA_LOGLEVEL, "info")
+        .build();
+
+    assert_that!(
+        s.volta("uninstall [email protected]"),
+        execs().with_status(1).with_stderr_contains(
+            "[..]error: uninstalling specific versions of tools is not supported yet."
+        )
+    );
+}
+
 #[test]
 fn uninstall_package_no_bins() {
     // the package doesn't contain any executables, it should uninstall without error
@@ -178,3 +204,14 @@ fn uninstall_package_orphaned_bins() {
     assert!(!Sandbox::shim_exists("cowsay"));
     assert!(!Sandbox::shim_exists("cowthink"));
 }
+
+#[test]
+fn uninstall_runtime() {
+    let s = sandbox().build();
+    assert_that!(
+        s.volta("uninstall node"),
+        execs()
+            .with_status(1)
+            .with_stderr_contains("[..]error: Uninstalling node is not supported yet.")
+    )
+}