[GR-51124] Improvements of consistency of delete, get and set operati…

…ons on foreign objects.

PullRequest: js/3020

graal-js/src/com.oracle.truffle.js.test/js/GR-51124.js

@@ -0,0 +1,74 @@
+/*
+ * Copyright (c) 2024, 2024, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * Licensed under the Universal Permissive License v 1.0 as shown at http://oss.oracle.com/licenses/upl.
+ */
+
+/**
+ * Regression test of the consistency of delete, get and set operations on foreign objects.
+ */
+
+load("assert.js");
+
+// Foreign array
+const list = new java.util.ArrayList();
+const listItem = 'someItem';
+list.add(listItem);
+const proxyList = new Proxy(list, {});
+
+// delete
+assertTrue(delete list['foo']);
+assertTrue(Reflect.deleteProperty(list, 'foo'));
+assertTrue(delete proxyList['foo']);
+
+// get
+assertSame(listItem, list[0]);
+assertSame(listItem, Reflect.get(list, 0));
+assertSame(listItem, proxyList[0]);
+
+// set
+const newValue = 'someValue';
+list[0] = newValue;
+assertSame(newValue, list[0]);
+list[0] = listItem;
+
+Reflect.set(list, 0, newValue);
+assertSame(newValue, list[0]);
+list[0] = listItem;
+
+proxyList[0] = newValue;
+assertSame(newValue, proxyList[0]);
+assertSame(newValue, list[0]);
+list[0] = listItem;
+
+// Foreign map
+const map = new java.util.HashMap();
+const mapKey = 'klic';
+const mapValue = 'hodnota';
+map.put(mapKey, mapValue);
+const proxyMap = new Proxy(map, {});
+
+// delete
+assertTrue(delete map['foo']);
+assertTrue(Reflect.deleteProperty(map, 'foo'));
+assertTrue(delete proxyMap['foo']);
+
+// get
+assertSame(mapValue, map[mapKey]);
+assertSame(mapValue, Reflect.get(map, mapKey));
+assertSame(mapValue, proxyMap[mapKey]);
+
+// set
+map[mapKey] = newValue;
+assertSame(newValue, map[mapKey]);
+map[mapKey] = mapValue;
+
+Reflect.set(map, mapKey, newValue);
+assertSame(newValue, map[mapKey]);
+map[mapKey] = mapValue;
+
+proxyMap[mapKey] = newValue;
+assertSame(newValue, proxyMap[mapKey]);
+assertSame(newValue, map[mapKey]);
+map[mapKey] = mapValue;

graal-js/src/com.oracle.truffle.js.test/js/github_graaljs_787.js

@@ -0,0 +1,40 @@
+/*
+ * Copyright (c) 2024, 2024, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * Licensed under the Universal Permissive License v 1.0 as shown at http://oss.oracle.com/licenses/upl.
+ */
+
+/**
+ * An example of how to implement a custom delete on a foreign array
+ * (by wrapping in a JS proxy with deleteProperty trap).
+ */
+
+load("assert.js");
+
+const array = new java.util.ArrayList();
+array.add('first');
+array.add('second');
+array.add('third');
+
+// Elements of foreign array cannot be deleted
+delete array[1];
+assertSame('second', array[1]);
+
+var handler = {};
+handler.deleteProperty = function(target, prop) {
+  const index = Number(prop);
+  if (0 <= index && index < target.length) {
+    target[prop] = 'deleted';
+    return true;
+  } else {
+    return Reflect.deleteProperty(target, prop);
+  }
+};
+const proxy = new Proxy(array, handler);
+
+// deleteProperty trap of the proxy intercepts the delete
+// and allows its custom implementation
+delete proxy[1];
+assertSame('deleted', proxy[1]);
+assertSame('deleted', array[1]);

graal-js/src/com.oracle.truffle.js.test/src/com/oracle/truffle/js/test/interop/GR51124.java

@@ -0,0 +1,172 @@
+/*
+ * Copyright (c) 2024, 2024, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * The Universal Permissive License (UPL), Version 1.0
+ *
+ * Subject to the condition set forth below, permission is hereby granted to any
+ * person obtaining a copy of this software, associated documentation and/or
+ * data (collectively the "Software"), free of charge and under any and all
+ * copyright rights in the Software, and any and all patent rights owned or
+ * freely licensable by each licensor hereunder covering either (i) the
+ * unmodified Software as contributed to or provided by such licensor, or (ii)
+ * the Larger Works (as defined below), to deal in both
+ *
+ * (a) the Software, and
+ *
+ * (b) any piece of software and/or hardware listed in the lrgrwrks.txt file if
+ * one is included with the Software each a "Larger Work" to which the Software
+ * is contributed by such licensors),
+ *
+ * without restriction, including without limitation the rights to copy, create
+ * derivative works of, display, perform, and distribute the Software and make,
+ * use, sell, offer for sale, import, export, have made, and have sold the
+ * Software and the Larger Work(s), and to sublicense the foregoing rights on
+ * either these or other terms.
+ *
+ * This license is subject to the following condition:
+ *
+ * The above copyright notice and either this complete permission notice or at a
+ * minimum a reference to the UPL must be included in all copies or substantial
+ * portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ */
+package com.oracle.truffle.js.test.interop;
+
+import static com.oracle.truffle.js.lang.JavaScriptLanguage.ID;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+import java.util.HashMap;
+import java.util.Map;
+
+import org.graalvm.polyglot.Context;
+import org.graalvm.polyglot.Value;
+import org.graalvm.polyglot.proxy.ProxyArray;
+import org.graalvm.polyglot.proxy.ProxyHashMap;
+import org.junit.Test;
+
+import com.oracle.truffle.js.test.JSTest;
+
+/**
+ * Regression test of the consistency of delete, get and set operations on foreign objects.
+ */
+public class GR51124 {
+
+    @Test
+    public void testArrayDelete() {
+        try (Context context = JSTest.newContextBuilder().build()) {
+            ProxyArray array = ProxyArray.fromArray();
+            context.getBindings(ID).putMember("array", array);
+            context.eval(ID, "var proxyArray = new Proxy(array, {});");
+            assertTrue(context.eval(ID, "delete array['foo']").asBoolean());
+            assertTrue(context.eval(ID, "Reflect.deleteProperty(array, 'foo')").asBoolean());
+            assertTrue(context.eval(ID, "delete proxyArray['foo']").asBoolean());
+        }
+    }
+
+    @Test
+    public void testMapDelete() {
+        try (Context context = JSTest.newContextBuilder().build()) {
+            ProxyHashMap map = ProxyHashMap.from(Map.of());
+            context.getBindings(ID).putMember("map", map);
+            context.eval(ID, "var proxyMap = new Proxy(map, {});");
+            assertTrue(context.eval(ID, "delete map['foo']").asBoolean());
+            assertTrue(context.eval(ID, "Reflect.deleteProperty(map, 'foo')").asBoolean());
+            assertTrue(context.eval(ID, "delete proxyMap['foo']").asBoolean());
+        }
+    }
+
+    @Test
+    public void testArrayGet() {
+        try (Context context = JSTest.newContextBuilder().build()) {
+            String arrayItem = "someItem";
+            ProxyArray array = ProxyArray.fromArray(arrayItem);
+            context.getBindings(ID).putMember("array", array);
+            context.eval(ID, "var proxyArray = new Proxy(array, {});");
+            assertEquals(arrayItem, context.eval(ID, "array[0]").asString());
+            assertEquals(arrayItem, context.eval(ID, "Reflect.get(array, 0)").asString());
+            assertEquals(arrayItem, context.eval(ID, "proxyArray[0]").asString());
+        }
+    }
+
+    @Test
+    public void testMapGet() {
+        try (Context context = JSTest.newContextBuilder().build()) {
+            String mapKey = "klic";
+            String mapValue = "hodnota";
+            ProxyHashMap map = ProxyHashMap.from(Map.of(mapKey, mapValue));
+            Value bindings = context.getBindings(ID);
+            bindings.putMember("map", map);
+            bindings.putMember("mapKey", mapKey);
+            context.eval(ID, "var proxyMap = new Proxy(map, {});");
+            assertEquals(mapValue, context.eval(ID, "map[mapKey]").asString());
+            assertEquals(mapValue, context.eval(ID, "Reflect.get(map, mapKey)").asString());
+            assertEquals(mapValue, context.eval(ID, "proxyMap[mapKey]").asString());
+        }
+    }
+
+    @Test
+    public void testArraySet() {
+        try (Context context = JSTest.newContextBuilder().build()) {
+            String oldItem = "someItem";
+            String newItem = "anotherItem";
+            ProxyArray array = ProxyArray.fromArray(oldItem);
+            Value bindings = context.getBindings(ID);
+            bindings.putMember("array", array);
+            bindings.putMember("oldItem", oldItem);
+            bindings.putMember("newItem", newItem);
+            context.eval(ID, "var proxyArray = new Proxy(array, {});");
+
+            context.eval(ID, "array[0] = newItem;");
+            assertEquals(newItem, context.eval(ID, "array[0]").asString());
+            context.eval(ID, "array[0] = oldItem;");
+
+            context.eval(ID, "Reflect.set(array, 0, newItem);");
+            assertEquals(newItem, context.eval(ID, "array[0]").asString());
+            context.eval(ID, "array[0] = oldItem;");
+
+            context.eval(ID, "proxyArray[0] = newItem;");
+            assertEquals(newItem, context.eval(ID, "array[0]").asString());
+            context.eval(ID, "array[0] = oldItem;");
+        }
+    }
+
+    @Test
+    public void testMapSet() {
+        try (Context context = JSTest.newContextBuilder().build()) {
+            String mapKey = "someKey";
+            String oldValue = "someValue";
+            String newValue = "anotherValue";
+            Map<Object, Object> javaMap = new HashMap<>();
+            javaMap.put(mapKey, oldValue);
+            ProxyHashMap map = ProxyHashMap.from(javaMap);
+            Value bindings = context.getBindings(ID);
+            bindings.putMember("map", map);
+            bindings.putMember("mapKey", mapKey);
+            bindings.putMember("oldValue", oldValue);
+            bindings.putMember("newValue", newValue);
+            context.eval(ID, "var proxyMap = new Proxy(map, {});");
+
+            context.eval(ID, "map[mapKey] = newValue;");
+            assertEquals(newValue, context.eval(ID, "map[mapKey]").asString());
+            context.eval(ID, "map[mapKey] = oldValue;");
+
+            context.eval(ID, "Reflect.set(map, mapKey, newValue);");
+            assertEquals(newValue, context.eval(ID, "map[mapKey]").asString());
+            context.eval(ID, "map[mapKey] = oldValue;");
+
+            context.eval(ID, "proxyMap[mapKey] = newValue;");
+            assertEquals(newValue, context.eval(ID, "map[mapKey]").asString());
+            context.eval(ID, "map[mapKey] = oldValue;");
+        }
+    }
+
+}

graal-js/src/com.oracle.truffle.js.test/src/com/oracle/truffle/js/test/interop/ReflectInteropTest.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2019, 2020, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2019, 2024, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * The Universal Permissive License (UPL), Version 1.0
@@ -154,10 +154,10 @@ public void reflectDeleteProperty() {
 
         Value reflectDelete = context.eval(ID, "Reflect.deleteProperty");
         assertTrue(reflectDelete.execute(proxyObject, "p1").asBoolean());
-        assertFalse(reflectDelete.execute(proxyObject, "p1").asBoolean());
+        assertTrue(reflectDelete.execute(proxyObject, "p1").asBoolean());
         assertEquals(2, map.size());
         assertTrue(map.containsKey("p2"));
         assertTrue(map.containsKey("p3"));
-        assertFalse(reflectDelete.execute(proxyObject, 42).asBoolean());
+        assertTrue(reflectDelete.execute(proxyObject, 42).asBoolean());
     }
 }

graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/builtins/ArrayPrototypeBuiltins.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, 2023, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2024, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * The Universal Permissive License (UPL), Version 1.0
@@ -2764,7 +2764,7 @@ protected Object[] foreignArrayToObjectArray(Object thisObj, int len) {
             }
             Object[] array = new Object[len];
             for (int index = 0; index < len; index++) {
-                array[index] = JSInteropUtil.readArrayElementOrDefault(thisObj, index, Undefined.instance, interop, importValue, this);
+                array[index] = JSInteropUtil.readArrayElementOrDefault(thisObj, index, Undefined.instance, interop, importValue);
             }
             return array;
         }
@@ -3449,7 +3449,7 @@ private void spliceForeignMoveValue(JSArrayObject destObj, Object srcObj, long f
                 CompilerDirectives.transferToInterpreterAndInvalidate();
                 importValueNode = insert(ImportValueNode.create());
             }
-            Object val = JSInteropUtil.readArrayElementOrDefault(srcObj, fromIndex, Undefined.instance, arrays, importValueNode, this);
+            Object val = JSInteropUtil.readArrayElementOrDefault(srcObj, fromIndex, Undefined.instance, arrays, importValueNode);
             writeOwn(destObj, toIndex, val);
         }
     }

graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/builtins/GlobalBuiltins.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, 2023, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2024, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * The Universal Permissive License (UPL), Version 1.0
@@ -1742,7 +1742,7 @@ private static final class ScriptEngineGlobalScopeBindingsPropertyProxy extends
             @Override
             @TruffleBoundary
             public Object get(JSDynamicObject store) {
-                return JSInteropUtil.readMemberOrDefault(globalContextBindings, key, Undefined.instance, bindingsInterop, ImportValueNode.getUncached(), null);
+                return JSInteropUtil.readMemberOrDefault(globalContextBindings, key, Undefined.instance, bindingsInterop, ImportValueNode.getUncached());
             }
 
             @TruffleBoundary

graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/builtins/JSConstructTypedArrayNode.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, 2023, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2024, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * The Universal Permissive License (UPL), Version 1.0
@@ -423,7 +423,7 @@ protected JSDynamicObject doForeignObject(JSDynamicObject newTarget, Object obje
         if (fromArray) {
             assert length <= Integer.MAX_VALUE;
             for (int k = 0; k < length; k++) {
-                Object kValue = JSInteropUtil.readArrayElementOrDefault(object, k, 0, interop, importValue, this);
+                Object kValue = JSInteropUtil.readArrayElementOrDefault(object, k, 0, interop, importValue);
                 writeOwnNode.executeWithTargetAndIndexAndValue(obj, k, kValue);
             }
         }

graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/builtins/JavaBuiltins.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, 2023, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2024, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * The Universal Permissive License (UPL), Version 1.0
@@ -515,7 +515,7 @@ abstract static class JavaSuperNode extends JSBuiltinNode {
         protected Object superAdapter(Object adapter,
                         @CachedLibrary(limit = "InteropLibraryLimit") InteropLibrary interop,
                         @Cached ImportValueNode toJSType) {
-            return JSInteropUtil.readMemberOrDefault(adapter, Strings.SUPER, Undefined.instance, interop, toJSType, this);
+            return JSInteropUtil.readMemberOrDefault(adapter, Strings.SUPER, Undefined.instance, interop, toJSType);
         }
     }