Proper treatment of return types

compiler/frontend.java/src/org/jetbrains/kotlin/load/java/components/typeQualifiers.kt

@@ -16,7 +16,6 @@
 
 package org.jetbrains.kotlin.load.java.components
 
-import org.jetbrains.kotlin.builtins.KotlinBuiltIns
 import org.jetbrains.kotlin.descriptors.annotations.Annotations
 import org.jetbrains.kotlin.load.java.JvmAnnotationNames
 import org.jetbrains.kotlin.load.java.components.MutabilityQualifier.MUTABLE
@@ -26,6 +25,7 @@ import org.jetbrains.kotlin.load.java.components.NullabilityQualifier.NULLABLE
 import org.jetbrains.kotlin.name.FqName
 import org.jetbrains.kotlin.platform.JavaToKotlinClassMap
 import org.jetbrains.kotlin.types.JetType
+import org.jetbrains.kotlin.types.checker.JetTypeChecker
 import org.jetbrains.kotlin.types.flexibility
 import org.jetbrains.kotlin.types.isFlexible
 import org.jetbrains.kotlin.utils.getOrDefault
@@ -112,10 +112,22 @@ fun JetType.computeIndexedQualifiersForOverride(fromSupertypes: Collection<JetTy
     val indexedFromSupertypes = fromSupertypes.map { it.toIndexed() }
     val indexedThisType = this.toIndexed()
 
-    return _r@ fun(index: Int): JavaTypeQualifiers {
+    // The covariant case may be hard, e.g. in the superclass the return may be Super<T>, but in the subclass it may be Derived, which
+    // is declared to extend Super<T>, and propagating data here is highly non-trivial, so we only look at the head type constructor
+    // (outermost type), unless the type in the subclass is interchangeable with the all the types in superclasses:
+    // e.g. we have (Mutable)List<String!>! in the subclass and { List<String!>, (Mutable)List<String>! } from superclasses
+    // Note that `this` is flexible here, so it's equal to it's bounds
+    val onlyHeadTypeConstructor = isCovariant && fromSupertypes.any { !JetTypeChecker.DEFAULT.equalTypes(it, this) }
+
+    return fun(index: Int): JavaTypeQualifiers {
+        val isHeadTypeConstructor = index == 0
+        if (!isHeadTypeConstructor && onlyHeadTypeConstructor) return JavaTypeQualifiers.NONE
+
         val qualifiers = indexedThisType.getOrDefault(index, { return JavaTypeQualifiers.NONE })
         val verticalSlice = indexedFromSupertypes.map { it.getOrDefault(index, { null }) }.filterNotNull()
-        return qualifiers.computeQualifiersForOverride(verticalSlice, isCovariant)
+
+        // Only the head type constructor is safely co-variant
+        return qualifiers.computeQualifiersForOverride(verticalSlice, isCovariant && isHeadTypeConstructor)
     }
 }
 

compiler/testData/diagnostics/tests/j+k/types/returnCollection.kt

@@ -0,0 +1,13 @@
+// FILE: k.kt
+
+trait K {
+    fun foo(): List<String>
+}
+
+// FILE: J.java
+
+import java.util.*;
+
+interface J extends K {
+    List<String> foo();
+}

compiler/testData/diagnostics/tests/j+k/types/returnCollection.txt

@@ -0,0 +1,17 @@
+package
+
+public/*package*/ /*synthesized*/ fun J(/*0*/ function: () -> kotlin.(Mutable)List<kotlin.String!>!): J
+
+public/*package*/ trait J : K {
+    public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean
+    public abstract override /*1*/ fun foo(): kotlin.List<kotlin.String>
+    public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int
+    public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String
+}
+
+internal trait K {
+    public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean
+    internal abstract fun foo(): kotlin.List<kotlin.String>
+    public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int
+    public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String
+}

compiler/testData/diagnostics/tests/j+k/types/shapeMismatchInCovariantPosition.kt

@@ -0,0 +1,19 @@
+// FILE: k.kt
+
+trait G<T>
+trait SubG : G<String>
+
+trait K {
+    fun foo(): G<String>
+    fun bar(): G<String>?
+}
+
+// FILE: J.java
+
+import java.util.*;
+
+interface J extends K {
+    SubG foo();
+    SubG bar();
+    SubG baz();
+}

compiler/testData/diagnostics/tests/j+k/types/shapeMismatchInCovariantPosition.txt

@@ -0,0 +1,30 @@
+package
+
+internal trait G</*0*/ T> {
+    public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean
+    public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int
+    public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String
+}
+
+public/*package*/ trait J : K {
+    public abstract override /*1*/ fun bar(): SubG?
+    public abstract fun baz(): SubG!
+    public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean
+    public abstract override /*1*/ fun foo(): SubG
+    public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int
+    public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String
+}
+
+internal trait K {
+    internal abstract fun bar(): G<kotlin.String>?
+    public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean
+    internal abstract fun foo(): G<kotlin.String>
+    public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int
+    public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String
+}
+
+internal trait SubG : G<kotlin.String> {
+    public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean
+    public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int
+    public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String
+}

compiler/testData/diagnostics/tests/j+k/types/shapeMismatchInCovariantPositionGeneric.kt

@@ -0,0 +1,19 @@
+// FILE: k.kt
+
+trait G<T>
+trait SubG<A, B> : G<String>
+
+trait K<KT> {
+    fun foo(): G<KT>
+    fun bar(): G<KT>?
+}
+
+// FILE: J.java
+
+import java.util.*;
+
+interface J extends K<String> {
+    SubG<String, G<String>> foo();
+    SubG<String, G<String>> bar();
+    SubG<String, G<String>> baz();
+}

compiler/testData/diagnostics/tests/j+k/types/shapeMismatchInCovariantPositionGeneric.txt

@@ -0,0 +1,30 @@
+package
+
+internal trait G</*0*/ T> {
+    public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean
+    public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int
+    public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String
+}
+
+public/*package*/ trait J : K<kotlin.String!> {
+    public abstract override /*1*/ fun bar(): SubG<kotlin.String!, G<kotlin.String!>!>?
+    public abstract fun baz(): SubG<kotlin.String!, G<kotlin.String!>!>!
+    public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean
+    public abstract override /*1*/ fun foo(): SubG<kotlin.String!, G<kotlin.String!>!>
+    public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int
+    public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String
+}
+
+internal trait K</*0*/ KT> {
+    internal abstract fun bar(): G<KT>?
+    public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean
+    internal abstract fun foo(): G<KT>
+    public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int
+    public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String
+}
+
+internal trait SubG</*0*/ A, /*1*/ B> : G<kotlin.String> {
+    public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean
+    public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int
+    public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String
+}

compiler/tests/org/jetbrains/kotlin/checkers/JetDiagnosticsTestGenerated.java

@@ -8183,6 +8183,33 @@ public class JetDiagnosticsTestGenerated extends AbstractJetDiagnosticsTest {
                 String fileName = JetTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/j+k/UnboxingNulls.kt");
                 doTest(fileName);
             }
+
+            @TestMetadata("compiler/testData/diagnostics/tests/j+k/types")
+            @TestDataPath("$PROJECT_ROOT")
+            @RunWith(JUnit3RunnerWithInners.class)
+            public static class Types extends AbstractJetDiagnosticsTest {
+                public void testAllFilesPresentInTypes() throws Exception {
+                    JetTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/diagnostics/tests/j+k/types"), Pattern.compile("^(.+)\\.kt$"), true);
+                }
+
+                @TestMetadata("returnCollection.kt")
+                public void testReturnCollection() throws Exception {
+                    String fileName = JetTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/j+k/types/returnCollection.kt");
+                    doTest(fileName);
+                }
+
+                @TestMetadata("shapeMismatchInCovariantPosition.kt")
+                public void testShapeMismatchInCovariantPosition() throws Exception {
+                    String fileName = JetTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/j+k/types/shapeMismatchInCovariantPosition.kt");
+                    doTest(fileName);
+                }
+
+                @TestMetadata("shapeMismatchInCovariantPositionGeneric.kt")
+                public void testShapeMismatchInCovariantPositionGeneric() throws Exception {
+                    String fileName = JetTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/j+k/types/shapeMismatchInCovariantPositionGeneric.kt");
+                    doTest(fileName);
+                }
+            }
         }
 
         @TestMetadata("compiler/testData/diagnostics/tests/jdk-annotations")

spec-docs/flexible-java-types.md

@@ -254,7 +254,7 @@ Enhancement rules (the result of their application is called a *propagated signa
  - collect annotations from all supertypes and the override in the subclass
  - for parts other than return type (which may be covariantly overridden) if there are conflicts (`@Nullable` together with `@NotNull` or
    `@ReadOnly` together with `@Mutable`), discard the respective annotations and issue appropriate warnings
- - for return types (only the 0-index, see below):
+ - for return types (full if the type from override is `~~`-equivalent to all from supertypes, and only 0-index (see below) otherwise)):
      - fist, take annotations from supertypes, and among them: if there's `@NotNull`, discard `@Nullable`, if there's `@Mutable` discard `@ReadOnly`
      - then if in the subtype there's `@Nullable` and in the supertype there's `@NotNull`, discard the nullability annotations (analogously,
        for mutability annotations)
@@ -366,8 +366,15 @@ fun foo(MutableList<T> p): R! // parameter propagated from superclass (@Mutable,
 the following procedure is used. First, every sub-tree of the type is assigned an index which is its zero-based position in the textual
 representation of the type (`0` is root). Example: for `A<B, C<D, E>>`, indices go as follows: `0 - A<...>, 1 - B, 2 - C<D, E>, 3 - D, 4 - E`,
 which corresponds to the left-to-right breadth-first walk of the tree representation of the type. For flexible types, both bounds are indexed
-in the same way: `(A<B>..C<D>)` gives `0 - (A<B>..C<D>), 1 - B and D`. Now, in the aforementioned procedure, annotations are collected and
-considered *at each index*, and only index 0 of the return type is considered as possibly covariant (this is done for simplicity).
+in the same way: `(A<B>..C<D>)` gives `0 - (A<B>..C<D>), 1 - B and D`. 
+
+Now, in the aforementioned procedure, annotations are collected and considered *at each index* for types other than return types. Return types
+are co-variant, thus the overriding type may not match the overridden ones in its shape (e.g. we can have `Foo<Bar>` from super, 
+and `Baz<One, Two<Three>>` in the override, where `Baz` extends `Foo<Bar>`). This makes it impossible sometimes to propagate data into 
+covariant overrides, and in such cases we resort to only looking at the head constructor (index == 0). The safe cases are detected by checking
+that the overriding type is `~~`-equivalent to all the overridden ones, which guarantees that their shapes match. For example, the overriding 
+type may be `(Mutable)List<Foo!>!` while the overridden ones may be `List<Foo>` and `List<Foo?>`, the equivalence holds and we can safely 
+assume the enhanced return type to be `List<Foo>` (subtype of both overridden ones).
 
 Example:
  - `Mutable(List)<A!>!`