diff --git a/core/engine/src/builtins/map/mod.rs b/core/engine/src/builtins/map/mod.rs index 47d87893356..0cf9f8be440 100644 --- a/core/engine/src/builtins/map/mod.rs +++ b/core/engine/src/builtins/map/mod.rs @@ -25,14 +25,14 @@ use crate::{ use num_traits::Zero; use super::{ - BuiltInBuilder, BuiltInConstructor, IntrinsicObject, iterable::if_abrupt_close_iterator, + BuiltInBuilder, BuiltInConstructor, IntrinsicObject, canonicalize_keyed_collection_value, + iterable::if_abrupt_close_iterator, }; mod map_iterator; pub(crate) use map_iterator::MapIterator; pub mod ordered_map; -use crate::value::JsVariant; use ordered_map::OrderedMap; #[cfg(test)] @@ -86,6 +86,12 @@ impl IntrinsicObject for Map { .method(Self::keys, js_string!("keys"), 0) .method(Self::set, js_string!("set"), 2) .method(Self::values, js_string!("values"), 0) + .method(Self::get_or_insert, js_string!("getOrInsert"), 2) + .method( + Self::get_or_insert_computed, + js_string!("getOrInsertComputed"), + 2, + ) .accessor( js_string!("size"), Some(get_size), @@ -229,29 +235,18 @@ impl Map { /// [spec]: https://tc39.es/ecma262/#sec-map.prototype.set /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Map/set pub(crate) fn set(this: &JsValue, args: &[JsValue], _: &mut Context) -> JsResult { - let key = args.get_or_undefined(0); let value = args.get_or_undefined(1); // 1. Let M be the this value. // 2. Perform ? RequireInternalSlot(M, [[MapData]]). - // 3. Let entries be the List that is M.[[MapData]]. let map = this.as_object(); let mut map = map .as_ref() .and_then(|obj| obj.downcast_mut::>()) .ok_or_else(|| JsNativeError::typ().with_message("`this` is not a Map"))?; - let key = match key.variant() { - JsVariant::Float64(r) => { - // 5. If key is -0š¯”½, set key to +0š¯”½. - if r.is_zero() { - JsValue::new(0) - } else { - key.clone() - } - } - _ => key.clone(), - }; + // 3. Set key to CanonicalizeKeyedCollectionKey(key). + let key = canonicalize_keyed_collection_value(args.get_or_undefined(0).clone()); // 4. For each Record { [[Key]], [[Value]] } p of entries, do // a. If p.[[Key]] is not empty and SameValueZero(p.[[Key]], key) is true, then @@ -549,6 +544,111 @@ impl Map { MapIterator::create_map_iterator(this, PropertyNameKind::Value, context) } + /// `Map.prototype.getOrInsert(key, value)` + /// + /// Given a key and a value, the getOrInsert method will return the existing value if it exists. + /// Otherwise insert the provided default value and return that value. + /// + /// More information: + /// - [Upsert proposal reference][spec] + /// - [MDN documentation][mdn] + /// + /// [spec]: https://tc39.es/proposal-upsert/#sec-map.prototype.getOrInsert + /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Map/getOrInsert + pub(crate) fn get_or_insert( + this: &JsValue, + args: &[JsValue], + _: &mut Context, + ) -> JsResult { + let value = args.get_or_undefined(1); + + // 1. Let M be the this value. + let map_obj = this.as_object(); + // 2. Perform ? RequireInternalSlot(M, [[MapData]]). + let mut map = map_obj + .as_ref() + .and_then(|obj| obj.downcast_mut::>()) + .ok_or_else(|| JsNativeError::typ().with_message("`this` is not a Map"))?; + + // 3. Set key to CanonicalizeKeyedCollectionKey(key). + let key = canonicalize_keyed_collection_value(args.get_or_undefined(0).clone()); + + // 4. For each Record { [[Key]], [[Value]] } p of M.[[MapData]], do + if let Some(existing) = map.get(&key) { + // a. If p.[[Key]] is not empty and SameValue(p.[[Key]], key) is true, return p.[[Value]]. + return Ok(existing.clone()); + } + + // 5. Let p be the Record { [[Key]]: key, [[Value]]: value }. + // 6. Append p to M.[[MapData]]. + map.insert(key, value.clone()); + // 7. Return value. + Ok(value.clone()) + } + + /// `Map.prototype.getOrInsertComputed(key, callback)` + /// + /// If the key exists in the Map, returns the existing value. + /// Otherwise computes a new value by calling `callback` with the key, + /// inserts it into the Map, and returns it. + /// + /// More information: + /// - [Upsert proposal reference][spec] + /// - [MDN documentation][mdn] + /// + /// [spec]: https://tc39.es/proposal-upsert/#sec-map.prototype.getOrInsertComputed + /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Map/getOrInsertComputed + pub(crate) fn get_or_insert_computed( + this: &JsValue, + args: &[JsValue], + context: &mut Context, + ) -> JsResult { + // 1. Let M be the this value. + let map_obj = this.as_object(); + // 2. Perform ? RequireInternalSlot(M, [[MapData]]). + let map = map_obj + .as_ref() + .and_then(|obj| obj.downcast_ref::>()) + .ok_or_else(|| JsNativeError::typ().with_message("`this` is not a Map"))?; + + // 3. If IsCallable(callbackfn) is false, throw a TypeError exception. + let Some(callback_fn) = args.get_or_undefined(1).as_callable() else { + return Err(JsNativeError::typ() + .with_message("Method Map.prototype.getOrInsertComputed called with non-callable callback function") + .into()); + }; + + // 4. Set key to CanonicalizeKeyedCollectionKey(key). + let key = canonicalize_keyed_collection_value(args.get_or_undefined(0).clone()); + + // 5. For each Record { [[Key]], [[Value]] } p of M.[[MapData]], do + if let Some(existing) = map.get(&key) { + // a. If p.[[Key]] is not empty and SameValue(p.[[Key]], key) is true, return p.[[Value]]. + return Ok(existing.clone()); + } + drop(map); + + // 6. Let value be ?Ā Call(callback, undefined, Ā« keyĀ Ā»). + // 7. NOTE: The Map may have been modified during execution of callback. + let value = callback_fn.call(&JsValue::undefined(), std::slice::from_ref(&key), context)?; + + let mut map = map_obj + .as_ref() + .and_then(|obj| obj.downcast_mut::>()) + .ok_or_else(|| JsNativeError::typ().with_message("`this` is not a Map"))?; + + // 8. For each Record { [[Key]], [[Value]] } p of M.[[MapData]], do + // a. If p.[[Key]] is not empty and SameValue(p.[[Key]], key) is true, then + // i. Set p.[[Value]] to value. + // ii. Return value. + // 9. Let p be the Record { [[Key]]: key, [[Value]]: valueĀ }. + // 10. Append p to M.[[MapData]]. + // [`OrderedMap::insert`] handles both cases + map.insert(key.clone(), value.clone()); + // 11. Return value. + Ok(value) + } + /// [`Map.groupBy ( items, callbackfn )`][spec] /// /// [spec]: https://tc39.es/ecma262/#sec-map.groupby @@ -617,14 +717,12 @@ impl Map { let key = callback.call(&JsValue::undefined(), &[value.clone(), k.into()], context); // f. IfAbruptCloseIterator(key, iteratorRecord). - let mut key = if_abrupt_close_iterator!(key, iterator, context); + let key = if_abrupt_close_iterator!(key, iterator, context); // h. Else, // i. Assert: keyCoercion is collection. // ii. Set key to CanonicalizeKeyedCollectionKey(key). - if key.as_number() == Some(-0.0) { - key = 0.into(); - } + let key = canonicalize_keyed_collection_value(key); // i. Perform AddValueToKeyedGroup(groups, key, value). groups.entry(key).or_default().push(value); diff --git a/core/engine/src/builtins/map/tests.rs b/core/engine/src/builtins/map/tests.rs index 25925943f3f..ee5c62361b4 100644 --- a/core/engine/src/builtins/map/tests.rs +++ b/core/engine/src/builtins/map/tests.rs @@ -305,3 +305,94 @@ fn for_of_delete() { "#}), ]); } + +#[test] +fn get_or_insert_inserts_on_miss() { + run_test_actions([ + TestAction::run("let map = new Map();"), + TestAction::assert_eq("map.getOrInsert('x', 42)", 42), + TestAction::assert("map.has('x')"), + TestAction::assert_eq("map.get('x')", 42), + ]); +} + +#[test] +fn get_or_insert_returns_existing_on_hit() { + run_test_actions([ + TestAction::run("let map = new Map([['y', 99]]);"), + TestAction::assert_eq("map.getOrInsert('y', 123)", 99), + TestAction::assert_eq("map.get('y')", 99), // unchanged + ]); +} + +#[test] +fn get_or_insert_canonicalizes_key() { + run_test_actions([ + TestAction::run("let map = new Map();"), + // -0 and +0 should canonicalize to +0 + TestAction::assert_eq("map.getOrInsert(-0, 'minus zero')", js_str!("minus zero")), + TestAction::assert("map.has(0)"), + TestAction::assert_eq("map.get(0)", js_str!("minus zero")), + ]); +} + +#[test] +fn get_or_insert_with_undefined_value() { + run_test_actions([ + TestAction::run("let map = new Map();"), + TestAction::assert_eq("map.getOrInsert('z', undefined)", JsValue::undefined()), + TestAction::assert("map.has('z')"), + TestAction::assert_eq("map.get('z')", JsValue::undefined()), + ]); +} + +#[test] +fn get_or_insert_computed_this_not_map() { + run_test_actions([TestAction::assert_native_error( + "Map.prototype.getOrInsertComputed.call({}, 'k', x => x)", + JsNativeErrorKind::Type, + "`this` is not a Map", + )]); +} + +#[test] +fn get_or_insert_computed_requires_callable() { + run_test_actions([TestAction::assert_native_error( + "new Map().getOrInsertComputed('k', undefined)", + JsNativeErrorKind::Type, + "Method Map.prototype.getOrInsertComputed called with non-callable callback function", + )]); +} + +#[test] +fn get_or_insert_computed_not_called_on_hit() { + run_test_actions([ + TestAction::run("const m = new Map([['k', 7]]); let calls = 0;"), + TestAction::assert_eq( + "m.getOrInsertComputed('k', (key) => { calls++; return 1; })", + 7, + ), + TestAction::assert_eq("calls", 0), + TestAction::assert_eq("m.get('k')", 7), + ]); +} + +#[test] +fn get_or_insert_computed_this_is_undefined_and_key_canonicalized() { + run_test_actions([ + TestAction::run( + r#" + const m = new Map(); + let seenThis, seenKey; + const v = m.getOrInsertComputed(-0, function(k) { 'use strict'; seenThis = this; seenKey = k; return 'ok'; }); + "#, + ), + // `this` inside callback is undefined + TestAction::assert("seenThis === undefined"), + // key argument is canonicalized (-0 ā†’ +0) + TestAction::assert("Object.is(seenKey, 0)"), + TestAction::assert_eq("v", js_str!("ok")), + TestAction::assert("m.has(0)"), + TestAction::assert_eq("m.get(0)", js_str!("ok")), + ]); +} diff --git a/core/engine/src/builtins/mod.rs b/core/engine/src/builtins/mod.rs index 2ad7f43740d..c7ea962c2c2 100644 --- a/core/engine/src/builtins/mod.rs +++ b/core/engine/src/builtins/mod.rs @@ -38,6 +38,7 @@ mod builder; use builder::BuiltInBuilder; use error::Error; +use num_traits::Zero; #[cfg(feature = "annex-b")] pub mod escape; @@ -194,6 +195,23 @@ fn global_binding(context: &mut Context) -> JsResult<()> { Ok(()) } +/// [`CanonicalizeKeyedCollectionKey ( key )`][spec] +/// +/// The abstract operation `CanonicalizeKeyedCollectionKey` takes argument key (an ECMAScript +/// language value) and returns an ECMAScript language value. It performs the following steps +/// when called: +/// +/// 1. If key is -0š¯”½, return +0š¯”½. +/// 2. Return key. +/// +/// [spec]: https://tc39.es/ecma262/multipage/keyed-collections.html#sec-canonicalizekeyedcollectionkey +fn canonicalize_keyed_collection_value(value: JsValue) -> JsValue { + match value.as_number() { + Some(n) if n.is_zero() => JsValue::new(0), + _ => value, + } +} + impl Realm { /// Abstract operation [`CreateIntrinsics ( realmRec )`][spec] /// diff --git a/core/engine/src/builtins/set/mod.rs b/core/engine/src/builtins/set/mod.rs index 0275539f394..b6e6e364cd7 100644 --- a/core/engine/src/builtins/set/mod.rs +++ b/core/engine/src/builtins/set/mod.rs @@ -21,7 +21,10 @@ use self::ordered_set::OrderedSet; use super::iterable::IteratorHint; use crate::{ Context, JsArgs, JsResult, JsString, JsValue, - builtins::{BuiltInBuilder, BuiltInConstructor, BuiltInObject, IntrinsicObject}, + builtins::{ + BuiltInBuilder, BuiltInConstructor, BuiltInObject, IntrinsicObject, + canonicalize_keyed_collection_value, + }, context::intrinsics::{Intrinsics, StandardConstructor, StandardConstructors}, error::JsNativeError, js_string, @@ -114,23 +117,6 @@ fn get_set_record(obj: &JsValue, context: &mut Context) -> JsResult { }) } -/// [`CanonicalizeKeyedCollectionKey ( key )`][spec] -/// -/// The abstract operation `CanonicalizeKeyedCollectionKey` takes argument key (an ECMAScript -/// language value) and returns an ECMAScript language value. It performs the following steps -/// when called: -/// -/// 1. If key is -0š¯”½, return +0š¯”½. -/// 2. Return key. -/// -/// [spec]: https://tc39.es/ecma262/#sec-set-iterable -fn canonicalize_keyed_collection_value(value: JsValue) -> JsValue { - match value.as_number() { - Some(n) if n.is_zero() => JsValue::new(0), - _ => value, - } -} - #[derive(Debug, Clone)] pub(crate) struct Set; diff --git a/test262_config.toml b/test262_config.toml index faf970ce297..1454637b873 100644 --- a/test262_config.toml +++ b/test262_config.toml @@ -48,10 +48,6 @@ features = [ # https://github.com/tc39/proposal-arraybuffer-base64 "uint8array-base64", - # Upsert - # https://github.com/tc39/proposal-upsert - "upsert", - # Source Phase Imports # https://github.com/tc39/proposal-source-phase-imports "source-phase-imports",