Merge pull request github#11017 from smowton/smowton/fix/kotlin-wildcard-suppression-annotation

Kotlin: fix wildcard suppression where the annotation applies to a parent type/argument.

Author: smowton

java/kotlin-extractor/src/main/kotlin/KotlinFileExtractor.kt

@@ -691,7 +691,7 @@ open class KotlinFileExtractor(
                     null
             } ?: vp.type
             val javaType = (vp.parent as? IrFunction)?.let { getJavaCallable(it)?.let { jCallable -> getJavaValueParameterType(jCallable, idx) } }
-            val typeWithWildcards = addJavaLoweringWildcards(maybeAlteredType, !hasWildcardSuppressionAnnotation(vp), javaType)
+            val typeWithWildcards = addJavaLoweringWildcards(maybeAlteredType, !getInnermostWildcardSupppressionAnnotation(vp), javaType)
             val substitutedType = typeSubstitution?.let { it(typeWithWildcards, TypeContext.OTHER, pluginContext) } ?: typeWithWildcards
             val id = useValueParameter(vp, parent)
             if (extractTypeAccess) {

java/kotlin-extractor/src/main/kotlin/KotlinUsesExtractor.kt

@@ -964,17 +964,24 @@ open class KotlinUsesExtractor(
         when {
             t.hasAnnotation(jvmWildcardAnnotation) -> true
             !addByDefault -> false
-            t.hasAnnotation(jvmWildcardSuppressionAnnotation) -> false
             // If a Java declaration specifies a variance, introduce it even if it's pointless (e.g. ? extends FinalClass, or ? super Object)
             javaVariance == v -> true
             v == Variance.IN_VARIANCE -> !(t.isNullableAny() || t.isAny())
             v == Variance.OUT_VARIANCE -> extendsAdditionAllowed(t)
             else -> false
         }
 
+    // Returns true if `t` has `@JvmSuppressWildcards` or `@JvmSuppressWildcards(true)`,
+    // false if it has `@JvmSuppressWildcards(false)`,
+    // and null if the annotation is not present.
+    @Suppress("UNCHECKED_CAST")
+    private fun getWildcardSuppressionDirective(t: IrAnnotationContainer) =
+        t.getAnnotation(jvmWildcardSuppressionAnnotation)?.let { (it.getValueArgument(0) as? IrConst<Boolean>)?.value ?: true }
+
     private fun addJavaLoweringArgumentWildcards(p: IrTypeParameter, t: IrTypeArgument, addByDefault: Boolean, javaType: JavaType?): IrTypeArgument =
         (t as? IrTypeProjection)?.let {
-            val newBase = addJavaLoweringWildcards(it.type, addByDefault, javaType)
+            val newAddByDefault = getWildcardSuppressionDirective(it.type)?.not() ?: addByDefault
+            val newBase = addJavaLoweringWildcards(it.type, newAddByDefault, javaType)
             // Note javaVariance == null means we don't have a Java type to conform to -- for example if this is a Kotlin source definition.
             val javaVariance = javaType?.let { jType ->
                 when (jType) {
@@ -989,7 +996,7 @@ open class KotlinUsesExtractor(
                     // For example, Java might declare f(Comparable<CharSequence> cs), in which case we shouldn't add a `? super ...`
                     // wildcard. Note if javaType is unknown (e.g. this is a Kotlin source element), we assume wildcards should be added.
                     (javaVariance == null || javaVariance == p.variance) &&
-                    wildcardAdditionAllowed(p.variance, it.type, addByDefault, javaVariance))
+                    wildcardAdditionAllowed(p.variance, it.type, newAddByDefault, javaVariance))
                     p.variance
                 else
                     it.variance
@@ -1008,12 +1015,13 @@ open class KotlinUsesExtractor(
 
     fun addJavaLoweringWildcards(t: IrType, addByDefault: Boolean, javaType: JavaType?): IrType =
         (t as? IrSimpleType)?.let {
+            val newAddByDefault = getWildcardSuppressionDirective(t)?.not() ?: addByDefault
             val typeParams = it.classOrNull?.owner?.typeParameters ?: return t
             val newArgs = typeParams.zip(it.arguments).mapIndexed { idx, pair ->
                 addJavaLoweringArgumentWildcards(
                     pair.first,
                     pair.second,
-                    addByDefault,
+                    newAddByDefault,
                     javaType?.let { jt -> getJavaTypeArgument(jt, idx) }
                 )
             }
@@ -1061,7 +1069,7 @@ open class KotlinUsesExtractor(
             classTypeArgsIncludingOuterClasses,
             overridesCollectionsMethodWithAlteredParameterTypes(f),
             getJavaCallable(f),
-            !hasWildcardSuppressionAnnotation(f)
+            !getInnermostWildcardSupppressionAnnotation(f)
         )
 
     /*
@@ -1220,10 +1228,11 @@ open class KotlinUsesExtractor(
         else -> null
     }
 
-    fun hasWildcardSuppressionAnnotation(d: IrDeclaration) =
-        d.hasAnnotation(jvmWildcardSuppressionAnnotation) ||
+    fun getInnermostWildcardSupppressionAnnotation(d: IrDeclaration) =
+        getWildcardSuppressionDirective(d) ?:
         // Note not using `parentsWithSelf` as that only works if `d` is an IrDeclarationParent
-        d.parents.any { (it as? IrAnnotationContainer)?.hasAnnotation(jvmWildcardSuppressionAnnotation) == true }
+        d.parents.filterIsInstance<IrAnnotationContainer>().mapNotNull { getWildcardSuppressionDirective(it) }.firstOrNull() ?:
+        false
 
     /**
      * Class to hold labels for generated classes around local functions, lambdas, function references, and property references.

java/ql/integration-tests/all-platforms/kotlin/kotlin_java_lowering_wildcards/JavaUser.java

@@ -49,6 +49,9 @@ public static void test() {
     KotlinDefnsSuppressedFn kdsf = new KotlinDefnsSuppressedFn();
     kdsf.outerFn((List<CharSequence>)null, (Comparable<CharSequence>)null);
 
+    kd.takesVariantTypesIndirectlySuppressedWildcards((List<CharSequence>)null, (Comparable<CharSequence>)null);
+    kd.takesVariantTypesComplexSuppressionWildcards((List<List<? extends List<? extends CharSequence>>>)null);
+
   }
 
-}
+}

java/ql/integration-tests/all-platforms/kotlin/kotlin_java_lowering_wildcards/kotlindefns.kt

@@ -33,6 +33,10 @@ class KotlinDefns {
 
   fun takesVariantTypesSuppressedWildcards(covar: List<@JvmSuppressWildcards CharSequence>, contravar: Comparable<@JvmSuppressWildcards CharSequence>) { }
 
+  fun takesVariantTypesIndirectlySuppressedWildcards(covar: @JvmSuppressWildcards List<CharSequence>, contravar: @JvmSuppressWildcards Comparable<CharSequence>) { }
+
+  fun takesVariantTypesComplexSuppressionWildcards(covar: @JvmSuppressWildcards(suppress = true) List<@JvmSuppressWildcards List<@JvmSuppressWildcards(suppress = false) List<CharSequence>>>) { }
+
   fun returnsInvar() : MutableList<CharSequence> = mutableListOf()
 
   fun returnsCovar(): List<CharSequence> = listOf()
@@ -75,4 +79,4 @@ class KotlinDefnsSuppressedFn {
 
   @JvmSuppressWildcards fun outerFn(covar: List<CharSequence>, contravar: Comparable<CharSequence>) { }
 
-}
+}

java/ql/integration-tests/all-platforms/kotlin/kotlin_java_lowering_wildcards/test.expected

@@ -40,6 +40,9 @@
 | KotlinDefns | takesNestedType | invar | List<List<CharSequence>> |
 | KotlinDefns | takesNestedType | mixed1 | List<? extends Comparable<? super CharSequence>> |
 | KotlinDefns | takesNestedType | mixed2 | Comparable<? super List<? extends CharSequence>> |
+| KotlinDefns | takesVariantTypesComplexSuppressionWildcards | covar | List<List<? extends List<? extends CharSequence>>> |
+| KotlinDefns | takesVariantTypesIndirectlySuppressedWildcards | contravar | Comparable<CharSequence> |
+| KotlinDefns | takesVariantTypesIndirectlySuppressedWildcards | covar | List<CharSequence> |
 | KotlinDefns | takesVariantTypesSuppressedWildcards | contravar | Comparable<CharSequence> |
 | KotlinDefns | takesVariantTypesSuppressedWildcards | covar | List<CharSequence> |
 | KotlinDefnsSuppressedFn | outerFn | contravar | Comparable<CharSequence> |