Skip to content

Commit

Permalink
Auto merge of #13186 - weihanglo:cargo-util-schemas-error-types, r=epage
Browse files Browse the repository at this point in the history
refactor: custom error types for `cargo-util-schemas`
  • Loading branch information
bors committed Dec 20, 2023
2 parents c21be2b + 0b0e78f commit a9c749c
Show file tree
Hide file tree
Showing 13 changed files with 243 additions and 135 deletions.
4 changes: 2 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ cargo-platform = { path = "crates/cargo-platform", version = "0.1.4" }
cargo-test-macro = { path = "crates/cargo-test-macro" }
cargo-test-support = { path = "crates/cargo-test-support" }
cargo-util = { version = "0.2.6", path = "crates/cargo-util" }
cargo-util-schemas = { version = "0.1.0", path = "crates/cargo-util-schemas" }
cargo-util-schemas = { version = "0.2.0", path = "crates/cargo-util-schemas" }
cargo_metadata = "0.18.1"
clap = "4.4.10"
color-print = "0.3.5"
Expand Down
4 changes: 2 additions & 2 deletions crates/cargo-util-schemas/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "cargo-util-schemas"
version = "0.1.1"
version = "0.2.0"
rust-version.workspace = true
edition.workspace = true
license.workspace = true
Expand All @@ -9,11 +9,11 @@ repository.workspace = true
description = "Deserialization schemas for Cargo"

[dependencies]
anyhow.workspace = true
semver.workspace = true
serde = { workspace = true, features = ["derive"] }
serde-untagged.workspace = true
serde-value.workspace = true
thiserror.workspace = true
toml.workspace = true
unicode-xid.workspace = true
url.workspace = true
Expand Down
2 changes: 2 additions & 0 deletions crates/cargo-util-schemas/src/core/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ 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;
pub use source_kind::SourceKind;
89 changes: 63 additions & 26 deletions crates/cargo-util-schemas/src/core/package_id_spec.rs
Original file line number Diff line number Diff line change
@@ -1,15 +1,17 @@
use std::fmt;

use anyhow::bail;
use anyhow::Result;
use semver::Version;
use serde::{de, ser};
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>;

/// Some or all of the data required to identify a package:
///
Expand Down Expand Up @@ -83,12 +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());
bail!(
"package ID specification `{}` looks like a file path, \
maybe try {}",
spec,
maybe_url
);
return Err(ErrorKind::MaybeFilePath {
spec: spec.into(),
maybe_url,
}
.into());
}
}
let mut parts = spec.splitn(2, [':', '@']);
Expand Down Expand Up @@ -119,51 +120,44 @@ impl PackageIdSpec {
}
"registry" => {
if url.query().is_some() {
bail!("cannot have a query string in a pkgid: {url}")
return Err(ErrorKind::UnexpectedQueryString(url).into());
}
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(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() {
bail!("cannot have a query string in a pkgid: {url}")
return Err(ErrorKind::UnexpectedQueryString(url).into());
}
if scheme != "file" {
anyhow::bail!("`path+{scheme}` is unsupported; `path+file` and `file` schemes are supported");
return Err(ErrorKind::UnsupportedPathPlusScheme(scheme.into()).into());
}
kind = Some(SourceKind::Path);
url = strip_url_protocol(&url);
}
kind => anyhow::bail!("unsupported source protocol: {kind}"),
kind => return Err(ErrorKind::UnsupportedProtocol(kind.into()).into()),
}
} else {
if url.query().is_some() {
bail!("cannot have a query string in a pkgid: {url}")
return Err(ErrorKind::UnexpectedQueryString(url).into());
}
}

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(ErrorKind::MissingUrlPath(url).into());
};
match frag {
Some(fragment) => match fragment.split_once([':', '@']) {
Some((name, part)) => {
Expand Down Expand Up @@ -259,7 +253,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,
{
Expand All @@ -268,7 +262,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>,
{
Expand All @@ -277,6 +271,49 @@ 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)]
enum ErrorKind {
#[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;
Expand Down
38 changes: 27 additions & 11 deletions crates/cargo-util-schemas/src/core/partial_version.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,27 +80,21 @@ impl From<semver::Version> for PartialVersion {
}

impl std::str::FromStr for PartialVersion {
type Err = anyhow::Error;
type Err = PartialVersionError;

fn from_str(value: &str) -> Result<Self, Self::Err> {
if is_req(value) {
anyhow::bail!("unexpected version requirement, expected a version like \"1.32\"")
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('-') => {
anyhow::bail!(
"unexpected prerelease field, expected a version like \"1.32\""
)
}
Err(_) if value.contains('+') => {
anyhow::bail!("unexpected build field, expected a version like \"1.32\"")
}
Err(_) => anyhow::bail!("expected a version like \"1.32\""),
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();
Expand Down Expand Up @@ -163,6 +157,28 @@ 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)]
enum ErrorKind {
#[error("unexpected version requirement, expected a version like \"1.32\"")]
VersionReq,

#[error("unexpected prerelease field, expected a version like \"1.32\"")]
Prerelease,

#[error("unexpected build field, expected a version like \"1.32\"")]
BuildMetadata,

#[error("expected a version like \"1.32\"")]
Unexpected,
}

fn is_req(value: &str) -> bool {
let Some(first) = value.chars().next() else {
return false;
Expand Down
46 changes: 34 additions & 12 deletions crates/cargo-util-schemas/src/manifest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,18 @@ use std::fmt::{self, Display, Write};
use std::path::PathBuf;
use std::str;

use anyhow::Result;
use serde::de::{self, IntoDeserializer as _, Unexpected};
use serde::ser;
use serde::{Deserialize, Serialize};
use serde_untagged::UntaggedEnumVisitor;

use crate::core::PackageIdSpec;
use crate::core::PartialVersion;
use crate::core::PartialVersionError;
use crate::restricted_names;

pub use crate::restricted_names::NameValidationError;

/// This type is used to deserialize `Cargo.toml` files.
#[derive(Debug, Deserialize, Serialize)]
#[serde(rename_all = "kebab-case")]
Expand Down Expand Up @@ -1144,7 +1146,7 @@ macro_rules! str_newtype {
}

impl<'a> std::str::FromStr for $name<String> {
type Err = anyhow::Error;
type Err = restricted_names::NameValidationError;

fn from_str(value: &str) -> Result<Self, Self::Err> {
Self::new(value.to_owned())
Expand Down Expand Up @@ -1173,8 +1175,8 @@ str_newtype!(PackageName);

impl<T: AsRef<str>> PackageName<T> {
/// Validated package name
pub fn new(name: T) -> Result<Self> {
restricted_names::validate_package_name(name.as_ref(), "package name", "")?;
pub fn new(name: T) -> Result<Self, NameValidationError> {
restricted_names::validate_package_name(name.as_ref(), "package name")?;
Ok(Self(name))
}
}
Expand All @@ -1195,8 +1197,8 @@ str_newtype!(RegistryName);

impl<T: AsRef<str>> RegistryName<T> {
/// Validated registry name
pub fn new(name: T) -> Result<Self> {
restricted_names::validate_package_name(name.as_ref(), "registry name", "")?;
pub fn new(name: T) -> Result<Self, NameValidationError> {
restricted_names::validate_package_name(name.as_ref(), "registry name")?;
Ok(Self(name))
}
}
Expand All @@ -1205,7 +1207,7 @@ str_newtype!(ProfileName);

impl<T: AsRef<str>> ProfileName<T> {
/// Validated profile name
pub fn new(name: T) -> Result<Self> {
pub fn new(name: T) -> Result<Self, NameValidationError> {
restricted_names::validate_profile_name(name.as_ref())?;
Ok(Self(name))
}
Expand All @@ -1215,7 +1217,7 @@ str_newtype!(FeatureName);

impl<T: AsRef<str>> FeatureName<T> {
/// Validated feature name
pub fn new(name: T) -> Result<Self> {
pub fn new(name: T) -> Result<Self, NameValidationError> {
restricted_names::validate_feature_name(name.as_ref())?;
Ok(Self(name))
}
Expand Down Expand Up @@ -1334,15 +1336,16 @@ impl std::ops::Deref for RustVersion {
}

impl std::str::FromStr for RustVersion {
type Err = anyhow::Error;
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() {
anyhow::bail!("unexpected prerelease field, expected a version like \"1.32\"")
return Err(RustVersionErrorKind::Prerelease.into());
}
if partial.build.is_some() {
anyhow::bail!("unexpected prerelease field, expected a version like \"1.32\"")
return Err(RustVersionErrorKind::BuildMetadata.into());
}
Ok(Self(partial))
}
Expand All @@ -1366,6 +1369,25 @@ impl Display for RustVersion {
}
}

/// 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)]
enum RustVersionErrorKind {
#[error("unexpected prerelease field, expected a version like \"1.32\"")]
Prerelease,

#[error("unexpected build field, expected a version like \"1.32\"")]
BuildMetadata,

#[error(transparent)]
PartialVersion(#[from] PartialVersionError),
}

#[derive(Copy, Clone, Debug)]
pub struct InvalidCargoFeatures {}

Expand Down
Loading

0 comments on commit a9c749c

Please sign in to comment.