Fix mutable collection stub methods generation for corner case

The problem is that
`override fun remove(element: E): CollectionWithRemove<E>`
seems to be illegal from Java's point of view, while it's OK for JVM

These declarations have the same signature (return type is isgnored)
- override fun remove(element: E): CollectionWithRemove<E>
- override fun remove(element: E): Boolean

When we meet such declaration we choose random declaration for fake override in synthetic class
that may lead to signature clash

compiler/backend/src/org/jetbrains/kotlin/codegen/CollectionStubMethodGenerator.kt

@@ -25,6 +25,7 @@ import org.jetbrains.kotlin.load.java.BuiltinMethodsWithSpecialGenericSignature.
 import org.jetbrains.kotlin.load.java.BuiltinMethodsWithSpecialGenericSignature.isBuiltinWithSpecialDescriptorInJvm
 import org.jetbrains.kotlin.load.java.isFromBuiltins
 import org.jetbrains.kotlin.name.Name
+import org.jetbrains.kotlin.platform.JavaToKotlinClassMap
 import org.jetbrains.kotlin.resolve.NonReportingOverrideStrategy
 import org.jetbrains.kotlin.resolve.OverrideResolver
 import org.jetbrains.kotlin.resolve.descriptorUtil.builtIns
@@ -204,8 +205,37 @@ class CollectionStubMethodGenerator(
         OverrideResolver.generateOverridesInAClass(klass, listOf(), object : NonReportingOverrideStrategy() {
             override fun addFakeOverride(fakeOverride: CallableMemberDescriptor) {
                 if (fakeOverride !is FunctionDescriptor) return
-                if (fakeOverride.findOverriddenFromDirectSuperClass(mutableCollectionClass)?.kind == DECLARATION) {
-                    result.add(fakeOverride)
+                val foundOverriddenFromDirectSuperClass = fakeOverride.findOverriddenFromDirectSuperClass(mutableCollectionClass) ?: return
+                if (foundOverriddenFromDirectSuperClass.kind == DECLARATION) {
+                    // For regular classes there should no be fake overrides having return types incompatible with return types of their
+                    // overridden, while here it's possible to create declaration like `fun remove(e: E): ImmutableCollection<E>`
+                    // in read-only class that obviously conflicts with `fun remove(e: E): Boolean`.
+                    // But overrides binding algorithm suppose there should be no conflicts like this, so it simply chooses a random
+                    // representative for fake override, while we interested here in ones from mutable version.
+                    //
+                    // NB: READ_ONLY_ARE_EQUAL_TO_MUTABLE_TYPE_CHECKER is used here for cases like:
+                    // `fun iterator(): CharIterator` defined in read-only collection
+                    // The problem is that 'CharIterator' is not a subtype of 'MutableIterator' while from Java's point of view it is,
+                    // so we must hack our subtyping a little bit
+                     val newDescriptor =
+                            if (READ_ONLY_ARE_EQUAL_TO_MUTABLE_TYPE_CHECKER.isSubtypeOf(
+                                    fakeOverride.returnType!!, foundOverriddenFromDirectSuperClass.returnType!!))
+                                fakeOverride
+                            else
+                                foundOverriddenFromDirectSuperClass.copy(
+                                        fakeOverride.containingDeclaration,
+                                        fakeOverride.modality,
+                                        fakeOverride.visibility,
+                                        fakeOverride.kind, false)
+
+                    newDescriptor.overriddenDescriptors =
+                            fakeOverride.overriddenDescriptors.filter {
+                                superDescriptor ->
+                                // filter out incompatible descriptors, e.g. `fun remove(e: E): ImmutableCollection<E>` for `fun remove(e: E): Boolean`
+                                READ_ONLY_ARE_EQUAL_TO_MUTABLE_TYPE_CHECKER.isSubtypeOf(newDescriptor.returnType!!, superDescriptor.returnType!!)
+                            }
+
+                    result.add(newDescriptor)
                 }
             }
 
@@ -266,3 +296,13 @@ class CollectionStubMethodGenerator(
         }
     }
 }
+
+private val READ_ONLY_ARE_EQUAL_TO_MUTABLE_TYPE_CHECKER = KotlinTypeChecker.withAxioms { x, y ->
+    val firstClass = x.declarationDescriptor as? ClassDescriptor ?: return@withAxioms x == y
+    val secondClass = y.declarationDescriptor as? ClassDescriptor ?: return@withAxioms x == y
+
+    val j2k = JavaToKotlinClassMap.INSTANCE
+    val firstReadOnly = if (j2k.isMutable(firstClass)) j2k.convertMutableToReadOnly(firstClass) else firstClass
+    val secondReadOnly = if (j2k.isMutable(secondClass)) j2k.convertMutableToReadOnly(secondClass) else secondClass
+    firstReadOnly.typeConstructor == secondReadOnly.typeConstructor
+}

compiler/testData/codegen/box/builtinStubMethods/customReadOnlyIterator.kt

@@ -0,0 +1,34 @@
+class A : Collection<Char> {
+    override val size: Int
+        get() = throw UnsupportedOperationException()
+
+    override fun contains(element: Char): Boolean {
+        throw UnsupportedOperationException("not implemented") //To change body of created functions use File | Settings | File Templates.
+    }
+
+    override fun containsAll(elements: Collection<Char>): Boolean {
+        throw UnsupportedOperationException("not implemented") //To change body of created functions use File | Settings | File Templates.
+    }
+
+    override fun isEmpty(): Boolean {
+        throw UnsupportedOperationException("not implemented") //To change body of created functions use File | Settings | File Templates.
+    }
+
+    override fun iterator() = MyIterator
+}
+
+object MyIterator : Iterator<Char> {
+    override fun hasNext() = true
+
+    override fun next() = 'a'
+}
+
+
+fun box(): String {
+    val it: MyIterator = A().iterator()
+
+    if (!it.hasNext()) return "fail 1"
+    if (it.next() != 'a') return "fail 2"
+
+    return "OK"
+}

compiler/testData/codegen/box/builtinStubMethods/immutableRemove.kt

@@ -0,0 +1,50 @@
+// FULL_JDK
+// WITH_RUNTIME
+interface ImmutableCollection<out E> : Collection<E> {
+    fun add(element: @UnsafeVariance E): ImmutableCollection<E>
+    fun addAll(elements: Collection<@UnsafeVariance E>): ImmutableCollection<E>
+    fun remove(element: @UnsafeVariance E): ImmutableCollection<E>
+}
+
+class ImmutableCollectionmpl<E> : ImmutableCollection<E> {
+    override val size: Int
+        get() = throw UnsupportedOperationException()
+
+    override fun contains(element: E): Boolean {
+        throw UnsupportedOperationException("not implemented") //To change body of created functions use File | Settings | File Templates.
+    }
+
+    override fun containsAll(elements: Collection<E>): Boolean {
+        throw UnsupportedOperationException("not implemented") //To change body of created functions use File | Settings | File Templates.
+    }
+
+    override fun isEmpty(): Boolean {
+        throw UnsupportedOperationException("not implemented") //To change body of created functions use File | Settings | File Templates.
+    }
+
+    override fun iterator(): Iterator<E> {
+        throw UnsupportedOperationException("not implemented") //To change body of created functions use File | Settings | File Templates.
+    }
+
+    override fun add(element: E): ImmutableCollection<E> = this
+    override fun addAll(elements: Collection<E>): ImmutableCollection<E> = this
+    override fun remove(element: E): ImmutableCollection<E> = this
+}
+
+fun box(): String {
+    val c = ImmutableCollectionmpl<String>()
+    if (c.remove("") !== c) return "fail 1"
+    if (c.add("") !== c) return "fail 2"
+    if (c.addAll(java.util.ArrayList()) !== c) return "fail 3"
+
+    val method = c.javaClass.methods.single { it.name == "remove" && it.returnType == Boolean::class.javaPrimitiveType }
+
+    try {
+        method.invoke(c, "")
+        return "fail 4"
+    } catch (e: java.lang.reflect.InvocationTargetException) {
+        if (e.cause!!.message != "Mutating immutable collection") return "fail 5: ${e.cause!!.message}"
+    }
+
+    return "OK"
+}

compiler/testData/codegen/bytecodeListing/immutableCollection.kt

@@ -0,0 +1,30 @@
+interface ImmutableCollection<out E> : Collection<E> {
+    fun add(element: @UnsafeVariance E): ImmutableCollection<E>
+    fun addAll(elements: Collection<@UnsafeVariance E>): ImmutableCollection<E>
+    fun remove(element: @UnsafeVariance E): ImmutableCollection<E>
+}
+
+class ImmutableCollectionmpl<E> : ImmutableCollection<E> {
+    override val size: Int
+        get() = throw UnsupportedOperationException()
+
+    override fun contains(element: E): Boolean {
+        throw UnsupportedOperationException("not implemented") //To change body of created functions use File | Settings | File Templates.
+    }
+
+    override fun containsAll(elements: Collection<E>): Boolean {
+        throw UnsupportedOperationException("not implemented") //To change body of created functions use File | Settings | File Templates.
+    }
+
+    override fun isEmpty(): Boolean {
+        throw UnsupportedOperationException("not implemented") //To change body of created functions use File | Settings | File Templates.
+    }
+
+    override fun iterator(): Iterator<E> {
+        throw UnsupportedOperationException("not implemented") //To change body of created functions use File | Settings | File Templates.
+    }
+
+    override fun add(element: E): ImmutableCollection<E> = this
+    override fun addAll(elements: Collection<E>): ImmutableCollection<E> = this
+    override fun remove(element: E): ImmutableCollection<E> = this
+}

compiler/testData/codegen/bytecodeListing/immutableCollection.txt

@@ -0,0 +1,35 @@
+@kotlin.Metadata
+public interface ImmutableCollection {
+    public abstract @org.jetbrains.annotations.NotNull method add(p0: java.lang.Object): ImmutableCollection
+    public abstract method add(p0: java.lang.Object): boolean
+    public abstract @org.jetbrains.annotations.NotNull method addAll(@org.jetbrains.annotations.NotNull p0: java.util.Collection): ImmutableCollection
+    public abstract method addAll(p0: java.util.Collection): boolean
+    public abstract method clear(): void
+    public abstract method iterator(): java.util.Iterator
+    public abstract @org.jetbrains.annotations.NotNull method remove(p0: java.lang.Object): ImmutableCollection
+    public abstract method remove(p0: java.lang.Object): boolean
+    public abstract method removeAll(p0: java.util.Collection): boolean
+    public abstract method retainAll(p0: java.util.Collection): boolean
+}
+
+@kotlin.Metadata
+public final class ImmutableCollectionmpl {
+    public method <init>(): void
+    public @org.jetbrains.annotations.NotNull method add(p0: java.lang.Object): ImmutableCollection
+    public method add(p0: java.lang.Object): boolean
+    public @org.jetbrains.annotations.NotNull method addAll(@org.jetbrains.annotations.NotNull p0: java.util.Collection): ImmutableCollection
+    public method addAll(p0: java.util.Collection): boolean
+    public method clear(): void
+    public method contains(p0: java.lang.Object): boolean
+    public method containsAll(@org.jetbrains.annotations.NotNull p0: java.util.Collection): boolean
+    public method getSize(): int
+    public method isEmpty(): boolean
+    public @org.jetbrains.annotations.NotNull method iterator(): java.util.Iterator
+    public @org.jetbrains.annotations.NotNull method remove(p0: java.lang.Object): ImmutableCollection
+    public method remove(p0: java.lang.Object): boolean
+    public method removeAll(p0: java.util.Collection): boolean
+    public method retainAll(p0: java.util.Collection): boolean
+    public final method size(): int
+    public method toArray(): java.lang.Object[]
+    public method toArray(p0: java.lang.Object[]): java.lang.Object[]
+}

compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java

@@ -1276,12 +1276,24 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest {
             doTest(fileName);
         }
 
+        @TestMetadata("customReadOnlyIterator.kt")
+        public void testCustomReadOnlyIterator() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/builtinStubMethods/customReadOnlyIterator.kt");
+            doTest(fileName);
+        }
+
         @TestMetadata("delegationToArrayList.kt")
         public void testDelegationToArrayList() throws Exception {
             String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/builtinStubMethods/delegationToArrayList.kt");
             doTest(fileName);
         }
 
+        @TestMetadata("immutableRemove.kt")
+        public void testImmutableRemove() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/builtinStubMethods/immutableRemove.kt");
+            doTest(fileName);
+        }
+
         @TestMetadata("implementationInTrait.kt")
         public void testImplementationInTrait() throws Exception {
             String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/builtinStubMethods/implementationInTrait.kt");

compiler/tests/org/jetbrains/kotlin/codegen/BytecodeListingTestGenerated.java

@@ -53,6 +53,12 @@ public class BytecodeListingTestGenerated extends AbstractBytecodeListingTest {
         doTest(fileName);
     }
 
+    @TestMetadata("immutableCollection.kt")
+    public void testImmutableCollection() throws Exception {
+        String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/bytecodeListing/immutableCollection.kt");
+        doTest(fileName);
+    }
+
     @TestMetadata("inlineOnly.kt")
     public void testInlineOnly() throws Exception {
         String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/bytecodeListing/inlineOnly.kt");