refactor(util-schemas): error type for PackageIdSpec

This commit is contained in:
Weihang Lo 2023-12-19 13:25:49 -05:00
parent a3267bfa29
commit 1e577614c5
No known key found for this signature in database
GPG Key ID: D7DBF189825E82E7
4 changed files with 47 additions and 29 deletions

View File

@ -3,6 +3,7 @@ mod partial_version;
mod source_kind; mod source_kind;
pub use package_id_spec::PackageIdSpec; pub use package_id_spec::PackageIdSpec;
pub use package_id_spec::PackageIdSpecError;
pub use partial_version::PartialVersion; pub use partial_version::PartialVersion;
pub use partial_version::PartialVersionError; pub use partial_version::PartialVersionError;
pub use source_kind::GitReference; pub use source_kind::GitReference;

View File

@ -1,7 +1,5 @@
use std::fmt; use std::fmt;
use anyhow::bail;
use anyhow::Result;
use semver::Version; use semver::Version;
use serde::{de, ser}; use serde::{de, ser};
use url::Url; use url::Url;
@ -11,6 +9,8 @@ use crate::core::PartialVersion;
use crate::core::SourceKind; use crate::core::SourceKind;
use crate::manifest::PackageName; use crate::manifest::PackageName;
type Result<T> = std::result::Result<T, PackageIdSpecError>;
/// Some or all of the data required to identify a package: /// Some or all of the data required to identify a package:
/// ///
/// 1. the package name (a `String`, required) /// 1. the package name (a `String`, required)
@ -83,12 +83,10 @@ impl PackageIdSpec {
if abs.exists() { if abs.exists() {
let maybe_url = Url::from_file_path(abs) let maybe_url = Url::from_file_path(abs)
.map_or_else(|_| "a file:// URL".to_string(), |url| url.to_string()); .map_or_else(|_| "a file:// URL".to_string(), |url| url.to_string());
bail!( return Err(PackageIdSpecError::MaybeFilePath {
"package ID specification `{}` looks like a file path, \ spec: spec.into(),
maybe try {}", maybe_url,
spec, });
maybe_url
);
} }
} }
let mut parts = spec.splitn(2, [':', '@']); let mut parts = spec.splitn(2, [':', '@']);
@ -119,14 +117,14 @@ impl PackageIdSpec {
} }
"registry" => { "registry" => {
if url.query().is_some() { if url.query().is_some() {
bail!("cannot have a query string in a pkgid: {url}") return Err(PackageIdSpecError::UnexpectedQueryString(url));
} }
kind = Some(SourceKind::Registry); kind = Some(SourceKind::Registry);
url = strip_url_protocol(&url); url = strip_url_protocol(&url);
} }
"sparse" => { "sparse" => {
if url.query().is_some() { if url.query().is_some() {
bail!("cannot have a query string in a pkgid: {url}") return Err(PackageIdSpecError::UnexpectedQueryString(url));
} }
kind = Some(SourceKind::SparseRegistry); kind = Some(SourceKind::SparseRegistry);
// Leave `sparse` as part of URL, see `SourceId::new` // Leave `sparse` as part of URL, see `SourceId::new`
@ -134,19 +132,19 @@ impl PackageIdSpec {
} }
"path" => { "path" => {
if url.query().is_some() { if url.query().is_some() {
bail!("cannot have a query string in a pkgid: {url}") return Err(PackageIdSpecError::UnexpectedQueryString(url));
} }
if scheme != "file" { if scheme != "file" {
anyhow::bail!("`path+{scheme}` is unsupported; `path+file` and `file` schemes are supported"); return Err(PackageIdSpecError::UnsupportedPathPlusScheme(scheme.into()));
} }
kind = Some(SourceKind::Path); kind = Some(SourceKind::Path);
url = strip_url_protocol(&url); url = strip_url_protocol(&url);
} }
kind => anyhow::bail!("unsupported source protocol: {kind}"), kind => return Err(PackageIdSpecError::UnsupportedProtocol(kind.into())),
} }
} else { } else {
if url.query().is_some() { if url.query().is_some() {
bail!("cannot have a query string in a pkgid: {url}") return Err(PackageIdSpecError::UnexpectedQueryString(url));
} }
} }
@ -154,16 +152,9 @@ impl PackageIdSpec {
url.set_fragment(None); url.set_fragment(None);
let (name, version) = { let (name, version) = {
let mut path = url let Some(path_name) = url.path_segments().and_then(|mut p| p.next_back()) else {
.path_segments() return Err(PackageIdSpecError::MissingUrlPath(url));
.ok_or_else(|| anyhow::format_err!("pkgid urls must have a path: {}", url))?; };
let path_name = path.next_back().ok_or_else(|| {
anyhow::format_err!(
"pkgid urls must have at least one path \
component: {}",
url
)
})?;
match frag { match frag {
Some(fragment) => match fragment.split_once([':', '@']) { Some(fragment) => match fragment.split_once([':', '@']) {
Some((name, part)) => { Some((name, part)) => {
@ -259,7 +250,7 @@ impl fmt::Display for PackageIdSpec {
} }
impl ser::Serialize for PackageIdSpec { impl ser::Serialize for PackageIdSpec {
fn serialize<S>(&self, s: S) -> Result<S::Ok, S::Error> fn serialize<S>(&self, s: S) -> std::result::Result<S::Ok, S::Error>
where where
S: ser::Serializer, S: ser::Serializer,
{ {
@ -268,7 +259,7 @@ impl ser::Serialize for PackageIdSpec {
} }
impl<'de> de::Deserialize<'de> for PackageIdSpec { impl<'de> de::Deserialize<'de> for PackageIdSpec {
fn deserialize<D>(d: D) -> Result<PackageIdSpec, D::Error> fn deserialize<D>(d: D) -> std::result::Result<PackageIdSpec, D::Error>
where where
D: de::Deserializer<'de>, D: de::Deserializer<'de>,
{ {
@ -277,6 +268,32 @@ impl<'de> de::Deserialize<'de> for PackageIdSpec {
} }
} }
/// Error parsing a [`PackageIdSpec`].
#[non_exhaustive]
#[derive(Debug, thiserror::Error)]
pub enum PackageIdSpecError {
#[error("unsupported source protocol: {0}")]
UnsupportedProtocol(String),
#[error("`path+{0}` is unsupported; `path+file` and `file` schemes are supported")]
UnsupportedPathPlusScheme(String),
#[error("cannot have a query string in a pkgid: {0}")]
UnexpectedQueryString(Url),
#[error("pkgid urls must have at least one path component: {0}")]
MissingUrlPath(Url),
#[error("package ID specification `{spec}` looks like a file path, maybe try {maybe_url}")]
MaybeFilePath { spec: String, maybe_url: String },
#[error(transparent)]
NameValidation(#[from] crate::restricted_names::NameValidationError),
#[error(transparent)]
PartialVersion(#[from] crate::core::PartialVersionError),
}
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
use super::PackageIdSpec; use super::PackageIdSpec;

View File

@ -72,7 +72,7 @@ impl Packages {
let mut specs = packages let mut specs = packages
.iter() .iter()
.map(|p| PackageIdSpec::parse(p)) .map(|p| PackageIdSpec::parse(p))
.collect::<CargoResult<Vec<_>>>()?; .collect::<Result<Vec<_>, _>>()?;
if !patterns.is_empty() { if !patterns.is_empty() {
let matched_pkgs = ws let matched_pkgs = ws
.members() .members()

View File

@ -186,7 +186,7 @@ pub fn build_and_print(ws: &Workspace<'_>, opts: &TreeOptions) -> CargoResult<()
opts.invert opts.invert
.iter() .iter()
.map(|p| PackageIdSpec::parse(p)) .map(|p| PackageIdSpec::parse(p))
.collect::<CargoResult<Vec<PackageIdSpec>>>()? .collect::<Result<Vec<PackageIdSpec>, _>>()?
}; };
let root_ids = ws_resolve.targeted_resolve.specs_to_ids(&root_specs)?; let root_ids = ws_resolve.targeted_resolve.specs_to_ids(&root_specs)?;
let root_indexes = graph.indexes_from_ids(&root_ids); let root_indexes = graph.indexes_from_ids(&root_ids);
@ -207,7 +207,7 @@ pub fn build_and_print(ws: &Workspace<'_>, opts: &TreeOptions) -> CargoResult<()
let pkgs_to_prune = opts let pkgs_to_prune = opts
.pkgs_to_prune .pkgs_to_prune
.iter() .iter()
.map(|p| PackageIdSpec::parse(p)) .map(|p| PackageIdSpec::parse(p).map_err(Into::into))
.map(|r| { .map(|r| {
// Provide an error message if pkgid is not within the resolved // Provide an error message if pkgid is not within the resolved
// dependencies graph. // dependencies graph.