Fixed array creation bug in `Array.slice` implementation (#198)

* Change unnecessary mutable ref to immutable ref

* Add helper function to create new array

src/lib/builtins/array.rs

@@ -1,15 +1,38 @@
 use crate::{
     builtins::{
         function::NativeFunctionData,
-        object::{Object, ObjectKind, PROTOTYPE},
+        object::{Object, ObjectKind, INSTANCE_PROTOTYPE, PROTOTYPE},
         property::Property,
         value::{from_value, to_value, ResultValue, Value, ValueData},
     },
     exec::Interpreter,
 };
 use gc::Gc;
+use std::borrow::Borrow;
 use std::cmp::{max, min};
 
+pub(crate) fn new_array(interpreter: &Interpreter) -> ResultValue {
+    let array = ValueData::new_obj(Some(
+        &interpreter
+            .get_realm()
+            .environment
+            .get_global_object()
+            .expect("Could not get global object"),
+    ));
+    array.set_kind(ObjectKind::Array);
+    array.borrow().set_internal_slot(
+        INSTANCE_PROTOTYPE,
+        interpreter
+            .get_realm()
+            .environment
+            .get_binding_value("Array")
+            .borrow()
+            .get_field_slice(PROTOTYPE),
+    );
+    array.borrow().set_field_slice("length", to_value(0));
+    Ok(array)
+}
+
 /// Utility function for creating array objects: `array_obj` can be any array with
 /// prototype already set (it will be wiped and recreated from `array_contents`)
 fn construct_array(array_obj: &Value, array_contents: &[Value]) -> ResultValue {
@@ -31,7 +54,7 @@ fn construct_array(array_obj: &Value, array_contents: &[Value]) -> ResultValue {
 
 /// Utility function which takes an existing array object and puts additional
 /// values on the end, correctly rewriting the length
-fn add_to_array_object(array_ptr: &Value, add_values: &[Value]) -> ResultValue {
+pub(crate) fn add_to_array_object(array_ptr: &Value, add_values: &[Value]) -> ResultValue {
     let orig_length: i32 =
         from_value(array_ptr.get_field_slice("length")).expect("failed to conveert lenth to i32");
 
@@ -499,8 +522,7 @@ pub fn includes_value(this: &Value, args: &[Value], _: &mut Interpreter) -> Resu
 /// length is the length of the array.
 /// <https://tc39.es/ecma262/#sec-array.prototype.slice>
 pub fn slice(this: &Value, args: &[Value], interpreter: &mut Interpreter) -> ResultValue {
-    let new_array = make_array(&to_value(Object::default()), &[], interpreter)?;
-    new_array.set_kind(ObjectKind::Array);
+    let new_array = new_array(interpreter)?;
     let len: i32 =
         from_value(this.get_field_slice("length")).expect("Could not convert argument to i32");
 

src/lib/environment/lexical_environment.rs

@@ -207,7 +207,7 @@ impl LexicalEnvironment {
             .any(|env| env.borrow().has_binding(name))
     }
 
-    pub fn get_binding_value(&mut self, name: &str) -> Value {
+    pub fn get_binding_value(&self, name: &str) -> Value {
         self.environments()
             .find(|env| env.borrow().has_binding(name))
             .map(|env| env.borrow().get_binding_value(name, false))

src/lib/exec.rs

@@ -1,5 +1,6 @@
 use crate::{
     builtins::{
+        array,
         function::{create_unmapped_arguments_object, Function, RegularFunction},
         object::{ObjectKind, INSTANCE_PROTOTYPE, PROTOTYPE},
         value::{from_value, to_value, ResultValue, Value, ValueData},
@@ -203,30 +204,10 @@ impl Executor for Interpreter {
                 Ok(obj)
             }
             ExprDef::ArrayDecl(ref arr) => {
-                let global_val = &self
-                    .realm
-                    .environment
-                    .get_global_object()
-                    .expect("Could not get the global object");
-                let arr_map = ValueData::new_obj(Some(global_val));
-                // Note that this object is an Array
-                arr_map.set_kind(ObjectKind::Array);
-                let mut index: i32 = 0;
-                for val in arr.iter() {
-                    let val = self.run(val)?;
-                    arr_map.borrow().set_field(index.to_string(), val);
-                    index += 1;
-                }
-                arr_map.borrow().set_internal_slot(
-                    INSTANCE_PROTOTYPE,
-                    self.realm
-                        .environment
-                        .get_binding_value("Array")
-                        .borrow()
-                        .get_field_slice(PROTOTYPE),
-                );
-                arr_map.borrow().set_field_slice("length", to_value(index));
-                Ok(arr_map)
+                let array = array::new_array(self)?;
+                let elements: Result<Vec<_>, _> = arr.iter().map(|val| self.run(val)).collect();
+                array::add_to_array_object(&array, &elements?)?;
+                Ok(array)
             }
             ExprDef::FunctionDecl(ref name, ref args, ref expr) => {
                 let function =
@@ -502,6 +483,11 @@ impl Executor for Interpreter {
 }
 
 impl Interpreter {
+    /// Get the Interpreter's realm
+    pub(crate) fn get_realm(&self) -> &Realm {
+        &self.realm
+    }
+
     /// https://tc39.es/ecma262/#sec-call
     pub(crate) fn call(&mut self, f: &Value, v: &Value, arguments_list: Vec<Value>) -> ResultValue {
         // All functions should be objects, and eventually will be.