refactor(util-schemas): make error enum private

Author: weihanglo

crates/cargo-util-schemas/src/core/package_id_spec.rs

@@ -6,8 +6,10 @@ use url::Url;
 
 use crate::core::GitReference;
 use crate::core::PartialVersion;
+use crate::core::PartialVersionError;
 use crate::core::SourceKind;
 use crate::manifest::PackageName;
+use crate::restricted_names::NameValidationError;
 
 type Result<T> = std::result::Result<T, PackageIdSpecError>;
 
@@ -83,10 +85,11 @@ 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());
-                return Err(PackageIdSpecError::MaybeFilePath {
+                return Err(ErrorKind::MaybeFilePath {
                     spec: spec.into(),
                     maybe_url,
-                });
+                }
+                .into());
             }
         }
         let mut parts = spec.splitn(2, [':', '@']);
@@ -117,34 +120,34 @@ impl PackageIdSpec {
                 }
                 "registry" => {
                     if url.query().is_some() {
-                        return Err(PackageIdSpecError::UnexpectedQueryString(url));
+                        return Err(ErrorKind::UnexpectedQueryString(url).into());
                     }
                     kind = Some(SourceKind::Registry);
                     url = strip_url_protocol(&url);
                 }
                 "sparse" => {
                     if url.query().is_some() {
-                        return Err(PackageIdSpecError::UnexpectedQueryString(url));
+                        return Err(ErrorKind::UnexpectedQueryString(url).into());
                     }
                     kind = Some(SourceKind::SparseRegistry);
                     // Leave `sparse` as part of URL, see `SourceId::new`
                     // url = strip_url_protocol(&url);
                 }
                 "path" => {
                     if url.query().is_some() {
-                        return Err(PackageIdSpecError::UnexpectedQueryString(url));
+                        return Err(ErrorKind::UnexpectedQueryString(url).into());
                     }
                     if scheme != "file" {
-                        return Err(PackageIdSpecError::UnsupportedPathPlusScheme(scheme.into()));
+                        return Err(ErrorKind::UnsupportedPathPlusScheme(scheme.into()).into());
                     }
                     kind = Some(SourceKind::Path);
                     url = strip_url_protocol(&url);
                 }
-                kind => return Err(PackageIdSpecError::UnsupportedProtocol(kind.into())),
+                kind => return Err(ErrorKind::UnsupportedProtocol(kind.into()).into()),
             }
         } else {
             if url.query().is_some() {
-                return Err(PackageIdSpecError::UnexpectedQueryString(url));
+                return Err(ErrorKind::UnexpectedQueryString(url).into());
             }
         }
 
@@ -153,7 +156,7 @@ impl PackageIdSpec {
 
         let (name, version) = {
             let Some(path_name) = url.path_segments().and_then(|mut p| p.next_back()) else {
-                return Err(PackageIdSpecError::MissingUrlPath(url));
+                return Err(ErrorKind::MissingUrlPath(url).into());
             };
             match frag {
                 Some(fragment) => match fragment.split_once([':', '@']) {
@@ -269,9 +272,26 @@ impl<'de> de::Deserialize<'de> for PackageIdSpec {
 }
 
 /// Error parsing a [`PackageIdSpec`].
+#[derive(Debug, thiserror::Error)]
+#[error(transparent)]
+pub struct PackageIdSpecError(#[from] ErrorKind);
+
+impl From<PartialVersionError> for PackageIdSpecError {
+    fn from(value: PartialVersionError) -> Self {
+        ErrorKind::PartialVersion(value).into()
+    }
+}
+
+impl From<NameValidationError> for PackageIdSpecError {
+    fn from(value: NameValidationError) -> Self {
+        ErrorKind::NameValidation(value).into()
+    }
+}
+
+/// Non-public error kind for [`PackageIdSpecError`].
 #[non_exhaustive]
 #[derive(Debug, thiserror::Error)]
-pub enum PackageIdSpecError {
+enum ErrorKind {
     #[error("unsupported source protocol: {0}")]
     UnsupportedProtocol(String),
 

crates/cargo-util-schemas/src/core/partial_version.rs

@@ -84,19 +84,17 @@ impl std::str::FromStr for PartialVersion {
 
     fn from_str(value: &str) -> Result<Self, Self::Err> {
         if is_req(value) {
-            return Err(PartialVersionError::VersionReq);
+            return Err(ErrorKind::VersionReq.into());
         }
         match semver::Version::parse(value) {
             Ok(ver) => Ok(ver.into()),
             Err(_) => {
                 // HACK: Leverage `VersionReq` for partial version parsing
                 let mut version_req = match semver::VersionReq::parse(value) {
                     Ok(req) => req,
-                    Err(_) if value.contains('-') => return Err(PartialVersionError::Prerelease),
-                    Err(_) if value.contains('+') => {
-                        return Err(PartialVersionError::BuildMetadata)
-                    }
-                    Err(_) => return Err(PartialVersionError::Unexpected),
+                    Err(_) if value.contains('-') => return Err(ErrorKind::Prerelease.into()),
+                    Err(_) if value.contains('+') => return Err(ErrorKind::BuildMetadata.into()),
+                    Err(_) => return Err(ErrorKind::Unexpected.into()),
                 };
                 assert_eq!(version_req.comparators.len(), 1, "guaranteed by is_req");
                 let comp = version_req.comparators.pop().unwrap();
@@ -160,9 +158,14 @@ impl<'de> serde::Deserialize<'de> for PartialVersion {
 }
 
 /// Error parsing a [`PartialVersion`].
+#[derive(Debug, thiserror::Error)]
+#[error(transparent)]
+pub struct PartialVersionError(#[from] ErrorKind);
+
+/// Non-public error kind for [`PartialVersionError`].
 #[non_exhaustive]
 #[derive(Debug, thiserror::Error)]
-pub enum PartialVersionError {
+enum ErrorKind {
     #[error("unexpected version requirement, expected a version like \"1.32\"")]
     VersionReq,
 

crates/cargo-util-schemas/src/manifest.rs

@@ -1339,12 +1339,13 @@ impl std::str::FromStr for RustVersion {
     type Err = RustVersionError;
 
     fn from_str(value: &str) -> Result<Self, Self::Err> {
-        let partial = value.parse::<PartialVersion>()?;
+        let partial = value.parse::<PartialVersion>();
+        let partial = partial.map_err(RustVersionErrorKind::PartialVersion)?;
         if partial.pre.is_some() {
-            return Err(RustVersionError::Prerelease);
+            return Err(RustVersionErrorKind::Prerelease.into());
         }
         if partial.build.is_some() {
-            return Err(RustVersionError::BuildMetadata);
+            return Err(RustVersionErrorKind::BuildMetadata.into());
         }
         Ok(Self(partial))
     }
@@ -1368,10 +1369,15 @@ impl Display for RustVersion {
     }
 }
 
-/// Error parsing a [`PartialVersion`].
+/// Error parsing a [`RustVersion`].
+#[derive(Debug, thiserror::Error)]
+#[error(transparent)]
+pub struct RustVersionError(#[from] RustVersionErrorKind);
+
+/// Non-public error kind for [`RustVersionError`].
 #[non_exhaustive]
 #[derive(Debug, thiserror::Error)]
-pub enum RustVersionError {
+enum RustVersionErrorKind {
     #[error("unexpected prerelease field, expected a version like \"1.32\"")]
     Prerelease,
 

crates/cargo-util-schemas/src/restricted_names.rs

@@ -3,9 +3,14 @@
 type Result<T> = std::result::Result<T, NameValidationError>;
 
 /// Error validating names in Cargo.
+#[derive(Debug, thiserror::Error)]
+#[error(transparent)]
+pub struct NameValidationError(#[from] ErrorKind);
+
+/// Non-public error kind for [`NameValidationError`].
 #[non_exhaustive]
 #[derive(Debug, thiserror::Error)]
-pub enum NameValidationError {
+enum ErrorKind {
     #[error("{0} cannot be empty")]
     Empty(&'static str),
 
@@ -36,39 +41,42 @@ pub enum NameValidationError {
 /// reserved names. crates.io has even more restrictions.
 pub(crate) fn validate_package_name(name: &str, what: &'static str) -> Result<()> {
     if name.is_empty() {
-        return Err(NameValidationError::Empty(what));
+        return Err(ErrorKind::Empty(what).into());
     }
 
     let mut chars = name.chars();
     if let Some(ch) = chars.next() {
         if ch.is_digit(10) {
             // A specific error for a potentially common case.
-            return Err(NameValidationError::InvalidCharacter {
+            return Err(ErrorKind::InvalidCharacter {
                 ch,
                 what,
                 name: name.into(),
                 reason: "the name cannot start with a digit",
-            });
+            }
+            .into());
         }
         if !(unicode_xid::UnicodeXID::is_xid_start(ch) || ch == '_') {
-            return Err(NameValidationError::InvalidCharacter {
+            return Err(ErrorKind::InvalidCharacter {
                 ch,
                 what,
                 name: name.into(),
                 reason: "the first character must be a Unicode XID start character \
                  (most letters or `_`)",
-            });
+            }
+            .into());
         }
     }
     for ch in chars {
         if !(unicode_xid::UnicodeXID::is_xid_continue(ch) || ch == '-') {
-            return Err(NameValidationError::InvalidCharacter {
+            return Err(ErrorKind::InvalidCharacter {
                 ch,
                 what,
                 name: name.into(),
                 reason: "characters must be Unicode XID characters \
                  (numbers, `-`, `_`, or most letters)",
-            });
+            }
+            .into());
         }
     }
     Ok(())
@@ -103,28 +111,31 @@ pub(crate) fn validate_profile_name(name: &str) -> Result<()> {
         .chars()
         .find(|ch| !ch.is_alphanumeric() && *ch != '_' && *ch != '-')
     {
-        return Err(NameValidationError::InvalidCharacter {
+        return Err(ErrorKind::InvalidCharacter {
             ch,
             what: "profile name",
             name: name.into(),
             reason: "allowed characters are letters, numbers, underscore, and hyphen",
-        });
+        }
+        .into());
     }
 
     let lower_name = name.to_lowercase();
     if lower_name == "debug" {
-        return Err(NameValidationError::ProfileNameReservedKeyword {
+        return Err(ErrorKind::ProfileNameReservedKeyword {
             name: name.into(),
             help: "To configure the default development profile, \
                 use the name `dev` as in [profile.dev]",
-        });
+        }
+        .into());
     }
     if lower_name == "build-override" {
-        return Err(NameValidationError::ProfileNameReservedKeyword {
+        return Err(ErrorKind::ProfileNameReservedKeyword {
             name: name.into(),
             help: "To configure build dependency settings, use [profile.dev.build-override] \
                  and [profile.release.build-override]",
-        });
+        }
+        .into());
     }
 
     // These are some arbitrary reservations. We have no plans to use
@@ -155,10 +166,11 @@ pub(crate) fn validate_profile_name(name: &str) -> Result<()> {
             | "uninstall"
     ) || lower_name.starts_with("cargo")
     {
-        return Err(NameValidationError::ProfileNameReservedKeyword {
+        return Err(ErrorKind::ProfileNameReservedKeyword {
             name: name.into(),
             help: "Please choose a different name.",
-        });
+        }
+        .into());
     }
 
     Ok(())
@@ -167,43 +179,44 @@ pub(crate) fn validate_profile_name(name: &str) -> Result<()> {
 pub(crate) fn validate_feature_name(name: &str) -> Result<()> {
     let what = "feature name";
     if name.is_empty() {
-        return Err(NameValidationError::Empty(what));
+        return Err(ErrorKind::Empty(what).into());
     }
 
     if name.starts_with("dep:") {
-        return Err(NameValidationError::FeatureNameStartsWithDepColon(
-            name.into(),
-        ));
+        return Err(ErrorKind::FeatureNameStartsWithDepColon(name.into()).into());
     }
     if name.contains('/') {
-        return Err(NameValidationError::InvalidCharacter {
+        return Err(ErrorKind::InvalidCharacter {
             ch: '/',
             what,
             name: name.into(),
             reason: "feature name is not allowed to contain slashes",
-        });
+        }
+        .into());
     }
     let mut chars = name.chars();
     if let Some(ch) = chars.next() {
         if !(unicode_xid::UnicodeXID::is_xid_start(ch) || ch == '_' || ch.is_digit(10)) {
-            return Err(NameValidationError::InvalidCharacter {
+            return Err(ErrorKind::InvalidCharacter {
                 ch,
                 what,
                 name: name.into(),
                 reason: "the first character must be a Unicode XID start character or digit \
                  (most letters or `_` or `0` to `9`)",
-            });
+            }
+            .into());
         }
     }
     for ch in chars {
         if !(unicode_xid::UnicodeXID::is_xid_continue(ch) || ch == '-' || ch == '+' || ch == '.') {
-            return Err(NameValidationError::InvalidCharacter {
+            return Err(ErrorKind::InvalidCharacter {
                 ch,
                 what,
                 name: name.into(),
                 reason: "characters must be Unicode XID characters, '-', `+`, or `.` \
                  (numbers, `+`, `-`, `_`, `.`, or most letters)",
-            });
+            }
+            .into());
         }
     }
     Ok(())