From 23c14e5f335107bde3e14aadfadc118243e5fae3 Mon Sep 17 00:00:00 2001 From: Mingun Date: Thu, 22 Oct 2020 23:50:48 +0500 Subject: [PATCH 1/2] Allow to run assert_de_tokens_error on token sequence that is not expected by enum deserializer Before that fix following code panics, because `Token::Unit` was unexpected by test deserializer: ``` #[derive(Deserialize)] enum E { ... } assert_de_tokens_error::(&[Token::Unit], "..."); ``` --- serde_test/src/de.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/serde_test/src/de.rs b/serde_test/src/de.rs index c9bcee5b3..a1d62758e 100644 --- a/serde_test/src/de.rs +++ b/serde_test/src/de.rs @@ -250,9 +250,7 @@ impl<'de, 'a> de::Deserializer<'de> for &'a mut Deserializer<'de> { { visitor.visit_enum(DeserializerEnumVisitor { de: self }) } - _ => { - unexpected!(self.next_token()); - } + _ => self.deserialize_any(visitor) } } From 104ad9a7dd7bd0f9fd55202def4579e0de9f13c8 Mon Sep 17 00:00:00 2001 From: Mingun Date: Fri, 23 Oct 2020 00:54:13 +0500 Subject: [PATCH 2/2] Allow to define custom expectation message for type with `#[serde(expecting = "...")]` Closes #1883 --- serde/src/private/de.rs | 6 +- serde_derive/src/de.rs | 22 ++- serde_derive/src/internals/attr.rs | 17 +++ serde_derive/src/internals/symbol.rs | 1 + test_suite/tests/test_annotations.rs | 197 +++++++++++++++++++++++++++ test_suite/tests/test_gen.rs | 49 +++++++ 6 files changed, 287 insertions(+), 5 deletions(-) diff --git a/serde/src/private/de.rs b/serde/src/private/de.rs index bcb964a9c..e3b685e88 100644 --- a/serde/src/private/de.rs +++ b/serde/src/private/de.rs @@ -824,15 +824,17 @@ mod content { /// Not public API. pub struct TaggedContentVisitor<'de, T> { tag_name: &'static str, + expecting: &'static str, value: PhantomData>, } impl<'de, T> TaggedContentVisitor<'de, T> { /// Visitor for the content of an internally tagged enum with the given /// tag name. - pub fn new(name: &'static str) -> Self { + pub fn new(name: &'static str, expecting: &'static str) -> Self { TaggedContentVisitor { tag_name: name, + expecting: expecting, value: PhantomData, } } @@ -861,7 +863,7 @@ mod content { type Value = TaggedContent<'de, T>; fn expecting(&self, fmt: &mut fmt::Formatter) -> fmt::Result { - fmt.write_str("internally tagged enum") + fmt.write_str(self.expecting) } fn visit_seq(self, mut seq: S) -> Result diff --git a/serde_derive/src/de.rs b/serde_derive/src/de.rs index 1f5733a6d..071db57bc 100644 --- a/serde_derive/src/de.rs +++ b/serde_derive/src/de.rs @@ -399,6 +399,7 @@ fn deserialize_unit_struct(params: &Parameters, cattrs: &attr::Container) -> Fra let type_name = cattrs.name().deserialize_name(); let expecting = format!("unit struct {}", params.type_name()); + let expecting = cattrs.expecting().unwrap_or(&expecting); quote_block! { struct __Visitor; @@ -456,6 +457,7 @@ fn deserialize_tuple( Some(variant_ident) => format!("tuple variant {}::{}", params.type_name(), variant_ident), None => format!("tuple struct {}", params.type_name()), }; + let expecting = cattrs.expecting().unwrap_or(&expecting); let nfields = fields.len(); @@ -542,6 +544,7 @@ fn deserialize_tuple_in_place( Some(variant_ident) => format!("tuple variant {}::{}", params.type_name(), variant_ident), None => format!("tuple struct {}", params.type_name()), }; + let expecting = cattrs.expecting().unwrap_or(&expecting); let nfields = fields.len(); @@ -630,6 +633,7 @@ fn deserialize_seq( } else { format!("{} with {} elements", expecting, deserialized_count) }; + let expecting = cattrs.expecting().unwrap_or(&expecting); let mut index_in_seq = 0_usize; let let_values = vars.clone().zip(fields).map(|(var, field)| { @@ -732,6 +736,7 @@ fn deserialize_seq_in_place( } else { format!("{} with {} elements", expecting, deserialized_count) }; + let expecting = cattrs.expecting().unwrap_or(&expecting); let mut index_in_seq = 0usize; let write_values = fields.iter().map(|field| { @@ -907,6 +912,7 @@ fn deserialize_struct( Some(variant_ident) => format!("struct variant {}::{}", params.type_name(), variant_ident), None => format!("struct {}", params.type_name()), }; + let expecting = cattrs.expecting().unwrap_or(&expecting); let visit_seq = Stmts(deserialize_seq( &type_path, params, fields, true, cattrs, &expecting, @@ -1048,6 +1054,7 @@ fn deserialize_struct_in_place( Some(variant_ident) => format!("struct variant {}::{}", params.type_name(), variant_ident), None => format!("struct {}", params.type_name()), }; + let expecting = cattrs.expecting().unwrap_or(&expecting); let visit_seq = Stmts(deserialize_seq_in_place(params, fields, cattrs, &expecting)); @@ -1200,6 +1207,7 @@ fn deserialize_externally_tagged_enum( let type_name = cattrs.name().deserialize_name(); let expecting = format!("enum {}", params.type_name()); + let expecting = cattrs.expecting().unwrap_or(&expecting); let (variants_stmt, variant_visitor) = prepare_enum_variant_enum(variants, cattrs); @@ -1309,6 +1317,9 @@ fn deserialize_internally_tagged_enum( } }); + let expecting = format!("internally tagged enum {}", params.type_name()); + let expecting = cattrs.expecting().unwrap_or(&expecting); + quote_block! { #variant_visitor @@ -1316,7 +1327,7 @@ fn deserialize_internally_tagged_enum( let __tagged = try!(_serde::Deserializer::deserialize_any( __deserializer, - _serde::private::de::TaggedContentVisitor::<__Field>::new(#tag))); + _serde::private::de::TaggedContentVisitor::<__Field>::new(#tag, #expecting))); match __tagged.tag { #(#variant_arms)* @@ -1359,6 +1370,7 @@ fn deserialize_adjacently_tagged_enum( .collect(); let expecting = format!("adjacently tagged enum {}", params.type_name()); + let expecting = cattrs.expecting().unwrap_or(&expecting); let type_name = cattrs.name().deserialize_name(); let deny_unknown_fields = cattrs.deny_unknown_fields(); @@ -1648,6 +1660,7 @@ fn deserialize_untagged_enum( "data did not match any variant of untagged enum {}", params.type_name() ); + let fallthrough_msg = cattrs.expecting().unwrap_or(&fallthrough_msg); quote_block! { let __content = try!(<_serde::private::de::Content as _serde::Deserialize>::deserialize(__deserializer)); @@ -1905,6 +1918,7 @@ fn deserialize_generated_identifier( is_variant, fallthrough, !is_variant && cattrs.has_flatten(), + None, )); let lifetime = if !is_variant && cattrs.has_flatten() { @@ -2012,6 +2026,7 @@ fn deserialize_custom_identifier( is_variant, fallthrough, false, + cattrs.expecting(), )); quote_block! { @@ -2042,6 +2057,7 @@ fn deserialize_identifier( is_variant: bool, fallthrough: Option, collect_other_fields: bool, + expecting: Option<&str>, ) -> Fragment { let mut flat_fields = Vec::new(); for (_, ident, aliases) in fields { @@ -2066,11 +2082,11 @@ fn deserialize_identifier( .map(|(_, ident, _)| quote!(#this::#ident)) .collect(); - let expecting = if is_variant { + let expecting = expecting.unwrap_or(if is_variant { "variant identifier" } else { "field identifier" - }; + }); let index_expecting = if is_variant { "variant" } else { "field" }; diff --git a/serde_derive/src/internals/attr.rs b/serde_derive/src/internals/attr.rs index e6f72dfe7..fc523d16d 100644 --- a/serde_derive/src/internals/attr.rs +++ b/serde_derive/src/internals/attr.rs @@ -223,6 +223,8 @@ pub struct Container { has_flatten: bool, serde_path: Option, is_packed: bool, + /// Error message generated when type can't be deserialized + expecting: Option, } /// Styles of representing an enum. @@ -305,6 +307,7 @@ impl Container { let mut field_identifier = BoolAttr::none(cx, FIELD_IDENTIFIER); let mut variant_identifier = BoolAttr::none(cx, VARIANT_IDENTIFIER); let mut serde_path = Attr::none(cx, CRATE); + let mut expecting = Attr::none(cx, EXPECTING); for meta_item in item .attrs @@ -575,6 +578,13 @@ impl Container { } } + // Parse `#[serde(expecting = "a message")]` + Meta(NameValue(m)) if m.path == EXPECTING => { + if let Ok(s) = get_lit_str(cx, EXPECTING, &m.lit) { + expecting.set(&m.path, s.value()); + } + } + Meta(meta_item) => { let path = meta_item .path() @@ -627,6 +637,7 @@ impl Container { has_flatten: false, serde_path: serde_path.get(), is_packed, + expecting: expecting.get(), } } @@ -702,6 +713,12 @@ impl Container { self.custom_serde_path() .map_or_else(|| Cow::Owned(parse_quote!(_serde)), Cow::Borrowed) } + + /// Error message generated when type can't be deserialized. + /// If `None`, default message will be used + pub fn expecting(&self) -> Option<&str> { + self.expecting.as_ref().map(String::as_ref) + } } fn decide_tag( diff --git a/serde_derive/src/internals/symbol.rs b/serde_derive/src/internals/symbol.rs index 318b81bbb..1fedd2754 100644 --- a/serde_derive/src/internals/symbol.rs +++ b/serde_derive/src/internals/symbol.rs @@ -35,6 +35,7 @@ pub const TRY_FROM: Symbol = Symbol("try_from"); pub const UNTAGGED: Symbol = Symbol("untagged"); pub const VARIANT_IDENTIFIER: Symbol = Symbol("variant_identifier"); pub const WITH: Symbol = Symbol("with"); +pub const EXPECTING: Symbol = Symbol("expecting"); impl PartialEq for Ident { fn eq(&self, word: &Symbol) -> bool { diff --git a/test_suite/tests/test_annotations.rs b/test_suite/tests/test_annotations.rs index 7604fe031..10d81ad4b 100644 --- a/test_suite/tests/test_annotations.rs +++ b/test_suite/tests/test_annotations.rs @@ -2633,3 +2633,200 @@ fn test_flatten_any_after_flatten_struct() { ], ); } + +/// https://github.com/serde-rs/serde/issues/1883 +#[test] +fn test_expecting_message() { + #[derive(Deserialize, PartialEq, Debug)] + #[serde(expecting = "something strange...")] + struct Unit; + + #[derive(Deserialize)] + #[serde(expecting = "something strange...")] + struct Newtype(bool); + + #[derive(Deserialize)] + #[serde(expecting = "something strange...")] + struct Tuple(u32, bool); + + #[derive(Deserialize)] + #[serde(expecting = "something strange...")] + struct Struct { + question: String, + answer: u32, + } + + assert_de_tokens_error::( + &[Token::Str("Unit")], + r#"invalid type: string "Unit", expected something strange..."# + ); + + assert_de_tokens_error::( + &[Token::Str("Newtype")], + r#"invalid type: string "Newtype", expected something strange..."# + ); + + assert_de_tokens_error::( + &[Token::Str("Tuple")], + r#"invalid type: string "Tuple", expected something strange..."# + ); + + assert_de_tokens_error::( + &[Token::Str("Struct")], + r#"invalid type: string "Struct", expected something strange..."# + ); +} + +#[test] +fn test_expecting_message_externally_tagged_enum() { + #[derive(Deserialize)] + #[serde(expecting = "something strange...")] + enum Enum { + ExternallyTagged, + } + + assert_de_tokens_error::( + &[ + Token::Str("ExternallyTagged"), + ], + r#"invalid type: string "ExternallyTagged", expected something strange..."# + ); + + // Check that #[serde(expecting = "...")] doesn't affect variant identifier error message + assert_de_tokens_error::( + &[ + Token::Enum { name: "Enum" }, + Token::Unit, + ], + r#"invalid type: unit value, expected variant identifier"# + ); +} + +#[test] +fn test_expecting_message_internally_tagged_enum() { + #[derive(Deserialize)] + #[serde(tag = "tag")] + #[serde(expecting = "something strange...")] + enum Enum { + InternallyTagged, + } + + assert_de_tokens_error::( + &[ + Token::Str("InternallyTagged"), + ], + r#"invalid type: string "InternallyTagged", expected something strange..."# + ); + + // Check that #[serde(expecting = "...")] doesn't affect variant identifier error message + assert_de_tokens_error::( + &[ + Token::Map { len: None }, + Token::Str("tag"), + Token::Unit, + ], + r#"invalid type: unit value, expected variant identifier"# + ); +} + +#[test] +fn test_expecting_message_adjacently_tagged_enum() { + #[derive(Deserialize)] + #[serde(tag = "tag", content = "content")] + #[serde(expecting = "something strange...")] + enum Enum { + AdjacentlyTagged, + } + + assert_de_tokens_error::( + &[ + Token::Str("AdjacentlyTagged"), + ], + r#"invalid type: string "AdjacentlyTagged", expected something strange..."# + ); + + assert_de_tokens_error::( + &[ + Token::Map { len: None }, + Token::Unit, + ], + r#"invalid type: unit value, expected "tag", "content", or other ignored fields"# + ); + + // Check that #[serde(expecting = "...")] doesn't affect variant identifier error message + assert_de_tokens_error::( + &[ + Token::Map { len: None }, + Token::Str("tag"), + Token::Unit, + ], + r#"invalid type: unit value, expected variant identifier"# + ); +} + +#[test] +fn test_expecting_message_untagged_tagged_enum() { + #[derive(Deserialize)] + #[serde(untagged)] + #[serde(expecting = "something strange...")] + enum Enum { + Untagged, + } + + assert_de_tokens_error::( + &[ + Token::Str("Untagged"), + ], + r#"something strange..."# + ); +} + +#[test] +fn test_expecting_message_identifier_enum() { + #[derive(Deserialize)] + #[serde(field_identifier)] + #[serde(expecting = "something strange...")] + enum FieldEnum { + Field, + } + + #[derive(Deserialize)] + #[serde(variant_identifier)] + #[serde(expecting = "something strange...")] + enum VariantEnum { + Variant, + } + + assert_de_tokens_error::( + &[ + Token::Unit, + ], + r#"invalid type: unit value, expected something strange..."# + ); + + assert_de_tokens_error::( + &[ + Token::Enum { name: "FieldEnum" }, + Token::Str("Unknown"), + Token::None, + ], + r#"invalid type: map, expected something strange..."# + ); + + + assert_de_tokens_error::( + &[ + Token::Unit, + ], + r#"invalid type: unit value, expected something strange..."# + ); + + assert_de_tokens_error::( + &[ + Token::Enum { name: "VariantEnum" }, + Token::Str("Unknown"), + Token::None, + ], + r#"invalid type: map, expected something strange..."# + ); +} diff --git a/test_suite/tests/test_gen.rs b/test_suite/tests/test_gen.rs index d8def742b..b5c12d606 100644 --- a/test_suite/tests/test_gen.rs +++ b/test_suite/tests/test_gen.rs @@ -723,6 +723,55 @@ fn test_gen() { } deriving!(&'a str); + + // https://github.com/serde-rs/serde/issues/1883 + #[derive(Deserialize)] + #[serde(expecting = "a message")] + struct CustomMessageUnit; + + #[derive(Deserialize)] + #[serde(expecting = "a message")] + struct CustomMessageNewtype(bool); + + #[derive(Deserialize)] + #[serde(expecting = "a message")] + struct CustomMessageTuple(u32, bool); + + #[derive(Deserialize)] + #[serde(expecting = "a message")] + struct CustomMessageStruct { + question: String, + answer: u32, + } + + #[derive(Deserialize)] + #[serde(expecting = "a message")] + enum CustomMessageExternallyTaggedEnum {} + + #[derive(Deserialize)] + #[serde(tag = "tag")] + #[serde(expecting = "a message")] + enum CustomMessageInternallyTaggedEnum {} + + #[derive(Deserialize)] + #[serde(tag = "tag", content = "content")] + #[serde(expecting = "a message")] + enum CustomMessageAdjacentlyTaggedEnum {} + + #[derive(Deserialize)] + #[serde(untagged)] + #[serde(expecting = "a message")] + enum CustomMessageUntaggedEnum {} + + #[derive(Deserialize)] + #[serde(field_identifier)] + #[serde(expecting = "a message")] + enum CustomMessageFieldIdentifierEnum {} + + #[derive(Deserialize)] + #[serde(variant_identifier)] + #[serde(expecting = "a message")] + enum CustomMessageVariantIdentifierEnum {} } //////////////////////////////////////////////////////////////////////////