From 1e577614c5177cdde44379fbbfee2c656365be5f Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Tue, 19 Dec 2023 13:25:49 -0500 Subject: [PATCH] refactor(util-schemas): error type for `PackageIdSpec` --- crates/cargo-util-schemas/src/core/mod.rs | 1 + .../src/core/package_id_spec.rs | 69 ++++++++++++------- src/cargo/ops/cargo_compile/packages.rs | 2 +- src/cargo/ops/tree/mod.rs | 4 +- 4 files changed, 47 insertions(+), 29 deletions(-) diff --git a/crates/cargo-util-schemas/src/core/mod.rs b/crates/cargo-util-schemas/src/core/mod.rs index e0cd09538..e8a878aa7 100644 --- a/crates/cargo-util-schemas/src/core/mod.rs +++ b/crates/cargo-util-schemas/src/core/mod.rs @@ -3,6 +3,7 @@ mod partial_version; mod source_kind; pub use package_id_spec::PackageIdSpec; +pub use package_id_spec::PackageIdSpecError; pub use partial_version::PartialVersion; pub use partial_version::PartialVersionError; pub use source_kind::GitReference; diff --git a/crates/cargo-util-schemas/src/core/package_id_spec.rs b/crates/cargo-util-schemas/src/core/package_id_spec.rs index e0a706750..9b5908b42 100644 --- a/crates/cargo-util-schemas/src/core/package_id_spec.rs +++ b/crates/cargo-util-schemas/src/core/package_id_spec.rs @@ -1,7 +1,5 @@ use std::fmt; -use anyhow::bail; -use anyhow::Result; use semver::Version; use serde::{de, ser}; use url::Url; @@ -11,6 +9,8 @@ use crate::core::PartialVersion; use crate::core::SourceKind; use crate::manifest::PackageName; +type Result = std::result::Result; + /// Some or all of the data required to identify a package: /// /// 1. the package name (a `String`, required) @@ -83,12 +83,10 @@ impl PackageIdSpec { if abs.exists() { let maybe_url = Url::from_file_path(abs) .map_or_else(|_| "a file:// URL".to_string(), |url| url.to_string()); - bail!( - "package ID specification `{}` looks like a file path, \ - maybe try {}", - spec, - maybe_url - ); + return Err(PackageIdSpecError::MaybeFilePath { + spec: spec.into(), + maybe_url, + }); } } let mut parts = spec.splitn(2, [':', '@']); @@ -119,14 +117,14 @@ impl PackageIdSpec { } "registry" => { if url.query().is_some() { - bail!("cannot have a query string in a pkgid: {url}") + return Err(PackageIdSpecError::UnexpectedQueryString(url)); } kind = Some(SourceKind::Registry); url = strip_url_protocol(&url); } "sparse" => { if url.query().is_some() { - bail!("cannot have a query string in a pkgid: {url}") + return Err(PackageIdSpecError::UnexpectedQueryString(url)); } kind = Some(SourceKind::SparseRegistry); // Leave `sparse` as part of URL, see `SourceId::new` @@ -134,19 +132,19 @@ impl PackageIdSpec { } "path" => { if url.query().is_some() { - bail!("cannot have a query string in a pkgid: {url}") + return Err(PackageIdSpecError::UnexpectedQueryString(url)); } 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); url = strip_url_protocol(&url); } - kind => anyhow::bail!("unsupported source protocol: {kind}"), + kind => return Err(PackageIdSpecError::UnsupportedProtocol(kind.into())), } } else { 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); let (name, version) = { - let mut path = url - .path_segments() - .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 - ) - })?; + let Some(path_name) = url.path_segments().and_then(|mut p| p.next_back()) else { + return Err(PackageIdSpecError::MissingUrlPath(url)); + }; match frag { Some(fragment) => match fragment.split_once([':', '@']) { Some((name, part)) => { @@ -259,7 +250,7 @@ impl fmt::Display for PackageIdSpec { } impl ser::Serialize for PackageIdSpec { - fn serialize(&self, s: S) -> Result + fn serialize(&self, s: S) -> std::result::Result where S: ser::Serializer, { @@ -268,7 +259,7 @@ impl ser::Serialize for PackageIdSpec { } impl<'de> de::Deserialize<'de> for PackageIdSpec { - fn deserialize(d: D) -> Result + fn deserialize(d: D) -> std::result::Result where 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)] mod tests { use super::PackageIdSpec; diff --git a/src/cargo/ops/cargo_compile/packages.rs b/src/cargo/ops/cargo_compile/packages.rs index 192bfcb3a..0472cc837 100644 --- a/src/cargo/ops/cargo_compile/packages.rs +++ b/src/cargo/ops/cargo_compile/packages.rs @@ -72,7 +72,7 @@ impl Packages { let mut specs = packages .iter() .map(|p| PackageIdSpec::parse(p)) - .collect::>>()?; + .collect::, _>>()?; if !patterns.is_empty() { let matched_pkgs = ws .members() diff --git a/src/cargo/ops/tree/mod.rs b/src/cargo/ops/tree/mod.rs index 6928ec5f9..79a69a66a 100644 --- a/src/cargo/ops/tree/mod.rs +++ b/src/cargo/ops/tree/mod.rs @@ -186,7 +186,7 @@ pub fn build_and_print(ws: &Workspace<'_>, opts: &TreeOptions) -> CargoResult<() opts.invert .iter() .map(|p| PackageIdSpec::parse(p)) - .collect::>>()? + .collect::, _>>()? }; let root_ids = ws_resolve.targeted_resolve.specs_to_ids(&root_specs)?; 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 .pkgs_to_prune .iter() - .map(|p| PackageIdSpec::parse(p)) + .map(|p| PackageIdSpec::parse(p).map_err(Into::into)) .map(|r| { // Provide an error message if pkgid is not within the resolved // dependencies graph.