Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: Clarify PackageId constructor names #13123

Merged
merged 2 commits into from
Dec 6, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions crates/resolver-tests/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
}

Expand Down Expand Up @@ -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 {
Expand Down
25 changes: 13 additions & 12 deletions src/cargo/core/package_id.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
}

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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);
}
Expand Down
6 changes: 3 additions & 3 deletions src/cargo/core/package_id_spec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand All @@ -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("meta@1.2").unwrap().matches(meta));
Expand All @@ -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("pre@1.2").unwrap().matches(pre));
Expand Down
4 changes: 2 additions & 2 deletions src/cargo/core/resolver/encode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion src/cargo/core/resolver/version_prefs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion src/cargo/core/summary.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
2 changes: 1 addition & 1 deletion src/cargo/ops/common_for_install_and_uninstall.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion src/cargo/sources/registry/index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
2 changes: 1 addition & 1 deletion src/cargo/util/toml/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
4 changes: 2 additions & 2 deletions tests/testsuite/profile_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down