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

"cannot infer type" from Deserialize derive macro with simple variants and untagged variants #2613

Merged
merged 4 commits into from Oct 13, 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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 13 additions & 1 deletion serde_derive/src/de.rs
Expand Up @@ -1737,7 +1737,6 @@ fn deserialize_untagged_enum_after(
quote!(__deserializer),
))
});
let attempts = first_attempt.into_iter().chain(attempts);
// 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
Expand All @@ -1750,10 +1749,23 @@ fn deserialize_untagged_enum_after(
);
let fallthrough_msg = cattrs.expecting().unwrap_or(&fallthrough_msg);

// Ignore any error associated with non-untagged deserialization so that we
// can fall through to the untagged variants. This may be infallible so we
// need to provide the error type.
let first_attempt = first_attempt.map(|expr| {
quote! {
if let _serde::__private::Result::<_, __D::Error>::Ok(__ok) = (|| {
#expr
})() {
return _serde::__private::Ok(__ok);
}
}
});
quote_block! {
let __content = <_serde::__private::de::Content as _serde::Deserialize>::deserialize(__deserializer)?;
let __deserializer = _serde::__private::de::ContentRefDeserializer::<__D::Error>::new(&__content);

#first_attempt
#(
if let _serde::__private::Ok(__ok) = #attempts {
Copy link
Member

Choose a reason for hiding this comment

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

if the type annotation is added here, we can keep sharing the logic between first_attempt and attempts

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed! As I noted, I was trying to minimize code change for code paths unrelated to the new untagged variant semantics, but I'm very happy to add the type annotation in all cases if that's preferable. Please let me know.

return _serde::__private::Ok(__ok);
Expand Down
62 changes: 62 additions & 0 deletions test_suite/tests/test_annotations.rs
Expand Up @@ -2380,6 +2380,68 @@ fn test_partially_untagged_enum_desugared() {
);
}

#[test]
fn test_partially_untagged_internally_tagged_enum() {
#[derive(Serialize, Deserialize, PartialEq, Debug)]
#[serde(tag = "t")]
enum Data {
A,
B,
#[serde(untagged)]
Var(u32),
}

let data = Data::A;

assert_de_tokens(
&data,
&[
Token::Map { len: None },
Token::Str("t"),
Token::Str("A"),
Token::MapEnd,
],
);

let data = Data::Var(42);

assert_de_tokens(&data, &[Token::U32(42)]);

// TODO test error output
}

#[test]
fn test_partially_untagged_adjacently_tagged_enum() {
#[derive(Serialize, Deserialize, PartialEq, Debug)]
#[serde(tag = "t", content = "c")]
enum Data {
A(u32),
B,
#[serde(untagged)]
Var(u32),
}

let data = Data::A(7);

assert_de_tokens(
&data,
&[
Token::Map { len: None },
Token::Str("t"),
Token::Str("A"),
Token::Str("c"),
Token::U32(7),
Token::MapEnd,
],
);

let data = Data::Var(42);

assert_de_tokens(&data, &[Token::U32(42)]);

// TODO test error output
}

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