Disallow declaring $isInstance methods that are private and cleanup…

… declarations in the JRE.

This is in preparation to allowing defining custom `$isInstance` methods in native types.

PiperOrigin-RevId: 561324465

jre/java/java/io/Serializable.java

@@ -25,7 +25,6 @@
  * The Java serialization protocol is explicitly not supported.
  */
 public interface Serializable {
-  // CHECKSTYLE_OFF: Utility methods.
   @JsMethod
   static boolean $isInstance(HasSerializableTypeMarker instance) {
     if (instance == null) {
@@ -40,5 +39,4 @@ public interface Serializable {
         // Arrays are implicitly instances of Serializable (JLS 10.7).
         || ArrayHelper.isArray(instance);
   }
-  // CHECKSTYLE_ON: end utility methods
 }

jre/java/java/lang/Boolean.java

@@ -106,10 +106,8 @@ public String toString() {
     return toString(booleanValue());
   }
 
-  // CHECKSTYLE_OFF: Utility Methods for unboxed Boolean.
   @JsMethod
-  protected static boolean $isInstance(Object instance) {
+  static boolean $isInstance(Object instance) {
     return "boolean".equals(JsUtils.typeOf(instance));
   }
-  //CHECKSTYLE_ON: End utility methods
 }

jre/java/java/lang/CharSequence.java

@@ -22,9 +22,7 @@
 import java.util.Spliterators;
 import java.util.stream.IntStream;
 import java.util.stream.StreamSupport;
-
 import javaemul.internal.JsUtils;
-
 import jsinterop.annotations.JsMethod;
 
 /**
@@ -60,7 +58,6 @@ public boolean hasNext() {
     }, Spliterator.SIZED | Spliterator.SUBSIZED | Spliterator.ORDERED, false);
   }
 
-  // CHECKSTYLE_OFF: Utility methods.
   @JsMethod
   static boolean $isInstance(HasCharSequenceTypeMarker instance) {
     if (JsUtils.typeOf(instance).equals("string")) {
@@ -69,5 +66,4 @@ public boolean hasNext() {
 
     return instance != null && instance.getTypeMarker() == true;
   }
-  // CHECKSTYLE_ON: end utility methods
 }

jre/java/java/lang/Cloneable.java

@@ -23,7 +23,6 @@
  */
 public interface Cloneable {
 
-  // CHECKSTYLE_OFF: Utility methods.
   @JsMethod
   static boolean $isInstance(HasCloneableTypeMarker instance) {
     if (instance == null) {
@@ -34,5 +33,4 @@ public interface Cloneable {
         // Arrays are implicitly instances of Cloneable (JLS 10.7).
         || ArrayHelper.isArray(instance);
   }
-  // CHECKSTYLE_ON: end utility methods
 }

jre/java/java/lang/Comparable.java

@@ -16,7 +16,6 @@
 package java.lang;
 
 import javaemul.internal.JsUtils;
-
 import jsinterop.annotations.JsMethod;
 
 /**
@@ -29,7 +28,6 @@
 public interface Comparable<T> {
   int compareTo(T other);
 
-  // CHECKSTYLE_OFF: Utility methods.
   @JsMethod
   static boolean $isInstance(HasComparableTypeMarker instance) {
     String type = JsUtils.typeOf(instance);
@@ -39,5 +37,4 @@ public interface Comparable<T> {
 
     return instance != null && instance.getTypeMarker() == true;
   }
-  // CHECKSTYLE_ON: end utility methods
 }

jre/java/java/lang/Double.java

@@ -182,10 +182,8 @@ public String toString() {
     return toString(doubleValue());
   }
 
-  // CHECKSTYLE_OFF: Utility Methods for unboxed Double.
   @JsMethod
-  protected static boolean $isInstance(Object instance) {
+  static boolean $isInstance(Object instance) {
     return "number".equals(JsUtils.typeOf(instance));
   }
-  //CHECKSTYLE_ON: End utility methods
 }

jre/java/java/lang/Number.java

@@ -32,9 +32,6 @@ public abstract class Number implements Serializable {
    */
   private static NativeRegExp floatRegex;
 
-  // CHECKSTYLE_OFF: A special need to use unusual identifiers to avoid
-  // introducing name collisions.
-
   static class __Decode {
     public final String payload;
     public final int radix;
@@ -132,7 +129,7 @@ static class __ParseLong {
   private static class JavaLangNumber { }
 
   @JsMethod
-  private static boolean $isInstance(Object instance) {
+  static boolean $isInstance(Object instance) {
     return "number".equals(JsUtils.typeOf(instance)) || instance instanceof JavaLangNumber;
   }
 
@@ -348,8 +345,6 @@ private static boolean __isValidDouble(String str) {
   @JsMethod(namespace = JsPackage.GLOBAL)
   private static native int parseInt(String s, int radix);
 
-  // CHECKSTYLE_ON
-
   public byte byteValue() {
     return (byte) intValue();
   }

jre/java/java/lang/Object.java

@@ -44,7 +44,7 @@ public final Class<?> getClass() {
   }
 
   @JsMethod
-  private static boolean $isInstance(Object instance) {
+  static boolean $isInstance(Object instance) {
     return instance != null;
   }
 }

jre/java/java/lang/String.java

@@ -694,10 +694,8 @@ private static class NativeString {
     public native String toUpperCase();
   }
 
-  // CHECKSTYLE_OFF: Utility Methods for unboxed String.
   @JsMethod
-  protected static boolean $isInstance(Object instance) {
+  static boolean $isInstance(Object instance) {
     return "string".equals(JsUtils.typeOf(instance));
   }
-  // CHECKSTYLE_ON: end utility methods
 }

transpiler/java/com/google/j2cl/transpiler/ast/DeclaredTypeDescriptor.java

@@ -243,6 +243,12 @@ public boolean hasOverlayImplementationType() {
     return getTypeDeclaration().hasOverlayImplementationType();
   }
 
+  @Memoized
+  public boolean hasCustomIsInstanceMethod() {
+    return getDeclaredMethodDescriptors().stream()
+        .anyMatch(MethodDescriptor::isCustomIsInstanceMethod);
+  }
+
   @Override
   public DeclaredTypeDescriptor toRawTypeDescriptor() {
     return getTypeDeclaration().toRawTypeDescriptor();

transpiler/java/com/google/j2cl/transpiler/ast/MemberDescriptor.java

@@ -184,6 +184,11 @@ public boolean isExtern() {
             && getJsNamespace().equals(getEnclosingTypeDescriptor().getQualifiedJsName()));
   }
 
+  /** Returns true if this is a user written $isInstance method. */
+  public boolean isCustomIsInstanceMethod() {
+    return false;
+  }
+
   /** Whether this member overrides a java.lang.Object method. */
   public boolean isOrOverridesJavaLangObjectMethod() {
     return false;

transpiler/java/com/google/j2cl/transpiler/ast/MethodDescriptor.java

@@ -617,6 +617,13 @@ public boolean isPropertySetter() {
 
   public abstract boolean isEnumSyntheticMethod();
 
+  @Override
+  public boolean isCustomIsInstanceMethod() {
+    return getOrigin() == MethodOrigin.SOURCE
+        && getParameterDescriptors().size() == 1
+        && getName().equals(IS_INSTANCE_METHOD_NAME);
+  }
+
   @Nullable
   public String getObjectiveCName() {
     KtObjcInfo ktObjcInfo = getKtObjcInfo();

transpiler/java/com/google/j2cl/transpiler/ast/Type.java

@@ -29,6 +29,7 @@
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
+import java.util.function.Predicate;
 import java.util.stream.Stream;
 import javax.annotation.Nullable;
 
@@ -63,7 +64,11 @@ public DeclaredTypeDescriptor getTypeDescriptor() {
   }
 
   public boolean containsMethod(String mangledName) {
-    return getMethods().stream().anyMatch(method -> method.getMangledName().equals(mangledName));
+    return containsMethod(method -> method.getMangledName().equals(mangledName));
+  }
+
+  public boolean containsMethod(Predicate<MethodDescriptor> methodPredicate) {
+    return getMethods().stream().map(Method::getDescriptor).anyMatch(methodPredicate);
   }
 
   public void setAbstract(boolean isAbstract) {

transpiler/java/com/google/j2cl/transpiler/backend/Backend.java

@@ -145,7 +145,7 @@
 import com.google.j2cl.transpiler.passes.PropagateNullabilityJ2kt;
 import com.google.j2cl.transpiler.passes.RecoverShortcutBooleanOperator;
 import com.google.j2cl.transpiler.passes.RemoveAssertStatements;
-import com.google.j2cl.transpiler.passes.RemoveIsInstanceMethods;
+import com.google.j2cl.transpiler.passes.RemoveCustomIsInstanceMethods;
 import com.google.j2cl.transpiler.passes.RemoveNativeTypes;
 import com.google.j2cl.transpiler.passes.RemoveNestedBlocks;
 import com.google.j2cl.transpiler.passes.RemoveNoopStatements;
@@ -454,10 +454,9 @@ public ImmutableList<Supplier<NormalizationPass>> getPassFactories(BackendOption
           // Needs to run at the end as the types in the ast will be invalid after the pass.
           ImplementArraysAsClasses::new,
           InsertExceptionConversionsWasm::new,
-
           NormalizeInstantiationThroughFactoryMethods::new,
           NormalizeNullLiterals::new,
-          RemoveIsInstanceMethods::new,
+          RemoveCustomIsInstanceMethods::new,
           RemoveNoopStatements::new,
           UpgradeInterfaceDispatch::new,
 
@@ -591,7 +590,7 @@ public ImmutableList<Supplier<NormalizationPass>> getPassFactories(BackendOption
           // TODO(b/283154656): Fork the instantiation code for the modular pipeline.
           // NormalizeInstantiationThroughFactoryMethods::new,
           NormalizeNullLiterals::new,
-          RemoveIsInstanceMethods::new,
+          RemoveCustomIsInstanceMethods::new,
           RemoveNoopStatements::new,
 
           // Post-verifications

transpiler/java/com/google/j2cl/transpiler/passes/AddAbstractMethodStubs.java

@@ -68,8 +68,8 @@ public void applyTo(Type type) {
         .filter(m -> m.getEnclosingTypeDescriptor().isInterface())
         .forEach(
             m -> {
-              if (type.containsMethod(m.getDeclarationDescriptor().getMangledName())) {
-                // The method is alread defined in the type; nothing to do.
+              if (type.containsMethod(m.getMangledName())) {
+                // The method is already defined in the type; nothing to do.
                 return;
               }
               type.addMember(

transpiler/java/com/google/j2cl/transpiler/passes/JsInteropRestrictionsChecker.java

@@ -1049,6 +1049,9 @@ private void checkMember(Member member) {
       if (!checkJsPropertyAccessor(method)) {
         return;
       }
+      if (method.getDescriptor().isCustomIsInstanceMethod()) {
+        checkCustomIsInstanceMethod(method);
+      }
     }
 
     if (memberDescriptor.canBeReferencedExternally()) {
@@ -1122,6 +1125,22 @@ private void checkJsAsyncMethod(Method method) {
         returnType.getReadableDescription());
   }
 
+  private void checkCustomIsInstanceMethod(Method method) {
+    MethodDescriptor methodDescriptor = method.getDescriptor();
+    if (methodDescriptor.isInstanceMember() || methodDescriptor.getVisibility().isPrivate()) {
+      problems.error(
+          method.getSourcePosition(),
+          "Custom '$isInstance' method '%s' has to be static and non private.",
+          method.getReadableDescription());
+    }
+    if (!TypeDescriptors.isPrimitiveBoolean(methodDescriptor.getReturnTypeDescriptor())) {
+      problems.error(
+          method.getSourcePosition(),
+          "Custom '$isInstance' method '%s' has to return 'boolean'.",
+          method.getReadableDescription());
+    }
+  }
+
   private void checkOverrideConsistency(Member member) {
     if (!member.isMethod() || !member.getDescriptor().isJsMember()) {
       return;

transpiler/java/com/google/j2cl/transpiler/passes/RemoveIsInstanceMethods.java → transpiler/java/com/google/j2cl/transpiler/passes/RemoveCustomIsInstanceMethods.java

@@ -15,24 +15,16 @@
  */
 package com.google.j2cl.transpiler.passes;
 
-import com.google.j2cl.transpiler.ast.Member;
-import com.google.j2cl.transpiler.ast.MethodDescriptor;
 import com.google.j2cl.transpiler.ast.Type;
 
 /**
- * Removes {@code $isInstance} support methods. {@code $isInstance} is unused in Wasm, and the
+ * Removes user written {@code $isInstance} methods. {@code $isInstance} is unused in Wasm, and the
  * explicit implementations in our JRE in {@code Comparable}, etc use a native type in a way that is
  * unsupported in Wasm.
  */
-public class RemoveIsInstanceMethods extends NormalizationPass {
-
+public class RemoveCustomIsInstanceMethods extends NormalizationPass {
   @Override
   public void applyTo(Type type) {
-    type.getMembers().removeIf(RemoveIsInstanceMethods::isIsInstanceMethod);
-  }
-
-  private static boolean isIsInstanceMethod(Member member) {
-    return member.isMethod()
-        && member.getDescriptor().getName().equals(MethodDescriptor.IS_INSTANCE_METHOD_NAME);
+    type.getMembers().removeIf(m -> m.getDescriptor().isCustomIsInstanceMethod());
   }
 }

transpiler/javatests/com/google/j2cl/transpiler/JsInteropRestrictionsCheckerTest.java

@@ -3342,6 +3342,69 @@ public void testJsAsyncFails() {
                 + " returns 'double'.");
   }
 
+  public void testCustomIsInstanceSucceeds() {
+    assertTranspileSucceeds(
+            "test.Buggy",
+            "import jsinterop.annotations.*;",
+            "interface InterfaceWithCustomIsInstance {",
+            "  public static boolean $isInstance(Object o) { return true; }",
+            "}",
+            "class ClassWithCustomIsInstance {",
+            "  static boolean $isInstance(Object o) { return true; }",
+            "}",
+            "@JsType(isNative = true)",
+            "interface NativeInterfaceWithCustomIsInstance {",
+            "  @JsOverlay",
+            "  static boolean $isInstance(Object o) { return true; }",
+            "}",
+            "@JsType(isNative = true)",
+            "class NativeClassWithCustomIsInstance {",
+            "  @JsOverlay",
+            "  protected static boolean $isInstance(Object o) { return true; }",
+            "}",
+            "@JsType(isNative = true)",
+            "class ClassWithNativeIsInstance {",
+            "  static native boolean $isInstance(Object o);",
+            "}",
+            "class Buggy {",
+            "  static void main() {",
+            "    Object o = null;",
+            "    boolean b = o instanceof InterfaceWithCustomIsInstance;",
+            "    b = o instanceof ClassWithCustomIsInstance;",
+            "    b = o instanceof NativeClassWithCustomIsInstance;",
+            "  }",
+            "}")
+        .assertNoWarnings();
+  }
+
+  public void testCustomIsInstanceFails() {
+    assertTranspileFails(
+            "test.Buggy",
+            "import jsinterop.annotations.*;",
+            "interface BadIsInstanceVisibility {",
+            "  private static boolean $isInstance(Object o) { return true; }",
+            "}",
+            "class BadIsInstanceReturnType {",
+            "  static void $isInstance(Object o) { }",
+            "}",
+            "class BadIsInstanceMembership {",
+            "  boolean $isInstance(Object o) { return true; }",
+            "}",
+            "@JsType(isNative = true)",
+            "class BadIsInstanceOnNativeType {",
+            "  static boolean $isInstance(Object o) { return true; }",
+            "}")
+        .assertErrorsWithoutSourcePosition(
+            "Custom '$isInstance' method 'boolean BadIsInstanceVisibility.$isInstance(Object o)'"
+                + " has to be static and non private.",
+            "Custom '$isInstance' method 'void BadIsInstanceReturnType.$isInstance(Object o)' has"
+                + " to return 'boolean'.",
+            "Custom '$isInstance' method 'boolean BadIsInstanceMembership.$isInstance(Object o)'"
+                + " has to be static and non private.",
+            "Native JsType method 'boolean BadIsInstanceOnNativeType.$isInstance(Object o)' should"
+                + " be native, abstract or JsOverlay.");
+  }
+
   public void testUnusableByJsSuppressionSucceeds() {
     assertTranspileSucceeds(
             "test.Buggy",