Fixes for Symbol.iterator handling in NativeArray

src/org/mozilla/javascript/Arguments.java

@@ -6,8 +6,6 @@
 
 package org.mozilla.javascript;
 
-import org.mozilla.javascript.NativeArrayIterator.ARRAY_ITERATOR_TYPE;
-
 /**
  * This class implements the "arguments" object.
  *
@@ -41,7 +39,12 @@ public Arguments(NativeCall activation) {
             callerObj = NOT_FOUND;
         }
 
-        defineProperty(SymbolKey.ITERATOR, iteratorMethod, ScriptableObject.DONTENUM);
+        defineProperty(
+                SymbolKey.ITERATOR,
+                TopLevel.getBuiltinPrototype(
+                                ScriptableObject.getTopLevelScope(parent), TopLevel.Builtins.Array)
+                        .get("values", parent),
+                ScriptableObject.DONTENUM);
     }
 
     @Override
@@ -402,22 +405,6 @@ void defineAttributesForStrictMode() {
         calleeObj = null;
     }
 
-    private static BaseFunction iteratorMethod =
-            new BaseFunction() {
-                private static final long serialVersionUID = 4239122318596177391L;
-
-                @Override
-                public Object call(
-                        Context cx, Scriptable scope, Scriptable thisObj, Object[] args) {
-                    // TODO : call %ArrayProto_values%
-                    // 9.4.4.6 CreateUnmappedArgumentsObject(argumentsList)
-                    //  1. Perform DefinePropertyOrThrow(obj, @@iterator, PropertyDescriptor
-                    // {[[Value]]:%ArrayProto_values%,
-                    //     [[Writable]]: true, [[Enumerable]]: false, [[Configurable]]: true}).
-                    return new NativeArrayIterator(scope, thisObj, ARRAY_ITERATOR_TYPE.VALUES);
-                }
-            };
-
     private static class ThrowTypeError extends BaseFunction {
         private static final long serialVersionUID = -744615873947395749L;
         private String propertyName;

src/org/mozilla/javascript/NativeArray.java

@@ -159,11 +159,6 @@ protected void fillConstructorProperties(IdFunctionObject ctor) {
 
     @Override
     protected void initPrototypeId(int id) {
-        if (id == SymbolId_iterator) {
-            initPrototypeMethod(ARRAY_TAG, id, SymbolKey.ITERATOR, "[Symbol.iterator]", 0);
-            return;
-        }
-
         String s, fnName = null;
         int arity;
         switch (id) {
@@ -471,7 +466,6 @@ public Object execIdCall(
                             scope, thisObj, NativeArrayIterator.ARRAY_ITERATOR_TYPE.ENTRIES);
 
                 case Id_values:
-                case SymbolId_iterator:
                     thisObj = ScriptRuntime.toObject(cx, scope, thisObj);
                     return new NativeArrayIterator(
                             scope, thisObj, NativeArrayIterator.ARRAY_ITERATOR_TYPE.VALUES);
@@ -495,6 +489,42 @@ public boolean has(int index, Scriptable start) {
         return super.has(index, start);
     }
 
+    @Override
+    public boolean has(Symbol key, Scriptable start) {
+        if (SymbolKey.ITERATOR.equals(key)) {
+            return super.has("values", start);
+        }
+
+        return super.has(key, start);
+    }
+
+    @Override
+    public Object get(Symbol key, Scriptable start) {
+        if (SymbolKey.ITERATOR.equals(key)) {
+            return super.get("values", start);
+        }
+
+        return super.get(key, start);
+    }
+
+    @Override
+    public void put(Symbol key, Scriptable start, Object value) {
+        if (SymbolKey.ITERATOR.equals(key)) {
+            super.put("values", start, value);
+        }
+
+        super.put(key, start, value);
+    }
+
+    @Override
+    public void delete(Symbol key) {
+        if (SymbolKey.ITERATOR.equals(key)) {
+            super.delete("values");
+        }
+
+        super.delete(key);
+    }
+
     private static long toArrayIndex(Object id) {
         if (id instanceof String) {
             return toArrayIndex((String) id);
@@ -2519,7 +2549,10 @@ private void checkModCount(int modCount) {
     @Override
     protected int findPrototypeId(Symbol k) {
         if (SymbolKey.ITERATOR.equals(k)) {
-            return SymbolId_iterator;
+            // "Symbol.iterator" property of the prototype has the "same value"
+            // as the "values" property. We implement this by returning the
+            // ID of "values" when the iterator symbol is accessed.
+            return Id_values;
         }
         return 0;
     }
@@ -2729,8 +2762,7 @@ protected int findPrototypeId(String s) {
             Id_at = 32,
             Id_flat = 33,
             Id_flatMap = 34,
-            SymbolId_iterator = 35,
-            MAX_PROTOTYPE_ID = SymbolId_iterator;
+            MAX_PROTOTYPE_ID = Id_flatMap;
     private static final int ConstructorId_join = -Id_join,
             ConstructorId_reverse = -Id_reverse,
             ConstructorId_sort = -Id_sort,

testsrc/jstests/harmony/for-of.js

@@ -132,7 +132,7 @@ assertThrows('for each (n of [1,2]) {}', SyntaxError);
 
 assertThrows('[n*n for each (n of [1,2])]', SyntaxError);
 
-assertEquals('[Symbol.iterator]', Array.prototype[Symbol.iterator].name);
+assertEquals('values', Array.prototype[Symbol.iterator].name);
 assertEquals('[Symbol.iterator]', String.prototype[Symbol.iterator].name);
 
 // should have `value` and `done` property.

testsrc/org/mozilla/javascript/tests/es6/ArgumentsTest.java

@@ -0,0 +1,66 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+package org.mozilla.javascript.tests.es6;
+
+import static org.junit.Assert.assertEquals;
+
+import org.junit.Test;
+import org.mozilla.javascript.Context;
+import org.mozilla.javascript.ScriptableObject;
+import org.mozilla.javascript.tests.Utils;
+
+/** Tests for Arguments support. */
+public class ArgumentsTest {
+
+    @Test
+    public void argumentsSymbolIterator() {
+        String code =
+                "function foo() {"
+                        + "  return arguments[Symbol.iterator] === Array.prototype.values;"
+                        + "}"
+                        + "foo()";
+
+        test(true, code);
+    }
+
+    @Test
+    public void argumentsSymbolIterator2() {
+        String code =
+                "function foo() {"
+                        + "  return arguments[Symbol.iterator] === [][Symbol.iterator];"
+                        + "}"
+                        + "foo()";
+
+        test(true, code);
+    }
+
+    @Test
+    public void argumentsForOf() {
+        String code =
+                "function foo() {"
+                        + "  var res = '';"
+                        + "  for (arg of arguments) {"
+                        + "    res += arg;"
+                        + "  }"
+                        + "  return res;"
+                        + "}"
+                        + "foo(1, 2, 3, 5)";
+
+        test("1235", code);
+    }
+
+    private static void test(Object expected, String js) {
+        Utils.runWithAllOptimizationLevels(
+                cx -> {
+                    cx.setLanguageVersion(Context.VERSION_ES6);
+                    ScriptableObject scope = cx.initStandardObjects();
+
+                    Object result = cx.evaluateString(scope, js, "test", 1, null);
+                    assertEquals(expected, result);
+
+                    return null;
+                });
+    }
+}

testsrc/org/mozilla/javascript/tests/es6/NativeArray3Test.java

@@ -0,0 +1,74 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+package org.mozilla.javascript.tests.es6;
+
+import static org.junit.Assert.assertEquals;
+
+import org.junit.Test;
+import org.mozilla.javascript.Context;
+import org.mozilla.javascript.ScriptableObject;
+import org.mozilla.javascript.tests.Utils;
+
+/** Tests for NativeArray support. */
+public class NativeArray3Test {
+
+    @Test
+    public void iteratorPrototype() {
+        String code = "Array.prototype.values === [][Symbol.iterator]";
+
+        test(true, code);
+    }
+
+    @Test
+    public void iteratorInstances() {
+        String code = "[1, 2][Symbol.iterator] === [][Symbol.iterator]";
+
+        test(true, code);
+    }
+
+    @Test
+    public void iteratorPrototypeName() {
+        String code = "Array.prototype.values.name;";
+
+        test("values", code);
+    }
+
+    @Test
+    public void iteratorInstanceName() {
+        String code = "[][Symbol.iterator].name;";
+
+        test("values", code);
+    }
+
+    @Test
+    public void redefineIterator() {
+        String code =
+                "var res = '';\n"
+                        + "var arr = ['hello', 'world'];\n"
+                        + "res += arr[Symbol.iterator].toString().includes('return i;');\n"
+                        + "res += ' - ';\n"
+                        + "arr[Symbol.iterator] = function () { return i; };\n"
+                        + "res += arr[Symbol.iterator].toString().includes('return i;');\n"
+                        + "res += ' - ';\n"
+                        + "delete arr[Symbol.iterator];\n"
+                        + "res += arr[Symbol.iterator].toString().includes('return i;');\n"
+                        + "res;";
+
+        test("false - true - false", code);
+    }
+
+    private static void test(Object expected, String js) {
+        Utils.runWithAllOptimizationLevels(
+                cx -> {
+                    cx.setLanguageVersion(Context.VERSION_ES6);
+                    ScriptableObject scope = cx.initStandardObjects();
+
+                    Object result = cx.evaluateString(scope, js, "test", 1, null);
+                    assertEquals(expected, result);
+
+                    return null;
+                });
+    }
+}

testsrc/test262.properties

@@ -1,6 +1,6 @@
 # This is a configuration file for Test262SuiteTest.java. See ./README.md for more info about this file
 
-built-ins/Array 177/2670 (6.63%)
+built-ins/Array 176/2670 (6.59%)
     from/calling-from-valid-1-noStrict.js non-strict Spec pretty clearly says this should be undefined
     from/elements-deleted-after.js Checking to see if length changed, but spec says it should not
     from/iter-map-fn-this-non-strict.js non-strict Error propagation needs work in general
@@ -169,7 +169,6 @@ built-ins/Array 177/2670 (6.63%)
     prototype/toLocaleString/primitive_this_value_getter.js strict
     prototype/unshift/throws-with-string-receiver.js
     prototype/methods-called-as-functions.js {unsupported: [Symbol.species]}
-    prototype/Symbol.iterator.js Expects a particular string value
     Symbol.species 4/4 (100.0%)
     proto-from-ctor-realm-one.js {unsupported: [Reflect]}
     proto-from-ctor-realm-two.js {unsupported: [Reflect]}
@@ -2447,7 +2446,7 @@ built-ins/WeakMap 1/88 (1.14%)
 built-ins/WeakSet 1/75 (1.33%)
     proto-from-ctor-realm.js {unsupported: [Reflect]}
 
-language/arguments-object 191/260 (73.46%)
+language/arguments-object 189/260 (72.69%)
     mapped/mapped-arguments-nonconfigurable-3.js non-strict
     mapped/mapped-arguments-nonconfigurable-delete-1.js non-strict
     mapped/mapped-arguments-nonconfigurable-delete-2.js non-strict
@@ -2483,8 +2482,6 @@ language/arguments-object 191/260 (73.46%)
     mapped/nonwritable-nonenumerable-nonconfigurable-descriptors-basic.js non-strict
     mapped/nonwritable-nonenumerable-nonconfigurable-descriptors-set-by-arguments.js non-strict
     mapped/nonwritable-nonenumerable-nonconfigurable-descriptors-set-by-param.js non-strict
-    mapped/Symbol.iterator.js non-strict
-    unmapped/Symbol.iterator.js non-strict
     unmapped/via-params-dflt.js
     unmapped/via-params-dstr.js non-strict
     unmapped/via-params-rest.js