From eb4af0945655a7a0ab4a6b48c40ed39a196a36ff Mon Sep 17 00:00:00 2001 From: Erick Tryzelaar Date: Wed, 18 Mar 2015 07:35:05 -0700 Subject: [PATCH] Simplify the variant deserializer visitor --- benches/bench_enum.rs | 32 +++--------- serde_macros/src/de.rs | 22 ++++---- src/de.rs | 34 +++++-------- src/json/de.rs | 42 ++-------------- src/json/value.rs | 68 +++++++++++++------------ tests/test_de.rs | 57 ++------------------- tests/test_json.rs | 112 ++++++++++++++++++++++------------------- 7 files changed, 131 insertions(+), 236 deletions(-) diff --git a/benches/bench_enum.rs b/benches/bench_enum.rs index a3ec8f27..41ddae83 100644 --- a/benches/bench_enum.rs +++ b/benches/bench_enum.rs @@ -340,20 +340,10 @@ mod deserializer { de::Deserialize::deserialize(self.de) } - fn visit_unit(&mut self) -> Result<(), Error> { - de::Deserialize::deserialize(self.de) - } - - fn visit_seq(&mut self, _visitor: V) -> Result - where V: de::EnumSeqVisitor + fn visit_value(&mut self, visitor: V) -> Result + where V: de::Visitor, { - Err(de::Error::syntax_error()) - } - - fn visit_map(&mut self, _visitor: V) -> Result - where V: de::EnumMapVisitor - { - Err(de::Error::syntax_error()) + de::Deserializer::visit(self.de, visitor) } } @@ -371,20 +361,10 @@ mod deserializer { de::Deserialize::deserialize(self.de) } - fn visit_unit(&mut self) -> Result<(), Error> { - Err(de::Error::syntax_error()) - } - - fn visit_seq(&mut self, mut visitor: V) -> Result - where V: de::EnumSeqVisitor, + fn visit_value(&mut self, mut visitor: V) -> Result + where V: de::Visitor, { - visitor.visit(self) - } - - fn visit_map(&mut self, _visitor: V) -> Result - where V: de::EnumMapVisitor - { - Err(de::Error::syntax_error()) + visitor.visit_seq(self) } } diff --git a/serde_macros/src/de.rs b/serde_macros/src/de.rs index f01d00f2..ea2d68bf 100644 --- a/serde_macros/src/de.rs +++ b/serde_macros/src/de.rs @@ -454,7 +454,7 @@ fn deserialize_variant( match variant.node.kind { ast::TupleVariantKind(ref args) if args.is_empty() => { quote_expr!(cx, { - try!(visitor.visit_unit()); + try!(visitor.visit_value(::serde::de::UnitVisitor)); Ok($type_ident::$variant_ident) }) } @@ -509,17 +509,17 @@ fn deserialize_tuple_variant( quote_expr!(cx, { $visitor_item - impl $generics ::serde::de::EnumSeqVisitor for $visitor_ty $where_clause { + impl $generics ::serde::de::Visitor for $visitor_ty $where_clause { type Value = $ty; - fn visit< - V: ::serde::de::SeqVisitor, - >(&mut self, mut visitor: V) -> Result<$ty, V::Error> { + fn visit_seq<__V>(&mut self, mut visitor: __V) -> Result<$ty, __V::Error> + where __V: ::serde::de::SeqVisitor, + { $visit_seq_expr } } - visitor.visit_seq($visitor_expr) + visitor.visit_value($visitor_expr) }) } @@ -551,17 +551,17 @@ fn deserialize_struct_variant( $visitor_item - impl $generics ::serde::de::EnumMapVisitor for $visitor_ty $where_clause { + impl $generics ::serde::de::Visitor for $visitor_ty $where_clause { type Value = $ty; - fn visit< - V: ::serde::de::MapVisitor, - >(&mut self, mut visitor: V) -> Result<$ty, V::Error> { + fn visit_map<__V>(&mut self, mut visitor: __V) -> Result<$ty, __V::Error> + where __V: ::serde::de::MapVisitor, + { $field_expr } } - visitor.visit_map($visitor_expr) + visitor.visit_value($visitor_expr) }) } diff --git a/src/de.rs b/src/de.rs index 700cb4f8..31d468c8 100644 --- a/src/de.rs +++ b/src/de.rs @@ -15,11 +15,15 @@ pub trait Error { fn missing_field_error(&'static str) -> Self; } +/////////////////////////////////////////////////////////////////////////////// + pub trait Deserialize { fn deserialize(deserializer: &mut D) -> Result where D: Deserializer; } +/////////////////////////////////////////////////////////////////////////////// + pub trait Deserializer { type Error: Error; @@ -50,6 +54,8 @@ pub trait Deserializer { } } +/////////////////////////////////////////////////////////////////////////////// + pub trait Visitor { type Value; @@ -335,13 +341,8 @@ pub trait VariantVisitor { fn visit_variant(&mut self) -> Result where V: Deserialize; - fn visit_unit(&mut self) -> Result<(), Self::Error>; - - fn visit_seq(&mut self, _visitor: V) -> Result - where V: EnumSeqVisitor; - - fn visit_map(&mut self, _visitor: V) -> Result - where V: EnumMapVisitor; + fn visit_value(&mut self, _visitor: V) -> Result + where V: Visitor; } impl<'a, T> VariantVisitor for &'a mut T where T: VariantVisitor { @@ -353,21 +354,10 @@ impl<'a, T> VariantVisitor for &'a mut T where T: VariantVisitor { (**self).visit_variant() } + fn visit_value(&mut self, visitor: V) -> Result + where V: Visitor, { - fn visit_unit(&mut self) -> Result<(), T::Error> { - (**self).visit_unit() - } - - fn visit_seq(&mut self, visitor: V) -> Result - where V: EnumSeqVisitor - { - (**self).visit_seq(visitor) - } - - fn visit_map(&mut self, visitor: V) -> Result - where V: EnumMapVisitor - { - (**self).visit_map(visitor) + (**self).visit_value(visitor) } } @@ -391,7 +381,7 @@ pub trait EnumMapVisitor { /////////////////////////////////////////////////////////////////////////////// -struct UnitVisitor; +pub struct UnitVisitor; impl Visitor for UnitVisitor { type Value = (); diff --git a/src/json/de.rs b/src/json/de.rs index a584598e..038a0f26 100644 --- a/src/json/de.rs +++ b/src/json/de.rs @@ -604,48 +604,12 @@ impl de::VariantVisitor for Deserializer de::Deserialize::deserialize(self) } - /* - fn visit_value(&mut self) -> Result - where V: de::Deserialize - { - de::Deserialize::deserialize(self) - } - */ - - fn visit_unit(&mut self) -> Result<(), Error> { - try!(self.parse_object_colon()); - - de::Deserialize::deserialize(self) - } - - fn visit_seq(&mut self, mut visitor: V) -> Result - where V: de::EnumSeqVisitor + fn visit_value(&mut self, visitor: V) -> Result + where V: de::Visitor, { try!(self.parse_object_colon()); - self.parse_whitespace(); - - if self.ch_is(b'[') { - self.bump(); - visitor.visit(SeqVisitor::new(self)) - } else { - Err(self.error(ErrorCode::ExpectedSomeValue)) - } - } - - fn visit_map(&mut self, mut visitor: V) -> Result - where V: de::EnumMapVisitor - { - try!(self.parse_object_colon()); - - self.parse_whitespace(); - - if self.ch_is(b'{') { - self.bump(); - visitor.visit(MapVisitor::new(self)) - } else { - Err(self.error(ErrorCode::ExpectedSomeValue)) - } + de::Deserializer::visit(self, visitor) } } diff --git a/src/json/value.rs b/src/json/value.rs index 66bd6be4..386225fc 100644 --- a/src/json/value.rs +++ b/src/json/value.rs @@ -443,13 +443,11 @@ impl de::Deserializer for Deserializer { })) } Some((variant, Value::Object(fields))) => { - self.value = Some(Value::String(variant)); - let len = fields.len(); try!(visitor.visit(MapDeserializer { de: self, iter: fields.into_iter(), - value: None, + value: Some(Value::String(variant)), len: len, })) } @@ -470,6 +468,21 @@ struct SeqDeserializer<'a> { len: usize, } +impl<'a> de::Deserializer for SeqDeserializer<'a> { + type Error = Error; + + #[inline] + fn visit(&mut self, mut visitor: V) -> Result + where V: de::Visitor, + { + if self.len == 0 { + visitor.visit_unit() + } else { + visitor.visit_seq(self) + } + } +} + impl<'a> de::SeqVisitor for SeqDeserializer<'a> { type Error = Error; @@ -508,24 +521,10 @@ impl<'a> de::VariantVisitor for SeqDeserializer<'a> { de::Deserialize::deserialize(self.de) } - fn visit_unit(&mut self) -> Result<(), Error> { - if self.len == 0 { - Ok(()) - } else { - Err(de::Error::syntax_error()) - } - } - - fn visit_seq(&mut self, mut visitor: V) -> Result - where V: de::EnumSeqVisitor, + fn visit_value(&mut self, visitor: V) -> Result + where V: de::Visitor, { - visitor.visit(self) - } - - fn visit_map(&mut self, _visitor: V) -> Result - where V: de::EnumMapVisitor - { - Err(de::Error::syntax_error()) + de::Deserializer::visit(self, visitor) } } @@ -599,29 +598,32 @@ impl<'a> de::MapVisitor for MapDeserializer<'a> { } } +impl<'a> de::Deserializer for MapDeserializer<'a> { + type Error = Error; + + #[inline] + fn visit(&mut self, mut visitor: V) -> Result + where V: de::Visitor, + { + println!("MapDeserializer!"); + visitor.visit_map(self) + } +} + impl<'a> de::VariantVisitor for MapDeserializer<'a> { type Error = Error; fn visit_variant(&mut self) -> Result where V: de::Deserialize, { + self.de.value = self.value.take(); de::Deserialize::deserialize(self.de) } - fn visit_unit(&mut self) -> Result<(), Error> { - Err(de::Error::syntax_error()) - } - - fn visit_seq(&mut self, _visitor: V) -> Result - where V: de::EnumSeqVisitor + fn visit_value(&mut self, visitor: V) -> Result + where V: de::Visitor, { - Err(de::Error::syntax_error()) - } - - fn visit_map(&mut self, mut visitor: V) -> Result - where V: de::EnumMapVisitor, - { - visitor.visit(self) + de::Deserializer::visit(self, visitor) } } diff --git a/tests/test_de.rs b/tests/test_de.rs index 8194a00c..ee2232f3 100644 --- a/tests/test_de.rs +++ b/tests/test_de.rs @@ -282,65 +282,16 @@ struct TokenDeserializerVariantVisitor<'a, 'b: 'a> { impl<'a, 'b> de::VariantVisitor for TokenDeserializerVariantVisitor<'a, 'b> { type Error = Error; - fn visit_kind(&mut self) -> Result + fn visit_variant(&mut self) -> Result where V: de::Deserialize, { de::Deserialize::deserialize(self.de) } - fn visit_unit(&mut self) -> Result<(), Error> { - let value = try!(Deserialize::deserialize(self.de)); - - match self.de.tokens.next() { - Some(Token::EnumEnd) => Ok(value), - Some(_) => Err(Error::SyntaxError), - None => Err(Error::EndOfStreamError), - } - } - - fn visit_seq(&mut self, mut visitor: V) -> Result - where V: de::EnumSeqVisitor, + fn visit_value(&mut self, visitor: V) -> Result + where V: de::Visitor, { - let token = self.de.tokens.next(); - match token { - Some(Token::SeqStart(len)) => { - let value = try!(visitor.visit(TokenDeserializerSeqVisitor { - de: self.de, - len: len, - first: true, - })); - - match self.de.tokens.next() { - Some(Token::EnumEnd) => Ok(value), - Some(_) => Err(Error::SyntaxError), - None => Err(Error::EndOfStreamError), - } - } - Some(_) => Err(Error::SyntaxError), - None => Err(Error::EndOfStreamError), - } - } - - fn visit_map(&mut self, mut visitor: V) -> Result - where V: de::EnumMapVisitor, - { - match self.de.tokens.next() { - Some(Token::MapStart(len)) => { - let value = try!(visitor.visit(TokenDeserializerMapVisitor { - de: self.de, - len: len, - first: true, - })); - - match self.de.tokens.next() { - Some(Token::EnumEnd) => Ok(value), - Some(_) => Err(Error::SyntaxError), - None => Err(Error::EndOfStreamError), - } - } - Some(_) => Err(Error::SyntaxError), - None => Err(Error::EndOfStreamError), - } + de::Deserializer::visit(self.de, visitor) } } diff --git a/tests/test_json.rs b/tests/test_json.rs index 28205ceb..02d87478 100644 --- a/tests/test_json.rs +++ b/tests/test_json.rs @@ -28,7 +28,7 @@ macro_rules! treemap { }) } -#[derive(PartialEq, Debug)] +#[derive(Clone, Debug, PartialEq)] #[derive_serialize] #[derive_deserialize] enum Animal { @@ -38,7 +38,7 @@ enum Animal { } -#[derive(PartialEq, Debug)] +#[derive(Clone, Debug, PartialEq)] #[derive_serialize] #[derive_deserialize] struct Inner { @@ -47,7 +47,7 @@ struct Inner { c: Vec, } -#[derive(PartialEq, Debug)] +#[derive(Clone, Debug, PartialEq)] #[derive_serialize] #[derive_deserialize] struct Outer { @@ -628,53 +628,56 @@ fn test_write_option() { ]); } -fn test_parse_ok<'a, T>(errors: &[(&'a str, T)]) - where T: PartialEq + Debug + ser::Serialize + de::Deserialize, +fn test_parse_ok(errors: Vec<(&'static str, T)>) + where T: Clone + Debug + PartialEq + ser::Serialize + de::Deserialize, { - for &(s, ref value) in errors { - let v: T = from_str(s).unwrap(); - assert_eq!(v, *value); + for (s, value) in errors { + let v: Result = from_str(s); + assert_eq!(v, Ok(value.clone())); // Make sure we can deserialize into a `Value`. - let json_value: Value = from_str(s).unwrap(); - assert_eq!(json_value, to_value(&value)); + let json_value: Result = from_str(s); + assert_eq!(json_value, Ok(to_value(&value))); + + let json_value = json_value.unwrap(); + // Make sure we can deserialize from a `Value`. - let v: T = from_value(json_value.clone()).unwrap(); - assert_eq!(v, *value); + let v: Result = from_value(json_value.clone()); + assert_eq!(v, Ok(value)); // Make sure we can round trip back to `Value`. - let json_value2: Value = from_value(json_value.clone()).unwrap(); - assert_eq!(json_value, json_value2); + let json_value2: Result = from_value(json_value.clone()); + assert_eq!(json_value2, Ok(json_value)); } } // FIXME (#5527): these could be merged once UFCS is finished. -fn test_parse_err<'a, T>(errors: &[(&'a str, Error)]) - where T: Debug + de::Deserialize, +fn test_parse_err(errors: Vec<(&'static str, Error)>) + where T: Debug + PartialEq + de::Deserialize, { - for &(s, ref err) in errors { + for (s, err) in errors { let v: Result = from_str(s); - assert_eq!(v.unwrap_err(), *err); + assert_eq!(v, Err(err)); } } #[test] fn test_parse_null() { - test_parse_err::<()>(&[ + test_parse_err::<()>(vec![ ("n", Error::SyntaxError(ErrorCode::ExpectedSomeIdent, 1, 2)), ("nul", Error::SyntaxError(ErrorCode::ExpectedSomeIdent, 1, 4)), ("nulla", Error::SyntaxError(ErrorCode::TrailingCharacters, 1, 5)), ]); - test_parse_ok(&[ + test_parse_ok(vec![ ("null", ()), ]); } #[test] fn test_parse_bool() { - test_parse_err::(&[ + test_parse_err::(vec![ ("t", Error::SyntaxError(ErrorCode::ExpectedSomeIdent, 1, 2)), ("truz", Error::SyntaxError(ErrorCode::ExpectedSomeIdent, 1, 4)), ("f", Error::SyntaxError(ErrorCode::ExpectedSomeIdent, 1, 2)), @@ -683,7 +686,7 @@ fn test_parse_bool() { ("falsea", Error::SyntaxError(ErrorCode::TrailingCharacters, 1, 6)), ]); - test_parse_ok(&[ + test_parse_ok(vec![ ("true", true), (" true ", true), ("false", false), @@ -693,7 +696,7 @@ fn test_parse_bool() { #[test] fn test_parse_number_errors() { - test_parse_err::(&[ + test_parse_err::(vec![ ("+", Error::SyntaxError(ErrorCode::ExpectedSomeValue, 1, 1)), (".", Error::SyntaxError(ErrorCode::ExpectedSomeValue, 1, 1)), ("-", Error::SyntaxError(ErrorCode::InvalidNumber, 1, 2)), @@ -707,7 +710,7 @@ fn test_parse_number_errors() { #[test] fn test_parse_i64() { - test_parse_ok(&[ + test_parse_ok(vec![ ("3", 3), ("-2", -2), ("-1234", -1234), @@ -717,7 +720,7 @@ fn test_parse_i64() { #[test] fn test_parse_f64() { - test_parse_ok(&[ + test_parse_ok(vec![ ("3.0", 3.0f64), ("3.1", 3.1), ("-1.2", -1.2), @@ -731,13 +734,13 @@ fn test_parse_f64() { #[test] fn test_parse_string() { - test_parse_err::(&[ + test_parse_err::(vec![ ("\"", Error::SyntaxError(ErrorCode::EOFWhileParsingString, 1, 2)), ("\"lol", Error::SyntaxError(ErrorCode::EOFWhileParsingString, 1, 5)), ("\"lol\"a", Error::SyntaxError(ErrorCode::TrailingCharacters, 1, 6)), ]); - test_parse_ok(&[ + test_parse_ok(vec![ ("\"\"", "".to_string()), ("\"foo\"", "foo".to_string()), (" \"foo\" ", "foo".to_string()), @@ -753,7 +756,7 @@ fn test_parse_string() { #[test] fn test_parse_list() { - test_parse_err::>(&[ + test_parse_err::>(vec![ ("[", Error::SyntaxError(ErrorCode::EOFWhileParsingValue, 1, 2)), ("[ ", Error::SyntaxError(ErrorCode::EOFWhileParsingValue, 1, 3)), ("[1", Error::SyntaxError(ErrorCode::EOFWhileParsingList, 1, 3)), @@ -763,39 +766,39 @@ fn test_parse_list() { ("[]a", Error::SyntaxError(ErrorCode::TrailingCharacters, 1, 3)), ]); - test_parse_ok(&[ + test_parse_ok(vec![ ("[]", vec![]), ("[ ]", vec![]), ("[null]", vec![()]), (" [ null ] ", vec![()]), ]); - test_parse_ok(&[ + test_parse_ok(vec![ ("[true]", vec![true]), ]); - test_parse_ok(&[ + test_parse_ok(vec![ ("[3,1]", vec![3, 1]), (" [ 3 , 1 ] ", vec![3, 1]), ]); - test_parse_ok(&[ + test_parse_ok(vec![ ("[[3], [1, 2]]", vec![vec![3], vec![1, 2]]), ]); - test_parse_ok(&[ + test_parse_ok(vec![ ("[1]", (1,)), ]); - test_parse_ok(&[ + test_parse_ok(vec![ ("[1, 2]", (1, 2)), ]); - test_parse_ok(&[ + test_parse_ok(vec![ ("[1, 2, 3]", (1, 2, 3)), ]); - test_parse_ok(&[ + test_parse_ok(vec![ ("[1, [2, 3]]", (1, (2, 3))), ]); @@ -805,7 +808,7 @@ fn test_parse_list() { #[test] fn test_parse_object() { - test_parse_err::>(&[ + test_parse_err::>(vec![ ("{", Error::SyntaxError(ErrorCode::EOFWhileParsingValue, 1, 2)), ("{ ", Error::SyntaxError(ErrorCode::EOFWhileParsingValue, 1, 3)), ("{1", Error::SyntaxError(ErrorCode::KeyMustBeAString, 1, 2)), @@ -820,7 +823,7 @@ fn test_parse_object() { ("{}a", Error::SyntaxError(ErrorCode::TrailingCharacters, 1, 3)), ]); - test_parse_ok(&[ + test_parse_ok(vec![ ("{}", treemap!()), ("{ }", treemap!()), ( @@ -841,7 +844,7 @@ fn test_parse_object() { ), ]); - test_parse_ok(&[ + test_parse_ok(vec![ ( "{\"a\": {\"b\": 3, \"c\": 4}}", treemap!("a".to_string() => treemap!("b".to_string() => 3, "c".to_string() => 4)), @@ -851,13 +854,13 @@ fn test_parse_object() { #[test] fn test_parse_struct() { - test_parse_err::(&[ + test_parse_err::(vec![ ("[]", Error::SyntaxError(ErrorCode::ExpectedSomeValue, 0, 0)), ("{}", Error::SyntaxError(ErrorCode::ExpectedSomeValue, 0, 0)), ("{\"inner\": true}", Error::SyntaxError(ErrorCode::ExpectedSomeValue, 0, 0)), ]); - test_parse_ok(&[ + test_parse_ok(vec![ ( "{ \"inner\": [] @@ -883,12 +886,12 @@ fn test_parse_struct() { #[test] fn test_parse_option() { - test_parse_ok(&[ + test_parse_ok(vec![ ("null", None::), ("\"jodhpurs\"", Some("jodhpurs".to_string())), ]); - #[derive(PartialEq, Debug)] + #[derive(Clone, Debug, PartialEq)] #[derive_serialize] #[derive_deserialize] struct Foo { @@ -898,23 +901,28 @@ fn test_parse_option() { let value: Foo = from_str("{}").unwrap(); assert_eq!(value, Foo { x: None }); - test_parse_ok(&[ + test_parse_ok(vec![ ("{\"x\": null}", Foo { x: None }), ("{\"x\": 5}", Foo { x: Some(5) }), ]); } #[test] -fn test_parse_enum() { - test_parse_err::(&[ +fn test_parse_enum_errors() { + test_parse_err::(vec![ ("{}", Error::SyntaxError(ErrorCode::ExpectedSomeValue, 1, 2)), + ("{\"Dog\":", Error::SyntaxError(ErrorCode::EOFWhileParsingValue, 1, 8)), + ("{\"Dog\":}", Error::SyntaxError(ErrorCode::ExpectedSomeValue, 1, 8)), ("{\"unknown\":[]}", Error::SyntaxError(ErrorCode::ExpectedSomeValue, 0, 0)), ("{\"Dog\":{}}", Error::SyntaxError(ErrorCode::ExpectedSomeValue, 0, 0)), - ("{\"Frog\":{}}", Error::SyntaxError(ErrorCode::ExpectedSomeValue, 1, 9)), - ("{\"Cat\":[]}", Error::SyntaxError(ErrorCode::ExpectedSomeValue, 1, 8)), + ("{\"Frog\":{}}", Error::SyntaxError(ErrorCode::ExpectedSomeValue, 0, 0)), + ("{\"Cat\":[]}", Error::SyntaxError(ErrorCode::ExpectedSomeValue, 0, 0)), ]); +} - test_parse_ok(&[ +#[test] +fn test_parse_enum() { + test_parse_ok(vec![ ("{\"Dog\":[]}", Animal::Dog), (" { \"Dog\" : [ ] } ", Animal::Dog), ( @@ -935,7 +943,7 @@ fn test_parse_enum() { ), ]); - test_parse_ok(&[ + test_parse_ok(vec![ ( concat!( "{", @@ -953,7 +961,7 @@ fn test_parse_enum() { #[test] fn test_parse_trailing_whitespace() { - test_parse_ok(&[ + test_parse_ok(vec![ ("[1, 2] ", vec![1, 2]), ("[1, 2]\n", vec![1, 2]), ("[1, 2]\t", vec![1, 2]), @@ -963,7 +971,7 @@ fn test_parse_trailing_whitespace() { #[test] fn test_multiline_errors() { - test_parse_err::>(&[ + test_parse_err::>(vec![ ("{\n \"foo\":\n \"bar\"", Error::SyntaxError(ErrorCode::EOFWhileParsingObject, 3, 8)), ]); }