refactor: Try to make PackageId conversion clearer

epage · Dec 1, 2023 · 378502b · 378502b
1 parent 9787229
commit 378502b
Show file tree

Hide file tree

Showing 8 changed files with 23 additions and 22 deletions.
diff --git a/crates/resolver-tests/src/lib.rs b/crates/resolver-tests/src/lib.rs
@@ -419,14 +419,14 @@ impl ToPkgId for PackageId {
 
 impl<'a> ToPkgId for &'a str {
     fn to_pkgid(&self) -> PackageId {
-        PackageId::new(*self, "1.0.0", registry_loc()).unwrap()
+        PackageId::try_new(*self, "1.0.0", registry_loc()).unwrap()
     }
 }
 
 impl<T: AsRef<str>, U: AsRef<str>> ToPkgId for (T, U) {
     fn to_pkgid(&self) -> PackageId {
         let (name, vers) = self;
-        PackageId::new(name.as_ref(), vers.as_ref(), registry_loc()).unwrap()
+        PackageId::try_new(name.as_ref(), vers.as_ref(), registry_loc()).unwrap()
     }
 }
 
@@ -472,15 +472,15 @@ pub fn pkg_dep<T: ToPkgId>(name: T, dep: Vec<Dependency>) -> Summary {
 }
 
 pub fn pkg_id(name: &str) -> PackageId {
-    PackageId::new(name, "1.0.0", registry_loc()).unwrap()
+    PackageId::try_new(name, "1.0.0", registry_loc()).unwrap()
 }
 
 fn pkg_id_loc(name: &str, loc: &str) -> PackageId {
     let remote = loc.into_url();
     let master = GitReference::Branch("master".to_string());
     let source_id = SourceId::for_git(&remote.unwrap(), master).unwrap();
 
-    PackageId::new(name, "1.0.0", source_id).unwrap()
+    PackageId::try_new(name, "1.0.0", source_id).unwrap()
 }
 
 pub fn pkg_loc(name: &str, loc: &str) -> Summary {

diff --git a/src/cargo/core/package_id.rs b/src/cargo/core/package_id.rs
@@ -123,7 +123,7 @@ impl Hash for PackageId {
 }
 
 impl PackageId {
-    pub fn new(
+    pub fn try_new(
         name: impl Into<InternedString>,
         version: &str,
         sid: SourceId,
@@ -242,25 +242,26 @@ mod tests {
         let loc = CRATES_IO_INDEX.into_url().unwrap();
         let repo = SourceId::for_registry(&loc).unwrap();
 
-        assert!(PackageId::new("foo", "1.0", repo).is_err());
-        assert!(PackageId::new("foo", "1", repo).is_err());
-        assert!(PackageId::new("foo", "bar", repo).is_err());
-        assert!(PackageId::new("foo", "", repo).is_err());
+        assert!(PackageId::try_new("foo", "1.0", repo).is_err());
+        assert!(PackageId::try_new("foo", "1", repo).is_err());
+        assert!(PackageId::try_new("foo", "bar", repo).is_err());
+        assert!(PackageId::try_new("foo", "", repo).is_err());
     }
 
     #[test]
     fn display() {
         let loc = CRATES_IO_INDEX.into_url().unwrap();
-        let pkg_id = PackageId::new("foo", "1.0.0", SourceId::for_registry(&loc).unwrap()).unwrap();
+        let pkg_id =
+            PackageId::try_new("foo", "1.0.0", SourceId::for_registry(&loc).unwrap()).unwrap();
         assert_eq!("foo v1.0.0", pkg_id.to_string());
     }
 
     #[test]
     fn unequal_build_metadata() {
         let loc = CRATES_IO_INDEX.into_url().unwrap();
         let repo = SourceId::for_registry(&loc).unwrap();
-        let first = PackageId::new("foo", "0.0.1+first", repo).unwrap();
-        let second = PackageId::new("foo", "0.0.1+second", repo).unwrap();
+        let first = PackageId::try_new("foo", "0.0.1+first", repo).unwrap();
+        let second = PackageId::try_new("foo", "0.0.1+second", repo).unwrap();
         assert_ne!(first, second);
         assert_ne!(first.inner, second.inner);
     }

diff --git a/src/cargo/core/package_id_spec.rs b/src/cargo/core/package_id_spec.rs
@@ -714,7 +714,7 @@ mod tests {
         let url = Url::parse("https://example.com").unwrap();
         let sid = SourceId::for_registry(&url).unwrap();
 
-        let foo = PackageId::new("foo", "1.2.3", sid).unwrap();
+        let foo = PackageId::try_new("foo", "1.2.3", sid).unwrap();
         assert!(PackageIdSpec::parse("foo").unwrap().matches(foo));
         assert!(!PackageIdSpec::parse("bar").unwrap().matches(foo));
         assert!(PackageIdSpec::parse("foo:1.2.3").unwrap().matches(foo));
@@ -735,7 +735,7 @@ mod tests {
             .unwrap()
             .matches(foo));
 
-        let meta = PackageId::new("meta", "1.2.3+hello", sid).unwrap();
+        let meta = PackageId::try_new("meta", "1.2.3+hello", sid).unwrap();
         assert!(PackageIdSpec::parse("meta").unwrap().matches(meta));
         assert!(PackageIdSpec::parse("meta@1").unwrap().matches(meta));
         assert!(PackageIdSpec::parse("[email protected]").unwrap().matches(meta));
@@ -750,7 +750,7 @@ mod tests {
             .unwrap()
             .matches(meta));
 
-        let pre = PackageId::new("pre", "1.2.3-alpha.0", sid).unwrap();
+        let pre = PackageId::try_new("pre", "1.2.3-alpha.0", sid).unwrap();
         assert!(PackageIdSpec::parse("pre").unwrap().matches(pre));
         assert!(!PackageIdSpec::parse("pre@1").unwrap().matches(pre));
         assert!(!PackageIdSpec::parse("[email protected]").unwrap().matches(pre));

diff --git a/src/cargo/core/resolver/encode.rs b/src/cargo/core/resolver/encode.rs
@@ -209,7 +209,7 @@ impl EncodableResolve {
                         debug!("path dependency now missing {} v{}", pkg.name, pkg.version);
                         continue;
                     }
-                    Some(&source) => PackageId::new(&pkg.name, &pkg.version, source)?,
+                    Some(&source) => PackageId::try_new(&pkg.name, &pkg.version, source)?,
                 };
 
                 // If a package has a checksum listed directly on it then record
@@ -365,7 +365,7 @@ impl EncodableResolve {
         let mut unused_patches = Vec::new();
         for pkg in self.patch.unused {
             let id = match pkg.source.as_deref().or_else(|| path_deps.get(&pkg.name)) {
-                Some(&src) => PackageId::new(&pkg.name, &pkg.version, src)?,
+                Some(&src) => PackageId::try_new(&pkg.name, &pkg.version, src)?,
                 None => continue,
             };
             unused_patches.push(id);

diff --git a/src/cargo/core/resolver/version_prefs.rs b/src/cargo/core/resolver/version_prefs.rs
@@ -139,7 +139,7 @@ mod test {
     fn pkgid(name: &str, version: &str) -> PackageId {
         let src_id =
             SourceId::from_url("registry+https://github.com/rust-lang/crates.io-index").unwrap();
-        PackageId::new(name, version, src_id).unwrap()
+        PackageId::try_new(name, version, src_id).unwrap()
     }
 
     fn dep(name: &str, version: &str) -> Dependency {

diff --git a/src/cargo/core/summary.rs b/src/cargo/core/summary.rs
@@ -474,7 +474,7 @@ mod tests {
     fn valid_feature_names() {
         let loc = CRATES_IO_INDEX.into_url().unwrap();
         let source_id = SourceId::for_registry(&loc).unwrap();
-        let pkg_id = PackageId::new("foo", "1.0.0", source_id).unwrap();
+        let pkg_id = PackageId::try_new("foo", "1.0.0", source_id).unwrap();
 
         assert!(validate_feature_name(pkg_id, "c++17").is_ok());
         assert!(validate_feature_name(pkg_id, "128bit").is_ok());

diff --git a/src/cargo/ops/common_for_install_and_uninstall.rs b/src/cargo/ops/common_for_install_and_uninstall.rs
@@ -650,7 +650,7 @@ cannot install package `{name} {ver}`, it requires rustc {msrv} or newer, while
             let is_yanked: bool = if dep.version_req().is_exact() {
                 let version: String = dep.version_req().to_string();
                 if let Ok(pkg_id) =
-                    PackageId::new(dep.package_name(), &version[1..], source.source_id())
+                    PackageId::try_new(dep.package_name(), &version[1..], source.source_id())
                 {
                     source.invalidate_cache();
                     loop {

diff --git a/tests/testsuite/profile_config.rs b/tests/testsuite/profile_config.rs
@@ -428,8 +428,8 @@ fn named_config_profile() {
     let profiles = Profiles::new(&ws, profile_name).unwrap();
 
     let crates_io = cargo::core::SourceId::crates_io(&config).unwrap();
-    let a_pkg = PackageId::new("a", "0.1.0", crates_io).unwrap();
-    let dep_pkg = PackageId::new("dep", "0.1.0", crates_io).unwrap();
+    let a_pkg = PackageId::try_new("a", "0.1.0", crates_io).unwrap();
+    let dep_pkg = PackageId::try_new("dep", "0.1.0", crates_io).unwrap();
 
     // normal package
     let kind = CompileKind::Host;