Skip to content

Commit 31c8f5b

Browse files
rbrirbri
authored
Reflect and Proxy get() improvements
* Reflect#get: * if receiver is not present, then set receiver to target * use the receiver when searching for the property * walk the prototype chain when searching for teh property * NativeProxy has to use the correct Receiver when calling the get trap
1 parent 26ae996 commit 31c8f5b

5 files changed

Lines changed: 172 additions & 17 deletions

File tree

‎rhino/src/main/java/org/mozilla/javascript/NativeProxy.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -387,7 +387,7 @@ public Object get(String name, Scriptable start) {
387387

388388
Function trap = getTrap(TRAP_GET);
389389
if (trap != null) {
390-
Object trapResult = callTrap(trap, new Object[] {target, name, this});
390+
Object trapResult = callTrap(trap, new Object[] {target, name, start});
391391

392392
DescriptorInfo targetDesc = target.getOwnPropertyDescriptor(Context.getContext(), name);
393393
if (targetDesc != null && targetDesc.isConfigurable(false)) {
@@ -443,7 +443,7 @@ public Object get(int index, Scriptable start) {
443443
Function trap = getTrap(TRAP_GET);
444444
if (trap != null) {
445445
Object trapResult =
446-
callTrap(trap, new Object[] {target, ScriptRuntime.toString(index), this});
446+
callTrap(trap, new Object[] {target, ScriptRuntime.toString(index), start});
447447

448448
DescriptorInfo targetDesc =
449449
target.getOwnPropertyDescriptor(Context.getContext(), index);
@@ -501,7 +501,7 @@ public Object get(Symbol key, Scriptable start) {
501501

502502
Function trap = getTrap(TRAP_GET);
503503
if (trap != null) {
504-
Object trapResult = callTrap(trap, new Object[] {target, key, this});
504+
Object trapResult = callTrap(trap, new Object[] {target, key, start});
505505

506506
DescriptorInfo targetDesc = target.getOwnPropertyDescriptor(Context.getContext(), key);
507507
if (targetDesc != null

‎rhino/src/main/java/org/mozilla/javascript/NativeReflect.java‎

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -258,20 +258,26 @@ private static Object get(
258258
*/
259259
ScriptableObject target = checkTarget(args);
260260

261-
if (args.length > 1) {
262-
if (ScriptRuntime.isSymbol(args[1])) {
263-
Object prop = ScriptableObject.getProperty(target, (Symbol) args[1]);
264-
return prop == Scriptable.NOT_FOUND ? Undefined.SCRIPTABLE_UNDEFINED : prop;
265-
}
266-
if (args[1] instanceof Number) {
267-
Object prop = ScriptableObject.getProperty(target, ScriptRuntime.toIndex(args[1]));
268-
return prop == Scriptable.NOT_FOUND ? Undefined.SCRIPTABLE_UNDEFINED : prop;
269-
}
261+
if (args.length < 2) {
262+
return Undefined.SCRIPTABLE_UNDEFINED;
263+
}
264+
265+
Scriptable receiver =
266+
(args.length > 2 && args[2] instanceof Scriptable) ? (Scriptable) args[2] : target;
270267

271-
Object prop = ScriptableObject.getProperty(target, ScriptRuntime.toString(args[1]));
272-
return prop == Scriptable.NOT_FOUND ? Undefined.SCRIPTABLE_UNDEFINED : prop;
268+
Object prop;
269+
if (ScriptRuntime.isSymbol(args[1])) {
270+
prop = ScriptableObject.getSuperProperty(target, receiver, (Symbol) args[1]);
271+
} else if (args[1] instanceof Number) {
272+
prop =
273+
ScriptableObject.getSuperProperty(
274+
target, receiver, ScriptRuntime.toIndex(args[1]));
275+
} else {
276+
prop =
277+
ScriptableObject.getSuperProperty(
278+
target, receiver, ScriptRuntime.toString(args[1]));
273279
}
274-
return Undefined.SCRIPTABLE_UNDEFINED;
280+
return prop == Scriptable.NOT_FOUND ? Undefined.SCRIPTABLE_UNDEFINED : prop;
275281
}
276282

277283
/**

‎tests/src/test/java/org/mozilla/javascript/tests/es6/NativeProxyTest.java‎

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -714,6 +714,91 @@ public void getTrapIsNullTargetIsProxy2() {
714714
Utils.assertWithAllModes_ES6("1 2 3 undefined", js);
715715
}
716716

717+
@Test
718+
public void getTrapReceivesCorrectReceiverWhenDifferentFromProxy() {
719+
String js =
720+
"var log = [];\n"
721+
+ "var target = {};\n"
722+
+ "var otherReceiver = { label: 'other' };\n"
723+
+ "var proxy = new Proxy(target, {\n"
724+
+ " get: function(t, prop, receiver) {\n"
725+
+ " log.push(receiver === otherReceiver ? 'correct' : 'wrong');\n"
726+
+ " return 42;\n"
727+
+ " }\n"
728+
+ "});\n"
729+
+ "Reflect.get(proxy, 'p', otherReceiver);\n"
730+
+ "'' + log;";
731+
Utils.assertWithAllModes_ES6("correct", js);
732+
}
733+
734+
@Test
735+
public void getTrapReceivesProxyAsReceiverWhenNoExplicitReceiverGiven() {
736+
String js =
737+
"var target = {};\n"
738+
+ "var proxy = new Proxy(target, {\n"
739+
+ " get: function(t, prop, receiver) {\n"
740+
+ " return receiver === proxy;\n"
741+
+ " }\n"
742+
+ "});\n"
743+
+ "'' + Reflect.get(proxy, 'p');";
744+
Utils.assertWithAllModes_ES6("true", js);
745+
}
746+
747+
@Test
748+
public void getTrapReceiverIsProxyForDirectPropertyRead() {
749+
String js =
750+
"var log = [];\n"
751+
+ "var target = {};\n"
752+
+ "var proxy = new Proxy(target, {\n"
753+
+ " get: function(t, prop, receiver) {\n"
754+
+ " log.push(receiver === proxy ? 'proxy' : 'other');\n"
755+
+ " return 1;\n"
756+
+ " }\n"
757+
+ "});\n"
758+
+ "var _ = proxy.p;\n"
759+
+ "'' + log;";
760+
Utils.assertWithAllModes_ES6("proxy", js);
761+
}
762+
763+
@Test
764+
public void getTrapReceivesCorrectReceiverWithIndexKey() {
765+
String js =
766+
"var log = [];\n"
767+
+ "var target = [];\n"
768+
+ "var otherReceiver = { label: 'other' };\n"
769+
+ "var proxy = new Proxy(target, {\n"
770+
+ " get: function(t, prop, receiver) {\n"
771+
+ " if (prop === '0') {\n"
772+
+ " log.push(receiver === otherReceiver ? 'correct' : 'wrong');\n"
773+
+ " }\n"
774+
+ " return undefined;\n"
775+
+ " }\n"
776+
+ "});\n"
777+
+ "Reflect.get(proxy, 0, otherReceiver);\n"
778+
+ "'' + log;";
779+
Utils.assertWithAllModes_ES6("correct", js);
780+
}
781+
782+
@Test
783+
public void getTrapReceivesCorrectReceiverWithSymbolKey() {
784+
String js =
785+
"var log = [];\n"
786+
+ "var sym = Symbol('test');\n"
787+
+ "var target = {};\n"
788+
+ "var otherReceiver = { label: 'other' };\n"
789+
+ "var proxy = new Proxy(target, {\n"
790+
+ " get: function(t, prop, receiver) {\n"
791+
+ " if (prop === sym) {\n"
792+
+ " log.push(receiver === otherReceiver ? 'correct' : 'wrong');\n"
793+
+ " }\n"
794+
+ " return undefined;\n"
795+
+ " }\n"
796+
+ "});\n"
797+
+ "Reflect.get(proxy, sym, otherReceiver);\n"
798+
+ "'' + log;";
799+
Utils.assertWithAllModes_ES6("correct", js);
800+
}
801+
717802
@Test
718803
public void setTrap() {
719804
String js =

‎tests/src/test/java/org/mozilla/javascript/tests/es6/NativeReflectTest.java‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -415,6 +415,71 @@ public void getProperty() {
415415
Utils.assertWithAllModes_ES6("value 1, undefined, foo, 42, undefined", js);
416416
}
417417

418+
@Test
419+
public void getReceiverDefaultsToTarget() {
420+
// When no receiver is supplied, getter sees target as 'this'.
421+
String js =
422+
"var o = {};\n"
423+
+ "Object.defineProperty(o, 'p', {\n"
424+
+ " get() { return this === o; }\n"
425+
+ "});\n"
426+
+ "'' + Reflect.get(o, 'p');";
427+
Utils.assertWithAllModes_ES6("true", js);
428+
}
429+
430+
@Test
431+
public void getReceiverPassedToGetter() {
432+
// When a receiver is supplied, getter sees receiver as 'this', not target.
433+
String js =
434+
"var target = {};\n"
435+
+ "var receiver = {};\n"
436+
+ "Object.defineProperty(target, 'p', {\n"
437+
+ " get() { return this === receiver; }\n"
438+
+ "});\n"
439+
+ "'' + Reflect.get(target, 'p', receiver);";
440+
Utils.assertWithAllModes_ES6("true", js);
441+
}
442+
443+
@Test
444+
public void getReceiverPassedToGetterOnPrototype() {
445+
// Getter is on the prototype, receiver is the leaf object — 'this' must
446+
// still be the receiver, not the prototype where the getter lives.
447+
String js =
448+
"var receiver = {};\n"
449+
+ "var proto = {};\n"
450+
+ "Object.defineProperty(proto, 'p', {\n"
451+
+ " get() { return this === receiver; }\n"
452+
+ "});\n"
453+
+ "var target = Object.create(proto);\n"
454+
+ "'' + Reflect.get(target, 'p', receiver);";
455+
Utils.assertWithAllModes_ES6("true", js);
456+
}
457+
458+
@Test
459+
public void getReceiverDoesNotAffectDataProperty() {
460+
// Receiver has no effect on plain data property reads — value comes from
461+
// the prototype chain of target, not receiver.
462+
String js =
463+
"var target = { p: 42 };\n"
464+
+ "var receiver = { p: 99 };\n"
465+
+ "'' + Reflect.get(target, 'p', receiver);";
466+
Utils.assertWithAllModes_ES6("42", js);
467+
}
468+
469+
@Test
470+
public void getReceiverWithSymbolKey() {
471+
// Receiver must also be forwarded for Symbol-keyed getters.
472+
String js =
473+
"var sym = Symbol('test');\n"
474+
+ "var target = {};\n"
475+
+ "var receiver = {};\n"
476+
+ "Object.defineProperty(target, sym, {\n"
477+
+ " get() { return this === receiver; }\n"
478+
+ "});\n"
479+
+ "'' + Reflect.get(target, sym, receiver);";
480+
Utils.assertWithAllModes_ES6("true", js);
481+
}
482+
418483
@Test
419484
public void setPrototypeOf() {
420485
String js =

‎tests/testsrc/test262.properties‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1989,11 +1989,10 @@ built-ins/Proxy 65/311 (20.9%)
19891989
get-fn-realm.js
19901990
get-fn-realm-recursive.js
19911991

1992-
built-ins/Reflect 11/153 (7.19%)
1992+
built-ins/Reflect 10/153 (6.54%)
19931993
defineProperty/return-abrupt-from-property-key.js
19941994
deleteProperty/return-abrupt-from-result.js
19951995
deleteProperty/return-boolean.js strict
1996-
get/return-value-from-receiver.js
19971996
ownKeys/return-on-corresponding-order-large-index.js
19981997
set/call-prototype-property-set.js
19991998
set/different-property-descriptors.js

0 commit comments

Comments
 (0)