diff --git a/compiler/frontend.java/src/org/jetbrains/kotlin/load/java/components/typeQualifiers.kt b/compiler/frontend.java/src/org/jetbrains/kotlin/load/java/components/typeQualifiers.kt index ff5bf35211c..b329265c78e 100644 --- a/compiler/frontend.java/src/org/jetbrains/kotlin/load/java/components/typeQualifiers.kt +++ b/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, but in the subclass it may be Derived, which + // is declared to extend Super, 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! in the subclass and { List, (Mutable)List! } 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) } } diff --git a/compiler/testData/diagnostics/tests/j+k/types/returnCollection.kt b/compiler/testData/diagnostics/tests/j+k/types/returnCollection.kt new file mode 100644 index 00000000000..84e5a4b8eb8 --- /dev/null +++ b/compiler/testData/diagnostics/tests/j+k/types/returnCollection.kt @@ -0,0 +1,13 @@ +// FILE: k.kt + +trait K { + fun foo(): List +} + +// FILE: J.java + +import java.util.*; + +interface J extends K { + List foo(); +} \ No newline at end of file diff --git a/compiler/testData/diagnostics/tests/j+k/types/returnCollection.txt b/compiler/testData/diagnostics/tests/j+k/types/returnCollection.txt new file mode 100644 index 00000000000..d918d8c2f36 --- /dev/null +++ b/compiler/testData/diagnostics/tests/j+k/types/returnCollection.txt @@ -0,0 +1,17 @@ +package + +public/*package*/ /*synthesized*/ fun J(/*0*/ function: () -> kotlin.(Mutable)List!): 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 + 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 + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} diff --git a/compiler/testData/diagnostics/tests/j+k/types/shapeMismatchInCovariantPosition.kt b/compiler/testData/diagnostics/tests/j+k/types/shapeMismatchInCovariantPosition.kt new file mode 100644 index 00000000000..33a4d10347f --- /dev/null +++ b/compiler/testData/diagnostics/tests/j+k/types/shapeMismatchInCovariantPosition.kt @@ -0,0 +1,19 @@ +// FILE: k.kt + +trait G +trait SubG : G + +trait K { + fun foo(): G + fun bar(): G? +} + +// FILE: J.java + +import java.util.*; + +interface J extends K { + SubG foo(); + SubG bar(); + SubG baz(); +} \ No newline at end of file diff --git a/compiler/testData/diagnostics/tests/j+k/types/shapeMismatchInCovariantPosition.txt b/compiler/testData/diagnostics/tests/j+k/types/shapeMismatchInCovariantPosition.txt new file mode 100644 index 00000000000..514a4497b84 --- /dev/null +++ b/compiler/testData/diagnostics/tests/j+k/types/shapeMismatchInCovariantPosition.txt @@ -0,0 +1,30 @@ +package + +internal trait G { + 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? + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + internal abstract fun foo(): G + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} + +internal trait SubG : G { + 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 +} diff --git a/compiler/testData/diagnostics/tests/j+k/types/shapeMismatchInCovariantPositionGeneric.kt b/compiler/testData/diagnostics/tests/j+k/types/shapeMismatchInCovariantPositionGeneric.kt new file mode 100644 index 00000000000..e07ec6ec4b3 --- /dev/null +++ b/compiler/testData/diagnostics/tests/j+k/types/shapeMismatchInCovariantPositionGeneric.kt @@ -0,0 +1,19 @@ +// FILE: k.kt + +trait G +trait SubG : G + +trait K { + fun foo(): G + fun bar(): G? +} + +// FILE: J.java + +import java.util.*; + +interface J extends K { + SubG> foo(); + SubG> bar(); + SubG> baz(); +} \ No newline at end of file diff --git a/compiler/testData/diagnostics/tests/j+k/types/shapeMismatchInCovariantPositionGeneric.txt b/compiler/testData/diagnostics/tests/j+k/types/shapeMismatchInCovariantPositionGeneric.txt new file mode 100644 index 00000000000..5927cab9c4c --- /dev/null +++ b/compiler/testData/diagnostics/tests/j+k/types/shapeMismatchInCovariantPositionGeneric.txt @@ -0,0 +1,30 @@ +package + +internal trait G { + 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? + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + internal abstract fun foo(): G + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} + +internal trait SubG : G { + 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 +} diff --git a/compiler/tests/org/jetbrains/kotlin/checkers/JetDiagnosticsTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/checkers/JetDiagnosticsTestGenerated.java index 7f828debc5d..1ae5226e81f 100644 --- a/compiler/tests/org/jetbrains/kotlin/checkers/JetDiagnosticsTestGenerated.java +++ b/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") diff --git a/spec-docs/flexible-java-types.md b/spec-docs/flexible-java-types.md index 74b190488ce..12b23399515 100644 --- a/spec-docs/flexible-java-types.md +++ b/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 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>`, indices go as follows: `0 - A<...>, 1 - B, 2 - C, 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..C)` gives `0 - (A..C), 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..C)` gives `0 - (A..C), 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` from super, +and `Baz>` in the override, where `Baz` extends `Foo`). 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!` while the overridden ones may be `List` and `List`, the equivalence holds and we can safely +assume the enhanced return type to be `List` (subtype of both overridden ones). Example: - `Mutable(List)!`