Kotlin: reintroduce obinit when we have multiple secondary constructors and no primary

This avoids DB inconsistencies because complex initialisers are extracted to more than one function.

Author: smowton

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

@@ -367,6 +367,27 @@ open class KotlinFileExtractor(
         tw.writeHasLocation(stmtId, locId)
     }
 
+    fun extractObinitFunction(c: IrClass, parentId: Label<out DbClassorinterface>) {
+        // add method:
+        val obinitLabel = getObinitLabel(c)
+        val obinitId = tw.getLabelFor<DbMethod>(obinitLabel)
+        val returnType = useType(pluginContext.irBuiltIns.unitType)
+        tw.writeMethods(obinitId, "<obinit>", "<obinit>()", returnType.javaResult.id, parentId, obinitId)
+        tw.writeMethodsKotlinType(obinitId, returnType.kotlinResult.id)
+
+        val locId = tw.getLocation(c)
+        tw.writeHasLocation(obinitId, locId)
+
+        addModifiers(obinitId, "private")
+
+        // add body:
+        val blockId = tw.getFreshIdLabel<DbBlock>()
+        tw.writeStmts_block(blockId, obinitId, 0, obinitId)
+        tw.writeHasLocation(blockId, locId)
+
+        extractDeclInitializers(c.declarations, false) { Pair(blockId, obinitId) }
+    }
+
     fun extractClassSource(c: IrClass, extractDeclarations: Boolean, extractStaticInitializer: Boolean, extractPrivateMembers: Boolean, extractFunctionBodies: Boolean): Label<out DbClassorinterface> {
         with("class source", c) {
             DeclarationStackAdjuster(c).use {
@@ -421,6 +442,9 @@ open class KotlinFileExtractor(
                     addModifiers(instance.id, "public", "static", "final")
                     tw.writeClass_object(id.cast<DbClass>(), instance.id)
                 }
+                if (needsObinitFunction(c)) {
+                    extractObinitFunction(c, id)
+                }
 
                 extractClassModifiers(c, id)
                 extractClassSupertypes(c, id, inReceiverContext = true) // inReceiverContext = true is specified to force extraction of member prototypes of base types
@@ -2101,6 +2125,22 @@ open class KotlinFileExtractor(
         enclosingStmt: Label<out DbStmt>
     ): Label<DbNewexpr> = extractNewExpr(useFunction<DbConstructor>(calledConstructor, constructorTypeArgs), constructedType, locId, parent, idx, callable, enclosingStmt)
 
+    private fun needsObinitFunction(c: IrClass) = c.primaryConstructor == null && c.constructors.count() > 1
+
+    private fun getObinitLabel(c: IrClass) = getFunctionLabel(
+        c,
+        null,
+        "<obinit>",
+        listOf(),
+        pluginContext.irBuiltIns.unitType,
+        null,
+        functionTypeParameters = listOf(),
+        classTypeArgsIncludingOuterClasses = listOf(),
+        overridesCollectionsMethod = false,
+        javaSignature = null,
+        addParameterWildcardsByDefault = false
+    )
+
     private fun extractConstructorCall(
         e: IrFunctionAccessExpression,
         parent: Label<out DbExprparent>,
@@ -2430,13 +2470,29 @@ open class KotlinFileExtractor(
                     loopIdMap.remove(e)
                 }
                 is IrInstanceInitializerCall -> {
-                    val stmtParent = parent.stmt(e, callable)
                     val irConstructor = declarationStack.peek() as? IrConstructor
                     if (irConstructor == null) {
                         logger.errorElement("IrInstanceInitializerCall outside constructor", e)
                         return
                     }
-                    extractInstanceInitializerBlock(stmtParent, irConstructor)
+                    if (needsObinitFunction(irConstructor.parentAsClass)) {
+                        val exprParent = parent.expr(e, callable)
+                        val id = tw.getFreshIdLabel<DbMethodaccess>()
+                        val type = useType(pluginContext.irBuiltIns.unitType)
+                        val locId = tw.getLocation(e)
+                        val methodLabel = getObinitLabel(irConstructor.parentAsClass)
+                        val methodId = tw.getLabelFor<DbMethod>(methodLabel)
+                        tw.writeExprs_methodaccess(id, type.javaResult.id, exprParent.parent, exprParent.idx)
+                        tw.writeExprsKotlinType(id, type.kotlinResult.id)
+                        tw.writeHasLocation(id, locId)
+                        tw.writeCallableEnclosingExpr(id, callable)
+                        tw.writeStatementEnclosingExpr(id, exprParent.enclosingStmt)
+                        tw.writeCallableBinding(id, methodId)
+                    }
+                    else {
+                        val stmtParent = parent.stmt(e, callable)
+                        extractInstanceInitializerBlock(stmtParent, irConstructor)
+                    }
                 }
                 is IrConstructorCall -> {
                     val exprParent = parent.expr(e, callable)

java/ql/consistency-queries/calls.ql

@@ -1,5 +1,7 @@
 import java
 
 from MethodAccess ma
-where not exists(ma.getQualifier()) and ma.getFile().isKotlinSourceFile()
+// Generally Kotlin calls will always use an explicit qualifier, except for calls
+// to the synthetic instance initializer <obinit>, which use an implicit `this`.
+where not exists(ma.getQualifier()) and ma.getFile().isKotlinSourceFile() and not ma.getCallee() instanceof InstanceInitializer
 select ma

java/ql/test/kotlin/library-tests/lazy-val-multiple-constructors/test.expected

@@ -0,0 +1,2 @@
+| test.kt:3:20:3:32 | new KProperty1<Test,Integer>(...) { ... } | test.kt:3:20:3:32 | ...::... |
+| test.kt:3:28:3:32 | new Function0<Integer>(...) { ... } | test.kt:3:28:3:32 | ...->... |

java/ql/test/kotlin/library-tests/lazy-val-multiple-constructors/test.kt

@@ -0,0 +1,12 @@
+public class Test {
+
+  val lazyVal: Int by lazy { 5 }
+
+  // Both of these constructors will need to extract the implicit classes created by `lazyVal` and initialize it--
+  // This test checks we don't introduce any inconsistency this way.
+
+  constructor(x: Int) { }
+
+  constructor(y: String) { }
+
+}

java/ql/test/kotlin/library-tests/lazy-val-multiple-constructors/test.ql

@@ -0,0 +1,4 @@
+import java
+
+from AnonymousClass ac
+select ac, ac.getClassInstanceExpr()