[J2KT] Remove JavaCollection/JavaList/JavaSet bridge types

On JVM/J2CL, these are now replaced with typealiases of the plain Kotlin `MutableCollection/MutableList/MutableSet` interfaces. On Kotlin Native, they are replaced with the emulated `java.util.Collection/List/Set` interfaces which are sub-interfaces of the Kotlin interfaces. The `toArray` methods are not visible on the interfaces in JVM/J2CL so we need to suppress warnings regarding the `override` modifier (but only for direct implementations of the interface methods. The much more common case of overriding AbstractCollection#toArray does not have this requirement). As a result, binaries (.class/.js/.klib) will all use java.util.Collection (etc.) as the implemented/extended interface. PiperOrigin-RevId: 712572687
google · Jan 6, 2025 · b7195d4 · b7195d4
1 parent 2e593ec
commit b7195d4
Show file tree

Hide file tree

Showing 6 changed files with 69 additions and 15 deletions.
diff --git a/transpiler/java/com/google/j2cl/transpiler/ast/Method.java b/transpiler/java/com/google/j2cl/transpiler/ast/Method.java
@@ -42,21 +42,25 @@ public class Method extends Member implements MethodLike {
   private final String wasmExportName;
   @Nullable private Boolean isForcedJavaOverride;
 
+  private boolean hasSuppressNothingToOverrideAnnotation;
+
   private Method(
       SourcePosition sourcePosition,
       MethodDescriptor methodDescriptor,
       List<Variable> parameters,
       Block body,
       String jsDocDescription,
       String wasmExportName,
-      @Nullable Boolean isForcedJavaOverride) {
+      @Nullable Boolean isForcedJavaOverride,
+      boolean hasSuppressNothingToOverrideAnnotation) {
     super(sourcePosition);
     this.methodDescriptor = checkNotNull(methodDescriptor);
     this.parameters.addAll(checkNotNull(parameters));
     this.jsDocDescription = jsDocDescription;
     this.wasmExportName = wasmExportName;
     this.body = checkNotNull(body);
     this.isForcedJavaOverride = isForcedJavaOverride;
+    this.hasSuppressNothingToOverrideAnnotation = hasSuppressNothingToOverrideAnnotation;
   }
 
   @Override
@@ -128,6 +132,15 @@ public final boolean isJavaOverride() {
     return isForcedJavaOverride != null ? isForcedJavaOverride : methodDescriptor.isJavaOverride();
   }
 
+  public boolean hasSuppressNothingToOverrideAnnotation() {
+    return hasSuppressNothingToOverrideAnnotation;
+  }
+
+  public void setHasSuppressNothingToOverrideAnnotation(
+      boolean hasSuppressNothingToOverrideAnnotation) {
+    this.hasSuppressNothingToOverrideAnnotation = hasSuppressNothingToOverrideAnnotation;
+  }
+
   public static Builder newBuilder() {
     return new Builder();
   }
@@ -196,6 +209,7 @@ public static class Builder {
     private SourcePosition bodySourcePosition;
     private SourcePosition sourcePosition;
     @Nullable private Boolean isForcedJavaOverride;
+    private boolean hasSuppressNothingToOverrideAnnotation;
 
     public static Builder from(Method method) {
       Builder builder = new Builder();
@@ -207,6 +221,8 @@ public static Builder from(Method method) {
       builder.bodySourcePosition = method.getBody().getSourcePosition();
       builder.sourcePosition = method.getSourcePosition();
       builder.isForcedJavaOverride = method.isForcedJavaOverride();
+      builder.hasSuppressNothingToOverrideAnnotation =
+          method.hasSuppressNothingToOverrideAnnotation;
       return builder;
     }
 
@@ -311,6 +327,13 @@ public Builder setForcedJavaOverride(@Nullable Boolean isForcedJavaOverride) {
       return this;
     }
 
+    @CanIgnoreReturnValue
+    public Builder setSuppressNothingToOverrideAnnotation(
+        boolean suppressNothingToOverrideAnnotation) {
+      hasSuppressNothingToOverrideAnnotation = suppressNothingToOverrideAnnotation;
+      return this;
+    }
+
     public Method build() {
       Block body =
           Block.newBuilder()
@@ -327,7 +350,8 @@ public Method build() {
           body,
           jsDocDescription,
           wasmExportName,
-          isForcedJavaOverride);
+          isForcedJavaOverride,
+          hasSuppressNothingToOverrideAnnotation);
     }
   }
 }
diff --git a/transpiler/java/com/google/j2cl/transpiler/backend/kotlin/MemberRenderer.kt b/transpiler/java/com/google/j2cl/transpiler/backend/kotlin/MemberRenderer.kt
@@ -54,6 +54,7 @@ import com.google.j2cl.transpiler.backend.kotlin.source.Source.Companion.block
 import com.google.j2cl.transpiler.backend.kotlin.source.Source.Companion.colonSeparated
 import com.google.j2cl.transpiler.backend.kotlin.source.Source.Companion.commaSeparated
 import com.google.j2cl.transpiler.backend.kotlin.source.Source.Companion.emptyLineSeparated
+import com.google.j2cl.transpiler.backend.kotlin.source.Source.Companion.emptyUnless
 import com.google.j2cl.transpiler.backend.kotlin.source.Source.Companion.inParentheses
 import com.google.j2cl.transpiler.backend.kotlin.source.Source.Companion.indented
 import com.google.j2cl.transpiler.backend.kotlin.source.Source.Companion.indentedIf
@@ -228,14 +229,23 @@ internal data class MemberRenderer(val nameRenderer: NameRenderer, val enclosing
       Source.emptyUnless(method.isJavaOverride) { KotlinSource.OVERRIDE_KEYWORD },
     )
 
-  fun annotationsSource(method: Method, methodObjCNames: MethodObjCNames?) =
+  fun annotationsSource(method: Method, methodObjCNames: MethodObjCNames?): Source =
     newLineSeparated(
       objCNameRenderer.objCAnnotationSource(method.descriptor, methodObjCNames),
       jsInteropAnnotationRenderer.jsInteropAnnotationsSource(method),
       memberDescriptorRenderer.jvmThrowsAnnotationSource(method.descriptor),
       memberDescriptorRenderer.nativeThrowsAnnotationSource(method.descriptor),
+      suppressNothingToOverrideSource(method),
     )
 
+  private fun suppressNothingToOverrideSource(method: Method): Source =
+    emptyUnless(method.hasSuppressNothingToOverrideAnnotation()) {
+      return annotation(
+        memberDescriptorRenderer.nameRenderer.topLevelQualifiedNameSource("kotlin.Suppress"),
+        KotlinSource.literal("NOTHING_TO_OVERRIDE"),
+      )
+    }
+
   fun methodParametersSource(method: MethodLike, objCParameterNames: List<String>? = null): Source {
     val methodDescriptor = method.descriptor
     val parameterDescriptors = methodDescriptor.parameterDescriptors

diff --git a/transpiler/java/com/google/j2cl/transpiler/passes/FixJavaKotlinMethodOverrideMismatch.java b/transpiler/java/com/google/j2cl/transpiler/passes/FixJavaKotlinMethodOverrideMismatch.java
@@ -30,11 +30,17 @@ public void applyTo(CompilationUnit compilationUnit) {
         new AbstractVisitor() {
           @Override
           public void exitMethod(Method method) {
-            // In Java clone() is magical: it is not declared in Cloneable but declared in Object
-            // and only really implemented if the interface Cloneable is implemented. In Kotlin it
-            // is explicitly declared in the Cloneable interface but not in Any.
             if (directlyOverridesJavaObjectClone(method.getDescriptor())) {
+              // In Java clone() is magical: it is not declared in Cloneable but declared in Object
+              // and only really implemented if the interface Cloneable is implemented. In Kotlin it
+              // is explicitly declared in the Cloneable interface but not in Any.
               method.setForcedJavaOverride(false);
+            } else if (directlyOverridesJavaUtilCollectionToArray(method.getDescriptor())) {
+              // In Kotlin JVM, (Mutable)Collection#toArray are not visible, so overrides cannot
+              // have the override modifier. On Kotlin Native, the emulated method is a regular
+              // method and overrides need the modifier. Mark the method for special handling in
+              // the renderer.
+              method.setHasSuppressNothingToOverrideAnnotation(true);
             }
           }
         });
@@ -50,4 +56,14 @@ private static boolean directlyOverridesJavaObjectClone(MethodDescriptor methodD
                 it ->
                     it.getDeclarationDescriptor().isMemberOf(TypeDescriptors.get().javaLangObject));
   }
+
+  private static boolean directlyOverridesJavaUtilCollectionToArray(
+      MethodDescriptor methodDescriptor) {
+    return methodDescriptor.getName().equals("toArray")
+        && methodDescriptor.getJavaOverriddenMethodDescriptors().stream()
+            .allMatch(
+                it ->
+                    it.getDeclarationDescriptor()
+                        .isMemberOf(TypeDescriptors.get().javaUtilCollection));
+  }
 }
diff --git a/transpiler/javatests/com/google/j2cl/readable/java/j2kt/output_kt/Collections.kt.txt b/transpiler/javatests/com/google/j2cl/readable/java/j2kt/output_kt/Collections.kt.txt
@@ -25,8 +25,8 @@ import java.util.AbstractCollection
 import java.util.AbstractList
 import java.util.AbstractMap
 import java.util.Spliterator
-import javaemul.lang.JavaCollection
-import javaemul.lang.JavaList
+import javaemul.lang.MutableCollectionJvm
+import javaemul.lang.MutableListJvm
 import javaemul.lang.MutableMapJvm
 import javaemul.lang.uninitializedArrayOf
 import kotlin.Any
@@ -315,7 +315,7 @@ open class Collections {
  }
 
  @ObjCName("J2ktJ2ktCollections_CustomCollectionDisambiguatingOverrides", exact = true)
- open class CustomCollectionDisambiguatingOverrides<T>: Collections.CustomCollection<T>(), JavaCollection<T> {
+ open class CustomCollectionDisambiguatingOverrides<T>: Collections.CustomCollection<T>(), MutableCollectionJvm<T> {
   override fun spliterator(): Spliterator<T> {
    return super<Collections.CustomCollection>.spliterator()
   }
@@ -511,20 +511,24 @@ open class Collections {
  }
 
  @ObjCName("J2ktJ2ktCollections_AbstractCollectionWithToArrayOverride", exact = true)
- abstract class AbstractCollectionWithToArrayOverride<E>: JavaCollection<E> {
+ abstract class AbstractCollectionWithToArrayOverride<E>: MutableCollectionJvm<E> {
+  @Suppress("NOTHING_TO_OVERRIDE")
   override fun toArray(): Array<Any?> {
    return uninitializedArrayOf<Any>(0) as Array<Any?>
   }
 
+  @Suppress("NOTHING_TO_OVERRIDE")
   override fun <T> toArray(a: Array<T>): Array<T> {
    return a
   }
  }
 
  @ObjCName("J2ktJ2ktCollections_CollectionInterfaceWithToArrayOverride", exact = true)
- interface CollectionInterfaceWithToArrayOverride<E>: JavaList<E> {
+ interface CollectionInterfaceWithToArrayOverride<E>: MutableListJvm<E> {
+  @Suppress("NOTHING_TO_OVERRIDE")
   override fun toArray(): Array<Any?>
 
+  @Suppress("NOTHING_TO_OVERRIDE")
   override fun <T> toArray(a: Array<T>): Array<T>
  }
 }
diff --git a/...javatests/com/google/j2cl/readable/java/j2kt/output_kt/NullabilityInferenceProblem.kt.txt b/...javatests/com/google/j2cl/readable/java/j2kt/output_kt/NullabilityInferenceProblem.kt.txt
@@ -22,7 +22,7 @@ import javaemul.lang.*
 import java.lang.RuntimeException
 import java.util.Optional
 import java.util.function.Function
-import javaemul.lang.JavaList
+import javaemul.lang.MutableListJvm
 import kotlin.Any
 import kotlin.Comparator
 import kotlin.OptIn
@@ -70,7 +70,7 @@ open class NullabilityInferenceProblem internal constructor() {
  }
 
  @ObjCName("J2ktJ2ktNullabilityInferenceProblem_ImmutableList", exact = true)
- interface ImmutableList<T: Any>: JavaList<T> {}
+ interface ImmutableList<T: Any>: MutableListJvm<T> {}
 
  @ObjCName("J2ktJ2ktNullabilityInferenceProblem_User", exact = true)
  fun interface User {

diff --git a/transpiler/javatests/com/google/j2cl/readable/java/nullability/output_kt/Nullability.kt.txt b/transpiler/javatests/com/google/j2cl/readable/java/nullability/output_kt/Nullability.kt.txt
@@ -22,7 +22,7 @@ import javaemul.lang.*
 import java.lang.RuntimeException
 import java.util.ArrayList
 import java.util.Comparator
-import javaemul.lang.JavaList
+import javaemul.lang.MutableListJvm
 import jsinterop.annotations.JsConstructor
 import jsinterop.annotations.JsFunction
 import jsinterop.annotations.JsMethod
@@ -214,7 +214,7 @@ open class Nullability {
  }
 
  @ObjCName("J2ktNullabilityNullability_StringList", exact = true)
- interface StringList: JavaList<String?> {}
+ interface StringList: MutableListJvm<String?> {}
 
  @ObjCName("J2ktNullabilityNullability_StringComparator", exact = true)
  open class StringComparator: Comparator<String> {