From 3871aecf3e8c2a830997a26179278aef3f90568d Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Sat, 26 Aug 2023 14:47:53 -0700 Subject: [PATCH] Improve deserialization errors of untagged enums --- Cargo.lock | 20 +++++++++ Cargo.toml | 2 + src/cargo/util/config/mod.rs | 79 ++++++++++++++++++++++++++++++----- src/cargo/util/toml/mod.rs | 35 ++++++++++++++-- tests/testsuite/bad_config.rs | 17 ++++++-- tests/testsuite/lints.rs | 2 +- 6 files changed, 136 insertions(+), 19 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 64b97bd0787..f2a8801d108 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -307,6 +307,7 @@ dependencies = [ "same-file", "semver", "serde", + "serde-untagged", "serde-value", "serde_ignored", "serde_json", @@ -836,6 +837,15 @@ version = "1.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5443807d6dff69373d433ab9ef5378ad8df50ca6298caf15de6e52e24aaf54d5" +[[package]] +name = "erased-serde" +version = "0.3.31" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6c138974f9d5e7fe373eb04df7cae98833802ae4b11c24ac7039a21d5af4b26c" +dependencies = [ + "serde", +] + [[package]] name = "errno" version = "0.3.1" @@ -2894,6 +2904,16 @@ dependencies = [ "serde_derive", ] +[[package]] +name = "serde-untagged" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e342997ced06c0793568a54cfc61109a8c5a7b5ecf8fd9de89a5066d82af936b" +dependencies = [ + "erased-serde", + "serde", +] + [[package]] name = "serde-value" version = "0.7.0" diff --git a/Cargo.toml b/Cargo.toml index 9cd54ef89be..5342157eb27 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -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" @@ -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"] } diff --git a/src/cargo/util/config/mod.rs b/src/cargo/util/config/mod.rs index b4280049cc7..1037e47a903 100644 --- a/src/cargo/util/config/mod.rs +++ b/src/cargo/util/config/mod.rs @@ -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; @@ -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(deserializer: D) -> Result + 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, @@ -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(deserializer: D) -> Result + 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 { @@ -2534,13 +2557,24 @@ pub struct BuildTargetConfig { inner: Value, } -#[derive(Debug, Deserialize)] -#[serde(untagged)] +#[derive(Debug)] enum BuildTargetConfigInner { One(String), Many(Vec), } +impl<'de> Deserialize<'de> for BuildTargetConfigInner { + fn deserialize(deserializer: D) -> Result + 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> { @@ -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(deserializer: D) -> Result + 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 { diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 0bc84c7f662..8bdd5c77f94 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -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; @@ -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(deserializer: D) -> Result + 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 { @@ -3513,13 +3526,27 @@ pub type TomlLints = BTreeMap; pub type TomlToolLints = BTreeMap; -#[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(deserializer: D) -> Result + 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 { diff --git a/tests/testsuite/bad_config.rs b/tests/testsuite/bad_config.rs index 93e0468ec5e..82da880ea07 100644 --- a/tests/testsuite/bad_config.rs +++ b/tests/testsuite/bad_config.rs @@ -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(); @@ -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(); @@ -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(); @@ -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(); diff --git a/tests/testsuite/lints.rs b/tests/testsuite/lints.rs index 7a9f1e4d694..17038ce870e 100644 --- a/tests/testsuite/lints.rs +++ b/tests/testsuite/lints.rs @@ -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` ", )