fix(cli): consolidate pkg parser for install & remove (#26298)

Closes https://github.com/denoland/deno/issues/26283

cli/tools/registry/pm.rs

@@ -470,7 +470,7 @@ pub async fn add(
   let mut package_reqs = Vec::with_capacity(add_flags.packages.len());
 
   for entry_text in add_flags.packages.iter() {
-    let req = AddPackageReq::parse(entry_text).with_context(|| {
+    let req = AddRmPackageReq::parse(entry_text).with_context(|| {
       format!("Failed to parse package required: {}", entry_text)
     })?;
 
@@ -596,10 +596,10 @@ enum PackageAndVersion {
 async fn find_package_and_select_version_for_req(
   jsr_resolver: Arc<JsrFetchResolver>,
   npm_resolver: Arc<NpmFetchResolver>,
-  add_package_req: AddPackageReq,
+  add_package_req: AddRmPackageReq,
 ) -> Result<PackageAndVersion, AnyError> {
   match add_package_req.value {
-    AddPackageReqValue::Jsr(req) => {
+    AddRmPackageReqValue::Jsr(req) => {
       let jsr_prefixed_name = format!("jsr:{}", &req.name);
       let Some(nv) = jsr_resolver.req_to_nv(&req).await else {
         if npm_resolver.req_to_nv(&req).await.is_some() {
@@ -628,7 +628,7 @@ async fn find_package_and_select_version_for_req(
         selected_version: nv.version.to_string(),
       }))
     }
-    AddPackageReqValue::Npm(req) => {
+    AddRmPackageReqValue::Npm(req) => {
       let npm_prefixed_name = format!("npm:{}", &req.name);
       let Some(nv) = npm_resolver.req_to_nv(&req).await else {
         return Ok(PackageAndVersion::NotFound {
@@ -653,18 +653,18 @@ async fn find_package_and_select_version_for_req(
 }
 
 #[derive(Debug, PartialEq, Eq)]
-enum AddPackageReqValue {
+enum AddRmPackageReqValue {
   Jsr(PackageReq),
   Npm(PackageReq),
 }
 
 #[derive(Debug, PartialEq, Eq)]
-struct AddPackageReq {
+struct AddRmPackageReq {
   alias: String,
-  value: AddPackageReqValue,
+  value: AddRmPackageReqValue,
 }
 
-impl AddPackageReq {
+impl AddRmPackageReq {
   pub fn parse(entry_text: &str) -> Result<Result<Self, PackageReq>, AnyError> {
     enum Prefix {
       Jsr,
@@ -719,9 +719,9 @@ impl AddPackageReq {
         let req_ref =
           JsrPackageReqReference::from_str(&format!("jsr:{}", entry_text))?;
         let package_req = req_ref.into_inner().req;
-        Ok(Ok(AddPackageReq {
+        Ok(Ok(AddRmPackageReq {
           alias: maybe_alias.unwrap_or_else(|| package_req.name.to_string()),
-          value: AddPackageReqValue::Jsr(package_req),
+          value: AddRmPackageReqValue::Jsr(package_req),
         }))
       }
       Prefix::Npm => {
@@ -739,9 +739,9 @@ impl AddPackageReq {
             deno_semver::RangeSetOrTag::Tag("latest".into()),
           );
         }
-        Ok(Ok(AddPackageReq {
+        Ok(Ok(AddRmPackageReq {
           alias: maybe_alias.unwrap_or_else(|| package_req.name.to_string()),
-          value: AddPackageReqValue::Npm(package_req),
+          value: AddRmPackageReqValue::Npm(package_req),
         }))
       }
     }
@@ -779,12 +779,28 @@ pub async fn remove(
   let mut removed_packages = vec![];
 
   for package in &remove_flags.packages {
-    let mut removed = false;
+    let req = AddRmPackageReq::parse(package).with_context(|| {
+      format!("Failed to parse package required: {}", package)
+    })?;
+    let mut parsed_pkg_name = None;
     for config in configs.iter_mut().flatten() {
-      removed |= config.remove(package);
+      match &req {
+        Ok(rm_pkg) => {
+          if config.remove(&rm_pkg.alias) && parsed_pkg_name.is_none() {
+            parsed_pkg_name = Some(rm_pkg.alias.clone());
           }
-    if removed {
+        }
-      removed_packages.push(package.clone());
+        Err(pkg) => {
+          // An alias or a package name without registry/version
+          // constraints. Try to remove the package anyway.
+          if config.remove(&pkg.name) && parsed_pkg_name.is_none() {
+            parsed_pkg_name = Some(pkg.name.clone());
+          }
+        }
+      }
+    }
+    if let Some(pkg) = parsed_pkg_name {
+      removed_packages.push(pkg);
     }
   }
 
@@ -913,48 +929,52 @@ mod test {
   #[test]
   fn test_parse_add_package_req() {
     assert_eq!(
-      AddPackageReq::parse("jsr:foo").unwrap().unwrap(),
+      AddRmPackageReq::parse("jsr:foo").unwrap().unwrap(),
-      AddPackageReq {
+      AddRmPackageReq {
         alias: "foo".to_string(),
-        value: AddPackageReqValue::Jsr(PackageReq::from_str("foo").unwrap())
+        value: AddRmPackageReqValue::Jsr(PackageReq::from_str("foo").unwrap())
       }
     );
     assert_eq!(
-      AddPackageReq::parse("alias@jsr:foo").unwrap().unwrap(),
+      AddRmPackageReq::parse("alias@jsr:foo").unwrap().unwrap(),
-      AddPackageReq {
+      AddRmPackageReq {
         alias: "alias".to_string(),
-        value: AddPackageReqValue::Jsr(PackageReq::from_str("foo").unwrap())
+        value: AddRmPackageReqValue::Jsr(PackageReq::from_str("foo").unwrap())
       }
     );
     assert_eq!(
-      AddPackageReq::parse("@alias/pkg@npm:foo").unwrap().unwrap(),
+      AddRmPackageReq::parse("@alias/pkg@npm:foo")
-      AddPackageReq {
+        .unwrap()
+        .unwrap(),
+      AddRmPackageReq {
         alias: "@alias/pkg".to_string(),
-        value: AddPackageReqValue::Npm(
+        value: AddRmPackageReqValue::Npm(
           PackageReq::from_str("foo@latest").unwrap()
         )
       }
     );
     assert_eq!(
-      AddPackageReq::parse("@alias/pkg@jsr:foo").unwrap().unwrap(),
+      AddRmPackageReq::parse("@alias/pkg@jsr:foo")
-      AddPackageReq {
+        .unwrap()
+        .unwrap(),
+      AddRmPackageReq {
         alias: "@alias/pkg".to_string(),
-        value: AddPackageReqValue::Jsr(PackageReq::from_str("foo").unwrap())
+        value: AddRmPackageReqValue::Jsr(PackageReq::from_str("foo").unwrap())
       }
     );
     assert_eq!(
-      AddPackageReq::parse("alias@jsr:foo@^1.5.0")
+      AddRmPackageReq::parse("alias@jsr:foo@^1.5.0")
         .unwrap()
         .unwrap(),
-      AddPackageReq {
+      AddRmPackageReq {
         alias: "alias".to_string(),
-        value: AddPackageReqValue::Jsr(
+        value: AddRmPackageReqValue::Jsr(
           PackageReq::from_str("foo@^1.5.0").unwrap()
         )
       }
     );
     assert_eq!(
-      AddPackageReq::parse("@scope/pkg@tag")
+      AddRmPackageReq::parse("@scope/pkg@tag")
         .unwrap()
         .unwrap_err()
         .to_string(),

tests/specs/remove/alias/__test__.jsonc

@@ -0,0 +1,29 @@
+{
+  "tempDir": true,
+  "tests": {
+    "alias_with_pkg": {
+      "steps": [{
+        "args": "remove my-alias@npm:@denotest/add",
+        "output": "[WILDCARD]"
+      }, {
+        "args": [
+          "eval",
+          "console.log(Deno.readTextFileSync('package.json').trim())"
+        ],
+        "output": "package.json.out"
+      }]
+    },
+    "alias": {
+      "steps": [{
+        "args": "remove my-alias",
+        "output": "[WILDCARD]"
+      }, {
+        "args": [
+          "eval",
+          "console.log(Deno.readTextFileSync('package.json').trim())"
+        ],
+        "output": "package.json.out"
+      }]
+    }
+  }
+}

tests/specs/remove/alias/package.json

@@ -0,0 +1,5 @@
+{
+  "dependencies": {
+    "my-alias": "npm:@denotest/add@^1.0.0"
+  }
+}

tests/specs/remove/alias/package.json.out

@@ -0,0 +1,2 @@
+{
+}

tests/specs/remove/basic/__test__.jsonc

@@ -7,7 +7,7 @@
     "args": ["eval", "console.log(Deno.readTextFileSync('deno.lock').trim())"],
     "output": "add_lock.out"
   }, {
-    "args": ["remove", "@std/assert", "@std/http"],
+    "args": ["remove", "jsr:@std/assert", "@std/http"],
     "output": "rm.out"
   }, {
     "args": ["eval", "console.log(Deno.readTextFileSync('deno.lock').trim())"],