From 264878e122d6d8e84c6bac5a4dc67b9a086f4c1e Mon Sep 17 00:00:00 2001 From: Benjamin Flin Date: Wed, 8 Jul 2020 12:36:16 -0700 Subject: [PATCH 1/4] Implement Array.prototype.reduce --- boa/src/builtins/array/mod.rs | 72 +++++++++++++++++++++++++++++++++ boa/src/builtins/array/tests.rs | 61 ++++++++++++++++++++++++++++ 2 files changed, 133 insertions(+) diff --git a/boa/src/builtins/array/mod.rs b/boa/src/builtins/array/mod.rs index c6650a7c875..04c344a7842 100644 --- a/boa/src/builtins/array/mod.rs +++ b/boa/src/builtins/array/mod.rs @@ -957,6 +957,77 @@ impl Array { Ok(Value::from(false)) } + /// `Array.prototype.reduce( callbackFn [ , initialValue ] )` + /// + /// The reduce method traverses left to right starting from the first defined value in the array, + /// accumulating a value using a given callback function. It returns the accumulated value. + /// + /// More information: + /// - [ECMAScript reference][spec] + /// - [MDN documentation][mdn] + /// + /// [spec]: https://tc39.es/ecma262/#sec-array.prototype.reduce + /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/reduce + pub(crate) fn reduce( + this: &Value, + args: &[Value], + interpreter: &mut Interpreter, + ) -> ResultValue { + if args.is_empty() { + return Err(Value::from( + "missing callback when calling function Array.prototype.reduce", + )); + } + let callback = args.get(0).cloned().unwrap_or_else(Value::undefined); + let initial_value = args.get(1).cloned().unwrap_or_else(Value::undefined); + let mut length = i32::from(&this.get_field("length")); + if length == 0 && initial_value.is_undefined() { + return Err(Value::from( + "Array contains no elements and initial value is not provided", + )); + } + + let mut k = 0; + let mut accumulator = if initial_value.is_undefined() { + let mut k_present = false; + while k < length { + if this.has_field(&k.to_string()) { + k_present = true; + break; + } + k += 1; + } + if !k_present { + return Err(Value::from( + "Array contains no elements and initial value is not provided", + )); + } + let result = this.get_field(k.to_string()); + k += 1; + result + } else { + initial_value + }; + while k < length { + if this.has_field(&k.to_string()) { + let arguments = [ + accumulator, + this.get_field(k.to_string()), + Value::integer(k), + this.clone(), + ]; + match interpreter.call(&callback, &Value::undefined(), &arguments) { + Ok(value) => accumulator = value, + Err(e) => return Err(e), + } + // reduce may change the length of the array + length = min(length, i32::from(&this.get_field("length"))); + } + k += 1; + } + Ok(accumulator) + } + /// Initialise the `Array` object on the global object. #[inline] pub(crate) fn init(global: &Value) -> (&str, Value) { @@ -988,6 +1059,7 @@ impl Array { make_builtin_fn(Self::find_index, "findIndex", &prototype, 1); make_builtin_fn(Self::slice, "slice", &prototype, 2); make_builtin_fn(Self::some, "some", &prototype, 2); + make_builtin_fn(Self::reduce, "reduce", &prototype, 2); let array = make_constructor_fn( Self::NAME, diff --git a/boa/src/builtins/array/tests.rs b/boa/src/builtins/array/tests.rs index c828940f03b..07b91e5dbc7 100644 --- a/boa/src/builtins/array/tests.rs +++ b/boa/src/builtins/array/tests.rs @@ -789,6 +789,67 @@ fn some() { assert_eq!(result, "true"); } +#[test] +fn reduce() { + let realm = Realm::create(); + let mut engine = Interpreter::new(realm); + + let init = r#" + var arr = [1, 2, 3, 4]; + function add(acc, x) { + return acc + x; + } + + function addIdx(acc, _, idx) { + return acc + idx; + } + + function addLen(acc, _x, _idx, arr) { + return acc + arr.length; + } + + function addResize(acc, x, idx, arr) { + if(idx == 0) { + arr.length = 3; + } + return acc + x; + } + var delArray = [1, 2, 3, 4, 5]; + delete delArray[0]; + delete delArray[1]; + delete delArray[3]; + "#; + forward(&mut engine, init); + + // empty array + let result = forward(&mut engine, "[].reduce(add, 0)"); + assert_eq!(result, "0"); + + // simple with initial value + let result = forward(&mut engine, "arr.reduce(add, 0)"); + assert_eq!(result, "10"); + + // without initial value + let result = forward(&mut engine, "arr.reduce(add)"); + assert_eq!(result, "10"); + + // with some items missing + let result = forward(&mut engine, "delArray.reduce(add, 0)"); + assert_eq!(result, "8"); + + // with index + let result = forward(&mut engine, "arr.reduce(addIdx, 0)"); + assert_eq!(result, "6"); + + // with array + let result = forward(&mut engine, "arr.reduce(addLen, 0)"); + assert_eq!(result, "16"); + + // resizing the array as reduce progresses + let result = forward(&mut engine, "arr.reduce(addResize, 0)"); + assert_eq!(result, "6"); +} + #[test] fn call_array_constructor_with_one_argument() { let realm = Realm::create(); From b7655a227a8d32850a24877b345d34b4301ae661 Mon Sep 17 00:00:00 2001 From: benjaminflin Date: Wed, 8 Jul 2020 22:06:05 -0700 Subject: [PATCH 2/4] Apply suggestions from code review Co-authored-by: HalidOdat --- boa/src/builtins/array/mod.rs | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/boa/src/builtins/array/mod.rs b/boa/src/builtins/array/mod.rs index 04c344a7842..bd7c04cb25d 100644 --- a/boa/src/builtins/array/mod.rs +++ b/boa/src/builtins/array/mod.rs @@ -973,18 +973,20 @@ impl Array { args: &[Value], interpreter: &mut Interpreter, ) -> ResultValue { - if args.is_empty() { - return Err(Value::from( - "missing callback when calling function Array.prototype.reduce", - )); - } - let callback = args.get(0).cloned().unwrap_or_else(Value::undefined); + let this = interpreter.to_object(this)?; + let callback = match args.get(0) { + Some(value) if value.is_function() => value, + _ => { + return interpreter.throw_type_error( + "missing callback when calling function Array.prototype.reduce", + ) + } + }; let initial_value = args.get(1).cloned().unwrap_or_else(Value::undefined); let mut length = i32::from(&this.get_field("length")); if length == 0 && initial_value.is_undefined() { - return Err(Value::from( - "Array contains no elements and initial value is not provided", - )); + return interpreter + .throw_type_error("Array contains no elements and initial value is not provided"); } let mut k = 0; @@ -998,9 +1000,9 @@ impl Array { k += 1; } if !k_present { - return Err(Value::from( + return interpreter.throw_type_error( "Array contains no elements and initial value is not provided", - )); + ); } let result = this.get_field(k.to_string()); k += 1; @@ -1016,10 +1018,7 @@ impl Array { Value::integer(k), this.clone(), ]; - match interpreter.call(&callback, &Value::undefined(), &arguments) { - Ok(value) => accumulator = value, - Err(e) => return Err(e), - } + accumulator = interpreter.call(&callback, &Value::undefined(), &arguments)?; // reduce may change the length of the array length = min(length, i32::from(&this.get_field("length"))); } From ed5f70ee5fe560784651b7dd6ac583973b342df7 Mon Sep 17 00:00:00 2001 From: Benjamin Flin Date: Thu, 9 Jul 2020 10:07:16 -0700 Subject: [PATCH 3/4] Changed i32::from to interpreter.to_length and added comment about length modification --- boa/src/builtins/array/mod.rs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/boa/src/builtins/array/mod.rs b/boa/src/builtins/array/mod.rs index bd7c04cb25d..2f330d6ca82 100644 --- a/boa/src/builtins/array/mod.rs +++ b/boa/src/builtins/array/mod.rs @@ -983,12 +983,11 @@ impl Array { } }; let initial_value = args.get(1).cloned().unwrap_or_else(Value::undefined); - let mut length = i32::from(&this.get_field("length")); + let mut length = interpreter.to_length(&this.get_field("length"))?; if length == 0 && initial_value.is_undefined() { return interpreter .throw_type_error("Array contains no elements and initial value is not provided"); } - let mut k = 0; let mut accumulator = if initial_value.is_undefined() { let mut k_present = false; @@ -1015,12 +1014,14 @@ impl Array { let arguments = [ accumulator, this.get_field(k.to_string()), - Value::integer(k), + Value::from(k), this.clone(), ]; accumulator = interpreter.call(&callback, &Value::undefined(), &arguments)?; - // reduce may change the length of the array - length = min(length, i32::from(&this.get_field("length"))); + /* We keep track of possibly shortened length in order to prevent unnecessary iteration. + It may also be necessary to do this since shortening the array length does not + delete array elements. See: https://github.com/boa-dev/boa/issues/557 */ + length = min(length, interpreter.to_length(&this.get_field("length"))?); } k += 1; } From e1cd08e184699f5ac2a2faca830bd92091a495fc Mon Sep 17 00:00:00 2001 From: Benjamin Flin Date: Tue, 14 Jul 2020 20:24:51 -0700 Subject: [PATCH 4/4] rewrote type error messages and added tests for error cases --- boa/src/builtins/array/mod.rs | 16 ++++------- boa/src/builtins/array/tests.rs | 49 +++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 10 deletions(-) diff --git a/boa/src/builtins/array/mod.rs b/boa/src/builtins/array/mod.rs index 2f330d6ca82..bfe76015d3d 100644 --- a/boa/src/builtins/array/mod.rs +++ b/boa/src/builtins/array/mod.rs @@ -127,9 +127,9 @@ impl Array { 1 if args[0].is_integer() => { length = i32::from(&args[0]); // TODO: It should not create an array of undefineds, but an empty array ("holy" array in V8) with length `n`. - for n in 0..length { - this.set_field(n.to_string(), Value::undefined()); - } + // for n in 0..length { + // this.set_field(n.to_string(), Value::undefined()); + // } } 1 if args[0].is_double() => { return ctx.throw_range_error("invalid array length"); @@ -976,17 +976,13 @@ impl Array { let this = interpreter.to_object(this)?; let callback = match args.get(0) { Some(value) if value.is_function() => value, - _ => { - return interpreter.throw_type_error( - "missing callback when calling function Array.prototype.reduce", - ) - } + _ => return interpreter.throw_type_error("Reduce was called without a callback"), }; let initial_value = args.get(1).cloned().unwrap_or_else(Value::undefined); let mut length = interpreter.to_length(&this.get_field("length"))?; if length == 0 && initial_value.is_undefined() { return interpreter - .throw_type_error("Array contains no elements and initial value is not provided"); + .throw_type_error("Reduce was called on an empty array and with no initial value"); } let mut k = 0; let mut accumulator = if initial_value.is_undefined() { @@ -1000,7 +996,7 @@ impl Array { } if !k_present { return interpreter.throw_type_error( - "Array contains no elements and initial value is not provided", + "Reduce was called on an empty array and with no initial value", ); } let result = this.get_field(k.to_string()); diff --git a/boa/src/builtins/array/tests.rs b/boa/src/builtins/array/tests.rs index 07b91e5dbc7..26dc512c1c8 100644 --- a/boa/src/builtins/array/tests.rs +++ b/boa/src/builtins/array/tests.rs @@ -818,6 +818,7 @@ fn reduce() { delete delArray[0]; delete delArray[1]; delete delArray[3]; + "#; forward(&mut engine, init); @@ -848,6 +849,54 @@ fn reduce() { // resizing the array as reduce progresses let result = forward(&mut engine, "arr.reduce(addResize, 0)"); assert_eq!(result, "6"); + + // Empty array + let result = forward( + &mut engine, + r#" + try { + [].reduce((acc, x) => acc + x); + } catch(e) { + e.message + } + "#, + ); + assert_eq!( + result, + "Reduce was called on an empty array and with no initial value" + ); + + // Array with no defined elements + let result = forward( + &mut engine, + r#" + try { + var arr = [0, 1]; + delete arr[0]; + delete arr[1]; + arr.reduce((acc, x) => acc + x); + } catch(e) { + e.message + } + "#, + ); + assert_eq!( + result, + "Reduce was called on an empty array and with no initial value" + ); + + // No callback + let result = forward( + &mut engine, + r#" + try { + arr.reduce(""); + } catch(e) { + e.message + } + "#, + ); + assert_eq!(result, "Reduce was called without a callback"); } #[test]