Merge pull request github#10737 from smowton/smowton/fix/type-instance-within-default-value-erasure

Kotlin: fix type variable erasure inside default function values

Author: smowton

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

@@ -254,9 +254,23 @@ open class KotlinFileExtractor(
         }
     }
 
+    fun extractClassInstance(classLabel: Label<out DbClassorinterface>, c: IrClass, argsIncludingOuterClasses: List<IrTypeArgument>?, shouldExtractOutline: Boolean, shouldExtractDetails: Boolean) {
+        DeclarationStackAdjuster(c).use {
+            if (shouldExtractOutline) {
+                extractClassWithoutMembers(c, argsIncludingOuterClasses)
+            }
+
+            if (shouldExtractDetails) {
+                val supertypeMode = if (argsIncludingOuterClasses == null) ExtractSupertypesMode.Raw else ExtractSupertypesMode.Specialised(argsIncludingOuterClasses)
+                extractClassSupertypes(c, classLabel, supertypeMode, true)
+                extractNonPrivateMemberPrototypes(c, argsIncludingOuterClasses, classLabel)
+            }
+        }
+    }
+
     // `argsIncludingOuterClasses` can be null to describe a raw generic type.
     // For non-generic types it will be zero-length list.
-    fun extractClassInstance(c: IrClass, argsIncludingOuterClasses: List<IrTypeArgument>?): Label<out DbClassorinterface> {
+    private fun extractClassWithoutMembers(c: IrClass, argsIncludingOuterClasses: List<IrTypeArgument>?): Label<out DbClassorinterface> {
         with("class instance", c) {
             if (argsIncludingOuterClasses?.isEmpty() == true) {
                 logger.error("Instance without type arguments: " + c.name.asString())
@@ -342,7 +356,7 @@ open class KotlinFileExtractor(
 
     // `argsIncludingOuterClasses` can be null to describe a raw generic type.
     // For non-generic types it will be zero-length list.
-    fun extractNonPrivateMemberPrototypes(c: IrClass, argsIncludingOuterClasses: List<IrTypeArgument>?, id: Label<out DbClassorinterface>) {
+    private fun extractNonPrivateMemberPrototypes(c: IrClass, argsIncludingOuterClasses: List<IrTypeArgument>?, id: Label<out DbClassorinterface>) {
         with("member prototypes", c) {
             val typeParamSubstitution =
                 when (argsIncludingOuterClasses) {

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

@@ -411,16 +411,9 @@ open class KotlinUsesExtractor(
         if (replacedArgsIncludingOuterClasses == null || replacedArgsIncludingOuterClasses.isNotEmpty()) {
             // If this is a generic type instantiation or a raw type then it has no
             // source entity, so we need to extract it here
-            val extractorWithCSource by lazy { this.withFileOfClass(replacedClass) }
-
-            if (!instanceSeenBefore) {
-                extractorWithCSource.extractClassInstance(replacedClass, replacedArgsIncludingOuterClasses)
-            }
-
-            if (inReceiverContext && tw.lm.genericSpecialisationsExtracted.add(classLabelResult.classLabel)) {
-                val supertypeMode = if (replacedArgsIncludingOuterClasses == null) ExtractSupertypesMode.Raw else ExtractSupertypesMode.Specialised(replacedArgsIncludingOuterClasses)
-                extractorWithCSource.extractClassSupertypes(replacedClass, classLabel, supertypeMode, true)
-                extractorWithCSource.extractNonPrivateMemberPrototypes(replacedClass, replacedArgsIncludingOuterClasses, classLabel)
+            val shouldExtractClassDetails = inReceiverContext && tw.lm.genericSpecialisationsExtracted.add(classLabelResult.classLabel)
+            if (!instanceSeenBefore || shouldExtractClassDetails) {
+                this.withFileOfClass(replacedClass).extractClassInstance(classLabel, replacedClass, replacedArgsIncludingOuterClasses, !instanceSeenBefore, shouldExtractClassDetails)
             }
         }
 

java/ql/test/kotlin/library-tests/parameter-defaults/PrintAst.expected

@@ -1165,3 +1165,59 @@ test.kt:
 #  161|             -1: [VarAccess] p0
 #  161|             0: [VarAccess] p1
 #  161|             1: [VarAccess] p2
+#  165|   15: [Class,GenericType,ParameterizedType] TestGenericUsedWithinDefaultValue
+#-----|     -2: (Generic Parameters)
+#  165|       0: [TypeVariable] T
+#  165|     1: [Constructor] TestGenericUsedWithinDefaultValue
+#  165|       5: [BlockStmt] { ... }
+#  165|         0: [SuperConstructorInvocationStmt] super(...)
+#  165|         1: [BlockStmt] { ... }
+#  171|     2: [Method] f
+#  171|       3: [TypeAccess] Unit
+#-----|       4: (Parameters)
+#  171|         0: [Parameter] x
+#  171|           0: [TypeAccess] int
+#  171|         1: [Parameter] y
+#  171|           0: [TypeAccess] String
+#  171|       5: [BlockStmt] { ... }
+#  171|     3: [Method] f$default
+#  171|       3: [TypeAccess] Unit
+#-----|       4: (Parameters)
+#  171|         0: [Parameter] p0
+#  171|           0: [TypeAccess] TestGenericUsedWithinDefaultValue<>
+#  171|         1: [Parameter] p1
+#  171|           0: [TypeAccess] int
+#  171|         2: [Parameter] p2
+#  171|           0: [TypeAccess] String
+#  171|         3: [Parameter] p3
+#  171|           0: [TypeAccess] int
+#  171|         4: [Parameter] p4
+#  171|           0: [TypeAccess] Object
+#  171|       5: [BlockStmt] { ... }
+#  171|         0: [IfStmt] if (...)
+#  171|           0: [EQExpr] ... == ...
+#  171|             0: [AndBitwiseExpr] ... & ...
+#  171|               0: [IntegerLiteral] 2
+#  171|               1: [VarAccess] p3
+#  171|             1: [IntegerLiteral] 0
+#  171|           1: [ExprStmt] <Expr>;
+#  171|             0: [AssignExpr] ...=...
+#  171|               0: [VarAccess] p2
+#  171|               1: [MethodAccess] ident(...)
+#  171|                 -1: [ClassInstanceExpr] new TestGenericUsedWithinDefaultValue<String>(...)
+#  171|                   -3: [TypeAccess] TestGenericUsedWithinDefaultValue<String>
+#  171|                     0: [TypeAccess] String
+#  171|                 0: [StringLiteral] Hello world
+#  171|         1: [ReturnStmt] return ...
+#  171|           0: [MethodAccess] f(...)
+#  171|             -1: [VarAccess] p0
+#  171|             0: [VarAccess] p1
+#  171|             1: [VarAccess] p2
+#  173|     4: [Method] ident
+#  173|       3: [TypeAccess] T
+#-----|       4: (Parameters)
+#  173|         0: [Parameter] t
+#  173|           0: [TypeAccess] T
+#  173|       5: [BlockStmt] { ... }
+#  173|         0: [ReturnStmt] return ...
+#  173|           0: [VarAccess] t

java/ql/test/kotlin/library-tests/parameter-defaults/erasure.ql

@@ -1,9 +1,15 @@
 import java
 
+class InstantiatedType extends ParameterizedType {
+  InstantiatedType() { typeArgs(_, _, this) }
+}
+
 // This checks that all type parameter references are erased in the context of a $default function.
 predicate containsTypeVariables(Type t) {
-  t != t.getErasure() and
-  not t.getErasure().(GenericType).getRawType() = t
+  t instanceof TypeVariable or
+  containsTypeVariables(t.(InstantiatedType).getATypeArgument()) or
+  containsTypeVariables(t.(NestedType).getEnclosingType()) or
+  containsTypeVariables(t.(Wildcard).getATypeBound().getType())
 }
 
 from Expr e

java/ql/test/kotlin/library-tests/parameter-defaults/test.kt

@@ -161,3 +161,15 @@ class VisibilityTests {
   private fun i(x: Int, y: Int = 0) = x + y
 
 }
+
+class TestGenericUsedWithinDefaultValue<T> {
+
+  // This tests parameter erasure works properly: we should notice that here the type variable T
+  // isn't used in the specialisation TestGenericUsedWithinDefaultValue<String>, but it can be
+  // cited in contexts like "the signature of the source declaration of 'TestGenericUsedWithinDefaultValue<String>.f(String)' is 'f(T)'",
+  // not 'f(Object)' as we might mistakenly conclude if we're inappropriately erasing 'T'.
+  fun f(x: Int, y: String = TestGenericUsedWithinDefaultValue<String>().ident("Hello world")) { }
+
+  fun ident(t: T) = t
+
+}