From 414fd694c0c7d770c9b12a44f2193c3b21327426 Mon Sep 17 00:00:00 2001 From: Johannes Willbold Date: Thu, 27 Dec 2018 17:57:29 +0100 Subject: [PATCH 1/4] Allowed serde(tag="...") on structs Added test test_internally_tagged_struct Renamed EnumTag to TagType as it now also used for structs Modified serialize_struct_as_struct --- serde_derive/src/de.rs | 8 +++--- serde_derive/src/internals/attr.rs | 35 +++++++++++--------------- serde_derive/src/internals/check.rs | 12 ++++----- serde_derive/src/ser.rs | 24 +++++++++++++----- test_suite/tests/test_macros.rs | 38 +++++++++++++++++++++++++++++ 5 files changed, 80 insertions(+), 37 deletions(-) diff --git a/serde_derive/src/de.rs b/serde_derive/src/de.rs index 7d0ce6a0..cadcdc08 100644 --- a/serde_derive/src/de.rs +++ b/serde_derive/src/de.rs @@ -1145,15 +1145,15 @@ fn deserialize_enum( cattrs: &attr::Container, ) -> Fragment { match *cattrs.tag() { - attr::EnumTag::External => deserialize_externally_tagged_enum(params, variants, cattrs), - attr::EnumTag::Internal { ref tag } => { + attr::TagType::External => deserialize_externally_tagged_enum(params, variants, cattrs), + attr::TagType::Internal { ref tag } => { deserialize_internally_tagged_enum(params, variants, cattrs, tag) } - attr::EnumTag::Adjacent { + attr::TagType::Adjacent { ref tag, ref content, } => deserialize_adjacently_tagged_enum(params, variants, cattrs, tag, content), - attr::EnumTag::None => deserialize_untagged_enum(params, variants, cattrs), + attr::TagType::None => deserialize_untagged_enum(params, variants, cattrs), } } diff --git a/serde_derive/src/internals/attr.rs b/serde_derive/src/internals/attr.rs index da66ac0f..8f580e31 100644 --- a/serde_derive/src/internals/attr.rs +++ b/serde_derive/src/internals/attr.rs @@ -120,7 +120,7 @@ pub struct Container { rename_all: RenameRule, ser_bound: Option>, de_bound: Option>, - tag: EnumTag, + tag: TagType, type_from: Option, type_into: Option, remote: Option, @@ -129,7 +129,7 @@ pub struct Container { } /// Styles of representing an enum. -pub enum EnumTag { +pub enum TagType { /// The default. /// /// ```json @@ -358,17 +358,10 @@ impl Container { Meta(NameValue(ref m)) if m.ident == "tag" => { if let Ok(s) = get_lit_str(cx, &m.ident, &m.ident, &m.lit) { match item.data { - syn::Data::Enum(_) => { + syn::Data::Enum(_) | + syn::Data::Struct(_, ..) => { internal_tag.set(&m.ident, s.value()); } - syn::Data::Struct(syn::DataStruct { - ref struct_token, .. - }) => { - cx.error_spanned_by( - struct_token, - "#[serde(tag = \"...\")] can only be used on enums", - ); - } syn::Data::Union(syn::DataUnion { ref union_token, .. }) => { @@ -505,7 +498,7 @@ impl Container { self.de_bound.as_ref().map(|vec| &vec[..]) } - pub fn tag(&self) -> &EnumTag { + pub fn tag(&self) -> &TagType { &self.tag } @@ -540,14 +533,14 @@ fn decide_tag( untagged: BoolAttr, internal_tag: Attr, content: Attr, -) -> EnumTag { +) -> TagType { match ( untagged.0.get_with_tokens(), internal_tag.get_with_tokens(), content.get_with_tokens(), ) { - (None, None, None) => EnumTag::External, - (Some(_), None, None) => EnumTag::None, + (None, None, None) => TagType::External, + (Some(_), None, None) => TagType::None, (None, Some((_, tag)), None) => { // Check that there are no tuple variants. if let syn::Data::Enum(ref data) = item.data { @@ -567,7 +560,7 @@ fn decide_tag( } } } - EnumTag::Internal { tag: tag } + TagType::Internal { tag: tag } } (Some((untagged_tokens, _)), Some((tag_tokens, _)), None) => { cx.error_spanned_by( @@ -578,14 +571,14 @@ fn decide_tag( tag_tokens, "enum cannot be both untagged and internally tagged", ); - EnumTag::External // doesn't matter, will error + TagType::External // doesn't matter, will error } (None, None, Some((content_tokens, _))) => { cx.error_spanned_by( content_tokens, "#[serde(tag = \"...\", content = \"...\")] must be used together", ); - EnumTag::External + TagType::External } (Some((untagged_tokens, _)), None, Some((content_tokens, _))) => { cx.error_spanned_by( @@ -596,9 +589,9 @@ fn decide_tag( content_tokens, "untagged enum cannot have #[serde(content = \"...\")]", ); - EnumTag::External + TagType::External } - (None, Some((_, tag)), Some((_, content))) => EnumTag::Adjacent { + (None, Some((_, tag)), Some((_, content))) => TagType::Adjacent { tag: tag, content: content, }, @@ -615,7 +608,7 @@ fn decide_tag( content_tokens, "untagged enum cannot have #[serde(tag = \"...\", content = \"...\")]", ); - EnumTag::External + TagType::External } } } diff --git a/serde_derive/src/internals/check.rs b/serde_derive/src/internals/check.rs index 87c2b6c2..6409dfc2 100644 --- a/serde_derive/src/internals/check.rs +++ b/serde_derive/src/internals/check.rs @@ -1,5 +1,5 @@ use internals::ast::{Container, Data, Field, Style}; -use internals::attr::{EnumTag, Identifier}; +use internals::attr::{TagType, Identifier}; use internals::{Ctxt, Derive}; use syn::{Member, Type}; @@ -127,7 +127,7 @@ fn check_identifier(cx: &Ctxt, cont: &Container) { } // Variant with `other` attribute cannot appear in untagged enum - (_, Identifier::No, true, &EnumTag::None) => { + (_, Identifier::No, true, &TagType::None) => { cx.error_spanned_by( variant.original, "#[serde(other)] cannot appear on untagged enum", @@ -276,8 +276,8 @@ fn check_internal_tag_field_name_conflict(cx: &Ctxt, cont: &Container) { }; let tag = match *cont.attrs.tag() { - EnumTag::Internal { ref tag } => tag.as_str(), - EnumTag::External | EnumTag::Adjacent { .. } | EnumTag::None => return, + TagType::Internal { ref tag } => tag.as_str(), + TagType::External | TagType::Adjacent { .. } | TagType::None => return, }; let diagnose_conflict = || { @@ -312,11 +312,11 @@ fn check_internal_tag_field_name_conflict(cx: &Ctxt, cont: &Container) { /// contents tag must differ, for the same reason. fn check_adjacent_tag_conflict(cx: &Ctxt, cont: &Container) { let (type_tag, content_tag) = match *cont.attrs.tag() { - EnumTag::Adjacent { + TagType::Adjacent { ref tag, ref content, } => (tag, content), - EnumTag::Internal { .. } | EnumTag::External | EnumTag::None => return, + TagType::Internal { .. } | TagType::External | TagType::None => return, }; if type_tag == content_tag { diff --git a/serde_derive/src/ser.rs b/serde_derive/src/ser.rs index bb6fb316..784c77ac 100644 --- a/serde_derive/src/ser.rs +++ b/serde_derive/src/ser.rs @@ -311,11 +311,23 @@ fn serialize_struct_as_struct( fields: &[Field], cattrs: &attr::Container, ) -> Fragment { - let serialize_fields = + let mut serialize_fields = serialize_struct_visitor(fields, params, false, &StructTrait::SerializeStruct); let type_name = cattrs.name().serialize_name(); + let additional_field_count: usize = match cattrs.tag() { + attr::TagType::Internal{ref tag} => { + let func = StructTrait::SerializeStruct.serialize_field(Span::call_site()); + serialize_fields.insert(0, quote! { + try!(#func(&mut __serde_state, #tag, #type_name)); + }); + + 1 + } + _ => 0 + }; + let mut serialized_fields = fields .iter() .filter(|&field| !field.attrs.skip_serializing()) @@ -331,7 +343,7 @@ fn serialize_struct_as_struct( quote!(if #path(#field_expr) { 0 } else { 1 }) } }) - .fold(quote!(0), |sum, expr| quote!(#sum + #expr)); + .fold(quote!(#additional_field_count), |sum, expr| quote!(#sum + #expr)); quote_block! { let #let_mut __serde_state = try!(_serde::Serializer::serialize_struct(__serializer, #type_name, #len)); @@ -452,17 +464,17 @@ fn serialize_variant( }; let body = Match(match *cattrs.tag() { - attr::EnumTag::External => { + attr::TagType::External => { serialize_externally_tagged_variant(params, variant, variant_index, cattrs) } - attr::EnumTag::Internal { ref tag } => { + attr::TagType::Internal { ref tag } => { serialize_internally_tagged_variant(params, variant, cattrs, tag) } - attr::EnumTag::Adjacent { + attr::TagType::Adjacent { ref tag, ref content, } => serialize_adjacently_tagged_variant(params, variant, cattrs, tag, content), - attr::EnumTag::None => serialize_untagged_variant(params, variant, cattrs), + attr::TagType::None => serialize_untagged_variant(params, variant, cattrs), }); quote! { diff --git a/test_suite/tests/test_macros.rs b/test_suite/tests/test_macros.rs index 4a5b79c9..c12a3f46 100644 --- a/test_suite/tests/test_macros.rs +++ b/test_suite/tests/test_macros.rs @@ -1376,6 +1376,44 @@ fn test_enum_in_internally_tagged_enum() { ); } +#[test] +fn test_internally_tagged_struct() { + #[derive(Debug, PartialEq, Serialize, Deserialize)] + #[serde(tag="type")] + pub struct Struct { + a: u8, + } + + assert_tokens( + &Struct{ a: 1 }, + &[ + Token::Struct { + name: "Struct", + len: 2, + }, + Token::Str("type"), + Token::Str("Struct"), + Token::Str("a"), + Token::U8(1), + Token::StructEnd, + ], + ); + + assert_de_tokens( + &Struct { a: 1 }, + &[ + Token::Struct { + name: "Struct", + len: 1, + }, + Token::Str("a"), + Token::U8(1), + Token::StructEnd, + ], + ); + +} + #[test] fn test_enum_in_untagged_enum() { #[derive(Debug, PartialEq, Serialize, Deserialize)] From 8aa5c2b45dcebd5222593a97f3b50fa11212a74c Mon Sep 17 00:00:00 2001 From: Johannes Willbold Date: Thu, 27 Dec 2018 20:53:08 +0100 Subject: [PATCH 2/4] Removed deprected ui/enum-representation/internally-tagged-struct test --- .../ui/enum-representation/internally-tagged-struct.rs | 8 -------- .../enum-representation/internally-tagged-struct.stderr | 8 -------- 2 files changed, 16 deletions(-) delete mode 100644 test_suite/tests/ui/enum-representation/internally-tagged-struct.rs delete mode 100644 test_suite/tests/ui/enum-representation/internally-tagged-struct.stderr diff --git a/test_suite/tests/ui/enum-representation/internally-tagged-struct.rs b/test_suite/tests/ui/enum-representation/internally-tagged-struct.rs deleted file mode 100644 index 54f895fd..00000000 --- a/test_suite/tests/ui/enum-representation/internally-tagged-struct.rs +++ /dev/null @@ -1,8 +0,0 @@ -#[macro_use] -extern crate serde_derive; - -#[derive(Serialize)] -#[serde(tag = "type")] -struct S; - -fn main() {} diff --git a/test_suite/tests/ui/enum-representation/internally-tagged-struct.stderr b/test_suite/tests/ui/enum-representation/internally-tagged-struct.stderr deleted file mode 100644 index 91d5ce1e..00000000 --- a/test_suite/tests/ui/enum-representation/internally-tagged-struct.stderr +++ /dev/null @@ -1,8 +0,0 @@ -error: #[serde(tag = "...")] can only be used on enums - --> $DIR/internally-tagged-struct.rs:6:1 - | -6 | struct S; - | ^^^^^^ - -error: aborting due to previous error - From 9e53405f43ad0e087dea7ed64ddafca0356c6560 Mon Sep 17 00:00:00 2001 From: Johannes Willbold Date: Thu, 27 Dec 2018 21:21:46 +0100 Subject: [PATCH 3/4] Fix for rustc 1.15.0 --- serde_derive/src/ser.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/serde_derive/src/ser.rs b/serde_derive/src/ser.rs index 784c77ac..6bdf58b7 100644 --- a/serde_derive/src/ser.rs +++ b/serde_derive/src/ser.rs @@ -317,7 +317,7 @@ fn serialize_struct_as_struct( let type_name = cattrs.name().serialize_name(); let additional_field_count: usize = match cattrs.tag() { - attr::TagType::Internal{ref tag} => { + &attr::TagType::Internal{ref tag} => { let func = StructTrait::SerializeStruct.serialize_field(Span::call_site()); serialize_fields.insert(0, quote! { try!(#func(&mut __serde_state, #tag, #type_name)); From 23594178040cb4cce46fedcbd149641955542652 Mon Sep 17 00:00:00 2001 From: Johannes Willbold Date: Thu, 27 Dec 2018 23:01:03 +0100 Subject: [PATCH 4/4] Added ui tests, Limited serde(tag = "...") to structs with named field Added ui test struct-representation/internally-tagged-unit Added ui test struct-representation/internally-tagged-tuple Limited the serde(tag = "...") to enums and structs with named field --- serde_derive/src/internals/attr.rs | 20 ++++++++++++++++--- test_suite/tests/test_macros.rs | 1 - .../internally-taged-tuple.rs | 11 ++++++++++ .../internally-taged-tuple.stderr | 12 +++++++++++ .../internally-tagged-unit.rs | 8 ++++++++ .../internally-tagged-unit.stderr | 8 ++++++++ 6 files changed, 56 insertions(+), 4 deletions(-) create mode 100644 test_suite/tests/ui/struct-representation/internally-taged-tuple.rs create mode 100644 test_suite/tests/ui/struct-representation/internally-taged-tuple.stderr create mode 100644 test_suite/tests/ui/struct-representation/internally-tagged-unit.rs create mode 100644 test_suite/tests/ui/struct-representation/internally-tagged-unit.stderr diff --git a/serde_derive/src/internals/attr.rs b/serde_derive/src/internals/attr.rs index 8f580e31..a7d72e1c 100644 --- a/serde_derive/src/internals/attr.rs +++ b/serde_derive/src/internals/attr.rs @@ -358,16 +358,30 @@ impl Container { Meta(NameValue(ref m)) if m.ident == "tag" => { if let Ok(s) = get_lit_str(cx, &m.ident, &m.ident, &m.lit) { match item.data { - syn::Data::Enum(_) | - syn::Data::Struct(_, ..) => { + syn::Data::Enum(_) => { internal_tag.set(&m.ident, s.value()); } + syn::Data::Struct(syn::DataStruct{ref fields, ..}) => { + match *fields { + syn::Fields::Named(_) => { + internal_tag.set(&m.ident, s.value()); + }, + syn::Fields::Unnamed(_) | syn::Fields::Unit => { + cx.error_spanned_by( + fields, + "#[serde(tag = \"...\")] can only be used on enums \ + and structs with named fields", + ); + } + } + } syn::Data::Union(syn::DataUnion { ref union_token, .. }) => { cx.error_spanned_by( union_token, - "#[serde(tag = \"...\")] can only be used on enums", + "#[serde(tag = \"...\")] can only be used on enums \ + and structs with named fields", ); } } diff --git a/test_suite/tests/test_macros.rs b/test_suite/tests/test_macros.rs index c12a3f46..1eea8f10 100644 --- a/test_suite/tests/test_macros.rs +++ b/test_suite/tests/test_macros.rs @@ -1411,7 +1411,6 @@ fn test_internally_tagged_struct() { Token::StructEnd, ], ); - } #[test] diff --git a/test_suite/tests/ui/struct-representation/internally-taged-tuple.rs b/test_suite/tests/ui/struct-representation/internally-taged-tuple.rs new file mode 100644 index 00000000..78437f17 --- /dev/null +++ b/test_suite/tests/ui/struct-representation/internally-taged-tuple.rs @@ -0,0 +1,11 @@ +#[macro_use] +extern crate serde_derive; + +#[derive(Serialize)] +#[serde(tag = "type")] +struct S ( + u8, + u8 +); + +fn main() {} diff --git a/test_suite/tests/ui/struct-representation/internally-taged-tuple.stderr b/test_suite/tests/ui/struct-representation/internally-taged-tuple.stderr new file mode 100644 index 00000000..6bae9760 --- /dev/null +++ b/test_suite/tests/ui/struct-representation/internally-taged-tuple.stderr @@ -0,0 +1,12 @@ +error: #[serde(tag = "...")] can only be used on enums and structs with named fields + --> $DIR/internally-taged-tuple.rs:6:10 + | +6 | struct S ( + | __________^ +7 | | u8, +8 | | u8 +9 | | ); + | |_^ + +error: aborting due to previous error + diff --git a/test_suite/tests/ui/struct-representation/internally-tagged-unit.rs b/test_suite/tests/ui/struct-representation/internally-tagged-unit.rs new file mode 100644 index 00000000..cd38cd76 --- /dev/null +++ b/test_suite/tests/ui/struct-representation/internally-tagged-unit.rs @@ -0,0 +1,8 @@ +#[macro_use] +extern crate serde_derive; + +#[derive(Serialize)] +#[serde(tag = "type")] +struct U; + +fn main() {} diff --git a/test_suite/tests/ui/struct-representation/internally-tagged-unit.stderr b/test_suite/tests/ui/struct-representation/internally-tagged-unit.stderr new file mode 100644 index 00000000..ba0cbb40 --- /dev/null +++ b/test_suite/tests/ui/struct-representation/internally-tagged-unit.stderr @@ -0,0 +1,8 @@ +error: #[serde(tag = "...")] can only be used on enums and structs with named fields + --> $DIR/internally-tagged-unit.rs:4:10 + | +4 | #[derive(Serialize)] + | ^^^^^^^^^ + +error: aborting due to previous error +