From 6ecfa6e985069e148af3193318e00c81e3867d89 Mon Sep 17 00:00:00 2001 From: Denis Zharkov Date: Wed, 26 Aug 2015 11:22:40 +0300 Subject: [PATCH] Refine subtyping check: pay attention to corresponding supertype nullability F: Any, T : F? => !isSubtype(T, Any) It helps to identify upper bounds violation like in KT-7455 #KT-7455 Fixed #KT-2924 Fixed #KT-3015 Fixed --- .../declarationsBoundsViolation.kt | 13 +++++++ .../declarationsBoundsViolation.txt | 38 +++++++++++++++++++ .../nullability/expressionsBoundsViolation.kt | 18 +++++++++ .../expressionsBoundsViolation.txt | 12 ++++++ .../generics/nullability/tpBoundsViolation.kt | 31 +++++++++++++++ .../nullability/tpBoundsViolation.txt | 11 ++++++ .../nullability/useAsValueArgument.kt | 17 +++++++++ .../nullability/useAsValueArgument.txt | 7 ++++ .../checkers/JetDiagnosticsTestGenerated.java | 24 ++++++++++++ .../types/checker/TypeCheckingProcedure.java | 9 ++++- .../jetbrains/kotlin/types/checker/utils.kt | 7 +++- 11 files changed, 184 insertions(+), 3 deletions(-) create mode 100644 compiler/testData/diagnostics/tests/generics/nullability/declarationsBoundsViolation.kt create mode 100644 compiler/testData/diagnostics/tests/generics/nullability/declarationsBoundsViolation.txt create mode 100644 compiler/testData/diagnostics/tests/generics/nullability/expressionsBoundsViolation.kt create mode 100644 compiler/testData/diagnostics/tests/generics/nullability/expressionsBoundsViolation.txt create mode 100644 compiler/testData/diagnostics/tests/generics/nullability/tpBoundsViolation.kt create mode 100644 compiler/testData/diagnostics/tests/generics/nullability/tpBoundsViolation.txt create mode 100644 compiler/testData/diagnostics/tests/generics/nullability/useAsValueArgument.kt create mode 100644 compiler/testData/diagnostics/tests/generics/nullability/useAsValueArgument.txt diff --git a/compiler/testData/diagnostics/tests/generics/nullability/declarationsBoundsViolation.kt b/compiler/testData/diagnostics/tests/generics/nullability/declarationsBoundsViolation.kt new file mode 100644 index 00000000000..b53d005d90a --- /dev/null +++ b/compiler/testData/diagnostics/tests/generics/nullability/declarationsBoundsViolation.kt @@ -0,0 +1,13 @@ +// !DIAGNOSTICS: -UNUSED_PARAMETER + +interface A + +fun foo1(a: A<S>) {} + +class B1 : A<E> +class B2 : A<E> +class B3 : A<E> + +class B4 : A + +fun foo(a: A, b: A, c: A<Y2>) {} diff --git a/compiler/testData/diagnostics/tests/generics/nullability/declarationsBoundsViolation.txt b/compiler/testData/diagnostics/tests/generics/nullability/declarationsBoundsViolation.txt new file mode 100644 index 00000000000..65129da80eb --- /dev/null +++ b/compiler/testData/diagnostics/tests/generics/nullability/declarationsBoundsViolation.txt @@ -0,0 +1,38 @@ +package + +internal fun foo(/*0*/ a: A, /*1*/ b: A, /*2*/ c: A): kotlin.Unit +internal fun foo1(/*0*/ a: A): kotlin.Unit + +internal interface A { + 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 +} + +internal final class B1 : A { + public constructor B1() + 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 +} + +internal final class B2 : A { + public constructor B2() + 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 +} + +internal final class B3 : A { + public constructor B3() + 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 +} + +internal final class B4 : A { + public constructor B4() + 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/generics/nullability/expressionsBoundsViolation.kt b/compiler/testData/diagnostics/tests/generics/nullability/expressionsBoundsViolation.kt new file mode 100644 index 00000000000..8362320b54f --- /dev/null +++ b/compiler/testData/diagnostics/tests/generics/nullability/expressionsBoundsViolation.kt @@ -0,0 +1,18 @@ +// !DIAGNOSTICS: -UNUSED_PARAMETER + +class A(x: T) + +fun foo1(x: E) {} +fun E.foo2() {} + +fun bar(x: F) { + A(x) + A<F>(x) + + foo1(x) + foo1<F>(x) + + x.foo2() + x.foo2<F>() +} + diff --git a/compiler/testData/diagnostics/tests/generics/nullability/expressionsBoundsViolation.txt b/compiler/testData/diagnostics/tests/generics/nullability/expressionsBoundsViolation.txt new file mode 100644 index 00000000000..8fdcb08d46e --- /dev/null +++ b/compiler/testData/diagnostics/tests/generics/nullability/expressionsBoundsViolation.txt @@ -0,0 +1,12 @@ +package + +internal fun bar(/*0*/ x: F): kotlin.Unit +internal fun foo1(/*0*/ x: E): kotlin.Unit +internal fun E.foo2(): kotlin.Unit + +internal final class A { + public constructor A(/*0*/ x: 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 +} diff --git a/compiler/testData/diagnostics/tests/generics/nullability/tpBoundsViolation.kt b/compiler/testData/diagnostics/tests/generics/nullability/tpBoundsViolation.kt new file mode 100644 index 00000000000..a2ae506610a --- /dev/null +++ b/compiler/testData/diagnostics/tests/generics/nullability/tpBoundsViolation.kt @@ -0,0 +1,31 @@ +// !DIAGNOSTICS: -UNUSED_PARAMETER, -BASE_WITH_NULLABLE_UPPER_BOUND + +class A { + fun foo1(x: E) {} + fun foo2(x: E) {} + + fun bar(x: F, y: F?, z: Z, w: W) { + foo1(x) + foo1(x) + foo2(x) + foo2(x) + + foo1<F?>(y) + foo1(y) + foo2(y) + foo2(y) + foo1(y) + foo2(y) + + foo1(z) + foo1(z) + foo2(z) + foo2(z) + + foo1<W>(w) + foo1(w) + foo2(w) + foo2(w) + foo1<W>(w) + } +} diff --git a/compiler/testData/diagnostics/tests/generics/nullability/tpBoundsViolation.txt b/compiler/testData/diagnostics/tests/generics/nullability/tpBoundsViolation.txt new file mode 100644 index 00000000000..ed21e4d1a31 --- /dev/null +++ b/compiler/testData/diagnostics/tests/generics/nullability/tpBoundsViolation.txt @@ -0,0 +1,11 @@ +package + +internal final class A { + public constructor A() + internal final fun bar(/*0*/ x: F, /*1*/ y: F?, /*2*/ z: Z, /*3*/ w: W): kotlin.Unit + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + internal final fun foo1(/*0*/ x: E): kotlin.Unit + internal final fun foo2(/*0*/ x: E): kotlin.Unit + 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/generics/nullability/useAsValueArgument.kt b/compiler/testData/diagnostics/tests/generics/nullability/useAsValueArgument.kt new file mode 100644 index 00000000000..31fc65f1188 --- /dev/null +++ b/compiler/testData/diagnostics/tests/generics/nullability/useAsValueArgument.kt @@ -0,0 +1,17 @@ +// !DIAGNOSTICS: -UNUSED_PARAMETER,-UNUSED_VARIABLE + +fun bar1(x: T) {} + +fun bar2(x: CharSequence?) {} + +fun bar3(x: T) {} + +fun bar4(x: String) {} + +fun foo(x: T) { + bar1(x) + bar2(x) + + bar3(x) + bar4(x) +} diff --git a/compiler/testData/diagnostics/tests/generics/nullability/useAsValueArgument.txt b/compiler/testData/diagnostics/tests/generics/nullability/useAsValueArgument.txt new file mode 100644 index 00000000000..b2f82b48b6d --- /dev/null +++ b/compiler/testData/diagnostics/tests/generics/nullability/useAsValueArgument.txt @@ -0,0 +1,7 @@ +package + +internal fun bar1(/*0*/ x: T): kotlin.Unit +internal fun bar2(/*0*/ x: kotlin.CharSequence?): kotlin.Unit +internal fun bar3(/*0*/ x: T): kotlin.Unit +internal fun bar4(/*0*/ x: kotlin.String): kotlin.Unit +internal fun foo(/*0*/ x: T): kotlin.Unit diff --git a/compiler/tests/org/jetbrains/kotlin/checkers/JetDiagnosticsTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/checkers/JetDiagnosticsTestGenerated.java index 8803ebf001c..22af0ec8dc4 100644 --- a/compiler/tests/org/jetbrains/kotlin/checkers/JetDiagnosticsTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/checkers/JetDiagnosticsTestGenerated.java @@ -6137,11 +6137,35 @@ public class JetDiagnosticsTestGenerated extends AbstractJetDiagnosticsTest { JetTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/diagnostics/tests/generics/nullability"), Pattern.compile("^(.+)\\.kt$"), true); } + @TestMetadata("declarationsBoundsViolation.kt") + public void testDeclarationsBoundsViolation() throws Exception { + String fileName = JetTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/generics/nullability/declarationsBoundsViolation.kt"); + doTest(fileName); + } + + @TestMetadata("expressionsBoundsViolation.kt") + public void testExpressionsBoundsViolation() throws Exception { + String fileName = JetTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/generics/nullability/expressionsBoundsViolation.kt"); + doTest(fileName); + } + @TestMetadata("nullToGeneric.kt") public void testNullToGeneric() throws Exception { String fileName = JetTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/generics/nullability/nullToGeneric.kt"); doTest(fileName); } + + @TestMetadata("tpBoundsViolation.kt") + public void testTpBoundsViolation() throws Exception { + String fileName = JetTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/generics/nullability/tpBoundsViolation.kt"); + doTest(fileName); + } + + @TestMetadata("useAsValueArgument.kt") + public void testUseAsValueArgument() throws Exception { + String fileName = JetTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/generics/nullability/useAsValueArgument.kt"); + doTest(fileName); + } } @TestMetadata("compiler/testData/diagnostics/tests/generics/starProjections") diff --git a/core/descriptors/src/org/jetbrains/kotlin/types/checker/TypeCheckingProcedure.java b/core/descriptors/src/org/jetbrains/kotlin/types/checker/TypeCheckingProcedure.java index 0844dc4548d..02e0ae4fe12 100644 --- a/core/descriptors/src/org/jetbrains/kotlin/types/checker/TypeCheckingProcedure.java +++ b/core/descriptors/src/org/jetbrains/kotlin/types/checker/TypeCheckingProcedure.java @@ -195,19 +195,24 @@ public class TypeCheckingProcedure { if (subtype.isError() || supertype.isError()) { return true; } + if (!supertype.isMarkedNullable() && subtype.isMarkedNullable()) { return false; } - subtype = TypeUtils.makeNotNullable(subtype); - supertype = TypeUtils.makeNotNullable(supertype); + if (KotlinBuiltIns.isNothingOrNullableNothing(subtype)) { return true; } + @Nullable JetType closestSupertype = findCorrespondingSupertype(subtype, supertype, constraints); if (closestSupertype == null) { return constraints.noCorrespondingSupertype(subtype, supertype); // if this returns true, there still isn't any supertype to continue with } + if (!supertype.isMarkedNullable() && closestSupertype.isMarkedNullable()) { + return false; + } + return checkSubtypeForTheSameConstructor(closestSupertype, supertype); } diff --git a/core/descriptors/src/org/jetbrains/kotlin/types/checker/utils.kt b/core/descriptors/src/org/jetbrains/kotlin/types/checker/utils.kt index bcd0681b245..dae7c30b0e3 100644 --- a/core/descriptors/src/org/jetbrains/kotlin/types/checker/utils.kt +++ b/core/descriptors/src/org/jetbrains/kotlin/types/checker/utils.kt @@ -18,6 +18,7 @@ package org.jetbrains.kotlin.types.checker import org.jetbrains.kotlin.types.JetType import org.jetbrains.kotlin.types.TypeSubstitutor +import org.jetbrains.kotlin.types.TypeUtils import org.jetbrains.kotlin.types.Variance import org.jetbrains.kotlin.types.checker.TypeCheckingProcedureCallbacks import java.util.* @@ -40,14 +41,18 @@ public fun findCorrespondingSupertype( if (typeCheckingProcedureCallbacks.assertEqualTypeConstructors(constructor, supertypeConstructor)) { var substituted = currentSubtype + var isAnyMarkedNullable = currentSubtype.isMarkedNullable + var currentPathNode = lastPathNode.previous while (currentPathNode != null) { substituted = TypeSubstitutor.create(currentPathNode.type).safeSubstitute(substituted, Variance.INVARIANT) + isAnyMarkedNullable = isAnyMarkedNullable || currentPathNode.type.isMarkedNullable + currentPathNode = currentPathNode.previous } - return substituted + return TypeUtils.makeNullableAsSpecified(substituted, isAnyMarkedNullable) } for (immediateSupertype in constructor.getSupertypes()) {