From ce8144fa5a192e89f5972d62fa9b2a67974e6227 Mon Sep 17 00:00:00 2001 From: Mingun Date: Fri, 19 Sep 2025 00:37:11 +0500 Subject: [PATCH 1/2] Implement parsing and checking of #[serde(default)] attribute on variants of internally-tagged enums --- serde_derive/src/internals/attr.rs | 13 ++++++ serde_derive/src/internals/check.rs | 45 ++++++++++++++++++- .../enum-representation/adjacently-default.rs | 11 +++++ .../adjacently-default.stderr | 6 +++ .../enum-representation/external-default.rs | 10 +++++ .../external-default.stderr | 6 +++ .../internal-default-and-skip.rs | 19 ++++++++ .../internal-default-and-skip.stderr | 13 ++++++ .../internal-default-duplicated.rs | 12 +++++ .../internal-default-duplicated.stderr | 6 +++ .../enum-representation/untagged-default.rs | 11 +++++ .../untagged-default.stderr | 6 +++ 12 files changed, 157 insertions(+), 1 deletion(-) create mode 100644 test_suite/tests/ui/enum-representation/adjacently-default.rs create mode 100644 test_suite/tests/ui/enum-representation/adjacently-default.stderr create mode 100644 test_suite/tests/ui/enum-representation/external-default.rs create mode 100644 test_suite/tests/ui/enum-representation/external-default.stderr create mode 100644 test_suite/tests/ui/enum-representation/internal-default-and-skip.rs create mode 100644 test_suite/tests/ui/enum-representation/internal-default-and-skip.stderr create mode 100644 test_suite/tests/ui/enum-representation/internal-default-duplicated.rs create mode 100644 test_suite/tests/ui/enum-representation/internal-default-duplicated.stderr create mode 100644 test_suite/tests/ui/enum-representation/untagged-default.rs create mode 100644 test_suite/tests/ui/enum-representation/untagged-default.stderr diff --git a/serde_derive/src/internals/attr.rs b/serde_derive/src/internals/attr.rs index f982f03a3..5de87caf4 100644 --- a/serde_derive/src/internals/attr.rs +++ b/serde_derive/src/internals/attr.rs @@ -737,6 +737,7 @@ pub struct Variant { deserialize_with: Option, borrow: Option, untagged: bool, + default: bool, } struct BorrowAttribute { @@ -760,6 +761,7 @@ impl Variant { let mut deserialize_with = Attr::none(cx, DESERIALIZE_WITH); let mut borrow = Attr::none(cx, BORROW); let mut untagged = BoolAttr::none(cx, UNTAGGED); + let mut default = BoolAttr::none(cx, DEFAULT); for attr in &variant.attrs { if attr.path() != SERDE { @@ -878,7 +880,11 @@ impl Variant { } } } else if meta.path == UNTAGGED { + // #[serde(untagged)] untagged.set_true(&meta.path); + } else if meta.path == DEFAULT { + // #[serde(default)] + default.set_true(&meta.path); } else { let path = meta.path.to_token_stream().to_string().replace(' ', ""); return Err( @@ -911,6 +917,7 @@ impl Variant { deserialize_with: deserialize_with.get(), borrow: borrow.get(), untagged: untagged.get(), + default: default.get(), } } @@ -972,6 +979,12 @@ impl Variant { pub fn untagged(&self) -> bool { self.untagged } + + /// This variant marked as default for internally tagged enums. If tag field + /// will not be found, that variant will be assumed + pub fn default(&self) -> bool { + self.default + } } /// Represents field attribute information diff --git a/serde_derive/src/internals/check.rs b/serde_derive/src/internals/check.rs index da2a0fbdc..d6dc636dc 100644 --- a/serde_derive/src/internals/check.rs +++ b/serde_derive/src/internals/check.rs @@ -1,4 +1,4 @@ -use crate::internals::ast::{Container, Data, Field, Style}; +use crate::internals::ast::{Container, Data, Field, Style, Variant}; use crate::internals::attr::{Default, Identifier, TagType}; use crate::internals::{ungroup, Ctxt, Derive}; use syn::{Member, Type}; @@ -13,6 +13,7 @@ pub fn check(cx: &Ctxt, cont: &mut Container, derive: Derive) { check_identifier(cx, cont); check_variant_skip_attrs(cx, cont); check_internal_tag_field_name_conflict(cx, cont); + check_internal_default_variant(cx, cont); check_adjacent_tag_conflict(cx, cont); check_transparent(cx, cont, derive); check_from_and_try_from(cx, cont); @@ -347,6 +348,48 @@ fn check_internal_tag_field_name_conflict(cx: &Ctxt, cont: &Container) { } } +// Variants may be marked with #[serde(default)] attribute, but only in internally-tagged +// enums and only when not skipped. Also, there must be only one such variant +fn check_internal_default_variant(cx: &Ctxt, cont: &Container) { + let variants = match &cont.data { + Data::Enum(variants) => variants, + Data::Struct(_, _) => return, + }; + + let mut default: Option<&Variant<'_>> = None; + for variant in variants { + if variant.attrs.default() { + if let TagType::Internal { .. } = cont.attrs.tag() { + if let Some(default) = default { + cx.error_spanned_by( + variant.original, + format!( + "#[serde(default)] already defined at variant {}", + default.ident + ), + ); + continue; + } + default = Some(variant); + + if variant.attrs.skip_deserializing() { + cx.error_spanned_by( + variant.original, + format!("variant `{}` cannot have both #[serde(default)] and #[serde(skip)] or #[serde(skip_deserializing)]", variant.ident), + ); + } + } else { + cx.error_spanned_by( + variant.original, + "#[serde(default)] can only be used on variants of internally tagged enums", + ); + // We do not want to repeat this error for each variant with #[serde(default)] + return; + } + } + } +} + // In the case of adjacently-tagged enums, the type and the contents tag must // differ, for the same reason. fn check_adjacent_tag_conflict(cx: &Ctxt, cont: &Container) { diff --git a/test_suite/tests/ui/enum-representation/adjacently-default.rs b/test_suite/tests/ui/enum-representation/adjacently-default.rs new file mode 100644 index 000000000..bc3b1b65f --- /dev/null +++ b/test_suite/tests/ui/enum-representation/adjacently-default.rs @@ -0,0 +1,11 @@ +use serde_derive::Deserialize; + +#[derive(Deserialize)] +#[serde(tag = "tag", content = "content")] +enum E { + V1 { f: u8 }, + #[serde(default)] + V2 { f: u8 }, +} + +fn main() {} diff --git a/test_suite/tests/ui/enum-representation/adjacently-default.stderr b/test_suite/tests/ui/enum-representation/adjacently-default.stderr new file mode 100644 index 000000000..8c67dc417 --- /dev/null +++ b/test_suite/tests/ui/enum-representation/adjacently-default.stderr @@ -0,0 +1,6 @@ +error: #[serde(default)] can only be used on variants of internally tagged enums + --> tests/ui/enum-representation/adjacently-default.rs:7:5 + | +7 | / #[serde(default)] +8 | | V2 { f: u8 }, + | |________________^ diff --git a/test_suite/tests/ui/enum-representation/external-default.rs b/test_suite/tests/ui/enum-representation/external-default.rs new file mode 100644 index 000000000..b7acc4dc9 --- /dev/null +++ b/test_suite/tests/ui/enum-representation/external-default.rs @@ -0,0 +1,10 @@ +use serde_derive::Deserialize; + +#[derive(Deserialize)] +enum E { + V1 { f: u8 }, + #[serde(default)] + V2 { f: u8 }, +} + +fn main() {} diff --git a/test_suite/tests/ui/enum-representation/external-default.stderr b/test_suite/tests/ui/enum-representation/external-default.stderr new file mode 100644 index 000000000..17f5889d6 --- /dev/null +++ b/test_suite/tests/ui/enum-representation/external-default.stderr @@ -0,0 +1,6 @@ +error: #[serde(default)] can only be used on variants of internally tagged enums + --> tests/ui/enum-representation/external-default.rs:6:5 + | +6 | / #[serde(default)] +7 | | V2 { f: u8 }, + | |________________^ diff --git a/test_suite/tests/ui/enum-representation/internal-default-and-skip.rs b/test_suite/tests/ui/enum-representation/internal-default-and-skip.rs new file mode 100644 index 000000000..5ecfcdac5 --- /dev/null +++ b/test_suite/tests/ui/enum-representation/internal-default-and-skip.rs @@ -0,0 +1,19 @@ +use serde_derive::Deserialize; + +#[derive(Deserialize)] +#[serde(tag = "tag")] +enum E1 { + V1 { f: u8 }, + #[serde(default, skip)] + V2 { f: u8 }, +} + +#[derive(Deserialize)] +#[serde(tag = "tag")] +enum E2 { + V1 { f: u8 }, + #[serde(default, skip_deserializing)] + V2 { f: u8 }, +} + +fn main() {} diff --git a/test_suite/tests/ui/enum-representation/internal-default-and-skip.stderr b/test_suite/tests/ui/enum-representation/internal-default-and-skip.stderr new file mode 100644 index 000000000..fb7f53a71 --- /dev/null +++ b/test_suite/tests/ui/enum-representation/internal-default-and-skip.stderr @@ -0,0 +1,13 @@ +error: variant `V2` cannot have both #[serde(default)] and #[serde(skip)] or #[serde(skip_deserializing)] + --> tests/ui/enum-representation/internal-default-and-skip.rs:7:5 + | +7 | / #[serde(default, skip)] +8 | | V2 { f: u8 }, + | |________________^ + +error: variant `V2` cannot have both #[serde(default)] and #[serde(skip)] or #[serde(skip_deserializing)] + --> tests/ui/enum-representation/internal-default-and-skip.rs:15:5 + | +15 | / #[serde(default, skip_deserializing)] +16 | | V2 { f: u8 }, + | |________________^ diff --git a/test_suite/tests/ui/enum-representation/internal-default-duplicated.rs b/test_suite/tests/ui/enum-representation/internal-default-duplicated.rs new file mode 100644 index 000000000..147774ba9 --- /dev/null +++ b/test_suite/tests/ui/enum-representation/internal-default-duplicated.rs @@ -0,0 +1,12 @@ +use serde_derive::Deserialize; + +#[derive(Deserialize)] +#[serde(tag = "tag")] +enum E { + #[serde(default)] + V1 { f: u8 }, + #[serde(default)] + V2 { f: u8 }, +} + +fn main() {} diff --git a/test_suite/tests/ui/enum-representation/internal-default-duplicated.stderr b/test_suite/tests/ui/enum-representation/internal-default-duplicated.stderr new file mode 100644 index 000000000..4731c90b9 --- /dev/null +++ b/test_suite/tests/ui/enum-representation/internal-default-duplicated.stderr @@ -0,0 +1,6 @@ +error: #[serde(default)] already defined at variant V1 + --> tests/ui/enum-representation/internal-default-duplicated.rs:8:5 + | +8 | / #[serde(default)] +9 | | V2 { f: u8 }, + | |________________^ diff --git a/test_suite/tests/ui/enum-representation/untagged-default.rs b/test_suite/tests/ui/enum-representation/untagged-default.rs new file mode 100644 index 000000000..1d0b152d1 --- /dev/null +++ b/test_suite/tests/ui/enum-representation/untagged-default.rs @@ -0,0 +1,11 @@ +use serde_derive::Deserialize; + +#[derive(Deserialize)] +#[serde(untagged)] +enum E { + V1 { f: u8 }, + #[serde(default)] + V2 { f: u8 }, +} + +fn main() {} diff --git a/test_suite/tests/ui/enum-representation/untagged-default.stderr b/test_suite/tests/ui/enum-representation/untagged-default.stderr new file mode 100644 index 000000000..abdc85746 --- /dev/null +++ b/test_suite/tests/ui/enum-representation/untagged-default.stderr @@ -0,0 +1,6 @@ +error: #[serde(default)] can only be used on variants of internally tagged enums + --> tests/ui/enum-representation/untagged-default.rs:7:5 + | +7 | / #[serde(default)] +8 | | V2 { f: u8 }, + | |________________^ From eb9ac6383e5b65f29b70a970f53f01c5e07bbfad Mon Sep 17 00:00:00 2001 From: Mingun Date: Thu, 18 Sep 2025 23:36:26 +0500 Subject: [PATCH 2/2] Implement handling of `#[serde(default)]` on variant in internally tagged enums If tag will not be found in the data, the default tag will be assumed --- serde/src/private/de.rs | 11 +- serde_derive/src/de/enum_internally.rs | 33 +++-- .../tests/test_enum_internally_tagged.rs | 120 ++++++++++++++++++ 3 files changed, 148 insertions(+), 16 deletions(-) diff --git a/serde/src/private/de.rs b/serde/src/private/de.rs index 6f657f50e..4dcc83008 100644 --- a/serde/src/private/de.rs +++ b/serde/src/private/de.rs @@ -816,17 +816,18 @@ mod content { pub struct TaggedContentVisitor { tag_name: &'static str, expecting: &'static str, - value: PhantomData, + /// If set, this tag will be used if tag will not be found in data + default: Option, } impl TaggedContentVisitor { /// Visitor for the content of an internally tagged enum with the given /// tag name. - pub fn new(name: &'static str, expecting: &'static str) -> Self { + pub fn new(name: &'static str, expecting: &'static str, default: Option) -> Self { TaggedContentVisitor { tag_name: name, expecting, - value: PhantomData, + default, } } } @@ -846,6 +847,8 @@ mod content { where S: SeqAccess<'de>, { + // We do not support sequence representation without tags, because that may + // create ambiguity during deserialization let tag = match tri!(seq.next_element()) { Some(tag) => tag, None => { @@ -879,7 +882,7 @@ mod content { } } } - match tag { + match tag.or(self.default) { None => Err(de::Error::missing_field(self.tag_name)), Some(tag) => Ok((tag, Content::Map(vec))), } diff --git a/serde_derive/src/de/enum_internally.rs b/serde_derive/src/de/enum_internally.rs index b669eef7e..c08439c9f 100644 --- a/serde_derive/src/de/enum_internally.rs +++ b/serde_derive/src/de/enum_internally.rs @@ -27,25 +27,34 @@ pub(super) fn deserialize( let (variants_stmt, variant_visitor) = enum_::prepare_enum_variant_enum(variants); // Match arms to extract a variant from a string - let variant_arms = variants + let mut variants = variants .iter() .enumerate() - .filter(|&(_, variant)| !variant.attrs.skip_deserializing()) - .map(|(i, variant)| { - let variant_name = field_i(i); + .filter(|&(_, variant)| !variant.attrs.skip_deserializing()); + let variant_arms = variants.clone().map(|(i, variant)| { + let variant_name = field_i(i); - let block = Match(deserialize_internally_tagged_variant( - params, variant, cattrs, - )); + let block = Match(deserialize_internally_tagged_variant( + params, variant, cattrs, + )); - quote! { - __Field::#variant_name => #block - } - }); + quote! { + __Field::#variant_name => #block + } + }); let expecting = format!("internally tagged enum {}", params.type_name()); let expecting = cattrs.expecting().unwrap_or(&expecting); + // We checked that only one variant is marked with #[serde(default)] + let default = match variants.find(|(_, variant)| variant.attrs.default()) { + Some((i, _)) => { + let default = field_i(i); + quote! { _serde::#private::Some(__Field::#default) } + } + None => quote! { _serde::#private::None }, + }; + quote_block! { #variant_visitor @@ -53,7 +62,7 @@ pub(super) fn deserialize( let (__tag, __content) = _serde::Deserializer::deserialize_any( __deserializer, - _serde::#private::de::TaggedContentVisitor::<__Field>::new(#tag, #expecting))?; + _serde::#private::de::TaggedContentVisitor::<__Field>::new(#tag, #expecting, #default))?; let __deserializer = _serde::#private::de::ContentDeserializer::<__D::Error>::new(__content); match __tag { diff --git a/test_suite/tests/test_enum_internally_tagged.rs b/test_suite/tests/test_enum_internally_tagged.rs index b4d428c4d..3be0dbf2c 100644 --- a/test_suite/tests/test_enum_internally_tagged.rs +++ b/test_suite/tests/test_enum_internally_tagged.rs @@ -1037,6 +1037,126 @@ mod struct_enum { } } +#[test] +fn default_variant() { + #[derive(Debug, PartialEq, Serialize, Deserialize)] + #[serde(tag = "tag")] + enum InternallyTaggedWithDefault { + Unit, + NewtypeUnit(()), + NewtypeUnitStruct(Unit), + NewtypeNewtype(Newtype), + NewtypeMap(BTreeMap), + NewtypeStruct(Struct), + NewtypeEnum(Enum), + #[serde(default)] + Struct { + a: u8, + }, + StructEnum { + enum_: Enum, + }, + } + + let value = InternallyTaggedWithDefault::Struct { a: 1 }; + + // Special case: no tag field, use enum tokens + assert_de_tokens( + &value, + &[ + Token::Struct { + name: "InternallyTagged", + len: 1, + }, + Token::Str("a"), + Token::U8(1), + Token::StructEnd, + ], + ); + assert_de_tokens( + &value, + &[ + Token::Struct { + name: "InternallyTagged", + len: 1, + }, + Token::BorrowedStr("a"), + Token::U8(1), + Token::StructEnd, + ], + ); + + // Special case: no tag field, Map representation + assert_de_tokens( + &value, + &[ + Token::Map { len: Some(1) }, + Token::Str("a"), + Token::U8(1), + Token::MapEnd, + ], + ); + assert_de_tokens( + &value, + &[ + Token::Map { len: Some(1) }, + Token::BorrowedStr("a"), + Token::U8(1), + Token::MapEnd, + ], + ); + + // Special case: Map representation, unknown tag + assert_de_tokens_error::( + &[ + Token::Map { len: Some(1) }, + Token::Str("tag"), + Token::Str("Z"), + Token::MapEnd, + ], + "unknown variant `Z`, expected one of \ + `Unit`, \ + `NewtypeUnit`, \ + `NewtypeUnitStruct`, \ + `NewtypeNewtype`, \ + `NewtypeMap`, \ + `NewtypeStruct`, \ + `NewtypeEnum`, \ + `Struct`, \ + `StructEnum`", + ); + + // Special case: Seq representation, unknown tag + assert_de_tokens_error::( + &[ + Token::Seq { len: Some(1) }, + Token::Str("Z"), // tag + Token::SeqEnd, + ], + "unknown variant `Z`, expected one of \ + `Unit`, \ + `NewtypeUnit`, \ + `NewtypeUnitStruct`, \ + `NewtypeNewtype`, \ + `NewtypeMap`, \ + `NewtypeStruct`, \ + `NewtypeEnum`, \ + `Struct`, \ + `StructEnum`", + ); + + // Special case: Seq representation cannot be used without a tag due to ambiguity + assert_de_tokens_error::( + &[ + Token::Seq { len: Some(1) }, + Token::U8(1), // tag (== NewtypeUnit) + Token::SeqEnd, + ], + // The error is not very clear, because actually we got end of sequence instead of a Unit + "invalid type: sequence, expected unit", + ); +} + #[test] fn wrong_tag() { assert_de_tokens_error::(