Skip to content

Commit

Permalink
Improve support for ScriptableObject subclasses
Browse files Browse the repository at this point in the history 
Add "putOwnProperty" methods, which can return boolean and be used in
strict mode, and wire them in so that there is no need to call "putImpl"
directly or override it.

Also, allow subclasses that don't override the attributes methods in
ScriptableObject to work as well.
  • Loading branch information
@gbrail
gbrail committed Dec 23, 2024
1 parent 6124763 commit 2c192ab
Show file tree
Hide file tree
Showing 4 changed files with 112 additions and 68 deletions.
4 changes: 2 additions & 2 deletions rhino/src/main/java/org/mozilla/javascript/AbstractEcmaObjectOperations.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ static void put(Context cx, Scriptable o, String p, Object v, boolean isThrow) {
if (base == null) base = o;

if (base instanceof ScriptableObject) {
if (((ScriptableObject) base).putImpl(p, 0, o, v, isThrow)) return;
if (((ScriptableObject) base).putOwnProperty(p, o, v, isThrow)) return;

o.put(p, o, v);
} else {
Expand All @@ -239,7 +239,7 @@ static void put(Context cx, Scriptable o, int p, Object v, boolean isThrow) {
if (base == null) base = o;

if (base instanceof ScriptableObject) {
if (((ScriptableObject) base).putImpl(null, p, o, v, isThrow)) return;
if (((ScriptableObject) base).putOwnProperty(p, o, v, isThrow)) return;

o.put(p, o, v);
} else {
Expand Down
6 changes: 4 additions & 2 deletions rhino/src/main/java/org/mozilla/javascript/NativeObject.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -674,14 +674,16 @@ public Object execIdCall(
if (sourceObj.has(intId, sourceObj)
&& isEnumerable(intId, sourceObj)) {
Object val = sourceObj.get(intId, sourceObj);
ScriptableObject.putPropertyStrict(targetObj, intId, val);
AbstractEcmaObjectOperations.put(
cx, targetObj, intId, val, true);
}
} else {
String stringId = ScriptRuntime.toString(key);
if (sourceObj.has(stringId, sourceObj)
&& isEnumerable(stringId, sourceObj)) {
Object val = sourceObj.get(stringId, sourceObj);
ScriptableObject.putPropertyStrict(targetObj, stringId, val);
AbstractEcmaObjectOperations.put(
cx, targetObj, stringId, val, true);
}
}
}
Expand Down
46 changes: 38 additions & 8 deletions rhino/src/main/java/org/mozilla/javascript/ScriptableObject.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -294,12 +294,23 @@ public Object get(Symbol key, Scriptable start) {
*/
@Override
public void put(String name, Scriptable start, Object value) {
if (putImpl(name, 0, start, value)) return;
if (putOwnProperty(name, start, value, Context.isCurrentContextStrict())) return;

if (start == this) throw Kit.codeBug();
start.put(name, start, value);
}

/**
* Set the value of the named property, and return true if the property is actually defined and
* can be set. Subclasses of ScriptableObject should override this method, and not "put," for
* proper strict mode operation in the future.
*
* @param isThrow if true, throw an exception as if in strict mode
*/
protected boolean putOwnProperty(String name, Scriptable start, Object value, boolean isThrow) {
return putImpl(name, 0, start, value, isThrow);
}

/**
* Sets the value of the indexed property, creating it if need be.
*
Expand All @@ -326,21 +337,43 @@ public void put(int index, Scriptable start, Object value) {
return;
}

if (putImpl(null, index, start, value)) return;
if (putOwnProperty(index, start, value, Context.isCurrentContextStrict())) return;

if (start == this) throw Kit.codeBug();
start.put(index, start, value);
}

/**
* Set the value of the named property, and return true if the property is actually defined and
* can be set. Subclasses of ScriptableObject should override this method, and not "put," for
* proper strict mode operation in the future
*
* @param isThrow if true, throw an exception as if in strict mode
*/
protected boolean putOwnProperty(int index, Scriptable start, Object value, boolean isThrow) {
return putImpl(null, index, start, value, isThrow);
}

/** Implementation of put required by SymbolScriptable objects. */
@Override
public void put(Symbol key, Scriptable start, Object value) {
if (putImpl(key, 0, start, value)) return;
if (putOwnProperty(key, start, value, Context.isCurrentContextStrict())) return;

if (start == this) throw Kit.codeBug();
ensureSymbolScriptable(start).put(key, start, value);
}

/**
* Set the value of the named property, and return true if the property is actually defined and
* can be set. Subclasses of ScriptableObject should override this method, and not "put," for
* proper strict mode operation in the future
*
* @param isThrow if true, throw an exception as if in strict mode
*/
protected boolean putOwnProperty(Symbol key, Scriptable start, Object value, boolean isThrow) {
return putImpl(key, 0, start, value, isThrow);
}

/**
* Removes a named property from the object.
*
Expand Down Expand Up @@ -2662,10 +2695,6 @@ public final synchronized Object associateValue(Object key, Object value) {
return Kit.initHash(h, key, value);
}

private boolean putImpl(Object key, int index, Scriptable start, Object value) {
return putImpl(key, index, start, value, Context.isCurrentContextStrict());
}

/**
* @param key
* @param index
Expand All @@ -2675,7 +2704,8 @@ private boolean putImpl(Object key, int index, Scriptable start, Object value) {
* @return false if this != start and no slot was found. true if this == start or this != start
* and a READONLY slot was found.
*/
boolean putImpl(Object key, int index, Scriptable start, Object value, boolean isThrow) {
private boolean putImpl(
Object key, int index, Scriptable start, Object value, boolean isThrow) {
// This method is very hot (basically called on each assignment)
// so we inline the extensible/sealed checks below.
Slot slot;
Expand Down
124 changes: 68 additions & 56 deletions rhino/src/test/java/org/mozilla/javascript/tests/AssignSubclassTest.java
View file
Original file line number Diff line number Diff line change
@@ -1,67 +1,78 @@
package org.mozilla.javascript.tests;

import static org.junit.jupiter.api.Assertions.*;

import org.junit.jupiter.api.Test;
import org.mozilla.javascript.ScriptRuntime;
import org.mozilla.javascript.Scriptable;
import org.mozilla.javascript.ScriptableObject;

import static org.junit.jupiter.api.Assertions.*;

public class AssignSubclassTest {
@Test
public void basicOperations() {
Utils.runWithAllOptimizationLevels(cx -> {
Scriptable scope = cx.initStandardObjects();
try {
ScriptableObject.defineClass(scope, MySubclass.class);
} catch (Exception e) {
fail(e);
}
Object result = cx.evaluateString(scope,
"let o = new MySubclass();" +
"o.foo = 'bar';\n" +
"o.something = 'else';\n" +
"o[0] = 'baz';\n" +
"o[1] = 'frooby';\n" +
"o;\n",
"test", 1, null);
assertInstanceOf(Scriptable.class, result);
Scriptable o = (Scriptable) result;
assertEquals("bar", o.get("foo", o));
assertEquals("else", o.get("something", o));
assertEquals("baz", o.get(0, o));
assertEquals("frooby", o.get(1, o));
return null;
});
Utils.runWithAllModes(
cx -> {
Scriptable scope = cx.initStandardObjects();
try {
ScriptableObject.defineClass(scope, MySubclass.class);
} catch (Exception e) {
fail(e);
}
Object result =
cx.evaluateString(
scope,
"let o = new MySubclass();"
+ "o.foo = 'bar';\n"
+ "o.something = 'else';\n"
+ "o[0] = 'baz';\n"
+ "o[1] = 'frooby';\n"
+ "o;\n",
"test",
1,
null);
assertInstanceOf(Scriptable.class, result);
Scriptable o = (Scriptable) result;
assertEquals("bar", o.get("foo", o));
assertEquals("else", o.get("something", o));
assertEquals("baz", o.get(0, o));
assertEquals("frooby", o.get(1, o));
return null;
});
}

@Test
public void assign() {
Utils.runWithAllOptimizationLevels(cx -> {
Scriptable scope = cx.initStandardObjects();
try {
ScriptableObject.defineClass(scope, MySubclass.class);
} catch (Exception e) {
fail(e);
}
Object result =cx.evaluateString(scope, "\n" +
"let s = {foo: 'bar', something: 'else', 0: 'baz', 1: 'frooby'};\n" +
"let o = new MySubclass();" +
"o.foo = 'x';\n" +
"o.something = 'y';\n" +
"o[0] = 'z';\n" +
"o[1] = false;\n" +
"Object.assign(o, s);\n" +
"o;\n",
"test", 1, null);
assertInstanceOf(Scriptable.class, result);
Scriptable o = (Scriptable) result;
assertEquals("bar", o.get("foo", o));
assertEquals("else", o.get("something", o));
assertEquals("baz", o.get(0, o));
assertEquals("frooby", o.get(1, o));
return null;
});
Utils.runWithAllModes(
cx -> {
Scriptable scope = cx.initStandardObjects();
try {
ScriptableObject.defineClass(scope, MySubclass.class);
} catch (Exception e) {
fail(e);
}
Object result =
cx.evaluateString(
scope,
"\n"
+ "let s = {foo: 'bar', something: 'else', 0: 'baz', 1: 'frooby'};\n"
+ "let o = new MySubclass();"
+ "o.foo = 'x';\n"
+ "o.something = 'y';\n"
+ "o[0] = 'z';\n"
+ "o[1] = false;\n"
+ "Object.assign(o, s);\n"
+ "o;\n",
"test",
1,
null);
assertInstanceOf(Scriptable.class, result);
Scriptable o = (Scriptable) result;
assertEquals("bar", o.get("foo", o));
assertEquals("else", o.get("something", o));
assertEquals("baz", o.get(0, o));
assertEquals("frooby", o.get(1, o));
return null;
});
}

public static class MySubclass extends ScriptableObject {
Expand Down Expand Up @@ -89,12 +100,13 @@ public boolean has(String name, Scriptable start) {
}

@Override
public void put(String name, Scriptable start, Object value) {
public boolean putOwnProperty(
String name, Scriptable start, Object value, boolean isThrow) {
if ("foo".equals(name)) {
foo = ScriptRuntime.toString(value);
} else {
super.put(name, start, value);
return true;
}
return super.putOwnProperty(name, start, value, isThrow);
}

@Override
Expand All @@ -111,12 +123,12 @@ public boolean has(int ix, Scriptable start) {
}

@Override
public void put(int ix, Scriptable start, Object value) {
public boolean putOwnProperty(int ix, Scriptable start, Object value, boolean isThrow) {
if (ix == 0) {
bar = ScriptRuntime.toString(value);
} else {
super.put(ix, start, value);
return true;
}
return super.putOwnProperty(ix, start, value, isThrow);
}
}
}

0 comments on commit 2c192ab

Please sign in to comment.