Auto merge of #13123 - epage:pure, r=weihanglo

refactor: Clarify PackageId constructor names

### What does this PR try to resolve?

From #13080

> I would love to see eithe rename this function or have a doc comment on it. It always puzzle me what pure means.

From my experience, `new` is more normally a name for more direct construction when there are alternatives

### How should we test and review this PR?

### Additional information

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 {

src/cargo/core/package_id.rs

@@ -88,7 +88,7 @@ impl<'de> de::Deserialize<'de> for PackageId {
             strip_parens(rest).ok_or_else(|| de::Error::custom("invalid serialized PackageId"))?;
         let source_id = SourceId::from_url(url).map_err(de::Error::custom)?;
 
-        Ok(PackageId::pure(name, version, source_id))
+        Ok(PackageId::new(name, version, source_id))
     }
 }
 
@@ -123,16 +123,16 @@ impl Hash for PackageId {
 }
 
 impl PackageId {
-    pub fn new(
+    pub fn try_new(
         name: impl Into<InternedString>,
         version: &str,
         sid: SourceId,
     ) -> CargoResult<PackageId> {
         let v = version.parse()?;
-        Ok(PackageId::pure(name.into(), v, sid))
+        Ok(PackageId::new(name.into(), v, sid))
     }
 
-    pub fn pure(name: InternedString, version: semver::Version, source_id: SourceId) -> PackageId {
+    pub fn new(name: InternedString, version: semver::Version, source_id: SourceId) -> PackageId {
         let inner = PackageIdInner {
             name,
             version,
@@ -161,7 +161,7 @@ impl PackageId {
     }
 
     pub fn with_source_id(self, source: SourceId) -> PackageId {
-        PackageId::pure(self.inner.name, self.inner.version.clone(), source)
+        PackageId::new(self.inner.name, self.inner.version.clone(), source)
     }
 
     pub fn map_source(self, to_replace: SourceId, replace_with: SourceId) -> Self {
@@ -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);
     }

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));

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);

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 {

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());

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 {

src/cargo/sources/registry/index.rs

@@ -944,7 +944,7 @@ impl IndexSummary {
         } = serde_json::from_slice(line)?;
         let v = v.unwrap_or(1);
         tracing::trace!("json parsed registry {}/{}", name, vers);
-        let pkgid = PackageId::pure(name.into(), vers.clone(), source_id);
+        let pkgid = PackageId::new(name.into(), vers.clone(), source_id);
         let deps = deps
             .into_iter()
             .map(|dep| dep.into_dep(source_id))

src/cargo/util/toml/mod.rs

@@ -500,7 +500,7 @@ pub fn to_real_manifest(
 
     package.version = version.clone().map(manifest::InheritableField::Value);
 
-    let pkgid = PackageId::pure(
+    let pkgid = PackageId::new(
         package.name.as_str().into(),
         version
             .clone()

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;