Kotlin: reintroduce pointless wildcards when a Java declaration explicitly uses them

For example, Java code might use `HasOutVariance<? extends String>`, or `HasInVariance<? super Object>`, both of which are needless wildcards and which the Kotlin extractor would previously have refused to reintroduce due to their not specifying a larger type than their bound. However this led to inconsistency with Java extraction, which
extracts the type as it appears in source.

This seems to particularly happen with generated code, e.g. the output of the Kotlin protobuf compiler.

Author: smowton

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

@@ -960,11 +960,13 @@ open class KotlinUsesExtractor(
             ((t as? IrSimpleType)?.classOrNull?.owner?.isFinalClass) != true
         }
 
-    private fun wildcardAdditionAllowed(v: Variance, t: IrType, addByDefault: Boolean) =
+    private fun wildcardAdditionAllowed(v: Variance, t: IrType, addByDefault: Boolean, javaVariance: Variance?) =
         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
@@ -973,14 +975,21 @@ open class KotlinUsesExtractor(
     private fun addJavaLoweringArgumentWildcards(p: IrTypeParameter, t: IrTypeArgument, addByDefault: Boolean, javaType: JavaType?): IrTypeArgument =
         (t as? IrTypeProjection)?.let {
             val newBase = addJavaLoweringWildcards(it.type, addByDefault, 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) {
+                    is JavaWildcardType -> if (jType.isExtends) Variance.OUT_VARIANCE else Variance.IN_VARIANCE
+                    else -> Variance.INVARIANT
+                }
+            }
             val newVariance =
                 if (it.variance == Variance.INVARIANT &&
                     p.variance != Variance.INVARIANT &&
                     // The next line forbids inferring a wildcard type when we have a corresponding Java type with conflicting variance.
                     // 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.
-                    (javaType?.let { jt -> jt is JavaWildcardType && jt.isExtends == (p.variance == Variance.OUT_VARIANCE) } != false) &&
-                    wildcardAdditionAllowed(p.variance, it.type, addByDefault))
+                    (javaVariance == null || javaVariance == p.variance) &&
+                    wildcardAdditionAllowed(p.variance, it.type, addByDefault, javaVariance))
                     p.variance
                 else
                     it.variance

java/ql/integration-tests/posix-only/kotlin/needless-java-wildcards/Test.java

@@ -0,0 +1,9 @@
+public class Test {
+
+  // This gets mapped to kotlin.Iterable<out T>, meaning we must reintroduce the use-site extends variance to get a type consistent with Java.
+  public static void needlessExtends(Iterable<? extends String> l) { }
+
+  // This type is defined KotlinConsumer<in T>, meaning we must reintroduce the use-site extends variance to get a type consistent with Java.
+  public static void needlessSuper(KotlinConsumer<? super Object> l) { }
+
+}

java/ql/integration-tests/posix-only/kotlin/needless-java-wildcards/kConsumer.kt

@@ -0,0 +1 @@
+public class KotlinConsumer<in T> { }

java/ql/integration-tests/posix-only/kotlin/needless-java-wildcards/test.expected

@@ -0,0 +1,2 @@
+| Test.java:4:22:4:36 | needlessExtends | file:///modules/java.base/java/lang/Iterable.class:0:0:0:0 | Iterable<? extends String> |
+| Test.java:7:22:7:34 | needlessSuper | build1/KotlinConsumer.class:0:0:0:0 | KotlinConsumer<? super Object> |

java/ql/integration-tests/posix-only/kotlin/needless-java-wildcards/test.py

@@ -0,0 +1,5 @@
+from create_database_utils import *
+
+os.mkdir('build1')
+os.mkdir('build2')
+run_codeql_database_create(["kotlinc kConsumer.kt -d build1", "javac Test.java -cp build1 -d build2", "kotlinc user.kt -cp build1:build2"], lang="java")

java/ql/integration-tests/posix-only/kotlin/needless-java-wildcards/test.ql

@@ -0,0 +1,5 @@
+import java
+
+from Method m
+where m.fromSource()
+select m, m.getAParamType()

java/ql/integration-tests/posix-only/kotlin/needless-java-wildcards/user.kt

@@ -0,0 +1,4 @@
+fun f() {
+  Test.needlessExtends(null)
+  Test.needlessSuper(null)
+}