Skip to content

Commit

Permalink
Support more spec-compliant ScriptableObject extensions
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 authored Dec 30, 2024
1 parent c2b2b59 commit d243106
Show file tree
Hide file tree
Showing 5 changed files with 194 additions and 21 deletions.
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
26 changes: 20 additions & 6 deletions rhino/src/main/java/org/mozilla/javascript/NativeObject.java
Original file line number Diff line number Diff line change
Expand Up @@ -737,8 +737,14 @@ && isEnumerable(stringId, sourceObj)) {
private boolean isEnumerable(int index, Object obj) {
if (obj instanceof ScriptableObject) {
ScriptableObject so = (ScriptableObject) obj;
int attrs = so.getAttributes(index);
return (attrs & ScriptableObject.DONTENUM) == 0;
try {
int attrs = so.getAttributes(index);
return (attrs & ScriptableObject.DONTENUM) == 0;
} catch (RhinoException re) {
// Not all ScriptableObject implementations implement
// "getAttributes" for all properties
return true;
}
} else {
return true;
}
Expand All @@ -747,8 +753,12 @@ private boolean isEnumerable(int index, Object obj) {
private boolean isEnumerable(String key, Object obj) {
if (obj instanceof ScriptableObject) {
ScriptableObject so = (ScriptableObject) obj;
int attrs = so.getAttributes(key);
return (attrs & ScriptableObject.DONTENUM) == 0;
try {
int attrs = so.getAttributes(key);
return (attrs & ScriptableObject.DONTENUM) == 0;
} catch (RhinoException re) {
return true;
}
} else {
return true;
}
Expand All @@ -757,8 +767,12 @@ private boolean isEnumerable(String key, Object obj) {
private boolean isEnumerable(Symbol sym, Object obj) {
if (obj instanceof ScriptableObject) {
ScriptableObject so = (ScriptableObject) obj;
int attrs = so.getAttributes(sym);
return (attrs & ScriptableObject.DONTENUM) == 0;
try {
int attrs = so.getAttributes(sym);
return (attrs & ScriptableObject.DONTENUM) == 0;
} catch (RhinoException re) {
return true;
}
} else {
return true;
}
Expand Down
46 changes: 38 additions & 8 deletions rhino/src/main/java/org/mozilla/javascript/ScriptableObject.java
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,134 @@
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;

public class AssignSubclassTest {
@Test
public void basicOperations() {
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.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 {
private String foo;
private String bar;

public MySubclass() {}

@Override
public String getClassName() {
return "MySubclass";
}

@Override
public Object get(String name, Scriptable start) {
if ("foo".equals(name)) {
return foo;
}
return super.get(name, start);
}

@Override
public boolean has(String name, Scriptable start) {
return "foo".equals(name) || super.has(name, start);
}

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

@Override
public Object get(int ix, Scriptable start) {
if (ix == 0) {
return bar;
}
return super.get(ix, start);
}

@Override
public boolean has(int ix, Scriptable start) {
return ix == 0 || super.has(ix, start);
}

@Override
public boolean putOwnProperty(int ix, Scriptable start, Object value, boolean isThrow) {
if (ix == 0) {
bar = ScriptRuntime.toString(value);
return true;
}
return super.putOwnProperty(ix, start, value, isThrow);
}
}
}
5 changes: 0 additions & 5 deletions tests/testsrc/test262.properties
Original file line number Diff line number Diff line change
Expand Up @@ -1132,7 +1132,6 @@ built-ins/Object 178/3408 (5.22%)
setPrototypeOf/not-a-constructor.js
values/not-a-constructor.js
values/observable-operations.js
values/order-after-define-property.js
property-order.js
proto-from-ctor-realm.js
subclass-object-arg.js {unsupported: [class]}
Expand Down Expand Up @@ -1560,10 +1559,7 @@ built-ins/Proxy 76/311 (24.44%)
getOwnPropertyDescriptor/resultdesc-is-not-configurable-not-writable-targetdesc-is-writable.js
getOwnPropertyDescriptor/resultdesc-is-not-configurable-targetdesc-is-configurable.js
getOwnPropertyDescriptor/resultdesc-is-not-configurable-targetdesc-is-undefined.js
getOwnPropertyDescriptor/trap-is-missing-target-is-proxy.js
getOwnPropertyDescriptor/trap-is-null-target-is-proxy.js
getOwnPropertyDescriptor/trap-is-undefined.js
getOwnPropertyDescriptor/trap-is-undefined-target-is-proxy.js
get/accessor-get-is-undefined-throws.js
get/trap-is-undefined-receiver.js
has/call-in-prototype.js
Expand Down Expand Up @@ -3188,7 +3184,6 @@ language/arguments-object 185/263 (70.34%)
mapped/nonwritable-nonenumerable-nonconfigurable-descriptors-set-by-param.js non-strict
unmapped/via-params-dstr.js non-strict
unmapped/via-params-rest.js non-strict
10.6-11-b-1.js
arguments-caller.js
async-gen-meth-args-trailing-comma-multiple.js {unsupported: [async-iteration, async]}
async-gen-meth-args-trailing-comma-null.js {unsupported: [async-iteration, async]}
Expand Down

0 comments on commit d243106

Please sign in to comment.