Merge pull request #11003 from smowton/smowton/fix/reintroduce-pointless-wildcards

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

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) &&
+                    (javaVariance == null || javaVariance == p.variance) &&
-                    wildcardAdditionAllowed(p.variance, it.type, addByDefault))
+                    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)
+}