Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve deserialization errors of untagged enums #12574

Merged
merged 2 commits into from
Aug 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
113 changes: 112 additions & 1 deletion 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 @@ -1420,6 +1420,117 @@ fn warn_semver_metadata() {
.run();
}

#[cargo_test]
fn bad_http_ssl_version() {
// Invalid type in SslVersionConfig.
let p = project()
.file(
".cargo/config.toml",
r#"
[http]
ssl-version = ["tlsv1.2", "tlsv1.3"]
"#,
)
.file("src/lib.rs", "")
.build();

p.cargo("check")
.with_status(101)
.with_stderr(
"\
[ERROR] error in [..]/config.toml: could not load config key `http.ssl-version`

Caused by:
invalid type: sequence, expected a string or map
",
)
.run();
}

#[cargo_test]
fn bad_http_ssl_version_range() {
// Invalid type in SslVersionConfigRange.
let p = project()
.file(
".cargo/config.toml",
r#"
[http]
ssl-version.min = false
"#,
)
.file("src/lib.rs", "")
.build();

p.cargo("check")
.with_status(101)
.with_stderr(
"\
[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();
}

#[cargo_test]
fn bad_build_jobs() {
// Invalid type in JobsConfig.
let p = project()
.file(
".cargo/config.toml",
r#"
[build]
jobs = { default = true }
"#,
)
.file("src/lib.rs", "")
.build();

p.cargo("check")
.with_status(101)
.with_stderr(
"\
[ERROR] error in [..]/config.toml: could not load config key `build.jobs`

Caused by:
invalid type: map, expected an integer or string
",
)
.run();
}

#[cargo_test]
fn bad_build_target() {
// Invalid type in BuildTargetConfig.
let p = project()
.file(
".cargo/config.toml",
r#"
[build]
target.'cfg(unix)' = "x86_64"
"#,
)
.file("src/lib.rs", "")
.build();

p.cargo("check")
.with_status(101)
.with_stderr(
"\
[ERROR] error in [..]/config.toml: could not load config key `build.target`

Caused by:
error in [..]/config.toml: could not load config key `build.target`

Caused by:
invalid type: map, expected a string or array
",
)
.run();
}

#[cargo_test]
fn bad_target_cfg() {
// Invalid type in a StringList.
Expand Down
Loading