refactor(util-schemas): error type for PackageIdSpec

Author: weihanglo

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;

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<T> = std::result::Result<T, PackageIdSpecError>;
+
 /// 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,51 +117,44 @@ 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`
                     // url = strip_url_protocol(&url);
                 }
                 "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));
             }
         }
 
         let frag = url.fragment().map(|s| s.to_owned());
         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<S>(&self, s: S) -> Result<S::Ok, S::Error>
+    fn serialize<S>(&self, s: S) -> std::result::Result<S::Ok, S::Error>
     where
         S: ser::Serializer,
     {
@@ -268,7 +259,7 @@ impl ser::Serialize 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
         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;

src/cargo/ops/cargo_compile/packages.rs

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

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::<CargoResult<Vec<PackageIdSpec>>>()?
+            .collect::<Result<Vec<PackageIdSpec>, _>>()?
     };
     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.