From 2d57d3aa782fd2fc11024729ed5469a573122801 Mon Sep 17 00:00:00 2001 From: Chris Smowton Date: Wed, 15 Jun 2022 17:47:19 +0100 Subject: [PATCH] Implement array type variance lowering Kotlin permits introducing a `? extends ...` wildcard against an Array even though the class is final, so long as its argument itself can be extended (i.e. isn't final or is another array type satisfying this condition). Contravariant arrays get lowered to Object[], and are subject to automatic `extends` wildcard introduction, unless their element type was already Any. --- .../src/main/kotlin/KotlinUsesExtractor.kt | 49 +++++++- .../arrays-with-variances/User.java | 11 ++ .../arrays-with-variances/takesArrayList.kt | 112 ++++++++++++++++++ .../arrays-with-variances/test.expected | 86 ++++++++++++++ .../arrays-with-variances/test.ql | 13 ++ 5 files changed, 265 insertions(+), 6 deletions(-) create mode 100644 java/ql/test/kotlin/library-tests/arrays-with-variances/User.java create mode 100644 java/ql/test/kotlin/library-tests/arrays-with-variances/takesArrayList.kt create mode 100644 java/ql/test/kotlin/library-tests/arrays-with-variances/test.expected create mode 100644 java/ql/test/kotlin/library-tests/arrays-with-variances/test.ql diff --git a/java/kotlin-extractor/src/main/kotlin/KotlinUsesExtractor.kt b/java/kotlin-extractor/src/main/kotlin/KotlinUsesExtractor.kt index 127f8b2f83e..85406ff4426 100644 --- a/java/kotlin-extractor/src/main/kotlin/KotlinUsesExtractor.kt +++ b/java/kotlin-extractor/src/main/kotlin/KotlinUsesExtractor.kt @@ -706,14 +706,31 @@ open class KotlinUsesExtractor( ) (s.isBoxedArray && s.arguments.isNotEmpty()) || s.isPrimitiveArray() -> { - var dimensions = 1 - var isPrimitiveArray = s.isPrimitiveArray() - val componentType = s.getArrayElementType(pluginContext.irBuiltIns) - var elementType = componentType + + fun replaceComponentTypeWithAny(t: IrSimpleType, dimensions: Int): IrSimpleType = + if (dimensions == 0) + pluginContext.irBuiltIns.anyType as IrSimpleType + else + t.toBuilder().also { it.arguments = (it.arguments[0] as IrTypeProjection) + .let { oldArg -> + listOf(makeTypeProjection(replaceComponentTypeWithAny(oldArg.type as IrSimpleType, dimensions - 1), oldArg.variance)) + } + }.buildSimpleType() + + var componentType = s.getArrayElementType(pluginContext.irBuiltIns) + var isPrimitiveArray = false + var dimensions = 0 + var elementType: IrType = s while (elementType.isBoxedArray || elementType.isPrimitiveArray()) { dimensions++ - if(elementType.isPrimitiveArray()) + if (elementType.isPrimitiveArray()) isPrimitiveArray = true + if (((elementType as IrSimpleType).arguments.singleOrNull() as? IrTypeProjection)?.variance == Variance.IN_VARIANCE) { + // Because Java's arrays are covariant, Kotlin will render Array as Object[], Array> as Object[][] etc. + componentType = replaceComponentTypeWithAny(s, dimensions - 1) + elementType = pluginContext.irBuiltIns.anyType as IrSimpleType + break + } elementType = elementType.getArrayElementType(pluginContext.irBuiltIns) } @@ -926,13 +943,33 @@ open class KotlinUsesExtractor( private val jvmWildcardAnnotation = FqName("kotlin.jvm.JvmWildcard") private val jvmWildcardSuppressionAnnotaton = FqName("kotlin.jvm.JvmSuppressWildcards") + private fun arrayExtendsAdditionAllowed(t: IrSimpleType): Boolean = + // Note the array special case includes Array<*>, which does permit adding `? extends ...` (making `? extends Object[]` in that case) + // Surprisingly Array does permit this as well, though the contravariant array lowers to Object[] so this ends up `? extends Object[]` as well. + t.arguments[0].let { + when (it) { + is IrTypeProjection -> when (it.variance) { + Variance.INVARIANT -> false + Variance.IN_VARIANCE -> !(it.type.isAny() || it.type.isNullableAny()) + Variance.OUT_VARIANCE -> extendsAdditionAllowed(it.type) + } + else -> true + } + } + + private fun extendsAdditionAllowed(t: IrType) = + if (t.isBoxedArray) + arrayExtendsAdditionAllowed(t as IrSimpleType) + else + ((t as? IrSimpleType)?.classOrNull?.owner?.isFinalClass) != true + private fun wildcardAdditionAllowed(v: Variance, t: IrType, addByDefault: Boolean) = when { t.hasAnnotation(jvmWildcardAnnotation) -> true !addByDefault -> false t.hasAnnotation(jvmWildcardSuppressionAnnotaton) -> false v == Variance.IN_VARIANCE -> !(t.isNullableAny() || t.isAny()) - v == Variance.OUT_VARIANCE -> ((t as? IrSimpleType)?.classOrNull?.owner?.isFinalClass) != true + v == Variance.OUT_VARIANCE -> extendsAdditionAllowed(t) else -> false } diff --git a/java/ql/test/kotlin/library-tests/arrays-with-variances/User.java b/java/ql/test/kotlin/library-tests/arrays-with-variances/User.java new file mode 100644 index 00000000000..d3c00088d13 --- /dev/null +++ b/java/ql/test/kotlin/library-tests/arrays-with-variances/User.java @@ -0,0 +1,11 @@ +public class User { + + public static void test() { + + TakesArrayList tal = new TakesArrayList(); + tal.invarArray(null); + // Using one method suffices to get the extractor to describe all the methods defined on takesArrayList. + + } + +} diff --git a/java/ql/test/kotlin/library-tests/arrays-with-variances/takesArrayList.kt b/java/ql/test/kotlin/library-tests/arrays-with-variances/takesArrayList.kt new file mode 100644 index 00000000000..1278d866d84 --- /dev/null +++ b/java/ql/test/kotlin/library-tests/arrays-with-variances/takesArrayList.kt @@ -0,0 +1,112 @@ +public class TakesArrayList { + + // There is a Java user to flag up any problems, as if the .class file differs from my + // claimed types above we'll see two overloads and two parameter types instead of one. + + // Test how Array types with a variance should be lowered: + fun invarArray(a: Array) { } + fun outArray(a: Array) { } + fun inArray(a: Array) { } + + fun invarInvarArray(a: Array>) { } + fun invarOutArray(a: Array>) { } + fun invarInArray(a: Array>) { } + + fun outInvarArray(a: Array>) { } + fun outOutArray(a: Array>) { } + fun outInArray(a: Array>) { } + + fun inInvarArray(a: Array>) { } + fun inOutArray(a: Array>) { } + fun inInArray(a: Array>) { } + + // Test how Array type arguments with a variance should acquire implicit wildcards: + fun invarArrayList(l: List>) { } + fun outArrayList(l: List>) { } + fun inArrayList(l: List>) { } + + // Check the cases of nested arrays: + fun invarInvarArrayList(l: List>>) { } + fun invarOutArrayList(l: List>>) { } + fun invarInArrayList(l: List>>) { } + fun outInvarArrayList(l: List>>) { } + fun outOutArrayList(l: List>>) { } + fun outInArrayList(l: List>>) { } + fun inInvarArrayList(l: List>>) { } + fun inOutArrayList(l: List>>) { } + fun inInArrayList(l: List>>) { } + + // Now check all of that again for Comparable, whose type parameter is contravariant: + fun invarArrayComparable(c: Comparable>) { } + fun outArrayComparable(c: Comparable>) { } + fun inArrayComparable(c: Comparable>) { } + + fun invarInvarArrayComparable(c: Comparable>>) { } + fun invarOutArrayComparable(c: Comparable>>) { } + fun invarInArrayComparable(c: Comparable>>) { } + fun outInvarArrayComparable(c: Comparable>>) { } + fun outOutArrayComparable(c: Comparable>>) { } + fun outInArrayComparable(c: Comparable>>) { } + fun inInvarArrayComparable(c: Comparable>>) { } + fun inOutArrayComparable(c: Comparable>>) { } + fun inInArrayComparable(c: Comparable>>) { } + + // ... duplicate all of that for a final array element (I choose String as a final type), which sometimes suppresses addition of `? extends ...` + fun invarArrayListFinal(l: List>) { } + fun outArrayListFinal(l: List>) { } + fun inArrayListFinal(l: List>) { } + + fun invarInvarArrayListFinal(l: List>>) { } + fun invarOutArrayListFinal(l: List>>) { } + fun invarInArrayListFinal(l: List>>) { } + fun outInvarArrayListFinal(l: List>>) { } + fun outOutArrayListFinal(l: List>>) { } + fun outInArrayListFinal(l: List>>) { } + fun inInvarArrayListFinal(l: List>>) { } + fun inOutArrayListFinal(l: List>>) { } + fun inInArrayListFinal(l: List>>) { } + + fun invarArrayComparableFinal(c: Comparable>) { } + fun outArrayComparableFinal(c: Comparable>) { } + fun inArrayComparableFinal(c: Comparable>) { } + + fun invarInvarArrayComparableFinal(c: Comparable>>) { } + fun invarOutArrayComparableFinal(c: Comparable>>) { } + fun invarInArrayComparableFinal(c: Comparable>>) { } + fun outInvarArrayComparableFinal(c: Comparable>>) { } + fun outOutArrayComparableFinal(c: Comparable>>) { } + fun outInArrayComparableFinal(c: Comparable>>) { } + fun inInvarArrayComparableFinal(c: Comparable>>) { } + fun inOutArrayComparableFinal(c: Comparable>>) { } + fun inInArrayComparableFinal(c: Comparable>>) { } + + // ... and duplicate it once more with Any as the array element, which can suppress adding `? super ...` + fun invarArrayListAny(l: List>) { } + fun outArrayListAny(l: List>) { } + fun inArrayListAny(l: List>) { } + + fun invarInvarArrayListAny(l: List>>) { } + fun invarOutArrayListAny(l: List>>) { } + fun invarInArrayListAny(l: List>>) { } + fun outInvarArrayListAny(l: List>>) { } + fun outOutArrayListAny(l: List>>) { } + fun outInArrayListAny(l: List>>) { } + fun inInvarArrayListAny(l: List>>) { } + fun inOutArrayListAny(l: List>>) { } + fun inInArrayListAny(l: List>>) { } + + fun invarArrayComparableAny(c: Comparable>) { } + fun outArrayComparableAny(c: Comparable>) { } + fun inArrayComparableAny(c: Comparable>) { } + + fun invarInvarArrayComparableAny(c: Comparable>>) { } + fun invarOutArrayComparableAny(c: Comparable>>) { } + fun invarInArrayComparableAny(c: Comparable>>) { } + fun outInvarArrayComparableAny(c: Comparable>>) { } + fun outOutArrayComparableAny(c: Comparable>>) { } + fun outInArrayComparableAny(c: Comparable>>) { } + fun inInvarArrayComparableAny(c: Comparable>>) { } + fun inOutArrayComparableAny(c: Comparable>>) { } + fun inInArrayComparableAny(c: Comparable>>) { } + +} diff --git a/java/ql/test/kotlin/library-tests/arrays-with-variances/test.expected b/java/ql/test/kotlin/library-tests/arrays-with-variances/test.expected new file mode 100644 index 00000000000..d552925819e --- /dev/null +++ b/java/ql/test/kotlin/library-tests/arrays-with-variances/test.expected @@ -0,0 +1,86 @@ +broken +#select +| inArray | Object[] | +| inArrayComparable | Comparable | +| inArrayComparableAny | Comparable | +| inArrayComparableFinal | Comparable | +| inArrayList | List | +| inArrayListAny | List | +| inArrayListFinal | List | +| inInArray | Object[] | +| inInArrayComparable | Comparable | +| inInArrayComparableAny | Comparable | +| inInArrayComparableFinal | Comparable | +| inInArrayList | List | +| inInArrayListAny | List | +| inInArrayListFinal | List | +| inInvarArray | Object[] | +| inInvarArrayComparable | Comparable | +| inInvarArrayComparableAny | Comparable | +| inInvarArrayComparableFinal | Comparable | +| inInvarArrayList | List | +| inInvarArrayListAny | List | +| inInvarArrayListFinal | List | +| inOutArray | Object[] | +| inOutArrayComparable | Comparable | +| inOutArrayComparableAny | Comparable | +| inOutArrayComparableFinal | Comparable | +| inOutArrayList | List | +| inOutArrayListAny | List | +| inOutArrayListFinal | List | +| invarArray | CharSequence[] | +| invarArrayComparable | Comparable | +| invarArrayComparableAny | Comparable | +| invarArrayComparableFinal | Comparable | +| invarArrayList | List | +| invarArrayListAny | List | +| invarArrayListFinal | List | +| invarInArray | Object[][] | +| invarInArrayComparable | Comparable | +| invarInArrayComparableAny | Comparable | +| invarInArrayComparableFinal | Comparable | +| invarInArrayList | List | +| invarInArrayListAny | List | +| invarInArrayListFinal | List | +| invarInvarArray | CharSequence[][] | +| invarInvarArrayComparable | Comparable | +| invarInvarArrayComparableAny | Comparable | +| invarInvarArrayComparableFinal | Comparable | +| invarInvarArrayList | List | +| invarInvarArrayListAny | List | +| invarInvarArrayListFinal | List | +| invarOutArray | CharSequence[][] | +| invarOutArrayComparable | Comparable | +| invarOutArrayComparableAny | Comparable | +| invarOutArrayComparableFinal | Comparable | +| invarOutArrayList | List | +| invarOutArrayListAny | List | +| invarOutArrayListFinal | List | +| outArray | CharSequence[] | +| outArrayComparable | Comparable | +| outArrayComparableAny | Comparable | +| outArrayComparableFinal | Comparable | +| outArrayList | List | +| outArrayListAny | List | +| outArrayListFinal | List | +| outInArray | Object[][] | +| outInArrayComparable | Comparable | +| outInArrayComparableAny | Comparable | +| outInArrayComparableFinal | Comparable | +| outInArrayList | List | +| outInArrayListAny | List | +| outInArrayListFinal | List | +| outInvarArray | CharSequence[][] | +| outInvarArrayComparable | Comparable | +| outInvarArrayComparableAny | Comparable | +| outInvarArrayComparableFinal | Comparable | +| outInvarArrayList | List | +| outInvarArrayListAny | List | +| outInvarArrayListFinal | List | +| outOutArray | CharSequence[][] | +| outOutArrayComparable | Comparable | +| outOutArrayComparableAny | Comparable | +| outOutArrayComparableFinal | Comparable | +| outOutArrayList | List | +| outOutArrayListAny | List | +| outOutArrayListFinal | List | diff --git a/java/ql/test/kotlin/library-tests/arrays-with-variances/test.ql b/java/ql/test/kotlin/library-tests/arrays-with-variances/test.ql new file mode 100644 index 00000000000..92c138cc0f2 --- /dev/null +++ b/java/ql/test/kotlin/library-tests/arrays-with-variances/test.ql @@ -0,0 +1,13 @@ +import java + +class InterestingMethod extends Method { + InterestingMethod() { this.getDeclaringType().getName() = "TakesArrayList" } +} + +query predicate broken(string methodName) { + methodName = any(InterestingMethod m).getName() and + count(Type t, InterestingMethod m | methodName = m.getName() and t = m.getAParamType() | t) != 1 +} + +from InterestingMethod m +select m.getName(), m.getAParamType().toString()