Make `Array.prototype.concat` spec compliant (#1353)

boa/src/builtins/array/mod.rs

@@ -284,6 +284,30 @@ impl Array {
         Ok(array_obj_ptr)
     }
 
+    /// Utility function for concatenating array objects.
+    ///
+    /// Returns a Boolean valued property that if `true` indicates that
+    /// an object should be flattened to its array elements
+    /// by `Array.prototype.concat`.
+    fn is_concat_spreadable(this: &Value, context: &mut Context) -> Result<bool> {
+        // 1. If Type(O) is not Object, return false.
+        if !this.is_object() {
+            return Ok(false);
+        }
+        // 2. Let spreadable be ? Get(O, @@isConcatSpreadable).
+        let spreadable = this.get_field(WellKnownSymbols::is_concat_spreadable(), context)?;
+
+        // 3. If spreadable is not undefined, return ! ToBoolean(spreadable).
+        if !spreadable.is_undefined() {
+            return Ok(spreadable.to_boolean());
+        }
+        // 4. Return ? IsArray(O).
+        match this.as_object() {
+            Some(obj) => Ok(obj.is_array()),
+            _ => Ok(false),
+        }
+    }
+
     /// `get Array [ @@species ]`
     ///
     /// The `Array [ @@species ]` accessor property returns the Array constructor.
@@ -459,30 +483,66 @@ impl Array {
     /// [spec]: https://tc39.es/ecma262/#sec-array.prototype.concat
     /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/concat
     pub(crate) fn concat(this: &Value, args: &[Value], context: &mut Context) -> Result<Value> {
-        if args.is_empty() {
-            // If concat is called with no arguments, it returns the original array
-            return Ok(this.clone());
-        }
-
-        // Make a new array (using this object as the prototype basis for the new
-        // one)
-        let mut new_values: Vec<Value> = Vec::new();
-
-        let this_length = this.get_field("length", context)?.to_length(context)?;
-        for n in 0..this_length {
-            new_values.push(this.get_field(n, context)?);
-        }
-
-        for concat_array in args {
-            let concat_length = concat_array
-                .get_field("length", context)?
-                .to_length(context)?;
-            for n in 0..concat_length {
-                new_values.push(concat_array.get_field(n, context)?);
+        // 1. Let O be ? ToObject(this value).
+        let obj = this.to_object(context)?;
+        // 2. Let A be ? ArraySpeciesCreate(O, 0).
+        let arr = Self::array_species_create(&obj, 0, context)?;
+        // 3. Let n be 0.
+        let mut n = 0;
+        // 4. Prepend O to items.
+        // 5. For each element E of items, do
+        for item in [Value::from(obj)].iter().chain(args.iter()) {
+            // a. Let spreadable be ? IsConcatSpreadable(E).
+            let spreadable = Self::is_concat_spreadable(&item, context)?;
+            // b. If spreadable is true, then
+            if spreadable {
+                // item is guaranteed to be an object since is_concat_spreadable checks it,
+                // so we can call `.unwrap()`
+                let item = item.as_object().unwrap();
+                // i. Let k be 0.
+                // ii. Let len be ? LengthOfArrayLike(E).
+                let len = item.length_of_array_like(context)?;
+                // iii. If n + len > 2^53 - 1, throw a TypeError exception.
+                if n + len > Number::MAX_SAFE_INTEGER as usize {
+                    return context.throw_type_error(
+                        "length + number of arguments exceeds the max safe integer limit",
+                    );
+                }
+                // iv. Repeat, while k < len,
+                for k in 0..len {
+                    // 1. Let P be ! ToString(𝔽(k)).
+                    // 2. Let exists be ? HasProperty(E, P).
+                    let exists = item.has_property(k, context)?;
+                    // 3. If exists is true, then
+                    if exists {
+                        // a. Let subElement be ? Get(E, P).
+                        let sub_element = item.get(k, context)?;
+                        // b. Perform ? CreateDataPropertyOrThrow(A, ! ToString(𝔽(n)), subElement).
+                        arr.create_data_property_or_throw(n, sub_element, context)?;
+                    }
+                    // 4. Set n to n + 1.
+                    n += 1;
+                    // 5. Set k to k + 1.
+                }
+            }
+            // c. Else,
+            else {
+                // i. NOTE: E is added as a single item rather than spread.
+                // ii. If n ≥ 253 - 1, throw a TypeError exception.
+                if n >= Number::MAX_SAFE_INTEGER as usize {
+                    return context.throw_type_error("length exceeds the max safe integer limit");
+                }
+                // iii. Perform ? CreateDataPropertyOrThrow(A, ! ToString(𝔽(n)), E).
+                arr.create_data_property_or_throw(n, item, context)?;
+                // iv. Set n to n + 1.
+                n += 1
             }
         }
+        // 6. Perform ? Set(A, "length", 𝔽(n), true).
+        arr.set("length", n, true, context)?;
 
-        Self::construct_array(this, &new_values, context)
+        // 7. Return A.
+        Ok(Value::from(arr))
     }
 
     /// `Array.prototype.push( ...items )`

test_ignore.txt

@@ -10,7 +10,6 @@ feature:json-modules
 //feature:class
 
 // These seem to run forever:
-arg-length-exceeding-integer-limit
 
 // These generate a stack overflow
 tco-call