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()