From e1f915904256075572d66a966e1a48c7de86b8ec Mon Sep 17 00:00:00 2001 From: RBRi Date: Mon, 15 Jan 2024 02:10:08 +0100 Subject: [PATCH] Fixes for Symbol.iterator handling in NativeArray (#1435) * Array.prototype[Symbol.iterator] now correctly points to the 'values' functions * arguments[Symbol.iterator] points to Array Array.prototype[Symbol.iterator] see https://github.com/HtmlUnit/htmlunit-rhino-fork/pull/14 --- src/org/mozilla/javascript/Arguments.java | 25 ++----- src/org/mozilla/javascript/NativeArray.java | 50 ++++++++++--- testsrc/jstests/harmony/for-of.js | 2 +- .../javascript/tests/es6/ArgumentsTest.java | 66 +++++++++++++++++ .../tests/es6/NativeArray3Test.java | 74 +++++++++++++++++++ testsrc/test262.properties | 7 +- 6 files changed, 190 insertions(+), 34 deletions(-) create mode 100644 testsrc/org/mozilla/javascript/tests/es6/ArgumentsTest.java create mode 100644 testsrc/org/mozilla/javascript/tests/es6/NativeArray3Test.java diff --git a/src/org/mozilla/javascript/Arguments.java b/src/org/mozilla/javascript/Arguments.java index b0300b22e6..7674b6f187 100644 --- a/src/org/mozilla/javascript/Arguments.java +++ b/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; diff --git a/src/org/mozilla/javascript/NativeArray.java b/src/org/mozilla/javascript/NativeArray.java index e861e809e2..6fb2b25f9b 100644 --- a/src/org/mozilla/javascript/NativeArray.java +++ b/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); @@ -503,6 +497,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); @@ -2527,7 +2557,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; } @@ -2737,8 +2770,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, diff --git a/testsrc/jstests/harmony/for-of.js b/testsrc/jstests/harmony/for-of.js index af51d6f502..1a22eb7b18 100644 --- a/testsrc/jstests/harmony/for-of.js +++ b/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. diff --git a/testsrc/org/mozilla/javascript/tests/es6/ArgumentsTest.java b/testsrc/org/mozilla/javascript/tests/es6/ArgumentsTest.java new file mode 100644 index 0000000000..d961ea53e5 --- /dev/null +++ b/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; + }); + } +} diff --git a/testsrc/org/mozilla/javascript/tests/es6/NativeArray3Test.java b/testsrc/org/mozilla/javascript/tests/es6/NativeArray3Test.java new file mode 100644 index 0000000000..059996c638 --- /dev/null +++ b/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; + }); + } +} diff --git a/testsrc/test262.properties b/testsrc/test262.properties index b4869c376d..626371927d 100644 --- a/testsrc/test262.properties +++ b/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