Skip to content

Commit

Permalink
PropertyDescriptor fixes (#1324)
Browse files Browse the repository at this point in the history
* the PropertyDescriptor has an setter slot even if no setter function is available
* fix the order of the slots
  • Loading branch information
rbri authored Jun 2, 2023
1 parent a63f215 commit 5a0e5b3
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 7 deletions.
28 changes: 27 additions & 1 deletion src/org/mozilla/javascript/AccessorSlot.java
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,22 @@ ScriptableObject getPropertyDescriptor(Context cx, Scriptable scope) {
// It sounds logical that this would be the same as the logic for a normal Slot,
// but the spec is super pedantic about things like the order of properties here,
// so we need special support here.

ScriptableObject desc = (ScriptableObject) cx.newObject(scope);
desc.setCommonDescriptorProperties(getAttributes(), getter == null && setter == null);
int attr = getAttributes();

boolean es6 = cx.getLanguageVersion() >= Context.VERSION_ES6;
if (es6) {
if (getter == null && setter == null) {
desc.defineProperty(
"writable",
(attr & ScriptableObject.READONLY) == 0,
ScriptableObject.EMPTY);
}
} else {
desc.setCommonDescriptorProperties(attr, getter == null && setter == null);
}

String fName = name == null ? "f" : name.toString();
if (getter != null) {
Function f = getter.asGetterFunction(fName, scope);
Expand All @@ -42,7 +56,19 @@ ScriptableObject getPropertyDescriptor(Context cx, Scriptable scope) {
if (setter != null) {
Function f = setter.asSetterFunction(fName, scope);
desc.defineProperty("set", f == null ? Undefined.instance : f, 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);
}

return desc;
}

Expand Down
15 changes: 15 additions & 0 deletions testsrc/org/mozilla/javascript/tests/es6/NativeObjectTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

/** Test for NativeObject. */
public class NativeObjectTest {

@Test
public void assignPropertyGetter() {
evaluateAndAssert(
Expand Down Expand Up @@ -218,6 +219,20 @@ public void assignWithPrototypeWithGettersAndSetters() {
"targetProto: {\"_a\":1,\"a\":1,\"_b\":2,\"b\":2} target: {\"_b\":26,\"_c\":5,\"c\":5} assigned: {\"_b\":26,\"_c\":5,\"c\":5} assigned.a: 1 assigned.b: 26 assigned.c: 5");
}

@Test
public void getOwnPropertyDescriptorSetPropertyIsAlwaysDefined() {
evaluateAndAssert(
"var obj = Object.defineProperty({}, 'prop', {\n"
+ " get: function() {}\n"
+ " });\n"
+ "var desc = Object.getOwnPropertyDescriptor(obj, 'prop');\n"
+ "'' + obj.prop"
+ "+ ' ' + desc.get"
+ "+ ' ' + desc.set"
+ "+ ' [' + Object.getOwnPropertyNames(desc) + ']'",
"undefined \nfunction () {\n}\n undefined [get,set,enumerable,configurable]");
}

@Test
public void setPrototypeOfNull() {
evaluateAndAssert(
Expand Down
7 changes: 1 addition & 6 deletions testsrc/test262.properties
Original file line number Diff line number Diff line change
Expand Up @@ -765,13 +765,12 @@ built-ins/Number 9/283 (3.18%)
S9.3.1_A3_T1_U180E.js {unsupported: [u180e]}
S9.3.1_A3_T2_U180E.js {unsupported: [u180e]}

built-ins/Object 132/3150 (4.19%)
built-ins/Object 127/3150 (4.03%)
assign/source-own-prop-desc-missing.js {unsupported: [Proxy]}
assign/source-own-prop-error.js {unsupported: [Proxy]}
assign/source-own-prop-keys-error.js {unsupported: [Proxy]}
assign/strings-and-symbol-order.js
assign/strings-and-symbol-order-proxy.js {unsupported: [Proxy]}
create/15.2.3.5-4-311.js
defineProperties/15.2.3.7-6-a-112.js non-strict
defineProperties/15.2.3.7-6-a-113.js non-strict
defineProperties/15.2.3.7-6-a-118.js
Expand Down Expand Up @@ -802,10 +801,7 @@ built-ins/Object 132/3150 (4.19%)
defineProperty/15.2.3.6-4-177.js non-strict
defineProperty/15.2.3.6-4-188.js
defineProperty/15.2.3.6-4-189.js
defineProperty/15.2.3.6-4-206.js
defineProperty/15.2.3.6-4-254.js
defineProperty/15.2.3.6-4-255.js
defineProperty/15.2.3.6-4-256.js
defineProperty/15.2.3.6-4-293-1.js
defineProperty/15.2.3.6-4-293-3.js non-strict
defineProperty/15.2.3.6-4-293-4.js strict
Expand Down Expand Up @@ -835,7 +831,6 @@ built-ins/Object 132/3150 (4.19%)
getOwnPropertyDescriptor/15.2.3.3-4-213.js
getOwnPropertyDescriptor/15.2.3.3-4-214.js
getOwnPropertyDescriptor/15.2.3.3-4-215.js
getOwnPropertyDescriptor/15.2.3.3-4-249.js
getOwnPropertyDescriptor/15.2.3.3-4-250.js
getOwnPropertyNames/order-after-define-property.js
getOwnPropertyNames/proxy-invariant-absent-not-configurable-symbol-key.js {unsupported: [Proxy]}
Expand Down

0 comments on commit 5a0e5b3

Please sign in to comment.