diff --git a/boa/src/builtins/json/mod.rs b/boa/src/builtins/json/mod.rs index f738292bcd..ed4d3e97d1 100644 --- a/boa/src/builtins/json/mod.rs +++ b/boa/src/builtins/json/mod.rs @@ -13,8 +13,6 @@ //! [json]: https://www.json.org/json-en.html //! [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/JSON -use std::collections::HashSet; - use crate::{ builtins::{ string::{is_leading_surrogate, is_trailing_surrogate}, @@ -172,7 +170,7 @@ impl Json { // ii. If isArray is true, then if replacer_obj.is_array() { // 1. Set PropertyList to a new empty List. - let mut property_set = HashSet::new(); + let mut property_set = indexmap::IndexSet::new(); // 2. Let len be ? LengthOfArrayLike(replacer). let len = replacer_obj.length_of_array_like(context)?; @@ -477,7 +475,7 @@ impl Json { state.indent = JsString::concat(&state.indent, &state.gap); // 5. If state.[[PropertyList]] is not undefined, then - let mut k = if let Some(p) = &state.property_list { + let k = if let Some(p) = &state.property_list { // a. Let K be state.[[PropertyList]]. p.clone() // 6. Else, @@ -488,9 +486,6 @@ impl Json { keys.iter().map(|v| v.to_string(context).unwrap()).collect() }; - // Sort the property key list, because the internal property hashmap of objects does not sort the properties. - k.sort(); - // 7. Let partial be a new empty List. let mut partial = Vec::new(); diff --git a/boa/src/object/property_map.rs b/boa/src/object/property_map.rs index 04c3a0edbe..4f5c98686d 100644 --- a/boa/src/object/property_map.rs +++ b/boa/src/object/property_map.rs @@ -1,18 +1,38 @@ use super::{PropertyDescriptor, PropertyKey}; use crate::{ - gc::{Finalize, Trace}, + gc::{custom_trace, Finalize, Trace}, JsString, JsSymbol, }; -use rustc_hash::FxHashMap; -use std::{collections::hash_map, iter::FusedIterator}; +use indexmap::IndexMap; +use rustc_hash::{FxHashMap, FxHasher}; +use std::{collections::hash_map, hash::BuildHasherDefault, iter::FusedIterator}; + +/// Wrapper around indexmap::IndexMap for usage in PropertyMap +#[derive(Debug, Finalize)] +struct OrderedHashMap(IndexMap>); + +impl Default for OrderedHashMap { + fn default() -> Self { + Self(IndexMap::with_hasher(BuildHasherDefault::default())) + } +} + +unsafe impl Trace for OrderedHashMap { + custom_trace!(this, { + for (k, v) in this.0.iter() { + mark(k); + mark(v); + } + }); +} #[derive(Default, Debug, Trace, Finalize)] pub struct PropertyMap { indexed_properties: FxHashMap, /// Properties - string_properties: FxHashMap, + string_properties: OrderedHashMap, /// Symbol Properties - symbol_properties: FxHashMap, + symbol_properties: OrderedHashMap, } impl PropertyMap { @@ -22,8 +42,8 @@ impl PropertyMap { pub fn get(&self, key: &PropertyKey) -> Option<&PropertyDescriptor> { match key { PropertyKey::Index(index) => self.indexed_properties.get(index), - PropertyKey::String(string) => self.string_properties.get(string), - PropertyKey::Symbol(symbol) => self.symbol_properties.get(symbol), + PropertyKey::String(string) => self.string_properties.0.get(string), + PropertyKey::Symbol(symbol) => self.symbol_properties.0.get(symbol), } } @@ -34,16 +54,20 @@ impl PropertyMap { ) -> Option { match &key { PropertyKey::Index(index) => self.indexed_properties.insert(*index, property), - PropertyKey::String(string) => self.string_properties.insert(string.clone(), property), - PropertyKey::Symbol(symbol) => self.symbol_properties.insert(symbol.clone(), property), + PropertyKey::String(string) => { + self.string_properties.0.insert(string.clone(), property) + } + PropertyKey::Symbol(symbol) => { + self.symbol_properties.0.insert(symbol.clone(), property) + } } } pub fn remove(&mut self, key: &PropertyKey) -> Option { match key { PropertyKey::Index(index) => self.indexed_properties.remove(index), - PropertyKey::String(string) => self.string_properties.remove(string), - PropertyKey::Symbol(symbol) => self.symbol_properties.remove(symbol), + PropertyKey::String(string) => self.string_properties.0.shift_remove(string), + PropertyKey::Symbol(symbol) => self.symbol_properties.0.shift_remove(symbol), } } @@ -54,8 +78,8 @@ impl PropertyMap { pub fn iter(&self) -> Iter<'_> { Iter { indexed_properties: self.indexed_properties.iter(), - string_properties: self.string_properties.iter(), - symbol_properties: self.symbol_properties.iter(), + string_properties: self.string_properties.0.iter(), + symbol_properties: self.symbol_properties.0.iter(), } } @@ -81,7 +105,7 @@ impl PropertyMap { /// This iterator does not recurse down the prototype chain. #[inline] pub fn symbol_properties(&self) -> SymbolProperties<'_> { - SymbolProperties(self.symbol_properties.iter()) + SymbolProperties(self.symbol_properties.0.iter()) } /// An iterator visiting all symbol keys in arbitrary order. The iterator element type is `&'a RcSymbol`. @@ -89,7 +113,7 @@ impl PropertyMap { /// This iterator does not recurse down the prototype chain. #[inline] pub fn symbol_property_keys(&self) -> SymbolPropertyKeys<'_> { - SymbolPropertyKeys(self.symbol_properties.keys()) + SymbolPropertyKeys(self.symbol_properties.0.keys()) } /// An iterator visiting all symbol values in arbitrary order. The iterator element type is `&'a Property`. @@ -97,7 +121,7 @@ impl PropertyMap { /// This iterator does not recurse down the prototype chain. #[inline] pub fn symbol_property_values(&self) -> SymbolPropertyValues<'_> { - SymbolPropertyValues(self.symbol_properties.values()) + SymbolPropertyValues(self.symbol_properties.0.values()) } /// An iterator visiting all indexed key-value pairs in arbitrary order. The iterator element type is `(&'a u32, &'a Property)`. @@ -129,7 +153,7 @@ impl PropertyMap { /// This iterator does not recurse down the prototype chain. #[inline] pub fn string_properties(&self) -> StringProperties<'_> { - StringProperties(self.string_properties.iter()) + StringProperties(self.string_properties.0.iter()) } /// An iterator visiting all string keys in arbitrary order. The iterator element type is `&'a RcString`. @@ -137,7 +161,7 @@ impl PropertyMap { /// This iterator does not recurse down the prototype chain. #[inline] pub fn string_property_keys(&self) -> StringPropertyKeys<'_> { - StringPropertyKeys(self.string_properties.keys()) + StringPropertyKeys(self.string_properties.0.keys()) } /// An iterator visiting all string values in arbitrary order. The iterator element type is `&'a Property`. @@ -145,15 +169,15 @@ impl PropertyMap { /// This iterator does not recurse down the prototype chain. #[inline] pub fn string_property_values(&self) -> StringPropertyValues<'_> { - StringPropertyValues(self.string_properties.values()) + StringPropertyValues(self.string_properties.0.values()) } #[inline] pub fn contains_key(&self, key: &PropertyKey) -> bool { match key { PropertyKey::Index(index) => self.indexed_properties.contains_key(index), - PropertyKey::String(string) => self.string_properties.contains_key(string), - PropertyKey::Symbol(symbol) => self.symbol_properties.contains_key(symbol), + PropertyKey::String(string) => self.string_properties.0.contains_key(string), + PropertyKey::Symbol(symbol) => self.symbol_properties.0.contains_key(symbol), } } } @@ -162,8 +186,8 @@ impl PropertyMap { #[derive(Debug, Clone)] pub struct Iter<'a> { indexed_properties: hash_map::Iter<'a, u32, PropertyDescriptor>, - string_properties: hash_map::Iter<'a, JsString, PropertyDescriptor>, - symbol_properties: hash_map::Iter<'a, JsSymbol, PropertyDescriptor>, + string_properties: indexmap::map::Iter<'a, JsString, PropertyDescriptor>, + symbol_properties: indexmap::map::Iter<'a, JsSymbol, PropertyDescriptor>, } impl<'a> Iterator for Iter<'a> { @@ -233,7 +257,7 @@ impl FusedIterator for Values<'_> {} /// An iterator over the `Symbol` property entries of an `Object` #[derive(Debug, Clone)] -pub struct SymbolProperties<'a>(hash_map::Iter<'a, JsSymbol, PropertyDescriptor>); +pub struct SymbolProperties<'a>(indexmap::map::Iter<'a, JsSymbol, PropertyDescriptor>); impl<'a> Iterator for SymbolProperties<'a> { type Item = (&'a JsSymbol, &'a PropertyDescriptor); @@ -260,7 +284,7 @@ impl FusedIterator for SymbolProperties<'_> {} /// An iterator over the keys (`RcSymbol`) of an `Object`. #[derive(Debug, Clone)] -pub struct SymbolPropertyKeys<'a>(hash_map::Keys<'a, JsSymbol, PropertyDescriptor>); +pub struct SymbolPropertyKeys<'a>(indexmap::map::Keys<'a, JsSymbol, PropertyDescriptor>); impl<'a> Iterator for SymbolPropertyKeys<'a> { type Item = &'a JsSymbol; @@ -287,7 +311,7 @@ impl FusedIterator for SymbolPropertyKeys<'_> {} /// An iterator over the `Symbol` values (`Property`) of an `Object`. #[derive(Debug, Clone)] -pub struct SymbolPropertyValues<'a>(hash_map::Values<'a, JsSymbol, PropertyDescriptor>); +pub struct SymbolPropertyValues<'a>(indexmap::map::Values<'a, JsSymbol, PropertyDescriptor>); impl<'a> Iterator for SymbolPropertyValues<'a> { type Item = &'a PropertyDescriptor; @@ -395,7 +419,7 @@ impl FusedIterator for IndexPropertyValues<'_> {} /// An iterator over the `String` property entries of an `Object` #[derive(Debug, Clone)] -pub struct StringProperties<'a>(hash_map::Iter<'a, JsString, PropertyDescriptor>); +pub struct StringProperties<'a>(indexmap::map::Iter<'a, JsString, PropertyDescriptor>); impl<'a> Iterator for StringProperties<'a> { type Item = (&'a JsString, &'a PropertyDescriptor); @@ -422,7 +446,7 @@ impl FusedIterator for StringProperties<'_> {} /// An iterator over the string keys (`RcString`) of an `Object`. #[derive(Debug, Clone)] -pub struct StringPropertyKeys<'a>(hash_map::Keys<'a, JsString, PropertyDescriptor>); +pub struct StringPropertyKeys<'a>(indexmap::map::Keys<'a, JsString, PropertyDescriptor>); impl<'a> Iterator for StringPropertyKeys<'a> { type Item = &'a JsString; @@ -449,7 +473,7 @@ impl FusedIterator for StringPropertyKeys<'_> {} /// An iterator over the string values (`Property`) of an `Object`. #[derive(Debug, Clone)] -pub struct StringPropertyValues<'a>(hash_map::Values<'a, JsString, PropertyDescriptor>); +pub struct StringPropertyValues<'a>(indexmap::map::Values<'a, JsString, PropertyDescriptor>); impl<'a> Iterator for StringPropertyValues<'a> { type Item = &'a PropertyDescriptor; diff --git a/boa/src/object/tests.rs b/boa/src/object/tests.rs index 8cbfe2cefc..9bf24ecf59 100644 --- a/boa/src/object/tests.rs +++ b/boa/src/object/tests.rs @@ -1,4 +1,4 @@ -use crate::exec; +use crate::{check_output, exec, TestAction}; #[test] fn ordinary_has_instance_nonobject_prototype() { @@ -17,3 +17,30 @@ fn ordinary_has_instance_nonobject_prototype() { "\"TypeError: function has non-object prototype in instanceof check\"" ); } + +#[test] +fn object_properties_return_order() { + let scenario = r#" + var o = { + p1: 'v1', + p2: 'v2', + p3: 'v3', + }; + o.p4 = 'v4'; + o[2] = 'iv2'; + o[0] = 'iv0'; + o[1] = 'iv1'; + delete o.p1; + delete o.p3; + o.p1 = 'v1'; + "#; + + check_output(&[ + TestAction::Execute(scenario), + TestAction::TestEq("Object.keys(o)", r#"[ "0", "1", "2", "p2", "p4", "p1" ]"#), + TestAction::TestEq( + "Object.values(o)", + r#"[ "iv0", "iv1", "iv2", "v2", "v4", "v1" ]"#, + ), + ]); +}