LibJS: Array.from mapFn fixes + thisArg support

* Callback mapFn now properly supports second argument (index)
* Support of thisArg to be passed as "this" in vm.call
* Tests for all cases

Userland/Libraries/LibJS/Runtime/ArrayConstructor.cpp

@@ -105,6 +105,8 @@ JS_DEFINE_NATIVE_FUNCTION(ArrayConstructor::from)
         map_fn = &callback.as_function();
     }
 
+    auto this_arg = vm.argument(2);
+
     // Array.from() lets you create Arrays from:
     if (auto size = object->indexed_properties().array_like_size()) {
         // * array-like objects (objects with a length property and indexed elements)
@@ -116,7 +118,7 @@ JS_DEFINE_NATIVE_FUNCTION(ArrayConstructor::from)
                 if (vm.exception())
                     return {};
 
-                auto map_fn_result = vm.call(*map_fn, value, element);
+                auto map_fn_result = vm.call(*map_fn, this_arg, element, Value((i32)i));
                 if (vm.exception())
                     return {};
 
@@ -130,12 +132,14 @@ JS_DEFINE_NATIVE_FUNCTION(ArrayConstructor::from)
         array->set_indexed_property_elements(move(elements));
     } else {
         // * iterable objects
+        i32 i = 0;
         get_iterator_values(global_object, value, [&](Value element) {
             if (vm.exception())
                 return IterationDecision::Break;
 
             if (map_fn) {
-                auto map_fn_result = vm.call(*map_fn, value, element);
+                auto map_fn_result = vm.call(*map_fn, this_arg, element, Value(i));
+                i++;
                 if (vm.exception())
                     return IterationDecision::Break;
 
@@ -150,8 +154,6 @@ JS_DEFINE_NATIVE_FUNCTION(ArrayConstructor::from)
             return {};
     }
 
-    // FIXME: if interpreter.argument_count() >= 3: thisArg
-
     return array;
 }
 

Userland/Libraries/LibJS/Tests/builtins/Array/Array.from.js

@@ -9,24 +9,48 @@ describe("normal behavior", () => {
         expect(a).toHaveLength(0);
     });
 
-    test("empty array, with mapFn", () => {
+    test("empty array, with mapFn, no thisArg", () => {
         const a = Array.from([], n => n);
         expect(a instanceof Array).toBeTrue();
         expect(a).toHaveLength(0);
     });
 
+    test("empty array, with mapFn, with thisArg", () => {
+        const a = Array.from(
+            [],
+            function (n) {
+                return n + this.value;
+            },
+            { value: 100 }
+        );
+        expect(a instanceof Array).toBeTrue();
+        expect(a).toHaveLength(0);
+    });
+
     test("empty string, no mapFn", () => {
         const a = Array.from("");
         expect(a instanceof Array).toBeTrue();
         expect(a).toHaveLength(0);
     });
 
-    test("empty string, with mapFn", () => {
+    test("empty string, with mapFn, no thisArg", () => {
         const a = Array.from("", n => n);
         expect(a instanceof Array).toBeTrue();
         expect(a).toHaveLength(0);
     });
 
+    test("empty string, with mapFn, with thisArg", () => {
+        const a = Array.from(
+            "",
+            function (n) {
+                return n + this.value;
+            },
+            { value: 100 }
+        );
+        expect(a instanceof Array).toBeTrue();
+        expect(a).toHaveLength(0);
+    });
+
     test("non-empty array, no mapFn", () => {
         const a = Array.from([5, 8, 1]);
         expect(a instanceof Array).toBeTrue();
@@ -36,13 +60,28 @@ describe("normal behavior", () => {
         expect(a[2]).toBe(1);
     });
 
-    test("non-empty array, with mapFn", () => {
+    test("non-empty array, with mapFn, no thisArg", () => {
-        const a = Array.from([5, 8, 1], n => ++n);
+        const a = Array.from([5, 8, 1], (n, i) => n - i);
         expect(a instanceof Array).toBeTrue();
         expect(a).toHaveLength(3);
-        expect(a[0]).toBe(6);
+        expect(a[0]).toBe(5);
-        expect(a[1]).toBe(9);
+        expect(a[1]).toBe(7);
-        expect(a[2]).toBe(2);
+        expect(a[2]).toBe(-1);
+    });
+
+    test("non-empty array, with mapFn, with thisArg", () => {
+        const a = Array.from(
+            [5, 8, 1],
+            function (n, i) {
+                return n - i + this.value;
+            },
+            { value: 100 }
+        );
+        expect(a instanceof Array).toBeTrue();
+        expect(a).toHaveLength(3);
+        expect(a[0]).toBe(105);
+        expect(a[1]).toBe(107);
+        expect(a[2]).toBe(99);
     });
 
     test("non-empty string, no mapFn", () => {
@@ -55,14 +94,30 @@ describe("normal behavior", () => {
         expect(a[3]).toBe("t");
     });
 
-    test("non-empty string, with mapFn", () => {
+    test("non-empty string, with mapFn, no thisArg", () => {
-        const a = Array.from("what", n => n + n);
+        const a = Array.from("what", (n, i) => n + n + i);
         expect(a instanceof Array).toBeTrue();
         expect(a).toHaveLength(4);
-        expect(a[0]).toBe("ww");
+        expect(a[0]).toBe("ww0");
-        expect(a[1]).toBe("hh");
+        expect(a[1]).toBe("hh1");
-        expect(a[2]).toBe("aa");
+        expect(a[2]).toBe("aa2");
-        expect(a[3]).toBe("tt");
+        expect(a[3]).toBe("tt3");
+    });
+
+    test("non-empty string, with mapFn, with thisArg", () => {
+        const a = Array.from(
+            "what",
+            function (n, i) {
+                return n + i + this.value;
+            },
+            { value: "a" }
+        );
+        expect(a instanceof Array).toBeTrue();
+        expect(a).toHaveLength(4);
+        expect(a[0]).toBe("w0a");
+        expect(a[1]).toBe("h1a");
+        expect(a[2]).toBe("a2a");
+        expect(a[3]).toBe("t3a");
     });
 
     test("shallow array copy, no mapFn", () => {
@@ -74,7 +129,7 @@ describe("normal behavior", () => {
         expect(a[0]).toBe(4);
     });
 
-    test("shallow array copy, with mapFn", () => {
+    test("shallow array copy, with mapFn, no thisArg", () => {
         const a = [1, 2, 3];
         const b = Array.from([a], n => n.map(n => n + 2));
         expect(b instanceof Array).toBeTrue();
@@ -86,6 +141,24 @@ describe("normal behavior", () => {
         expect(b[0][2]).toBe(5);
     });
 
+    test("shallow array copy, with mapFn, with thisArg", () => {
+        const a = [1, 2, 3];
+        const b = Array.from(
+            [a],
+            function (n, i) {
+                return n.map(n => n + 2 + i + this.value);
+            },
+            { value: 100 }
+        );
+        expect(b instanceof Array).toBeTrue();
+        expect(b).toHaveLength(1);
+        b[0][0] = 10;
+        expect(a[0]).toBe(1);
+        expect(b[0][0]).toBe(10);
+        expect(b[0][1]).toBe(104);
+        expect(b[0][2]).toBe(105);
+    });
+
     const rangeIterator = function (begin, end) {
         return {
             [Symbol.iterator]() {
@@ -110,11 +183,25 @@ describe("normal behavior", () => {
         expect(a[1]).toBe(9);
     });
 
-    test("from iterator, with mapFn", () => {
+    test("from iterator, with mapFn, no thisArg", () => {
         const a = Array.from(rangeIterator(8, 10), n => --n);
         expect(a instanceof Array).toBeTrue();
         expect(a).toHaveLength(2);
         expect(a[0]).toBe(7);
         expect(a[1]).toBe(8);
     });
+
+    test("from iterator, with mapFn, with thisArg", () => {
+        const a = Array.from(
+            rangeIterator(8, 10),
+            function (n, i) {
+                return n + i + this.value;
+            },
+            { value: 100 }
+        );
+        expect(a instanceof Array).toBeTrue();
+        expect(a).toHaveLength(2);
+        expect(a[0]).toBe(108);
+        expect(a[1]).toBe(110);
+    });
 });