From a6aacaa58116f9fe7396630a3829ab8ab7b0ebba Mon Sep 17 00:00:00 2001 From: croraf Date: Wed, 7 Oct 2020 14:48:49 +0200 Subject: [PATCH 1/4] [exec Array] implement spread operator using iterator --- boa/src/builtins/array/tests.rs | 24 ++++++++++++++++++++++++ boa/src/builtins/map/tests.rs | 3 +++ boa/src/context.rs | 1 + boa/src/syntax/ast/node/array/mod.rs | 23 ++++++++++++++++++----- 4 files changed, 46 insertions(+), 5 deletions(-) diff --git a/boa/src/builtins/array/tests.rs b/boa/src/builtins/array/tests.rs index 876de0e8537..464383d75bc 100644 --- a/boa/src/builtins/array/tests.rs +++ b/boa/src/builtins/array/tests.rs @@ -1210,3 +1210,27 @@ fn array_values_symbol_iterator() { "#; assert_eq!(forward(&mut engine, init), "true"); } + +#[test] +fn array_spread_arrays() { + let mut engine = Context::new(); + let init = r#" + const array1 = [2, 3]; + const array2 = [1, ...array1]; + array2[0] === 1 && array2[1] === 2 && array2[2] === 3; + "#; + assert_eq!(forward(&mut engine, init), "true"); +} + +#[test] +fn array_spread_non_iterable() { + let mut engine = Context::new(); + let init = r#" + try { + const array2 = [...5]; + } catch (err) { + err.name === "TypeError" && err.message === "Not an iterable" + } + "#; + assert_eq!(forward(&mut engine, init), "true"); +} diff --git a/boa/src/builtins/map/tests.rs b/boa/src/builtins/map/tests.rs index 761cbdf025f..b7ee1b96676 100644 --- a/boa/src/builtins/map/tests.rs +++ b/boa/src/builtins/map/tests.rs @@ -43,7 +43,10 @@ fn clone() { assert_eq!(result, "2"); } +// depends on the https://github.com/boa-dev/boa/issues/810 #[test] +#[ignore] + fn merge() { let mut engine = Context::new(); let init = r#" diff --git a/boa/src/context.rs b/boa/src/context.rs index d68faacc1ec..fe87c6cb8ef 100644 --- a/boa/src/context.rs +++ b/boa/src/context.rs @@ -449,6 +449,7 @@ impl Context { /// Converts an array object into a rust vector of values. /// /// This is useful for the spread operator, for any other object an `Err` is returned + /// TODO: Not needed for spread of arrays. Check in the future for Map and remove if necessary pub(crate) fn extract_array_properties(&mut self, value: &Value) -> StdResult, ()> { if let Value::Object(ref x) = value { // Check if object is array diff --git a/boa/src/syntax/ast/node/array/mod.rs b/boa/src/syntax/ast/node/array/mod.rs index b98f9514942..fd71bd79090 100644 --- a/boa/src/syntax/ast/node/array/mod.rs +++ b/boa/src/syntax/ast/node/array/mod.rs @@ -39,14 +39,27 @@ impl Executable for ArrayDecl { for elem in self.as_ref() { if let Node::Spread(ref x) = elem { let val = x.run(interpreter)?; - let mut vals = interpreter.extract_array_properties(&val).unwrap(); - elements.append(&mut vals); - continue; // Don't push array after spread + let iterator_record = + super::super::super::super::builtins::iterable::get_iterator(interpreter, val)?; + //not sure what to do with this next_index mentioned in the spec + //it is mentioned that it has to be returned from somewhere + //https://tc39.es/ecma262/#sec-runtime-semantics-arrayaccumulation + //let mut next_index = 0; + loop { + let next = iterator_record.next(interpreter)?; + if next.is_done() { + break; + } + let next_value = next.value(); + //next_index += 1; + elements.push(next_value.clone()); + } + } else { + elements.push(elem.run(interpreter)?); } - elements.push(elem.run(interpreter)?); } - Array::add_to_array_object(&array, &elements)?; + Array::add_to_array_object(&array, &elements)?; Ok(array) } } From c4992cd38a462b04c39d4ef808ee50e2af003aa9 Mon Sep 17 00:00:00 2001 From: croraf Date: Fri, 9 Oct 2020 16:33:08 +0200 Subject: [PATCH 2/4] Minor --- boa/src/builtins/map/tests.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/boa/src/builtins/map/tests.rs b/boa/src/builtins/map/tests.rs index b7ee1b96676..fa714cc896d 100644 --- a/boa/src/builtins/map/tests.rs +++ b/boa/src/builtins/map/tests.rs @@ -43,10 +43,9 @@ fn clone() { assert_eq!(result, "2"); } -// depends on the https://github.com/boa-dev/boa/issues/810 +// TODO depends on the https://github.com/boa-dev/boa/issues/810 #[test] #[ignore] - fn merge() { let mut engine = Context::new(); let init = r#" From 8496a9a89ffd5c4a7bcb273fdd404f42b9ef0390 Mon Sep 17 00:00:00 2001 From: croraf Date: Sat, 10 Oct 2020 13:40:33 +0200 Subject: [PATCH 3/4] Fix the next_index comment --- boa/src/syntax/ast/node/array/mod.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/boa/src/syntax/ast/node/array/mod.rs b/boa/src/syntax/ast/node/array/mod.rs index fd71bd79090..80003514126 100644 --- a/boa/src/syntax/ast/node/array/mod.rs +++ b/boa/src/syntax/ast/node/array/mod.rs @@ -41,10 +41,9 @@ impl Executable for ArrayDecl { let val = x.run(interpreter)?; let iterator_record = super::super::super::super::builtins::iterable::get_iterator(interpreter, val)?; - //not sure what to do with this next_index mentioned in the spec - //it is mentioned that it has to be returned from somewhere - //https://tc39.es/ecma262/#sec-runtime-semantics-arrayaccumulation - //let mut next_index = 0; + // TODO after proper internal Array representation as per https://github.com/boa-dev/boa/pull/811#discussion_r502460858 + // next_index variable should be utilized here as per https://tc39.es/ecma262/#sec-runtime-semantics-arrayaccumulation + // let mut next_index = 0; loop { let next = iterator_record.next(interpreter)?; if next.is_done() { From 84116d2f466ee7b744ad37500fcb373a5ab3c344 Mon Sep 17 00:00:00 2001 From: croraf Date: Sat, 10 Oct 2020 22:40:52 +0200 Subject: [PATCH 4/4] Minor --- boa/src/syntax/ast/node/array/mod.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/boa/src/syntax/ast/node/array/mod.rs b/boa/src/syntax/ast/node/array/mod.rs index 80003514126..02dda7badd0 100644 --- a/boa/src/syntax/ast/node/array/mod.rs +++ b/boa/src/syntax/ast/node/array/mod.rs @@ -1,7 +1,11 @@ //! Array declaration node. use super::{join_nodes, Node}; -use crate::{builtins::Array, exec::Executable, BoaProfiler, Context, Result, Value}; +use crate::{ + builtins::{iterable, Array}, + exec::Executable, + BoaProfiler, Context, Result, Value, +}; use gc::{Finalize, Trace}; use std::fmt; @@ -39,8 +43,7 @@ impl Executable for ArrayDecl { for elem in self.as_ref() { if let Node::Spread(ref x) = elem { let val = x.run(interpreter)?; - let iterator_record = - super::super::super::super::builtins::iterable::get_iterator(interpreter, val)?; + let iterator_record = iterable::get_iterator(interpreter, val)?; // TODO after proper internal Array representation as per https://github.com/boa-dev/boa/pull/811#discussion_r502460858 // next_index variable should be utilized here as per https://tc39.es/ecma262/#sec-runtime-semantics-arrayaccumulation // let mut next_index = 0;