diff --git a/serde_v8/de.rs b/serde_v8/de.rs index dff29b8f93806b..87e3fb1f672db3 100644 --- a/serde_v8/de.rs +++ b/serde_v8/de.rs @@ -1,5 +1,5 @@ // Copyright 2018-2022 the Deno authors. All rights reserved. MIT license. -use serde::de::{self, Visitor}; +use serde::de::{self, SeqAccess as _, Visitor}; use serde::Deserialize; use crate::error::{Error, Result}; @@ -259,15 +259,7 @@ impl<'de, 'a, 'b, 's, 'x> de::Deserializer<'de> { let arr = v8::Local::::try_from(self.input) .map_err(|_| Error::ExpectedArray)?; - let len = arr.length(); - let obj = v8::Local::::from(arr); - let seq = SeqAccess { - pos: 0, - len, - obj, - scope: self.scope, - }; - visitor.visit_seq(seq) + visitor.visit_seq(SeqAccess::new(arr.into(), self.scope, 0..arr.length())) } // Like deserialize_seq except it prefers tuple's length over input array's length @@ -283,13 +275,7 @@ impl<'de, 'a, 'b, 's, 'x> de::Deserializer<'de> return Err(Error::LengthMismatch); } } - let seq = SeqAccess { - pos: 0, - len: len as u32, - obj, - scope: self.scope, - }; - visitor.visit_seq(seq) + visitor.visit_seq(SeqAccess::new(obj, self.scope, 0..len as u32)) } // Tuple structs look just like sequences in JSON. @@ -325,24 +311,7 @@ impl<'de, 'a, 'b, 's, 'x> de::Deserializer<'de> }; visitor.visit_map(map) } else { - let prop_names = obj.get_own_property_names( - self.scope, - v8::GetPropertyNamesArgsBuilder::new() - .key_conversion(v8::KeyConversionMode::ConvertToString) - .build(), - ); - let keys: Vec = match prop_names { - Some(names) => from_v8(self.scope, names.into()).unwrap(), - None => vec![], - }; - - let map = MapObjectAccess { - obj, - keys: keys.into_iter(), - next_value: None, - scope: self.scope, - }; - visitor.visit_map(map) + visitor.visit_map(MapObjectAccess::new(obj, self.scope)) } } @@ -376,24 +345,12 @@ impl<'de, 'a, 'b, 's, 'x> de::Deserializer<'de> } _ => { // Regular struct - let obj: v8::Local = - self.input.try_into().or(Err(Error::ExpectedObject))?; + let obj = v8::Local::::try_from(self.input) + .or(Err(Error::ExpectedObject))?; // Fields names are a hint and must be inferred when not provided if fields.is_empty() { - let prop_names = - obj.get_own_property_names(self.scope, Default::default()); - let keys: Vec = match prop_names { - Some(names) => from_v8(self.scope, names.into()).unwrap(), - None => vec![], - }; - - visitor.visit_map(MapObjectAccess { - obj, - scope: self.scope, - keys: keys.into_iter(), - next_value: None, - }) + visitor.visit_map(MapObjectAccess::new(obj, self.scope)) } else { visitor.visit_map(StructAccess { obj, @@ -491,9 +448,31 @@ impl<'de, 'a, 'b, 's, 'x> de::Deserializer<'de> struct MapObjectAccess<'a, 's> { obj: v8::Local<'a, v8::Object>, - scope: &'a mut v8::HandleScope<'s>, - keys: std::vec::IntoIter>, - next_value: Option>, + keys: SeqAccess<'a, 's>, + next_value: Option>, +} + +impl<'a, 's> MapObjectAccess<'a, 's> { + pub fn new( + obj: v8::Local<'a, v8::Object>, + scope: &'a mut v8::HandleScope<'s>, + ) -> Self { + let keys = match obj.get_own_property_names( + scope, + v8::GetPropertyNamesArgsBuilder::new() + .key_conversion(v8::KeyConversionMode::ConvertToString) + .build(), + ) { + Some(keys) => SeqAccess::new(keys.into(), scope, 0..keys.length()), + None => SeqAccess::new(obj, scope, 0..0), + }; + + Self { + obj, + keys, + next_value: None, + } + } } impl<'de> de::MapAccess<'de> for MapObjectAccess<'_, '_> { @@ -503,14 +482,15 @@ impl<'de> de::MapAccess<'de> for MapObjectAccess<'_, '_> { &mut self, seed: K, ) -> Result> { - for key in self.keys.by_ref() { - let v8_val = self.obj.get(self.scope, key.v8_value).unwrap(); + while let Some(key) = self.keys.next_element::()? { + let v8_val = self.obj.get(self.keys.scope, key.v8_value).unwrap(); if v8_val.is_undefined() { // Historically keys/value pairs with undefined values are not added to the output continue; } self.next_value = Some(v8_val); - let mut deserializer = Deserializer::new(self.scope, key.v8_value, None); + let mut deserializer = + Deserializer::new(self.keys.scope, key.v8_value, None); return seed.deserialize(&mut deserializer).map(Some); } Ok(None) @@ -524,12 +504,12 @@ impl<'de> de::MapAccess<'de> for MapObjectAccess<'_, '_> { .next_value .take() .expect("Call next_key_seed before next_value_seed"); - let mut deserializer = Deserializer::new(self.scope, v8_val, None); + let mut deserializer = Deserializer::new(self.keys.scope, v8_val, None); seed.deserialize(&mut deserializer) } fn size_hint(&self) -> Option { - Some(self.keys.len()) + self.keys.size_hint() } } @@ -574,14 +554,14 @@ impl<'de> de::MapAccess<'de> for MapPairsAccess<'_, '_> { } } -struct StructAccess<'a, 'b, 's> { +struct StructAccess<'a, 's> { obj: v8::Local<'a, v8::Object>, - scope: &'b mut v8::HandleScope<'s>, + scope: &'a mut v8::HandleScope<'s>, keys: std::slice::Iter<'static, &'static str>, - next_value: Option>, + next_value: Option>, } -impl<'de> de::MapAccess<'de> for StructAccess<'_, '_, '_> { +impl<'de> de::MapAccess<'de> for StructAccess<'_, '_> { type Error = Error; fn next_key_seed(&mut self, seed: K) -> Result> @@ -615,34 +595,40 @@ impl<'de> de::MapAccess<'de> for StructAccess<'_, '_, '_> { } } -struct SeqAccess<'a, 'b, 's> { +struct SeqAccess<'a, 's> { obj: v8::Local<'a, v8::Object>, - scope: &'b mut v8::HandleScope<'s>, - len: u32, - pos: u32, + scope: &'a mut v8::HandleScope<'s>, + range: std::ops::Range, +} + +impl<'a, 's> SeqAccess<'a, 's> { + pub fn new( + obj: v8::Local<'a, v8::Object>, + scope: &'a mut v8::HandleScope<'s>, + range: std::ops::Range, + ) -> Self { + Self { obj, scope, range } + } } -impl<'de> de::SeqAccess<'de> for SeqAccess<'_, '_, '_> { +impl<'de> de::SeqAccess<'de> for SeqAccess<'_, '_> { type Error = Error; fn next_element_seed>( &mut self, seed: T, ) -> Result> { - let pos = self.pos; - self.pos += 1; - - if pos < self.len { + if let Some(pos) = self.range.next() { let val = self.obj.get_index(self.scope, pos).unwrap(); let mut deserializer = Deserializer::new(self.scope, val, None); - Ok(Some(seed.deserialize(&mut deserializer)?)) + seed.deserialize(&mut deserializer).map(Some) } else { Ok(None) } } fn size_hint(&self) -> Option { - Some((self.len - self.pos) as usize) + self.range.size_hint().1 } } diff --git a/serde_v8/tests/de.rs b/serde_v8/tests/de.rs index c67b4012c55668..0f2c85d95f589f 100644 --- a/serde_v8/tests/de.rs +++ b/serde_v8/tests/de.rs @@ -93,6 +93,7 @@ detest!(de_bool, bool, "true", true); detest!(de_char, char, "'é'", 'é'); detest!(de_u64, u64, "32", 32); detest!(de_string, String, "'Hello'", "Hello".to_owned()); +detest!(de_vec_empty, Vec, "[]", vec![0; 0]); detest!(de_vec_u64, Vec, "[1,2,3,4,5]", vec![1, 2, 3, 4, 5]); detest!( de_vec_str, @@ -107,7 +108,13 @@ detest!( (123, true, ()) ); defail!( - de_tuple_wrong_len, + de_tuple_wrong_len_short, + (u64, bool, ()), + "[123, true]", + |e| e == Err(Error::LengthMismatch) +); +defail!( + de_tuple_wrong_len_long, (u64, bool, ()), "[123, true, null, 'extra']", |e| e == Err(Error::LengthMismatch)