Skip to content

Commit

Permalink
Improve deserialization errors of untagged enums
Browse files Browse the repository at this point in the history
  • Loading branch information
dtolnay committed Aug 27, 2023
1 parent 58d0425 commit 3871aec
Show file tree
Hide file tree
Showing 6 changed files with 136 additions and 19 deletions.
20 changes: 20 additions & 0 deletions Cargo.lock

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

2 changes: 2 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ same-file = "1.0.6"
security-framework = "2.9.2"
semver = { version = "1.0.18", features = ["serde"] }
serde = "1.0.188"
serde-untagged = "0.1.0"
serde-value = "0.7.0"
serde_ignored = "0.1.9"
serde_json = "1.0.104"
Expand Down Expand Up @@ -163,6 +164,7 @@ rand.workspace = true
rustfix.workspace = true
semver.workspace = true
serde = { workspace = true, features = ["derive"] }
serde-untagged.workspace = true
serde-value.workspace = true
serde_ignored.workspace = true
serde_json = { workspace = true, features = ["raw_value"] }
Expand Down
79 changes: 69 additions & 10 deletions src/cargo/util/config/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ use curl::easy::Easy;
use lazycell::LazyCell;
use serde::de::IntoDeserializer as _;
use serde::Deserialize;
use serde_untagged::UntaggedEnumVisitor;
use time::OffsetDateTime;
use toml_edit::Item;
use url::Url;
Expand Down Expand Up @@ -2453,13 +2454,24 @@ impl CargoFutureIncompatConfig {
/// ssl-version.min = "tlsv1.2"
/// ssl-version.max = "tlsv1.3"
/// ```
#[derive(Clone, Debug, Deserialize, PartialEq)]
#[serde(untagged)]
#[derive(Clone, Debug, PartialEq)]
pub enum SslVersionConfig {
Single(String),
Range(SslVersionConfigRange),
}

impl<'de> Deserialize<'de> for SslVersionConfig {
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
where
D: serde::Deserializer<'de>,
{
UntaggedEnumVisitor::new()
.string(|single| Ok(SslVersionConfig::Single(single.to_owned())))
.map(|map| map.deserialize().map(SslVersionConfig::Range))
.deserialize(deserializer)
}
}

#[derive(Clone, Debug, Deserialize, PartialEq)]
pub struct SslVersionConfigRange {
pub min: Option<String>,
Expand Down Expand Up @@ -2493,13 +2505,24 @@ pub struct CargoSshConfig {
/// [build]
/// jobs = "default" # Currently only support "default".
/// ```
#[derive(Debug, Deserialize, Clone)]
#[serde(untagged)]
#[derive(Debug, Clone)]
pub enum JobsConfig {
Integer(i32),
String(String),
}

impl<'de> Deserialize<'de> for JobsConfig {
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
where
D: serde::Deserializer<'de>,
{
UntaggedEnumVisitor::new()
.i32(|int| Ok(JobsConfig::Integer(int)))
.string(|string| Ok(JobsConfig::String(string.to_owned())))
.deserialize(deserializer)
}
}

#[derive(Debug, Deserialize)]
#[serde(rename_all = "kebab-case")]
pub struct CargoBuildConfig {
Expand Down Expand Up @@ -2534,13 +2557,24 @@ pub struct BuildTargetConfig {
inner: Value<BuildTargetConfigInner>,
}

#[derive(Debug, Deserialize)]
#[serde(untagged)]
#[derive(Debug)]
enum BuildTargetConfigInner {
One(String),
Many(Vec<String>),
}

impl<'de> Deserialize<'de> for BuildTargetConfigInner {
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
where
D: serde::Deserializer<'de>,
{
UntaggedEnumVisitor::new()
.string(|one| Ok(BuildTargetConfigInner::One(one.to_owned())))
.seq(|many| many.deserialize().map(BuildTargetConfigInner::Many))
.deserialize(deserializer)
}
}

impl BuildTargetConfig {
/// Gets values of `build.target` as a list of strings.
pub fn values(&self, config: &Config) -> CargoResult<Vec<String>> {
Expand Down Expand Up @@ -2652,19 +2686,44 @@ where
deserializer.deserialize_option(ProgressVisitor)
}

#[derive(Debug, Deserialize)]
#[serde(untagged)]
#[derive(Debug)]
enum EnvConfigValueInner {
Simple(String),
WithOptions {
value: String,
#[serde(default)]
force: bool,
#[serde(default)]
relative: bool,
},
}

impl<'de> Deserialize<'de> for EnvConfigValueInner {
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
where
D: serde::Deserializer<'de>,
{
#[derive(Deserialize)]
struct WithOptions {
value: String,
#[serde(default)]
force: bool,
#[serde(default)]
relative: bool,
}

UntaggedEnumVisitor::new()
.string(|simple| Ok(EnvConfigValueInner::Simple(simple.to_owned())))
.map(|map| {
let with_options: WithOptions = map.deserialize()?;
Ok(EnvConfigValueInner::WithOptions {
value: with_options.value,
force: with_options.force,
relative: with_options.relative,
})
})
.deserialize(deserializer)
}
}

#[derive(Debug, Deserialize)]
#[serde(transparent)]
pub struct EnvConfigValue {
Expand Down
35 changes: 31 additions & 4 deletions src/cargo/util/toml/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,10 @@ use cargo_util::paths;
use itertools::Itertools;
use lazycell::LazyCell;
use semver::{self, VersionReq};
use serde::de::{self, Unexpected};
use serde::de::{self, IntoDeserializer as _, Unexpected};
use serde::ser;
use serde::{Deserialize, Serialize};
use serde_untagged::UntaggedEnumVisitor;
use tracing::{debug, trace};
use url::Url;

Expand Down Expand Up @@ -961,13 +962,25 @@ impl StringOrVec {
}
}

#[derive(Clone, Debug, Deserialize, Serialize, Eq, PartialEq)]
#[serde(untagged, expecting = "expected a boolean or a string")]
#[derive(Clone, Debug, Serialize, Eq, PartialEq)]
#[serde(untagged)]
pub enum StringOrBool {
String(String),
Bool(bool),
}

impl<'de> Deserialize<'de> for StringOrBool {
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
where
D: de::Deserializer<'de>,
{
UntaggedEnumVisitor::new()
.string(|s| Ok(StringOrBool::String(s.to_owned())))
.bool(|b| Ok(StringOrBool::Bool(b)))
.deserialize(deserializer)
}
}

#[derive(PartialEq, Clone, Debug, Serialize)]
#[serde(untagged)]
pub enum VecStringOrBool {
Expand Down Expand Up @@ -3513,13 +3526,27 @@ pub type TomlLints = BTreeMap<String, TomlToolLints>;

pub type TomlToolLints = BTreeMap<String, TomlLint>;

#[derive(Serialize, Deserialize, Debug, Clone)]
#[derive(Serialize, Debug, Clone)]
#[serde(untagged)]
pub enum TomlLint {
Level(TomlLintLevel),
Config(TomlLintConfig),
}

impl<'de> Deserialize<'de> for TomlLint {
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
where
D: de::Deserializer<'de>,
{
UntaggedEnumVisitor::new()
.string(|string| {
TomlLintLevel::deserialize(string.into_deserializer()).map(TomlLint::Level)
})
.map(|map| map.deserialize().map(TomlLint::Config))
.deserialize(deserializer)
}
}

impl TomlLint {
fn level(&self) -> TomlLintLevel {
match self {
Expand Down
17 changes: 13 additions & 4 deletions tests/testsuite/bad_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1392,7 +1392,7 @@ Caused by:
|
6 | build = 3
| ^
expected a boolean or a string
invalid type: integer `3`, expected a boolean or string
",
)
.run();
Expand Down Expand Up @@ -1465,7 +1465,10 @@ fn bad_http_ssl_version_range() {
.with_status(101)
.with_stderr(
"\
[ERROR] data did not match any variant of untagged enum SslVersionConfig
[ERROR] error in [..]/config.toml: could not load config key `http.ssl-version`
Caused by:
error in [..]/config.toml: `http.ssl-version.min` expected a string, but found a boolean
",
)
.run();
Expand All @@ -1489,7 +1492,10 @@ fn bad_build_jobs() {
.with_status(101)
.with_stderr(
"\
[ERROR] data did not match any variant of untagged enum JobsConfig
[ERROR] error in [..]/config.toml: could not load config key `build.jobs`
Caused by:
invalid type: map, expected an integer or string
",
)
.run();
Expand All @@ -1516,7 +1522,10 @@ fn bad_build_target() {
[ERROR] error in [..]/config.toml: could not load config key `build.target`
Caused by:
data did not match any variant of untagged enum BuildTargetConfigInner
error in [..]/config.toml: could not load config key `build.target`
Caused by:
invalid type: map, expected a string or array
",
)
.run();
Expand Down
2 changes: 1 addition & 1 deletion tests/testsuite/lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ fn invalid_type_in_lint_value() {
error: failed to parse manifest at `[..]/Cargo.toml`
Caused by:
data did not match any variant of untagged enum TomlLint
invalid type: integer `-1`, expected a string or map
in `rust.rust-2018-idioms`
",
)
Expand Down

0 comments on commit 3871aec

Please sign in to comment.