From 632e33678253daab7d66720eaaf1bfb0e5ffd5bd Mon Sep 17 00:00:00 2001 From: Denis Zharkov Date: Fri, 28 Aug 2015 19:09:07 +0300 Subject: [PATCH] Prohibit unsafe covariant conversion for collections invariant in Java --- .../diagnostics/DefaultErrorMessagesJvm.java | 2 + .../resolve/jvm/diagnostics/ErrorsJvm.java | 1 + ...JavaGenericVarianceViolationTypeChecker.kt | 96 +++++++++++++++++++ .../jvm/platform/JvmPlatformConfigurator.kt | 4 +- .../genericVarianceViolation/listSuperType.kt | 16 ++++ .../listSuperType.txt | 39 ++++++++ .../genericVarianceViolation/rawTypes.kt | 14 +++ .../genericVarianceViolation/rawTypes.txt | 11 +++ .../genericVarianceViolation/simple.kt | 67 +++++++++++++ .../genericVarianceViolation/simple.txt | 17 ++++ .../genericVarianceViolation/smartCast.kt | 16 ++++ .../genericVarianceViolation/smartCast.txt | 11 +++ .../strangeVariance.kt | 18 ++++ .../strangeVariance.txt | 12 +++ .../userDefinedOut.kt | 17 ++++ .../userDefinedOut.txt | 19 ++++ .../genericVarianceViolation/valueFromJava.kt | 14 +++ .../valueFromJava.txt | 12 +++ .../genericVarianceViolation/wildcards.kt | 49 ++++++++++ .../genericVarianceViolation/wildcards.txt | 16 ++++ .../checkers/JetDiagnosticsTestGenerated.java | 57 +++++++++++ .../kotlin/load/java/lazy/types/RawType.kt | 3 + spec-docs/flexible-java-types.md | 33 +++++++ 23 files changed, 543 insertions(+), 1 deletion(-) create mode 100644 compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/platform/JavaGenericVarianceViolationTypeChecker.kt create mode 100644 compiler/testData/diagnostics/tests/platformTypes/genericVarianceViolation/listSuperType.kt create mode 100644 compiler/testData/diagnostics/tests/platformTypes/genericVarianceViolation/listSuperType.txt create mode 100644 compiler/testData/diagnostics/tests/platformTypes/genericVarianceViolation/rawTypes.kt create mode 100644 compiler/testData/diagnostics/tests/platformTypes/genericVarianceViolation/rawTypes.txt create mode 100644 compiler/testData/diagnostics/tests/platformTypes/genericVarianceViolation/simple.kt create mode 100644 compiler/testData/diagnostics/tests/platformTypes/genericVarianceViolation/simple.txt create mode 100644 compiler/testData/diagnostics/tests/platformTypes/genericVarianceViolation/smartCast.kt create mode 100644 compiler/testData/diagnostics/tests/platformTypes/genericVarianceViolation/smartCast.txt create mode 100644 compiler/testData/diagnostics/tests/platformTypes/genericVarianceViolation/strangeVariance.kt create mode 100644 compiler/testData/diagnostics/tests/platformTypes/genericVarianceViolation/strangeVariance.txt create mode 100644 compiler/testData/diagnostics/tests/platformTypes/genericVarianceViolation/userDefinedOut.kt create mode 100644 compiler/testData/diagnostics/tests/platformTypes/genericVarianceViolation/userDefinedOut.txt create mode 100644 compiler/testData/diagnostics/tests/platformTypes/genericVarianceViolation/valueFromJava.kt create mode 100644 compiler/testData/diagnostics/tests/platformTypes/genericVarianceViolation/valueFromJava.txt create mode 100644 compiler/testData/diagnostics/tests/platformTypes/genericVarianceViolation/wildcards.kt create mode 100644 compiler/testData/diagnostics/tests/platformTypes/genericVarianceViolation/wildcards.txt diff --git a/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/diagnostics/DefaultErrorMessagesJvm.java b/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/diagnostics/DefaultErrorMessagesJvm.java index c6428484d35..16f4e589ee8 100644 --- a/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/diagnostics/DefaultErrorMessagesJvm.java +++ b/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/diagnostics/DefaultErrorMessagesJvm.java @@ -81,6 +81,8 @@ public class DefaultErrorMessagesJvm implements DefaultErrorMessages.Extension { "Please use the more clear ''::class.java'' syntax to avoid confusion", Renderers.RENDER_TYPE, Renderers.RENDER_TYPE ); + MAP.put(ErrorsJvm.JAVA_TYPE_MISMATCH, + "Java type mismatch expected {1} but found {0}. Use explicit cast", Renderers.RENDER_TYPE, Renderers.RENDER_TYPE); } @NotNull diff --git a/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/diagnostics/ErrorsJvm.java b/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/diagnostics/ErrorsJvm.java index ce9246d1f60..9481e3f2b70 100644 --- a/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/diagnostics/ErrorsJvm.java +++ b/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/diagnostics/ErrorsJvm.java @@ -67,6 +67,7 @@ public interface ErrorsJvm { DiagnosticFactory0 NO_REFLECTION_IN_CLASS_PATH = DiagnosticFactory0.create(WARNING); DiagnosticFactory2 JAVA_CLASS_ON_COMPANION = DiagnosticFactory2.create(WARNING); + DiagnosticFactory2 JAVA_TYPE_MISMATCH = DiagnosticFactory2.create(ERROR); enum NullabilityInformationSource { KOTLIN { diff --git a/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/platform/JavaGenericVarianceViolationTypeChecker.kt b/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/platform/JavaGenericVarianceViolationTypeChecker.kt new file mode 100644 index 00000000000..036e3ee171f --- /dev/null +++ b/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/platform/JavaGenericVarianceViolationTypeChecker.kt @@ -0,0 +1,96 @@ +/* + * Copyright 2010-2015 JetBrains s.r.o. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.jetbrains.kotlin.resolve.jvm.platform + +import org.jetbrains.kotlin.descriptors.ReceiverParameterDescriptor +import org.jetbrains.kotlin.load.java.lazy.types.RawTypeTag +import org.jetbrains.kotlin.psi.JetExpression +import org.jetbrains.kotlin.resolve.calls.checkers.AdditionalTypeChecker +import org.jetbrains.kotlin.resolve.calls.context.CallResolutionContext +import org.jetbrains.kotlin.resolve.calls.context.ResolutionContext +import org.jetbrains.kotlin.resolve.jvm.diagnostics.ErrorsJvm +import org.jetbrains.kotlin.resolve.scopes.receivers.ReceiverValue +import org.jetbrains.kotlin.types.* +import org.jetbrains.kotlin.types.checker.JetTypeChecker +import org.jetbrains.kotlin.types.checker.TypeCheckingProcedure + +public object JavaGenericVarianceViolationTypeChecker : AdditionalTypeChecker { + // Prohibits covariant type argument conversions `List -> (MutableList..List)` when expected type's lower bound is invariant. + // It's needed to prevent accident unsafe covariant conversions of mutable collections. + // + // Example: + // class JavaClass { static void fillWithDefaultObjects(List list); // add Object's to list } + // + // val x: MutableList + // JavaClass.fillWithDefaultObjects(x) // using `x` after this call may lead to CCE + override fun checkType( + expression: JetExpression, + expressionType: JetType, + expressionTypeWithSmartCast: JetType, + c: ResolutionContext<*> + ) { + val expectedType = c.expectedType + if (TypeUtils.noExpectedType(expectedType) || ErrorUtils.containsErrorType(expectedType) || ErrorUtils.containsUninferredParameter(expectedType)) return + + // optimization: if no arguments or flexibility, everything is OK + if (expectedType.arguments.isEmpty() || !expectedType.isFlexible()) return + + val lowerBound = expectedType.flexibility().lowerBound + val upperBound = expectedType.flexibility().upperBound + + // Use site variance projection is always the same for flexible types + if (lowerBound.constructor == upperBound.constructor) return + // Anything is acceptable for raw types + if (expectedType.getCapability() != null) return + + val correspondingSubType = TypeCheckingProcedure.findCorrespondingSupertype(expressionTypeWithSmartCast, lowerBound) ?: return + + assert(lowerBound.arguments.size() == upperBound.arguments.size()) { + "Different arguments count in flexible bounds: " + + "($lowerBound(${lowerBound.arguments.size()})..$upperBound(${upperBound.arguments.size()})" + } + + assert(lowerBound.arguments.size() == correspondingSubType.arguments.size()) { + "Different arguments count in corresponding subtype and supertype: " + + "($lowerBound(${lowerBound.arguments.size()})..$correspondingSubType(${correspondingSubType.arguments.size()})" + } + + + val lowerParameters = lowerBound.constructor.parameters + val upperParameters = upperBound.constructor.parameters + val lowerArguments = lowerBound.arguments + + correspondingSubType.arguments.indices.forEach { + index -> + val lowerArgument = lowerArguments[index] + // Currently we don't have flexible types with different constructors with contravariant arguments + // So check just covariant case + if (lowerParameters[index].variance == Variance.INVARIANT + && upperParameters[index].variance == Variance.OUT_VARIANCE + && lowerArgument.projectionKind != Variance.OUT_VARIANCE + && !JetTypeChecker.DEFAULT.equalTypes(correspondingSubType.arguments[index].type, lowerArgument.type) + ) { + c.trace.report(ErrorsJvm.JAVA_TYPE_MISMATCH.on(expression, expressionTypeWithSmartCast, expectedType)) + } + } + } + + override fun checkReceiver( + receiverParameter: ReceiverParameterDescriptor, + receiverArgument: ReceiverValue, + safeAccess: Boolean, c: CallResolutionContext<*>) { } +} diff --git a/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/platform/JvmPlatformConfigurator.kt b/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/platform/JvmPlatformConfigurator.kt index f9b40563c61..39ecb20f586 100644 --- a/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/platform/JvmPlatformConfigurator.kt +++ b/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/platform/JvmPlatformConfigurator.kt @@ -28,6 +28,7 @@ import org.jetbrains.kotlin.diagnostics.DiagnosticSink import org.jetbrains.kotlin.diagnostics.Errors import org.jetbrains.kotlin.jvm.RuntimeAssertionsTypeChecker import org.jetbrains.kotlin.lexer.JetTokens +import org.jetbrains.kotlin.load.java.lazy.types.RawTypeTag import org.jetbrains.kotlin.load.java.lazy.types.isMarkedNotNull import org.jetbrains.kotlin.load.java.lazy.types.isMarkedNullable import org.jetbrains.kotlin.load.kotlin.JavaAnnotationCallChecker @@ -83,7 +84,8 @@ public object JvmPlatformConfigurator : PlatformConfigurator( additionalTypeCheckers = listOf( JavaNullabilityWarningsChecker(), - RuntimeAssertionsTypeChecker + RuntimeAssertionsTypeChecker, + JavaGenericVarianceViolationTypeChecker ), additionalSymbolUsageValidators = listOf(), diff --git a/compiler/testData/diagnostics/tests/platformTypes/genericVarianceViolation/listSuperType.kt b/compiler/testData/diagnostics/tests/platformTypes/genericVarianceViolation/listSuperType.kt new file mode 100644 index 00000000000..bf945327fce --- /dev/null +++ b/compiler/testData/diagnostics/tests/platformTypes/genericVarianceViolation/listSuperType.kt @@ -0,0 +1,16 @@ +// !DIAGNOSTICS: -UNUSED_VARIABLE +// FILE: A.java + +import java.util.*; + +public class A { + void foo(List x) {} +} +// FILE: main.kt + +abstract class B : MutableList + +fun main(a: A, b: B) { + a.foo(b) + a.foo(b as List) +} diff --git a/compiler/testData/diagnostics/tests/platformTypes/genericVarianceViolation/listSuperType.txt b/compiler/testData/diagnostics/tests/platformTypes/genericVarianceViolation/listSuperType.txt new file mode 100644 index 00000000000..6eccfbf7016 --- /dev/null +++ b/compiler/testData/diagnostics/tests/platformTypes/genericVarianceViolation/listSuperType.txt @@ -0,0 +1,39 @@ +package + +internal fun main(/*0*/ a: A, /*1*/ b: B): kotlin.Unit + +public open class A { + public constructor A() + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public/*package*/ open fun foo(/*0*/ x: kotlin.(Mutable)List!): kotlin.Unit + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} + +internal abstract class B : kotlin.MutableList { + public constructor B() + public abstract override /*1*/ /*fake_override*/ fun add(/*0*/ index: kotlin.Int, /*1*/ element: kotlin.String): kotlin.Unit + public abstract override /*1*/ /*fake_override*/ fun add(/*0*/ e: kotlin.String): kotlin.Boolean + public abstract override /*1*/ /*fake_override*/ fun addAll(/*0*/ c: kotlin.Collection): kotlin.Boolean + public abstract override /*1*/ /*fake_override*/ fun addAll(/*0*/ index: kotlin.Int, /*1*/ c: kotlin.Collection): kotlin.Boolean + public abstract override /*1*/ /*fake_override*/ fun clear(): kotlin.Unit + public abstract override /*1*/ /*fake_override*/ fun contains(/*0*/ o: kotlin.Any?): kotlin.Boolean + public abstract override /*1*/ /*fake_override*/ fun containsAll(/*0*/ c: kotlin.Collection): kotlin.Boolean + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public abstract override /*1*/ /*fake_override*/ fun get(/*0*/ index: kotlin.Int): kotlin.String + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public abstract override /*1*/ /*fake_override*/ fun indexOf(/*0*/ o: kotlin.Any?): kotlin.Int + public abstract override /*1*/ /*fake_override*/ fun isEmpty(): kotlin.Boolean + public abstract override /*1*/ /*fake_override*/ fun iterator(): kotlin.MutableIterator + public abstract override /*1*/ /*fake_override*/ fun lastIndexOf(/*0*/ o: kotlin.Any?): kotlin.Int + public abstract override /*1*/ /*fake_override*/ fun listIterator(): kotlin.MutableListIterator + public abstract override /*1*/ /*fake_override*/ fun listIterator(/*0*/ index: kotlin.Int): kotlin.MutableListIterator + public abstract override /*1*/ /*fake_override*/ fun remove(/*0*/ o: kotlin.Any?): kotlin.Boolean + public abstract override /*1*/ /*fake_override*/ fun remove(/*0*/ index: kotlin.Int): kotlin.String + public abstract override /*1*/ /*fake_override*/ fun removeAll(/*0*/ c: kotlin.Collection): kotlin.Boolean + public abstract override /*1*/ /*fake_override*/ fun retainAll(/*0*/ c: kotlin.Collection): kotlin.Boolean + public abstract override /*1*/ /*fake_override*/ fun set(/*0*/ index: kotlin.Int, /*1*/ element: kotlin.String): kotlin.String + public abstract override /*1*/ /*fake_override*/ fun size(): kotlin.Int + public abstract override /*1*/ /*fake_override*/ fun subList(/*0*/ fromIndex: kotlin.Int, /*1*/ toIndex: kotlin.Int): kotlin.MutableList + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} diff --git a/compiler/testData/diagnostics/tests/platformTypes/genericVarianceViolation/rawTypes.kt b/compiler/testData/diagnostics/tests/platformTypes/genericVarianceViolation/rawTypes.kt new file mode 100644 index 00000000000..6be7f3dcf05 --- /dev/null +++ b/compiler/testData/diagnostics/tests/platformTypes/genericVarianceViolation/rawTypes.kt @@ -0,0 +1,14 @@ +// !DIAGNOSTICS: -UNUSED_VARIABLE +// FILE: A.java + +import java.util.*; + +public class A { + void foo(List x) {} +} +// FILE: main.kt + +fun main(a: A, ml: MutableList, l: List) { + a.foo(ml) + a.foo(l) +} diff --git a/compiler/testData/diagnostics/tests/platformTypes/genericVarianceViolation/rawTypes.txt b/compiler/testData/diagnostics/tests/platformTypes/genericVarianceViolation/rawTypes.txt new file mode 100644 index 00000000000..3be78f779be --- /dev/null +++ b/compiler/testData/diagnostics/tests/platformTypes/genericVarianceViolation/rawTypes.txt @@ -0,0 +1,11 @@ +package + +internal fun main(/*0*/ a: A, /*1*/ ml: kotlin.MutableList, /*2*/ l: kotlin.List): kotlin.Unit + +public open class A { + public constructor A() + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public/*package*/ open fun foo(/*0*/ x: kotlin.(Mutable)List<(raw) kotlin.Any?>!): 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/platformTypes/genericVarianceViolation/simple.kt b/compiler/testData/diagnostics/tests/platformTypes/genericVarianceViolation/simple.kt new file mode 100644 index 00000000000..5c754741f59 --- /dev/null +++ b/compiler/testData/diagnostics/tests/platformTypes/genericVarianceViolation/simple.kt @@ -0,0 +1,67 @@ +// !DIAGNOSTICS: -UNUSED_VARIABLE +// FILE: A.java + +import java.util.*; + +public class A { + void foo(List x) {} + void foo(Iterable x) {} + void foo(Iterator x) {} + void foo(Set x) {} + void foo(Map x) {} + void foo(Map.Entry x) {} + + void foo(List> x) {} +} + +// FILE: main.kt + +fun main( + a: A, + ml: MutableList, l: List, + ms: MutableSet, s: Set, + mm: MutableMap, m: Map, + mme: MutableMap.MutableEntry, me: Map.Entry, + mll: MutableList>, ll: List> +) { + // Lists + a.foo(ml) + a.foo(l) + a.foo(ml as MutableList) + a.foo(l as List) + + // Iterables + val mit: MutableIterable = ml + val it: Iterable = ml + a.foo(mit) + a.foo(it) + + // Iterators + a.foo(ml.iterator()) + a.foo(l.iterator()) + + // Sets + a.foo(ms) + a.foo(s) + a.foo(ms as MutableSet) + a.foo(s as Set) + + // Maps + a.foo(mm) + a.foo(m) + a.foo(mm as MutableMap) + a.foo(m as Map) + + // Map entries + a.foo(mme) + a.foo(me) + a.foo(mme as MutableMap.MutableEntry) + a.foo(me as Map.Entry) + + // Lists of lists + a.foo(mll) + a.foo(ll) + a.foo(mll as MutableList>) + a.foo(ll as List>) + +} diff --git a/compiler/testData/diagnostics/tests/platformTypes/genericVarianceViolation/simple.txt b/compiler/testData/diagnostics/tests/platformTypes/genericVarianceViolation/simple.txt new file mode 100644 index 00000000000..87413edcb6a --- /dev/null +++ b/compiler/testData/diagnostics/tests/platformTypes/genericVarianceViolation/simple.txt @@ -0,0 +1,17 @@ +package + +internal fun main(/*0*/ a: A, /*1*/ ml: kotlin.MutableList, /*2*/ l: kotlin.List, /*3*/ ms: kotlin.MutableSet, /*4*/ s: kotlin.Set, /*5*/ mm: kotlin.MutableMap, /*6*/ m: kotlin.Map, /*7*/ mme: kotlin.MutableMap.MutableEntry, /*8*/ me: kotlin.Map.Entry, /*9*/ mll: kotlin.MutableList>, /*10*/ ll: kotlin.List>): kotlin.Unit + +public open class A { + public constructor A() + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public/*package*/ open fun foo(/*0*/ x: kotlin.(Mutable)Iterable!): kotlin.Unit + public/*package*/ open fun foo(/*0*/ x: kotlin.(Mutable)Iterator!): kotlin.Unit + public/*package*/ open fun foo(/*0*/ x: kotlin.(Mutable)List!>!): kotlin.Unit + public/*package*/ open fun foo(/*0*/ x: kotlin.(Mutable)List!): kotlin.Unit + public/*package*/ open fun foo(/*0*/ x: kotlin.(Mutable)Map.(Mutable)Entry!): kotlin.Unit + public/*package*/ open fun foo(/*0*/ x: kotlin.(Mutable)Map!): kotlin.Unit + public/*package*/ open fun foo(/*0*/ x: kotlin.(Mutable)Set!): 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/platformTypes/genericVarianceViolation/smartCast.kt b/compiler/testData/diagnostics/tests/platformTypes/genericVarianceViolation/smartCast.kt new file mode 100644 index 00000000000..5f88836bdab --- /dev/null +++ b/compiler/testData/diagnostics/tests/platformTypes/genericVarianceViolation/smartCast.kt @@ -0,0 +1,16 @@ +// !DIAGNOSTICS: -UNUSED_VARIABLE +// FILE: A.java + +import java.util.*; + +public class A { + void foo(List x) {} +} +// FILE: main.kt + +fun main(a: A, ml: Any) { + if (ml is MutableList) { + a.foo(ml) + a.foo(ml as List) + } +} diff --git a/compiler/testData/diagnostics/tests/platformTypes/genericVarianceViolation/smartCast.txt b/compiler/testData/diagnostics/tests/platformTypes/genericVarianceViolation/smartCast.txt new file mode 100644 index 00000000000..7d2fa78dc4a --- /dev/null +++ b/compiler/testData/diagnostics/tests/platformTypes/genericVarianceViolation/smartCast.txt @@ -0,0 +1,11 @@ +package + +internal fun main(/*0*/ a: A, /*1*/ ml: kotlin.Any): kotlin.Unit + +public open class A { + public constructor A() + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public/*package*/ open fun foo(/*0*/ x: kotlin.(Mutable)List!): 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/platformTypes/genericVarianceViolation/strangeVariance.kt b/compiler/testData/diagnostics/tests/platformTypes/genericVarianceViolation/strangeVariance.kt new file mode 100644 index 00000000000..73c623589fa --- /dev/null +++ b/compiler/testData/diagnostics/tests/platformTypes/genericVarianceViolation/strangeVariance.kt @@ -0,0 +1,18 @@ +// !DIAGNOSTICS: -UNUSED_VARIABLE +// FILE: A.java + +import java.util.*; + +public class A { + void foo(List x) {} + void bar(List x) {} +} +// FILE: main.kt + +fun main(a: A, ml: MutableList, l: List) { + a.foo(ml) + a.foo(l) + + a.bar(ml) + a.bar(l) +} diff --git a/compiler/testData/diagnostics/tests/platformTypes/genericVarianceViolation/strangeVariance.txt b/compiler/testData/diagnostics/tests/platformTypes/genericVarianceViolation/strangeVariance.txt new file mode 100644 index 00000000000..3e8a5ef5c9c --- /dev/null +++ b/compiler/testData/diagnostics/tests/platformTypes/genericVarianceViolation/strangeVariance.txt @@ -0,0 +1,12 @@ +package + +internal fun main(/*0*/ a: A, /*1*/ ml: kotlin.MutableList, /*2*/ l: kotlin.List): kotlin.Unit + +public open class A { + public constructor A() + public/*package*/ open fun bar(/*0*/ x: kotlin.(Mutable)List!): kotlin.Unit + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public/*package*/ open fun foo(/*0*/ x: kotlin.(Mutable)List!): 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/platformTypes/genericVarianceViolation/userDefinedOut.kt b/compiler/testData/diagnostics/tests/platformTypes/genericVarianceViolation/userDefinedOut.kt new file mode 100644 index 00000000000..f06e8d918d6 --- /dev/null +++ b/compiler/testData/diagnostics/tests/platformTypes/genericVarianceViolation/userDefinedOut.kt @@ -0,0 +1,17 @@ +// !DIAGNOSTICS: -UNUSED_VARIABLE +// FILE: A.java + +import java.util.*; + +public class A { + void foo(Out x) {} + void bar(Out x) {} +} +// FILE: main.kt + +public class Out() + +fun main(a: A, o: Out) { + a.foo(o) + a.bar(o) +} diff --git a/compiler/testData/diagnostics/tests/platformTypes/genericVarianceViolation/userDefinedOut.txt b/compiler/testData/diagnostics/tests/platformTypes/genericVarianceViolation/userDefinedOut.txt new file mode 100644 index 00000000000..edc175bb60f --- /dev/null +++ b/compiler/testData/diagnostics/tests/platformTypes/genericVarianceViolation/userDefinedOut.txt @@ -0,0 +1,19 @@ +package + +internal fun main(/*0*/ a: A, /*1*/ o: Out): kotlin.Unit + +public open class A { + public constructor A() + public/*package*/ open fun bar(/*0*/ x: Out!): kotlin.Unit + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public/*package*/ open fun foo(/*0*/ x: Out!): kotlin.Unit + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} + +public final class Out { + public constructor Out() + 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/platformTypes/genericVarianceViolation/valueFromJava.kt b/compiler/testData/diagnostics/tests/platformTypes/genericVarianceViolation/valueFromJava.kt new file mode 100644 index 00000000000..09777f731c6 --- /dev/null +++ b/compiler/testData/diagnostics/tests/platformTypes/genericVarianceViolation/valueFromJava.kt @@ -0,0 +1,14 @@ +// !DIAGNOSTICS: -UNUSED_VARIABLE +// FILE: A.java + +import java.util.*; + +public class A { + void foo(List x) {} + List bar() {} +} +// FILE: main.kt + +fun main(a: A) { + a.foo(a.bar()) +} diff --git a/compiler/testData/diagnostics/tests/platformTypes/genericVarianceViolation/valueFromJava.txt b/compiler/testData/diagnostics/tests/platformTypes/genericVarianceViolation/valueFromJava.txt new file mode 100644 index 00000000000..2f8a112f1b4 --- /dev/null +++ b/compiler/testData/diagnostics/tests/platformTypes/genericVarianceViolation/valueFromJava.txt @@ -0,0 +1,12 @@ +package + +internal fun main(/*0*/ a: A): kotlin.Unit + +public open class A { + public constructor A() + public/*package*/ open fun bar(): kotlin.(Mutable)List! + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public/*package*/ open fun foo(/*0*/ x: kotlin.(Mutable)List!): 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/platformTypes/genericVarianceViolation/wildcards.kt b/compiler/testData/diagnostics/tests/platformTypes/genericVarianceViolation/wildcards.kt new file mode 100644 index 00000000000..844dc40d09a --- /dev/null +++ b/compiler/testData/diagnostics/tests/platformTypes/genericVarianceViolation/wildcards.kt @@ -0,0 +1,49 @@ +// !DIAGNOSTICS: -UNUSED_VARIABLE +// FILE: A.java + +import java.util.*; + +public class A { + void foo(List x) {} + void foo(Iterable x) {} + void foo(Iterator x) {} + void foo(Set x) {} + void foo(Map x) {} + void foo(Map.Entry x) {} +} + +// FILE: main.kt + +fun main( + a: A, + ml: MutableList, l: List, + ms: MutableSet, s: Set, + mm: MutableMap, m: Map, + mme: MutableMap.MutableEntry, me: Map.Entry +) { + // Lists + a.foo(ml) + a.foo(l) + + // Iterables + val mit: MutableIterable = ml + val it: Iterable = ml + a.foo(mit) + a.foo(it) + + // Iterators + a.foo(ml.iterator()) + a.foo(l.iterator()) + + // Sets + a.foo(ms) + a.foo(s) + + // Maps + a.foo(mm) + a.foo(m) + + // Map entries + a.foo(mme) + a.foo(me) +} diff --git a/compiler/testData/diagnostics/tests/platformTypes/genericVarianceViolation/wildcards.txt b/compiler/testData/diagnostics/tests/platformTypes/genericVarianceViolation/wildcards.txt new file mode 100644 index 00000000000..5a986783a8d --- /dev/null +++ b/compiler/testData/diagnostics/tests/platformTypes/genericVarianceViolation/wildcards.txt @@ -0,0 +1,16 @@ +package + +internal fun main(/*0*/ a: A, /*1*/ ml: kotlin.MutableList, /*2*/ l: kotlin.List, /*3*/ ms: kotlin.MutableSet, /*4*/ s: kotlin.Set, /*5*/ mm: kotlin.MutableMap, /*6*/ m: kotlin.Map, /*7*/ mme: kotlin.MutableMap.MutableEntry, /*8*/ me: kotlin.Map.Entry): kotlin.Unit + +public open class A { + public constructor A() + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public/*package*/ open fun foo(/*0*/ x: kotlin.(Mutable)Iterable<*>!): kotlin.Unit + public/*package*/ open fun foo(/*0*/ x: kotlin.(Mutable)Iterator<*>!): kotlin.Unit + public/*package*/ open fun foo(/*0*/ x: kotlin.(Mutable)List<*>!): kotlin.Unit + public/*package*/ open fun foo(/*0*/ x: kotlin.(Mutable)Map.(Mutable)Entry<*, *>!): kotlin.Unit + public/*package*/ open fun foo(/*0*/ x: kotlin.(Mutable)Map<*, *>!): kotlin.Unit + public/*package*/ open fun foo(/*0*/ x: kotlin.(Mutable)Set<*>!): 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/tests/org/jetbrains/kotlin/checkers/JetDiagnosticsTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/checkers/JetDiagnosticsTestGenerated.java index f3d3c79370d..bab6235a5db 100644 --- a/compiler/tests/org/jetbrains/kotlin/checkers/JetDiagnosticsTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/checkers/JetDiagnosticsTestGenerated.java @@ -10395,6 +10395,63 @@ public class JetDiagnosticsTestGenerated extends AbstractJetDiagnosticsTest { } } + @TestMetadata("compiler/testData/diagnostics/tests/platformTypes/genericVarianceViolation") + @TestDataPath("$PROJECT_ROOT") + @RunWith(JUnit3RunnerWithInners.class) + public static class GenericVarianceViolation extends AbstractJetDiagnosticsTest { + public void testAllFilesPresentInGenericVarianceViolation() throws Exception { + JetTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/diagnostics/tests/platformTypes/genericVarianceViolation"), Pattern.compile("^(.+)\\.kt$"), true); + } + + @TestMetadata("listSuperType.kt") + public void testListSuperType() throws Exception { + String fileName = JetTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/platformTypes/genericVarianceViolation/listSuperType.kt"); + doTest(fileName); + } + + @TestMetadata("rawTypes.kt") + public void testRawTypes() throws Exception { + String fileName = JetTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/platformTypes/genericVarianceViolation/rawTypes.kt"); + doTest(fileName); + } + + @TestMetadata("simple.kt") + public void testSimple() throws Exception { + String fileName = JetTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/platformTypes/genericVarianceViolation/simple.kt"); + doTest(fileName); + } + + @TestMetadata("smartCast.kt") + public void testSmartCast() throws Exception { + String fileName = JetTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/platformTypes/genericVarianceViolation/smartCast.kt"); + doTest(fileName); + } + + @TestMetadata("strangeVariance.kt") + public void testStrangeVariance() throws Exception { + String fileName = JetTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/platformTypes/genericVarianceViolation/strangeVariance.kt"); + doTest(fileName); + } + + @TestMetadata("userDefinedOut.kt") + public void testUserDefinedOut() throws Exception { + String fileName = JetTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/platformTypes/genericVarianceViolation/userDefinedOut.kt"); + doTest(fileName); + } + + @TestMetadata("valueFromJava.kt") + public void testValueFromJava() throws Exception { + String fileName = JetTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/platformTypes/genericVarianceViolation/valueFromJava.kt"); + doTest(fileName); + } + + @TestMetadata("wildcards.kt") + public void testWildcards() throws Exception { + String fileName = JetTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/platformTypes/genericVarianceViolation/wildcards.kt"); + doTest(fileName); + } + } + @TestMetadata("compiler/testData/diagnostics/tests/platformTypes/intersection") @TestDataPath("$PROJECT_ROOT") @RunWith(JUnit3RunnerWithInners.class) diff --git a/core/descriptor.loader.java/src/org/jetbrains/kotlin/load/java/lazy/types/RawType.kt b/core/descriptor.loader.java/src/org/jetbrains/kotlin/load/java/lazy/types/RawType.kt index 5d344bcf58f..d4b9176b3bb 100644 --- a/core/descriptor.loader.java/src/org/jetbrains/kotlin/load/java/lazy/types/RawType.kt +++ b/core/descriptor.loader.java/src/org/jetbrains/kotlin/load/java/lazy/types/RawType.kt @@ -24,6 +24,8 @@ import org.jetbrains.kotlin.renderer.CustomFlexibleRendering import org.jetbrains.kotlin.renderer.DescriptorRenderer import org.jetbrains.kotlin.types.* +public object RawTypeTag : TypeCapability + public object RawTypeCapabilities : TypeCapabilities { private object RawSubstitutionCapability : CustomSubstitutionCapability { override val substitution = RawSubstitution @@ -72,6 +74,7 @@ public object RawTypeCapabilities : TypeCapabilities { return when(capabilityClass) { javaClass() -> RawSubstitutionCapability as T javaClass() -> RawFlexibleRendering as T + javaClass() -> RawTypeTag as T else -> null } } diff --git a/spec-docs/flexible-java-types.md b/spec-docs/flexible-java-types.md index 9fde075f640..f7695044497 100644 --- a/spec-docs/flexible-java-types.md +++ b/spec-docs/flexible-java-types.md @@ -116,6 +116,39 @@ Erase(A) = Raw(A) // `A` is a type constructor without parameters // then it becomes `Foo<(raw) Any!>` inside Erase(A) ``` +## Unsafe covariant conversions + +In case of platform collections their upper bound contains covariant parameter, which means they may behave covariantly even it doesn't meant to do so. + +Example: +```java +class JavaClass { + void addObject(List x) { + x.add(new Object()); + } +} +``` + +```kotlin +val x: MutableList = arrayListOf() +JavaClass.addObject(x) // Ok +x[0].length() // ClassCastException +``` + +This happens because `MutableList` <: `List` <: `List` and by subtyping rule for flexible types `MutableList` <: `(Mutable)Collection!` follows. + +While it's legal from point of view of type system, in most cases such conversion is unintended and must be prohibited when being made implicitly. + +So implicit covariant conversion by i-th argument from type `source` to `target` is prohibited when: +- `target` is flexible type with invariant i-th *parameter* of lower bound (when same parameter in upper bound may be covariant) +- i-th *argument* of `target`'s lower bound is invariant (which means it declared as invariant in Java) +- type of i-th argument of `source` is not *equal* to same argument in `target`'s lower bound. + +NOTE: Such conversion still may be done explicitly, with covariant upcast. E.g. for upper case: + ``` + JavaClass.addObject(x as List) // No unchecked cast warning + ``` + ## Overriding When overriding a method from a Java class, one can not use flexible type, only replace them with denotable Kotlin types: