From d8394835588f775fb0f60dfa3852bcdccde810e2 Mon Sep 17 00:00:00 2001 From: Andrea Bergia Date: Wed, 7 Aug 2024 11:17:54 +0200 Subject: [PATCH] Implement `Symbol.prototype.description` Refactored `NativeSymbol` to use `LambdaConstructor` instead of `IdScriptableObject`. --- .../mozilla/javascript/LambdaConstructor.java | 19 +- .../org/mozilla/javascript/NativeSymbol.java | 209 +++++++----------- .../org/mozilla/javascript/SymbolKey.java | 17 +- tests/testsrc/jstests/es6/symbols.js | 15 ++ tests/testsrc/test262.properties | 12 +- 5 files changed, 129 insertions(+), 143 deletions(-) diff --git a/rhino/src/main/java/org/mozilla/javascript/LambdaConstructor.java b/rhino/src/main/java/org/mozilla/javascript/LambdaConstructor.java index d0a8c0e86e..8ce1555caa 100644 --- a/rhino/src/main/java/org/mozilla/javascript/LambdaConstructor.java +++ b/rhino/src/main/java/org/mozilla/javascript/LambdaConstructor.java @@ -35,7 +35,7 @@ public class LambdaConstructor extends LambdaFunction { public static final int CONSTRUCTOR_DEFAULT = CONSTRUCTOR_FUNCTION | CONSTRUCTOR_NEW; // Lambdas should not be serialized. - private final transient Constructable targetConstructor; + protected final transient Constructable targetConstructor; private final int flags; /** @@ -112,6 +112,23 @@ public void definePrototypeMethod( proto.defineProperty(name, f, attributes); } + /** + * Define a function property on the prototype of the constructor using a LambdaFunction under + * the covers. + */ + public void definePrototypeMethod( + Scriptable scope, + SymbolKey name, + int length, + Callable target, + int attributes, + int propertyAttributes) { + LambdaFunction f = new LambdaFunction(scope, "[" + name.getName() + "]", length, target); + f.setStandardPropertyAttributes(propertyAttributes); + ScriptableObject proto = getPrototypeScriptable(); + proto.defineProperty(name, f, attributes); + } + /** Define a property that may be of any type on the prototype of this constructor. */ public void definePrototypeProperty(String name, Object value, int attributes) { ScriptableObject proto = getPrototypeScriptable(); diff --git a/rhino/src/main/java/org/mozilla/javascript/NativeSymbol.java b/rhino/src/main/java/org/mozilla/javascript/NativeSymbol.java index 8add15dae2..60da104e3e 100644 --- a/rhino/src/main/java/org/mozilla/javascript/NativeSymbol.java +++ b/rhino/src/main/java/org/mozilla/javascript/NativeSymbol.java @@ -14,7 +14,7 @@ * properties. One of them is that some objects can have an "internal data slot" that makes them a * Symbol and others cannot. */ -public class NativeSymbol extends IdScriptableObject implements Symbol { +public class NativeSymbol extends ScriptableObject implements Symbol { private static final long serialVersionUID = -589539749749830003L; public static final String CLASS_NAME = "Symbol"; @@ -27,8 +27,59 @@ public class NativeSymbol extends IdScriptableObject implements Symbol { private final NativeSymbol symbolData; public static void init(Context cx, Scriptable scope, boolean sealed) { - NativeSymbol obj = new NativeSymbol(""); - ScriptableObject ctor = obj.exportAsJSClass(MAX_PROTOTYPE_ID, scope, false); + LambdaConstructor ctor = + new LambdaConstructor( + scope, + CLASS_NAME, + 0, + 0, // Unused + NativeSymbol::js_constructor) { + // Calling new Symbol('foo') is not allowed. However, we need to be able to + // create + // a new symbol instance to register built-in ones and for the implementation of + // "for", + // so we have this trick of a thread-local variable to allow it. + @Override + public Scriptable construct(Context cx, Scriptable scope, Object[] args) { + if (cx.getThreadLocal(CONSTRUCTOR_SLOT) == null) { + throw ScriptRuntime.typeErrorById("msg.no.new", getFunctionName()); + } + return (Scriptable) call(cx, scope, null, args); + } + + // Calling Symbol('foo') should act like a constructor call + @Override + public Object call( + Context cx, Scriptable scope, Scriptable thisObj, Object[] args) { + Scriptable obj = targetConstructor.construct(cx, scope, args); + obj.setPrototype(getClassPrototype()); + obj.setParentScope(scope); + return obj; + } + }; + + ctor.setPrototypePropertyAttributes(DONTENUM | READONLY | PERMANENT); + + ctor.defineConstructorMethod( + scope, "for", 1, NativeSymbol::js_for, DONTENUM, DONTENUM | READONLY); + ctor.defineConstructorMethod( + scope, "keyFor", 1, NativeSymbol::js_keyFor, DONTENUM, DONTENUM | READONLY); + + ctor.definePrototypeMethod( + scope, "toString", 0, NativeSymbol::js_toString, DONTENUM, DONTENUM | READONLY); + ctor.definePrototypeMethod( + scope, "valueOf", 0, NativeSymbol::js_valueOf, DONTENUM, DONTENUM | READONLY); + ctor.definePrototypeMethod( + scope, + SymbolKey.TO_PRIMITIVE, + 1, + NativeSymbol::js_valueOf, + DONTENUM | READONLY, + DONTENUM | READONLY); + ctor.definePrototypeProperty(SymbolKey.TO_STRING_TAG, CLASS_NAME, DONTENUM | READONLY); + ctor.definePrototypeProperty(cx, "description", NativeSymbol::js_description, DONTENUM | READONLY); + + ScriptableObject.defineProperty(scope, CLASS_NAME, ctor, DONTENUM); cx.putThreadLocal(CONSTRUCTOR_SLOT, Boolean.TRUE); try { @@ -45,7 +96,6 @@ public static void init(Context cx, Scriptable scope, boolean sealed) { createStandardSymbol(cx, scope, ctor, "search", SymbolKey.SEARCH); createStandardSymbol(cx, scope, ctor, "split", SymbolKey.SPLIT); createStandardSymbol(cx, scope, ctor, "unscopables", SymbolKey.UNSCOPABLES); - } finally { cx.removeThreadLocal(CONSTRUCTOR_SLOT); } @@ -95,117 +145,12 @@ public String getClassName() { return CLASS_NAME; } - @Override - protected void fillConstructorProperties(IdFunctionObject ctor) { - super.fillConstructorProperties(ctor); - addIdFunctionProperty(ctor, CLASS_NAME, ConstructorId_for, "for", 1); - addIdFunctionProperty(ctor, CLASS_NAME, ConstructorId_keyFor, "keyFor", 1); - } - private static void createStandardSymbol( - Context cx, Scriptable scope, ScriptableObject ctor, String name, SymbolKey key) { - Scriptable sym = cx.newObject(scope, CLASS_NAME, new Object[] {name, key}); + Context cx, Scriptable scope, LambdaConstructor ctor, String name, SymbolKey key) { + Scriptable sym = (Scriptable) ctor.call(cx, scope, scope, new Object[] {name, key}); ctor.defineProperty(name, sym, DONTENUM | READONLY | PERMANENT); } - @Override - protected int findPrototypeId(String s) { - int id = 0; - switch (s) { - case "constructor": - id = Id_constructor; - break; - case "toString": - id = Id_toString; - break; - case "valueOf": - id = Id_valueOf; - break; - default: - id = 0; - break; - } - return id; - } - - @Override - protected int findPrototypeId(Symbol key) { - if (SymbolKey.TO_STRING_TAG.equals(key)) { - return SymbolId_toStringTag; - } else if (SymbolKey.TO_PRIMITIVE.equals(key)) { - return SymbolId_toPrimitive; - } - return 0; - } - - private static final int ConstructorId_keyFor = -2, - ConstructorId_for = -1, - Id_constructor = 1, - Id_toString = 2, - Id_valueOf = 4, - SymbolId_toStringTag = 3, - SymbolId_toPrimitive = 5, - MAX_PROTOTYPE_ID = SymbolId_toPrimitive; - - @Override - protected void initPrototypeId(int id) { - switch (id) { - case Id_constructor: - initPrototypeMethod(CLASS_NAME, id, "constructor", 0); - break; - case Id_toString: - initPrototypeMethod(CLASS_NAME, id, "toString", 0); - break; - case Id_valueOf: - initPrototypeMethod(CLASS_NAME, id, "valueOf", 0); - break; - case SymbolId_toStringTag: - initPrototypeValue(id, SymbolKey.TO_STRING_TAG, CLASS_NAME, DONTENUM | READONLY); - break; - case SymbolId_toPrimitive: - initPrototypeMethod( - CLASS_NAME, id, SymbolKey.TO_PRIMITIVE, "Symbol.toPrimitive", 1); - break; - default: - super.initPrototypeId(id); - break; - } - } - - @Override - public Object execIdCall( - IdFunctionObject f, Context cx, Scriptable scope, Scriptable thisObj, Object[] args) { - if (!f.hasTag(CLASS_NAME)) { - return super.execIdCall(f, cx, scope, thisObj, args); - } - int id = f.methodId(); - switch (id) { - case ConstructorId_for: - return js_for(cx, scope, args); - case ConstructorId_keyFor: - return js_keyFor(cx, scope, args); - - case Id_constructor: - if (thisObj == null) { - if (cx.getThreadLocal(CONSTRUCTOR_SLOT) == null) { - // We should never get to this via "new". - throw ScriptRuntime.typeErrorById("msg.no.symbol.new"); - } - // Unless we are being called by our own internal "new" - return js_constructor(args); - } - return construct(cx, scope, args); - - case Id_toString: - return getSelf(cx, scope, thisObj).toString(); - case Id_valueOf: - case SymbolId_toPrimitive: - return getSelf(cx, scope, thisObj).js_valueOf(); - default: - return super.execIdCall(f, cx, scope, thisObj, args); - } - } - private static NativeSymbol getSelf(Context cx, Scriptable scope, Object thisObj) { try { return (NativeSymbol) ScriptRuntime.toObject(cx, scope, thisObj); @@ -214,16 +159,10 @@ private static NativeSymbol getSelf(Context cx, Scriptable scope, Object thisObj } } - private static NativeSymbol js_constructor(Object[] args) { - String desc; - if (args.length > 0) { - if (Undefined.instance.equals(args[0])) { - desc = ""; - } else { - desc = ScriptRuntime.toString(args[0]); - } - } else { - desc = ""; + private static NativeSymbol js_constructor(Context cx, Scriptable scope, Object[] args) { + String desc = null; + if (args.length > 0 && !Undefined.isUndefined(args[0])) { + desc = ScriptRuntime.toString(args[0]); } if (args.length > 1) { @@ -233,18 +172,29 @@ private static NativeSymbol js_constructor(Object[] args) { return new NativeSymbol(new SymbolKey(desc)); } - private Object js_valueOf() { + private static String js_toString( + Context cx, Scriptable scope, Scriptable thisObj, Object[] args) { + return getSelf(cx, scope, thisObj).toString(); + } + + private static Object js_valueOf( + Context cx, Scriptable scope, Scriptable thisObj, Object[] args) { // In the case that "Object()" was called we actually have a different "internal slot" - return symbolData; + return getSelf(cx, scope, thisObj).symbolData; + } + + private static Object js_description(Scriptable thisObj) { + NativeSymbol self = LambdaConstructor.convertThisObject(thisObj, NativeSymbol.class); + return self.getKey().getDescription(); } - private Object js_for(Context cx, Scriptable scope, Object[] args) { + private static Object js_for(Context cx, Scriptable scope, Scriptable thisObj, Object[] args) { String name = (args.length > 0 ? ScriptRuntime.toString(args[0]) : ScriptRuntime.toString(Undefined.instance)); - Map table = getGlobalMap(); + Map table = getGlobalMap(scope); NativeSymbol ret = table.get(name); if (ret == null) { @@ -255,14 +205,15 @@ private Object js_for(Context cx, Scriptable scope, Object[] args) { } @SuppressWarnings("ReferenceEquality") - private Object js_keyFor(Context cx, Scriptable scope, Object[] args) { + private static Object js_keyFor( + Context cx, Scriptable scope, Scriptable thisObj, Object[] args) { Object s = (args.length > 0 ? args[0] : Undefined.instance); if (!(s instanceof NativeSymbol)) { throw ScriptRuntime.throwCustomError(cx, scope, "TypeError", "Not a Symbol"); } NativeSymbol sym = (NativeSymbol) s; - Map table = getGlobalMap(); + Map table = getGlobalMap(scope); for (Map.Entry e : table.entrySet()) { if (e.getValue().key == sym.key) { return e.getKey(); @@ -341,8 +292,8 @@ SymbolKey getKey() { } @SuppressWarnings("unchecked") - private Map getGlobalMap() { - ScriptableObject top = (ScriptableObject) getTopLevelScope(this); + private static Map getGlobalMap(Scriptable scope) { + ScriptableObject top = (ScriptableObject) getTopLevelScope(scope); Map map = (Map) top.getAssociatedValue(GLOBAL_TABLE_KEY); if (map == null) { diff --git a/rhino/src/main/java/org/mozilla/javascript/SymbolKey.java b/rhino/src/main/java/org/mozilla/javascript/SymbolKey.java index 4649777cf2..a922b760cf 100644 --- a/rhino/src/main/java/org/mozilla/javascript/SymbolKey.java +++ b/rhino/src/main/java/org/mozilla/javascript/SymbolKey.java @@ -25,14 +25,27 @@ public class SymbolKey implements Symbol, Serializable { public static final SymbolKey SPLIT = new SymbolKey("Symbol.split"); public static final SymbolKey UNSCOPABLES = new SymbolKey("Symbol.unscopables"); - private String name; + // If passed a javascript undefined, this will be a (java) null + private final String name; public SymbolKey(String name) { this.name = name; } + /** + * Returns the symbol's name. Returns empty string for anonymous symbol (i.e. something created + * with Symbol()). + */ public String getName() { - return name; + return name != null ? name : ""; + } + + /** + * Returns the symbol's description - will return {@link Undefined#instance} if we have an + * anonymous symbol (i.e. something created with Symbol()). + */ + public Object getDescription() { + return name != null ? name : Undefined.instance; } @Override diff --git a/tests/testsrc/jstests/es6/symbols.js b/tests/testsrc/jstests/es6/symbols.js index 534fbe22fd..021d36bbd8 100644 --- a/tests/testsrc/jstests/es6/symbols.js +++ b/tests/testsrc/jstests/es6/symbols.js @@ -571,4 +571,19 @@ TestStringify('{}', symbol_wrapper); symbol_wrapper.a = 1; TestStringify('{"a":1}', symbol_wrapper); + +function TestDescription() { + assertEquals(Symbol("abc").description, "abc"); + assertEquals(Symbol.iterator.description, "Symbol.iterator"); + assertEquals(Symbol().description, undefined); + + assertFalse(Symbol("abc").hasOwnProperty("description")); + assertTrue(Symbol.prototype.hasOwnProperty("description")); + assertTrue(Object.getOwnPropertyDescriptor(Symbol.prototype, "description").configurable); + assertFalse(Object.getOwnPropertyDescriptor(Symbol.prototype, "description").enumerable); + assertTrue(Object.getOwnPropertyDescriptor(Symbol.prototype, "description").get !== null); +} +TestDescription(); + + "success"; diff --git a/tests/testsrc/test262.properties b/tests/testsrc/test262.properties index 734e5b37e0..1fbc8d0a68 100644 --- a/tests/testsrc/test262.properties +++ b/tests/testsrc/test262.properties @@ -2299,27 +2299,17 @@ built-ins/String 140/1182 (11.84%) built-ins/StringIteratorPrototype 0/7 (0.0%) -built-ins/Symbol 34/92 (36.96%) +built-ins/Symbol 24/92 (26.09%) asyncIterator/prop-desc.js - for/cross-realm.js - for/description.js for/not-a-constructor.js {unsupported: [Reflect.construct]} hasInstance/cross-realm.js isConcatSpreadable/cross-realm.js iterator/cross-realm.js keyFor/arg-non-symbol.js - keyFor/cross-realm.js keyFor/not-a-constructor.js {unsupported: [Reflect.construct]} matchAll 2/2 (100.0%) match/cross-realm.js - prototype/description/description-symboldescriptivestring.js - prototype/description/descriptor.js - prototype/description/get.js prototype/description/this-val-non-symbol.js - prototype/description/this-val-symbol.js - prototype/description/wrapper.js - prototype/Symbol.toPrimitive/name.js - prototype/Symbol.toPrimitive/prop-desc.js prototype/Symbol.toPrimitive/redefined-symbol-wrapper-ordinary-toprimitive.js prototype/Symbol.toPrimitive/removed-symbol-wrapper-ordinary-toprimitive.js prototype/toString/not-a-constructor.js {unsupported: [Reflect.construct]}