From e1a2bb2055601fcbf981b5df0d585e8f04c7e94d Mon Sep 17 00:00:00 2001 From: Dirk de Visser Date: Thu, 13 Jul 2023 13:27:21 +0200 Subject: [PATCH] Refactor `Array.prototype.find*` and TypedArray variants to use `FindViaPredicate` (#3134) * Refactor `Array.prototype.{find,findIndex}` and TypedArray variants to use `FindViaPredicate` * Fix cargo fmt * Remove duplicate callable check, simplify returns --- boa_engine/src/builtins/array/mod.rs | 228 +++++++++++---------- boa_engine/src/builtins/typed_array/mod.rs | 98 +++------ 2 files changed, 151 insertions(+), 175 deletions(-) diff --git a/boa_engine/src/builtins/array/mod.rs b/boa_engine/src/builtins/array/mod.rs index 1d7f32e3d7..a9976ab8e8 100644 --- a/boa_engine/src/builtins/array/mod.rs +++ b/boa_engine/src/builtins/array/mod.rs @@ -36,6 +36,13 @@ pub(crate) use array_iterator::ArrayIterator; #[cfg(test)] mod tests; +/// Direction for `find_via_predicate` +#[derive(Clone, Copy, Eq, PartialEq)] +pub(crate) enum Direction { + Ascending, + Descending, +} + /// JavaScript `Array` built-in implementation. #[derive(Debug, Clone, Copy)] pub(crate) struct Array; @@ -1519,38 +1526,22 @@ impl Array { // 2. Let len be ? LengthOfArrayLike(O). let len = o.length_of_array_like(context)?; - // 3. If IsCallable(predicate) is false, throw a TypeError exception. - let predicate = args.get_or_undefined(0).as_callable().ok_or_else(|| { - JsNativeError::typ().with_message("Array.prototype.find: predicate is not callable") - })?; - + let predicate = args.get_or_undefined(0); let this_arg = args.get_or_undefined(1); - // 4. Let k be 0. - let mut k = 0; - // 5. Repeat, while k < len, - while k < len { - // a. Let Pk be ! ToString(𝔽(k)). - let pk = k; - // b. Let kValue be ? Get(O, Pk). - let k_value = o.get(pk, context)?; - // c. Let testResult be ! ToBoolean(? Call(predicate, thisArg, « kValue, 𝔽(k), O »)). - let test_result = predicate - .call( - this_arg, - &[k_value.clone(), k.into(), o.clone().into()], - context, - )? - .to_boolean(); - // d. If testResult is true, return kValue. - if test_result { - return Ok(k_value); - } - // e. Set k to k + 1. - k += 1; - } - // 6. Return undefined. - Ok(JsValue::undefined()) + // 3. Let findRec be ? FindViaPredicate(O, len, ascending, predicate, thisArg). + let (_, value) = find_via_predicate( + &o, + len, + Direction::Ascending, + predicate, + this_arg, + context, + "Array.prototype.find", + )?; + + // 4. Return findRec.[[Value]]. + Ok(value) } /// `Array.prototype.findIndex( predicate [ , thisArg ] )` @@ -1576,35 +1567,22 @@ impl Array { // 2. Let len be ? LengthOfArrayLike(O). let len = o.length_of_array_like(context)?; - // 3. If IsCallable(predicate) is false, throw a TypeError exception. - let predicate = args.get_or_undefined(0).as_callable().ok_or_else(|| { - JsNativeError::typ() - .with_message("Array.prototype.findIndex: predicate is not callable") - })?; - + let predicate = args.get_or_undefined(0); let this_arg = args.get_or_undefined(1); - // 4. Let k be 0. - let mut k = 0; - // 5. Repeat, while k < len, - while k < len { - // a. Let Pk be ! ToString(𝔽(k)). - let pk = k; - // b. Let kValue be ? Get(O, Pk). - let k_value = o.get(pk, context)?; - // c. Let testResult be ! ToBoolean(? Call(predicate, thisArg, « kValue, 𝔽(k), O »)). - let test_result = predicate - .call(this_arg, &[k_value, k.into(), o.clone().into()], context)? - .to_boolean(); - // d. If testResult is true, return 𝔽(k). - if test_result { - return Ok(JsValue::new(k)); - } - // e. Set k to k + 1. - k += 1; - } - // 6. Return -1𝔽. - Ok(JsValue::new(-1)) + // 3. Let findRec be ? FindViaPredicate(O, len, ascending, predicate, thisArg). + let (index, _) = find_via_predicate( + &o, + len, + Direction::Ascending, + predicate, + this_arg, + context, + "Array.prototype.findIndex", + )?; + + // 4. Return findRec.[[Index]]. + Ok(index) } /// `Array.prototype.findLast( predicate, [thisArg] )` @@ -1628,35 +1606,22 @@ impl Array { // 2. Let len be ? LengthOfArrayLike(O). let len = o.length_of_array_like(context)?; - // 3. If IsCallable(predicate) is false, throw a TypeError exception. - let predicate = args.get_or_undefined(0).as_callable().ok_or_else(|| { - JsNativeError::typ().with_message("Array.prototype.findLast: predicate is not callable") - })?; - + let predicate = args.get_or_undefined(0); let this_arg = args.get_or_undefined(1); - // 4. Let k be len - 1. (implementation differs slightly from spec because k is unsigned) - // 5. Repeat, while k >= 0, (implementation differs slightly from spec because k is unsigned) - for k in (0..len).rev() { - // a. Let Pk be ! ToString(𝔽(k)). - // b. Let kValue be ? Get(O, Pk). - let k_value = o.get(k, context)?; - // c. Let testResult be ! ToBoolean(? Call(predicate, thisArg, « kValue, 𝔽(k), O »)). - let test_result = predicate - .call( - this_arg, - &[k_value.clone(), k.into(), this.clone()], - context, - )? - .to_boolean(); - // d. If testResult is true, return kValue. - if test_result { - return Ok(k_value); - } - // e. Set k to k - 1. - } - // 6. Return undefined. - Ok(JsValue::undefined()) + // 3. Let findRec be ? FindViaPredicate(O, len, descending, predicate, thisArg). + let (_, value) = find_via_predicate( + &o, + len, + Direction::Descending, + predicate, + this_arg, + context, + "Array.prototype.findLast", + )?; + + // 4. Return findRec.[[Value]]. + Ok(value) } /// `Array.prototype.findLastIndex( predicate [ , thisArg ] )` @@ -1680,32 +1645,22 @@ impl Array { // 2. Let len be ? LengthOfArrayLike(O). let len = o.length_of_array_like(context)?; - // 3. If IsCallable(predicate) is false, throw a TypeError exception. - let predicate = args.get_or_undefined(0).as_callable().ok_or_else(|| { - JsNativeError::typ() - .with_message("Array.prototype.findLastIndex: predicate is not callable") - })?; - + let predicate = args.get_or_undefined(0); let this_arg = args.get_or_undefined(1); - // 4. Let k be len - 1. (implementation differs slightly from spec because k is unsigned) - // 5. Repeat, while k >= 0, (implementation differs slightly from spec because k is unsigned) - for k in (0..len).rev() { - // a. Let Pk be ! ToString(𝔽(k)). - // b. Let kValue be ? Get(O, Pk). - let k_value = o.get(k, context)?; - // c. Let testResult be ! ToBoolean(? Call(predicate, thisArg, « kValue, 𝔽(k), O »)). - let test_result = predicate - .call(this_arg, &[k_value, k.into(), this.clone()], context)? - .to_boolean(); - // d. If testResult is true, return 𝔽(k). - if test_result { - return Ok(JsValue::new(k)); - } - // e. Set k to k - 1. - } - // 6. Return -1𝔽. - Ok(JsValue::new(-1)) + // 3. Let findRec be ? FindViaPredicate(O, len, descending, predicate, thisArg). + let (index, _) = find_via_predicate( + &o, + len, + Direction::Descending, + predicate, + this_arg, + context, + "Array.prototype.findLastIndex", + )?; + + // 4. Return findRec.[[Index]]. + Ok(index) } /// `Array.prototype.flat( [depth] )` @@ -3076,3 +3031,60 @@ impl Array { unscopable_list } } + +/// `FindViaPredicate ( O, len, direction, predicate, thisArg )` +/// +/// More information: +/// - [ECMAScript reference][spec] +/// +/// [spec]: https://tc39.es/ecma262/#sec-findviapredicate +pub(crate) fn find_via_predicate( + o: &JsObject, + len: u64, + direction: Direction, + predicate: &JsValue, + this_arg: &JsValue, + context: &mut Context<'_>, + caller_name: &str, +) -> JsResult<(JsValue, JsValue)> { + // 1. If IsCallable(predicate) is false, throw a TypeError exception. + let predicate = predicate.as_callable().ok_or_else(|| { + JsNativeError::typ().with_message(format!("{caller_name}: predicate is not callable")) + })?; + + let indices = match direction { + // 2. If direction is ascending, then + // a. Let indices be a List of the integers in the interval from 0 (inclusive) to len (exclusive), in ascending order. + Direction::Ascending => itertools::Either::Left(0..len), + // 3. Else, + // a. Let indices be a List of the integers in the interval from 0 (inclusive) to len (exclusive), in descending order. + Direction::Descending => itertools::Either::Right((0..len).rev()), + }; + + // 4. For each integer k of indices, do + for k in indices { + // a. Let Pk be ! ToString(𝔽(k)). + let pk = k; + + // b. NOTE: If O is a TypedArray, the following invocation of Get will return a normal completion. + // c. Let kValue be ? Get(O, Pk). + let k_value = o.get(pk, context)?; + + // d. Let testResult be ? Call(predicate, thisArg, « kValue, 𝔽(k), O »). + let test_result = predicate + .call( + this_arg, + &[k_value.clone(), k.into(), o.clone().into()], + context, + )? + .to_boolean(); + + if test_result { + // e. If ToBoolean(testResult) is true, return the Record { [[Index]]: 𝔽(k), [[Value]]: kValue }. + return Ok((JsValue::new(k), k_value)); + } + } + + // 5. Return the Record { [[Index]]: -1𝔽, [[Value]]: undefined } + Ok((JsValue::new(-1), JsValue::undefined())) +} diff --git a/boa_engine/src/builtins/typed_array/mod.rs b/boa_engine/src/builtins/typed_array/mod.rs index 85543ff4c7..544b537d49 100644 --- a/boa_engine/src/builtins/typed_array/mod.rs +++ b/boa_engine/src/builtins/typed_array/mod.rs @@ -14,7 +14,7 @@ use crate::{ builtins::{ - array::ArrayIterator, + array::{find_via_predicate, ArrayIterator, Direction}, array_buffer::{ArrayBuffer, SharedMemoryOrder}, iterable::iterable_to_list, typed_array::integer_indexed_object::{ContentType, IntegerIndexed}, @@ -320,7 +320,7 @@ impl IntrinsicObject for TypedArray { .method(Self::fill, "fill", 1) .method(Self::filter, "filter", 1) .method(Self::find, "find", 1) - .method(Self::findindex, "findIndex", 1) + .method(Self::find_index, "findIndex", 1) .method(Self::foreach, "forEach", 1) .method(Self::includes, "includes", 1) .method(Self::index_of, "indexOf", 1) @@ -1160,41 +1160,22 @@ impl TypedArray { // 3. Let len be O.[[ArrayLength]]. let len = o.array_length(); - // 4. If IsCallable(predicate) is false, throw a TypeError exception. - let predicate = match args.get_or_undefined(0).as_object() { - Some(obj) if obj.is_callable() => obj, - _ => { - return Err(JsNativeError::typ() - .with_message( - "TypedArray.prototype.find called with non-callable predicate function", - ) - .into()) - } - }; - - // 5. Let k be 0. - // 6. Repeat, while k < len, - for k in 0..len { - // a. Let Pk be ! ToString(𝔽(k)). - // b. Let kValue be ! Get(O, Pk). - let k_value = obj.get(k, context).expect("Get cannot fail here"); + let predicate = args.get_or_undefined(0); + let this_arg = args.get_or_undefined(1); - // c. Let testResult be ! ToBoolean(? Call(predicate, thisArg, « kValue, 𝔽(k), O »)). - // d. If testResult is true, return kValue. - if predicate - .call( - args.get_or_undefined(1), - &[k_value.clone(), k.into(), this.clone()], - context, - )? - .to_boolean() - { - return Ok(k_value); - } - } + // 4. Let findRec be ? FindViaPredicate(O, len, ascending, predicate, thisArg). + let (_, value) = find_via_predicate( + obj, + len, + Direction::Ascending, + predicate, + this_arg, + context, + "TypedArray.prototype.find", + )?; - // 7. Return undefined. - Ok(JsValue::undefined()) + // 5. Return findRec.[[Value]]. + Ok(value) } /// `23.2.3.12 %TypedArray%.prototype.findIndex ( predicate [ , thisArg ] )` @@ -1203,7 +1184,7 @@ impl TypedArray { /// - [ECMAScript reference][spec] /// /// [spec]: https://tc39.es/ecma262/#sec-%typedarray%.prototype.findindex - pub(crate) fn findindex( + pub(crate) fn find_index( this: &JsValue, args: &[JsValue], context: &mut Context<'_>, @@ -1226,39 +1207,22 @@ impl TypedArray { // 3. Let len be O.[[ArrayLength]]. let len = o.array_length(); - // 4. If IsCallable(predicate) is false, throw a TypeError exception. - let predicate = match args.get_or_undefined(0).as_object() { - Some(obj) if obj.is_callable() => obj, - _ => return Err(JsNativeError::typ() - .with_message( - "TypedArray.prototype.findindex called with non-callable predicate function", - ) - .into()), - }; - - // 5. Let k be 0. - // 6. Repeat, while k < len, - for k in 0..len { - // a. Let Pk be ! ToString(𝔽(k)). - // b. Let kValue be ! Get(O, Pk). - let k_value = obj.get(k, context).expect("Get cannot fail here"); + let predicate = args.get_or_undefined(0); + let this_arg = args.get_or_undefined(1); - // c. Let testResult be ! ToBoolean(? Call(predicate, thisArg, « kValue, 𝔽(k), O »)). - // d. If testResult is true, return 𝔽(k). - if predicate - .call( - args.get_or_undefined(1), - &[k_value.clone(), k.into(), this.clone()], - context, - )? - .to_boolean() - { - return Ok(k.into()); - } - } + // 4. Let findRec be ? FindViaPredicate(O, len, ascending, predicate, thisArg). + let (index, _) = find_via_predicate( + obj, + len, + Direction::Ascending, + predicate, + this_arg, + context, + "TypedArray.prototype.findIndex", + )?; - // 7. Return -1𝔽. - Ok((-1).into()) + // 5. Return findRec.[[Index]]. + Ok(index) } /// `23.2.3.13 %TypedArray%.prototype.forEach ( callbackfn [ , thisArg ] )`