From 2929d583d2f690b5a3d600296363a8ecd90440eb Mon Sep 17 00:00:00 2001 From: Satya Rohith Date: Wed, 16 Oct 2024 14:50:41 +0530 Subject: [PATCH] fix(cli): consolidate pkg parser for install & remove (#26298) Closes https://github.com/denoland/deno/issues/26283 --- cli/tools/registry/pm.rs | 84 ++++++++++++++--------- tests/specs/remove/alias/__test__.jsonc | 29 ++++++++ tests/specs/remove/alias/package.json | 5 ++ tests/specs/remove/alias/package.json.out | 2 + tests/specs/remove/basic/__test__.jsonc | 2 +- 5 files changed, 89 insertions(+), 33 deletions(-) create mode 100644 tests/specs/remove/alias/__test__.jsonc create mode 100644 tests/specs/remove/alias/package.json create mode 100644 tests/specs/remove/alias/package.json.out diff --git a/cli/tools/registry/pm.rs b/cli/tools/registry/pm.rs index ff900d113d..f716dd2ca6 100644 --- a/cli/tools/registry/pm.rs +++ b/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, npm_resolver: Arc, - add_package_req: AddPackageReq, + add_package_req: AddRmPackageReq, ) -> Result { 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, 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()); + } + } + 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 removed { - removed_packages.push(package.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(), - AddPackageReq { + AddRmPackageReq::parse("jsr:foo").unwrap().unwrap(), + 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(), - AddPackageReq { + AddRmPackageReq::parse("alias@jsr:foo").unwrap().unwrap(), + 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(), - AddPackageReq { + AddRmPackageReq::parse("@alias/pkg@npm:foo") + .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(), - AddPackageReq { + AddRmPackageReq::parse("@alias/pkg@jsr:foo") + .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(), diff --git a/tests/specs/remove/alias/__test__.jsonc b/tests/specs/remove/alias/__test__.jsonc new file mode 100644 index 0000000000..a0c98edd90 --- /dev/null +++ b/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" + }] + } + } +} diff --git a/tests/specs/remove/alias/package.json b/tests/specs/remove/alias/package.json new file mode 100644 index 0000000000..b6326e8bfb --- /dev/null +++ b/tests/specs/remove/alias/package.json @@ -0,0 +1,5 @@ +{ + "dependencies": { + "my-alias": "npm:@denotest/add@^1.0.0" + } +} diff --git a/tests/specs/remove/alias/package.json.out b/tests/specs/remove/alias/package.json.out new file mode 100644 index 0000000000..2c63c08510 --- /dev/null +++ b/tests/specs/remove/alias/package.json.out @@ -0,0 +1,2 @@ +{ +} diff --git a/tests/specs/remove/basic/__test__.jsonc b/tests/specs/remove/basic/__test__.jsonc index fd74900b44..3ca396f8ba 100644 --- a/tests/specs/remove/basic/__test__.jsonc +++ b/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())"],