Skip to content

Commit

Permalink
Collect errors when deserializing untagged enums
Browse files Browse the repository at this point in the history
This also adds a verbose-debug feature that enables the new behavior.

Previously:

> data did not match any variant of untagged enum ReferenceOr at line 6 column 4

Now, with `verbose-debug` feature enabled:

> data did not match any variant of untagged enum `ReferenceOr`
> 	- attempted to deserialize `Reference` but failed with: missing field `$ref`
> 	- attempted to deserialize `Item` but failed with: invalid value: string "lol", expected expected format `\dXX` at line 6 column 4

cf. serde-rs#773
  • Loading branch information
killercup committed Sep 25, 2020
1 parent b539cb4 commit 59650e1
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 20 deletions.
3 changes: 3 additions & 0 deletions serde/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -53,3 +53,6 @@ alloc = []
# does not preserve identity and may result in multiple copies of the same data.
# Be sure that this is what you want before enabling this feature.
rc = []

# Opt into verbose errors that are more expensive to compute.
verbose-debug = ["serde_derive/verbose-debug"]
1 change: 1 addition & 0 deletions serde_derive/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ include = ["build.rs", "src/**/*.rs", "crates-io.md", "README.md", "LICENSE-APAC
[features]
default = []
deserialize_in_place = []
verbose-debug = []

[lib]
name = "serde_derive"
Expand Down
74 changes: 54 additions & 20 deletions serde_derive/src/de.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1630,35 +1630,69 @@ fn deserialize_untagged_enum(
.iter()
.filter(|variant| !variant.attrs.skip_deserializing())
.map(|variant| {
Expr(deserialize_untagged_variant(
let de_any = Expr(deserialize_untagged_variant(
params,
variant,
cattrs,
quote!(_serde::private::de::ContentRefDeserializer::<__D::Error>::new(&__content)),
))
));

if cfg!(feature = "verbose-debug") {
let variant_name = variant.ident.to_string();
quote! {
_serde::export::Result::map_err(
#de_any,
|e| -> __D::Error {
_serde::de::Error::custom(
format!("attempted to deserialize `{}` but failed with: {}", #variant_name, e.to_string())
)
},
)
}
} else {
quote! { #de_any }
}
});

// TODO this message could be better by saving the errors from the failed
// attempts. The heuristic used by TOML was to count the number of fields
// processed before an error, and use the error that happened after the
// largest number of fields. I'm not sure I like that. Maybe it would be
// better to save all the errors and combine them into one message that
// explains why none of the variants matched.
let fallthrough_msg = format!(
"data did not match any variant of untagged enum {}",
params.type_name()
);
if cfg!(feature = "verbose-debug") {
let type_name = params.type_name();

quote_block! {
let __content = try!(<_serde::private::de::Content as _serde::Deserialize>::deserialize(__deserializer));
quote_block! {
let __content = try!(<_serde::private::de::Content as _serde::Deserialize>::deserialize(__deserializer));
let mut fallthrough_msg = format!(
"data did not match any variant of untagged enum `{}`",
#type_name
);

#(
if let _serde::export::Ok(__ok) = #attempts {
return _serde::export::Ok(__ok);
}
)*
#(
match #attempts {
_serde::export::Ok(__ok) => return _serde::export::Ok(__ok),
_serde::export::Err(__err) => {
fallthrough_msg.push_str("\n\t- ");
fallthrough_msg.push_str(&__err.to_string());
}
}
)*

_serde::export::Err(_serde::de::Error::custom(fallthrough_msg))
}
} else {
let fallthrough_msg = format!(
"data did not match any variant of untagged enum {}",
params.type_name()
);

_serde::export::Err(_serde::de::Error::custom(#fallthrough_msg))
quote_block! {
let __content = try!(<_serde::private::de::Content as _serde::Deserialize>::deserialize(__deserializer));

#(
if let _serde::export::Ok(__ok) = #attempts {
return _serde::export::Ok(__ok);
}
)*

_serde::export::Err(_serde::de::Error::custom(#fallthrough_msg))
}
}
}

Expand Down

0 comments on commit 59650e1

Please sign in to comment.