From 4a27e8eaf3e8d3ba690ed3841212a3d13de16887 Mon Sep 17 00:00:00 2001 From: nabacg Date: Wed, 7 Aug 2024 12:05:03 +0100 Subject: [PATCH 01/10] adding new type of Slot called OwnerAwareLambdaSlot, that allows defining getter / setter properties using Lambda functions that accept owner object ('this') as one of parameters, thus allowing to implement properties that require access to instance fields etc. --- .../mozilla/javascript/LambdaConstructor.java | 14 + .../javascript/OwnerAwareLambdaSlot.java | 88 +++++ .../mozilla/javascript/ScriptableObject.java | 40 +++ .../tests/OwnerAwareLambdaSlotTest.java | 319 ++++++++++++++++++ 4 files changed, 461 insertions(+) create mode 100644 rhino/src/main/java/org/mozilla/javascript/OwnerAwareLambdaSlot.java create mode 100644 tests/src/test/java/org/mozilla/javascript/tests/OwnerAwareLambdaSlotTest.java diff --git a/rhino/src/main/java/org/mozilla/javascript/LambdaConstructor.java b/rhino/src/main/java/org/mozilla/javascript/LambdaConstructor.java index 03c36c0b22..e248dd0f10 100644 --- a/rhino/src/main/java/org/mozilla/javascript/LambdaConstructor.java +++ b/rhino/src/main/java/org/mozilla/javascript/LambdaConstructor.java @@ -6,6 +6,9 @@ package org.mozilla.javascript; +import java.util.function.BiConsumer; +import java.util.function.Function; + /** * This class implements a JavaScript function that may be used as a constructor by delegating to an * interface that can be easily implemented as a lambda. The LambdaFunction class may be used to add @@ -120,6 +123,17 @@ public void definePrototypeProperty(Symbol key, Object value, int attributes) { proto.defineProperty(key, value, attributes); } + public void definePrototypeProperty(String name, java.util.function.Function getter, int attributes) { + ScriptableObject proto = getPrototypeScriptable(); + proto.defineProperty(name, getter, null, attributes ); + } + + public void definePrototypeProperty(String name, Function getter, BiConsumer setter, int attributes) { + ScriptableObject proto = getPrototypeScriptable(); + proto.defineProperty(name, getter, setter, attributes ); + } + + /** * Define a function property directly on the constructor that is implemented under the covers * by a LambdaFunction. diff --git a/rhino/src/main/java/org/mozilla/javascript/OwnerAwareLambdaSlot.java b/rhino/src/main/java/org/mozilla/javascript/OwnerAwareLambdaSlot.java new file mode 100644 index 0000000000..93c8917ffb --- /dev/null +++ b/rhino/src/main/java/org/mozilla/javascript/OwnerAwareLambdaSlot.java @@ -0,0 +1,88 @@ +package org.mozilla.javascript; + +import java.util.IdentityHashMap; +import java.util.function.BiConsumer; +import java.util.function.Function; + +/** + * A specialized property accessor using lambda functions, similar to {@link LambdaSlot}, + * but allows defining properties with getter and setter lambdas that require access to the owner object ('this'). + * This enables the implementation of properties that can access instance fields of the owner. + * + * Unlike {@link LambdaSlot}, Lambda functions used to define getter and setter logic require the owner's + * `Scriptable` object as one of the parameters. + * This is particularly useful for implementing properties that behave like standard JavaScript properties, + * but are implemented with native functionality without the need for reflection. + */ +public class OwnerAwareLambdaSlot extends Slot { + private transient Function getter; + private transient BiConsumer setter; + OwnerAwareLambdaSlot(Object name, int index) { + super(name, index, 0); + } + + OwnerAwareLambdaSlot(Slot oldSlot) { + super(oldSlot); + } + + @Override + boolean isValueSlot() { + return false; + } + + + @Override + ScriptableObject getPropertyDescriptor(Context cx, Scriptable scope) { + ScriptableObject desc = (ScriptableObject) cx.newObject(scope); + if (getter != null) { + desc.defineProperty("get", + new LambdaFunction(scope, "get " + super.name, 0, (cx1, scope1, thisObj, args) -> + getter.apply(thisObj)), ScriptableObject.EMPTY); + } + + if (setter != null) { + desc.defineProperty("set", + new LambdaFunction(scope, "set " + super.name, 1, (cx1, scope1, thisObj, args) -> + { + setter.accept(thisObj, args[0]); + return Undefined.instance; + }), ScriptableObject.EMPTY); + } + desc.setCommonDescriptorProperties(getAttributes(), false); + return desc; + } + + @Override + public boolean setValue(Object value, Scriptable scope, Scriptable owner, boolean isStrict) { + if (setter != null) { + setter.accept(owner, value); + return true; + } + + if (isStrict) { + // in strict mode + throw ScriptRuntime.typeErrorById("msg.modify.readonly", name); + } else { + super.setValue(value, scope, owner, false); + } + + return true; + } + + @Override + public Object getValue(Scriptable owner) { + if (getter != null) { + return getter.apply(owner); + } + Object v = super.getValue(owner); + return v == null ? Undefined.instance : v; + } + + public void setGetter(Function getter) { + this.getter = getter; + } + + public void setSetter(BiConsumer setter) { + this.setter = setter; + } +} diff --git a/rhino/src/main/java/org/mozilla/javascript/ScriptableObject.java b/rhino/src/main/java/org/mozilla/javascript/ScriptableObject.java index abe2881e6e..a9af9c734a 100644 --- a/rhino/src/main/java/org/mozilla/javascript/ScriptableObject.java +++ b/rhino/src/main/java/org/mozilla/javascript/ScriptableObject.java @@ -28,6 +28,7 @@ import java.util.Locale; import java.util.Map; import java.util.Objects; +import java.util.function.BiConsumer; import java.util.function.Consumer; import java.util.function.Supplier; import org.mozilla.javascript.ScriptRuntime.StringIdOrIndex; @@ -1690,6 +1691,35 @@ public void defineProperty( slot.setter = setter; } + /** + * Define a property on this object that is implemented using lambda functions accepting Scriptable `this` object + * as first parameter. Unlike with `defineProperty(String name, Supplier getter, Consumer setter, int attributes)` + * where getter and setter need to have access to target object instance, this allows for defining properties on + * LambdaConstructor prototype providing getter and setter logic with java instance methods. + * If a property with the same name already exists, then it will be replaced. This property will appear to the + * JavaScript user exactly like any other property -- unlike Function properties and those based + * on reflection, the property descriptor will only reflect the value as defined by this + * function. + * + * @param name the name of the property + * @param getter a function that given Scriptable `this` returns the value of the property. If null, throws typeError + * @param setter a function that Scriptable `this` and a value sets the value of the property, by calling appropriate + * method on `this`. If null, then the value will be + * set directly and may not be retrieved by the getter. + * @param attributes the attributes to set on the property + */ + public void defineProperty( + String name, java.util.function.Function getter, BiConsumer setter, int attributes) { + if (getter == null && setter == null) + throw ScriptRuntime.typeError("at least one of {getter, setter} is required"); + + OwnerAwareLambdaSlot slot = slotMap.compute(name, 0, ScriptableObject::ensureOwnerAwareLambdaSlot); + slot.setAttributes(attributes); + + slot.setGetter(getter); + slot.setSetter(setter); + } + protected void checkPropertyDefinition(ScriptableObject desc) { Object getter = getProperty(desc, "get"); if (getter != NOT_FOUND && getter != Undefined.instance && !(getter instanceof Callable)) { @@ -2695,6 +2725,16 @@ private static LambdaSlot ensureLambdaSlot(Object name, int index, Slot existing } } + private static OwnerAwareLambdaSlot ensureOwnerAwareLambdaSlot(Object name, int index, Slot existing) { + if (existing == null) { + return new OwnerAwareLambdaSlot(name, index); + } else if (existing instanceof OwnerAwareLambdaSlot) { + return (OwnerAwareLambdaSlot) existing; + } else { + return new OwnerAwareLambdaSlot(existing); + } + } + private void writeObject(ObjectOutputStream out) throws IOException { out.defaultWriteObject(); final long stamp = slotMap.readLock(); diff --git a/tests/src/test/java/org/mozilla/javascript/tests/OwnerAwareLambdaSlotTest.java b/tests/src/test/java/org/mozilla/javascript/tests/OwnerAwareLambdaSlotTest.java new file mode 100644 index 0000000000..d0f90969d6 --- /dev/null +++ b/tests/src/test/java/org/mozilla/javascript/tests/OwnerAwareLambdaSlotTest.java @@ -0,0 +1,319 @@ +package org.mozilla.javascript.tests; + +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.mozilla.javascript.Context; +import org.mozilla.javascript.ContextFactory; +import org.mozilla.javascript.EcmaError; +import org.mozilla.javascript.LambdaConstructor; +import org.mozilla.javascript.Scriptable; +import org.mozilla.javascript.ScriptableObject; +import org.mozilla.javascript.Undefined; + +import static org.junit.Assert.*; +import static org.mozilla.javascript.ScriptableObject.DONTENUM; +import static org.mozilla.javascript.tests.OwnerAwareLambdaSlotTest.StatusHolder.self; + +public class OwnerAwareLambdaSlotTest { + private Context cx; + private ScriptableObject scope; + + @Before + public void setUp() throws Exception { + cx = Context.enter(); + scope = cx.initStandardObjects(); + } + + @After + public void tearDown() throws Exception { + Context.exit(); + } + + + @Test + public void testGetterProperty() { + StatusHolder + .init(scope) + .definePrototypeProperty("status", + (thisObj) -> self(thisObj).getStatus(), + (thisObj, value) -> + self(thisObj).setStatus(value), + DONTENUM); + + Object getterResult = cx.evaluateString(scope, "s = new StatusHolder('InProgress'); s.status", "source", 1, null); + assertEquals("InProgress", getterResult); + + } + + @Test + public void testThrowIfNeitherGetterOrSetterAreDefined() { + var error = assertThrows(EcmaError.class, () -> + StatusHolder + .init(scope) + .definePrototypeProperty("status", + null, + null, + DONTENUM)); + assertTrue(error.toString().contains("at least one of {getter, setter} is required")); + } + + @Test + public void testCanUpdateValueUsingSetter() { + StatusHolder + .init(scope) + .definePrototypeProperty("status", + (thisObj) -> self(thisObj).getStatus(), + (thisObj, value) -> + self(thisObj).setStatus(value), + DONTENUM); + + Object getterResult = cx.evaluateString(scope, "s = new StatusHolder('InProgress'); s.status", "source", 1, null); + assertEquals("InProgress", getterResult); + + Object setResult = cx.evaluateString(scope, "s.status = 'DONE';", "source", 1, null); + + Object newStatus = cx.evaluateString(scope, "s.status", "source", 1, null); + assertEquals( "NewStatus: DONE", newStatus); + } + + @Test + public void testOnlyGetterCanBeAccessed() { + StatusHolder + .init(scope) + .definePrototypeProperty("status", (thisObj) -> self(thisObj).getStatus(), DONTENUM); + + Object getterResult = cx.evaluateString(scope, "new StatusHolder('OK').status", "source", 1, null); + assertEquals("OK", getterResult); + + Object hiddenFieldResult = cx.evaluateString(scope, "new StatusHolder('OK').hiddenStatus", "source", 1, null); + assertEquals("fields not explicitly defined as properties should return undefined", Undefined.instance, hiddenFieldResult); + } + + @Test + public void testWhenNoSetterDefined_InStrictMode_WillThrowException() { + StatusHolder + .init(scope) + .definePrototypeProperty("status", + (thisObj) -> self(thisObj).getStatus(), + DONTENUM); + Object getterResult = cx.evaluateString(scope, "s = new StatusHolder('Constant'); s.status", "source", 1, null); + assertEquals("Constant", getterResult); + + var error = assertThrows(EcmaError.class, () -> + cx.evaluateString(scope, + "\"use strict\"; s.status = 'DONE'; s.status", "source", 1, null)); + assertTrue(error.toString().contains("Cannot modify readonly property: status.")); + } + + @Test + public void testWhenNoSetterDefined_InNormalMode_NoErrorButValueIsNotChanged() { + StatusHolder + .init(scope) + .definePrototypeProperty("status", + (thisObj) -> self(thisObj).getStatus(), + DONTENUM); + + Object getterResult = cx.evaluateString(scope, "s = new StatusHolder('Constant'); s.status", "source", 1, null); + assertEquals("Constant", getterResult); + + Object setResult = cx.evaluateString(scope, "s.status = 'DONE'; s.status", "source", 1, null); + assertEquals("status won't be changed", "Constant", setResult); + + Object shObj = cx.evaluateString(scope, "s", "source", 1, null); + var statusHolder = (StatusHolder) shObj; + assertEquals("Constant", statusHolder.getStatus()); + } + + @Test + public void testSetterOnly_WillModifyUnderlyingValue() { + StatusHolder + .init(scope) + .definePrototypeProperty("status", + null, + (thisObj, value) -> + self(thisObj).setStatus(value), + DONTENUM); + cx.evaluateString(scope, "s = new StatusHolder('Constant')", "source", 1, null); + + cx.evaluateString(scope, "s.status = 'DONE'; s.status", "source", 1, null); + + + Object newStatus = cx.evaluateString(scope, "s.status", "source", 1, null); + assertEquals( Undefined.instance, newStatus); + Object shObj = cx.evaluateString(scope, "s", "source", 1, null); + var statusHolder = (StatusHolder) shObj; + assertEquals("NewStatus: DONE", statusHolder.getStatus()); + } + + + + // using getOwnPropertyDescriptor to access property + + @Test + public void testGetterUsing_getOwnPropertyDescriptor() { + StatusHolder + .init(scope) + .definePrototypeProperty("status", + (thisObj) -> self(thisObj).getStatus(), DONTENUM); + + Object result = cx.evaluateString(scope, "s = new StatusHolder('InProgress');" + + "f = Object.getOwnPropertyDescriptor(Object.getPrototypeOf(s), 'status');" + + "f.get.call(s)", + "source", 1, null); + assertEquals("InProgress", result); + } + + + @Test + public void testSetterOnlyUsing_getOwnPropertyDescriptor() { + StatusHolder + .init(scope) + .definePrototypeProperty("status", + null, + (thisObj, value) -> + self(thisObj).setStatus(value), DONTENUM); + + Object shObj = cx.evaluateString(scope, + "s = new StatusHolder('InProgress');" + + "f = Object.getOwnPropertyDescriptor(Object.getPrototypeOf(s), 'status');" + + "f.set.call(s, 'DONE');" + + "s", + "source", 1, null); + var statusHolder = (StatusHolder) shObj; + assertEquals("NewStatus: DONE", statusHolder.getStatus()); + } + + @Test + public void testSetValueUsing_getOwnPropertyDescriptor() { + StatusHolder + .init(scope) + .definePrototypeProperty("status", + (thisObj) -> self(thisObj).getStatus(), + (thisObj, value) -> + self(thisObj).setStatus(value), DONTENUM); + + Object result = cx.evaluateString(scope, + "s = new StatusHolder('InProgress');" + + "f = Object.getOwnPropertyDescriptor(Object.getPrototypeOf(s), 'status');" + + "f.set.call(s, 'DONE');" + + "s.status", + "source", 1, null); + assertEquals("Status with prefix", "NewStatus: DONE", result); + } + + + + @Test + public void testSetterOnlyUsing_getOwnPropertyDescriptor_ErrorOnGet() { + StatusHolder + .init(scope) + .definePrototypeProperty("status", + null, + (thisObj, value) -> + self(thisObj).setStatus(value), DONTENUM); + + var error = assertThrows(EcmaError.class, () -> + cx.evaluateString(scope, + "var s = new StatusHolder('InProgress');" + + "var f = Object.getOwnPropertyDescriptor(Object.getPrototypeOf(s), 'status');" + + "f.get.call(s)", + "source", 1, null)); + assertTrue(error.toString().contains("Cannot call method \"call\" of undefined")); + } + + @Test + public void testSetterOnlyUsing_getOwnPropertyDescriptor_InStrictMode_ErrorOnGet() { + StatusHolder + .init(scope) + .definePrototypeProperty("status", + null, + (thisObj, value) -> + self(thisObj).setStatus(value), DONTENUM); + + var error = assertThrows(EcmaError.class, () -> + cx.evaluateString(scope, + "\"use strict\";"+ + "var s = new StatusHolder('InProgress');" + + "var f = Object.getOwnPropertyDescriptor(Object.getPrototypeOf(s), 'status');" + + "f.get.call(s)", + "source", 1, null)); + assertTrue(error.toString().contains("Cannot call method \"call\" of undefined")); + } + + @Test + public void testGetterOnlyUsing_getOwnPropertyDescriptor_ErrorOnSet() { + StatusHolder + .init(scope) + .definePrototypeProperty("status", + (thisObj) -> self(thisObj).getStatus(), DONTENUM); + + + var error = assertThrows(EcmaError.class, () -> + cx.evaluateString(scope, + "var s = new StatusHolder('InProgress');" + + "var f = Object.getOwnPropertyDescriptor(Object.getPrototypeOf(s), 'status');" + + "f.set.call(s, 'DONE');" + + "s.status", + "source", 1, null)); + assertTrue(error.toString().contains("Cannot call method \"call\" of undefined")); + } + + @Test + public void testGetterOnlyUsing_getOwnPropertyDescriptor_InStrictMode_ErrorOnSet() { + StatusHolder + .init(scope) + .definePrototypeProperty("status", + (thisObj) -> self(thisObj).getStatus(), DONTENUM); + + + var error = assertThrows(EcmaError.class, () -> + cx.evaluateString(scope, + "\"use strict\";" + + "var s = new StatusHolder('InProgress');" + + "var f = Object.getOwnPropertyDescriptor(Object.getPrototypeOf(s), 'status');" + + "f.set.call(s, 'DONE');" + + "s.status", + "source", 1, null)); + assertTrue(error.toString().contains("Cannot call method \"call\" of undefined")); + } + + + + + static class StatusHolder extends ScriptableObject { + private String status; + private final String hiddenStatus; + + static LambdaConstructor init(Scriptable scope) { + LambdaConstructor constructor = new LambdaConstructor(scope, "StatusHolder", 1, + LambdaConstructor.CONSTRUCTOR_NEW, + (cx, scope1, args) -> new StatusHolder((String)args[0])); + + ScriptableObject.defineProperty(scope, "StatusHolder", constructor, DONTENUM); + return constructor; + } + + static StatusHolder self(Scriptable thisObj) { + return LambdaConstructor.convertThisObject(thisObj, StatusHolder.class); + } + + StatusHolder(String status) { + this.status = status; + this.hiddenStatus = "NotQuiteReady"; + } + + public String getStatus() { + return status; + } + + @Override + public String getClassName() { + return "StatusHolder"; + } + + public void setStatus(Object value) { + this.status = "NewStatus: "+ (String)value; + } + } +} From e0af91aff78e059125e654b41d2cf4a6855eb9a9 Mon Sep 17 00:00:00 2001 From: Grzegorz Caban Date: Thu, 22 Aug 2024 11:41:50 +0100 Subject: [PATCH 02/10] Update tests/src/test/java/org/mozilla/javascript/tests/OwnerAwareLambdaSlotTest.java Co-authored-by: Roland Praml --- .../org/mozilla/javascript/tests/OwnerAwareLambdaSlotTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/src/test/java/org/mozilla/javascript/tests/OwnerAwareLambdaSlotTest.java b/tests/src/test/java/org/mozilla/javascript/tests/OwnerAwareLambdaSlotTest.java index d0f90969d6..264718322c 100644 --- a/tests/src/test/java/org/mozilla/javascript/tests/OwnerAwareLambdaSlotTest.java +++ b/tests/src/test/java/org/mozilla/javascript/tests/OwnerAwareLambdaSlotTest.java @@ -74,7 +74,7 @@ public void testCanUpdateValueUsingSetter() { Object setResult = cx.evaluateString(scope, "s.status = 'DONE';", "source", 1, null); Object newStatus = cx.evaluateString(scope, "s.status", "source", 1, null); - assertEquals( "NewStatus: DONE", newStatus); + assertEquals("NewStatus: DONE", newStatus); } @Test From d5cafc7b4e55d6506e9c267f8d79220f1dbed63f Mon Sep 17 00:00:00 2001 From: nabacg Date: Thu, 22 Aug 2024 16:48:51 +0100 Subject: [PATCH 03/10] re-formating with spotlessApply --- .../mozilla/javascript/LambdaConstructor.java | 14 +- .../javascript/OwnerAwareLambdaSlot.java | 46 ++- .../mozilla/javascript/ScriptableObject.java | 37 +- .../tests/OwnerAwareLambdaSlotTest.java | 347 ++++++++++-------- 4 files changed, 251 insertions(+), 193 deletions(-) diff --git a/rhino/src/main/java/org/mozilla/javascript/LambdaConstructor.java b/rhino/src/main/java/org/mozilla/javascript/LambdaConstructor.java index e248dd0f10..866e5a0b8b 100644 --- a/rhino/src/main/java/org/mozilla/javascript/LambdaConstructor.java +++ b/rhino/src/main/java/org/mozilla/javascript/LambdaConstructor.java @@ -123,17 +123,21 @@ public void definePrototypeProperty(Symbol key, Object value, int attributes) { proto.defineProperty(key, value, attributes); } - public void definePrototypeProperty(String name, java.util.function.Function getter, int attributes) { + public void definePrototypeProperty( + String name, java.util.function.Function getter, int attributes) { ScriptableObject proto = getPrototypeScriptable(); - proto.defineProperty(name, getter, null, attributes ); + proto.defineProperty(name, getter, null, attributes); } - public void definePrototypeProperty(String name, Function getter, BiConsumer setter, int attributes) { + public void definePrototypeProperty( + String name, + Function getter, + BiConsumer setter, + int attributes) { ScriptableObject proto = getPrototypeScriptable(); - proto.defineProperty(name, getter, setter, attributes ); + proto.defineProperty(name, getter, setter, attributes); } - /** * Define a function property directly on the constructor that is implemented under the covers * by a LambdaFunction. diff --git a/rhino/src/main/java/org/mozilla/javascript/OwnerAwareLambdaSlot.java b/rhino/src/main/java/org/mozilla/javascript/OwnerAwareLambdaSlot.java index 93c8917ffb..362d76038a 100644 --- a/rhino/src/main/java/org/mozilla/javascript/OwnerAwareLambdaSlot.java +++ b/rhino/src/main/java/org/mozilla/javascript/OwnerAwareLambdaSlot.java @@ -1,22 +1,23 @@ package org.mozilla.javascript; -import java.util.IdentityHashMap; import java.util.function.BiConsumer; import java.util.function.Function; /** - * A specialized property accessor using lambda functions, similar to {@link LambdaSlot}, - * but allows defining properties with getter and setter lambdas that require access to the owner object ('this'). - * This enables the implementation of properties that can access instance fields of the owner. + * A specialized property accessor using lambda functions, similar to {@link LambdaSlot}, but allows + * defining properties with getter and setter lambdas that require access to the owner object + * ('this'). This enables the implementation of properties that can access instance fields of the + * owner. * - * Unlike {@link LambdaSlot}, Lambda functions used to define getter and setter logic require the owner's - * `Scriptable` object as one of the parameters. - * This is particularly useful for implementing properties that behave like standard JavaScript properties, - * but are implemented with native functionality without the need for reflection. + *

Unlike {@link LambdaSlot}, Lambda functions used to define getter and setter logic require the + * owner's `Scriptable` object as one of the parameters. This is particularly useful for + * implementing properties that behave like standard JavaScript properties, but are implemented with + * native functionality without the need for reflection. */ public class OwnerAwareLambdaSlot extends Slot { private transient Function getter; private transient BiConsumer setter; + OwnerAwareLambdaSlot(Object name, int index) { super(name, index, 0); } @@ -30,23 +31,32 @@ boolean isValueSlot() { return false; } - @Override ScriptableObject getPropertyDescriptor(Context cx, Scriptable scope) { ScriptableObject desc = (ScriptableObject) cx.newObject(scope); if (getter != null) { - desc.defineProperty("get", - new LambdaFunction(scope, "get " + super.name, 0, (cx1, scope1, thisObj, args) -> - getter.apply(thisObj)), ScriptableObject.EMPTY); + desc.defineProperty( + "get", + new LambdaFunction( + scope, + "get " + super.name, + 0, + (cx1, scope1, thisObj, args) -> getter.apply(thisObj)), + ScriptableObject.EMPTY); } if (setter != null) { - desc.defineProperty("set", - new LambdaFunction(scope, "set " + super.name, 1, (cx1, scope1, thisObj, args) -> - { - setter.accept(thisObj, args[0]); - return Undefined.instance; - }), ScriptableObject.EMPTY); + desc.defineProperty( + "set", + new LambdaFunction( + scope, + "set " + super.name, + 1, + (cx1, scope1, thisObj, args) -> { + setter.accept(thisObj, args[0]); + return Undefined.instance; + }), + ScriptableObject.EMPTY); } desc.setCommonDescriptorProperties(getAttributes(), false); return desc; diff --git a/rhino/src/main/java/org/mozilla/javascript/ScriptableObject.java b/rhino/src/main/java/org/mozilla/javascript/ScriptableObject.java index a9af9c734a..7810d48ea2 100644 --- a/rhino/src/main/java/org/mozilla/javascript/ScriptableObject.java +++ b/rhino/src/main/java/org/mozilla/javascript/ScriptableObject.java @@ -1692,28 +1692,34 @@ public void defineProperty( } /** - * Define a property on this object that is implemented using lambda functions accepting Scriptable `this` object - * as first parameter. Unlike with `defineProperty(String name, Supplier getter, Consumer setter, int attributes)` - * where getter and setter need to have access to target object instance, this allows for defining properties on - * LambdaConstructor prototype providing getter and setter logic with java instance methods. - * If a property with the same name already exists, then it will be replaced. This property will appear to the - * JavaScript user exactly like any other property -- unlike Function properties and those based - * on reflection, the property descriptor will only reflect the value as defined by this - * function. + * Define a property on this object that is implemented using lambda functions accepting + * Scriptable `this` object as first parameter. Unlike with `defineProperty(String name, + * Supplier getter, Consumer setter, int attributes)` where getter and setter + * need to have access to target object instance, this allows for defining properties on + * LambdaConstructor prototype providing getter and setter logic with java instance methods. If + * a property with the same name already exists, then it will be replaced. This property will + * appear to the JavaScript user exactly like any other property -- unlike Function properties + * and those based on reflection, the property descriptor will only reflect the value as defined + * by this function. * * @param name the name of the property - * @param getter a function that given Scriptable `this` returns the value of the property. If null, throws typeError - * @param setter a function that Scriptable `this` and a value sets the value of the property, by calling appropriate - * method on `this`. If null, then the value will be - * set directly and may not be retrieved by the getter. + * @param getter a function that given Scriptable `this` returns the value of the property. If + * null, throws typeError + * @param setter a function that Scriptable `this` and a value sets the value of the property, + * by calling appropriate method on `this`. If null, then the value will be set directly and + * may not be retrieved by the getter. * @param attributes the attributes to set on the property */ public void defineProperty( - String name, java.util.function.Function getter, BiConsumer setter, int attributes) { + String name, + java.util.function.Function getter, + BiConsumer setter, + int attributes) { if (getter == null && setter == null) throw ScriptRuntime.typeError("at least one of {getter, setter} is required"); - OwnerAwareLambdaSlot slot = slotMap.compute(name, 0, ScriptableObject::ensureOwnerAwareLambdaSlot); + OwnerAwareLambdaSlot slot = + slotMap.compute(name, 0, ScriptableObject::ensureOwnerAwareLambdaSlot); slot.setAttributes(attributes); slot.setGetter(getter); @@ -2725,7 +2731,8 @@ private static LambdaSlot ensureLambdaSlot(Object name, int index, Slot existing } } - private static OwnerAwareLambdaSlot ensureOwnerAwareLambdaSlot(Object name, int index, Slot existing) { + private static OwnerAwareLambdaSlot ensureOwnerAwareLambdaSlot( + Object name, int index, Slot existing) { if (existing == null) { return new OwnerAwareLambdaSlot(name, index); } else if (existing instanceof OwnerAwareLambdaSlot) { diff --git a/tests/src/test/java/org/mozilla/javascript/tests/OwnerAwareLambdaSlotTest.java b/tests/src/test/java/org/mozilla/javascript/tests/OwnerAwareLambdaSlotTest.java index 264718322c..32cc563367 100644 --- a/tests/src/test/java/org/mozilla/javascript/tests/OwnerAwareLambdaSlotTest.java +++ b/tests/src/test/java/org/mozilla/javascript/tests/OwnerAwareLambdaSlotTest.java @@ -1,20 +1,19 @@ package org.mozilla.javascript.tests; +import static org.junit.Assert.*; +import static org.mozilla.javascript.ScriptableObject.DONTENUM; +import static org.mozilla.javascript.tests.OwnerAwareLambdaSlotTest.StatusHolder.self; + import org.junit.After; import org.junit.Before; import org.junit.Test; import org.mozilla.javascript.Context; -import org.mozilla.javascript.ContextFactory; import org.mozilla.javascript.EcmaError; import org.mozilla.javascript.LambdaConstructor; import org.mozilla.javascript.Scriptable; import org.mozilla.javascript.ScriptableObject; import org.mozilla.javascript.Undefined; -import static org.junit.Assert.*; -import static org.mozilla.javascript.ScriptableObject.DONTENUM; -import static org.mozilla.javascript.tests.OwnerAwareLambdaSlotTest.StatusHolder.self; - public class OwnerAwareLambdaSlotTest { private Context cx; private ScriptableObject scope; @@ -30,45 +29,44 @@ public void tearDown() throws Exception { Context.exit(); } - @Test public void testGetterProperty() { - StatusHolder - .init(scope) - .definePrototypeProperty("status", + StatusHolder.init(scope) + .definePrototypeProperty( + "status", (thisObj) -> self(thisObj).getStatus(), - (thisObj, value) -> - self(thisObj).setStatus(value), + (thisObj, value) -> self(thisObj).setStatus(value), DONTENUM); - Object getterResult = cx.evaluateString(scope, "s = new StatusHolder('InProgress'); s.status", "source", 1, null); + Object getterResult = + cx.evaluateString( + scope, "s = new StatusHolder('InProgress'); s.status", "source", 1, null); assertEquals("InProgress", getterResult); - } @Test public void testThrowIfNeitherGetterOrSetterAreDefined() { - var error = assertThrows(EcmaError.class, () -> - StatusHolder - .init(scope) - .definePrototypeProperty("status", - null, - null, - DONTENUM)); + var error = + assertThrows( + EcmaError.class, + () -> + StatusHolder.init(scope) + .definePrototypeProperty("status", null, null, DONTENUM)); assertTrue(error.toString().contains("at least one of {getter, setter} is required")); } @Test public void testCanUpdateValueUsingSetter() { - StatusHolder - .init(scope) - .definePrototypeProperty("status", + StatusHolder.init(scope) + .definePrototypeProperty( + "status", (thisObj) -> self(thisObj).getStatus(), - (thisObj, value) -> - self(thisObj).setStatus(value), + (thisObj, value) -> self(thisObj).setStatus(value), DONTENUM); - Object getterResult = cx.evaluateString(scope, "s = new StatusHolder('InProgress'); s.status", "source", 1, null); + Object getterResult = + cx.evaluateString( + scope, "s = new StatusHolder('InProgress'); s.status", "source", 1, null); assertEquals("InProgress", getterResult); Object setResult = cx.evaluateString(scope, "s.status = 'DONE';", "source", 1, null); @@ -79,216 +77,255 @@ public void testCanUpdateValueUsingSetter() { @Test public void testOnlyGetterCanBeAccessed() { - StatusHolder - .init(scope) - .definePrototypeProperty("status", (thisObj) -> self(thisObj).getStatus(), DONTENUM); + StatusHolder.init(scope) + .definePrototypeProperty( + "status", (thisObj) -> self(thisObj).getStatus(), DONTENUM); - Object getterResult = cx.evaluateString(scope, "new StatusHolder('OK').status", "source", 1, null); + Object getterResult = + cx.evaluateString(scope, "new StatusHolder('OK').status", "source", 1, null); assertEquals("OK", getterResult); - Object hiddenFieldResult = cx.evaluateString(scope, "new StatusHolder('OK').hiddenStatus", "source", 1, null); - assertEquals("fields not explicitly defined as properties should return undefined", Undefined.instance, hiddenFieldResult); + Object hiddenFieldResult = + cx.evaluateString(scope, "new StatusHolder('OK').hiddenStatus", "source", 1, null); + assertEquals( + "fields not explicitly defined as properties should return undefined", + Undefined.instance, + hiddenFieldResult); } @Test public void testWhenNoSetterDefined_InStrictMode_WillThrowException() { - StatusHolder - .init(scope) - .definePrototypeProperty("status", - (thisObj) -> self(thisObj).getStatus(), - DONTENUM); - Object getterResult = cx.evaluateString(scope, "s = new StatusHolder('Constant'); s.status", "source", 1, null); + StatusHolder.init(scope) + .definePrototypeProperty( + "status", (thisObj) -> self(thisObj).getStatus(), DONTENUM); + Object getterResult = + cx.evaluateString( + scope, "s = new StatusHolder('Constant'); s.status", "source", 1, null); assertEquals("Constant", getterResult); - var error = assertThrows(EcmaError.class, () -> - cx.evaluateString(scope, - "\"use strict\"; s.status = 'DONE'; s.status", "source", 1, null)); + var error = + assertThrows( + EcmaError.class, + () -> + cx.evaluateString( + scope, + "\"use strict\"; s.status = 'DONE'; s.status", + "source", + 1, + null)); assertTrue(error.toString().contains("Cannot modify readonly property: status.")); } @Test public void testWhenNoSetterDefined_InNormalMode_NoErrorButValueIsNotChanged() { - StatusHolder - .init(scope) - .definePrototypeProperty("status", - (thisObj) -> self(thisObj).getStatus(), - DONTENUM); + StatusHolder.init(scope) + .definePrototypeProperty( + "status", (thisObj) -> self(thisObj).getStatus(), DONTENUM); - Object getterResult = cx.evaluateString(scope, "s = new StatusHolder('Constant'); s.status", "source", 1, null); + Object getterResult = + cx.evaluateString( + scope, "s = new StatusHolder('Constant'); s.status", "source", 1, null); assertEquals("Constant", getterResult); - Object setResult = cx.evaluateString(scope, "s.status = 'DONE'; s.status", "source", 1, null); + Object setResult = + cx.evaluateString(scope, "s.status = 'DONE'; s.status", "source", 1, null); assertEquals("status won't be changed", "Constant", setResult); - Object shObj = cx.evaluateString(scope, "s", "source", 1, null); + Object shObj = cx.evaluateString(scope, "s", "source", 1, null); var statusHolder = (StatusHolder) shObj; assertEquals("Constant", statusHolder.getStatus()); } @Test public void testSetterOnly_WillModifyUnderlyingValue() { - StatusHolder - .init(scope) - .definePrototypeProperty("status", + StatusHolder.init(scope) + .definePrototypeProperty( + "status", null, - (thisObj, value) -> - self(thisObj).setStatus(value), + (thisObj, value) -> self(thisObj).setStatus(value), DONTENUM); cx.evaluateString(scope, "s = new StatusHolder('Constant')", "source", 1, null); cx.evaluateString(scope, "s.status = 'DONE'; s.status", "source", 1, null); - Object newStatus = cx.evaluateString(scope, "s.status", "source", 1, null); - assertEquals( Undefined.instance, newStatus); - Object shObj = cx.evaluateString(scope, "s", "source", 1, null); + assertEquals(Undefined.instance, newStatus); + Object shObj = cx.evaluateString(scope, "s", "source", 1, null); var statusHolder = (StatusHolder) shObj; assertEquals("NewStatus: DONE", statusHolder.getStatus()); } - - // using getOwnPropertyDescriptor to access property @Test public void testGetterUsing_getOwnPropertyDescriptor() { - StatusHolder - .init(scope) - .definePrototypeProperty("status", - (thisObj) -> self(thisObj).getStatus(), DONTENUM); - - Object result = cx.evaluateString(scope, "s = new StatusHolder('InProgress');" + - "f = Object.getOwnPropertyDescriptor(Object.getPrototypeOf(s), 'status');" + - "f.get.call(s)", - "source", 1, null); + StatusHolder.init(scope) + .definePrototypeProperty( + "status", (thisObj) -> self(thisObj).getStatus(), DONTENUM); + + Object result = + cx.evaluateString( + scope, + "s = new StatusHolder('InProgress');" + + "f = Object.getOwnPropertyDescriptor(Object.getPrototypeOf(s), 'status');" + + "f.get.call(s)", + "source", + 1, + null); assertEquals("InProgress", result); } - @Test - public void testSetterOnlyUsing_getOwnPropertyDescriptor() { - StatusHolder - .init(scope) - .definePrototypeProperty("status", + public void testSetterOnlyUsing_getOwnPropertyDescriptor() { + StatusHolder.init(scope) + .definePrototypeProperty( + "status", null, - (thisObj, value) -> - self(thisObj).setStatus(value), DONTENUM); - - Object shObj = cx.evaluateString(scope, - "s = new StatusHolder('InProgress');" + - "f = Object.getOwnPropertyDescriptor(Object.getPrototypeOf(s), 'status');" + - "f.set.call(s, 'DONE');" + - "s", - "source", 1, null); + (thisObj, value) -> self(thisObj).setStatus(value), + DONTENUM); + + Object shObj = + cx.evaluateString( + scope, + "s = new StatusHolder('InProgress');" + + "f = Object.getOwnPropertyDescriptor(Object.getPrototypeOf(s), 'status');" + + "f.set.call(s, 'DONE');" + + "s", + "source", + 1, + null); var statusHolder = (StatusHolder) shObj; assertEquals("NewStatus: DONE", statusHolder.getStatus()); } @Test public void testSetValueUsing_getOwnPropertyDescriptor() { - StatusHolder - .init(scope) - .definePrototypeProperty("status", + StatusHolder.init(scope) + .definePrototypeProperty( + "status", (thisObj) -> self(thisObj).getStatus(), - (thisObj, value) -> - self(thisObj).setStatus(value), DONTENUM); - - Object result = cx.evaluateString(scope, - "s = new StatusHolder('InProgress');" + - "f = Object.getOwnPropertyDescriptor(Object.getPrototypeOf(s), 'status');" + - "f.set.call(s, 'DONE');" + - "s.status", - "source", 1, null); + (thisObj, value) -> self(thisObj).setStatus(value), + DONTENUM); + + Object result = + cx.evaluateString( + scope, + "s = new StatusHolder('InProgress');" + + "f = Object.getOwnPropertyDescriptor(Object.getPrototypeOf(s), 'status');" + + "f.set.call(s, 'DONE');" + + "s.status", + "source", + 1, + null); assertEquals("Status with prefix", "NewStatus: DONE", result); } - - @Test public void testSetterOnlyUsing_getOwnPropertyDescriptor_ErrorOnGet() { - StatusHolder - .init(scope) - .definePrototypeProperty("status", + StatusHolder.init(scope) + .definePrototypeProperty( + "status", null, - (thisObj, value) -> - self(thisObj).setStatus(value), DONTENUM); - - var error = assertThrows(EcmaError.class, () -> - cx.evaluateString(scope, - "var s = new StatusHolder('InProgress');" + - "var f = Object.getOwnPropertyDescriptor(Object.getPrototypeOf(s), 'status');" + - "f.get.call(s)", - "source", 1, null)); + (thisObj, value) -> self(thisObj).setStatus(value), + DONTENUM); + + var error = + assertThrows( + EcmaError.class, + () -> + cx.evaluateString( + scope, + "var s = new StatusHolder('InProgress');" + + "var f = Object.getOwnPropertyDescriptor(Object.getPrototypeOf(s), 'status');" + + "f.get.call(s)", + "source", + 1, + null)); assertTrue(error.toString().contains("Cannot call method \"call\" of undefined")); } @Test public void testSetterOnlyUsing_getOwnPropertyDescriptor_InStrictMode_ErrorOnGet() { - StatusHolder - .init(scope) - .definePrototypeProperty("status", + StatusHolder.init(scope) + .definePrototypeProperty( + "status", null, - (thisObj, value) -> - self(thisObj).setStatus(value), DONTENUM); - - var error = assertThrows(EcmaError.class, () -> - cx.evaluateString(scope, - "\"use strict\";"+ - "var s = new StatusHolder('InProgress');" + - "var f = Object.getOwnPropertyDescriptor(Object.getPrototypeOf(s), 'status');" + - "f.get.call(s)", - "source", 1, null)); + (thisObj, value) -> self(thisObj).setStatus(value), + DONTENUM); + + var error = + assertThrows( + EcmaError.class, + () -> + cx.evaluateString( + scope, + "\"use strict\";" + + "var s = new StatusHolder('InProgress');" + + "var f = Object.getOwnPropertyDescriptor(Object.getPrototypeOf(s), 'status');" + + "f.get.call(s)", + "source", + 1, + null)); assertTrue(error.toString().contains("Cannot call method \"call\" of undefined")); } @Test public void testGetterOnlyUsing_getOwnPropertyDescriptor_ErrorOnSet() { - StatusHolder - .init(scope) - .definePrototypeProperty("status", - (thisObj) -> self(thisObj).getStatus(), DONTENUM); - - - var error = assertThrows(EcmaError.class, () -> - cx.evaluateString(scope, - "var s = new StatusHolder('InProgress');" + - "var f = Object.getOwnPropertyDescriptor(Object.getPrototypeOf(s), 'status');" + - "f.set.call(s, 'DONE');" + - "s.status", - "source", 1, null)); + StatusHolder.init(scope) + .definePrototypeProperty( + "status", (thisObj) -> self(thisObj).getStatus(), DONTENUM); + + var error = + assertThrows( + EcmaError.class, + () -> + cx.evaluateString( + scope, + "var s = new StatusHolder('InProgress');" + + "var f = Object.getOwnPropertyDescriptor(Object.getPrototypeOf(s), 'status');" + + "f.set.call(s, 'DONE');" + + "s.status", + "source", + 1, + null)); assertTrue(error.toString().contains("Cannot call method \"call\" of undefined")); } @Test public void testGetterOnlyUsing_getOwnPropertyDescriptor_InStrictMode_ErrorOnSet() { - StatusHolder - .init(scope) - .definePrototypeProperty("status", - (thisObj) -> self(thisObj).getStatus(), DONTENUM); - - - var error = assertThrows(EcmaError.class, () -> - cx.evaluateString(scope, - "\"use strict\";" + - "var s = new StatusHolder('InProgress');" + - "var f = Object.getOwnPropertyDescriptor(Object.getPrototypeOf(s), 'status');" + - "f.set.call(s, 'DONE');" + - "s.status", - "source", 1, null)); + StatusHolder.init(scope) + .definePrototypeProperty( + "status", (thisObj) -> self(thisObj).getStatus(), DONTENUM); + + var error = + assertThrows( + EcmaError.class, + () -> + cx.evaluateString( + scope, + "\"use strict\";" + + "var s = new StatusHolder('InProgress');" + + "var f = Object.getOwnPropertyDescriptor(Object.getPrototypeOf(s), 'status');" + + "f.set.call(s, 'DONE');" + + "s.status", + "source", + 1, + null)); assertTrue(error.toString().contains("Cannot call method \"call\" of undefined")); } - - - static class StatusHolder extends ScriptableObject { private String status; private final String hiddenStatus; static LambdaConstructor init(Scriptable scope) { - LambdaConstructor constructor = new LambdaConstructor(scope, "StatusHolder", 1, - LambdaConstructor.CONSTRUCTOR_NEW, - (cx, scope1, args) -> new StatusHolder((String)args[0])); + LambdaConstructor constructor = + new LambdaConstructor( + scope, + "StatusHolder", + 1, + LambdaConstructor.CONSTRUCTOR_NEW, + (cx, scope1, args) -> new StatusHolder((String) args[0])); ScriptableObject.defineProperty(scope, "StatusHolder", constructor, DONTENUM); return constructor; @@ -313,7 +350,7 @@ public String getClassName() { } public void setStatus(Object value) { - this.status = "NewStatus: "+ (String)value; + this.status = "NewStatus: " + (String) value; } } } From c4789fc5e8c48974a96fbb669b58d91411a22d6c Mon Sep 17 00:00:00 2001 From: nabacg Date: Thu, 22 Aug 2024 17:09:45 +0100 Subject: [PATCH 04/10] getValue to return null, not Undefined --- .../main/java/org/mozilla/javascript/OwnerAwareLambdaSlot.java | 3 +-- .../org/mozilla/javascript/tests/OwnerAwareLambdaSlotTest.java | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/rhino/src/main/java/org/mozilla/javascript/OwnerAwareLambdaSlot.java b/rhino/src/main/java/org/mozilla/javascript/OwnerAwareLambdaSlot.java index 362d76038a..bc2abf4bd3 100644 --- a/rhino/src/main/java/org/mozilla/javascript/OwnerAwareLambdaSlot.java +++ b/rhino/src/main/java/org/mozilla/javascript/OwnerAwareLambdaSlot.java @@ -84,8 +84,7 @@ public Object getValue(Scriptable owner) { if (getter != null) { return getter.apply(owner); } - Object v = super.getValue(owner); - return v == null ? Undefined.instance : v; + return super.getValue(owner); } public void setGetter(Function getter) { diff --git a/tests/src/test/java/org/mozilla/javascript/tests/OwnerAwareLambdaSlotTest.java b/tests/src/test/java/org/mozilla/javascript/tests/OwnerAwareLambdaSlotTest.java index 32cc563367..ed075a588b 100644 --- a/tests/src/test/java/org/mozilla/javascript/tests/OwnerAwareLambdaSlotTest.java +++ b/tests/src/test/java/org/mozilla/javascript/tests/OwnerAwareLambdaSlotTest.java @@ -149,7 +149,7 @@ public void testSetterOnly_WillModifyUnderlyingValue() { cx.evaluateString(scope, "s.status = 'DONE'; s.status", "source", 1, null); Object newStatus = cx.evaluateString(scope, "s.status", "source", 1, null); - assertEquals(Undefined.instance, newStatus); + assertEquals(null, newStatus); Object shObj = cx.evaluateString(scope, "s", "source", 1, null); var statusHolder = (StatusHolder) shObj; assertEquals("NewStatus: DONE", statusHolder.getStatus()); From e0999319ad50b98f352bb8260f3bc39af379b94a Mon Sep 17 00:00:00 2001 From: nabacg Date: Fri, 23 Aug 2024 08:27:40 +0100 Subject: [PATCH 05/10] OwnerAwareLambdaSlot -> LambdaAccessorSlot rename, plus making sure getPropertyDescriptor only creates single instance of get/set LambdaFunctions, instead of instance per call --- ...ambdaSlot.java => LambdaAccessorSlot.java} | 49 ++++++++++--------- .../mozilla/javascript/ScriptableObject.java | 14 +++--- ...tTest.java => LambdaAccessorSlotTest.java} | 4 +- 3 files changed, 36 insertions(+), 31 deletions(-) rename rhino/src/main/java/org/mozilla/javascript/{OwnerAwareLambdaSlot.java => LambdaAccessorSlot.java} (64%) rename tests/src/test/java/org/mozilla/javascript/tests/{OwnerAwareLambdaSlotTest.java => LambdaAccessorSlotTest.java} (99%) diff --git a/rhino/src/main/java/org/mozilla/javascript/OwnerAwareLambdaSlot.java b/rhino/src/main/java/org/mozilla/javascript/LambdaAccessorSlot.java similarity index 64% rename from rhino/src/main/java/org/mozilla/javascript/OwnerAwareLambdaSlot.java rename to rhino/src/main/java/org/mozilla/javascript/LambdaAccessorSlot.java index bc2abf4bd3..422cf67723 100644 --- a/rhino/src/main/java/org/mozilla/javascript/OwnerAwareLambdaSlot.java +++ b/rhino/src/main/java/org/mozilla/javascript/LambdaAccessorSlot.java @@ -14,15 +14,17 @@ * implementing properties that behave like standard JavaScript properties, but are implemented with * native functionality without the need for reflection. */ -public class OwnerAwareLambdaSlot extends Slot { +public class LambdaAccessorSlot extends Slot { private transient Function getter; private transient BiConsumer setter; + private LambdaFunction getterFunction; + private LambdaFunction setterFunction; - OwnerAwareLambdaSlot(Object name, int index) { + LambdaAccessorSlot(Object name, int index) { super(name, index, 0); } - OwnerAwareLambdaSlot(Slot oldSlot) { + LambdaAccessorSlot(Slot oldSlot) { super(oldSlot); } @@ -35,28 +37,31 @@ boolean isValueSlot() { ScriptableObject getPropertyDescriptor(Context cx, Scriptable scope) { ScriptableObject desc = (ScriptableObject) cx.newObject(scope); if (getter != null) { - desc.defineProperty( - "get", - new LambdaFunction( - scope, - "get " + super.name, - 0, - (cx1, scope1, thisObj, args) -> getter.apply(thisObj)), - ScriptableObject.EMPTY); + if (getterFunction == null) { + this.getterFunction = + new LambdaFunction( + scope, + "get " + super.name, + 0, + (cx1, scope1, thisObj, args) -> getter.apply(thisObj)); + } + + desc.defineProperty("get", this.getterFunction, ScriptableObject.EMPTY); } if (setter != null) { - desc.defineProperty( - "set", - new LambdaFunction( - scope, - "set " + super.name, - 1, - (cx1, scope1, thisObj, args) -> { - setter.accept(thisObj, args[0]); - return Undefined.instance; - }), - ScriptableObject.EMPTY); + if (setterFunction == null) { + this.setterFunction = + new LambdaFunction( + scope, + "set " + super.name, + 1, + (cx1, scope1, thisObj, args) -> { + setter.accept(thisObj, args[0]); + return Undefined.instance; + }); + } + desc.defineProperty("set", this.setterFunction, ScriptableObject.EMPTY); } desc.setCommonDescriptorProperties(getAttributes(), false); return desc; diff --git a/rhino/src/main/java/org/mozilla/javascript/ScriptableObject.java b/rhino/src/main/java/org/mozilla/javascript/ScriptableObject.java index 7810d48ea2..800d38fece 100644 --- a/rhino/src/main/java/org/mozilla/javascript/ScriptableObject.java +++ b/rhino/src/main/java/org/mozilla/javascript/ScriptableObject.java @@ -1718,8 +1718,8 @@ public void defineProperty( if (getter == null && setter == null) throw ScriptRuntime.typeError("at least one of {getter, setter} is required"); - OwnerAwareLambdaSlot slot = - slotMap.compute(name, 0, ScriptableObject::ensureOwnerAwareLambdaSlot); + LambdaAccessorSlot slot = + slotMap.compute(name, 0, ScriptableObject::ensureLambdaAccessorSlot); slot.setAttributes(attributes); slot.setGetter(getter); @@ -2731,14 +2731,14 @@ private static LambdaSlot ensureLambdaSlot(Object name, int index, Slot existing } } - private static OwnerAwareLambdaSlot ensureOwnerAwareLambdaSlot( + private static LambdaAccessorSlot ensureLambdaAccessorSlot( Object name, int index, Slot existing) { if (existing == null) { - return new OwnerAwareLambdaSlot(name, index); - } else if (existing instanceof OwnerAwareLambdaSlot) { - return (OwnerAwareLambdaSlot) existing; + return new LambdaAccessorSlot(name, index); + } else if (existing instanceof LambdaAccessorSlot) { + return (LambdaAccessorSlot) existing; } else { - return new OwnerAwareLambdaSlot(existing); + return new LambdaAccessorSlot(existing); } } diff --git a/tests/src/test/java/org/mozilla/javascript/tests/OwnerAwareLambdaSlotTest.java b/tests/src/test/java/org/mozilla/javascript/tests/LambdaAccessorSlotTest.java similarity index 99% rename from tests/src/test/java/org/mozilla/javascript/tests/OwnerAwareLambdaSlotTest.java rename to tests/src/test/java/org/mozilla/javascript/tests/LambdaAccessorSlotTest.java index ed075a588b..02f1ca91be 100644 --- a/tests/src/test/java/org/mozilla/javascript/tests/OwnerAwareLambdaSlotTest.java +++ b/tests/src/test/java/org/mozilla/javascript/tests/LambdaAccessorSlotTest.java @@ -2,7 +2,7 @@ import static org.junit.Assert.*; import static org.mozilla.javascript.ScriptableObject.DONTENUM; -import static org.mozilla.javascript.tests.OwnerAwareLambdaSlotTest.StatusHolder.self; +import static org.mozilla.javascript.tests.LambdaAccessorSlotTest.StatusHolder.self; import org.junit.After; import org.junit.Before; @@ -14,7 +14,7 @@ import org.mozilla.javascript.ScriptableObject; import org.mozilla.javascript.Undefined; -public class OwnerAwareLambdaSlotTest { +public class LambdaAccessorSlotTest { private Context cx; private ScriptableObject scope; From b9c528b639b2a8ec153e5e38531850200e277ec4 Mon Sep 17 00:00:00 2001 From: nabacg Date: Sun, 25 Aug 2024 18:30:45 +0100 Subject: [PATCH 06/10] Bringing LambdaAccessorSlot more in line with AccessSlot, making sure PropertyDescriptor sets writable, enumerable and configurable attributes, plus calling validation on those when creating new LambdaAccessorSlot properties --- .../javascript/LambdaAccessorSlot.java | 94 ++++++++++++------- .../mozilla/javascript/LambdaConstructor.java | 10 +- .../mozilla/javascript/ScriptableObject.java | 58 +++++++++++- .../tests/LambdaAccessorSlotTest.java | 76 +++++++++++++-- 4 files changed, 188 insertions(+), 50 deletions(-) diff --git a/rhino/src/main/java/org/mozilla/javascript/LambdaAccessorSlot.java b/rhino/src/main/java/org/mozilla/javascript/LambdaAccessorSlot.java index 422cf67723..abc0e29781 100644 --- a/rhino/src/main/java/org/mozilla/javascript/LambdaAccessorSlot.java +++ b/rhino/src/main/java/org/mozilla/javascript/LambdaAccessorSlot.java @@ -33,55 +33,62 @@ boolean isValueSlot() { return false; } + @Override + boolean isSetterSlot() { + return true; + } + @Override ScriptableObject getPropertyDescriptor(Context cx, Scriptable scope) { ScriptableObject desc = (ScriptableObject) cx.newObject(scope); - if (getter != null) { - if (getterFunction == null) { - this.getterFunction = - new LambdaFunction( - scope, - "get " + super.name, - 0, - (cx1, scope1, thisObj, args) -> getter.apply(thisObj)); + + int attr = getAttributes(); + boolean es6 = cx.getLanguageVersion() >= Context.VERSION_ES6; + if (es6) { + if (getterFunction == null && setterFunction == null) { + desc.defineProperty( + "writable", + (attr & ScriptableObject.READONLY) == 0, + ScriptableObject.EMPTY); } + } else { + desc.setCommonDescriptorProperties(attr, getterFunction == null && setterFunction == null); + } + if (getterFunction != null) { desc.defineProperty("get", this.getterFunction, ScriptableObject.EMPTY); } - if (setter != null) { - if (setterFunction == null) { - this.setterFunction = - new LambdaFunction( - scope, - "set " + super.name, - 1, - (cx1, scope1, thisObj, args) -> { - setter.accept(thisObj, args[0]); - return Undefined.instance; - }); - } + if (setterFunction != null) { desc.defineProperty("set", this.setterFunction, ScriptableObject.EMPTY); + } else if (es6) { + desc.defineProperty("set", Undefined.instance, ScriptableObject.EMPTY); + } + + if (es6) { + desc.defineProperty( + "enumerable", (attr & ScriptableObject.DONTENUM) == 0, ScriptableObject.EMPTY); + desc.defineProperty( + "configurable", + (attr & ScriptableObject.PERMANENT) == 0, + ScriptableObject.EMPTY); } - desc.setCommonDescriptorProperties(getAttributes(), false); return desc; } @Override - public boolean setValue(Object value, Scriptable scope, Scriptable owner, boolean isStrict) { - if (setter != null) { - setter.accept(owner, value); - return true; - } - - if (isStrict) { - // in strict mode - throw ScriptRuntime.typeErrorById("msg.modify.readonly", name); + public boolean setValue(Object value, Scriptable scope, Scriptable start, boolean isThrow) { + if (setter == null) { + if (getter != null) { + throwNoSetterException(start, value); + return true; + } } else { - super.setValue(value, scope, owner, false); + setter.accept(start, value); + return true; } - return true; + return super.setValue(value, start, start, isThrow); } @Override @@ -92,11 +99,30 @@ public Object getValue(Scriptable owner) { return super.getValue(owner); } - public void setGetter(Function getter) { + public void setGetter(Scriptable scope, Function getter) { this.getter = getter; + if (getter != null) { + this.getterFunction = + new LambdaFunction( + scope, + "get " + super.name, + 0, + (cx1, scope1, thisObj, args) -> getter.apply(thisObj)); + } } - public void setSetter(BiConsumer setter) { + public void setSetter(Scriptable scope, BiConsumer setter) { this.setter = setter; + if (setter != null) { + this.setterFunction = + new LambdaFunction( + scope, + "set " + super.name, + 1, + (cx1, scope1, thisObj, args) -> { + setter.accept(thisObj, args[0]); + return Undefined.instance; + }); + } } } diff --git a/rhino/src/main/java/org/mozilla/javascript/LambdaConstructor.java b/rhino/src/main/java/org/mozilla/javascript/LambdaConstructor.java index 866e5a0b8b..d0a8c0e86e 100644 --- a/rhino/src/main/java/org/mozilla/javascript/LambdaConstructor.java +++ b/rhino/src/main/java/org/mozilla/javascript/LambdaConstructor.java @@ -124,18 +124,22 @@ public void definePrototypeProperty(Symbol key, Object value, int attributes) { } public void definePrototypeProperty( - String name, java.util.function.Function getter, int attributes) { + Context cx, + String name, + java.util.function.Function getter, + int attributes) { ScriptableObject proto = getPrototypeScriptable(); - proto.defineProperty(name, getter, null, attributes); + proto.defineProperty(cx, name, getter, null, attributes); } public void definePrototypeProperty( + Context cx, String name, Function getter, BiConsumer setter, int attributes) { ScriptableObject proto = getPrototypeScriptable(); - proto.defineProperty(name, getter, setter, attributes); + proto.defineProperty(cx, name, getter, setter, attributes); } /** diff --git a/rhino/src/main/java/org/mozilla/javascript/ScriptableObject.java b/rhino/src/main/java/org/mozilla/javascript/ScriptableObject.java index 800d38fece..1c7f6baf36 100644 --- a/rhino/src/main/java/org/mozilla/javascript/ScriptableObject.java +++ b/rhino/src/main/java/org/mozilla/javascript/ScriptableObject.java @@ -1711,6 +1711,7 @@ public void defineProperty( * @param attributes the attributes to set on the property */ public void defineProperty( + Context cx, String name, java.util.function.Function getter, BiConsumer setter, @@ -1718,12 +1719,61 @@ public void defineProperty( if (getter == null && setter == null) throw ScriptRuntime.typeError("at least one of {getter, setter} is required"); - LambdaAccessorSlot slot = - slotMap.compute(name, 0, ScriptableObject::ensureLambdaAccessorSlot); + slotMap.compute( + name, + 0, + (id, index, existing) -> + ensureLambdaAccessorSlot( + cx, id, index, existing, getter, setter, attributes)); + } + + private LambdaAccessorSlot createLambdaAccessorSlot( + Object name, + int index, + Slot existing, + java.util.function.Function getter, + BiConsumer setter, + int attributes) { + LambdaAccessorSlot slot; + if (existing == null) { + slot = new LambdaAccessorSlot(name, index); + } else if (existing instanceof LambdaAccessorSlot) { + slot = (LambdaAccessorSlot) existing; + } else { + slot = new LambdaAccessorSlot(existing); + } + + slot.setGetter(this, getter); + slot.setSetter(this, setter); slot.setAttributes(attributes); + return slot; + } + + private LambdaAccessorSlot ensureLambdaAccessorSlot( + Context cx, + Object name, + int index, + Slot existing, + java.util.function.Function getter, + BiConsumer setter, + int attributes) { + var newSlot = createLambdaAccessorSlot(name, index, existing, getter, setter, attributes); + var newDesc = newSlot.getPropertyDescriptor(cx, this); + checkPropertyDefinition(newDesc); - slot.setGetter(getter); - slot.setSetter(setter); + if (existing == null) { + checkPropertyChange(name, null, newDesc); + return newSlot; + } else if (existing instanceof LambdaAccessorSlot) { + var slot = (LambdaAccessorSlot) existing; + var existingDesc = slot.getPropertyDescriptor(cx, this); + checkPropertyChange(name, existingDesc, newDesc); + return newSlot; + } else { + var existingDesc = existing.getPropertyDescriptor(cx, this); + checkPropertyChange(name, existingDesc, newDesc); + return newSlot; + } } protected void checkPropertyDefinition(ScriptableObject desc) { diff --git a/tests/src/test/java/org/mozilla/javascript/tests/LambdaAccessorSlotTest.java b/tests/src/test/java/org/mozilla/javascript/tests/LambdaAccessorSlotTest.java index 02f1ca91be..6f3ccf7fd5 100644 --- a/tests/src/test/java/org/mozilla/javascript/tests/LambdaAccessorSlotTest.java +++ b/tests/src/test/java/org/mozilla/javascript/tests/LambdaAccessorSlotTest.java @@ -1,7 +1,7 @@ package org.mozilla.javascript.tests; import static org.junit.Assert.*; -import static org.mozilla.javascript.ScriptableObject.DONTENUM; +import static org.mozilla.javascript.ScriptableObject.*; import static org.mozilla.javascript.tests.LambdaAccessorSlotTest.StatusHolder.self; import org.junit.After; @@ -10,6 +10,7 @@ import org.mozilla.javascript.Context; import org.mozilla.javascript.EcmaError; import org.mozilla.javascript.LambdaConstructor; +import org.mozilla.javascript.ScriptRuntime; import org.mozilla.javascript.Scriptable; import org.mozilla.javascript.ScriptableObject; import org.mozilla.javascript.Undefined; @@ -33,6 +34,7 @@ public void tearDown() throws Exception { public void testGetterProperty() { StatusHolder.init(scope) .definePrototypeProperty( + cx, "status", (thisObj) -> self(thisObj).getStatus(), (thisObj, value) -> self(thisObj).setStatus(value), @@ -51,7 +53,8 @@ public void testThrowIfNeitherGetterOrSetterAreDefined() { EcmaError.class, () -> StatusHolder.init(scope) - .definePrototypeProperty("status", null, null, DONTENUM)); + .definePrototypeProperty( + cx, "status", null, null, DONTENUM)); assertTrue(error.toString().contains("at least one of {getter, setter} is required")); } @@ -59,6 +62,7 @@ public void testThrowIfNeitherGetterOrSetterAreDefined() { public void testCanUpdateValueUsingSetter() { StatusHolder.init(scope) .definePrototypeProperty( + cx, "status", (thisObj) -> self(thisObj).getStatus(), (thisObj, value) -> self(thisObj).setStatus(value), @@ -79,7 +83,7 @@ public void testCanUpdateValueUsingSetter() { public void testOnlyGetterCanBeAccessed() { StatusHolder.init(scope) .definePrototypeProperty( - "status", (thisObj) -> self(thisObj).getStatus(), DONTENUM); + cx, "status", (thisObj) -> self(thisObj).getStatus(), DONTENUM); Object getterResult = cx.evaluateString(scope, "new StatusHolder('OK').status", "source", 1, null); @@ -97,7 +101,7 @@ public void testOnlyGetterCanBeAccessed() { public void testWhenNoSetterDefined_InStrictMode_WillThrowException() { StatusHolder.init(scope) .definePrototypeProperty( - "status", (thisObj) -> self(thisObj).getStatus(), DONTENUM); + cx, "status", (thisObj) -> self(thisObj).getStatus(), DONTENUM); Object getterResult = cx.evaluateString( scope, "s = new StatusHolder('Constant'); s.status", "source", 1, null); @@ -113,14 +117,19 @@ public void testWhenNoSetterDefined_InStrictMode_WillThrowException() { "source", 1, null)); - assertTrue(error.toString().contains("Cannot modify readonly property: status.")); + String expectedError = ScriptRuntime.getMessageById( + "msg.set.prop.no.setter", "[StatusHolder].status", "DONE"); + assertTrue( + error.toString() + .contains( + expectedError)); } @Test public void testWhenNoSetterDefined_InNormalMode_NoErrorButValueIsNotChanged() { StatusHolder.init(scope) .definePrototypeProperty( - "status", (thisObj) -> self(thisObj).getStatus(), DONTENUM); + cx, "status", (thisObj) -> self(thisObj).getStatus(), DONTENUM); Object getterResult = cx.evaluateString( @@ -140,6 +149,7 @@ public void testWhenNoSetterDefined_InNormalMode_NoErrorButValueIsNotChanged() { public void testSetterOnly_WillModifyUnderlyingValue() { StatusHolder.init(scope) .definePrototypeProperty( + cx, "status", null, (thisObj, value) -> self(thisObj).setStatus(value), @@ -155,13 +165,14 @@ public void testSetterOnly_WillModifyUnderlyingValue() { assertEquals("NewStatus: DONE", statusHolder.getStatus()); } + // using getOwnPropertyDescriptor to access property @Test public void testGetterUsing_getOwnPropertyDescriptor() { StatusHolder.init(scope) .definePrototypeProperty( - "status", (thisObj) -> self(thisObj).getStatus(), DONTENUM); + cx, "status", (thisObj) -> self(thisObj).getStatus(), DONTENUM); Object result = cx.evaluateString( @@ -179,6 +190,7 @@ public void testGetterUsing_getOwnPropertyDescriptor() { public void testSetterOnlyUsing_getOwnPropertyDescriptor() { StatusHolder.init(scope) .definePrototypeProperty( + cx, "status", null, (thisObj, value) -> self(thisObj).setStatus(value), @@ -202,6 +214,7 @@ public void testSetterOnlyUsing_getOwnPropertyDescriptor() { public void testSetValueUsing_getOwnPropertyDescriptor() { StatusHolder.init(scope) .definePrototypeProperty( + cx, "status", (thisObj) -> self(thisObj).getStatus(), (thisObj, value) -> self(thisObj).setStatus(value), @@ -224,6 +237,7 @@ public void testSetValueUsing_getOwnPropertyDescriptor() { public void testSetterOnlyUsing_getOwnPropertyDescriptor_ErrorOnGet() { StatusHolder.init(scope) .definePrototypeProperty( + cx, "status", null, (thisObj, value) -> self(thisObj).setStatus(value), @@ -244,10 +258,54 @@ public void testSetterOnlyUsing_getOwnPropertyDescriptor_ErrorOnGet() { assertTrue(error.toString().contains("Cannot call method \"call\" of undefined")); } + @Test + public void testRedefineExistingProperty_ChangingConfigurableAttr_ShouldFailValidation() { + var sh = new StatusHolder("PENDING"); + ScriptableObject existingDesc = (ScriptableObject) cx.newObject(scope); + + // + existingDesc.defineProperty("configurable", false, ScriptableObject.EMPTY); + + sh.defineOwnProperty(cx, "status", existingDesc); + + var error = + assertThrows( + EcmaError.class, + () -> + sh.defineProperty(cx, "status", + (thisObj) -> self(thisObj).getStatus(), + (thisObj, value) -> self(thisObj).setStatus(value), + DONTENUM)); + assertTrue(error.toString().contains(ScriptRuntime.getMessageById("msg.change.configurable.false.to.true", "status"))); + } + + @Test + public void testRedefineExistingProperty_ModifyingNotConfigurableProperty_ShouldFailValidation() { + var sh = new StatusHolder("PENDING"); + ScriptableObject existingDesc = (ScriptableObject) cx.newObject(scope); + + // + existingDesc.defineProperty("configurable", false, ScriptableObject.EMPTY); + existingDesc.defineProperty("enumerable", true, ScriptableObject.EMPTY); + + sh.defineOwnProperty(cx, "status", existingDesc); + + var error = + assertThrows( + EcmaError.class, + () -> + sh.defineProperty(cx, "status", + (thisObj) -> self(thisObj).getStatus(), + (thisObj, value) -> self(thisObj).setStatus(value), + DONTENUM | PERMANENT)); // making new property configurable == false and enumerable == false + assertTrue(error.toString().contains(ScriptRuntime.getMessageById("msg.change.enumerable.with.configurable.false", "status"))); + } + @Test public void testSetterOnlyUsing_getOwnPropertyDescriptor_InStrictMode_ErrorOnGet() { StatusHolder.init(scope) .definePrototypeProperty( + cx, "status", null, (thisObj, value) -> self(thisObj).setStatus(value), @@ -273,7 +331,7 @@ public void testSetterOnlyUsing_getOwnPropertyDescriptor_InStrictMode_ErrorOnGet public void testGetterOnlyUsing_getOwnPropertyDescriptor_ErrorOnSet() { StatusHolder.init(scope) .definePrototypeProperty( - "status", (thisObj) -> self(thisObj).getStatus(), DONTENUM); + cx, "status", (thisObj) -> self(thisObj).getStatus(), DONTENUM); var error = assertThrows( @@ -295,7 +353,7 @@ public void testGetterOnlyUsing_getOwnPropertyDescriptor_ErrorOnSet() { public void testGetterOnlyUsing_getOwnPropertyDescriptor_InStrictMode_ErrorOnSet() { StatusHolder.init(scope) .definePrototypeProperty( - "status", (thisObj) -> self(thisObj).getStatus(), DONTENUM); + cx, "status", (thisObj) -> self(thisObj).getStatus(), DONTENUM); var error = assertThrows( From 0d53988694ced8df3261626d55af7177832fe466 Mon Sep 17 00:00:00 2001 From: nabacg Date: Sun, 25 Aug 2024 18:42:07 +0100 Subject: [PATCH 07/10] spotlessApply --- .../javascript/LambdaAccessorSlot.java | 3 +- .../tests/LambdaAccessorSlotTest.java | 47 ++++++++++++------- 2 files changed, 32 insertions(+), 18 deletions(-) diff --git a/rhino/src/main/java/org/mozilla/javascript/LambdaAccessorSlot.java b/rhino/src/main/java/org/mozilla/javascript/LambdaAccessorSlot.java index abc0e29781..18538473c5 100644 --- a/rhino/src/main/java/org/mozilla/javascript/LambdaAccessorSlot.java +++ b/rhino/src/main/java/org/mozilla/javascript/LambdaAccessorSlot.java @@ -52,7 +52,8 @@ ScriptableObject getPropertyDescriptor(Context cx, Scriptable scope) { ScriptableObject.EMPTY); } } else { - desc.setCommonDescriptorProperties(attr, getterFunction == null && setterFunction == null); + desc.setCommonDescriptorProperties( + attr, getterFunction == null && setterFunction == null); } if (getterFunction != null) { diff --git a/tests/src/test/java/org/mozilla/javascript/tests/LambdaAccessorSlotTest.java b/tests/src/test/java/org/mozilla/javascript/tests/LambdaAccessorSlotTest.java index 6f3ccf7fd5..29f505958c 100644 --- a/tests/src/test/java/org/mozilla/javascript/tests/LambdaAccessorSlotTest.java +++ b/tests/src/test/java/org/mozilla/javascript/tests/LambdaAccessorSlotTest.java @@ -117,12 +117,10 @@ public void testWhenNoSetterDefined_InStrictMode_WillThrowException() { "source", 1, null)); - String expectedError = ScriptRuntime.getMessageById( - "msg.set.prop.no.setter", "[StatusHolder].status", "DONE"); - assertTrue( - error.toString() - .contains( - expectedError)); + String expectedError = + ScriptRuntime.getMessageById( + "msg.set.prop.no.setter", "[StatusHolder].status", "DONE"); + assertTrue(error.toString().contains(expectedError)); } @Test @@ -165,7 +163,6 @@ public void testSetterOnly_WillModifyUnderlyingValue() { assertEquals("NewStatus: DONE", statusHolder.getStatus()); } - // using getOwnPropertyDescriptor to access property @Test @@ -260,7 +257,7 @@ public void testSetterOnlyUsing_getOwnPropertyDescriptor_ErrorOnGet() { @Test public void testRedefineExistingProperty_ChangingConfigurableAttr_ShouldFailValidation() { - var sh = new StatusHolder("PENDING"); + var sh = new StatusHolder("PENDING"); ScriptableObject existingDesc = (ScriptableObject) cx.newObject(scope); // @@ -272,15 +269,22 @@ public void testRedefineExistingProperty_ChangingConfigurableAttr_ShouldFailVali assertThrows( EcmaError.class, () -> - sh.defineProperty(cx, "status", - (thisObj) -> self(thisObj).getStatus(), - (thisObj, value) -> self(thisObj).setStatus(value), - DONTENUM)); - assertTrue(error.toString().contains(ScriptRuntime.getMessageById("msg.change.configurable.false.to.true", "status"))); + sh.defineProperty( + cx, + "status", + (thisObj) -> self(thisObj).getStatus(), + (thisObj, value) -> self(thisObj).setStatus(value), + DONTENUM)); + assertTrue( + error.toString() + .contains( + ScriptRuntime.getMessageById( + "msg.change.configurable.false.to.true", "status"))); } @Test - public void testRedefineExistingProperty_ModifyingNotConfigurableProperty_ShouldFailValidation() { + public void + testRedefineExistingProperty_ModifyingNotConfigurableProperty_ShouldFailValidation() { var sh = new StatusHolder("PENDING"); ScriptableObject existingDesc = (ScriptableObject) cx.newObject(scope); @@ -294,11 +298,20 @@ public void testRedefineExistingProperty_ModifyingNotConfigurableProperty_Should assertThrows( EcmaError.class, () -> - sh.defineProperty(cx, "status", + sh.defineProperty( + cx, + "status", (thisObj) -> self(thisObj).getStatus(), (thisObj, value) -> self(thisObj).setStatus(value), - DONTENUM | PERMANENT)); // making new property configurable == false and enumerable == false - assertTrue(error.toString().contains(ScriptRuntime.getMessageById("msg.change.enumerable.with.configurable.false", "status"))); + // making new property configurable: false and enumerable: + // false + DONTENUM | PERMANENT)); + assertTrue( + error.toString() + .contains( + ScriptRuntime.getMessageById( + "msg.change.enumerable.with.configurable.false", + "status"))); } @Test From 7369f3fcd16d7925b9ca0d43e96100b1708cbdff Mon Sep 17 00:00:00 2001 From: nabacg Date: Tue, 27 Aug 2024 09:29:58 +0100 Subject: [PATCH 08/10] removing no longer used ensureLambdaAccessorSlot overload --- .../mozilla/javascript/ScriptableObject.java | 53 ++++++++----------- 1 file changed, 21 insertions(+), 32 deletions(-) diff --git a/rhino/src/main/java/org/mozilla/javascript/ScriptableObject.java b/rhino/src/main/java/org/mozilla/javascript/ScriptableObject.java index 1c7f6baf36..cddb2ca8b5 100644 --- a/rhino/src/main/java/org/mozilla/javascript/ScriptableObject.java +++ b/rhino/src/main/java/org/mozilla/javascript/ScriptableObject.java @@ -1749,33 +1749,6 @@ private LambdaAccessorSlot createLambdaAccessorSlot( return slot; } - private LambdaAccessorSlot ensureLambdaAccessorSlot( - Context cx, - Object name, - int index, - Slot existing, - java.util.function.Function getter, - BiConsumer setter, - int attributes) { - var newSlot = createLambdaAccessorSlot(name, index, existing, getter, setter, attributes); - var newDesc = newSlot.getPropertyDescriptor(cx, this); - checkPropertyDefinition(newDesc); - - if (existing == null) { - checkPropertyChange(name, null, newDesc); - return newSlot; - } else if (existing instanceof LambdaAccessorSlot) { - var slot = (LambdaAccessorSlot) existing; - var existingDesc = slot.getPropertyDescriptor(cx, this); - checkPropertyChange(name, existingDesc, newDesc); - return newSlot; - } else { - var existingDesc = existing.getPropertyDescriptor(cx, this); - checkPropertyChange(name, existingDesc, newDesc); - return newSlot; - } - } - protected void checkPropertyDefinition(ScriptableObject desc) { Object getter = getProperty(desc, "get"); if (getter != NOT_FOUND && getter != Undefined.instance && !(getter instanceof Callable)) { @@ -2781,14 +2754,30 @@ private static LambdaSlot ensureLambdaSlot(Object name, int index, Slot existing } } - private static LambdaAccessorSlot ensureLambdaAccessorSlot( - Object name, int index, Slot existing) { + private LambdaAccessorSlot ensureLambdaAccessorSlot( + Context cx, + Object name, + int index, + Slot existing, + java.util.function.Function getter, + BiConsumer setter, + int attributes) { + var newSlot = createLambdaAccessorSlot(name, index, existing, getter, setter, attributes); + var newDesc = newSlot.getPropertyDescriptor(cx, this); + checkPropertyDefinition(newDesc); + if (existing == null) { - return new LambdaAccessorSlot(name, index); + checkPropertyChange(name, null, newDesc); + return newSlot; } else if (existing instanceof LambdaAccessorSlot) { - return (LambdaAccessorSlot) existing; + var slot = (LambdaAccessorSlot) existing; + var existingDesc = slot.getPropertyDescriptor(cx, this); + checkPropertyChange(name, existingDesc, newDesc); + return newSlot; } else { - return new LambdaAccessorSlot(existing); + var existingDesc = existing.getPropertyDescriptor(cx, this); + checkPropertyChange(name, existingDesc, newDesc); + return newSlot; } } From 323396f7ad7759da04aac12fdd936dba3c519898 Mon Sep 17 00:00:00 2001 From: Grzegorz Caban Date: Wed, 28 Aug 2024 09:30:02 +0100 Subject: [PATCH 09/10] updating outdated comment on defineProperty --- .../main/java/org/mozilla/javascript/ScriptableObject.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/rhino/src/main/java/org/mozilla/javascript/ScriptableObject.java b/rhino/src/main/java/org/mozilla/javascript/ScriptableObject.java index cddb2ca8b5..a01c62674f 100644 --- a/rhino/src/main/java/org/mozilla/javascript/ScriptableObject.java +++ b/rhino/src/main/java/org/mozilla/javascript/ScriptableObject.java @@ -1698,9 +1698,8 @@ public void defineProperty( * need to have access to target object instance, this allows for defining properties on * LambdaConstructor prototype providing getter and setter logic with java instance methods. If * a property with the same name already exists, then it will be replaced. This property will - * appear to the JavaScript user exactly like any other property -- unlike Function properties - * and those based on reflection, the property descriptor will only reflect the value as defined - * by this function. + * appear to the JavaScript user exactly like descriptor with a getter and setter, just as if + * they had been defined in JavaScript using Object.defineOwnProperty. * * @param name the name of the property * @param getter a function that given Scriptable `this` returns the value of the property. If From a48d1c0e97456d873d861362b02b5e9a15af1020 Mon Sep 17 00:00:00 2001 From: Grzegorz Caban Date: Fri, 30 Aug 2024 09:44:36 +0100 Subject: [PATCH 10/10] refactoring tests to use Utils.runWithAllOptimizationLevels --- .../tests/LambdaAccessorSlotTest.java | 707 ++++++++++-------- 1 file changed, 402 insertions(+), 305 deletions(-) diff --git a/tests/src/test/java/org/mozilla/javascript/tests/LambdaAccessorSlotTest.java b/tests/src/test/java/org/mozilla/javascript/tests/LambdaAccessorSlotTest.java index 29f505958c..92b696a04a 100644 --- a/tests/src/test/java/org/mozilla/javascript/tests/LambdaAccessorSlotTest.java +++ b/tests/src/test/java/org/mozilla/javascript/tests/LambdaAccessorSlotTest.java @@ -4,10 +4,7 @@ import static org.mozilla.javascript.ScriptableObject.*; import static org.mozilla.javascript.tests.LambdaAccessorSlotTest.StatusHolder.self; -import org.junit.After; -import org.junit.Before; import org.junit.Test; -import org.mozilla.javascript.Context; import org.mozilla.javascript.EcmaError; import org.mozilla.javascript.LambdaConstructor; import org.mozilla.javascript.ScriptRuntime; @@ -16,373 +13,473 @@ import org.mozilla.javascript.Undefined; public class LambdaAccessorSlotTest { - private Context cx; - private ScriptableObject scope; - - @Before - public void setUp() throws Exception { - cx = Context.enter(); - scope = cx.initStandardObjects(); - } - - @After - public void tearDown() throws Exception { - Context.exit(); - } - @Test public void testGetterProperty() { - StatusHolder.init(scope) - .definePrototypeProperty( - cx, - "status", - (thisObj) -> self(thisObj).getStatus(), - (thisObj, value) -> self(thisObj).setStatus(value), - DONTENUM); - - Object getterResult = - cx.evaluateString( - scope, "s = new StatusHolder('InProgress'); s.status", "source", 1, null); - assertEquals("InProgress", getterResult); + Utils.runWithAllOptimizationLevels( + cx -> { + Scriptable scope = cx.initStandardObjects(); + StatusHolder.init(scope) + .definePrototypeProperty( + cx, + "status", + (thisObj) -> self(thisObj).getStatus(), + (thisObj, value) -> self(thisObj).setStatus(value), + DONTENUM); + + Object getterResult = + cx.evaluateString( + scope, + "s = new StatusHolder('InProgress'); s.status", + "source", + 1, + null); + assertEquals("InProgress", getterResult); + return null; + }); } @Test public void testThrowIfNeitherGetterOrSetterAreDefined() { - var error = - assertThrows( - EcmaError.class, - () -> - StatusHolder.init(scope) - .definePrototypeProperty( - cx, "status", null, null, DONTENUM)); - assertTrue(error.toString().contains("at least one of {getter, setter} is required")); + Utils.runWithAllOptimizationLevels( + cx -> { + Scriptable scope = cx.initStandardObjects(); + var error = + assertThrows( + EcmaError.class, + () -> + StatusHolder.init(scope) + .definePrototypeProperty( + cx, "status", null, null, DONTENUM)); + assertTrue( + error.toString() + .contains("at least one of {getter, setter} is required")); + return null; + }); } @Test public void testCanUpdateValueUsingSetter() { - StatusHolder.init(scope) - .definePrototypeProperty( - cx, - "status", - (thisObj) -> self(thisObj).getStatus(), - (thisObj, value) -> self(thisObj).setStatus(value), - DONTENUM); - - Object getterResult = - cx.evaluateString( - scope, "s = new StatusHolder('InProgress'); s.status", "source", 1, null); - assertEquals("InProgress", getterResult); - - Object setResult = cx.evaluateString(scope, "s.status = 'DONE';", "source", 1, null); - - Object newStatus = cx.evaluateString(scope, "s.status", "source", 1, null); - assertEquals("NewStatus: DONE", newStatus); + Utils.runWithAllOptimizationLevels( + cx -> { + Scriptable scope = cx.initStandardObjects(); + StatusHolder.init(scope) + .definePrototypeProperty( + cx, + "status", + (thisObj) -> self(thisObj).getStatus(), + (thisObj, value) -> self(thisObj).setStatus(value), + DONTENUM); + + Object getterResult = + cx.evaluateString( + scope, + "s = new StatusHolder('InProgress'); s.status", + "source", + 1, + null); + assertEquals("InProgress", getterResult); + + Object setResult = + cx.evaluateString(scope, "s.status = 'DONE';", "source", 1, null); + + Object newStatus = cx.evaluateString(scope, "s.status", "source", 1, null); + assertEquals("NewStatus: DONE", newStatus); + return null; + }); } @Test public void testOnlyGetterCanBeAccessed() { - StatusHolder.init(scope) - .definePrototypeProperty( - cx, "status", (thisObj) -> self(thisObj).getStatus(), DONTENUM); - - Object getterResult = - cx.evaluateString(scope, "new StatusHolder('OK').status", "source", 1, null); - assertEquals("OK", getterResult); - - Object hiddenFieldResult = - cx.evaluateString(scope, "new StatusHolder('OK').hiddenStatus", "source", 1, null); - assertEquals( - "fields not explicitly defined as properties should return undefined", - Undefined.instance, - hiddenFieldResult); + Utils.runWithAllOptimizationLevels( + cx -> { + Scriptable scope = cx.initStandardObjects(); + StatusHolder.init(scope) + .definePrototypeProperty( + cx, "status", (thisObj) -> self(thisObj).getStatus(), DONTENUM); + + Object getterResult = + cx.evaluateString( + scope, "new StatusHolder('OK').status", "source", 1, null); + assertEquals("OK", getterResult); + + Object hiddenFieldResult = + cx.evaluateString( + scope, + "new StatusHolder('OK').hiddenStatus", + "source", + 1, + null); + assertEquals( + "fields not explicitly defined as properties should return undefined", + Undefined.instance, + hiddenFieldResult); + return null; + }); } @Test public void testWhenNoSetterDefined_InStrictMode_WillThrowException() { - StatusHolder.init(scope) - .definePrototypeProperty( - cx, "status", (thisObj) -> self(thisObj).getStatus(), DONTENUM); - Object getterResult = - cx.evaluateString( - scope, "s = new StatusHolder('Constant'); s.status", "source", 1, null); - assertEquals("Constant", getterResult); - - var error = - assertThrows( - EcmaError.class, - () -> - cx.evaluateString( - scope, - "\"use strict\"; s.status = 'DONE'; s.status", - "source", - 1, - null)); - String expectedError = - ScriptRuntime.getMessageById( - "msg.set.prop.no.setter", "[StatusHolder].status", "DONE"); - assertTrue(error.toString().contains(expectedError)); + Utils.runWithAllOptimizationLevels( + cx -> { + Scriptable scope = cx.initStandardObjects(); + StatusHolder.init(scope) + .definePrototypeProperty( + cx, "status", (thisObj) -> self(thisObj).getStatus(), DONTENUM); + Object getterResult = + cx.evaluateString( + scope, + "s = new StatusHolder('Constant'); s.status", + "source", + 1, + null); + assertEquals("Constant", getterResult); + + var error = + assertThrows( + EcmaError.class, + () -> + cx.evaluateString( + scope, + "\"use strict\"; s.status = 'DONE'; s.status", + "source", + 1, + null)); + String expectedError = + ScriptRuntime.getMessageById( + "msg.set.prop.no.setter", "[StatusHolder].status", "DONE"); + assertTrue(error.toString().contains(expectedError)); + return null; + }); } @Test public void testWhenNoSetterDefined_InNormalMode_NoErrorButValueIsNotChanged() { - StatusHolder.init(scope) - .definePrototypeProperty( - cx, "status", (thisObj) -> self(thisObj).getStatus(), DONTENUM); - - Object getterResult = - cx.evaluateString( - scope, "s = new StatusHolder('Constant'); s.status", "source", 1, null); - assertEquals("Constant", getterResult); - - Object setResult = - cx.evaluateString(scope, "s.status = 'DONE'; s.status", "source", 1, null); - assertEquals("status won't be changed", "Constant", setResult); - - Object shObj = cx.evaluateString(scope, "s", "source", 1, null); - var statusHolder = (StatusHolder) shObj; - assertEquals("Constant", statusHolder.getStatus()); + Utils.runWithAllOptimizationLevels( + cx -> { + Scriptable scope = cx.initStandardObjects(); + StatusHolder.init(scope) + .definePrototypeProperty( + cx, "status", (thisObj) -> self(thisObj).getStatus(), DONTENUM); + + Object getterResult = + cx.evaluateString( + scope, + "s = new StatusHolder('Constant'); s.status", + "source", + 1, + null); + assertEquals("Constant", getterResult); + + Object setResult = + cx.evaluateString( + scope, "s.status = 'DONE'; s.status", "source", 1, null); + assertEquals("status won't be changed", "Constant", setResult); + + Object shObj = cx.evaluateString(scope, "s", "source", 1, null); + var statusHolder = (StatusHolder) shObj; + assertEquals("Constant", statusHolder.getStatus()); + return null; + }); } @Test public void testSetterOnly_WillModifyUnderlyingValue() { - StatusHolder.init(scope) - .definePrototypeProperty( - cx, - "status", - null, - (thisObj, value) -> self(thisObj).setStatus(value), - DONTENUM); - cx.evaluateString(scope, "s = new StatusHolder('Constant')", "source", 1, null); - - cx.evaluateString(scope, "s.status = 'DONE'; s.status", "source", 1, null); - - Object newStatus = cx.evaluateString(scope, "s.status", "source", 1, null); - assertEquals(null, newStatus); - Object shObj = cx.evaluateString(scope, "s", "source", 1, null); - var statusHolder = (StatusHolder) shObj; - assertEquals("NewStatus: DONE", statusHolder.getStatus()); + Utils.runWithAllOptimizationLevels( + cx -> { + Scriptable scope = cx.initStandardObjects(); + StatusHolder.init(scope) + .definePrototypeProperty( + cx, + "status", + null, + (thisObj, value) -> self(thisObj).setStatus(value), + DONTENUM); + cx.evaluateString(scope, "s = new StatusHolder('Constant')", "source", 1, null); + + cx.evaluateString(scope, "s.status = 'DONE'; s.status", "source", 1, null); + + Object newStatus = cx.evaluateString(scope, "s.status", "source", 1, null); + assertEquals(null, newStatus); + Object shObj = cx.evaluateString(scope, "s", "source", 1, null); + var statusHolder = (StatusHolder) shObj; + assertEquals("NewStatus: DONE", statusHolder.getStatus()); + return null; + }); } // using getOwnPropertyDescriptor to access property @Test public void testGetterUsing_getOwnPropertyDescriptor() { - StatusHolder.init(scope) - .definePrototypeProperty( - cx, "status", (thisObj) -> self(thisObj).getStatus(), DONTENUM); - - Object result = - cx.evaluateString( - scope, - "s = new StatusHolder('InProgress');" - + "f = Object.getOwnPropertyDescriptor(Object.getPrototypeOf(s), 'status');" - + "f.get.call(s)", - "source", - 1, - null); - assertEquals("InProgress", result); + Utils.runWithAllOptimizationLevels( + cx -> { + Scriptable scope = cx.initStandardObjects(); + StatusHolder.init(scope) + .definePrototypeProperty( + cx, "status", (thisObj) -> self(thisObj).getStatus(), DONTENUM); + + Object result = + cx.evaluateString( + scope, + "s = new StatusHolder('InProgress');" + + "f = Object.getOwnPropertyDescriptor(Object.getPrototypeOf(s), 'status');" + + "f.get.call(s)", + "source", + 1, + null); + assertEquals("InProgress", result); + return null; + }); } @Test public void testSetterOnlyUsing_getOwnPropertyDescriptor() { - StatusHolder.init(scope) - .definePrototypeProperty( - cx, - "status", - null, - (thisObj, value) -> self(thisObj).setStatus(value), - DONTENUM); - - Object shObj = - cx.evaluateString( - scope, - "s = new StatusHolder('InProgress');" - + "f = Object.getOwnPropertyDescriptor(Object.getPrototypeOf(s), 'status');" - + "f.set.call(s, 'DONE');" - + "s", - "source", - 1, - null); - var statusHolder = (StatusHolder) shObj; - assertEquals("NewStatus: DONE", statusHolder.getStatus()); + Utils.runWithAllOptimizationLevels( + cx -> { + Scriptable scope = cx.initStandardObjects(); + StatusHolder.init(scope) + .definePrototypeProperty( + cx, + "status", + null, + (thisObj, value) -> self(thisObj).setStatus(value), + DONTENUM); + + Object shObj = + cx.evaluateString( + scope, + "s = new StatusHolder('InProgress');" + + "f = Object.getOwnPropertyDescriptor(Object.getPrototypeOf(s), 'status');" + + "f.set.call(s, 'DONE');" + + "s", + "source", + 1, + null); + var statusHolder = (StatusHolder) shObj; + assertEquals("NewStatus: DONE", statusHolder.getStatus()); + return null; + }); } @Test public void testSetValueUsing_getOwnPropertyDescriptor() { - StatusHolder.init(scope) - .definePrototypeProperty( - cx, - "status", - (thisObj) -> self(thisObj).getStatus(), - (thisObj, value) -> self(thisObj).setStatus(value), - DONTENUM); - - Object result = - cx.evaluateString( - scope, - "s = new StatusHolder('InProgress');" - + "f = Object.getOwnPropertyDescriptor(Object.getPrototypeOf(s), 'status');" - + "f.set.call(s, 'DONE');" - + "s.status", - "source", - 1, - null); - assertEquals("Status with prefix", "NewStatus: DONE", result); + Utils.runWithAllOptimizationLevels( + cx -> { + Scriptable scope = cx.initStandardObjects(); + StatusHolder.init(scope) + .definePrototypeProperty( + cx, + "status", + (thisObj) -> self(thisObj).getStatus(), + (thisObj, value) -> self(thisObj).setStatus(value), + DONTENUM); + + Object result = + cx.evaluateString( + scope, + "s = new StatusHolder('InProgress');" + + "f = Object.getOwnPropertyDescriptor(Object.getPrototypeOf(s), 'status');" + + "f.set.call(s, 'DONE');" + + "s.status", + "source", + 1, + null); + assertEquals("Status with prefix", "NewStatus: DONE", result); + return null; + }); } @Test public void testSetterOnlyUsing_getOwnPropertyDescriptor_ErrorOnGet() { - StatusHolder.init(scope) - .definePrototypeProperty( - cx, - "status", - null, - (thisObj, value) -> self(thisObj).setStatus(value), - DONTENUM); - - var error = - assertThrows( - EcmaError.class, - () -> - cx.evaluateString( - scope, - "var s = new StatusHolder('InProgress');" - + "var f = Object.getOwnPropertyDescriptor(Object.getPrototypeOf(s), 'status');" - + "f.get.call(s)", - "source", - 1, - null)); - assertTrue(error.toString().contains("Cannot call method \"call\" of undefined")); + Utils.runWithAllOptimizationLevels( + cx -> { + Scriptable scope = cx.initStandardObjects(); + StatusHolder.init(scope) + .definePrototypeProperty( + cx, + "status", + null, + (thisObj, value) -> self(thisObj).setStatus(value), + DONTENUM); + + var error = + assertThrows( + EcmaError.class, + () -> + cx.evaluateString( + scope, + "var s = new StatusHolder('InProgress');" + + "var f = Object.getOwnPropertyDescriptor(Object.getPrototypeOf(s), 'status');" + + "f.get.call(s)", + "source", + 1, + null)); + assertTrue( + error.toString().contains("Cannot call method \"call\" of undefined")); + return null; + }); } @Test public void testRedefineExistingProperty_ChangingConfigurableAttr_ShouldFailValidation() { - var sh = new StatusHolder("PENDING"); - ScriptableObject existingDesc = (ScriptableObject) cx.newObject(scope); - - // - existingDesc.defineProperty("configurable", false, ScriptableObject.EMPTY); - - sh.defineOwnProperty(cx, "status", existingDesc); - - var error = - assertThrows( - EcmaError.class, - () -> - sh.defineProperty( - cx, - "status", - (thisObj) -> self(thisObj).getStatus(), - (thisObj, value) -> self(thisObj).setStatus(value), - DONTENUM)); - assertTrue( - error.toString() - .contains( - ScriptRuntime.getMessageById( - "msg.change.configurable.false.to.true", "status"))); + Utils.runWithAllOptimizationLevels( + cx -> { + Scriptable scope = cx.initStandardObjects(); + var sh = new StatusHolder("PENDING"); + ScriptableObject existingDesc = (ScriptableObject) cx.newObject(scope); + + // + existingDesc.defineProperty("configurable", false, ScriptableObject.EMPTY); + + sh.defineOwnProperty(cx, "status", existingDesc); + + var error = + assertThrows( + EcmaError.class, + () -> + sh.defineProperty( + cx, + "status", + (thisObj) -> self(thisObj).getStatus(), + (thisObj, value) -> + self(thisObj).setStatus(value), + DONTENUM)); + assertTrue( + error.toString() + .contains( + ScriptRuntime.getMessageById( + "msg.change.configurable.false.to.true", + "status"))); + return null; + }); } @Test public void testRedefineExistingProperty_ModifyingNotConfigurableProperty_ShouldFailValidation() { - var sh = new StatusHolder("PENDING"); - ScriptableObject existingDesc = (ScriptableObject) cx.newObject(scope); - - // - existingDesc.defineProperty("configurable", false, ScriptableObject.EMPTY); - existingDesc.defineProperty("enumerable", true, ScriptableObject.EMPTY); - - sh.defineOwnProperty(cx, "status", existingDesc); - - var error = - assertThrows( - EcmaError.class, - () -> - sh.defineProperty( - cx, - "status", - (thisObj) -> self(thisObj).getStatus(), - (thisObj, value) -> self(thisObj).setStatus(value), - // making new property configurable: false and enumerable: - // false - DONTENUM | PERMANENT)); - assertTrue( - error.toString() - .contains( - ScriptRuntime.getMessageById( - "msg.change.enumerable.with.configurable.false", - "status"))); + Utils.runWithAllOptimizationLevels( + cx -> { + Scriptable scope = cx.initStandardObjects(); + var sh = new StatusHolder("PENDING"); + ScriptableObject existingDesc = (ScriptableObject) cx.newObject(scope); + + // + existingDesc.defineProperty("configurable", false, ScriptableObject.EMPTY); + existingDesc.defineProperty("enumerable", true, ScriptableObject.EMPTY); + + sh.defineOwnProperty(cx, "status", existingDesc); + + var error = + assertThrows( + EcmaError.class, + () -> + sh.defineProperty( + cx, + "status", + (thisObj) -> self(thisObj).getStatus(), + (thisObj, value) -> + self(thisObj).setStatus(value), + // making new property configurable: false and + // enumerable: + // false + DONTENUM | PERMANENT)); + assertTrue( + error.toString() + .contains( + ScriptRuntime.getMessageById( + "msg.change.enumerable.with.configurable.false", + "status"))); + return null; + }); } @Test public void testSetterOnlyUsing_getOwnPropertyDescriptor_InStrictMode_ErrorOnGet() { - StatusHolder.init(scope) - .definePrototypeProperty( - cx, - "status", - null, - (thisObj, value) -> self(thisObj).setStatus(value), - DONTENUM); - - var error = - assertThrows( - EcmaError.class, - () -> - cx.evaluateString( - scope, - "\"use strict\";" - + "var s = new StatusHolder('InProgress');" - + "var f = Object.getOwnPropertyDescriptor(Object.getPrototypeOf(s), 'status');" - + "f.get.call(s)", - "source", - 1, - null)); - assertTrue(error.toString().contains("Cannot call method \"call\" of undefined")); + Utils.runWithAllOptimizationLevels( + cx -> { + Scriptable scope = cx.initStandardObjects(); + StatusHolder.init(scope) + .definePrototypeProperty( + cx, + "status", + null, + (thisObj, value) -> self(thisObj).setStatus(value), + DONTENUM); + + var error = + assertThrows( + EcmaError.class, + () -> + cx.evaluateString( + scope, + "\"use strict\";" + + "var s = new StatusHolder('InProgress');" + + "var f = Object.getOwnPropertyDescriptor(Object.getPrototypeOf(s), 'status');" + + "f.get.call(s)", + "source", + 1, + null)); + assertTrue( + error.toString().contains("Cannot call method \"call\" of undefined")); + return null; + }); } @Test public void testGetterOnlyUsing_getOwnPropertyDescriptor_ErrorOnSet() { - StatusHolder.init(scope) - .definePrototypeProperty( - cx, "status", (thisObj) -> self(thisObj).getStatus(), DONTENUM); - - var error = - assertThrows( - EcmaError.class, - () -> - cx.evaluateString( - scope, - "var s = new StatusHolder('InProgress');" - + "var f = Object.getOwnPropertyDescriptor(Object.getPrototypeOf(s), 'status');" - + "f.set.call(s, 'DONE');" - + "s.status", - "source", - 1, - null)); - assertTrue(error.toString().contains("Cannot call method \"call\" of undefined")); + Utils.runWithAllOptimizationLevels( + cx -> { + Scriptable scope = cx.initStandardObjects(); + StatusHolder.init(scope) + .definePrototypeProperty( + cx, "status", (thisObj) -> self(thisObj).getStatus(), DONTENUM); + + var error = + assertThrows( + EcmaError.class, + () -> + cx.evaluateString( + scope, + "var s = new StatusHolder('InProgress');" + + "var f = Object.getOwnPropertyDescriptor(Object.getPrototypeOf(s), 'status');" + + "f.set.call(s, 'DONE');" + + "s.status", + "source", + 1, + null)); + assertTrue( + error.toString().contains("Cannot call method \"call\" of undefined")); + return null; + }); } @Test public void testGetterOnlyUsing_getOwnPropertyDescriptor_InStrictMode_ErrorOnSet() { - StatusHolder.init(scope) - .definePrototypeProperty( - cx, "status", (thisObj) -> self(thisObj).getStatus(), DONTENUM); - - var error = - assertThrows( - EcmaError.class, - () -> - cx.evaluateString( - scope, - "\"use strict\";" - + "var s = new StatusHolder('InProgress');" - + "var f = Object.getOwnPropertyDescriptor(Object.getPrototypeOf(s), 'status');" - + "f.set.call(s, 'DONE');" - + "s.status", - "source", - 1, - null)); - assertTrue(error.toString().contains("Cannot call method \"call\" of undefined")); + Utils.runWithAllOptimizationLevels( + cx -> { + Scriptable scope = cx.initStandardObjects(); + StatusHolder.init(scope) + .definePrototypeProperty( + cx, "status", (thisObj) -> self(thisObj).getStatus(), DONTENUM); + + var error = + assertThrows( + EcmaError.class, + () -> + cx.evaluateString( + scope, + "\"use strict\";" + + "var s = new StatusHolder('InProgress');" + + "var f = Object.getOwnPropertyDescriptor(Object.getPrototypeOf(s), 'status');" + + "f.set.call(s, 'DONE');" + + "s.status", + "source", + 1, + null)); + assertTrue( + error.toString().contains("Cannot call method \"call\" of undefined")); + return null; + }); } static class StatusHolder extends ScriptableObject {