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

When a serde(untagged) enum can't deser, show all the reasons why #2376

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
38 changes: 27 additions & 11 deletions serde_derive/src/de.rs
Expand Up @@ -1674,28 +1674,44 @@ fn deserialize_untagged_enum(
))
});

// 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()
);
let fallthrough_msg = cattrs.expecting().unwrap_or(&fallthrough_msg);

quote_block! {
// If all variants cannot be deserialized, we will try to write an error
// message explaining why it failed for each one
// The format string we are building will have the following structure:
// "data did not match any variant of untagged enum{}\nvar1: {}\nvar2: {}\nvar3: {}"
let mut err_format_string = fallthrough_msg.to_owned();
let mut num_variants = 0usize;
for var in variants.iter().filter(|variant| !variant.attrs.skip_deserializing()) {
err_format_string.push_str("\n");
err_format_string.push_str(&var.ident.to_string());
err_format_string.push_str(": {}");
num_variants += 1;
}
Comment on lines +1685 to +1694
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not confident this will render well in all the error libraries out there, but we can adjust it once we have seen some of these messages in the wild. The Display/Debug output isn't something we provide back compat guarantees for anyway.


// We need two copies of this iterator
let err_identifiers1 = (0..num_variants).map(|idx| format_ident!("_err{}", idx));
let err_identifiers2 = err_identifiers1.clone();
Comment on lines +1696 to +1698
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While not strictly necessary, as you could probably do the iteration over the attempts directly in the format_args! arguments, this seems better to avoid any question about the order of evaluation.


quote_block! {
let __content = try!(<_serde::__private::de::Content as _serde::Deserialize>::deserialize(__deserializer));

#(
if let _serde::__private::Ok(__ok) = #attempts {
return _serde::__private::Ok(__ok);
}
let #err_identifiers1 = match #attempts {
_serde::__private::Ok(__ok) => {
return _serde::__private::Ok(__ok);
},
_serde::__private::Err(__err) => {
__err
},
};
)*

_serde::__private::Err(_serde::de::Error::custom(#fallthrough_msg))
_serde::__private::Err(_serde::de::Error::custom(format_args!(#err_format_string, #(#err_identifiers2),*)))
}
}

Expand Down
3 changes: 2 additions & 1 deletion test_suite/tests/test_annotations.rs
Expand Up @@ -2776,7 +2776,8 @@ fn test_expecting_message_untagged_tagged_enum() {
Untagged,
}

assert_de_tokens_error::<Enum>(&[Token::Str("Untagged")], r#"something strange..."#);
assert_de_tokens_error::<Enum>(&[Token::Str("Untagged")], r#"something strange...
Untagged: invalid type: string "Untagged", expected unit variant Enum::Untagged"#);
}

#[test]
Expand Down
67 changes: 65 additions & 2 deletions test_suite/tests/test_macros.rs
Expand Up @@ -662,7 +662,13 @@ fn test_untagged_enum() {

assert_de_tokens_error::<Untagged>(
&[Token::Tuple { len: 1 }, Token::U8(1), Token::TupleEnd],
"data did not match any variant of untagged enum Untagged",
r#"data did not match any variant of untagged enum Untagged
A: invalid type: sequence, expected struct variant Untagged::A
B: invalid type: sequence, expected struct variant Untagged::B
C: invalid type: sequence, expected unit variant Untagged::C
D: invalid type: sequence, expected u8
E: invalid type: sequence, expected a string
F: invalid length 1, expected tuple variant Untagged::F with 2 elements"#,
);

assert_de_tokens_error::<Untagged>(
Expand All @@ -673,10 +679,67 @@ fn test_untagged_enum() {
Token::U8(3),
Token::TupleEnd,
],
"data did not match any variant of untagged enum Untagged",
r#"data did not match any variant of untagged enum Untagged
A: invalid type: sequence, expected struct variant Untagged::A
B: invalid type: sequence, expected struct variant Untagged::B
C: invalid type: sequence, expected unit variant Untagged::C
D: invalid type: sequence, expected u8
E: invalid type: sequence, expected a string
F: invalid length 3, expected 2 elements in sequence"#,
);
}

#[test]
fn test_untagged_enum_with_disallow_unknown_fields() {
#[derive(Debug, PartialEq, Serialize, Deserialize)]
#[serde(untagged)]
#[serde(deny_unknown_fields)]
enum Untagged {
A { a: String },
B { b: String, #[serde(default)] c: Vec<String> },
}

assert_de_tokens_error::<Untagged>(
&[
Token::Map { len: Some(1) },
Token::Str("d"),
Token::Str("foo"),
Token::MapEnd
],
r#"data did not match any variant of untagged enum Untagged
A: unknown field `d`, expected `a`
B: unknown field `d`, expected `b` or `c`"#);

assert_de_tokens_error::<Untagged>(
&[
Token::Map { len: Some(2) },
Token::Str("a"),
Token::Str("foo"),
Token::Str("c"),
Token::Seq { len: Some(1) },
Token::Str("bar"),
Token::SeqEnd,
Token::MapEnd
],
r#"data did not match any variant of untagged enum Untagged
A: unknown field `c`, expected `a`
B: unknown field `a`, expected `b` or `c`"#);

assert_de_tokens_error::<Untagged>(
&[
Token::Map { len: Some(1) },
Token::Str("c"),
Token::Seq { len: Some(1) },
Token::Str("bar"),
Token::SeqEnd,
Token::MapEnd
],
r#"data did not match any variant of untagged enum Untagged
A: unknown field `c`, expected `a`
B: missing field `b`"#);

}

#[test]
fn test_internally_tagged_enum() {
#[derive(Debug, PartialEq, Serialize, Deserialize)]
Expand Down