From b6a12a15d5586843be0decdf27521953dfe5e9ce Mon Sep 17 00:00:00 2001 From: Bilal Mahmoud Date: Mon, 17 Apr 2023 19:05:28 +0200 Subject: [PATCH 01/19] feat: create struct visitor --- libs/deer/src/lib.rs | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/libs/deer/src/lib.rs b/libs/deer/src/lib.rs index 7f6d1745d9a..45698e1edde 100644 --- a/libs/deer/src/lib.rs +++ b/libs/deer/src/lib.rs @@ -375,6 +375,33 @@ pub trait OptionalVisitor<'de>: Sized { } } +#[allow(unused_variables)] +pub trait StructVisitor<'de>: Sized { + type Value; + + fn expecting(&self) -> Document; + + fn visit_array(self, array: A) -> Result + where + A: ArrayAccess<'de>, + { + Err(Report::new(TypeError.into_error()) + .attach(ReceivedType::new(visitor::ArraySchema::document())) + .attach(ExpectedType::new(self.expecting())) + .change_context(VisitorError)) + } + + fn visit_object(self, object: A) -> Result + where + A: ObjectAccess<'de>, + { + Err(Report::new(TypeError.into_error()) + .attach(ReceivedType::new(visitor::ObjectSchema::document())) + .attach(ExpectedType::new(self.expecting())) + .change_context(VisitorError)) + } +} + // internal visitor, which is used during the default implementation of the `deserialize_i*` and // `deserialize_u*` methods. struct NumberVisitor(PhantomData *const T>); @@ -622,6 +649,10 @@ pub trait Deserializer<'de>: Sized { where V: EnumVisitor<'de>; + fn deserialize_struct(self, visitor: V) -> Result + where + V: StructVisitor<'de>; + derive_from_number![ deserialize_i8(to_i8: i8) -> visit_i8, deserialize_i16(to_i16: i16) -> visit_i16, From 2d102c5175b5d17329936e72cf53aba3e0860fdb Mon Sep 17 00:00:00 2001 From: Bilal Mahmoud Date: Mon, 17 Apr 2023 19:09:18 +0200 Subject: [PATCH 02/19] feat: add mandatory `visit_null` and `visit_none` --- libs/deer/src/lib.rs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/libs/deer/src/lib.rs b/libs/deer/src/lib.rs index 45698e1edde..d8591f3f984 100644 --- a/libs/deer/src/lib.rs +++ b/libs/deer/src/lib.rs @@ -381,6 +381,19 @@ pub trait StructVisitor<'de>: Sized { fn expecting(&self) -> Document; + fn visit_none(self) -> Result { + Err(Report::new(MissingError.into_error()) + .attach(ExpectedType::new(self.expecting())) + .change_context(VisitorError)) + } + + fn visit_null(self) -> Result { + Err(Report::new(TypeError.into_error()) + .attach(ReceivedType::new(<()>::reflection())) + .attach(ExpectedType::new(self.expecting())) + .change_context(VisitorError)) + } + fn visit_array(self, array: A) -> Result where A: ArrayAccess<'de>, From 40b33ae3433c595b198747018a8a328afb980c08 Mon Sep 17 00:00:00 2001 From: Bilal Mahmoud Date: Mon, 17 Apr 2023 19:18:09 +0200 Subject: [PATCH 03/19] feat: implement `deserialize_struct` for desert --- libs/deer/desert/src/deserializer.rs | 28 +++++++++++++++++-- libs/deer/desert/src/token.rs | 41 +++++++++++++++++++++++++++- 2 files changed, 66 insertions(+), 3 deletions(-) diff --git a/libs/deer/desert/src/deserializer.rs b/libs/deer/desert/src/deserializer.rs index c2aecaa82ce..69f460b4d64 100644 --- a/libs/deer/desert/src/deserializer.rs +++ b/libs/deer/desert/src/deserializer.rs @@ -1,9 +1,9 @@ use alloc::borrow::ToOwned; use deer::{ - error::{DeserializerError, TypeError, Variant}, + error::{DeserializerError, ExpectedType, ReceivedType, TypeError, Variant}, value::NoneDeserializer, - Context, EnumVisitor, OptionalVisitor, Visitor, + Context, EnumVisitor, OptionalVisitor, StructVisitor, Visitor, }; use error_stack::{Report, Result, ResultExt}; @@ -138,6 +138,23 @@ impl<'a, 'de> deer::Deserializer<'de> for &mut Deserializer<'a, 'de> { Ok(value) } + + fn deserialize_struct(self, visitor: V) -> Result + where + V: StructVisitor<'de>, + { + let token = self.next(); + + match token { + Token::Array { length } => visitor.visit_array(ArrayAccess::new(self, length)), + Token::Object { length } => visitor.visit_object(ArrayAccess::new(self, length)), + Token::Null => visitor.visit_null(), + other => Err(Report::new(TypeError.into_error()) + .attach(ExpectedType::new(visitor.expecting())) + .attach(ReceivedType::new(other.schema()))), + } + .change_context(DeserializerError) + } } impl<'a, 'de> Deserializer<'a, 'de> { @@ -228,4 +245,11 @@ impl<'de> deer::Deserializer<'de> for DeserializerNone<'_> { .visit_value(discriminant, self) .change_context(DeserializerError) } + + fn deserialize_struct(self, visitor: V) -> Result + where + V: StructVisitor<'de>, + { + visitor.visit_none().change_context(DeserializerError) + } } diff --git a/libs/deer/desert/src/token.rs b/libs/deer/desert/src/token.rs index 8abf4514898..ec263fece7d 100644 --- a/libs/deer/desert/src/token.rs +++ b/libs/deer/desert/src/token.rs @@ -1,6 +1,6 @@ use core::fmt::{Debug, Display, Formatter}; -use deer::Number; +use deer::{Document, Number, Reflection, Schema}; // TODO: test // TODO: this should be `Copy`, but `Number` has no &'static constructor @@ -206,3 +206,42 @@ impl Display for Token { Debug::fmt(self, f) } } + +struct AnyArray; + +impl Reflection for AnyArray { + fn schema(_: &mut Document) -> Schema { + Schema::new("array") + } +} + +struct AnyObject; + +impl Reflection for AnyObject { + fn schema(_: &mut Document) -> Schema { + Schema::new("object") + } +} + +impl Token { + pub(crate) fn schema(&self) -> Document { + match self { + Token::Bool(_) => Document::new::(), + Token::Number(_) => Document::new::(), + Token::U128(_) => Document::new::(), + Token::I128(_) => Document::new::(), + Token::USize(_) => Document::new::(), + Token::ISize(_) => Document::new::(), + Token::Char(_) => Document::new::(), + Token::Str(_) => Document::new::(), + Token::BorrowedStr(_) => Document::new::(), + Token::String(_) => Document::new::(), + Token::Bytes(_) => Document::new::<[u8]>(), + Token::BorrowedBytes(_) => Document::new::<[u8]>(), + Token::BytesBuf(_) => Document::new::<[u8]>(), + Token::Array { .. } | Token::ArrayEnd => Document::new::(), + Token::Object { .. } | Token::ObjectEnd => Document::new::(), + Token::Null => Document::new::<()>(), + } + } +} From c4864a179a79067546f94a7a700f31cfe116fcf9 Mon Sep 17 00:00:00 2001 From: Bilal Mahmoud Date: Mon, 17 Apr 2023 19:22:04 +0200 Subject: [PATCH 04/19] feat: implement `deserialize_struct` for json --- libs/deer/json/src/lib.rs | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/libs/deer/json/src/lib.rs b/libs/deer/json/src/lib.rs index 978215f033a..83cda4b2381 100644 --- a/libs/deer/json/src/lib.rs +++ b/libs/deer/json/src/lib.rs @@ -25,7 +25,7 @@ use deer::{ }, value::NoneDeserializer, Context, Deserialize, DeserializeOwned, Document, EnumVisitor, FieldVisitor, OptionalVisitor, - Reflection, Schema, Visitor, + Reflection, Schema, StructVisitor, Visitor, }; use error_stack::{IntoReport, Report, Result, ResultExt}; use serde_json::{Map, Value}; @@ -427,6 +427,23 @@ impl<'a, 'de> deer::Deserializer<'de> for Deserializer<'a> { .change_context(DeserializerError) } } + + fn deserialize_struct(self, visitor: V) -> Result + where + V: StructVisitor<'de>, + { + match self.value { + None => visitor.visit_none(), + Some(Value::Object(object)) => { + visitor.visit_object(ObjectAccess::new(object, self.context)) + } + // we do not allow arrays as struct, only objects are allowed for structs + Some(value) => Err(Report::new(TypeError.into_error()) + .attach(ExpectedType::new(visitor.expecting())) + .attach(ReceivedType::new(into_document(&value)))), + } + .change_context(DeserializerError) + } } #[must_use] From efe37e9b6363640023676e9ed82c34bc6f5677c6 Mon Sep 17 00:00:00 2001 From: Bilal Mahmoud Date: Mon, 17 Apr 2023 20:04:08 +0200 Subject: [PATCH 05/19] feat: implement manual implementation of `Deserialize` for struct --- libs/deer/desert/src/token.rs | 5 + libs/deer/tests/common.rs | 68 ++++++ libs/deer/tests/test_struct_visitor.rs | 309 +++++++++++++++++++++++++ 3 files changed, 382 insertions(+) create mode 100644 libs/deer/tests/test_struct_visitor.rs diff --git a/libs/deer/desert/src/token.rs b/libs/deer/desert/src/token.rs index ec263fece7d..cbeb0488022 100644 --- a/libs/deer/desert/src/token.rs +++ b/libs/deer/desert/src/token.rs @@ -245,3 +245,8 @@ impl Token { } } } + +// TODO: maybe number +// TODO: IdentifierVisitor (u8, u64, str, borrowed_str, string, +// bytes, bytes_buf, borrowed_bytes) +// TODO: test diff --git a/libs/deer/tests/common.rs b/libs/deer/tests/common.rs index 8b137891791..a243949ed58 100644 --- a/libs/deer/tests/common.rs +++ b/libs/deer/tests/common.rs @@ -1 +1,69 @@ +//! Temporary helper trait for folding reports until [#2377](https://github.com/hashintel/hash/discussions/2377) +//! is resolved and implemented. +use error_stack::{Context, Report}; + +pub trait TupleExt { + type Context: Context; + type Ok; + + fn fold_reports(self) -> Result>; +} + +#[rustfmt::skip] +macro_rules! all_the_tuples { + ($name:ident) => { + $name!([T1], T2); + $name!([T1, T2], T3); + $name!([T1, T2, T3], T4); + $name!([T1, T2, T3, T4], T5); + $name!([T1, T2, T3, T4, T5], T6); + $name!([T1, T2, T3, T4, T5, T6], T7); + $name!([T1, T2, T3, T4, T5, T6, T7], T8); + $name!([T1, T2, T3, T4, T5, T6, T7, T8], T9); + $name!([T1, T2, T3, T4, T5, T6, T7, T8, T9], T10); + $name!([T1, T2, T3, T4, T5, T6, T7, T8, T9, T10], T11); + $name!([T1, T2, T3, T4, T5, T6, T7, T8, T9, T10, T11], T12); + $name!([T1, T2, T3, T4, T5, T6, T7, T8, T9, T10, T11, T12], T13); + $name!([T1, T2, T3, T4, T5, T6, T7, T8, T9, T10, T11, T12, T13], T14); + $name!([T1, T2, T3, T4, T5, T6, T7, T8, T9, T10, T11, T12, T13, T14], T15); + $name!([T1, T2, T3, T4, T5, T6, T7, T8, T9, T10, T11, T12, T13, T14, T15], T16); + }; +} + +impl TupleExt for (Result>,) { + type Context = C; + type Ok = (T1,); + + fn fold_reports(self) -> Result> { + self.0.map(|value| (value,)) + } +} + +macro_rules! impl_tuple_ext { + ([$($elem:ident),*], $other:ident) => { + #[allow(non_snake_case)] + impl TupleExt for ($(Result<$elem, Report>, )* Result<$other, Report>) { + type Context = C; + type Ok = ($($elem ,)* $other); + + fn fold_reports(self) -> Result> { + let ( $($elem ,)* $other ) = self; + + let lhs = ( $($elem ,)* ).fold_reports(); + + match (lhs, $other) { + (Ok(( $($elem ,)* )), Ok(rhs)) => Ok(($($elem ,)* rhs)), + (Ok(_), Err(err)) | (Err(err), Ok(_)) => Err(err), + (Err(mut lhs), Err(rhs)) => { + lhs.extend_one(rhs); + + Err(lhs) + } + } + } + } + }; +} + +all_the_tuples!(impl_tuple_ext); diff --git a/libs/deer/tests/test_struct_visitor.rs b/libs/deer/tests/test_struct_visitor.rs new file mode 100644 index 00000000000..1628964329d --- /dev/null +++ b/libs/deer/tests/test_struct_visitor.rs @@ -0,0 +1,309 @@ +use deer::{ + error::{ + ArrayAccessError, ArrayLengthError, DeserializeError, DeserializerError, ExpectedLength, + ReceivedLength, Variant, VisitorError, + }, + ArrayAccess, Deserialize, Deserializer, Document, FieldVisitor, ObjectAccess, Reflection, + Schema, StructVisitor, Visitor, +}; +use error_stack::{FutureExt, Report, Result, ResultExt}; +use serde::{ser::SerializeMap, Serialize, Serializer}; + +mod common; + +use common::TupleExt; +use deer::{ + error::{ + ExpectedField, ExpectedType, Location, MissingError, ObjectAccessError, ReceivedField, + UnknownFieldError, + }, + schema::Reference, +}; + +struct Example { + a: u8, + b: u16, + c: u32, +} + +struct ExampleFieldVisitor; + +enum ExampleFieldDiscriminator { + A, + B, + C, +} + +impl Reflection for ExampleFieldDiscriminator { + fn schema(_: &mut Document) -> Schema { + Schema::new("string").with("enum", ["a", "b", "c"]) + } +} + +impl<'de> Deserialize<'de> for ExampleFieldDiscriminator { + type Reflection = Self; + + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + struct IdentVisitor; + + impl<'de> Visitor<'de> for IdentVisitor { + type Value = ExampleFieldDiscriminator; + + fn expecting(&self) -> Document { + Self::Value::reflection() + } + + fn visit_u64(self, v: u64) -> Result { + match v { + 0 => Ok(ExampleFieldDiscriminator::A), + 1 => Ok(ExampleFieldDiscriminator::B), + 2 => Ok(ExampleFieldDiscriminator::C), + // TODO: accommodate for numeric identifier, bytes identifier + n => { + Err(Report::new(UnknownFieldError.into_error()) + .change_context(VisitorError)) + } + } + } + + fn visit_str(self, v: &str) -> Result { + match v { + "a" => Ok(ExampleFieldDiscriminator::A), + "b" => Ok(ExampleFieldDiscriminator::B), + "c" => Ok(ExampleFieldDiscriminator::C), + other => Err(Report::new(UnknownFieldError.into_error()) + .attach(ExpectedField::new("a")) + .attach(ExpectedField::new("b")) + .attach(ExpectedField::new("c")) + .attach(ReceivedField::new(other)) + .change_context(VisitorError)), + } + } + + fn visit_bytes(self, v: &[u8]) -> Result { + match v { + b"a" => Ok(ExampleFieldDiscriminator::A), + b"b" => Ok(ExampleFieldDiscriminator::B), + b"c" => Ok(ExampleFieldDiscriminator::C), + other => { + let mut error = Report::new(UnknownFieldError.into_error()) + .attach(ExpectedField::new("a")) + .attach(ExpectedField::new("b")) + .attach(ExpectedField::new("c")); + + if let Ok(other) = core::str::from_utf8(other) { + error = error.attach(ReceivedField::new(other)); + } + + Err(error.change_context(VisitorError)) + } + } + } + } + + deserializer + .deserialize_str(IdentVisitor) + .change_context(DeserializeError) + } +} + +enum ExampleField { + A(u8), + B(u16), + C(u32), +} + +impl<'de> FieldVisitor<'de> for ExampleFieldVisitor { + type Key = ExampleFieldDiscriminator; + type Value = ExampleField; + + fn visit_value(self, key: Self::Key, deserializer: D) -> Result + where + D: Deserializer<'de>, + { + match key { + ExampleFieldDiscriminator::A => u8::deserialize(deserializer) + .map(ExampleField::A) + .attach(Location::Field("a")) + .change_context(VisitorError), + ExampleFieldDiscriminator::B => u16::deserialize(deserializer) + .map(ExampleField::B) + .attach(Location::Field("b")) + .change_context(VisitorError), + ExampleFieldDiscriminator::C => u32::deserialize(deserializer) + .map(ExampleField::C) + .attach(Location::Field("c")) + .change_context(VisitorError), + } + } +} + +struct ExampleVisitor; + +impl<'de> StructVisitor<'de> for Example { + type Value = Example; + + fn expecting(&self) -> Document { + Example::reflection() + } + + fn visit_array(self, mut array: A) -> Result + where + A: ArrayAccess<'de>, + { + array.set_bounded(3).change_context(VisitorError)?; + + // while the contract states that we're guaranteed to always `Some` for the first 3 + // due to set_bounded we make sure that even if implementations are not correct we are still + // correct but doing `unwrap_or_else`. + // TODO: we might be able to expose that through the type system? + let a = array + .next() + .unwrap_or_else(|| { + Err(Report::new(ArrayLengthError.into_error()) + .attach(ExpectedLength::new(3)) + .attach(ReceivedLength::new(0)) + .change_context(ArrayAccessError)) + }) + .attach(Location::Tuple(0)); + + let b = array + .next() + .unwrap_or_else(|| { + Err(Report::new(ArrayLengthError.into_error()) + .attach(ExpectedLength::new(3)) + .attach(ReceivedLength::new(1)) + .change_context(ArrayAccessError)) + }) + .attach(Location::Tuple(1)); + + let c = array + .next() + .unwrap_or_else(|| { + Err(Report::new(ArrayLengthError.into_error()) + .attach(ExpectedLength::new(3)) + .attach(ReceivedLength::new(2)) + .change_context(ArrayAccessError)) + }) + .attach(Location::Tuple(2)); + + let (a, b, c, _) = (a, b, c, array.end()) + .fold_reports() + .change_context(VisitorError)?; + + Ok(Self { a, b, c }) + } + + fn visit_object(self, mut object: A) -> Result + where + A: ObjectAccess<'de>, + { + object.set_bounded(3).change_context(VisitorError)?; + + let mut a = None; + let mut b = None; + let mut c = None; + + let mut errors: Result<(), ObjectAccessError> = Ok(()); + + while let Some(field) = object.field(ExampleFieldVisitor) { + match field { + Err(error) => match &mut errors { + Err(errors) => { + errors.extend_one(error); + } + errors => *errors = Err(error), + }, + Ok(ExampleField::A(value)) => match a { + // we have no error type for this! + Some(_) => todo!(), + a => *a = Some(value), + }, + Ok(ExampleField::B(value)) => match b { + Some(_) => todo!(), + b => *b = Some(value), + }, + Ok(ExampleField::C(value)) => match c { + Some(_) => todo!(), + c => *c = Some(value), + }, + } + } + + let a = a.ok_or_else(|| { + Report::new(MissingError.into_error()) + .attach(ExpectedType::new(u8::reflection())) + .attach(Location::Field("a")) + .change_context(ObjectAccessError) + }); + + let b = b.ok_or_else(|| { + Report::new(MissingError.into_error()) + .attach(ExpectedType::new(u16::reflection())) + .attach(Location::Field("b")) + .change_context(ObjectAccessError) + }); + + let c = c.ok_or_else(|| { + Report::new(MissingError.into_error()) + .attach(ExpectedType::new(u32::reflection())) + .attach(Location::Field("c")) + .change_context(ObjectAccessError) + }); + + let (a, b, c, ..) = (a, b, c, errors, object.end()) + .fold_results() + .change_context(VisitorError)?; + + Ok(Example { a, b, c }) + } +} + +struct Properties(&'static [(&'static str, Reference)]); + +impl Serialize for Properties { + fn serialize(&self, serializer: S) -> std::result::Result + where + S: Serializer, + { + let mut map = serializer.serialize_map(Some(self.0.len()))?; + + for (key, value) in self.0 { + map.serialize_entry(key, value)?; + } + + map.end() + } +} + +impl Reflection for Example { + fn schema(doc: &mut Document) -> Schema { + // TODO: we cannot express or constraints right now + // we would need to say: object if e.g. JSON-schema, other format might do both + // blueprint must support "struct" + Schema::new("object").with( + "properties", + Properties(&[ + ("a", doc.add::()), + ("b", doc.add::()), + ("c", doc.add::()), + ]), + ) + } +} + +impl<'de> Deserialize<'de> for Example { + type Reflection = Self; + + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + deserializer + .deserialize_struct(ExampleVisitor) + .change_context(DeserializeError) + } +} From 5a5b8feb206ff7133e8b32ecf5ce759cf53f44fc Mon Sep 17 00:00:00 2001 From: Bilal Mahmoud Date: Mon, 17 Apr 2023 20:06:16 +0200 Subject: [PATCH 06/19] feat: outline ok --- libs/deer/tests/test_struct_visitor.rs | 30 ++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/libs/deer/tests/test_struct_visitor.rs b/libs/deer/tests/test_struct_visitor.rs index 1628964329d..b3b9c3e6ead 100644 --- a/libs/deer/tests/test_struct_visitor.rs +++ b/libs/deer/tests/test_struct_visitor.rs @@ -307,3 +307,33 @@ impl<'de> Deserialize<'de> for Example { .change_context(DeserializeError) } } + +#[test] +fn struct_object_ok() {} + +#[test] +fn struct_object_missing_err() {} + +#[test] +fn struct_object_too_many_err() {} + +#[test] +fn struct_object_too_few_err() {} + +#[test] +#[ignore] +fn struct_object_duplicate_err() { + // for now we cannot test this because there's no error for us to use +} + +#[test] +fn struct_array_ok() {} + +#[test] +fn struct_array_missing_err() {} + +#[test] +fn struct_array_too_many_err() {} + +#[test] +fn struct_array_too_few_err() {} From 0348a13560dfdbdc82f321c4502718eff7906f1c Mon Sep 17 00:00:00 2001 From: Bilal Mahmoud Date: Mon, 17 Apr 2023 20:21:18 +0200 Subject: [PATCH 07/19] fix: value deserializer --- libs/deer/desert/src/token.rs | 4 +-- libs/deer/src/value.rs | 34 ++++++++++++++++++++++++-- libs/deer/src/value/array.rs | 11 ++++++++- libs/deer/src/value/bytes.rs | 34 ++++++++++++++++++++------ libs/deer/src/value/object.rs | 12 ++++++++- libs/deer/src/value/string.rs | 26 +++++++++++++++++--- libs/deer/tests/test_struct_visitor.rs | 28 ++++++++++++++++++++- 7 files changed, 132 insertions(+), 17 deletions(-) diff --git a/libs/deer/desert/src/token.rs b/libs/deer/desert/src/token.rs index cbeb0488022..f73eb3b9bdb 100644 --- a/libs/deer/desert/src/token.rs +++ b/libs/deer/desert/src/token.rs @@ -1,6 +1,6 @@ use core::fmt::{Debug, Display, Formatter}; -use deer::{Document, Number, Reflection, Schema}; +use deer::{Deserialize, Document, Number, Reflection, Schema}; // TODO: test // TODO: this should be `Copy`, but `Number` has no &'static constructor @@ -241,7 +241,7 @@ impl Token { Token::BytesBuf(_) => Document::new::<[u8]>(), Token::Array { .. } | Token::ArrayEnd => Document::new::(), Token::Object { .. } | Token::ObjectEnd => Document::new::(), - Token::Null => Document::new::<()>(), + Token::Null => Document::new::<<() as Deserialize>::Reflection>(), } } } diff --git a/libs/deer/src/value.rs b/libs/deer/src/value.rs index 26a0fded243..ab8397d1c5c 100644 --- a/libs/deer/src/value.rs +++ b/libs/deer/src/value.rs @@ -5,12 +5,14 @@ mod string; pub use array::ArrayAccessDeserializer; pub use bytes::{BorrowedBytesDeserializer, BytesBufferDeserializer, BytesDeserializer}; -use error_stack::{Result, ResultExt}; +use error_stack::{Report, Result, ResultExt}; pub use object::ObjectAccessDeserializer; pub use string::{BorrowedStrDeserializer, StrDeserializer, StringDeserializer}; use crate::{ - error::DeserializerError, Context, Deserializer, EnumVisitor, Number, OptionalVisitor, Visitor, + error::{DeserializerError, ExpectedType, ReceivedType, TypeError, Variant}, + Context, Deserialize, Deserializer, EnumVisitor, Number, OptionalVisitor, StructVisitor, + Visitor, }; macro_rules! impl_owned { @@ -75,6 +77,20 @@ macro_rules! impl_owned { { $crate::value::EnumUnitDeserializer::new(self.context, self).deserialize_enum(visitor) } + + fn deserialize_struct(self, visitor: V) -> error_stack::Result + where + V: StructVisitor<'de> + { + Err( + Report::new(TypeError.into_error()) + .attach(ExpectedType::new(visitor.expecting())) + // TODO: enable once String and Vec have reflection + // (or we rework this macro c:) + // .attach(ReceivedType::new(<$ty>::reflection())) + .change_context(DeserializerError) + ) + } } impl<'de> IntoDeserializer<'de> for $ty { @@ -205,6 +221,13 @@ impl<'de> Deserializer<'de> for NoneDeserializer<'_> { .visit_value(discriminant, self) .change_context(DeserializerError) } + + fn deserialize_struct(self, visitor: V) -> Result + where + V: StructVisitor<'de>, + { + visitor.visit_none().change_context(DeserializerError) + } } #[derive(Debug, Copy, Clone)] @@ -256,6 +279,13 @@ impl<'de> Deserializer<'de> for NullDeserializer<'_> { { EnumUnitDeserializer::new(self.context, self).deserialize_enum(visitor) } + + fn deserialize_struct(self, visitor: V) -> Result + where + V: StructVisitor<'de>, + { + visitor.visit_null().change_context(DeserializerError) + } } impl_owned!(bool, BoolDeserializer, visit_bool); diff --git a/libs/deer/src/value/array.rs b/libs/deer/src/value/array.rs index 7a3d20cdf81..b07d169483c 100644 --- a/libs/deer/src/value/array.rs +++ b/libs/deer/src/value/array.rs @@ -2,7 +2,7 @@ use error_stack::{Result, ResultExt}; use crate::{ error::DeserializerError, value::EnumUnitDeserializer, ArrayAccess, Context, Deserializer, - EnumVisitor, OptionalVisitor, Visitor, + EnumVisitor, OptionalVisitor, StructVisitor, Visitor, }; // TODO: SliceDeserializer/IteratorDeserializer @@ -62,4 +62,13 @@ where { EnumUnitDeserializer::new(self.context, self).deserialize_enum(visitor) } + + fn deserialize_struct(self, visitor: V) -> Result + where + V: StructVisitor<'de>, + { + visitor + .visit_array(self.value) + .change_context(DeserializerError) + } } diff --git a/libs/deer/src/value/bytes.rs b/libs/deer/src/value/bytes.rs index aa10d4f0b31..2145b080dbb 100644 --- a/libs/deer/src/value/bytes.rs +++ b/libs/deer/src/value/bytes.rs @@ -1,11 +1,11 @@ use alloc::vec::Vec; -use error_stack::{Result, ResultExt}; +use error_stack::{Report, Result, ResultExt}; use crate::{ - error::DeserializerError, + error::{DeserializerError, ExpectedType, ReceivedType, TypeError, Variant}, value::{impl_owned, EnumUnitDeserializer, IntoDeserializer}, - Context, Deserializer, EnumVisitor, OptionalVisitor, Visitor, + Context, Deserializer, EnumVisitor, OptionalVisitor, Reflection, StructVisitor, Visitor, }; #[derive(Debug, Copy, Clone)] @@ -38,7 +38,7 @@ impl<'de> Deserializer<'de> for BytesDeserializer<'_, '_> { self.context } - fn deserialize_any(self, visitor: V) -> error_stack::Result + fn deserialize_any(self, visitor: V) -> Result where V: Visitor<'de>, { @@ -47,7 +47,7 @@ impl<'de> Deserializer<'de> for BytesDeserializer<'_, '_> { .change_context(DeserializerError) } - fn deserialize_optional(self, visitor: V) -> error_stack::Result + fn deserialize_optional(self, visitor: V) -> Result where V: OptionalVisitor<'de>, { @@ -60,6 +60,16 @@ impl<'de> Deserializer<'de> for BytesDeserializer<'_, '_> { { EnumUnitDeserializer::new(self.context, self).deserialize_enum(visitor) } + + fn deserialize_struct(self, visitor: V) -> Result + where + V: StructVisitor<'de>, + { + Err(Report::new(TypeError.into_error()) + .attach(ReceivedType::new(<[u8]>::document())) + .attach(ExpectedType::new(visitor.expecting())) + .change_context(DeserializerError)) + } } impl<'de, 'b> IntoDeserializer<'de> for &'b [u8] { @@ -103,7 +113,7 @@ impl<'de> Deserializer<'de> for BorrowedBytesDeserializer<'_, 'de> { self.context } - fn deserialize_any(self, visitor: V) -> error_stack::Result + fn deserialize_any(self, visitor: V) -> Result where V: Visitor<'de>, { @@ -112,7 +122,7 @@ impl<'de> Deserializer<'de> for BorrowedBytesDeserializer<'_, 'de> { .change_context(DeserializerError) } - fn deserialize_optional(self, visitor: V) -> error_stack::Result + fn deserialize_optional(self, visitor: V) -> Result where V: OptionalVisitor<'de>, { @@ -125,6 +135,16 @@ impl<'de> Deserializer<'de> for BorrowedBytesDeserializer<'_, 'de> { { EnumUnitDeserializer::new(self.context, self).deserialize_enum(visitor) } + + fn deserialize_struct(self, visitor: V) -> Result + where + V: StructVisitor<'de>, + { + Err(Report::new(TypeError.into_error()) + .attach(ReceivedType::new(<[u8]>::document())) + .attach(ExpectedType::new(visitor.expecting())) + .change_context(DeserializerError)) + } } impl_owned!(!copy: Vec, BytesBufferDeserializer, visit_bytes_buffer); diff --git a/libs/deer/src/value/object.rs b/libs/deer/src/value/object.rs index eee99e5dc18..e0204b22da8 100644 --- a/libs/deer/src/value/object.rs +++ b/libs/deer/src/value/object.rs @@ -4,7 +4,8 @@ use crate::{ error::{ DeserializerError, ExpectedLength, ObjectLengthError, ReceivedLength, Variant, VisitorError, }, - Context, Deserializer, EnumVisitor, FieldVisitor, ObjectAccess, OptionalVisitor, Visitor, + Context, Deserializer, EnumVisitor, FieldVisitor, ObjectAccess, OptionalVisitor, StructVisitor, + Visitor, }; // TODO: MapDeserializer/IteratorDeserializer @@ -109,4 +110,13 @@ where self.value.end().change_context(DeserializerError)?; value.change_context(DeserializerError) } + + fn deserialize_struct(self, visitor: V) -> Result + where + V: StructVisitor<'de>, + { + visitor + .visit_object(self.value) + .change_context(DeserializerError) + } } diff --git a/libs/deer/src/value/string.rs b/libs/deer/src/value/string.rs index a85627e7a1b..06115c4d782 100644 --- a/libs/deer/src/value/string.rs +++ b/libs/deer/src/value/string.rs @@ -1,11 +1,11 @@ use alloc::string::String; -use error_stack::ResultExt; +use error_stack::{Report, ResultExt}; use crate::{ - error::DeserializerError, + error::{DeserializerError, ExpectedType, ReceivedType, TypeError, Variant}, value::{impl_owned, EnumUnitDeserializer, IntoDeserializer, NoneDeserializer}, - Context, Deserializer, EnumVisitor, OptionalVisitor, Visitor, + Context, Deserializer, EnumVisitor, OptionalVisitor, Reflection, StructVisitor, Visitor, }; #[derive(Debug, Copy, Clone)] @@ -68,6 +68,16 @@ impl<'de> Deserializer<'de> for StrDeserializer<'_, '_> { .visit_value(discriminant, NoneDeserializer::new(context)) .change_context(DeserializerError) } + + fn deserialize_struct(self, visitor: V) -> error_stack::Result + where + V: StructVisitor<'de>, + { + Err(Report::new(TypeError.into_error()) + .attach(ReceivedType::new(str::document())) + .attach(ExpectedType::new(visitor.expecting())) + .change_context(DeserializerError)) + } } impl<'de, 'b> IntoDeserializer<'de> for &'b str { @@ -133,6 +143,16 @@ impl<'de> Deserializer<'de> for BorrowedStrDeserializer<'_, 'de> { { EnumUnitDeserializer::new(self.context, self).deserialize_enum(visitor) } + + fn deserialize_struct(self, visitor: V) -> error_stack::Result + where + V: StructVisitor<'de>, + { + Err(Report::new(TypeError.into_error()) + .attach(ReceivedType::new(str::document())) + .attach(ExpectedType::new(visitor.expecting())) + .change_context(DeserializerError)) + } } impl_owned!(!copy: String, StringDeserializer, visit_string); diff --git a/libs/deer/tests/test_struct_visitor.rs b/libs/deer/tests/test_struct_visitor.rs index b3b9c3e6ead..ff7b4471975 100644 --- a/libs/deer/tests/test_struct_visitor.rs +++ b/libs/deer/tests/test_struct_visitor.rs @@ -19,6 +19,7 @@ use deer::{ }, schema::Reference, }; +use deer_desert::{assert_tokens, Token}; struct Example { a: u8, @@ -309,7 +310,32 @@ impl<'de> Deserialize<'de> for Example { } #[test] -fn struct_object_ok() {} +fn struct_object_ok() { + assert_tokens(&Example { a: 2, b: 3, c: 4 }, &[ + Token::Object { length: Some(3) }, + Token::Str("a"), + Token::Number(2.into()), + Token::Str("b"), + Token::Number(3.into()), + Token::Str("c"), + Token::Number(4.into()), + Token::ObjectEnd, + ]) +} + +#[test] +fn struct_object_out_of_order_ok() { + assert_tokens(&Example { a: 2, b: 3, c: 4 }, &[ + Token::Object { length: Some(3) }, + Token::Str("c"), + Token::Number(4.into()), + Token::Str("b"), + Token::Number(3.into()), + Token::Str("a"), + Token::Number(2.into()), + Token::ObjectEnd, + ]) +} #[test] fn struct_object_missing_err() {} From 8a3784f9074c24142a6d854ce7c1bad8efb27482 Mon Sep 17 00:00:00 2001 From: Bilal Mahmoud Date: Mon, 17 Apr 2023 20:41:24 +0200 Subject: [PATCH 08/19] test: `StructVisitor` --- libs/deer/desert/src/deserializer.rs | 14 +- libs/deer/desert/src/token.rs | 28 ++- libs/deer/json/src/lib.rs | 12 +- libs/deer/src/value.rs | 5 +- libs/deer/tests/common.rs | 2 +- libs/deer/tests/test_struct_visitor.rs | 244 ++++++++++++++++++++----- 6 files changed, 231 insertions(+), 74 deletions(-) diff --git a/libs/deer/desert/src/deserializer.rs b/libs/deer/desert/src/deserializer.rs index 69f460b4d64..2ca2cb3b42f 100644 --- a/libs/deer/desert/src/deserializer.rs +++ b/libs/deer/desert/src/deserializer.rs @@ -146,14 +146,18 @@ impl<'a, 'de> deer::Deserializer<'de> for &mut Deserializer<'a, 'de> { let token = self.next(); match token { - Token::Array { length } => visitor.visit_array(ArrayAccess::new(self, length)), - Token::Object { length } => visitor.visit_object(ArrayAccess::new(self, length)), - Token::Null => visitor.visit_null(), + Token::Array { length } => visitor + .visit_array(ArrayAccess::new(self, length)) + .change_context(DeserializerError), + Token::Object { length } => visitor + .visit_object(ObjectAccess::new(self, length)) + .change_context(DeserializerError), + Token::Null => visitor.visit_null().change_context(DeserializerError), other => Err(Report::new(TypeError.into_error()) .attach(ExpectedType::new(visitor.expecting())) - .attach(ReceivedType::new(other.schema()))), + .attach(ReceivedType::new(other.schema())) + .change_context(DeserializerError)), } - .change_context(DeserializerError) } } diff --git a/libs/deer/desert/src/token.rs b/libs/deer/desert/src/token.rs index f73eb3b9bdb..5971a443890 100644 --- a/libs/deer/desert/src/token.rs +++ b/libs/deer/desert/src/token.rs @@ -226,22 +226,18 @@ impl Reflection for AnyObject { impl Token { pub(crate) fn schema(&self) -> Document { match self { - Token::Bool(_) => Document::new::(), - Token::Number(_) => Document::new::(), - Token::U128(_) => Document::new::(), - Token::I128(_) => Document::new::(), - Token::USize(_) => Document::new::(), - Token::ISize(_) => Document::new::(), - Token::Char(_) => Document::new::(), - Token::Str(_) => Document::new::(), - Token::BorrowedStr(_) => Document::new::(), - Token::String(_) => Document::new::(), - Token::Bytes(_) => Document::new::<[u8]>(), - Token::BorrowedBytes(_) => Document::new::<[u8]>(), - Token::BytesBuf(_) => Document::new::<[u8]>(), - Token::Array { .. } | Token::ArrayEnd => Document::new::(), - Token::Object { .. } | Token::ObjectEnd => Document::new::(), - Token::Null => Document::new::<<() as Deserialize>::Reflection>(), + Self::Bool(_) => Document::new::(), + Self::Number(_) => Document::new::(), + Self::U128(_) => Document::new::(), + Self::I128(_) => Document::new::(), + Self::USize(_) => Document::new::(), + Self::ISize(_) => Document::new::(), + Self::Char(_) => Document::new::(), + Self::Str(_) | Self::BorrowedStr(_) | Self::String(_) => Document::new::(), + Self::Bytes(_) | Self::BorrowedBytes(_) | Self::BytesBuf(_) => Document::new::<[u8]>(), + Self::Array { .. } | Self::ArrayEnd => Document::new::(), + Self::Object { .. } | Self::ObjectEnd => Document::new::(), + Self::Null => Document::new::<<() as Deserialize>::Reflection>(), } } } diff --git a/libs/deer/json/src/lib.rs b/libs/deer/json/src/lib.rs index 83cda4b2381..fb0355f8daa 100644 --- a/libs/deer/json/src/lib.rs +++ b/libs/deer/json/src/lib.rs @@ -433,16 +433,16 @@ impl<'a, 'de> deer::Deserializer<'de> for Deserializer<'a> { V: StructVisitor<'de>, { match self.value { - None => visitor.visit_none(), - Some(Value::Object(object)) => { - visitor.visit_object(ObjectAccess::new(object, self.context)) - } + None => visitor.visit_none().change_context(DeserializerError), + Some(Value::Object(object)) => visitor + .visit_object(ObjectAccess::new(object, self.context)) + .change_context(DeserializerError), // we do not allow arrays as struct, only objects are allowed for structs Some(value) => Err(Report::new(TypeError.into_error()) .attach(ExpectedType::new(visitor.expecting())) - .attach(ReceivedType::new(into_document(&value)))), + .attach(ReceivedType::new(into_document(&value))) + .change_context(DeserializerError)), } - .change_context(DeserializerError) } } diff --git a/libs/deer/src/value.rs b/libs/deer/src/value.rs index ab8397d1c5c..15a28f6f0c8 100644 --- a/libs/deer/src/value.rs +++ b/libs/deer/src/value.rs @@ -10,9 +10,8 @@ pub use object::ObjectAccessDeserializer; pub use string::{BorrowedStrDeserializer, StrDeserializer, StringDeserializer}; use crate::{ - error::{DeserializerError, ExpectedType, ReceivedType, TypeError, Variant}, - Context, Deserialize, Deserializer, EnumVisitor, Number, OptionalVisitor, StructVisitor, - Visitor, + error::{DeserializerError, ExpectedType, TypeError, Variant}, + Context, Deserializer, EnumVisitor, Number, OptionalVisitor, StructVisitor, Visitor, }; macro_rules! impl_owned { diff --git a/libs/deer/tests/common.rs b/libs/deer/tests/common.rs index a243949ed58..e3d45e8e017 100644 --- a/libs/deer/tests/common.rs +++ b/libs/deer/tests/common.rs @@ -3,7 +3,7 @@ use error_stack::{Context, Report}; -pub trait TupleExt { +pub(crate) trait TupleExt { type Context: Context; type Ok; diff --git a/libs/deer/tests/test_struct_visitor.rs b/libs/deer/tests/test_struct_visitor.rs index ff7b4471975..f6e1b1cfc6e 100644 --- a/libs/deer/tests/test_struct_visitor.rs +++ b/libs/deer/tests/test_struct_visitor.rs @@ -1,13 +1,14 @@ use deer::{ error::{ - ArrayAccessError, ArrayLengthError, DeserializeError, DeserializerError, ExpectedLength, - ReceivedLength, Variant, VisitorError, + ArrayAccessError, ArrayLengthError, DeserializeError, ExpectedLength, ReceivedLength, + Variant, VisitorError, }, ArrayAccess, Deserialize, Deserializer, Document, FieldVisitor, ObjectAccess, Reflection, Schema, StructVisitor, Visitor, }; -use error_stack::{FutureExt, Report, Result, ResultExt}; +use error_stack::{Report, Result, ResultExt}; use serde::{ser::SerializeMap, Serialize, Serializer}; +use serde_json::json; mod common; @@ -19,8 +20,9 @@ use deer::{ }, schema::Reference, }; -use deer_desert::{assert_tokens, Token}; +use deer_desert::{assert_tokens, assert_tokens_error, error, Token}; +#[derive(Debug, Eq, PartialEq)] struct Example { a: u8, b: u16, @@ -57,19 +59,6 @@ impl<'de> Deserialize<'de> for ExampleFieldDiscriminator { Self::Value::reflection() } - fn visit_u64(self, v: u64) -> Result { - match v { - 0 => Ok(ExampleFieldDiscriminator::A), - 1 => Ok(ExampleFieldDiscriminator::B), - 2 => Ok(ExampleFieldDiscriminator::C), - // TODO: accommodate for numeric identifier, bytes identifier - n => { - Err(Report::new(UnknownFieldError.into_error()) - .change_context(VisitorError)) - } - } - } - fn visit_str(self, v: &str) -> Result { match v { "a" => Ok(ExampleFieldDiscriminator::A), @@ -103,6 +92,19 @@ impl<'de> Deserialize<'de> for ExampleFieldDiscriminator { } } } + + fn visit_u64(self, v: u64) -> Result { + match v { + 0 => Ok(ExampleFieldDiscriminator::A), + 1 => Ok(ExampleFieldDiscriminator::B), + 2 => Ok(ExampleFieldDiscriminator::C), + // TODO: accommodate for numeric identifier, bytes identifier + _ => { + Err(Report::new(UnknownFieldError.into_error()) + .change_context(VisitorError)) + } + } + } } deserializer @@ -144,7 +146,7 @@ impl<'de> FieldVisitor<'de> for ExampleFieldVisitor { struct ExampleVisitor; -impl<'de> StructVisitor<'de> for Example { +impl<'de> StructVisitor<'de> for ExampleVisitor { type Value = Example; fn expecting(&self) -> Document { @@ -195,7 +197,7 @@ impl<'de> StructVisitor<'de> for Example { .fold_reports() .change_context(VisitorError)?; - Ok(Self { a, b, c }) + Ok(Example { a, b, c }) } fn visit_object(self, mut object: A) -> Result @@ -218,17 +220,17 @@ impl<'de> StructVisitor<'de> for Example { } errors => *errors = Err(error), }, - Ok(ExampleField::A(value)) => match a { + Ok(ExampleField::A(value)) => match &mut a { // we have no error type for this! - Some(_) => todo!(), + Some(_) => unimplemented!("planned in follow up PR"), a => *a = Some(value), }, - Ok(ExampleField::B(value)) => match b { - Some(_) => todo!(), + Ok(ExampleField::B(value)) => match &mut b { + Some(_) => unimplemented!("planned in follow up PR"), b => *b = Some(value), }, - Ok(ExampleField::C(value)) => match c { - Some(_) => todo!(), + Ok(ExampleField::C(value)) => match &mut c { + Some(_) => unimplemented!("planned in follow up PR"), c => *c = Some(value), }, } @@ -256,24 +258,24 @@ impl<'de> StructVisitor<'de> for Example { }); let (a, b, c, ..) = (a, b, c, errors, object.end()) - .fold_results() + .fold_reports() .change_context(VisitorError)?; Ok(Example { a, b, c }) } } -struct Properties(&'static [(&'static str, Reference)]); +struct Properties([(&'static str, Reference); N]); -impl Serialize for Properties { - fn serialize(&self, serializer: S) -> std::result::Result +impl Serialize for Properties { + fn serialize(&self, serializer: S) -> core::result::Result where S: Serializer, { let mut map = serializer.serialize_map(Some(self.0.len()))?; for (key, value) in self.0 { - map.serialize_entry(key, value)?; + map.serialize_entry(key, &value)?; } map.end() @@ -287,7 +289,7 @@ impl Reflection for Example { // blueprint must support "struct" Schema::new("object").with( "properties", - Properties(&[ + Properties([ ("a", doc.add::()), ("b", doc.add::()), ("c", doc.add::()), @@ -320,7 +322,7 @@ fn struct_object_ok() { Token::Str("c"), Token::Number(4.into()), Token::ObjectEnd, - ]) + ]); } #[test] @@ -334,32 +336,188 @@ fn struct_object_out_of_order_ok() { Token::Str("a"), Token::Number(2.into()), Token::ObjectEnd, - ]) + ]); } +// TODO: key missing instead of value missing (or discriminant missing) ~> only possible with +// IdentifierVisitor? #[test] -fn struct_object_missing_err() {} +fn struct_object_missing_err() { + assert_tokens_error::( + &error!([{ + ns: "deer", + id: ["value", "missing"], + properties: { + "expected": u16::reflection(), + "location": [{"type": "field", "value": "b"}] + } + }, { + ns: "deer", + // TODO: this should be key/discriminator missing! (or should this just silently fail?) + id: ["value", "missing"], + properties: { + "expected": ExampleFieldDiscriminator::reflection(), + "location": [] + } + }]), + &[ + Token::Object { length: Some(2) }, + Token::Str("a"), + Token::Number(2.into()), + Token::Str("c"), + Token::Number(4.into()), + Token::ObjectEnd, + ], + ); +} #[test] -fn struct_object_too_many_err() {} +fn struct_object_missing_multiple_err() { + assert_tokens_error::( + &error!([{ + ns: "deer", + id: ["value", "missing"], + properties: { + "expected": u16::reflection(), + "location": [{"type": "field", "value": "b"}] + } + },{ + ns: "deer", + id: ["value", "missing"], + properties: { + "expected": u32::reflection(), + "location": [{"type": "field", "value": "c"}] + } + },{ + ns: "deer", + id: ["value", "missing"], + properties: { + "expected": ExampleFieldDiscriminator::reflection(), + "location": [] + } + },{ + ns: "deer", + id: ["value", "missing"], + properties: { + "expected": ExampleFieldDiscriminator::reflection(), + "location": [] + } + }]), + &[ + Token::Object { length: Some(1) }, + Token::Str("a"), + Token::Number(2.into()), + Token::ObjectEnd, + ], + ); +} #[test] -fn struct_object_too_few_err() {} +fn struct_object_too_many_err() { + assert_tokens_error::( + &error!([{ + ns: "deer", + id: ["object", "length"], + properties: { + "expected": 3, + "received": 4, + "location": [] + } + }]), + &[ + Token::Object { length: Some(4) }, + Token::Str("a"), + Token::Number(2.into()), + Token::Str("b"), + Token::Number(3.into()), + Token::Str("c"), + Token::Number(4.into()), + Token::Str("d"), + Token::Number(5.into()), + Token::ObjectEnd, + ], + ); +} #[test] -#[ignore] +#[ignore = "not yet implemented"] fn struct_object_duplicate_err() { // for now we cannot test this because there's no error for us to use -} -#[test] -fn struct_array_ok() {} + // this will fail (this is on purpose) + assert_tokens_error::( + &error!([{ + ns: "deer", + id: ["object", "length"], + properties: { + "expected": 3, + "received": 4, + "location": [] + } + }]), + &[ + Token::Object { length: Some(3) }, + Token::Str("a"), + Token::Number(2.into()), + Token::Str("b"), + Token::Number(3.into()), + Token::Str("b"), + Token::Number(4.into()), + Token::ObjectEnd, + ], + ); +} #[test] -fn struct_array_missing_err() {} +fn struct_array_ok() { + assert_tokens(&Example { a: 2, b: 3, c: 4 }, &[ + Token::Array { length: Some(3) }, + Token::Number(2.into()), + Token::Number(3.into()), + Token::Number(4.into()), + Token::ArrayEnd, + ]); +} #[test] -fn struct_array_too_many_err() {} +fn struct_array_missing_err() { + assert_tokens_error::( + &error!([{ + ns: "deer", + id: ["value", "missing"], + properties: { + "expected": u32::reflection(), + "location": [{"type": "tuple", "value": 2}] + } + }]), + &[ + Token::Array { length: Some(2) }, + Token::Number(2.into()), + Token::Number(3.into()), + Token::ArrayEnd, + ], + ); +} #[test] -fn struct_array_too_few_err() {} +fn struct_array_too_many_err() { + assert_tokens_error::( + &error!([{ + ns: "deer", + id: ["array", "length"], + properties: { + "expected": 3, + "received": 4, + "location": [] + } + }]), + &[ + Token::Array { length: Some(4) }, + Token::Number(2.into()), + Token::Number(3.into()), + Token::Number(4.into()), + Token::Number(5.into()), + Token::ArrayEnd, + ], + ); +} From 4a9949f02704c95e0e7e58a188f10dd572a22228 Mon Sep 17 00:00:00 2001 From: Bilal Mahmoud Date: Mon, 17 Apr 2023 21:06:00 +0200 Subject: [PATCH 09/19] feat: remove `visit_null`/`visit_none` from `StructVisitor` --- libs/deer/desert/src/deserializer.rs | 7 ++++--- libs/deer/src/lib.rs | 14 ++------------ libs/deer/src/value.rs | 14 +++++++++++--- 3 files changed, 17 insertions(+), 18 deletions(-) diff --git a/libs/deer/desert/src/deserializer.rs b/libs/deer/desert/src/deserializer.rs index 2ca2cb3b42f..96034f3fee8 100644 --- a/libs/deer/desert/src/deserializer.rs +++ b/libs/deer/desert/src/deserializer.rs @@ -1,7 +1,7 @@ use alloc::borrow::ToOwned; use deer::{ - error::{DeserializerError, ExpectedType, ReceivedType, TypeError, Variant}, + error::{DeserializerError, ExpectedType, MissingError, ReceivedType, TypeError, Variant}, value::NoneDeserializer, Context, EnumVisitor, OptionalVisitor, StructVisitor, Visitor, }; @@ -152,7 +152,6 @@ impl<'a, 'de> deer::Deserializer<'de> for &mut Deserializer<'a, 'de> { Token::Object { length } => visitor .visit_object(ObjectAccess::new(self, length)) .change_context(DeserializerError), - Token::Null => visitor.visit_null().change_context(DeserializerError), other => Err(Report::new(TypeError.into_error()) .attach(ExpectedType::new(visitor.expecting())) .attach(ReceivedType::new(other.schema())) @@ -254,6 +253,8 @@ impl<'de> deer::Deserializer<'de> for DeserializerNone<'_> { where V: StructVisitor<'de>, { - visitor.visit_none().change_context(DeserializerError) + Err(Report::new(MissingError.into_error()) + .attach(ExpectedType::new(visitor.expecting())) + .change_context(DeserializerError)) } } diff --git a/libs/deer/src/lib.rs b/libs/deer/src/lib.rs index d8591f3f984..43c23593b48 100644 --- a/libs/deer/src/lib.rs +++ b/libs/deer/src/lib.rs @@ -381,18 +381,8 @@ pub trait StructVisitor<'de>: Sized { fn expecting(&self) -> Document; - fn visit_none(self) -> Result { - Err(Report::new(MissingError.into_error()) - .attach(ExpectedType::new(self.expecting())) - .change_context(VisitorError)) - } - - fn visit_null(self) -> Result { - Err(Report::new(TypeError.into_error()) - .attach(ReceivedType::new(<()>::reflection())) - .attach(ExpectedType::new(self.expecting())) - .change_context(VisitorError)) - } + // visit_none and visit_null are not implemented, as they can be used more expressively using + // `OptionalVisitor` fn visit_array(self, array: A) -> Result where diff --git a/libs/deer/src/value.rs b/libs/deer/src/value.rs index 15a28f6f0c8..746dae39a93 100644 --- a/libs/deer/src/value.rs +++ b/libs/deer/src/value.rs @@ -11,7 +11,8 @@ pub use string::{BorrowedStrDeserializer, StrDeserializer, StringDeserializer}; use crate::{ error::{DeserializerError, ExpectedType, TypeError, Variant}, - Context, Deserializer, EnumVisitor, Number, OptionalVisitor, StructVisitor, Visitor, + Context, Deserialize, Deserializer, EnumVisitor, Number, OptionalVisitor, StructVisitor, + Visitor, }; macro_rules! impl_owned { @@ -120,6 +121,8 @@ macro_rules! impl_owned { use impl_owned; +use crate::error::{MissingError, ReceivedType}; + pub trait IntoDeserializer<'de> { type Deserializer<'a>: Deserializer<'de> where @@ -225,7 +228,9 @@ impl<'de> Deserializer<'de> for NoneDeserializer<'_> { where V: StructVisitor<'de>, { - visitor.visit_none().change_context(DeserializerError) + Err(Report::new(MissingError.into_error()) + .attach(ExpectedType::new(visitor.expecting())) + .change_context(DeserializerError)) } } @@ -283,7 +288,10 @@ impl<'de> Deserializer<'de> for NullDeserializer<'_> { where V: StructVisitor<'de>, { - visitor.visit_null().change_context(DeserializerError) + Err(Report::new(TypeError.into_error()) + .attach(ExpectedType::new(visitor.expecting())) + .attach(ReceivedType::new(<()>::reflection())) + .change_context(DeserializerError)) } } From af640af72a028eff88ed69d945c59ea5c73eec67 Mon Sep 17 00:00:00 2001 From: Bilal Mahmoud Date: Mon, 17 Apr 2023 21:09:13 +0200 Subject: [PATCH 10/19] fix: json `Deserializer` --- libs/deer/json/src/lib.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/libs/deer/json/src/lib.rs b/libs/deer/json/src/lib.rs index fb0355f8daa..05052ce9d20 100644 --- a/libs/deer/json/src/lib.rs +++ b/libs/deer/json/src/lib.rs @@ -19,9 +19,9 @@ use std::any::Demand; use deer::{ error::{ ArrayAccessError, ArrayLengthError, BoundedContractViolationError, DeserializeError, - DeserializerError, ExpectedLength, ExpectedType, ObjectAccessError, ObjectItemsExtraError, - ObjectLengthError, ReceivedKey, ReceivedLength, ReceivedType, ReceivedValue, TypeError, - ValueError, Variant, + DeserializerError, ExpectedLength, ExpectedType, MissingError, ObjectAccessError, + ObjectItemsExtraError, ObjectLengthError, ReceivedKey, ReceivedLength, ReceivedType, + ReceivedValue, TypeError, ValueError, Variant, }, value::NoneDeserializer, Context, Deserialize, DeserializeOwned, Document, EnumVisitor, FieldVisitor, OptionalVisitor, @@ -433,7 +433,9 @@ impl<'a, 'de> deer::Deserializer<'de> for Deserializer<'a> { V: StructVisitor<'de>, { match self.value { - None => visitor.visit_none().change_context(DeserializerError), + None => Err(Report::new(MissingError.into_error()) + .attach(ExpectedType::new(visitor.expecting())) + .change_context(DeserializerError)), Some(Value::Object(object)) => visitor .visit_object(ObjectAccess::new(object, self.context)) .change_context(DeserializerError), From 885249bed9f1a151106d7b4822f46b2d2c0bbedd Mon Sep 17 00:00:00 2001 From: Bilal Mahmoud Date: Tue, 18 Apr 2023 11:34:10 +0200 Subject: [PATCH 11/19] test: remove indirection in struct code --- libs/deer/tests/test_struct_visitor.rs | 95 +++++++++++++++----------- 1 file changed, 57 insertions(+), 38 deletions(-) diff --git a/libs/deer/tests/test_struct_visitor.rs b/libs/deer/tests/test_struct_visitor.rs index f6e1b1cfc6e..f70b09a5d93 100644 --- a/libs/deer/tests/test_struct_visitor.rs +++ b/libs/deer/tests/test_struct_visitor.rs @@ -29,7 +29,11 @@ struct Example { c: u32, } -struct ExampleFieldVisitor; +struct ExampleFieldVisitor<'a> { + a: &'a mut Option, + b: &'a mut Option, + c: &'a mut Option, +} enum ExampleFieldDiscriminator { A, @@ -113,33 +117,54 @@ impl<'de> Deserialize<'de> for ExampleFieldDiscriminator { } } -enum ExampleField { - A(u8), - B(u16), - C(u32), -} - -impl<'de> FieldVisitor<'de> for ExampleFieldVisitor { +impl<'de> FieldVisitor<'de> for ExampleFieldVisitor<'_> { type Key = ExampleFieldDiscriminator; - type Value = ExampleField; + type Value = (); fn visit_value(self, key: Self::Key, deserializer: D) -> Result where D: Deserializer<'de>, { + // doing this instead of using an enum as return value resolves the issue that the enum + // would take always take the size of the biggest element in the enum itself, + // which is not ideal. match key { - ExampleFieldDiscriminator::A => u8::deserialize(deserializer) - .map(ExampleField::A) - .attach(Location::Field("a")) - .change_context(VisitorError), - ExampleFieldDiscriminator::B => u16::deserialize(deserializer) - .map(ExampleField::B) - .attach(Location::Field("b")) - .change_context(VisitorError), - ExampleFieldDiscriminator::C => u32::deserialize(deserializer) - .map(ExampleField::C) - .attach(Location::Field("c")) - .change_context(VisitorError), + ExampleFieldDiscriminator::A => { + let value = u8::deserialize(deserializer) + .attach(Location::Field("a")) + .change_context(VisitorError)?; + + match self.a { + Some(_) => unimplemented!("planned in follow up PR"), + other => *other = Some(value), + }; + + Ok(()) + } + ExampleFieldDiscriminator::B => { + let value = u16::deserialize(deserializer) + .attach(Location::Field("b")) + .change_context(VisitorError)?; + + match self.b { + Some(_) => unimplemented!("planned in follow up PR"), + other => *other = Some(value), + } + + Ok(()) + } + ExampleFieldDiscriminator::C => { + let value = u32::deserialize(deserializer) + .attach(Location::Field("c")) + .change_context(VisitorError)?; + + match self.c { + Some(_) => unimplemented!("panned in follow up PR"), + other => *other = Some(value), + } + + Ok(()) + } } } } @@ -163,6 +188,7 @@ impl<'de> StructVisitor<'de> for ExampleVisitor { // due to set_bounded we make sure that even if implementations are not correct we are still // correct but doing `unwrap_or_else`. // TODO: we might be able to expose that through the type system? + // TODO: instead of doing this we need to use `NoneDeserializer` let a = array .next() .unwrap_or_else(|| { @@ -212,30 +238,23 @@ impl<'de> StructVisitor<'de> for ExampleVisitor { let mut errors: Result<(), ObjectAccessError> = Ok(()); - while let Some(field) = object.field(ExampleFieldVisitor) { - match field { - Err(error) => match &mut errors { + while let Some(field) = object.field(ExampleFieldVisitor { + a: &mut a, + b: &mut b, + c: &mut c, + }) { + if let Err(error) = field { + match &mut errors { Err(errors) => { errors.extend_one(error); } errors => *errors = Err(error), - }, - Ok(ExampleField::A(value)) => match &mut a { - // we have no error type for this! - Some(_) => unimplemented!("planned in follow up PR"), - a => *a = Some(value), - }, - Ok(ExampleField::B(value)) => match &mut b { - Some(_) => unimplemented!("planned in follow up PR"), - b => *b = Some(value), - }, - Ok(ExampleField::C(value)) => match &mut c { - Some(_) => unimplemented!("planned in follow up PR"), - c => *c = Some(value), - }, + } } } + // TODO: instead of doing this we need to use `NoneDeserializer`, this means that access + // needs to expose context! let a = a.ok_or_else(|| { Report::new(MissingError.into_error()) .attach(ExpectedType::new(u8::reflection())) From 180d7a7c9df7e6a32c0cdf014a1aa950ffc3a6a8 Mon Sep 17 00:00:00 2001 From: Bilal Mahmoud Date: Tue, 18 Apr 2023 13:49:08 +0200 Subject: [PATCH 12/19] chore: comments --- libs/deer/tests/test_struct_visitor.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/libs/deer/tests/test_struct_visitor.rs b/libs/deer/tests/test_struct_visitor.rs index f70b09a5d93..81ca64dec1a 100644 --- a/libs/deer/tests/test_struct_visitor.rs +++ b/libs/deer/tests/test_struct_visitor.rs @@ -254,7 +254,7 @@ impl<'de> StructVisitor<'de> for ExampleVisitor { } // TODO: instead of doing this we need to use `NoneDeserializer`, this means that access - // needs to expose context! + // needs to expose context! no it doesn't you doo doo head let a = a.ok_or_else(|| { Report::new(MissingError.into_error()) .attach(ExpectedType::new(u8::reflection())) @@ -371,8 +371,9 @@ fn struct_object_missing_err() { "location": [{"type": "field", "value": "b"}] } }, { + // TODO: this is the wrong error, it should be `key`.`missing` detailing the key we + // expected. This will be changed in a follow up PR ns: "deer", - // TODO: this should be key/discriminator missing! (or should this just silently fail?) id: ["value", "missing"], properties: { "expected": ExampleFieldDiscriminator::reflection(), @@ -408,6 +409,8 @@ fn struct_object_missing_multiple_err() { "location": [{"type": "field", "value": "c"}] } },{ + // TODO: this is the wrong error, it should be `key`.`missing` detailing the key we + // expected. This will be changed in a follow up PR ns: "deer", id: ["value", "missing"], properties: { @@ -415,6 +418,8 @@ fn struct_object_missing_multiple_err() { "location": [] } },{ + // TODO: this is the wrong error, it should be `key`.`missing` detailing the key we + // expected. This will be changed in a follow up PR ns: "deer", id: ["value", "missing"], properties: { From 9470e221a6232921fbd5649864e1cf98051c72b6 Mon Sep 17 00:00:00 2001 From: Bilal Mahmoud Date: Tue, 18 Apr 2023 13:53:35 +0200 Subject: [PATCH 13/19] chore: comments (II) --- libs/deer/tests/test_struct_visitor.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/libs/deer/tests/test_struct_visitor.rs b/libs/deer/tests/test_struct_visitor.rs index 81ca64dec1a..015dcc8c1f5 100644 --- a/libs/deer/tests/test_struct_visitor.rs +++ b/libs/deer/tests/test_struct_visitor.rs @@ -188,7 +188,8 @@ impl<'de> StructVisitor<'de> for ExampleVisitor { // due to set_bounded we make sure that even if implementations are not correct we are still // correct but doing `unwrap_or_else`. // TODO: we might be able to expose that through the type system? - // TODO: instead of doing this we need to use `NoneDeserializer` + // TODO: instead of doing this we need to use `NoneDeserializer`, + // this needs `.context()` on access rules to ensure proper ownership let a = array .next() .unwrap_or_else(|| { @@ -254,7 +255,7 @@ impl<'de> StructVisitor<'de> for ExampleVisitor { } // TODO: instead of doing this we need to use `NoneDeserializer`, this means that access - // needs to expose context! no it doesn't you doo doo head + // needs to expose context! let a = a.ok_or_else(|| { Report::new(MissingError.into_error()) .attach(ExpectedType::new(u8::reflection())) From bef152c6cb9e6c243e4942b699a314f8c9780cb2 Mon Sep 17 00:00:00 2001 From: Bilal Mahmoud Date: Wed, 19 Apr 2023 14:42:04 +0200 Subject: [PATCH 14/19] feat: use `NoneDeserializer` for values --- libs/deer/tests/test_struct_visitor.rs | 122 +++++++++---------------- 1 file changed, 45 insertions(+), 77 deletions(-) diff --git a/libs/deer/tests/test_struct_visitor.rs b/libs/deer/tests/test_struct_visitor.rs index 015dcc8c1f5..dc33e822bd3 100644 --- a/libs/deer/tests/test_struct_visitor.rs +++ b/libs/deer/tests/test_struct_visitor.rs @@ -1,8 +1,5 @@ use deer::{ - error::{ - ArrayAccessError, ArrayLengthError, DeserializeError, ExpectedLength, ReceivedLength, - Variant, VisitorError, - }, + error::{ArrayAccessError, DeserializeError, Variant, VisitorError}, ArrayAccess, Deserialize, Deserializer, Document, FieldVisitor, ObjectAccess, Reflection, Schema, StructVisitor, Visitor, }; @@ -14,11 +11,9 @@ mod common; use common::TupleExt; use deer::{ - error::{ - ExpectedField, ExpectedType, Location, MissingError, ObjectAccessError, ReceivedField, - UnknownFieldError, - }, + error::{ExpectedField, Location, ObjectAccessError, ReceivedField, UnknownFieldError}, schema::Reference, + value::NoneDeserializer, }; use deer_desert::{assert_tokens, assert_tokens_error, error, Token}; @@ -193,30 +188,27 @@ impl<'de> StructVisitor<'de> for ExampleVisitor { let a = array .next() .unwrap_or_else(|| { - Err(Report::new(ArrayLengthError.into_error()) - .attach(ExpectedLength::new(3)) - .attach(ReceivedLength::new(0)) - .change_context(ArrayAccessError)) + Deserialize::deserialize(NoneDeserializer::new(array.context())) + .attach(Location::Tuple(0)) + .change_context(ArrayAccessError) }) .attach(Location::Tuple(0)); let b = array .next() .unwrap_or_else(|| { - Err(Report::new(ArrayLengthError.into_error()) - .attach(ExpectedLength::new(3)) - .attach(ReceivedLength::new(1)) - .change_context(ArrayAccessError)) + Deserialize::deserialize(NoneDeserializer::new(array.context())) + .attach(Location::Tuple(1)) + .change_context(ArrayAccessError) }) .attach(Location::Tuple(1)); let c = array .next() .unwrap_or_else(|| { - Err(Report::new(ArrayLengthError.into_error()) - .attach(ExpectedLength::new(3)) - .attach(ReceivedLength::new(2)) - .change_context(ArrayAccessError)) + Deserialize::deserialize(NoneDeserializer::new(array.context())) + .attach(Location::Tuple(2)) + .change_context(ArrayAccessError) }) .attach(Location::Tuple(2)); @@ -231,8 +223,6 @@ impl<'de> StructVisitor<'de> for ExampleVisitor { where A: ObjectAccess<'de>, { - object.set_bounded(3).change_context(VisitorError)?; - let mut a = None; let mut b = None; let mut c = None; @@ -254,28 +244,32 @@ impl<'de> StructVisitor<'de> for ExampleVisitor { } } - // TODO: instead of doing this we need to use `NoneDeserializer`, this means that access - // needs to expose context! - let a = a.ok_or_else(|| { - Report::new(MissingError.into_error()) - .attach(ExpectedType::new(u8::reflection())) - .attach(Location::Field("a")) - .change_context(ObjectAccessError) - }); - - let b = b.ok_or_else(|| { - Report::new(MissingError.into_error()) - .attach(ExpectedType::new(u16::reflection())) - .attach(Location::Field("b")) - .change_context(ObjectAccessError) - }); - - let c = c.ok_or_else(|| { - Report::new(MissingError.into_error()) - .attach(ExpectedType::new(u32::reflection())) - .attach(Location::Field("c")) - .change_context(ObjectAccessError) - }); + let a = a.map_or_else( + || { + Deserialize::deserialize(NoneDeserializer::new(object.context())) + .attach(Location::Field("a")) + .change_context(ObjectAccessError) + }, + Ok, + ); + + let b = b.map_or_else( + || { + Deserialize::deserialize(NoneDeserializer::new(object.context())) + .attach(Location::Field("b")) + .change_context(ObjectAccessError) + }, + Ok, + ); + + let c = c.map_or_else( + || { + Deserialize::deserialize(NoneDeserializer::new(object.context())) + .attach(Location::Field("c")) + .change_context(ObjectAccessError) + }, + Ok, + ); let (a, b, c, ..) = (a, b, c, errors, object.end()) .fold_reports() @@ -371,15 +365,6 @@ fn struct_object_missing_err() { "expected": u16::reflection(), "location": [{"type": "field", "value": "b"}] } - }, { - // TODO: this is the wrong error, it should be `key`.`missing` detailing the key we - // expected. This will be changed in a follow up PR - ns: "deer", - id: ["value", "missing"], - properties: { - "expected": ExampleFieldDiscriminator::reflection(), - "location": [] - } }]), &[ Token::Object { length: Some(2) }, @@ -409,24 +394,6 @@ fn struct_object_missing_multiple_err() { "expected": u32::reflection(), "location": [{"type": "field", "value": "c"}] } - },{ - // TODO: this is the wrong error, it should be `key`.`missing` detailing the key we - // expected. This will be changed in a follow up PR - ns: "deer", - id: ["value", "missing"], - properties: { - "expected": ExampleFieldDiscriminator::reflection(), - "location": [] - } - },{ - // TODO: this is the wrong error, it should be `key`.`missing` detailing the key we - // expected. This will be changed in a follow up PR - ns: "deer", - id: ["value", "missing"], - properties: { - "expected": ExampleFieldDiscriminator::reflection(), - "location": [] - } }]), &[ Token::Object { length: Some(1) }, @@ -442,10 +409,10 @@ fn struct_object_too_many_err() { assert_tokens_error::( &error!([{ ns: "deer", - id: ["object", "length"], + id: ["unknown", "field"], properties: { - "expected": 3, - "received": 4, + "expected": ["a", "b", "c"], + "received": ["d"], "location": [] } }]), @@ -457,6 +424,7 @@ fn struct_object_too_many_err() { Token::Number(3.into()), Token::Str("c"), Token::Number(4.into()), + // TODO: the token deserializer does not skip correctly :/ Token::Str("d"), Token::Number(5.into()), Token::ObjectEnd, @@ -473,10 +441,10 @@ fn struct_object_duplicate_err() { assert_tokens_error::( &error!([{ ns: "deer", - id: ["object", "length"], + id: ["unknown", "field"], properties: { - "expected": 3, - "received": 4, + "expected": ["a", "b", "c"], + "received": ["d"], "location": [] } }]), From 0589a237676e191d5a78a71a0e2d9b96b85e7b75 Mon Sep 17 00:00:00 2001 From: Bilal Mahmoud Date: Wed, 19 Apr 2023 14:42:27 +0200 Subject: [PATCH 15/19] fix: drive-by: generalize skip logic, fix regression, skip value if key is wrong --- libs/deer/desert/src/array.rs | 31 ++----------------- libs/deer/desert/src/deserializer.rs | 4 --- libs/deer/desert/src/lib.rs | 1 + libs/deer/desert/src/object.rs | 38 +++++------------------ libs/deer/desert/src/skip.rs | 45 ++++++++++++++++++++++++++++ 5 files changed, 56 insertions(+), 63 deletions(-) create mode 100644 libs/deer/desert/src/skip.rs diff --git a/libs/deer/desert/src/array.rs b/libs/deer/desert/src/array.rs index 23deb297846..ca15c135925 100644 --- a/libs/deer/desert/src/array.rs +++ b/libs/deer/desert/src/array.rs @@ -9,6 +9,7 @@ use error_stack::{Report, Result, ResultExt}; use crate::{ deserializer::{Deserializer, DeserializerNone}, + skip::skip_tokens, token::Token, }; @@ -29,31 +30,6 @@ impl<'a, 'b, 'de> ArrayAccess<'a, 'b, 'de> { remaining: None, } } - - fn scan_end(&self) -> Option { - let mut objects: usize = 0; - let mut arrays: usize = 0; - - let mut n = 0; - - loop { - let token = self.deserializer.peek_n(n)?; - - match token { - Token::Array { .. } => arrays += 1, - Token::ArrayEnd if arrays == 0 && objects == 0 => { - // we're at the outer layer, meaning we can know where we end - return Some(n); - } - Token::ArrayEnd => arrays = arrays.saturating_sub(1), - Token::Object { .. } => objects += 1, - Token::ObjectEnd => objects = objects.saturating_sub(1), - _ => {} - } - - n += 1; - } - } } impl<'de> deer::ArrayAccess<'de> for ArrayAccess<'_, '_, 'de> { @@ -133,10 +109,7 @@ impl<'de> deer::ArrayAccess<'de> for ArrayAccess<'_, '_, 'de> { }; // bump until the very end, which ensures that deserialize calls after this might succeed! - let bump = self - .scan_end() - .map_or_else(|| self.deserializer.tape().remaining(), |index| index + 1); - self.deserializer.tape_mut().bump_n(bump); + skip_tokens(self.deserializer, &Token::Array { length: None }); if let Some(remaining) = self.remaining { if remaining > 0 { diff --git a/libs/deer/desert/src/deserializer.rs b/libs/deer/desert/src/deserializer.rs index 96034f3fee8..0b68f18ede9 100644 --- a/libs/deer/desert/src/deserializer.rs +++ b/libs/deer/desert/src/deserializer.rs @@ -181,10 +181,6 @@ impl<'a, 'de> Deserializer<'a, 'de> { self.tape.next().expect("should have token to deserialize") } - pub(crate) const fn tape(&self) -> &Tape<'de> { - &self.tape - } - pub(crate) fn tape_mut(&mut self) -> &mut Tape<'de> { &mut self.tape } diff --git a/libs/deer/desert/src/lib.rs b/libs/deer/desert/src/lib.rs index d6c922615bf..fccb746d00f 100644 --- a/libs/deer/desert/src/lib.rs +++ b/libs/deer/desert/src/lib.rs @@ -9,6 +9,7 @@ pub mod error; pub(crate) mod object; pub(crate) mod tape; mod token; +mod skip; pub use assert::{ assert_tokens, assert_tokens_any_error, assert_tokens_deserialize, assert_tokens_error, diff --git a/libs/deer/desert/src/object.rs b/libs/deer/desert/src/object.rs index dea443e2cec..c8b1a9278b1 100644 --- a/libs/deer/desert/src/object.rs +++ b/libs/deer/desert/src/object.rs @@ -9,6 +9,7 @@ use error_stack::{Report, Result, ResultExt}; use crate::{ deserializer::{Deserializer, DeserializerNone}, + skip::skip_tokens, token::Token, }; @@ -29,31 +30,6 @@ impl<'a, 'b, 'de: 'a> ObjectAccess<'a, 'b, 'de> { consumed: 0, } } - - fn scan_end(&self) -> Option { - let mut objects: usize = 0; - let mut arrays: usize = 0; - - let mut n = 0; - - loop { - let token = self.deserializer.peek_n(n)?; - - match token { - Token::Array { .. } => arrays += 1, - Token::ArrayEnd => arrays = arrays.saturating_sub(1), - Token::Object { .. } => objects += 1, - Token::ObjectEnd if arrays == 0 && objects == 0 => { - // we're at the outer layer, meaning we can know where we end - return Some(n); - } - Token::ObjectEnd => objects = objects.saturating_sub(1), - _ => {} - } - - n += 1; - } - } } impl<'de> deer::ObjectAccess<'de> for ObjectAccess<'_, '_, 'de> { @@ -115,6 +91,12 @@ impl<'de> deer::ObjectAccess<'de> for ObjectAccess<'_, '_, 'de> { } else { let key = access.visit_key(&mut *self.deserializer); + if key.is_err() { + // the key is an error, we need to swallow the value + let next = self.deserializer.next(); + skip_tokens(self.deserializer, &next); + } + key.and_then(|key| access.visit_value(key, &mut *self.deserializer)) }; @@ -141,11 +123,7 @@ impl<'de> deer::ObjectAccess<'de> for ObjectAccess<'_, '_, 'de> { }; // bump until the very end, which ensures that deserialize calls after this might succeed! - let bump = self - .scan_end() - .unwrap_or_else(|| self.deserializer.tape().remaining()); - - self.deserializer.tape_mut().bump_n(bump + 1); + skip_tokens(self.deserializer, &Token::Object { length: None }); if let Some(remaining) = self.remaining { if remaining > 0 { diff --git a/libs/deer/desert/src/skip.rs b/libs/deer/desert/src/skip.rs new file mode 100644 index 00000000000..7de9b35baba --- /dev/null +++ b/libs/deer/desert/src/skip.rs @@ -0,0 +1,45 @@ +use crate::{deserializer::Deserializer, Token}; + +fn scan_object(deserializer: &Deserializer, stop: Token) -> usize { + let mut objects: usize = 0; + let mut arrays: usize = 0; + + let mut n = 0; + + loop { + let Some(token) = deserializer.peek_n(n) else { + // we're at the end + return n; + }; + + if token == stop && arrays == 0 && objects == 0 { + // we're at the outer layer, meaning we can know where we end + // need to increment by one as we want to also skip the ObjectEnd + return n + 1; + } + + match token { + Token::Array { .. } => arrays += 1, + Token::ArrayEnd => arrays = arrays.saturating_sub(1), + Token::Object { .. } => objects += 1, + Token::ObjectEnd => objects = objects.saturating_sub(1), + _ => {} + } + + n += 1; + } +} + +/// Skips all tokens required for the start token, be aware that the start token should no longer be +/// on the tape +pub(crate) fn skip_tokens(deserializer: &mut Deserializer, start: &Token) { + let n = match start { + Token::Array { .. } => scan_object(&*deserializer, Token::ArrayEnd), + Token::Object { .. } => scan_object(&*deserializer, Token::ObjectEnd), + _ => 0, + }; + + if n > 0 { + deserializer.tape_mut().bump_n(n); + } +} From 4bc447f13fe3d8719dbedd641e0d3ef7e09edc2c Mon Sep 17 00:00:00 2001 From: Bilal Mahmoud Date: Wed, 19 Apr 2023 14:45:04 +0200 Subject: [PATCH 16/19] fix: clippy --- libs/deer/desert/src/skip.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/libs/deer/desert/src/skip.rs b/libs/deer/desert/src/skip.rs index 7de9b35baba..7424a9668cf 100644 --- a/libs/deer/desert/src/skip.rs +++ b/libs/deer/desert/src/skip.rs @@ -1,6 +1,6 @@ use crate::{deserializer::Deserializer, Token}; -fn scan_object(deserializer: &Deserializer, stop: Token) -> usize { +fn scan_object(deserializer: &Deserializer, stop: &Token) -> usize { let mut objects: usize = 0; let mut arrays: usize = 0; @@ -34,8 +34,8 @@ fn scan_object(deserializer: &Deserializer, stop: Token) -> usize { /// on the tape pub(crate) fn skip_tokens(deserializer: &mut Deserializer, start: &Token) { let n = match start { - Token::Array { .. } => scan_object(&*deserializer, Token::ArrayEnd), - Token::Object { .. } => scan_object(&*deserializer, Token::ObjectEnd), + Token::Array { .. } => scan_object(&*deserializer, &Token::ArrayEnd), + Token::Object { .. } => scan_object(&*deserializer, &Token::ObjectEnd), _ => 0, }; From 2a2fa12f92ddd6636db4fc793b952a41d3fb9787 Mon Sep 17 00:00:00 2001 From: Bilal Mahmoud Date: Wed, 19 Apr 2023 14:48:34 +0200 Subject: [PATCH 17/19] fix: clippy --- libs/deer/desert/src/skip.rs | 2 +- libs/deer/tests/test_struct_visitor.rs | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/libs/deer/desert/src/skip.rs b/libs/deer/desert/src/skip.rs index 7424a9668cf..f6e1b36057c 100644 --- a/libs/deer/desert/src/skip.rs +++ b/libs/deer/desert/src/skip.rs @@ -12,7 +12,7 @@ fn scan_object(deserializer: &Deserializer, stop: &Token) -> usize { return n; }; - if token == stop && arrays == 0 && objects == 0 { + if token == *stop && arrays == 0 && objects == 0 { // we're at the outer layer, meaning we can know where we end // need to increment by one as we want to also skip the ObjectEnd return n + 1; diff --git a/libs/deer/tests/test_struct_visitor.rs b/libs/deer/tests/test_struct_visitor.rs index dc33e822bd3..7fbd00c9879 100644 --- a/libs/deer/tests/test_struct_visitor.rs +++ b/libs/deer/tests/test_struct_visitor.rs @@ -424,7 +424,6 @@ fn struct_object_too_many_err() { Token::Number(3.into()), Token::Str("c"), Token::Number(4.into()), - // TODO: the token deserializer does not skip correctly :/ Token::Str("d"), Token::Number(5.into()), Token::ObjectEnd, From 7b8282a8c688efb73171a3555f87041e1b7d9213 Mon Sep 17 00:00:00 2001 From: Bilal Mahmoud Date: Wed, 19 Apr 2023 15:01:02 +0200 Subject: [PATCH 18/19] fix: rustfmt --- libs/deer/desert/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/deer/desert/src/lib.rs b/libs/deer/desert/src/lib.rs index fccb746d00f..ecf4306fb4f 100644 --- a/libs/deer/desert/src/lib.rs +++ b/libs/deer/desert/src/lib.rs @@ -7,9 +7,9 @@ mod assert; mod deserializer; pub mod error; pub(crate) mod object; +mod skip; pub(crate) mod tape; mod token; -mod skip; pub use assert::{ assert_tokens, assert_tokens_any_error, assert_tokens_deserialize, assert_tokens_error, From cbecc2319ccdb0ff016f9b1bd58a77f44f939d65 Mon Sep 17 00:00:00 2001 From: Bilal Mahmoud Date: Fri, 28 Apr 2023 18:13:22 +0200 Subject: [PATCH 19/19] fix: remove reflection of removed tokens --- libs/deer/desert/src/token.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/libs/deer/desert/src/token.rs b/libs/deer/desert/src/token.rs index 3dcbd12b867..aa6c088babd 100644 --- a/libs/deer/desert/src/token.rs +++ b/libs/deer/desert/src/token.rs @@ -205,8 +205,6 @@ impl Token { Self::Number(_) => Document::new::(), Self::U128(_) => Document::new::(), Self::I128(_) => Document::new::(), - Self::USize(_) => Document::new::(), - Self::ISize(_) => Document::new::(), Self::Char(_) => Document::new::(), Self::Str(_) | Self::BorrowedStr(_) | Self::String(_) => Document::new::(), Self::Bytes(_) | Self::BorrowedBytes(_) | Self::BytesBuf(_) => Document::new::<[u8]>(),