Skip to content

Commit a9c749c

Browse files
committed
Auto merge of #13186 - weihanglo:cargo-util-schemas-error-types, r=epage
refactor: custom error types for `cargo-util-schemas`
2 parents c21be2b + 0b0e78f commit a9c749c

File tree

13 files changed

+243
-135
lines changed

13 files changed

+243
-135
lines changed

Cargo.lock

+2-2
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

+1-1
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ cargo-platform = { path = "crates/cargo-platform", version = "0.1.4" }
3232
cargo-test-macro = { path = "crates/cargo-test-macro" }
3333
cargo-test-support = { path = "crates/cargo-test-support" }
3434
cargo-util = { version = "0.2.6", path = "crates/cargo-util" }
35-
cargo-util-schemas = { version = "0.1.0", path = "crates/cargo-util-schemas" }
35+
cargo-util-schemas = { version = "0.2.0", path = "crates/cargo-util-schemas" }
3636
cargo_metadata = "0.18.1"
3737
clap = "4.4.10"
3838
color-print = "0.3.5"

crates/cargo-util-schemas/Cargo.toml

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[package]
22
name = "cargo-util-schemas"
3-
version = "0.1.1"
3+
version = "0.2.0"
44
rust-version.workspace = true
55
edition.workspace = true
66
license.workspace = true
@@ -9,11 +9,11 @@ repository.workspace = true
99
description = "Deserialization schemas for Cargo"
1010

1111
[dependencies]
12-
anyhow.workspace = true
1312
semver.workspace = true
1413
serde = { workspace = true, features = ["derive"] }
1514
serde-untagged.workspace = true
1615
serde-value.workspace = true
16+
thiserror.workspace = true
1717
toml.workspace = true
1818
unicode-xid.workspace = true
1919
url.workspace = true

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

+2
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ mod partial_version;
33
mod source_kind;
44

55
pub use package_id_spec::PackageIdSpec;
6+
pub use package_id_spec::PackageIdSpecError;
67
pub use partial_version::PartialVersion;
8+
pub use partial_version::PartialVersionError;
79
pub use source_kind::GitReference;
810
pub use source_kind::SourceKind;

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

+63-26
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,17 @@
11
use std::fmt;
22

3-
use anyhow::bail;
4-
use anyhow::Result;
53
use semver::Version;
64
use serde::{de, ser};
75
use url::Url;
86

97
use crate::core::GitReference;
108
use crate::core::PartialVersion;
9+
use crate::core::PartialVersionError;
1110
use crate::core::SourceKind;
1211
use crate::manifest::PackageName;
12+
use crate::restricted_names::NameValidationError;
13+
14+
type Result<T> = std::result::Result<T, PackageIdSpecError>;
1315

1416
/// Some or all of the data required to identify a package:
1517
///
@@ -83,12 +85,11 @@ impl PackageIdSpec {
8385
if abs.exists() {
8486
let maybe_url = Url::from_file_path(abs)
8587
.map_or_else(|_| "a file:// URL".to_string(), |url| url.to_string());
86-
bail!(
87-
"package ID specification `{}` looks like a file path, \
88-
maybe try {}",
89-
spec,
90-
maybe_url
91-
);
88+
return Err(ErrorKind::MaybeFilePath {
89+
spec: spec.into(),
90+
maybe_url,
91+
}
92+
.into());
9293
}
9394
}
9495
let mut parts = spec.splitn(2, [':', '@']);
@@ -119,51 +120,44 @@ impl PackageIdSpec {
119120
}
120121
"registry" => {
121122
if url.query().is_some() {
122-
bail!("cannot have a query string in a pkgid: {url}")
123+
return Err(ErrorKind::UnexpectedQueryString(url).into());
123124
}
124125
kind = Some(SourceKind::Registry);
125126
url = strip_url_protocol(&url);
126127
}
127128
"sparse" => {
128129
if url.query().is_some() {
129-
bail!("cannot have a query string in a pkgid: {url}")
130+
return Err(ErrorKind::UnexpectedQueryString(url).into());
130131
}
131132
kind = Some(SourceKind::SparseRegistry);
132133
// Leave `sparse` as part of URL, see `SourceId::new`
133134
// url = strip_url_protocol(&url);
134135
}
135136
"path" => {
136137
if url.query().is_some() {
137-
bail!("cannot have a query string in a pkgid: {url}")
138+
return Err(ErrorKind::UnexpectedQueryString(url).into());
138139
}
139140
if scheme != "file" {
140-
anyhow::bail!("`path+{scheme}` is unsupported; `path+file` and `file` schemes are supported");
141+
return Err(ErrorKind::UnsupportedPathPlusScheme(scheme.into()).into());
141142
}
142143
kind = Some(SourceKind::Path);
143144
url = strip_url_protocol(&url);
144145
}
145-
kind => anyhow::bail!("unsupported source protocol: {kind}"),
146+
kind => return Err(ErrorKind::UnsupportedProtocol(kind.into()).into()),
146147
}
147148
} else {
148149
if url.query().is_some() {
149-
bail!("cannot have a query string in a pkgid: {url}")
150+
return Err(ErrorKind::UnexpectedQueryString(url).into());
150151
}
151152
}
152153

153154
let frag = url.fragment().map(|s| s.to_owned());
154155
url.set_fragment(None);
155156

156157
let (name, version) = {
157-
let mut path = url
158-
.path_segments()
159-
.ok_or_else(|| anyhow::format_err!("pkgid urls must have a path: {}", url))?;
160-
let path_name = path.next_back().ok_or_else(|| {
161-
anyhow::format_err!(
162-
"pkgid urls must have at least one path \
163-
component: {}",
164-
url
165-
)
166-
})?;
158+
let Some(path_name) = url.path_segments().and_then(|mut p| p.next_back()) else {
159+
return Err(ErrorKind::MissingUrlPath(url).into());
160+
};
167161
match frag {
168162
Some(fragment) => match fragment.split_once([':', '@']) {
169163
Some((name, part)) => {
@@ -259,7 +253,7 @@ impl fmt::Display for PackageIdSpec {
259253
}
260254

261255
impl ser::Serialize for PackageIdSpec {
262-
fn serialize<S>(&self, s: S) -> Result<S::Ok, S::Error>
256+
fn serialize<S>(&self, s: S) -> std::result::Result<S::Ok, S::Error>
263257
where
264258
S: ser::Serializer,
265259
{
@@ -268,7 +262,7 @@ impl ser::Serialize for PackageIdSpec {
268262
}
269263

270264
impl<'de> de::Deserialize<'de> for PackageIdSpec {
271-
fn deserialize<D>(d: D) -> Result<PackageIdSpec, D::Error>
265+
fn deserialize<D>(d: D) -> std::result::Result<PackageIdSpec, D::Error>
272266
where
273267
D: de::Deserializer<'de>,
274268
{
@@ -277,6 +271,49 @@ impl<'de> de::Deserialize<'de> for PackageIdSpec {
277271
}
278272
}
279273

274+
/// Error parsing a [`PackageIdSpec`].
275+
#[derive(Debug, thiserror::Error)]
276+
#[error(transparent)]
277+
pub struct PackageIdSpecError(#[from] ErrorKind);
278+
279+
impl From<PartialVersionError> for PackageIdSpecError {
280+
fn from(value: PartialVersionError) -> Self {
281+
ErrorKind::PartialVersion(value).into()
282+
}
283+
}
284+
285+
impl From<NameValidationError> for PackageIdSpecError {
286+
fn from(value: NameValidationError) -> Self {
287+
ErrorKind::NameValidation(value).into()
288+
}
289+
}
290+
291+
/// Non-public error kind for [`PackageIdSpecError`].
292+
#[non_exhaustive]
293+
#[derive(Debug, thiserror::Error)]
294+
enum ErrorKind {
295+
#[error("unsupported source protocol: {0}")]
296+
UnsupportedProtocol(String),
297+
298+
#[error("`path+{0}` is unsupported; `path+file` and `file` schemes are supported")]
299+
UnsupportedPathPlusScheme(String),
300+
301+
#[error("cannot have a query string in a pkgid: {0}")]
302+
UnexpectedQueryString(Url),
303+
304+
#[error("pkgid urls must have at least one path component: {0}")]
305+
MissingUrlPath(Url),
306+
307+
#[error("package ID specification `{spec}` looks like a file path, maybe try {maybe_url}")]
308+
MaybeFilePath { spec: String, maybe_url: String },
309+
310+
#[error(transparent)]
311+
NameValidation(#[from] crate::restricted_names::NameValidationError),
312+
313+
#[error(transparent)]
314+
PartialVersion(#[from] crate::core::PartialVersionError),
315+
}
316+
280317
#[cfg(test)]
281318
mod tests {
282319
use super::PackageIdSpec;

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

+27-11
Original file line numberDiff line numberDiff line change
@@ -80,27 +80,21 @@ impl From<semver::Version> for PartialVersion {
8080
}
8181

8282
impl std::str::FromStr for PartialVersion {
83-
type Err = anyhow::Error;
83+
type Err = PartialVersionError;
8484

8585
fn from_str(value: &str) -> Result<Self, Self::Err> {
8686
if is_req(value) {
87-
anyhow::bail!("unexpected version requirement, expected a version like \"1.32\"")
87+
return Err(ErrorKind::VersionReq.into());
8888
}
8989
match semver::Version::parse(value) {
9090
Ok(ver) => Ok(ver.into()),
9191
Err(_) => {
9292
// HACK: Leverage `VersionReq` for partial version parsing
9393
let mut version_req = match semver::VersionReq::parse(value) {
9494
Ok(req) => req,
95-
Err(_) if value.contains('-') => {
96-
anyhow::bail!(
97-
"unexpected prerelease field, expected a version like \"1.32\""
98-
)
99-
}
100-
Err(_) if value.contains('+') => {
101-
anyhow::bail!("unexpected build field, expected a version like \"1.32\"")
102-
}
103-
Err(_) => anyhow::bail!("expected a version like \"1.32\""),
95+
Err(_) if value.contains('-') => return Err(ErrorKind::Prerelease.into()),
96+
Err(_) if value.contains('+') => return Err(ErrorKind::BuildMetadata.into()),
97+
Err(_) => return Err(ErrorKind::Unexpected.into()),
10498
};
10599
assert_eq!(version_req.comparators.len(), 1, "guaranteed by is_req");
106100
let comp = version_req.comparators.pop().unwrap();
@@ -163,6 +157,28 @@ impl<'de> serde::Deserialize<'de> for PartialVersion {
163157
}
164158
}
165159

160+
/// Error parsing a [`PartialVersion`].
161+
#[derive(Debug, thiserror::Error)]
162+
#[error(transparent)]
163+
pub struct PartialVersionError(#[from] ErrorKind);
164+
165+
/// Non-public error kind for [`PartialVersionError`].
166+
#[non_exhaustive]
167+
#[derive(Debug, thiserror::Error)]
168+
enum ErrorKind {
169+
#[error("unexpected version requirement, expected a version like \"1.32\"")]
170+
VersionReq,
171+
172+
#[error("unexpected prerelease field, expected a version like \"1.32\"")]
173+
Prerelease,
174+
175+
#[error("unexpected build field, expected a version like \"1.32\"")]
176+
BuildMetadata,
177+
178+
#[error("expected a version like \"1.32\"")]
179+
Unexpected,
180+
}
181+
166182
fn is_req(value: &str) -> bool {
167183
let Some(first) = value.chars().next() else {
168184
return false;

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

+34-12
Original file line numberDiff line numberDiff line change
@@ -10,16 +10,18 @@ use std::fmt::{self, Display, Write};
1010
use std::path::PathBuf;
1111
use std::str;
1212

13-
use anyhow::Result;
1413
use serde::de::{self, IntoDeserializer as _, Unexpected};
1514
use serde::ser;
1615
use serde::{Deserialize, Serialize};
1716
use serde_untagged::UntaggedEnumVisitor;
1817

1918
use crate::core::PackageIdSpec;
2019
use crate::core::PartialVersion;
20+
use crate::core::PartialVersionError;
2121
use crate::restricted_names;
2222

23+
pub use crate::restricted_names::NameValidationError;
24+
2325
/// This type is used to deserialize `Cargo.toml` files.
2426
#[derive(Debug, Deserialize, Serialize)]
2527
#[serde(rename_all = "kebab-case")]
@@ -1144,7 +1146,7 @@ macro_rules! str_newtype {
11441146
}
11451147

11461148
impl<'a> std::str::FromStr for $name<String> {
1147-
type Err = anyhow::Error;
1149+
type Err = restricted_names::NameValidationError;
11481150

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

11741176
impl<T: AsRef<str>> PackageName<T> {
11751177
/// Validated package name
1176-
pub fn new(name: T) -> Result<Self> {
1177-
restricted_names::validate_package_name(name.as_ref(), "package name", "")?;
1178+
pub fn new(name: T) -> Result<Self, NameValidationError> {
1179+
restricted_names::validate_package_name(name.as_ref(), "package name")?;
11781180
Ok(Self(name))
11791181
}
11801182
}
@@ -1195,8 +1197,8 @@ str_newtype!(RegistryName);
11951197

11961198
impl<T: AsRef<str>> RegistryName<T> {
11971199
/// Validated registry name
1198-
pub fn new(name: T) -> Result<Self> {
1199-
restricted_names::validate_package_name(name.as_ref(), "registry name", "")?;
1200+
pub fn new(name: T) -> Result<Self, NameValidationError> {
1201+
restricted_names::validate_package_name(name.as_ref(), "registry name")?;
12001202
Ok(Self(name))
12011203
}
12021204
}
@@ -1205,7 +1207,7 @@ str_newtype!(ProfileName);
12051207

12061208
impl<T: AsRef<str>> ProfileName<T> {
12071209
/// Validated profile name
1208-
pub fn new(name: T) -> Result<Self> {
1210+
pub fn new(name: T) -> Result<Self, NameValidationError> {
12091211
restricted_names::validate_profile_name(name.as_ref())?;
12101212
Ok(Self(name))
12111213
}
@@ -1215,7 +1217,7 @@ str_newtype!(FeatureName);
12151217

12161218
impl<T: AsRef<str>> FeatureName<T> {
12171219
/// Validated feature name
1218-
pub fn new(name: T) -> Result<Self> {
1220+
pub fn new(name: T) -> Result<Self, NameValidationError> {
12191221
restricted_names::validate_feature_name(name.as_ref())?;
12201222
Ok(Self(name))
12211223
}
@@ -1334,15 +1336,16 @@ impl std::ops::Deref for RustVersion {
13341336
}
13351337

13361338
impl std::str::FromStr for RustVersion {
1337-
type Err = anyhow::Error;
1339+
type Err = RustVersionError;
13381340

13391341
fn from_str(value: &str) -> Result<Self, Self::Err> {
1340-
let partial = value.parse::<PartialVersion>()?;
1342+
let partial = value.parse::<PartialVersion>();
1343+
let partial = partial.map_err(RustVersionErrorKind::PartialVersion)?;
13411344
if partial.pre.is_some() {
1342-
anyhow::bail!("unexpected prerelease field, expected a version like \"1.32\"")
1345+
return Err(RustVersionErrorKind::Prerelease.into());
13431346
}
13441347
if partial.build.is_some() {
1345-
anyhow::bail!("unexpected prerelease field, expected a version like \"1.32\"")
1348+
return Err(RustVersionErrorKind::BuildMetadata.into());
13461349
}
13471350
Ok(Self(partial))
13481351
}
@@ -1366,6 +1369,25 @@ impl Display for RustVersion {
13661369
}
13671370
}
13681371

1372+
/// Error parsing a [`RustVersion`].
1373+
#[derive(Debug, thiserror::Error)]
1374+
#[error(transparent)]
1375+
pub struct RustVersionError(#[from] RustVersionErrorKind);
1376+
1377+
/// Non-public error kind for [`RustVersionError`].
1378+
#[non_exhaustive]
1379+
#[derive(Debug, thiserror::Error)]
1380+
enum RustVersionErrorKind {
1381+
#[error("unexpected prerelease field, expected a version like \"1.32\"")]
1382+
Prerelease,
1383+
1384+
#[error("unexpected build field, expected a version like \"1.32\"")]
1385+
BuildMetadata,
1386+
1387+
#[error(transparent)]
1388+
PartialVersion(#[from] PartialVersionError),
1389+
}
1390+
13691391
#[derive(Copy, Clone, Debug)]
13701392
pub struct InvalidCargoFeatures {}
13711393

0 commit comments

Comments
 (0)