From d7bcc0cd26983b00daf7de5f00e175a4dc4ef0b6 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Fri, 1 Dec 2023 12:49:07 -0600 Subject: [PATCH] refactor(spec): Move queries to a extension trait This is a step towards moving `PackageIdSpec` into `schema` as manifests need it as part of #12801. While I didn't take this approach in #13080, that was largely how core these functions are / how pervasive their use is. --- src/bin/cargo/commands/remove.rs | 1 + src/cargo/core/mod.rs | 2 +- src/cargo/core/package_id_spec.rs | 161 +++++++++++++---------- src/cargo/core/profiles.rs | 4 +- src/cargo/core/resolver/dep_cache.rs | 4 +- src/cargo/core/resolver/resolve.rs | 2 +- src/cargo/core/workspace.rs | 4 +- src/cargo/ops/cargo_clean.rs | 2 +- src/cargo/ops/cargo_generate_lockfile.rs | 2 +- src/cargo/ops/cargo_pkgid.rs | 2 +- src/cargo/ops/cargo_uninstall.rs | 2 +- src/cargo/ops/registry/publish.rs | 1 + src/cargo/ops/resolve.rs | 4 +- src/cargo/ops/tree/mod.rs | 2 +- 14 files changed, 109 insertions(+), 84 deletions(-) diff --git a/src/bin/cargo/commands/remove.rs b/src/bin/cargo/commands/remove.rs index 8344d38d236..b7abb171510 100644 --- a/src/bin/cargo/commands/remove.rs +++ b/src/bin/cargo/commands/remove.rs @@ -1,5 +1,6 @@ use cargo::core::dependency::DepKind; use cargo::core::PackageIdSpec; +use cargo::core::PackageIdSpecQuery; use cargo::core::Resolve; use cargo::core::Workspace; use cargo::ops::cargo_remove::remove; diff --git a/src/cargo/core/mod.rs b/src/cargo/core/mod.rs index eea910b6651..41a5a315162 100644 --- a/src/cargo/core/mod.rs +++ b/src/cargo/core/mod.rs @@ -4,7 +4,7 @@ pub use self::manifest::{EitherManifest, VirtualManifest}; pub use self::manifest::{Manifest, Target, TargetKind}; pub use self::package::{Package, PackageSet}; pub use self::package_id::PackageId; -pub use self::package_id_spec::PackageIdSpec; +pub use self::package_id_spec::{PackageIdSpec, PackageIdSpecQuery}; pub use self::registry::Registry; pub use self::resolver::{Resolve, ResolveVersion}; pub use self::shell::{Shell, Verbosity}; diff --git a/src/cargo/core/package_id_spec.rs b/src/cargo/core/package_id_spec.rs index b25cb826c5f..ca4b9427af0 100644 --- a/src/cargo/core/package_id_spec.rs +++ b/src/cargo/core/package_id_spec.rs @@ -85,19 +85,6 @@ impl PackageIdSpec { }) } - /// Roughly equivalent to `PackageIdSpec::parse(spec)?.query(i)` - pub fn query_str(spec: &str, i: I) -> CargoResult - where - I: IntoIterator, - { - let i: Vec<_> = i.into_iter().collect(); - let spec = PackageIdSpec::parse(spec).with_context(|| { - let suggestion = edit_distance::closest_msg(spec, i.iter(), |id| id.name().as_str()); - format!("invalid package ID specification: `{}`{}", spec, suggestion) - })?; - spec.query(i) - } - /// Convert a `PackageId` to a `PackageIdSpec`, which will have both the `PartialVersion` and `Url` /// fields filled in. pub fn from_package_id(package_id: PackageId) -> PackageIdSpec { @@ -221,9 +208,94 @@ impl PackageIdSpec { pub fn set_kind(&mut self, kind: SourceKind) { self.kind = Some(kind); } +} + +fn strip_url_protocol(url: &Url) -> Url { + // Ridiculous hoop because `Url::set_scheme` errors when changing to http/https + let raw = url.to_string(); + raw.split_once('+').unwrap().1.parse().unwrap() +} + +impl fmt::Display for PackageIdSpec { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let mut printed_name = false; + match self.url { + Some(ref url) => { + if let Some(protocol) = self.kind.as_ref().and_then(|k| k.protocol()) { + write!(f, "{protocol}+")?; + } + write!(f, "{}", url)?; + if let Some(SourceKind::Git(git_ref)) = self.kind.as_ref() { + if let Some(pretty) = git_ref.pretty_ref(true) { + write!(f, "?{}", pretty)?; + } + } + if url.path_segments().unwrap().next_back().unwrap() != &*self.name { + printed_name = true; + write!(f, "#{}", self.name)?; + } + } + None => { + printed_name = true; + write!(f, "{}", self.name)?; + } + } + if let Some(ref v) = self.version { + write!(f, "{}{}", if printed_name { "@" } else { "#" }, v)?; + } + Ok(()) + } +} + +impl ser::Serialize for PackageIdSpec { + fn serialize(&self, s: S) -> Result + where + S: ser::Serializer, + { + self.to_string().serialize(s) + } +} + +impl<'de> de::Deserialize<'de> for PackageIdSpec { + fn deserialize(d: D) -> Result + where + D: de::Deserializer<'de>, + { + let string = String::deserialize(d)?; + PackageIdSpec::parse(&string).map_err(de::Error::custom) + } +} + +pub trait PackageIdSpecQuery { + /// Roughly equivalent to `PackageIdSpec::parse(spec)?.query(i)` + fn query_str(spec: &str, i: I) -> CargoResult + where + I: IntoIterator; /// Checks whether the given `PackageId` matches the `PackageIdSpec`. - pub fn matches(&self, package_id: PackageId) -> bool { + fn matches(&self, package_id: PackageId) -> bool; + + /// Checks a list of `PackageId`s to find 1 that matches this `PackageIdSpec`. If 0, 2, or + /// more are found, then this returns an error. + fn query(&self, i: I) -> CargoResult + where + I: IntoIterator; +} + +impl PackageIdSpecQuery for PackageIdSpec { + fn query_str(spec: &str, i: I) -> CargoResult + where + I: IntoIterator, + { + let i: Vec<_> = i.into_iter().collect(); + let spec = PackageIdSpec::parse(spec).with_context(|| { + let suggestion = edit_distance::closest_msg(spec, i.iter(), |id| id.name().as_str()); + format!("invalid package ID specification: `{}`{}", spec, suggestion) + })?; + spec.query(i) + } + + fn matches(&self, package_id: PackageId) -> bool { if self.name() != package_id.name().as_str() { return false; } @@ -249,9 +321,7 @@ impl PackageIdSpec { true } - /// Checks a list of `PackageId`s to find 1 that matches this `PackageIdSpec`. If 0, 2, or - /// more are found, then this returns an error. - pub fn query(&self, i: I) -> CargoResult + fn query(&self, i: I) -> CargoResult where I: IntoIterator, { @@ -342,65 +412,10 @@ impl PackageIdSpec { } } -fn strip_url_protocol(url: &Url) -> Url { - // Ridiculous hoop because `Url::set_scheme` errors when changing to http/https - let raw = url.to_string(); - raw.split_once('+').unwrap().1.parse().unwrap() -} - -impl fmt::Display for PackageIdSpec { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - let mut printed_name = false; - match self.url { - Some(ref url) => { - if let Some(protocol) = self.kind.as_ref().and_then(|k| k.protocol()) { - write!(f, "{protocol}+")?; - } - write!(f, "{}", url)?; - if let Some(SourceKind::Git(git_ref)) = self.kind.as_ref() { - if let Some(pretty) = git_ref.pretty_ref(true) { - write!(f, "?{}", pretty)?; - } - } - if url.path_segments().unwrap().next_back().unwrap() != &*self.name { - printed_name = true; - write!(f, "#{}", self.name)?; - } - } - None => { - printed_name = true; - write!(f, "{}", self.name)?; - } - } - if let Some(ref v) = self.version { - write!(f, "{}{}", if printed_name { "@" } else { "#" }, v)?; - } - Ok(()) - } -} - -impl ser::Serialize for PackageIdSpec { - fn serialize(&self, s: S) -> Result - where - S: ser::Serializer, - { - self.to_string().serialize(s) - } -} - -impl<'de> de::Deserialize<'de> for PackageIdSpec { - fn deserialize(d: D) -> Result - where - D: de::Deserializer<'de>, - { - let string = String::deserialize(d)?; - PackageIdSpec::parse(&string).map_err(de::Error::custom) - } -} - #[cfg(test)] mod tests { use super::PackageIdSpec; + use super::PackageIdSpecQuery; use crate::core::{GitReference, PackageId, SourceId, SourceKind}; use url::Url; diff --git a/src/cargo/core/profiles.rs b/src/cargo/core/profiles.rs index 34365008ee5..4d2a23f5060 100644 --- a/src/cargo/core/profiles.rs +++ b/src/cargo/core/profiles.rs @@ -25,7 +25,9 @@ use crate::core::compiler::{CompileKind, CompileTarget, Unit}; use crate::core::dependency::Artifact; use crate::core::resolver::features::FeaturesFor; use crate::core::Feature; -use crate::core::{PackageId, PackageIdSpec, Resolve, Shell, Target, Workspace}; +use crate::core::{ + PackageId, PackageIdSpec, PackageIdSpecQuery, Resolve, Shell, Target, Workspace, +}; use crate::util::interning::InternedString; use crate::util::toml::validate_profile; use crate::util::{closest_msg, config, CargoResult, Config}; diff --git a/src/cargo/core/resolver/dep_cache.rs b/src/cargo/core/resolver/dep_cache.rs index 00a269482b7..9e8ffd3510c 100644 --- a/src/cargo/core/resolver/dep_cache.rs +++ b/src/cargo/core/resolver/dep_cache.rs @@ -16,7 +16,9 @@ use crate::core::resolver::{ ActivateError, ActivateResult, CliFeatures, RequestedFeatures, ResolveOpts, VersionOrdering, VersionPreferences, }; -use crate::core::{Dependency, FeatureValue, PackageId, PackageIdSpec, Registry, Summary}; +use crate::core::{ + Dependency, FeatureValue, PackageId, PackageIdSpec, PackageIdSpecQuery, Registry, Summary, +}; use crate::sources::source::QueryKind; use crate::util::errors::CargoResult; use crate::util::interning::InternedString; diff --git a/src/cargo/core/resolver/resolve.rs b/src/cargo/core/resolver/resolve.rs index b401e923275..02f112166dc 100644 --- a/src/cargo/core/resolver/resolve.rs +++ b/src/cargo/core/resolver/resolve.rs @@ -1,6 +1,6 @@ use super::encode::Metadata; use crate::core::dependency::DepKind; -use crate::core::{Dependency, PackageId, PackageIdSpec, Summary, Target}; +use crate::core::{Dependency, PackageId, PackageIdSpec, PackageIdSpecQuery, Summary, Target}; use crate::util::errors::CargoResult; use crate::util::interning::InternedString; use crate::util::Graph; diff --git a/src/cargo/core/workspace.rs b/src/cargo/core/workspace.rs index 3467fe18ee8..a47d994cff1 100644 --- a/src/cargo/core/workspace.rs +++ b/src/cargo/core/workspace.rs @@ -15,7 +15,9 @@ use crate::core::features::Features; use crate::core::registry::PackageRegistry; use crate::core::resolver::features::CliFeatures; use crate::core::resolver::ResolveBehavior; -use crate::core::{Dependency, Edition, FeatureValue, PackageId, PackageIdSpec}; +use crate::core::{ + Dependency, Edition, FeatureValue, PackageId, PackageIdSpec, PackageIdSpecQuery, +}; use crate::core::{EitherManifest, Package, SourceId, VirtualManifest}; use crate::ops; use crate::sources::{PathSource, CRATES_IO_INDEX, CRATES_IO_REGISTRY}; diff --git a/src/cargo/ops/cargo_clean.rs b/src/cargo/ops/cargo_clean.rs index 923b2decdc4..4add5d86326 100644 --- a/src/cargo/ops/cargo_clean.rs +++ b/src/cargo/ops/cargo_clean.rs @@ -1,6 +1,6 @@ use crate::core::compiler::{CompileKind, CompileMode, Layout, RustcTargetData}; use crate::core::profiles::Profiles; -use crate::core::{PackageIdSpec, TargetKind, Workspace}; +use crate::core::{PackageIdSpec, PackageIdSpecQuery, TargetKind, Workspace}; use crate::ops; use crate::util::edit_distance; use crate::util::errors::CargoResult; diff --git a/src/cargo/ops/cargo_generate_lockfile.rs b/src/cargo/ops/cargo_generate_lockfile.rs index 03c38630e92..1bba64925ec 100644 --- a/src/cargo/ops/cargo_generate_lockfile.rs +++ b/src/cargo/ops/cargo_generate_lockfile.rs @@ -1,6 +1,6 @@ use crate::core::registry::PackageRegistry; use crate::core::resolver::features::{CliFeatures, HasDevUnits}; -use crate::core::{PackageId, PackageIdSpec}; +use crate::core::{PackageId, PackageIdSpec, PackageIdSpecQuery}; use crate::core::{Resolve, SourceId, Workspace}; use crate::ops; use crate::util::cache_lock::CacheLockMode; diff --git a/src/cargo/ops/cargo_pkgid.rs b/src/cargo/ops/cargo_pkgid.rs index bbae154a736..2324e25b44e 100644 --- a/src/cargo/ops/cargo_pkgid.rs +++ b/src/cargo/ops/cargo_pkgid.rs @@ -1,4 +1,4 @@ -use crate::core::{PackageIdSpec, Workspace}; +use crate::core::{PackageIdSpec, PackageIdSpecQuery, Workspace}; use crate::ops; use crate::util::CargoResult; diff --git a/src/cargo/ops/cargo_uninstall.rs b/src/cargo/ops/cargo_uninstall.rs index 7b9fdccd4cb..7b45a69b4a6 100644 --- a/src/cargo/ops/cargo_uninstall.rs +++ b/src/cargo/ops/cargo_uninstall.rs @@ -1,5 +1,5 @@ use crate::core::PackageId; -use crate::core::{PackageIdSpec, SourceId}; +use crate::core::{PackageIdSpec, PackageIdSpecQuery, SourceId}; use crate::ops::common_for_install_and_uninstall::*; use crate::sources::PathSource; use crate::util::errors::CargoResult; diff --git a/src/cargo/ops/registry/publish.rs b/src/cargo/ops/registry/publish.rs index 201907bb268..2313792c8cd 100644 --- a/src/cargo/ops/registry/publish.rs +++ b/src/cargo/ops/registry/publish.rs @@ -21,6 +21,7 @@ use crate::core::manifest::ManifestMetadata; use crate::core::resolver::CliFeatures; use crate::core::Dependency; use crate::core::Package; +use crate::core::PackageIdSpecQuery; use crate::core::SourceId; use crate::core::Workspace; use crate::ops; diff --git a/src/cargo/ops/resolve.rs b/src/cargo/ops/resolve.rs index c3ae6b2def8..06716a2b4cf 100644 --- a/src/cargo/ops/resolve.rs +++ b/src/cargo/ops/resolve.rs @@ -64,7 +64,9 @@ use crate::core::resolver::{ self, HasDevUnits, Resolve, ResolveOpts, ResolveVersion, VersionOrdering, VersionPreferences, }; use crate::core::summary::Summary; -use crate::core::{GitReference, PackageId, PackageIdSpec, PackageSet, SourceId, Workspace}; +use crate::core::{ + GitReference, PackageId, PackageIdSpec, PackageIdSpecQuery, PackageSet, SourceId, Workspace, +}; use crate::ops; use crate::sources::PathSource; use crate::util::cache_lock::CacheLockMode; diff --git a/src/cargo/ops/tree/mod.rs b/src/cargo/ops/tree/mod.rs index ce3bae8ccd0..6928ec5f94b 100644 --- a/src/cargo/ops/tree/mod.rs +++ b/src/cargo/ops/tree/mod.rs @@ -4,7 +4,7 @@ use self::format::Pattern; use crate::core::compiler::{CompileKind, RustcTargetData}; use crate::core::dependency::DepKind; use crate::core::resolver::{features::CliFeatures, ForceAllTargets, HasDevUnits}; -use crate::core::{Package, PackageId, PackageIdSpec, Workspace}; +use crate::core::{Package, PackageId, PackageIdSpec, PackageIdSpecQuery, Workspace}; use crate::ops::{self, Packages}; use crate::util::{CargoResult, Config}; use crate::{drop_print, drop_println};