Merge pull request github#11069 from smowton/smowton/fix/kotlin-vararg-constructor-defaults

Kotlin vararg constructors: don't call a default-parameter constructor just because there are no varargs

Author: smowton

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

@@ -1809,7 +1809,7 @@ open class KotlinFileExtractor(
         }
 
     private fun getDefaultsMethodLabel(f: IrFunction): Label<out DbCallable> {
-        val defaultsMethodName = getDefaultsMethodName(f)
+        val defaultsMethodName = if (f is IrConstructor) "<init>" else getDefaultsMethodName(f)
         val normalArgTypes = getDefaultsMethodArgTypes(f)
         val extensionParamType = f.extensionReceiverParameter?.let { erase(it.type) }
 
@@ -1961,6 +1961,15 @@ open class KotlinFileExtractor(
             target
     }
 
+    private fun callUsesDefaultArguments(callTarget: IrFunction, valueArguments: List<IrExpression?>): Boolean {
+        val varargParam = callTarget.valueParameters.withIndex().find { it.value.isVararg }
+        // If the vararg param is the only one not specified, and it has no default value, then we don't need to call a $default method,
+        // as omitting it already implies passing an empty vararg array.
+        val nullAllowedIdx = if (varargParam != null && varargParam.value.defaultValue == null) varargParam.index else -1
+        return valueArguments.withIndex().any { (index, it) -> it == null && index != nullAllowedIdx }
+    }
+
+
     fun extractRawMethodAccess(
         syntacticCallTarget: IrFunction,
         locElement: IrElement,
@@ -1977,12 +1986,8 @@ open class KotlinFileExtractor(
         superQualifierSymbol: IrClassSymbol? = null) {
 
         val locId = tw.getLocation(locElement)
-        val varargParam = syntacticCallTarget.valueParameters.withIndex().find { it.value.isVararg }
-        // If the vararg param is the only one not specified, and it has no default value, then we don't need to call a $default method,
-        // as omitting it already implies passing an empty vararg array.
-        val nullAllowedIdx = if (varargParam != null && varargParam.value.defaultValue == null) varargParam.index else -1
 
-        if (valueArguments.withIndex().any { (index, it) -> it == null && index != nullAllowedIdx }) {
+        if (callUsesDefaultArguments(syntacticCallTarget, valueArguments)) {
             extractsDefaultsCall(
                 syntacticCallTarget,
                 locId,
@@ -3034,8 +3039,7 @@ open class KotlinFileExtractor(
         val valueArgs = (0 until e.valueArgumentsCount).map { e.getValueArgument(it) }
         // For now, don't try to use default methods for enum constructor calls,
         // which have null arguments even though the parameters don't give default values.
-        val anyDefaultArgs = e !is IrEnumConstructorCall && valueArgs.any { it == null }
-        val id = if (anyDefaultArgs) {
+        val id = if (e !is IrEnumConstructorCall && callUsesDefaultArguments(e.symbol.owner, valueArgs)) {
             extractNewExpr(getDefaultsMethodLabel(e.symbol.owner).cast(), type, locId, parent, idx, callable, enclosingStmt).also {
                 extractDefaultsCallArguments(it, e.symbol.owner, callable, enclosingStmt, valueArgs, null, null)
             }

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

@@ -395,6 +395,23 @@ test.kt:
 #    1|             2: [NullLiteral] null
 #    1|             3: [IntegerLiteral] 3
 #    1|             4: [NullLiteral] null
+#  232|     11: [Method] varargsConstructorUser
+#  232|       3: [TypeAccess] Unit
+#  232|       5: [BlockStmt] { ... }
+#  233|         0: [ExprStmt] <Expr>;
+#  233|           0: [ImplicitCoercionToUnitExpr] <implicit coercion to unit>
+#  233|             0: [TypeAccess] Unit
+#  233|             1: [ClassInstanceExpr] new VarargsConstructorTest(...)
+#  233|               -3: [TypeAccess] VarargsConstructorTest
+#  233|               0: [StringLiteral] varargs constructor test sunk
+#  234|         1: [ExprStmt] <Expr>;
+#  234|           0: [ImplicitCoercionToUnitExpr] <implicit coercion to unit>
+#  234|             0: [TypeAccess] Unit
+#  234|             1: [ClassInstanceExpr] new VarargsConstructorTest(...)
+#  234|               -3: [TypeAccess] VarargsConstructorTest
+#  234|               0: [StringLiteral] varargs constructor test sunk 2
+#  234|               1: [StringLiteral] varargs constructor test not sunk 1
+#  234|               2: [StringLiteral] varargs constructor test not sunk 2
 #    3|   2: [Class] TestMember
 #    3|     1: [Constructor] TestMember
 #    3|       5: [BlockStmt] { ... }
@@ -1688,3 +1705,19 @@ test.kt:
 #  180|             -1: [VarAccess] p0
 #  180|             0: [VarAccess] p1
 #  180|             1: [VarAccess] p2
+#  226|   18: [Class] VarargsConstructorTest
+#  226|     1: [Constructor] VarargsConstructorTest
+#-----|       4: (Parameters)
+#  226|         0: [Parameter] x
+#  226|           0: [TypeAccess] String
+#  226|         1: [Parameter] y
+#  226|           0: [TypeAccess] String[]
+#  226|             0: [WildcardTypeAccess] ? ...
+#  226|               0: [TypeAccess] String
+#  226|       5: [BlockStmt] { ... }
+#  226|         0: [SuperConstructorInvocationStmt] super(...)
+#  226|         1: [BlockStmt] { ... }
+#  228|           0: [ExprStmt] <Expr>;
+#  228|             0: [MethodAccess] sink(...)
+#  228|               -1: [TypeAccess] TestKt
+#  228|               0: [VarAccess] x

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

@@ -222,3 +222,14 @@ fun varargsUserOnlySinkRegularArgs() {
   varargsTestOnlySinkRegularArgs(x = "two-varargs-before sunk 3", "two-vararg-first not sunk 3", "two-vararg-second not sunk 3", z = "two-varargs-after sunk 3")
   varargsTestOnlySinkRegularArgs("no-z-parmeter sunk 3", "no-z-parameter first vararg not sunk 3", "no-z-parameter second vararg not sunk 3")
 }
+
+class VarargsConstructorTest(x: String, vararg y: String) {
+  init {
+    sink(x)
+  }
+}
+
+fun varargsConstructorUser() {
+  VarargsConstructorTest("varargs constructor test sunk")
+  VarargsConstructorTest("varargs constructor test sunk 2", "varargs constructor test not sunk 1", "varargs constructor test not sunk 2")
+}