Merge pull request #9571 from smowton/smowton/fix/array-variance-lowering

Kotlin: Implement array type variance lowering
2025-12-22 19:56:32 +01:00 · 2022-06-22 13:38:21 +01:00
parent cc354caa1f 2d57d3aa78
commit 44cf260762
5 changed files with 265 additions and 6 deletions
--- a/java/kotlin-extractor/src/main/kotlin/KotlinUsesExtractor.kt
+++ b/java/kotlin-extractor/src/main/kotlin/KotlinUsesExtractor.kt
@@ -637,14 +637,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<in X> as Object[], Array<Array<in X>> as Object[][] etc.
+                        componentType = replaceComponentTypeWithAny(s, dimensions - 1)
+                        elementType = pluginContext.irBuiltIns.anyType as IrSimpleType
+                        break
+                    }
                    elementType = elementType.getArrayElementType(pluginContext.irBuiltIns)
                }

@@ -857,13 +874,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<in X> 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
        }

--- a/java/ql/test/kotlin/library-tests/arrays-with-variances/User.java
+++ 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.
+
+  }
+
+}
--- a/java/ql/test/kotlin/library-tests/arrays-with-variances/takesArrayList.kt
+++ 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<CharSequence>) { }
+  fun outArray(a: Array<out CharSequence>) { }
+  fun inArray(a: Array<in CharSequence>) { }
+
+  fun invarInvarArray(a: Array<Array<CharSequence>>) { }
+  fun invarOutArray(a: Array<Array<out CharSequence>>) { }
+  fun invarInArray(a: Array<Array<in CharSequence>>) { }
+
+  fun outInvarArray(a: Array<out Array<CharSequence>>) { }
+  fun outOutArray(a: Array<out Array<out CharSequence>>) { }
+  fun outInArray(a: Array<out Array<in CharSequence>>) { }
+
+  fun inInvarArray(a: Array<in Array<CharSequence>>) { }
+  fun inOutArray(a: Array<in Array<out CharSequence>>) { }
+  fun inInArray(a: Array<in Array<in CharSequence>>) { }
+
+  // Test how Array type arguments with a variance should acquire implicit wildcards:
+  fun invarArrayList(l: List<Array<CharSequence>>) { }
+  fun outArrayList(l: List<Array<out CharSequence>>) { }
+  fun inArrayList(l: List<Array<in CharSequence>>) { }
+
+  // Check the cases of nested arrays:
+  fun invarInvarArrayList(l: List<Array<Array<CharSequence>>>) { }
+  fun invarOutArrayList(l: List<Array<Array<out CharSequence>>>) { }
+  fun invarInArrayList(l: List<Array<Array<in CharSequence>>>) { }
+  fun outInvarArrayList(l: List<Array<out Array<CharSequence>>>) { }
+  fun outOutArrayList(l: List<Array<out Array<out CharSequence>>>) { }
+  fun outInArrayList(l: List<Array<out Array<in CharSequence>>>) { }
+  fun inInvarArrayList(l: List<Array<in Array<CharSequence>>>) { }
+  fun inOutArrayList(l: List<Array<in Array<out CharSequence>>>) { }
+  fun inInArrayList(l: List<Array<in Array<in CharSequence>>>) { }
+
+  // Now check all of that again for Comparable, whose type parameter is contravariant:
+  fun invarArrayComparable(c: Comparable<Array<CharSequence>>) { }
+  fun outArrayComparable(c: Comparable<Array<out CharSequence>>) { }
+  fun inArrayComparable(c: Comparable<Array<in CharSequence>>) { }
+
+  fun invarInvarArrayComparable(c: Comparable<Array<Array<CharSequence>>>) { }
+  fun invarOutArrayComparable(c: Comparable<Array<Array<out CharSequence>>>) { }
+  fun invarInArrayComparable(c: Comparable<Array<Array<in CharSequence>>>) { }
+  fun outInvarArrayComparable(c: Comparable<Array<out Array<CharSequence>>>) { }
+  fun outOutArrayComparable(c: Comparable<Array<out Array<out CharSequence>>>) { }
+  fun outInArrayComparable(c: Comparable<Array<out Array<in CharSequence>>>) { }
+  fun inInvarArrayComparable(c: Comparable<Array<in Array<CharSequence>>>) { }
+  fun inOutArrayComparable(c: Comparable<Array<in Array<out CharSequence>>>) { }
+  fun inInArrayComparable(c: Comparable<Array<in Array<in CharSequence>>>) { }
+
+  // ... 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<Array<String>>) { }
+  fun outArrayListFinal(l: List<Array<out String>>) { }
+  fun inArrayListFinal(l: List<Array<in String>>) { }
+
+  fun invarInvarArrayListFinal(l: List<Array<Array<String>>>) { }
+  fun invarOutArrayListFinal(l: List<Array<Array<out String>>>) { }
+  fun invarInArrayListFinal(l: List<Array<Array<in String>>>) { }
+  fun outInvarArrayListFinal(l: List<Array<out Array<String>>>) { }
+  fun outOutArrayListFinal(l: List<Array<out Array<out String>>>) { }
+  fun outInArrayListFinal(l: List<Array<out Array<in String>>>) { }
+  fun inInvarArrayListFinal(l: List<Array<in Array<String>>>) { }
+  fun inOutArrayListFinal(l: List<Array<in Array<out String>>>) { }
+  fun inInArrayListFinal(l: List<Array<in Array<in String>>>) { }
+
+  fun invarArrayComparableFinal(c: Comparable<Array<String>>) { }
+  fun outArrayComparableFinal(c: Comparable<Array<out String>>) { }
+  fun inArrayComparableFinal(c: Comparable<Array<in String>>) { }
+
+  fun invarInvarArrayComparableFinal(c: Comparable<Array<Array<String>>>) { }
+  fun invarOutArrayComparableFinal(c: Comparable<Array<Array<out String>>>) { }
+  fun invarInArrayComparableFinal(c: Comparable<Array<Array<in String>>>) { }
+  fun outInvarArrayComparableFinal(c: Comparable<Array<out Array<String>>>) { }
+  fun outOutArrayComparableFinal(c: Comparable<Array<out Array<out String>>>) { }
+  fun outInArrayComparableFinal(c: Comparable<Array<out Array<in String>>>) { }
+  fun inInvarArrayComparableFinal(c: Comparable<Array<in Array<String>>>) { }
+  fun inOutArrayComparableFinal(c: Comparable<Array<in Array<out String>>>) { }
+  fun inInArrayComparableFinal(c: Comparable<Array<in Array<in String>>>) { }
+
+  // ... and duplicate it once more with Any as the array element, which can suppress adding `? super ...`
+  fun invarArrayListAny(l: List<Array<Any>>) { }
+  fun outArrayListAny(l: List<Array<out Any>>) { }
+  fun inArrayListAny(l: List<Array<in Any>>) { }
+
+  fun invarInvarArrayListAny(l: List<Array<Array<Any>>>) { }
+  fun invarOutArrayListAny(l: List<Array<Array<out Any>>>) { }
+  fun invarInArrayListAny(l: List<Array<Array<in Any>>>) { }
+  fun outInvarArrayListAny(l: List<Array<out Array<Any>>>) { }
+  fun outOutArrayListAny(l: List<Array<out Array<out Any>>>) { }
+  fun outInArrayListAny(l: List<Array<out Array<in Any>>>) { }
+  fun inInvarArrayListAny(l: List<Array<in Array<Any>>>) { }
+  fun inOutArrayListAny(l: List<Array<in Array<out Any>>>) { }
+  fun inInArrayListAny(l: List<Array<in Array<in Any>>>) { }
+
+  fun invarArrayComparableAny(c: Comparable<Array<Any>>) { }
+  fun outArrayComparableAny(c: Comparable<Array<out Any>>) { }
+  fun inArrayComparableAny(c: Comparable<Array<in Any>>) { }
+
+  fun invarInvarArrayComparableAny(c: Comparable<Array<Array<Any>>>) { }
+  fun invarOutArrayComparableAny(c: Comparable<Array<Array<out Any>>>) { }
+  fun invarInArrayComparableAny(c: Comparable<Array<Array<in Any>>>) { }
+  fun outInvarArrayComparableAny(c: Comparable<Array<out Array<Any>>>) { }
+  fun outOutArrayComparableAny(c: Comparable<Array<out Array<out Any>>>) { }
+  fun outInArrayComparableAny(c: Comparable<Array<out Array<in Any>>>) { }
+  fun inInvarArrayComparableAny(c: Comparable<Array<in Array<Any>>>) { }
+  fun inOutArrayComparableAny(c: Comparable<Array<in Array<out Any>>>) { }
+  fun inInArrayComparableAny(c: Comparable<Array<in Array<in Any>>>) { }
+
+}
--- a/java/ql/test/kotlin/library-tests/arrays-with-variances/test.expected
+++ b/java/ql/test/kotlin/library-tests/arrays-with-variances/test.expected
@@ -0,0 +1,86 @@
+broken
+#select
+| inArray | Object[] |
+| inArrayComparable | Comparable<? super Object[]> |
+| inArrayComparableAny | Comparable<? super Object[]> |
+| inArrayComparableFinal | Comparable<? super Object[]> |
+| inArrayList | List<? extends Object[]> |
+| inArrayListAny | List<Object[]> |
+| inArrayListFinal | List<? extends Object[]> |
+| inInArray | Object[] |
+| inInArrayComparable | Comparable<? super Object[]> |
+| inInArrayComparableAny | Comparable<? super Object[]> |
+| inInArrayComparableFinal | Comparable<? super Object[]> |
+| inInArrayList | List<? extends Object[]> |
+| inInArrayListAny | List<? extends Object[]> |
+| inInArrayListFinal | List<? extends Object[]> |
+| inInvarArray | Object[] |
+| inInvarArrayComparable | Comparable<? super Object[]> |
+| inInvarArrayComparableAny | Comparable<? super Object[]> |
+| inInvarArrayComparableFinal | Comparable<? super Object[]> |
+| inInvarArrayList | List<? extends Object[]> |
+| inInvarArrayListAny | List<? extends Object[]> |
+| inInvarArrayListFinal | List<? extends Object[]> |
+| inOutArray | Object[] |
+| inOutArrayComparable | Comparable<? super Object[]> |
+| inOutArrayComparableAny | Comparable<? super Object[]> |
+| inOutArrayComparableFinal | Comparable<? super Object[]> |
+| inOutArrayList | List<? extends Object[]> |
+| inOutArrayListAny | List<? extends Object[]> |
+| inOutArrayListFinal | List<? extends Object[]> |
+| invarArray | CharSequence[] |
+| invarArrayComparable | Comparable<? super CharSequence[]> |
+| invarArrayComparableAny | Comparable<? super Object[]> |
+| invarArrayComparableFinal | Comparable<? super String[]> |
+| invarArrayList | List<CharSequence[]> |
+| invarArrayListAny | List<Object[]> |
+| invarArrayListFinal | List<String[]> |
+| invarInArray | Object[][] |
+| invarInArrayComparable | Comparable<? super Object[][]> |
+| invarInArrayComparableAny | Comparable<? super Object[][]> |
+| invarInArrayComparableFinal | Comparable<? super Object[][]> |
+| invarInArrayList | List<Object[][]> |
+| invarInArrayListAny | List<Object[][]> |
+| invarInArrayListFinal | List<Object[][]> |
+| invarInvarArray | CharSequence[][] |
+| invarInvarArrayComparable | Comparable<? super CharSequence[][]> |
+| invarInvarArrayComparableAny | Comparable<? super Object[][]> |
+| invarInvarArrayComparableFinal | Comparable<? super String[][]> |
+| invarInvarArrayList | List<CharSequence[][]> |
+| invarInvarArrayListAny | List<Object[][]> |
+| invarInvarArrayListFinal | List<String[][]> |
+| invarOutArray | CharSequence[][] |
+| invarOutArrayComparable | Comparable<? super CharSequence[][]> |
+| invarOutArrayComparableAny | Comparable<? super Object[][]> |
+| invarOutArrayComparableFinal | Comparable<? super String[][]> |
+| invarOutArrayList | List<CharSequence[][]> |
+| invarOutArrayListAny | List<Object[][]> |
+| invarOutArrayListFinal | List<String[][]> |
+| outArray | CharSequence[] |
+| outArrayComparable | Comparable<? super CharSequence[]> |
+| outArrayComparableAny | Comparable<? super Object[]> |
+| outArrayComparableFinal | Comparable<? super String[]> |
+| outArrayList | List<? extends CharSequence[]> |
+| outArrayListAny | List<? extends Object[]> |
+| outArrayListFinal | List<String[]> |
+| outInArray | Object[][] |
+| outInArrayComparable | Comparable<? super Object[][]> |
+| outInArrayComparableAny | Comparable<? super Object[][]> |
+| outInArrayComparableFinal | Comparable<? super Object[][]> |
+| outInArrayList | List<? extends Object[][]> |
+| outInArrayListAny | List<Object[][]> |
+| outInArrayListFinal | List<? extends Object[][]> |
+| outInvarArray | CharSequence[][] |
+| outInvarArrayComparable | Comparable<? super CharSequence[][]> |
+| outInvarArrayComparableAny | Comparable<? super Object[][]> |
+| outInvarArrayComparableFinal | Comparable<? super String[][]> |
+| outInvarArrayList | List<CharSequence[][]> |
+| outInvarArrayListAny | List<Object[][]> |
+| outInvarArrayListFinal | List<String[][]> |
+| outOutArray | CharSequence[][] |
+| outOutArrayComparable | Comparable<? super CharSequence[][]> |
+| outOutArrayComparableAny | Comparable<? super Object[][]> |
+| outOutArrayComparableFinal | Comparable<? super String[][]> |
+| outOutArrayList | List<? extends CharSequence[][]> |
+| outOutArrayListAny | List<? extends Object[][]> |
+| outOutArrayListFinal | List<String[][]> |
--- a/java/ql/test/kotlin/library-tests/arrays-with-variances/test.ql
+++ 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()