From db9ec79acc57c0294d2e58f9e1d6e12c147f7eb8 Mon Sep 17 00:00:00 2001 From: Roberto Lublinerman Date: Wed, 30 Aug 2023 21:04:07 -0700 Subject: [PATCH] [JS] Allow customization of `$isInstance` for native types. This allows `instanceof` on native interfaces if they define a custom `$isInstance` method. The custom `$isInstance` method cannot be private. PiperOrigin-RevId: 561531763 --- jre/java/java/io/Serializable.java | 2 - jre/java/java/lang/Boolean.java | 2 - jre/java/java/lang/CharSequence.java | 2 - jre/java/java/lang/Cloneable.java | 3 - jre/java/java/lang/Comparable.java | 2 - jre/java/java/lang/Double.java | 2 - jre/java/java/lang/Number.java | 1 - jre/java/java/lang/Object.java | 1 - jre/java/java/lang/String.java | 2 - .../j2cl/tools/rta/RapidTypeAnalyser.java | 7 ++ .../j2cl/transpiler/ast/MethodDescriptor.java | 10 +- .../passes/ImplementInstanceOfs.java | 2 +- .../passes/JsInteropRestrictionsChecker.java | 4 +- .../java/jsinteroptests/JsTypeTest.java | 91 +++++++++++++++++++ .../JsInteropRestrictionsCheckerTest.java | 1 + 15 files changed, 108 insertions(+), 24 deletions(-) diff --git a/jre/java/java/io/Serializable.java b/jre/java/java/io/Serializable.java index 2a87e172da..dbfd3bb25b 100644 --- a/jre/java/java/io/Serializable.java +++ b/jre/java/java/io/Serializable.java @@ -17,7 +17,6 @@ import javaemul.internal.ArrayHelper; import javaemul.internal.JsUtils; -import jsinterop.annotations.JsMethod; /** * Provided for interoperability; RPC treats this interface synonymously with @@ -25,7 +24,6 @@ * The Java serialization protocol is explicitly not supported. */ public interface Serializable { - @JsMethod static boolean $isInstance(HasSerializableTypeMarker instance) { if (instance == null) { return false; diff --git a/jre/java/java/lang/Boolean.java b/jre/java/java/lang/Boolean.java index 4f27076d10..9213b9911b 100644 --- a/jre/java/java/lang/Boolean.java +++ b/jre/java/java/lang/Boolean.java @@ -19,7 +19,6 @@ import java.io.Serializable; import javaemul.internal.JsUtils; import javaemul.internal.Platform; -import jsinterop.annotations.JsMethod; /** * Wraps native boolean as an object. @@ -106,7 +105,6 @@ public String toString() { return toString(booleanValue()); } - @JsMethod static boolean $isInstance(Object instance) { return "boolean".equals(JsUtils.typeOf(instance)); } diff --git a/jre/java/java/lang/CharSequence.java b/jre/java/java/lang/CharSequence.java index 8bec7288ca..aba2261d6a 100644 --- a/jre/java/java/lang/CharSequence.java +++ b/jre/java/java/lang/CharSequence.java @@ -23,7 +23,6 @@ import java.util.stream.IntStream; import java.util.stream.StreamSupport; import javaemul.internal.JsUtils; -import jsinterop.annotations.JsMethod; /** * Abstracts the notion of a sequence of characters. @@ -58,7 +57,6 @@ public boolean hasNext() { }, Spliterator.SIZED | Spliterator.SUBSIZED | Spliterator.ORDERED, false); } - @JsMethod static boolean $isInstance(HasCharSequenceTypeMarker instance) { if (JsUtils.typeOf(instance).equals("string")) { return true; diff --git a/jre/java/java/lang/Cloneable.java b/jre/java/java/lang/Cloneable.java index 2e87658965..1eb325e2bf 100644 --- a/jre/java/java/lang/Cloneable.java +++ b/jre/java/java/lang/Cloneable.java @@ -16,14 +16,11 @@ package java.lang; import javaemul.internal.ArrayHelper; -import jsinterop.annotations.JsMethod; /** * Indicates that a class implements clone(). */ public interface Cloneable { - - @JsMethod static boolean $isInstance(HasCloneableTypeMarker instance) { if (instance == null) { return false; diff --git a/jre/java/java/lang/Comparable.java b/jre/java/java/lang/Comparable.java index 3642be2d0d..08392414d9 100644 --- a/jre/java/java/lang/Comparable.java +++ b/jre/java/java/lang/Comparable.java @@ -16,7 +16,6 @@ package java.lang; import javaemul.internal.JsUtils; -import jsinterop.annotations.JsMethod; /** * An interface used a basis for implementing custom ordering. { int compareTo(T other); - @JsMethod static boolean $isInstance(HasComparableTypeMarker instance) { String type = JsUtils.typeOf(instance); if (type.equals("boolean") || type.equals("number") || type.equals("string")) { diff --git a/jre/java/java/lang/Double.java b/jre/java/java/lang/Double.java index 7ecda9b8d2..9da9b8f2a9 100644 --- a/jre/java/java/lang/Double.java +++ b/jre/java/java/lang/Double.java @@ -18,7 +18,6 @@ import javaemul.internal.JsUtils; import javaemul.internal.Platform; -import jsinterop.annotations.JsMethod; /** * Wraps a primitive double as an object. @@ -182,7 +181,6 @@ public String toString() { return toString(doubleValue()); } - @JsMethod static boolean $isInstance(Object instance) { return "number".equals(JsUtils.typeOf(instance)); } diff --git a/jre/java/java/lang/Number.java b/jre/java/java/lang/Number.java index 5585c1c3b3..5d63eac4a3 100644 --- a/jre/java/java/lang/Number.java +++ b/jre/java/java/lang/Number.java @@ -128,7 +128,6 @@ static class __ParseLong { @JsType(isNative = true, name = "Number$impl", namespace = "java.lang") private static class JavaLangNumber { } - @JsMethod static boolean $isInstance(Object instance) { return "number".equals(JsUtils.typeOf(instance)) || instance instanceof JavaLangNumber; } diff --git a/jre/java/java/lang/Object.java b/jre/java/java/lang/Object.java index 4218175ca8..5b891253d8 100644 --- a/jre/java/java/lang/Object.java +++ b/jre/java/java/lang/Object.java @@ -43,7 +43,6 @@ public final Class getClass() { return Class.$get(Constructor.of(this)); } - @JsMethod static boolean $isInstance(Object instance) { return instance != null; } diff --git a/jre/java/java/lang/String.java b/jre/java/java/lang/String.java index 02dcb84964..94f8fa0bc4 100644 --- a/jre/java/java/lang/String.java +++ b/jre/java/java/lang/String.java @@ -34,7 +34,6 @@ import javaemul.internal.JsUtils; import javaemul.internal.NativeRegExp; import javaemul.internal.StringUtil; -import jsinterop.annotations.JsMethod; import jsinterop.annotations.JsNonNull; import jsinterop.annotations.JsPackage; import jsinterop.annotations.JsProperty; @@ -694,7 +693,6 @@ private static class NativeString { public native String toUpperCase(); } - @JsMethod static boolean $isInstance(Object instance) { return "string".equals(JsUtils.typeOf(instance)); } diff --git a/tools/java/com/google/j2cl/tools/rta/RapidTypeAnalyser.java b/tools/java/com/google/j2cl/tools/rta/RapidTypeAnalyser.java index a1ba734be2..6b6b965fe3 100644 --- a/tools/java/com/google/j2cl/tools/rta/RapidTypeAnalyser.java +++ b/tools/java/com/google/j2cl/tools/rta/RapidTypeAnalyser.java @@ -114,6 +114,13 @@ private static void markTypeLive(Type type) { // When a type is marked as live, we need to explicitly mark the super interfaces as live since // we need markImplementor call (which are not tracked in AST). type.getSuperInterfaces().forEach(RapidTypeAnalyser::markTypeLive); + + // Types are made live by `instanceof` and casts, so if the type has a custom $isInstance + // it should be also considered as if it was called. + Member isInstanceMember = type.getMemberByName("$isInstance"); + if (isInstanceMember != null) { + onMemberReference(isInstanceMember); + } } private RapidTypeAnalyser() {} diff --git a/transpiler/java/com/google/j2cl/transpiler/ast/MethodDescriptor.java b/transpiler/java/com/google/j2cl/transpiler/ast/MethodDescriptor.java index 0188db851d..17a9698120 100644 --- a/transpiler/java/com/google/j2cl/transpiler/ast/MethodDescriptor.java +++ b/transpiler/java/com/google/j2cl/transpiler/ast/MethodDescriptor.java @@ -175,15 +175,15 @@ public boolean isSyntheticInstanceOfSupportMember() { } public static final String CONSTRUCTOR_METHOD_NAME = ""; - public static final String INIT_METHOD_NAME = "$init"; + static final String INIT_METHOD_NAME = "$init"; public static final String CTOR_METHOD_PREFIX = "$ctor"; public static final String CLINIT_METHOD_NAME = "$clinit"; public static final String VALUE_OF_METHOD_NAME = "valueOf"; // Boxed type valueOf() method. - public static final String IS_INSTANCE_METHOD_NAME = "$isInstance"; + static final String IS_INSTANCE_METHOD_NAME = "$isInstance"; public static final String MARK_IMPLEMENTOR_METHOD_NAME = "$markImplementor"; public static final String CREATE_METHOD_NAME = "$create"; - public static final String LOAD_MODULES_METHOD_NAME = "$loadModules"; - public static final String COPY_METHOD_NAME = "$copy"; + static final String LOAD_MODULES_METHOD_NAME = "$loadModules"; + static final String COPY_METHOD_NAME = "$copy"; public static String buildMethodSignature( String name, TypeDescriptor... parameterTypeDescriptors) { @@ -683,7 +683,7 @@ public String getMangledName() { return getSimpleJsName(); } - if (getOrigin().isSyntheticInstanceOfSupportMember()) { + if (getOrigin().isSyntheticInstanceOfSupportMember() || isCustomIsInstanceMethod()) { // Class support methods, like $isInstance and $markImplementor, should not be mangled. return getName(); } diff --git a/transpiler/java/com/google/j2cl/transpiler/passes/ImplementInstanceOfs.java b/transpiler/java/com/google/j2cl/transpiler/passes/ImplementInstanceOfs.java index a1b1637f0b..d0bbca6e3c 100644 --- a/transpiler/java/com/google/j2cl/transpiler/passes/ImplementInstanceOfs.java +++ b/transpiler/java/com/google/j2cl/transpiler/passes/ImplementInstanceOfs.java @@ -108,7 +108,7 @@ private static ExpressionStatement createMarkImplementorCall( /** Synthesizes the $isInstance method on the type. */ private static void synthesizeIsInstanceMethod(Type type) { - if (type.containsMethod(MethodDescriptor.IS_INSTANCE_METHOD_NAME)) { + if (type.containsMethod(MethodDescriptor::isCustomIsInstanceMethod)) { // User provided an $isInstance method, skip generation. return; } diff --git a/transpiler/java/com/google/j2cl/transpiler/passes/JsInteropRestrictionsChecker.java b/transpiler/java/com/google/j2cl/transpiler/passes/JsInteropRestrictionsChecker.java index eb1d20e3f9..75f7bfa5a4 100644 --- a/transpiler/java/com/google/j2cl/transpiler/passes/JsInteropRestrictionsChecker.java +++ b/transpiler/java/com/google/j2cl/transpiler/passes/JsInteropRestrictionsChecker.java @@ -909,7 +909,9 @@ private void checkTypeReferences(Type type) { @Override public void exitInstanceOfExpression(InstanceOfExpression instanceOfExpression) { TypeDescriptor testTypeDescriptor = instanceOfExpression.getTestTypeDescriptor(); - if (testTypeDescriptor.isNative() && testTypeDescriptor.isInterface()) { + if (testTypeDescriptor.isNative() + && testTypeDescriptor.isInterface() + && !((DeclaredTypeDescriptor) testTypeDescriptor).hasCustomIsInstanceMethod()) { problems.error( instanceOfExpression.getSourcePosition(), "Cannot do instanceof against native JsType interface '%s'.", diff --git a/transpiler/javatests/com/google/j2cl/integration/java/jsinteroptests/JsTypeTest.java b/transpiler/javatests/com/google/j2cl/integration/java/jsinteroptests/JsTypeTest.java index 9193e1d935..521aa668a3 100644 --- a/transpiler/javatests/com/google/j2cl/integration/java/jsinteroptests/JsTypeTest.java +++ b/transpiler/javatests/com/google/j2cl/integration/java/jsinteroptests/JsTypeTest.java @@ -26,6 +26,7 @@ import jsinterop.annotations.JsConstructor; import jsinterop.annotations.JsFunction; import jsinterop.annotations.JsMethod; +import jsinterop.annotations.JsOverlay; import jsinterop.annotations.JsPackage; import jsinterop.annotations.JsProperty; import jsinterop.annotations.JsType; @@ -47,6 +48,8 @@ public static void testAll() { testInstanceOf_jsoWithNativeButtonProto(); testInstanceOf_jsoWithoutProto(); testInstanceOf_jsoWithProto(); + testInstanceOf_classWithCustomIsInstance(); + testInstanceOf_interfaceWithCustomIsInstance(); testInstanceOf_withNameSpace(); testJsMethodWithDifferentVisiblities(); testJsTypeField(); @@ -74,6 +77,38 @@ static class MyNativeJsTypeSubclass extends MyNativeJsType { public MyNativeJsTypeSubclass() {} } + @JsType(isNative = true, namespace = JsPackage.GLOBAL, name = "Array") + static class MyNativeClassWithCustomIsInstance { + @JsOverlay + static boolean $isInstance(Object o) { + return isCustomIsInstanceClassSingleton(o); + } + } + + private static final String CUSTOM_IS_INSTANCE_CLASS_SINGLETON = "CustomIsInstanceClass"; + + // This method was extracted from $isInstance and ONLY called from there to ensure that + // rta traverses custom isInstance methods. + private static boolean isCustomIsInstanceClassSingleton(Object o) { + return o == CUSTOM_IS_INSTANCE_CLASS_SINGLETON; + } + + @JsType(isNative = true, namespace = JsPackage.GLOBAL, name = "Array") + interface MyNativeInterfaceWithCustomIsInstance { + @JsOverlay + static boolean $isInstance(Object o) { + return isCustomIsInstanceInterfaceSingleton(o); + } + } + + private static final String CUSTOM_IS_INSTANCE_INTERFACE_SINGLETON = "CustomIsInstanceInterface"; + + // This method was extracted from $isInstance and ONLY called from there to ensure that + // rta traverses custom isInstance methods. + private static boolean isCustomIsInstanceInterfaceSingleton(Object o) { + return o == CUSTOM_IS_INSTANCE_INTERFACE_SINGLETON; + } + static class MyNativeJsTypeSubclassWithIterator extends MyNativeJsType implements Iterable { @JsConstructor public MyNativeJsTypeSubclassWithIterator() {} @@ -262,6 +297,8 @@ private static void testInstanceOf_jsoWithProto() { assertFalse(object instanceof Iterator); assertTrue(object instanceof MyNativeJsType); assertFalse(object instanceof MyNativeJsTypeInterfaceImpl); + assertFalse(object instanceof MyNativeClassWithCustomIsInstance); + assertFalse(object instanceof MyNativeInterfaceWithCustomIsInstance); assertFalse(object instanceof ElementLikeNativeInterfaceImpl); assertFalse(object instanceof MyJsInterfaceWithOnlyInstanceofReference); assertTrue(object instanceof AliasToMyNativeJsTypeWithOnlyInstanceofReference); @@ -280,6 +317,8 @@ private static void testInstanceOf_jsoWithoutProto() { assertFalse(object instanceof Iterator); assertFalse(object instanceof MyNativeJsType); assertFalse(object instanceof MyNativeJsTypeInterfaceImpl); + assertFalse(object instanceof MyNativeClassWithCustomIsInstance); + assertFalse(object instanceof MyNativeInterfaceWithCustomIsInstance); assertFalse(object instanceof ElementLikeNativeInterfaceImpl); assertFalse(object instanceof MyJsInterfaceWithOnlyInstanceofReference); assertFalse(object instanceof AliasToMyNativeJsTypeWithOnlyInstanceofReference); @@ -298,6 +337,8 @@ private static void testInstanceOf_jsoWithNativeButtonProto() { assertFalse(object instanceof Iterator); assertFalse(object instanceof MyNativeJsType); assertFalse(object instanceof MyNativeJsTypeInterfaceImpl); + assertFalse(object instanceof MyNativeClassWithCustomIsInstance); + assertFalse(object instanceof MyNativeInterfaceWithCustomIsInstance); assertFalse(object instanceof ElementLikeNativeInterfaceImpl); assertFalse(object instanceof MyJsInterfaceWithOnlyInstanceofReference); assertFalse(object instanceof AliasToMyNativeJsTypeWithOnlyInstanceofReference); @@ -317,6 +358,8 @@ private static void testInstanceOf_implementsJsType() { assertFalse(object instanceof Iterator); assertFalse(object instanceof MyNativeJsType); assertFalse(object instanceof MyNativeJsTypeInterfaceImpl); + assertFalse(object instanceof MyNativeClassWithCustomIsInstance); + assertFalse(object instanceof MyNativeInterfaceWithCustomIsInstance); assertTrue(object instanceof ElementLikeNativeInterfaceImpl); assertFalse(object instanceof MyJsInterfaceWithOnlyInstanceofReference); assertFalse(object instanceof AliasToMyNativeJsTypeWithOnlyInstanceofReference); @@ -336,6 +379,8 @@ private static void testInstanceOf_implementsJsTypeWithPrototype() { assertFalse(object instanceof Iterator); assertFalse(object instanceof MyNativeJsType); assertTrue(object instanceof MyNativeJsTypeInterfaceImpl); + assertFalse(object instanceof MyNativeClassWithCustomIsInstance); + assertFalse(object instanceof MyNativeInterfaceWithCustomIsInstance); assertFalse(object instanceof ElementLikeNativeInterfaceImpl); assertFalse(object instanceof MyJsInterfaceWithOnlyInstanceofReference); assertFalse(object instanceof AliasToMyNativeJsTypeWithOnlyInstanceofReference); @@ -355,6 +400,8 @@ private static void testInstanceOf_concreteJsType() { assertFalse(object instanceof Iterator); assertFalse(object instanceof MyNativeJsType); assertFalse(object instanceof MyNativeJsTypeInterfaceImpl); + assertFalse(object instanceof MyNativeClassWithCustomIsInstance); + assertFalse(object instanceof MyNativeInterfaceWithCustomIsInstance); assertFalse(object instanceof ElementLikeNativeInterfaceImpl); assertFalse(object instanceof MyJsInterfaceWithOnlyInstanceofReference); assertFalse(object instanceof AliasToMyNativeJsTypeWithOnlyInstanceofReference); @@ -376,6 +423,8 @@ private static void testInstanceOf_extendsJsTypeWithProto() { assertFalse(object instanceof HTMLButtonElementConcreteNativeJsType); assertTrue(object instanceof Iterable); assertFalse(object instanceof MyNativeJsTypeInterfaceImpl); + assertFalse(object instanceof MyNativeClassWithCustomIsInstance); + assertFalse(object instanceof MyNativeInterfaceWithCustomIsInstance); assertFalse(object instanceof ElementLikeNativeInterfaceImpl); assertFalse(object instanceof MyJsInterfaceWithOnlyInstanceofReference); assertTrue(object instanceof AliasToMyNativeJsTypeWithOnlyInstanceofReference); @@ -384,6 +433,48 @@ private static void testInstanceOf_extendsJsTypeWithProto() { assertFalse(object instanceof MyNativeJsTypeInterfaceImpl[][]); } + private static void testInstanceOf_classWithCustomIsInstance() { + Object object = CUSTOM_IS_INSTANCE_CLASS_SINGLETON; + + assertTrue(object instanceof Object); + assertTrue(object instanceof String); + assertFalse(object instanceof HTMLElementConcreteNativeJsType); + assertFalse(object instanceof HTMLElementAnotherConcreteNativeJsType); + assertFalse(object instanceof HTMLButtonElementConcreteNativeJsType); + assertFalse(object instanceof Iterator); + assertFalse(object instanceof MyNativeJsType); + assertFalse(object instanceof MyNativeJsTypeInterfaceImpl); + assertTrue(object instanceof MyNativeClassWithCustomIsInstance); + assertFalse(object instanceof MyNativeInterfaceWithCustomIsInstance); + assertFalse(object instanceof ElementLikeNativeInterfaceImpl); + assertFalse(object instanceof MyJsInterfaceWithOnlyInstanceofReference); + assertFalse(object instanceof AliasToMyNativeJsTypeWithOnlyInstanceofReference); + assertFalse(object instanceof ConcreteJsType); + assertFalse(object instanceof MyNativeJsTypeInterface[]); + assertFalse(object instanceof MyNativeJsTypeInterfaceImpl[][]); + } + + private static void testInstanceOf_interfaceWithCustomIsInstance() { + Object object = CUSTOM_IS_INSTANCE_INTERFACE_SINGLETON; + + assertTrue(object instanceof Object); + assertTrue(object instanceof String); + assertFalse(object instanceof HTMLElementConcreteNativeJsType); + assertFalse(object instanceof HTMLElementAnotherConcreteNativeJsType); + assertFalse(object instanceof HTMLButtonElementConcreteNativeJsType); + assertFalse(object instanceof Iterator); + assertFalse(object instanceof MyNativeJsType); + assertFalse(object instanceof MyNativeJsTypeInterfaceImpl); + assertFalse(object instanceof MyNativeClassWithCustomIsInstance); + assertTrue(object instanceof MyNativeInterfaceWithCustomIsInstance); + assertFalse(object instanceof ElementLikeNativeInterfaceImpl); + assertFalse(object instanceof MyJsInterfaceWithOnlyInstanceofReference); + assertFalse(object instanceof AliasToMyNativeJsTypeWithOnlyInstanceofReference); + assertFalse(object instanceof ConcreteJsType); + assertFalse(object instanceof MyNativeJsTypeInterface[]); + assertFalse(object instanceof MyNativeJsTypeInterfaceImpl[][]); + } + private static void testInstanceOf_withNameSpace() { Object obj1 = createMyNamespacedJsInterface(); diff --git a/transpiler/javatests/com/google/j2cl/transpiler/JsInteropRestrictionsCheckerTest.java b/transpiler/javatests/com/google/j2cl/transpiler/JsInteropRestrictionsCheckerTest.java index 9d135fb693..a526930e87 100644 --- a/transpiler/javatests/com/google/j2cl/transpiler/JsInteropRestrictionsCheckerTest.java +++ b/transpiler/javatests/com/google/j2cl/transpiler/JsInteropRestrictionsCheckerTest.java @@ -3371,6 +3371,7 @@ public void testCustomIsInstanceSucceeds() { " Object o = null;", " boolean b = o instanceof InterfaceWithCustomIsInstance;", " b = o instanceof ClassWithCustomIsInstance;", + " b = o instanceof NativeInterfaceWithCustomIsInstance;", " b = o instanceof NativeClassWithCustomIsInstance;", " }", "}")