From 18a5c2344b65fd9234b4e4149f31d6ebafb6ffa8 Mon Sep 17 00:00:00 2001 From: Phaiax Date: Sat, 1 Jul 2017 01:51:23 +0200 Subject: [PATCH 1/7] WIP: Allow #[serde(rename=str|bool|int)] for enum variants. (This commit contains untested code and a dirty hacks so that the rename="str" still works) --- serde_derive/src/de.rs | 36 ++++++-- serde_derive/src/fragment.rs | 52 ++++++++++++ serde_derive/src/ser.rs | 16 ++-- serde_derive_internals/src/attr.rs | 82 +++++++++++++++++-- serde_derive_internals/src/check.rs | 41 +++++++++- .../externally-renamed-as-bool.rs | 18 ++++ .../externally-renamed-as-int.rs | 18 ++++ .../enum-representation/renamed-wrong.rs | 20 +++++ .../untagged-renamed-as-bool.rs | 19 +++++ .../untagged-renamed-as-int.rs | 19 +++++ 10 files changed, 295 insertions(+), 26 deletions(-) create mode 100644 test_suite/tests/compile-fail/enum-representation/externally-renamed-as-bool.rs create mode 100644 test_suite/tests/compile-fail/enum-representation/externally-renamed-as-int.rs create mode 100644 test_suite/tests/compile-fail/enum-representation/renamed-wrong.rs create mode 100644 test_suite/tests/compile-fail/enum-representation/untagged-renamed-as-bool.rs create mode 100644 test_suite/tests/compile-fail/enum-representation/untagged-renamed-as-int.rs diff --git a/serde_derive/src/de.rs b/serde_derive/src/de.rs index 03e265af4..2a2b41567 100644 --- a/serde_derive/src/de.rs +++ b/serde_derive/src/de.rs @@ -10,7 +10,7 @@ use syn::{self, Ident}; use quote::{self, Tokens, ToTokens}; use bound; -use fragment::{Fragment, Expr, Stmts, Match}; +use fragment::{Fragment, Expr, Stmts, Match, StrBoolInt}; use internals::ast::{Body, Container, Field, Style, Variant}; use internals::{self, attr}; @@ -602,7 +602,11 @@ fn deserialize_externally_tagged_enum( .iter() .enumerate() .filter(|&(_, variant)| !variant.attrs.skip_deserializing()) - .map(|(i, variant)| (variant.attrs.name().deserialize_name(), field_i(i)),) + .map(|(i, variant)| (match variant.attrs.name().deserialize_name() { + attr::StrBoolInt::Str(s) => s, + _ => unreachable!(), + }, + field_i(i)),) .collect(); let variants_stmt = { @@ -695,16 +699,21 @@ fn deserialize_internally_tagged_enum( .iter() .enumerate() .filter(|&(_, variant)| !variant.attrs.skip_deserializing()) - .map(|(i, variant)| (variant.attrs.name().deserialize_name(), field_i(i)),) + .map(|(i, variant)| (StrBoolInt::from(variant.attrs.name().deserialize_name()), field_i(i)),) .collect(); let variants_stmt = { - let variant_names = variant_names_idents.iter().map(|&(ref name, _)| name); + let variant_names = variant_names_idents.iter().map(|&(ref name, _)| name.stringify()); quote! { const VARIANTS: &'static [&'static str] = &[ #(#variant_names),* ]; } }; + // TODO: just to keep the compiler quiet for now, this must not be strinfied here! + let variant_names_idents = variant_names_idents.iter().map(|&(ref variant, ref ident)| + (variant.stringify(), ident.clone())) + .collect(); + let variant_visitor = Stmts(deserialize_generated_identifier(variant_names_idents, cattrs, true),); // Match arms to extract a variant from a string @@ -755,16 +764,21 @@ fn deserialize_adjacently_tagged_enum( .iter() .enumerate() .filter(|&(_, variant)| !variant.attrs.skip_deserializing()) - .map(|(i, variant)| (variant.attrs.name().deserialize_name(), field_i(i)),) + .map(|(i, variant)| (StrBoolInt::from(variant.attrs.name().deserialize_name()), field_i(i)),) .collect(); let variants_stmt = { - let variant_names = variant_names_idents.iter().map(|&(ref name, _)| name); + let variant_names = variant_names_idents.iter().map(|&(ref name, _)| name.stringify()); quote! { const VARIANTS: &'static [&'static str] = &[ #(#variant_names),* ]; } }; + // TODO: just to keep the compiler quiet for now, this must not be strinfied here! + let variant_names_idents = variant_names_idents.iter().map(|&(ref variant, ref ident)| + (variant.stringify(), ident.clone())) + .collect(); + let variant_visitor = Stmts(deserialize_generated_identifier(variant_names_idents, cattrs, true),); let ref variant_arms: Vec<_> = variants @@ -1319,10 +1333,10 @@ fn deserialize_custom_identifier( let names_idents: Vec<_> = ordinary .iter() - .map(|variant| (variant.attrs.name().deserialize_name(), variant.ident.clone()),) + .map(|variant| (StrBoolInt::from(variant.attrs.name().deserialize_name()), variant.ident.clone()),) .collect(); - let names = names_idents.iter().map(|&(ref name, _)| name); + let names = names_idents.iter().map(|&(ref name, _)| name.stringify()); let names_const = if fallthrough.is_some() { None @@ -1338,6 +1352,12 @@ fn deserialize_custom_identifier( Some(fields) }; + // TODO: just to keep the compiler quiet for now, this must not be strinfied here! + let names_idents : Vec<_> = names_idents.iter().map(|&(ref variant, ref ident)| + (variant.stringify(), ident.clone())) + .collect(); + + let (de_impl_generics, de_ty_generics, ty_generics, where_clause) = split_with_de_lifetime(params,); let visitor_impl = Stmts(deserialize_identifier(this.clone(), &names_idents, is_variant, fallthrough),); diff --git a/serde_derive/src/fragment.rs b/serde_derive/src/fragment.rs index 58cf0a2ca..7c024afc8 100644 --- a/serde_derive/src/fragment.rs +++ b/serde_derive/src/fragment.rs @@ -73,3 +73,55 @@ impl ToTokens for Match { } } } + +use internals::attr::StrBoolInt as StrBoolIntInternal; + +/// An extended name, used for the type tag for enums. +#[derive(Debug, Clone)] +pub enum StrBoolInt { + Str(String), + Bool(bool), + Int(u64), +} + +impl StrBoolInt { + /// Get a string description for use in error messages. + /// + /// It will be used as second argument in + /// `_serde::Serializer::serialize_struct(__serializer, name, len)` + /// while serializing the inner struct in adjacently tagged enums. + /// + /// It will be used in the `VARIANTS` const array, that is given to + /// `serde::de::Error::unknown_variant(variant, expected)`. + /// and + /// `_serde::Deserializer::deserialize_enum(name, variants, visitor)` + /// + pub fn stringify(&self) -> String { + match *self { + StrBoolInt::Str(ref s) => s.clone(), + StrBoolInt::Bool(true) => "bool: true".to_owned(), + StrBoolInt::Bool(false) => "bool: false".to_owned(), + StrBoolInt::Int(i) => format!("integer: {}", i), + } + } +} + +impl From for StrBoolInt { + fn from(src: StrBoolIntInternal) -> StrBoolInt { + match src { + StrBoolIntInternal::Str(s) => StrBoolInt::Str(s), + StrBoolIntInternal::Bool(b) => StrBoolInt::Bool(b), + StrBoolIntInternal::Int(i) => StrBoolInt::Int(i), + } + } +} + +impl ToTokens for StrBoolInt { + fn to_tokens(&self, out: &mut Tokens) { + match *self { + StrBoolInt::Str(ref s) => s.to_tokens(out), + StrBoolInt::Bool(b) => b.to_tokens(out), + StrBoolInt::Int(i) => i.to_tokens(out), + } + } +} diff --git a/serde_derive/src/ser.rs b/serde_derive/src/ser.rs index 66603d8d3..1b361fa13 100644 --- a/serde_derive/src/ser.rs +++ b/serde_derive/src/ser.rs @@ -10,7 +10,7 @@ use syn::{self, Ident}; use quote::Tokens; use bound; -use fragment::{Fragment, Stmts, Match}; +use fragment::{Fragment, Stmts, Match, StrBoolInt}; use internals::ast::{Body, Container, Field, Style, Variant}; use internals::{attr, Ctxt}; @@ -392,7 +392,10 @@ fn serialize_externally_tagged_variant( cattrs: &attr::Container, ) -> Fragment { let type_name = cattrs.name().serialize_name(); - let variant_name = variant.attrs.name().serialize_name(); + let variant_name = match variant.attrs.name().serialize_name() { + attr::StrBoolInt::Str(s) => s, + _ => unreachable!(), + }; match variant.style { Style::Unit => { @@ -454,7 +457,7 @@ fn serialize_internally_tagged_variant( tag: &str, ) -> Fragment { let type_name = cattrs.name().serialize_name(); - let variant_name = variant.attrs.name().serialize_name(); + let variant_name = StrBoolInt::from(variant.attrs.name().serialize_name()); let enum_ident_str = params.type_name(); let variant_ident_str = variant.ident.as_ref(); @@ -511,7 +514,7 @@ fn serialize_adjacently_tagged_variant( ) -> Fragment { let this = ¶ms.this; let type_name = cattrs.name().serialize_name(); - let variant_name = variant.attrs.name().serialize_name(); + let variant_name = StrBoolInt::from(variant.attrs.name().serialize_name()); let inner = Stmts( match variant.style { @@ -539,11 +542,12 @@ fn serialize_adjacently_tagged_variant( serialize_tuple_variant(TupleVariant::Untagged, params, &variant.fields) } Style::Struct => { + let str_variant_name = variant_name.stringify(); serialize_struct_variant( StructVariant::Untagged, params, &variant.fields, - &variant_name, + &str_variant_name, ) } }, @@ -696,7 +700,7 @@ enum StructVariant<'a> { variant_index: u32, variant_name: String, }, - InternallyTagged { tag: &'a str, variant_name: String }, + InternallyTagged { tag: &'a str, variant_name: StrBoolInt }, Untagged, } diff --git a/serde_derive_internals/src/attr.rs b/serde_derive_internals/src/attr.rs index caf306cfa..b3b90033f 100644 --- a/serde_derive_internals/src/attr.rs +++ b/serde_derive_internals/src/attr.rs @@ -99,6 +99,32 @@ impl Name { } } +/// An extended name, used for the type tag for enums. +#[derive(Debug, Clone)] +pub enum StrBoolInt { + Str(String), + Bool(bool), + Int(u64), +} + +#[derive(Debug)] +pub struct VariantName { + serialize: StrBoolInt, + deserialize: StrBoolInt, +} + +impl VariantName { + /// Return the container name for the container when serializing. + pub fn serialize_name(&self) -> StrBoolInt { + self.serialize.clone() + } + + /// Return the container name for the container when deserializing. + pub fn deserialize_name(&self) -> StrBoolInt { + self.deserialize.clone() + } +} + /// Represents container (e.g. struct) attribute information #[derive(Debug)] pub struct Container { @@ -503,7 +529,7 @@ fn decide_identifier( /// Represents variant attribute information #[derive(Debug)] pub struct Variant { - name: Name, + name: VariantName, ser_renamed: bool, de_renamed: bool, rename_all: RenameRule, @@ -526,7 +552,10 @@ impl Variant { match meta_item { // Parse `#[serde(rename = "foo")]` MetaItem(NameValue(ref name, ref lit)) if name == "rename" => { - if let Ok(s) = get_string_from_lit(cx, name.as_ref(), name.as_ref(), lit) { + if let Ok(s) = get_int_or_string_or_bool_from_lit(cx, + name.as_ref(), + name.as_ref(), + lit) { ser_name.set(s.clone()); de_name.set(s); } @@ -534,7 +563,7 @@ impl Variant { // Parse `#[serde(rename(serialize = "foo", deserialize = "bar"))]` MetaItem(List(ref name, ref meta_items)) if name == "rename" => { - if let Ok((ser, de)) = get_renames(cx, meta_items) { + if let Ok((ser, de)) = get_revariantnames(cx, meta_items) { ser_name.set_opt(ser); de_name.set_opt(de); } @@ -585,9 +614,9 @@ impl Variant { let de_name = de_name.get(); let de_renamed = de_name.is_some(); Variant { - name: Name { - serialize: ser_name.unwrap_or_else(|| variant.ident.to_string()), - deserialize: de_name.unwrap_or_else(|| variant.ident.to_string()), + name: VariantName { + serialize: ser_name.unwrap_or_else(|| StrBoolInt::Str(variant.ident.to_string())), + deserialize: de_name.unwrap_or_else(|| StrBoolInt::Str(variant.ident.to_string())), }, ser_renamed: ser_renamed, de_renamed: de_renamed, @@ -598,16 +627,20 @@ impl Variant { } } - pub fn name(&self) -> &Name { + pub fn name(&self) -> &VariantName { &self.name } pub fn rename_by_rule(&mut self, rule: &RenameRule) { if !self.ser_renamed { - self.name.serialize = rule.apply_to_variant(&self.name.serialize); + if let StrBoolInt::Str(ref mut serialize) = self.name.serialize { + *serialize = rule.apply_to_variant(&serialize); + } } if !self.de_renamed { - self.name.deserialize = rule.apply_to_variant(&self.name.deserialize); + if let StrBoolInt::Str(ref mut deserialize) = self.name.deserialize { + *deserialize = rule.apply_to_variant(&deserialize); + } } } @@ -976,6 +1009,10 @@ fn get_renames(cx: &Ctxt, items: &[syn::NestedMetaItem]) -> Result Result, ()> { + get_ser_and_de(cx, "rename", items, get_int_or_string_or_bool_from_lit) +} + fn get_where_predicates( cx: &Ctxt, items: &[syn::NestedMetaItem], @@ -1010,6 +1047,33 @@ fn get_string_from_lit( } } +fn get_int_or_string_or_bool_from_lit( + cx: &Ctxt, + attr_name: &str, + meta_item_name: &str, + lit: &syn::Lit, +) -> Result { + if let syn::Lit::Str(ref s, _) = *lit { + Ok(StrBoolInt::Str(s.clone())) + } else if let syn::Lit::Int(u, _) = *lit { + Ok(StrBoolInt::Int(u)) + } else if let syn::Lit::Bool(b) = *lit { + Ok(StrBoolInt::Bool(b)) + } else { + cx.error( + format!( + "expected serde {} attribute to be a string, an int or a bool: \ + `{mn} = \"...\"` or \ + `{mn} = 3 or \ + `{mn} = true` ", + attr_name, + mn=meta_item_name + ), + ); + Err(()) + } +} + fn parse_lit_into_path(cx: &Ctxt, attr_name: &str, lit: &syn::Lit) -> Result { let string = try!(get_string_from_lit(cx, attr_name, attr_name, lit)); syn::parse_path(&string).map_err(|err| cx.error(err)) diff --git a/serde_derive_internals/src/check.rs b/serde_derive_internals/src/check.rs index 84c958ea9..43247fbe4 100644 --- a/serde_derive_internals/src/check.rs +++ b/serde_derive_internals/src/check.rs @@ -7,7 +7,7 @@ // except according to those terms. use ast::{Body, Container, Style}; -use attr::Identifier; +use attr::{Identifier, EnumTag, StrBoolInt}; use Ctxt; /// Cross-cutting checks that require looking at more than a single attrs @@ -15,6 +15,7 @@ use Ctxt; pub fn check(cx: &Ctxt, cont: &Container) { check_getter(cx, cont); check_identifier(cx, cont); + check_variant_name(cx, cont); } /// Getters are only allowed inside structs (not enums) with the `remote` @@ -30,7 +31,7 @@ fn check_getter(cx: &Ctxt, cont: &Container) { if cont.body.has_getter() && cont.attrs.remote().is_none() { cx.error( "#[serde(getter = \"...\")] can only be used in structs \ - that have #[serde(remote = \"...\")]", + that have #[serde(remote = \"...\")]", ); } } @@ -52,7 +53,11 @@ fn check_identifier(cx: &Ctxt, cont: &Container) { }; for (i, variant) in variants.iter().enumerate() { - match (variant.style, cont.attrs.identifier(), variant.attrs.other()) { + match ( + variant.style, + cont.attrs.identifier(), + variant.attrs.other(), + ) { // The `other` attribute may only be used in a field_identifier. (_, Identifier::Variant, true) | (_, Identifier::No, true) => { @@ -94,3 +99,33 @@ fn check_identifier(cx: &Ctxt, cont: &Container) { } } } + +/// Renaming variants is only allowed for internal and adjacent tagging. +fn check_variant_name(cx: &Ctxt, cont: &Container) { + let tagtype = cont.attrs.tag(); + + match cont.body { + Body::Struct(_, _) => {} + Body::Enum(ref variants) => { + for name in variants.iter().map(|v| v.attrs.name()) { + match (name.serialize_name(), name.deserialize_name()) { + (StrBoolInt::Bool(_), _) | + (_, StrBoolInt::Bool(_)) | + (_, StrBoolInt::Int(_)) | + (StrBoolInt::Int(_), _) => { + match *tagtype { + EnumTag::External => { + cx.error("#[serde(rename = int|bool)] cannot be used with external tagging"); + }, + EnumTag::None => { + cx.error("#[serde(rename = int|bool)] cannot be used with #[serde(untagged)]"); + }, + _ => {} + } + }, + _ => {} + } + } + } + } +} diff --git a/test_suite/tests/compile-fail/enum-representation/externally-renamed-as-bool.rs b/test_suite/tests/compile-fail/enum-representation/externally-renamed-as-bool.rs new file mode 100644 index 000000000..1ad46c8bf --- /dev/null +++ b/test_suite/tests/compile-fail/enum-representation/externally-renamed-as-bool.rs @@ -0,0 +1,18 @@ +// Copyright 2017 Serde Developers +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#[macro_use] +extern crate serde_derive; + +#[derive(Serialize)] //~ ERROR: proc-macro derive panicked +enum E { + #[serde(rename = true)] //~^^ HELP: #[serde(rename = int|bool)] cannot be used with external tagging + Tuple(u8, u8), +} + +fn main() {} diff --git a/test_suite/tests/compile-fail/enum-representation/externally-renamed-as-int.rs b/test_suite/tests/compile-fail/enum-representation/externally-renamed-as-int.rs new file mode 100644 index 000000000..fd4f835c0 --- /dev/null +++ b/test_suite/tests/compile-fail/enum-representation/externally-renamed-as-int.rs @@ -0,0 +1,18 @@ +// Copyright 2017 Serde Developers +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#[macro_use] +extern crate serde_derive; + +#[derive(Serialize)] //~ ERROR: proc-macro derive panicked +enum E { + #[serde(rename = 3)] //~^^ HELP: #[serde(rename = int|bool)] cannot be used with external tagging + Tuple(u8, u8), +} + +fn main() {} diff --git a/test_suite/tests/compile-fail/enum-representation/renamed-wrong.rs b/test_suite/tests/compile-fail/enum-representation/renamed-wrong.rs new file mode 100644 index 000000000..089d4252b --- /dev/null +++ b/test_suite/tests/compile-fail/enum-representation/renamed-wrong.rs @@ -0,0 +1,20 @@ +// Copyright 2017 Serde Developers +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#[macro_use] +extern crate serde_derive; + +#[derive(Serialize)] //~ ERROR: proc-macro derive panicked +#[serde(tag = "type")] +enum E { + #[serde(rename = 2.1)] //~^^^ HELP: expected serde rename attribute to be a string, an int or a bool: `rename = "..."` or `rename = 3 or `rename = true` + Newtype(u8), +} + + +fn main() {} diff --git a/test_suite/tests/compile-fail/enum-representation/untagged-renamed-as-bool.rs b/test_suite/tests/compile-fail/enum-representation/untagged-renamed-as-bool.rs new file mode 100644 index 000000000..3585322bd --- /dev/null +++ b/test_suite/tests/compile-fail/enum-representation/untagged-renamed-as-bool.rs @@ -0,0 +1,19 @@ +// Copyright 2017 Serde Developers +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#[macro_use] +extern crate serde_derive; + +#[derive(Serialize)] //~ ERROR: proc-macro derive panicked +#[serde(untagged)] +enum E { + #[serde(rename = false)] //~^^^ HELP: #[serde(rename = int|bool)] cannot be used with #[serde(untagged)] + Tuple(u8, u8), +} + +fn main() {} diff --git a/test_suite/tests/compile-fail/enum-representation/untagged-renamed-as-int.rs b/test_suite/tests/compile-fail/enum-representation/untagged-renamed-as-int.rs new file mode 100644 index 000000000..af0188fe2 --- /dev/null +++ b/test_suite/tests/compile-fail/enum-representation/untagged-renamed-as-int.rs @@ -0,0 +1,19 @@ +// Copyright 2017 Serde Developers +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#[macro_use] +extern crate serde_derive; + +#[derive(Serialize)] //~ ERROR: proc-macro derive panicked +#[serde(untagged)] +enum E { + #[serde(rename = 3)] //~^^^ HELP: #[serde(rename = int|bool)] cannot be used with #[serde(untagged)] + Tuple(u8, u8), +} + +fn main() {} From 957615064cfb7e52cc190fb7cff1f1e4c2ceb51b Mon Sep 17 00:00:00 2001 From: Phaiax Date: Mon, 3 Jul 2017 14:22:34 +0200 Subject: [PATCH 2/7] WIP: Add #[serde(rename_as)) as fallback for old rustc that do not support non-str attribute literals --- serde_derive/src/fragment.rs | 10 +- serde_derive_internals/src/attr.rs | 122 ++++++++++++++++-- .../rename-as-bool-parse-error.rs | 19 +++ .../rename-as-int-parse-error.rs | 19 +++ .../enum-representation/renamed-as-wrong.rs | 19 +++ .../renamed-literal-and-renamed-as.rs | 19 +++ test_suite/tests/test_macros.rs | 41 ++++++ 7 files changed, 239 insertions(+), 10 deletions(-) create mode 100644 test_suite/tests/compile-fail/enum-representation/rename-as-bool-parse-error.rs create mode 100644 test_suite/tests/compile-fail/enum-representation/rename-as-int-parse-error.rs create mode 100644 test_suite/tests/compile-fail/enum-representation/renamed-as-wrong.rs create mode 100644 test_suite/tests/compile-fail/enum-representation/renamed-literal-and-renamed-as.rs diff --git a/serde_derive/src/fragment.rs b/serde_derive/src/fragment.rs index 7c024afc8..67ad58108 100644 --- a/serde_derive/src/fragment.rs +++ b/serde_derive/src/fragment.rs @@ -120,8 +120,14 @@ impl ToTokens for StrBoolInt { fn to_tokens(&self, out: &mut Tokens) { match *self { StrBoolInt::Str(ref s) => s.to_tokens(out), - StrBoolInt::Bool(b) => b.to_tokens(out), - StrBoolInt::Int(i) => i.to_tokens(out), + StrBoolInt::Bool(b) => { + out.append("&"); + b.to_tokens(out); + }, + StrBoolInt::Int(i) => { + out.append("&"); + i.to_tokens(out); + } } } } diff --git a/serde_derive_internals/src/attr.rs b/serde_derive_internals/src/attr.rs index b3b90033f..9c3241685 100644 --- a/serde_derive_internals/src/attr.rs +++ b/serde_derive_internals/src/attr.rs @@ -100,7 +100,7 @@ impl Name { } /// An extended name, used for the type tag for enums. -#[derive(Debug, Clone)] +#[derive(Debug, Clone, Eq, PartialEq)] pub enum StrBoolInt { Str(String), Bool(bool), @@ -541,7 +541,9 @@ pub struct Variant { impl Variant { pub fn from_ast(cx: &Ctxt, variant: &syn::Variant) -> Self { let mut ser_name = Attr::none(cx, "rename"); + let mut ser_rename_as = Attr::none(cx, "rename_as"); let mut de_name = Attr::none(cx, "rename"); + let mut de_rename_as = Attr::none(cx, "rename_as"); let mut skip_deserializing = BoolAttr::none(cx, "skip_deserializing"); let mut skip_serializing = BoolAttr::none(cx, "skip_serializing"); let mut rename_all = Attr::none(cx, "rename_all"); @@ -563,12 +565,31 @@ impl Variant { // Parse `#[serde(rename(serialize = "foo", deserialize = "bar"))]` MetaItem(List(ref name, ref meta_items)) if name == "rename" => { - if let Ok((ser, de)) = get_revariantnames(cx, meta_items) { + if let Ok((ser, de)) = get_ser_and_de(cx, "rename", meta_items, + get_int_or_string_or_bool_from_lit) { ser_name.set_opt(ser); de_name.set_opt(de); } } + // Parse `#[serde(rename_as = "int")]` + MetaItem(NameValue(ref name, ref lit)) if name == "rename_as" => { + if let Ok(s) = get_string_from_lit(cx, "rename_as", name.as_ref(), lit) { + let rename_as = match_rename_as(cx, &Some(s)); + ser_rename_as.set(rename_as.clone()); + de_rename_as.set(rename_as); + } + } + + // Parse `#[serde(rename_as(serialize = "int", deserialize = "bool"))]` + MetaItem(List(ref name, ref meta_items)) if name == "rename_as" => { + if let Ok((ser, de)) = get_ser_and_de(cx, "rename_as", meta_items, + get_string_from_lit) { + ser_rename_as.set(match_rename_as(cx, &ser)); + de_rename_as.set(match_rename_as(cx, &de)); + } + } + // Parse `#[serde(rename_all = "foo")]` MetaItem(NameValue(ref name, ref lit)) if name == "rename_all" => { if let Ok(s) = get_string_from_lit(cx, name.as_ref(), name.as_ref(), lit) { @@ -609,10 +630,12 @@ impl Variant { } } - let ser_name = ser_name.get(); + let (ser_name, de_name) = ser_de_interpret_as(cx, + ser_name.get(), de_name.get(), + ser_rename_as.get(), de_rename_as.get()); let ser_renamed = ser_name.is_some(); - let de_name = de_name.get(); let de_renamed = de_name.is_some(); + Variant { name: VariantName { serialize: ser_name.unwrap_or_else(|| StrBoolInt::Str(variant.ident.to_string())), @@ -1009,10 +1032,6 @@ fn get_renames(cx: &Ctxt, items: &[syn::NestedMetaItem]) -> Result Result, ()> { - get_ser_and_de(cx, "rename", items, get_int_or_string_or_bool_from_lit) -} - fn get_where_predicates( cx: &Ctxt, items: &[syn::NestedMetaItem], @@ -1138,6 +1157,93 @@ fn parse_lit_into_lifetimes( Err(cx.error(format!("failed to parse borrowed lifetimes: {:?}", string)),) } +fn match_rename_as(cx: &Ctxt, rename_as : &Option) -> StrBoolInt { + match rename_as.as_ref() { + Some(s) if s == "bool" => { + StrBoolInt::Bool(false) + }, + Some(s) if s == "int" => { + StrBoolInt::Int(0) + }, + None => { + StrBoolInt::Str("".to_owned()) + }, + Some(s) => { + cx.error(format!("unknown rename type for #[serde(rename_as = \"{}\")]. \ + expected \"int\" or \"bool\"", + s)); + StrBoolInt::Str("".to_owned()) + }, + } +} + +fn interpret_as( + cx: &Ctxt, + name : StrBoolInt, + rename_as : &Option +) -> Result { + match (name, rename_as) { + (StrBoolInt::Str(name), &Some(StrBoolInt::Int(_))) => { + if let Ok(i) = u64::from_str(&name) { + Ok(StrBoolInt::Int(i)) + } else { + cx.error(format!("failed to parse {:?} as int", name)); + Err(StrBoolInt::Str(name)) + } + }, + (StrBoolInt::Str(name), &Some(StrBoolInt::Bool(_))) => { + if let Ok(b) = bool::from_str(&name) { + Ok(StrBoolInt::Bool(b)) + } else { + cx.error(format!("failed to parse {:?} as bool", name)); + Err(StrBoolInt::Str(name)) + } + }, + (StrBoolInt::Int(i), &Some(_)) => { + cx.error(format!("#[serde(rename = {i})] is not compatible with \ + #[serde(rename_as = ...)]. Use a string literal as in \ + #[serde(rename = \"{i}\")]" + , i=i)); + Err(StrBoolInt::Int(i)) + }, + (StrBoolInt::Bool(b), &Some(_)) => { + cx.error(format!("#[serde(rename = {b})] is not compatible with \ + #[serde(rename_as = ...)]. Use a string literal as in \ + #[serde(rename = \"{b}\")]" + , b=b)); + Err(StrBoolInt::Bool(b)) + }, + (r @ _, _) => Ok(r) + } +} + +fn ser_de_interpret_as( + cx: &Ctxt, + ser_name: Option, + de_name: Option, + ser_rename_as: Option, + de_rename_as: Option +) -> (Option, Option) { + // Display rename_as related errors only once. + let equal = ser_name == de_name && ser_rename_as == de_rename_as; + let mut rename_as_error = false; + let ser_name = ser_name.map(|name| { + interpret_as(cx, name, &ser_rename_as) + .unwrap_or_else(|name| { + rename_as_error = true; + name + }) + }); + let de_name = de_name.map(|name| { + if !rename_as_error || !equal { + interpret_as(cx, name, &de_rename_as).unwrap_or_else(|n| n) + } else { + name + } + }); + (ser_name, de_name) +} + // Whether the type looks like it might be `std::borrow::Cow` where elem="T". // This can have false negatives and false positives. // diff --git a/test_suite/tests/compile-fail/enum-representation/rename-as-bool-parse-error.rs b/test_suite/tests/compile-fail/enum-representation/rename-as-bool-parse-error.rs new file mode 100644 index 000000000..0d95580d9 --- /dev/null +++ b/test_suite/tests/compile-fail/enum-representation/rename-as-bool-parse-error.rs @@ -0,0 +1,19 @@ +// Copyright 2017 Serde Developers +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#[macro_use] +extern crate serde_derive; + +#[derive(Serialize)] //~ ERROR: proc-macro derive panicked +#[serde(tag = "type")] +enum E { + #[serde(rename = "foo", rename_as = "bool")] //~^^^ HELP: failed to parse "foo" as bool + Newtype(u8), +} + +fn main() {} diff --git a/test_suite/tests/compile-fail/enum-representation/rename-as-int-parse-error.rs b/test_suite/tests/compile-fail/enum-representation/rename-as-int-parse-error.rs new file mode 100644 index 000000000..20adb1621 --- /dev/null +++ b/test_suite/tests/compile-fail/enum-representation/rename-as-int-parse-error.rs @@ -0,0 +1,19 @@ +// Copyright 2017 Serde Developers +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#[macro_use] +extern crate serde_derive; + +#[derive(Serialize)] //~ ERROR: proc-macro derive panicked +#[serde(tag = "type")] +enum E { + #[serde(rename = "foo", rename_as = "int")] //~^^^ HELP: failed to parse "foo" as int + Newtype(u8), +} + +fn main() {} diff --git a/test_suite/tests/compile-fail/enum-representation/renamed-as-wrong.rs b/test_suite/tests/compile-fail/enum-representation/renamed-as-wrong.rs new file mode 100644 index 000000000..80ebff2dd --- /dev/null +++ b/test_suite/tests/compile-fail/enum-representation/renamed-as-wrong.rs @@ -0,0 +1,19 @@ +// Copyright 2017 Serde Developers +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#[macro_use] +extern crate serde_derive; + +#[derive(Serialize)] //~ ERROR: proc-macro derive panicked +#[serde(tag = "type")] +enum E { + #[serde(rename = "3.3", rename_as = "float")] //~^^^ HELP: unknown rename type for #[serde(rename_as = "float")]. expected "int" or "bool" + Newtype(u8), +} + +fn main() {} diff --git a/test_suite/tests/compile-fail/enum-representation/renamed-literal-and-renamed-as.rs b/test_suite/tests/compile-fail/enum-representation/renamed-literal-and-renamed-as.rs new file mode 100644 index 000000000..7a560dd3f --- /dev/null +++ b/test_suite/tests/compile-fail/enum-representation/renamed-literal-and-renamed-as.rs @@ -0,0 +1,19 @@ +// Copyright 2017 Serde Developers +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#[macro_use] +extern crate serde_derive; + +#[derive(Serialize)] //~ ERROR: proc-macro derive panicked +#[serde(tag = "type")] +enum E { + #[serde(rename = false, rename_as = "bool")] //~^^^ HELP: #[serde(rename = false)] is not compatible with #[serde(rename_as = ...)]. Use a string literal as in #[serde(rename = "false")] + Newtype(u8), +} + +fn main() {} diff --git a/test_suite/tests/test_macros.rs b/test_suite/tests/test_macros.rs index c4c615664..e5431200a 100644 --- a/test_suite/tests/test_macros.rs +++ b/test_suite/tests/test_macros.rs @@ -695,6 +695,47 @@ fn test_internally_tagged_enum() { ); } +#[test] +fn test_internally_tagged_enum_renamed() { + #[derive(Debug, PartialEq, Serialize, Deserialize)] + struct Newtype(BTreeMap); + + #[derive(Debug, PartialEq, Serialize, Deserialize)] + struct Struct { + f: u8, + } + + #[derive(Debug, PartialEq, Serialize, Deserialize)] + #[serde(tag = "type")] + enum InternallyTagged { + #[serde(rename="abc")] + A { a: u8 }, + #[serde(rename="true", rename_as="bool")] + B { b: u8 }, + #[serde(rename="3", rename_as="int")] + C, + D(BTreeMap), + E(Newtype), + F(Struct), + } + + // TODO: Deserialize also + assert_ser_tokens( + &InternallyTagged::A { a: 1 }, + &[ + Token::Struct { name: "InternallyTagged", len: 2 }, + + Token::Str("type"), + Token::Str("abc"), + + Token::Str("a"), + Token::U8(1), + + Token::StructEnd, + ], + ); +} + #[test] fn test_internally_tagged_struct_variant_containing_unit_variant() { #[derive(Debug, PartialEq, Serialize, Deserialize)] From bbabd5be73888783ad56058a5baa402cbc1d95fa Mon Sep 17 00:00:00 2001 From: Phaiax Date: Mon, 3 Jul 2017 17:16:44 +0200 Subject: [PATCH 3/7] Fix #[serde(rename_as)] serialize for non struct variants --- serde/src/private/ser.rs | 58 +++++++++++++++++++++++++++------ test_suite/tests/test_macros.rs | 32 +++++++++++++++++- 2 files changed, 79 insertions(+), 11 deletions(-) diff --git a/serde/src/private/ser.rs b/serde/src/private/ser.rs index 51db594da..b7099b4e7 100644 --- a/serde/src/private/ser.rs +++ b/serde/src/private/ser.rs @@ -19,25 +19,63 @@ pub fn constrain(t: &T) -> &T { t } +pub enum VariantName { + Str(&'static str), + Bool(bool), + Int(u64), +} + +impl From<&'static str> for VariantName { + fn from(src: &'static str) -> VariantName { + VariantName::Str(src) + } +} + +impl<'a> From<&'a bool> for VariantName { + fn from(src: &bool) -> VariantName { + VariantName::Bool(*src) + } +} + +impl<'a> From<&'a u64> for VariantName { + fn from(src: &u64) -> VariantName { + VariantName::Int(*src) + } +} + +impl Serialize for VariantName { + fn serialize(&self, serializer: S) -> Result + where + S: Serializer + { + match self { + &VariantName::Str(s) => serializer.serialize_str(s), + &VariantName::Bool(b) => serializer.serialize_bool(b), + &VariantName::Int(i) => serializer.serialize_u64(i), + } + } +} + /// Not public API. -pub fn serialize_tagged_newtype( +pub fn serialize_tagged_newtype( serializer: S, type_ident: &'static str, variant_ident: &'static str, tag: &'static str, - variant_name: &'static str, + variant_name: U, value: &T, ) -> Result where S: Serializer, T: Serialize, + U: Into { value.serialize( TaggedSerializer { type_ident: type_ident, variant_ident: variant_ident, tag: tag, - variant_name: variant_name, + variant_name: variant_name.into(), delegate: serializer, }, ) @@ -47,7 +85,7 @@ struct TaggedSerializer { type_ident: &'static str, variant_ident: &'static str, tag: &'static str, - variant_name: &'static str, + variant_name: VariantName, delegate: S, } @@ -209,7 +247,7 @@ where inner_variant: &'static str, ) -> Result { let mut map = try!(self.delegate.serialize_map(Some(2))); - try!(map.serialize_entry(self.tag, self.variant_name)); + try!(map.serialize_entry(self.tag, &self.variant_name)); try!(map.serialize_entry(inner_variant, &())); map.end() } @@ -236,7 +274,7 @@ where T: Serialize, { let mut map = try!(self.delegate.serialize_map(Some(2))); - try!(map.serialize_entry(self.tag, self.variant_name)); + try!(map.serialize_entry(self.tag, &self.variant_name)); try!(map.serialize_entry(inner_variant, inner_value)); map.end() } @@ -279,14 +317,14 @@ where len: usize, ) -> Result { let mut map = try!(self.delegate.serialize_map(Some(2))); - try!(map.serialize_entry(self.tag, self.variant_name)); + try!(map.serialize_entry(self.tag, &self.variant_name)); try!(map.serialize_key(inner_variant)); Ok(SerializeTupleVariantAsMapValue::new(map, inner_variant, len),) } fn serialize_map(self, len: Option) -> Result { let mut map = try!(self.delegate.serialize_map(len.map(|len| len + 1))); - try!(map.serialize_entry(self.tag, self.variant_name)); + try!(map.serialize_entry(self.tag, &self.variant_name)); Ok(map) } @@ -296,7 +334,7 @@ where len: usize, ) -> Result { let mut state = try!(self.delegate.serialize_struct(name, len + 1)); - try!(state.serialize_field(self.tag, self.variant_name)); + try!(state.serialize_field(self.tag, &self.variant_name)); Ok(state) } @@ -322,7 +360,7 @@ where len: usize, ) -> Result { let mut map = try!(self.delegate.serialize_map(Some(2))); - try!(map.serialize_entry(self.tag, self.variant_name)); + try!(map.serialize_entry(self.tag, &self.variant_name)); try!(map.serialize_key(inner_variant)); Ok(SerializeStructVariantAsMapValue::new(map, inner_variant, len),) } diff --git a/test_suite/tests/test_macros.rs b/test_suite/tests/test_macros.rs index e5431200a..542481a6f 100644 --- a/test_suite/tests/test_macros.rs +++ b/test_suite/tests/test_macros.rs @@ -714,13 +714,16 @@ fn test_internally_tagged_enum_renamed() { B { b: u8 }, #[serde(rename="3", rename_as="int")] C, + #[serde(rename="4", rename_as="int")] D(BTreeMap), + #[serde(rename="5", rename_as="int")] E(Newtype), + #[serde(rename="6", rename_as="int")] F(Struct), } // TODO: Deserialize also - assert_ser_tokens( + assert_tokens( &InternallyTagged::A { a: 1 }, &[ Token::Struct { name: "InternallyTagged", len: 2 }, @@ -734,6 +737,33 @@ fn test_internally_tagged_enum_renamed() { Token::StructEnd, ], ); + + assert_ser_tokens( + &InternallyTagged::B { b: 1 }, + &[ + Token::Struct { name: "InternallyTagged", len: 2 }, + + Token::Str("type"), + Token::Bool(true), + + Token::Str("b"), + Token::U8(1), + + Token::StructEnd, + ], + ); + + assert_ser_tokens( + &InternallyTagged::C, + &[ + Token::Struct { name: "InternallyTagged", len: 1 }, + + Token::Str("type"), + Token::U64(3), + + Token::StructEnd, + ], + ); } #[test] From face89c4c328881fa0927a7ea971da8e573dff39 Mon Sep 17 00:00:00 2001 From: Phaiax Date: Mon, 3 Jul 2017 22:23:05 +0200 Subject: [PATCH 4/7] Deserialize with #[serde(rename=bool|int)] --- serde/src/export.rs | 39 +++++++++ serde_derive/src/de.rs | 137 ++++++++++++++++++++++---------- serde_derive/src/fragment.rs | 33 +++++++- test_suite/tests/test_macros.rs | 62 ++++++++++++++- 4 files changed, 221 insertions(+), 50 deletions(-) diff --git a/serde/src/export.rs b/serde/src/export.rs index cb416d9f0..1d32580a3 100644 --- a/serde/src/export.rs +++ b/serde/src/export.rs @@ -15,6 +15,8 @@ pub use lib::option::Option::{self, None, Some}; pub use lib::result::Result::{self, Ok, Err}; pub use self::string::from_utf8_lossy; +pub use self::string::from_int; +pub use self::string::from_bool; mod string { use lib::*; @@ -38,4 +40,41 @@ mod string { // UTF-8. str::from_utf8(bytes).unwrap_or("\u{fffd}\u{fffd}\u{fffd}") } + + pub fn from_bool(b : bool) -> &'static str { + match b { + true => "true", + false => "false", + } + } + + #[cfg(any(feature = "std", feature = "alloc"))] + pub fn from_int(i: u64) -> Vec { + format!("{}", i).into_bytes() + } + + #[cfg(not(any(feature = "std", feature = "alloc")))] + pub fn from_int(i: u64) -> [u8; 20] { + let buf = [0; 20]; + let wrap = Wrapper { buf: &buf }; + write!(wrap, "{}", i); + buf + } + + #[cfg(not(any(feature = "std", feature = "alloc")))] + struct Wrapper<'a> { + buf: &'a mut [u8], + } + + #[cfg(not(any(feature = "std", feature = "alloc")))] + impl<'a> fmt::Write for Wrapper<'a> { + // Could panic if buf is too small. + fn write_str(&mut self, s: &str) -> fmt::Result { + let bytes = s.as_bytes(); + self.buf[..bytes.len()].copy_from_slice(bytes); + let this : &mut[u8] = mem::replace(&mut self.buf, &mut []); + self.buf = &mut this[bytes.len()..]; + Ok(()) + } + } } diff --git a/serde_derive/src/de.rs b/serde_derive/src/de.rs index 2a2b41567..21e295afc 100644 --- a/serde_derive/src/de.rs +++ b/serde_derive/src/de.rs @@ -602,11 +602,7 @@ fn deserialize_externally_tagged_enum( .iter() .enumerate() .filter(|&(_, variant)| !variant.attrs.skip_deserializing()) - .map(|(i, variant)| (match variant.attrs.name().deserialize_name() { - attr::StrBoolInt::Str(s) => s, - _ => unreachable!(), - }, - field_i(i)),) + .map(|(i, variant)| (StrBoolInt::from(variant.attrs.name().deserialize_name()), field_i(i))) .collect(); let variants_stmt = { @@ -711,7 +707,7 @@ fn deserialize_internally_tagged_enum( // TODO: just to keep the compiler quiet for now, this must not be strinfied here! let variant_names_idents = variant_names_idents.iter().map(|&(ref variant, ref ident)| - (variant.stringify(), ident.clone())) + (variant.clone(), ident.clone())) .collect(); let variant_visitor = Stmts(deserialize_generated_identifier(variant_names_idents, cattrs, true),); @@ -774,11 +770,6 @@ fn deserialize_adjacently_tagged_enum( } }; - // TODO: just to keep the compiler quiet for now, this must not be strinfied here! - let variant_names_idents = variant_names_idents.iter().map(|&(ref variant, ref ident)| - (variant.stringify(), ident.clone())) - .collect(); - let variant_visitor = Stmts(deserialize_generated_identifier(variant_names_idents, cattrs, true),); let ref variant_arms: Vec<_> = variants @@ -1252,7 +1243,7 @@ fn deserialize_untagged_newtype_variant( } fn deserialize_generated_identifier( - fields: Vec<(String, Ident)>, + fields: Vec<(StrBoolInt, Ident)>, cattrs: &attr::Container, is_variant: bool, ) -> Fragment { @@ -1352,12 +1343,6 @@ fn deserialize_custom_identifier( Some(fields) }; - // TODO: just to keep the compiler quiet for now, this must not be strinfied here! - let names_idents : Vec<_> = names_idents.iter().map(|&(ref variant, ref ident)| - (variant.stringify(), ident.clone())) - .collect(); - - let (de_impl_generics, de_ty_generics, ty_generics, where_clause) = split_with_de_lifetime(params,); let visitor_impl = Stmts(deserialize_identifier(this.clone(), &names_idents, is_variant, fallthrough),); @@ -1386,14 +1371,29 @@ fn deserialize_custom_identifier( fn deserialize_identifier( this: Tokens, - fields: &[(String, Ident)], + fields: &[(StrBoolInt, Ident)], is_variant: bool, fallthrough: Option, ) -> Fragment { - let field_strs = fields.iter().map(|&(ref name, _)| name); - let field_bytes = fields.iter().map(|&(ref name, _)| quote::ByteStr(name)); - - let constructors: &Vec<_> = &fields + let field_strs = fields.iter().filter_map(|&(ref n, _)| n.as_str()); + let field_bytes = fields.iter().filter_map(|&(ref n, _)| n.as_str().map(|n| quote::ByteStr(n))); + let field_ints = fields.iter().filter_map(|&(ref n, _)| n.as_int()); + let field_bools = fields.iter().filter_map(|&(ref n, _)| n.as_bool()); + + let constructors_strs : &Vec<_> = &fields.iter() + .filter_map(|&(ref n, ref i)| n.as_str().map(|_| i)) + .map(|i| quote!(#this::#i)) + .collect(); + let constructors_bools : &Vec<_> = &fields.iter() + .filter_map(|&(ref n, ref i)| n.as_bool().map(|_| i)) + .map(|i| quote!(#this::#i)) + .collect(); + let constructors_ints : &Vec<_> = &fields.iter() + .filter_map(|&(ref n, ref i)| n.as_int().map(|_| i)) + .map(|i| quote!(#this::#i)) + .collect(); + + let constructors_index: &Vec<_> = &fields .iter() .map(|&(_, ref ident)| quote!(#this::#ident)) .collect(); @@ -1404,7 +1404,36 @@ fn deserialize_identifier( "field identifier" }; - let visit_index = if is_variant { + + let (bytes_to_str, int_to_str, bool_to_str) = if fallthrough.is_some() { + (None, None, None) + } else { + let conversion_bytes = quote! { + let __value = &_serde::export::from_utf8_lossy(__value); + }; + let conversion_int = quote! { + let __value = &_serde::export::from_int(__value); + let __value = &_serde::export::from_utf8_lossy(__value); + }; + let conversion_bool = quote! { + let __value = _serde::export::from_bool(__value); + }; + (Some(conversion_bytes), Some(conversion_int), Some(conversion_bool)) + }; + + let fallthrough_arm = if let Some(fallthrough) = fallthrough { + fallthrough + } else if is_variant { + quote! { + _serde::export::Err(_serde::de::Error::unknown_variant(__value, VARIANTS)) + } + } else { + quote! { + _serde::export::Err(_serde::de::Error::unknown_field(__value, FIELDS)) + } + }; + + let visit_int = if constructors_ints.is_empty() && is_variant { let variant_indices = 0u32..; let fallthrough_msg = format!("variant index 0 <= i < {}", fields.len()); let visit_index = quote! { @@ -1413,7 +1442,7 @@ fn deserialize_identifier( { match __value { #( - #variant_indices => _serde::export::Ok(#constructors), + #variant_indices => _serde::export::Ok(#constructors_index), )* _ => _serde::export::Err(_serde::de::Error::invalid_value( _serde::de::Unexpected::Unsigned(__value as u64), @@ -1422,44 +1451,64 @@ fn deserialize_identifier( } }; Some(visit_index) + } else if ! constructors_ints.is_empty() { + let visit_int = quote! { + fn visit_u64<__E>(self, __value: u64) -> _serde::export::Result + where __E: _serde::de::Error + { + match __value { + #( + #field_ints => _serde::export::Ok(#constructors_ints), + )* + _ => { + #int_to_str + #fallthrough_arm + } + } + } + }; + Some(visit_int) } else { None }; - let bytes_to_str = if fallthrough.is_some() { + let visit_bool = if constructors_bools.is_empty() { None } else { - let conversion = quote! { - let __value = &_serde::export::from_utf8_lossy(__value); + let visit_bool = quote! { + fn visit_bool<__E>(self, __value: bool) -> _serde::export::Result + where __E: _serde::de::Error + { + match __value { + #( + #field_bools => _serde::export::Ok(#constructors_bools), + )* + _ => { + #bool_to_str + #fallthrough_arm + } + } + } }; - Some(conversion) + Some(visit_bool) }; - let fallthrough_arm = if let Some(fallthrough) = fallthrough { - fallthrough - } else if is_variant { - quote! { - _serde::export::Err(_serde::de::Error::unknown_variant(__value, VARIANTS)) - } - } else { - quote! { - _serde::export::Err(_serde::de::Error::unknown_field(__value, FIELDS)) - } - }; quote_block! { fn expecting(&self, formatter: &mut _serde::export::Formatter) -> _serde::export::fmt::Result { _serde::export::Formatter::write_str(formatter, #expecting) } - #visit_index + #visit_int + + #visit_bool fn visit_str<__E>(self, __value: &str) -> _serde::export::Result where __E: _serde::de::Error { match __value { #( - #field_strs => _serde::export::Ok(#constructors), + #field_strs => _serde::export::Ok(#constructors_strs), )* _ => #fallthrough_arm } @@ -1470,7 +1519,7 @@ fn deserialize_identifier( { match __value { #( - #field_bytes => _serde::export::Ok(#constructors), + #field_bytes => _serde::export::Ok(#constructors_strs), )* _ => { #bytes_to_str @@ -1491,7 +1540,7 @@ fn deserialize_struct_visitor( .iter() .enumerate() .filter(|&(_, field)| !field.attrs.skip_deserializing()) - .map(|(i, field)| (field.attrs.name().deserialize_name(), field_i(i)),) + .map(|(i, field)| (StrBoolInt::from(field.attrs.name().deserialize_name()), field_i(i)),) .collect(); let fields_stmt = { diff --git a/serde_derive/src/fragment.rs b/serde_derive/src/fragment.rs index 67ad58108..6b74b40f0 100644 --- a/serde_derive/src/fragment.rs +++ b/serde_derive/src/fragment.rs @@ -99,9 +99,30 @@ impl StrBoolInt { pub fn stringify(&self) -> String { match *self { StrBoolInt::Str(ref s) => s.clone(), - StrBoolInt::Bool(true) => "bool: true".to_owned(), - StrBoolInt::Bool(false) => "bool: false".to_owned(), - StrBoolInt::Int(i) => format!("integer: {}", i), + StrBoolInt::Bool(true) => "true".to_owned(), + StrBoolInt::Bool(false) => "false".to_owned(), + StrBoolInt::Int(i) => format!("{}", i), + } + } + + pub fn as_str(&self) -> Option<&str> { + match *self { + StrBoolInt::Str(ref s) => Some(&s), + _ => None, + } + } + + pub fn as_int(&self) -> Option { + match *self { + StrBoolInt::Int(i) => Some(i), + _ => None, + } + } + + pub fn as_bool(&self) -> Option { + match *self { + StrBoolInt::Bool(b) => Some(b), + _ => None, } } } @@ -116,6 +137,12 @@ impl From for StrBoolInt { } } +impl From for StrBoolInt { + fn from(src: String) -> StrBoolInt { + StrBoolInt::Str(src) + } +} + impl ToTokens for StrBoolInt { fn to_tokens(&self, out: &mut Tokens) { match *self { diff --git a/test_suite/tests/test_macros.rs b/test_suite/tests/test_macros.rs index 542481a6f..ee941ec35 100644 --- a/test_suite/tests/test_macros.rs +++ b/test_suite/tests/test_macros.rs @@ -722,7 +722,6 @@ fn test_internally_tagged_enum_renamed() { F(Struct), } - // TODO: Deserialize also assert_tokens( &InternallyTagged::A { a: 1 }, &[ @@ -738,7 +737,7 @@ fn test_internally_tagged_enum_renamed() { ], ); - assert_ser_tokens( + assert_tokens( &InternallyTagged::B { b: 1 }, &[ Token::Struct { name: "InternallyTagged", len: 2 }, @@ -753,7 +752,7 @@ fn test_internally_tagged_enum_renamed() { ], ); - assert_ser_tokens( + assert_tokens( &InternallyTagged::C, &[ Token::Struct { name: "InternallyTagged", len: 1 }, @@ -764,6 +763,63 @@ fn test_internally_tagged_enum_renamed() { Token::StructEnd, ], ); + + assert_tokens( + &InternallyTagged::D(BTreeMap::new()), + &[ + Token::Map { len: Some(1) }, + + Token::Str("type"), + Token::U64(4), + + Token::MapEnd, + ], + ); + + assert_tokens( + &InternallyTagged::E(Newtype(BTreeMap::new())), + &[ + Token::Map { len: Some(1) }, + + Token::Str("type"), + Token::U64(5), + + Token::MapEnd, + ], + ); + + assert_tokens( + &InternallyTagged::F(Struct { f: 6 }), + &[ + Token::Struct { name: "Struct", len: 2 }, + + Token::Str("type"), + Token::U64(6), + + Token::Str("f"), + Token::U8(6), + + Token::StructEnd, + ], + ); + + assert_de_tokens_error::( + &[ + Token::Map { len: Some(1) }, + Token::Str("type"), + Token::Bool(false) + ], + "unknown variant `false`, expected one of `abc`, `true`, `3`, `4`, `5`, `6`", + ); + + assert_de_tokens_error::( + &[ + Token::Struct { name: "Struct", len: 2 }, + Token::Str("type"), + Token::U8(9), + ], + "unknown variant `9`, expected one of `abc`, `true`, `3`, `4`, `5`, `6`", + ); } #[test] From f1cd5a27b1e0ba4e899b77d6fade605070682149 Mon Sep 17 00:00:00 2001 From: Phaiax Date: Tue, 4 Jul 2017 13:05:46 +0200 Subject: [PATCH 5/7] Add tests for adjacently tagged enum rename --- serde_derive/src/de.rs | 5 -- serde_derive/src/ser.rs | 2 + test_suite/tests/test_macros.rs | 129 ++++++++++++++++++++++++++++++++ 3 files changed, 131 insertions(+), 5 deletions(-) diff --git a/serde_derive/src/de.rs b/serde_derive/src/de.rs index 21e295afc..304c0ebd4 100644 --- a/serde_derive/src/de.rs +++ b/serde_derive/src/de.rs @@ -705,11 +705,6 @@ fn deserialize_internally_tagged_enum( } }; - // TODO: just to keep the compiler quiet for now, this must not be strinfied here! - let variant_names_idents = variant_names_idents.iter().map(|&(ref variant, ref ident)| - (variant.clone(), ident.clone())) - .collect(); - let variant_visitor = Stmts(deserialize_generated_identifier(variant_names_idents, cattrs, true),); // Match arms to extract a variant from a string diff --git a/serde_derive/src/ser.rs b/serde_derive/src/ser.rs index 1b361fa13..08b73f528 100644 --- a/serde_derive/src/ser.rs +++ b/serde_derive/src/ser.rs @@ -542,6 +542,8 @@ fn serialize_adjacently_tagged_variant( serialize_tuple_variant(TupleVariant::Untagged, params, &variant.fields) } Style::Struct => { + // TODO: stringify variant name? See test_adjacently_tagged_enum_renamed() + // AdjacentlyTagged::B for consequences. let str_variant_name = variant_name.stringify(); serialize_struct_variant( StructVariant::Untagged, diff --git a/test_suite/tests/test_macros.rs b/test_suite/tests/test_macros.rs index ee941ec35..7aa913661 100644 --- a/test_suite/tests/test_macros.rs +++ b/test_suite/tests/test_macros.rs @@ -822,6 +822,135 @@ fn test_internally_tagged_enum_renamed() { ); } +#[test] +fn test_adjacently_tagged_enum_renamed() { + #[derive(Debug, PartialEq, Serialize, Deserialize)] + struct Newtype(BTreeMap); + + #[derive(Debug, PartialEq, Serialize, Deserialize)] + struct Struct { + f: u8, + } + + #[derive(Debug, PartialEq, Serialize, Deserialize)] + #[serde(tag = "t", content = "c")] + enum AdjacentlyTagged { + #[serde(rename="abc")] + A { a: u8 }, + #[serde(rename="true", rename_as="bool")] + B { b: u8 }, + #[serde(rename="3", rename_as="int")] + C, + #[serde(rename="4", rename_as="int")] + D(BTreeMap), + #[serde(rename="5", rename_as="int")] + E(Newtype), + #[serde(rename="6", rename_as="int")] + F(Struct), + } + + assert_tokens( + &AdjacentlyTagged::A { a: 1 }, + &[ + Token::Struct { name: "AdjacentlyTagged", len: 2 }, + + Token::Str("t"), + Token::Str("abc"), + + Token::Str("c"), + Token::Struct { name: "abc", len: 1 }, + Token::Str("a"), + Token::U8(1), + Token::StructEnd, + + Token::StructEnd, + ], + ); + + assert_tokens( + &AdjacentlyTagged::B { b: 1 }, + &[ + Token::Struct { name: "AdjacentlyTagged", len: 2 }, + + Token::Str("t"), + Token::Bool(true), + + Token::Str("c"), + // TODO: Is this the correct way to name the struct? stringify the rename? + Token::Struct { name: "true", len: 1 }, + Token::Str("b"), + Token::U8(1), + Token::StructEnd, + + Token::StructEnd, + ], + ); + + assert_tokens( + &AdjacentlyTagged::C, + &[ + Token::Struct { name: "AdjacentlyTagged", len: 1 }, + + Token::Str("t"), + Token::U64(3), + + Token::StructEnd, + ], + ); + + assert_tokens( + &AdjacentlyTagged::D(BTreeMap::new()), + &[ + Token::Struct { name: "AdjacentlyTagged", len: 2 }, + + Token::Str("t"), + Token::U64(4), + + Token::Str("c"), + Token::Map { len: Some(0) }, + Token::MapEnd, + + Token::StructEnd, + ], + ); + + assert_tokens( + &AdjacentlyTagged::E(Newtype(BTreeMap::new())), + &[ + Token::Struct { name: "AdjacentlyTagged", len: 2 }, + + Token::Str("t"), + Token::U64(5), + + Token::Str("c"), + Token::NewtypeStruct { name: "Newtype" }, + Token::Map { len: Some(0) }, + Token::MapEnd, + + Token::StructEnd, + ], + ); + + assert_tokens( + &AdjacentlyTagged::F(Struct { f: 6 }), + &[ + Token::Struct { name: "AdjacentlyTagged", len: 2 }, + + Token::Str("t"), + Token::U64(6), + + Token::Str("c"), + Token::Struct { name: "Struct", len: 1 }, + Token::Str("f"), + Token::U8(6), + Token::StructEnd, + + Token::StructEnd, + ], + ); + +} + #[test] fn test_internally_tagged_struct_variant_containing_unit_variant() { #[derive(Debug, PartialEq, Serialize, Deserialize)] From 8f1d86c9a6f9d16fb1313d1fcbedc0e421acf785 Mon Sep 17 00:00:00 2001 From: Phaiax Date: Tue, 4 Jul 2017 13:38:49 +0200 Subject: [PATCH 6/7] Fix no_std Bugs, clippy errors (#[serde(rename=int|bool)]) --- serde/src/export.rs | 26 ++++++++++++++++---------- serde/src/private/ser.rs | 8 ++++---- 2 files changed, 20 insertions(+), 14 deletions(-) diff --git a/serde/src/export.rs b/serde/src/export.rs index 1d32580a3..7cc9f7b31 100644 --- a/serde/src/export.rs +++ b/serde/src/export.rs @@ -14,9 +14,7 @@ pub use lib::marker::PhantomData; pub use lib::option::Option::{self, None, Some}; pub use lib::result::Result::{self, Ok, Err}; -pub use self::string::from_utf8_lossy; -pub use self::string::from_int; -pub use self::string::from_bool; +pub use self::string::{from_utf8_lossy, from_int, from_bool}; mod string { use lib::*; @@ -42,22 +40,30 @@ mod string { } pub fn from_bool(b : bool) -> &'static str { - match b { - true => "true", - false => "false", + if b { + "true" + } else { + "false" } } #[cfg(any(feature = "std", feature = "alloc"))] pub fn from_int(i: u64) -> Vec { - format!("{}", i).into_bytes() + use lib::fmt::Write; + let mut buf = String::with_capacity(20); + write!(&mut buf, "{}", i).ok(); + buf.into_bytes() } #[cfg(not(any(feature = "std", feature = "alloc")))] pub fn from_int(i: u64) -> [u8; 20] { - let buf = [0; 20]; - let wrap = Wrapper { buf: &buf }; - write!(wrap, "{}", i); + use lib::fmt::Write; + // len(str(1<<64)) = 20 + let mut buf = [0; 20]; + { + let mut wrap = Wrapper { buf: &mut buf }; + write!(wrap, "{}", i).ok(); + } buf } diff --git a/serde/src/private/ser.rs b/serde/src/private/ser.rs index b7099b4e7..3a9839449 100644 --- a/serde/src/private/ser.rs +++ b/serde/src/private/ser.rs @@ -48,10 +48,10 @@ impl Serialize for VariantName { where S: Serializer { - match self { - &VariantName::Str(s) => serializer.serialize_str(s), - &VariantName::Bool(b) => serializer.serialize_bool(b), - &VariantName::Int(i) => serializer.serialize_u64(i), + match *self { + VariantName::Str(s) => serializer.serialize_str(s), + VariantName::Bool(b) => serializer.serialize_bool(b), + VariantName::Int(i) => serializer.serialize_u64(i), } } } From ea177de57152163c1a46664e9e325a7f4aaee189 Mon Sep 17 00:00:00 2001 From: Phaiax Date: Tue, 4 Jul 2017 16:27:06 +0200 Subject: [PATCH 7/7] Fix no_std libc error #966. Use cargos libc and not #[feature(libc)]. --- test_suite/no_std/Cargo.toml | 1 + test_suite/no_std/src/main.rs | 15 +-------------- 2 files changed, 2 insertions(+), 14 deletions(-) diff --git a/test_suite/no_std/Cargo.toml b/test_suite/no_std/Cargo.toml index caef3d1c6..2bee170aa 100644 --- a/test_suite/no_std/Cargo.toml +++ b/test_suite/no_std/Cargo.toml @@ -6,3 +6,4 @@ publish = false [dependencies] serde = { path = "../../serde", default-features = false } serde_derive = { path = "../../serde_derive" } +libc = "*" \ No newline at end of file diff --git a/test_suite/no_std/src/main.rs b/test_suite/no_std/src/main.rs index a4469819c..d921fa6e2 100644 --- a/test_suite/no_std/src/main.rs +++ b/test_suite/no_std/src/main.rs @@ -1,4 +1,4 @@ -#![feature(lang_items, start, libc, compiler_builtins_lib)] +#![feature(lang_items, start, compiler_builtins_lib)] #![no_std] extern crate libc; @@ -9,23 +9,10 @@ fn start(_argc: isize, _argv: *const *const u8) -> isize { 0 } -#[lang = "eh_personality"] -#[no_mangle] -pub extern fn rust_eh_personality() {} - #[lang = "eh_unwind_resume"] #[no_mangle] pub extern fn rust_eh_unwind_resume() {} -#[lang = "panic_fmt"] -#[no_mangle] -pub extern fn rust_begin_panic(_msg: core::fmt::Arguments, - _file: &'static str, - _line: u32) -> ! { - unsafe { - libc::abort() - } -} //////////////////////////////////////////////////////////////////////////////