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

Allowed Enum variants to be individually marked as untagged #2403

Merged
merged 8 commits into from
Jun 8, 2023
41 changes: 37 additions & 4 deletions serde_derive/src/de.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1165,6 +1165,29 @@ fn deserialize_enum(
params: &Parameters,
variants: &[Variant],
cattrs: &attr::Container,
) -> Fragment {
match variants
.iter()
.enumerate()
.find(|(_, var)| var.attrs.untagged())
dewert99 marked this conversation as resolved.
Show resolved Hide resolved
{
Some((variant_idx, _)) => {
let (tagged, untagged) = variants.split_at(variant_idx);
let tagged_frag = Expr(deserialize_homogenious_enum(params, tagged, cattrs));
let tagged_frag = |deserializer| Expr(quote_block! {
let __deserializer = #deserializer;
#tagged_frag
});
deserialize_untagged_enum_after(params, untagged, cattrs, Some(tagged_frag))
}
None => deserialize_homogenious_enum(params, variants, cattrs),
}
}

fn deserialize_homogenious_enum(
dewert99 marked this conversation as resolved.
Show resolved Hide resolved
params: &Parameters,
variants: &[Variant],
cattrs: &attr::Container,
) -> Fragment {
match cattrs.tag() {
attr::TagType::External => deserialize_externally_tagged_enum(params, variants, cattrs),
Expand Down Expand Up @@ -1661,6 +1684,18 @@ fn deserialize_untagged_enum(
variants: &[Variant],
cattrs: &attr::Container,
) -> Fragment {
deserialize_untagged_enum_after(params, variants, cattrs, None::<fn(_) -> _>)
}

fn deserialize_untagged_enum_after(
params: &Parameters,
variants: &[Variant],
cattrs: &attr::Container,
first_attempt: Option<impl FnOnce(TokenStream) -> Expr>,
) -> Fragment {
let deserializer = || quote!(
_serde::__private::de::ContentRefDeserializer::<__D::Error>::new(&__content)
);
dewert99 marked this conversation as resolved.
Show resolved Hide resolved
let attempts = variants
.iter()
.filter(|variant| !variant.attrs.skip_deserializing())
Expand All @@ -1669,12 +1704,10 @@ fn deserialize_untagged_enum(
params,
variant,
cattrs,
quote!(
_serde::__private::de::ContentRefDeserializer::<__D::Error>::new(&__content)
),
deserializer(),
))
});

let attempts = first_attempt.map(|f| f(deserializer())).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 Down
9 changes: 7 additions & 2 deletions serde_derive/src/internals/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,7 @@ fn enum_from_ast<'a>(
variants: &'a Punctuated<syn::Variant, Token![,]>,
container_default: &attr::Default,
) -> Vec<Variant<'a>> {
let mut seen_untagged = false;
variants
.iter()
.map(|variant| {
Expand All @@ -153,8 +154,12 @@ fn enum_from_ast<'a>(
fields,
original: variant,
}
})
.collect()
}).inspect(|variant| {
if !variant.attrs.untagged() && seen_untagged {
cx.error_spanned_by(&variant.ident, "all variants with the #[serde(untagged)] attribute must be placed at the end of the enum")

Choose a reason for hiding this comment

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

Why only allowing #[serde(untagged)] to be at the end?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This keeps the implementation more consistent with the deserialisation strategy of going though each variant in order and choosing the first variant that matches.

}
seen_untagged = variant.attrs.untagged()
}).collect()
}

fn struct_from_ast<'a>(
Expand Down
12 changes: 12 additions & 0 deletions serde_derive/src/internals/attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -798,6 +798,7 @@ pub struct Variant {
serialize_with: Option<syn::ExprPath>,
deserialize_with: Option<syn::ExprPath>,
borrow: Option<BorrowAttribute>,
untagged: bool,
}

struct BorrowAttribute {
Expand All @@ -820,6 +821,7 @@ impl Variant {
let mut serialize_with = Attr::none(cx, SERIALIZE_WITH);
let mut deserialize_with = Attr::none(cx, DESERIALIZE_WITH);
let mut borrow = Attr::none(cx, BORROW);
let mut untagged = BoolAttr::none(cx, UNTAGGED);

for meta_item in variant
.attrs
Expand Down Expand Up @@ -991,6 +993,11 @@ impl Variant {
}
},

// Parse `#[serde(untagged)]`
Meta(Path(word)) if word == UNTAGGED => {
untagged.set_true(word);
}

Meta(meta_item) => {
let path = meta_item
.path()
Expand Down Expand Up @@ -1022,6 +1029,7 @@ impl Variant {
serialize_with: serialize_with.get(),
deserialize_with: deserialize_with.get(),
borrow: borrow.get(),
untagged: untagged.get(),
}
}

Expand Down Expand Up @@ -1073,6 +1081,10 @@ impl Variant {
pub fn deserialize_with(&self) -> Option<&syn::ExprPath> {
self.deserialize_with.as_ref()
}

pub fn untagged(&self) -> bool {
self.untagged
}
}

/// Represents field attribute information
Expand Down
10 changes: 5 additions & 5 deletions serde_derive/src/ser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -477,17 +477,17 @@ fn serialize_variant(
}
};

let body = Match(match cattrs.tag() {
attr::TagType::External => {
let body = Match(match (cattrs.tag(), variant.attrs.untagged()) {
(attr::TagType::External, false) => {
serialize_externally_tagged_variant(params, variant, variant_index, cattrs)
}
attr::TagType::Internal { tag } => {
(attr::TagType::Internal { tag }, false) => {
serialize_internally_tagged_variant(params, variant, cattrs, tag)
}
attr::TagType::Adjacent { tag, content } => {
(attr::TagType::Adjacent { tag, content }, false) => {
serialize_adjacently_tagged_variant(params, variant, cattrs, tag, content)
}
attr::TagType::None => serialize_untagged_variant(params, variant, cattrs),
(attr::TagType::None, _) | (_, true) => serialize_untagged_variant(params, variant, cattrs),
});

quote! {
Expand Down
27 changes: 27 additions & 0 deletions test_suite/tests/test_annotations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2395,6 +2395,33 @@ fn test_untagged_enum_containing_flatten() {
);
}

#[test]
fn test_partially_untagged_enum() {
#[derive(Serialize, Deserialize, PartialEq, Debug)]
enum Exp {
Lambda(u32, Box<Exp>),
#[serde(untagged)]
App(Box<Exp>, Box<Exp>),
#[serde(untagged)]
Var(u32),
}
use Exp::*;
oli-obk marked this conversation as resolved.
Show resolved Hide resolved

let data = Lambda(0, Box::new(App(Box::new(Var(0)), Box::new(Var(0)))));
assert_tokens(
&data,
&[
Token::TupleVariant {name: "Exp", variant: "Lambda", len: 2},
Token::U32(0),
Token::Tuple{len: 2},
Token::U32(0),
Token::U32(0),
Token::TupleEnd,
Token::TupleVariantEnd,
],
);
}

#[test]
fn test_flatten_untagged_enum() {
#[derive(Serialize, Deserialize, PartialEq, Debug)]
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
use serde_derive::Serialize;

#[derive(Serialize)]
enum E {
#[serde(untagged)]
A(u8),
B(String),
}

fn main() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
error: all variants with the #[serde(untagged)] attribute must be placed at the end of the enum
--> tests/ui/enum-representation/partially_tagged_wrong_order.rs:7:5
|
7 | B(String),
| ^