Merge pull request #197 from ProjectMapK/fix/vararg

Fix handling of vararg deserialization.

Author: k163377

src/main/kotlin/io/github/projectmapk/jackson/module/kogera/InternalCommons.kt

@@ -3,7 +3,6 @@ package io.github.projectmapk.jackson.module.kogera
 import com.fasterxml.jackson.annotation.JsonCreator
 import kotlinx.metadata.KmClassifier
 import kotlinx.metadata.KmType
-import kotlinx.metadata.KmValueParameter
 import kotlinx.metadata.isNullable
 import kotlinx.metadata.jvm.JvmMethodSignature
 import java.lang.reflect.AnnotatedElement
@@ -57,9 +56,6 @@ internal fun Constructor<*>.toSignature(): JvmMethodSignature =
 internal fun Method.toSignature(): JvmMethodSignature =
     JvmMethodSignature(this.name, parameterTypes.toDescBuilder().appendDescriptor(this.returnType).toString())
 
-internal fun List<KmValueParameter>.hasVarargParam(): Boolean =
-    lastOrNull()?.let { it.varargElementType != null } ?: false
-
 internal val defaultConstructorMarker: Class<*> by lazy {
     Class.forName("kotlin.jvm.internal.DefaultConstructorMarker")
 }

src/main/kotlin/io/github/projectmapk/jackson/module/kogera/annotationIntrospector/KotlinPrimaryAnnotationIntrospector.kt

@@ -103,6 +103,8 @@ internal class KotlinPrimaryAnnotationIntrospector(
             paramDef.type.isNullable -> false
             // Default argument are defined
             paramDef.declaresDefaultValue -> false
+            // vararg is treated as an empty array because undefined input is allowed
+            paramDef.varargElementType != null -> false
             // The conversion in case of null is defined.
             type.hasDefaultEmptyValue() -> false
             else -> true

src/main/kotlin/io/github/projectmapk/jackson/module/kogera/deser/valueInstantiator/KotlinValueInstantiator.kt

@@ -92,7 +92,7 @@ internal class KotlinValueInstantiator(
                 }
             } else {
                 when {
-                    paramDef.isOptional -> return@forEachIndexed
+                    paramDef.isOptional || paramDef.isVararg -> return@forEachIndexed
                     // do not try to create any object if it is nullable and the value is missing
                     paramDef.isNullable -> null
                     // to get suitable "missing" value provided by deserializer

src/main/kotlin/io/github/projectmapk/jackson/module/kogera/deser/valueInstantiator/argumentBucket/ArgumentBucket.kt

@@ -2,6 +2,7 @@ package io.github.projectmapk.jackson.module.kogera.deser.valueInstantiator.argu
 
 import io.github.projectmapk.jackson.module.kogera.ValueClassUnboxConverter
 import io.github.projectmapk.jackson.module.kogera.deser.valueInstantiator.calcMaskSize
+import io.github.projectmapk.jackson.module.kogera.deser.valueInstantiator.creator.ValueParameter
 import java.lang.reflect.Array as ReflectArray
 
 private fun defaultPrimitiveValue(type: Class<*>): Any = when (type) {
@@ -20,26 +21,69 @@ private fun defaultPrimitiveValue(type: Class<*>): Any = when (type) {
 private fun defaultEmptyArray(arrayType: Class<*>): Any =
     ReflectArray.newInstance(arrayType.componentType, 0)
 
+// List of Int with only 1 bit enabled.
+private val BIT_FLAGS: List<Int> = IntArray(Int.SIZE_BITS) { (1 shl it).inv() }.asList()
+
+private enum class MaskOperation {
+    // Set argument.
+    SET {
+        override fun invoke(i: Int, j: Int) = i and j
+    },
+
+    // Mark the argument as uninitialized.
+    // A vararg argument that has no default value need not be given and is therefore marked as initialized.
+    INIT {
+        override fun invoke(i: Int, j: Int): Int = i or j.inv()
+    };
+
+    abstract operator fun invoke(i: Int, j: Int): Int
+}
+
+private fun IntArray.update(index: Int, operation: MaskOperation) {
+    val maskIndex = index / Integer.SIZE
+    this[maskIndex] = operation(this[maskIndex], BIT_FLAGS[index % Integer.SIZE])
+}
+
 // @see https://github.com/JetBrains/kotlin/blob/4c925d05883a8073e6732bca95bf575beb031a59/core/reflection.jvm/src/kotlin/reflect/jvm/internal/KCallableImpl.kt#L114
 internal class BucketGenerator(
     parameterTypes: List<Class<*>>,
-    hasVarargParam: Boolean,
+    valueParameters: List<ValueParameter>,
     private val converters: List<ValueClassUnboxConverter<Any>?>
 ) {
     private val valueParameterSize: Int = parameterTypes.size
-    private val originalAbsentArgs: Array<Any?> = Array(valueParameterSize) { i ->
-        // Set values of primitive arguments to the boxed default values (such as 0, 0.0, false) instead of nulls.
-        parameterTypes[i].takeIf { it.isPrimitive }?.let { defaultPrimitiveValue(it) }
-    }
+    private val originalAbsentArgs: Array<Any?>
 
-    // -1 is the filled bit mask.
-    private val originalMasks: IntArray = IntArray(calcMaskSize(parameterTypes.size)) { -1 }
+    // Mask initialized to 0 when all arguments are set.
+    // The contents are initialized in the init block.
+    private val originalMasks: IntArray = IntArray(calcMaskSize(parameterTypes.size)) { 0 }
 
     init {
-        if (hasVarargParam) {
-            // vararg argument is always at the end of the arguments.
-            val i = valueParameterSize - 1
-            originalAbsentArgs[i] = defaultEmptyArray(parameterTypes[i])
+        originalAbsentArgs = Array(valueParameterSize) { i ->
+            val paramType = parameterTypes[i]
+            val metaParam = valueParameters[i]
+
+            when {
+                // In Kotlin, it is possible to define non-tail arguments with vararg,
+                // which cannot be determined by Java reflection, so they are read from Metadata.
+                metaParam.isVararg -> {
+                    // If no default arguments are set,
+                    // the call may be made with an empty array even if no arguments are passed,
+                    // so it is treated as initialized.
+                    // Conversely, if default arguments are set,
+                    // the initial value is treated as uninitialized to detect that no arguments has passed.
+                    if (metaParam.isOptional) originalMasks.update(i, MaskOperation.INIT)
+                    defaultEmptyArray(paramType)
+                }
+                // Set values of primitive arguments to the boxed default values (such as 0, 0.0, false) instead of nulls.
+                paramType.isPrimitive -> {
+                    originalMasks.update(i, MaskOperation.INIT)
+                    defaultPrimitiveValue(paramType)
+                }
+                else -> {
+                    originalMasks.update(i, MaskOperation.INIT)
+                    null
+                }
+            }
         }
     }
 
@@ -53,13 +97,6 @@ internal class ArgumentBucket(
     val masks: IntArray,
     private val converters: List<ValueClassUnboxConverter<Any>?>
 ) {
-    companion object {
-        // List of Int with only 1 bit enabled.
-        private val BIT_FLAGS: List<Int> = IntArray(Int.SIZE_BITS) { (1 shl it).inv() }.asList()
-    }
-
-    private var count = 0
-
     /**
      * Sets the argument corresponding to index.
      * Note that, arguments defined in the value class must be passed as boxed.
@@ -77,8 +114,8 @@ internal class ArgumentBucket(
         val maskIndex = index / Integer.SIZE
         masks[maskIndex] = masks[maskIndex] and BIT_FLAGS[index % Integer.SIZE]
 
-        count++
+        masks.update(index, MaskOperation.SET)
     }
 
-    val isFullInitialized: Boolean get() = count == valueParameterSize
+    val isFullInitialized: Boolean get() = masks.all { it == 0 }
 }

src/main/kotlin/io/github/projectmapk/jackson/module/kogera/deser/valueInstantiator/creator/ConstructorValueCreator.kt

@@ -8,7 +8,6 @@ import io.github.projectmapk.jackson.module.kogera.deser.valueInstantiator.argum
 import io.github.projectmapk.jackson.module.kogera.deser.valueInstantiator.argumentBucket.BucketGenerator
 import io.github.projectmapk.jackson.module.kogera.deser.valueInstantiator.calcMaskSize
 import io.github.projectmapk.jackson.module.kogera.getDeclaredConstructorBy
-import io.github.projectmapk.jackson.module.kogera.hasVarargParam
 import java.lang.reflect.Constructor
 
 internal class ConstructorValueCreator<T : Any>(
@@ -33,7 +32,7 @@ internal class ConstructorValueCreator<T : Any>(
         val rawTypes = constructor.parameterTypes.asList()
         bucketGenerator = BucketGenerator(
             rawTypes,
-            constructorParameters.hasVarargParam(),
+            valueParameters,
             constructorParameters.mapToConverters(rawTypes, cache)
         )
     }

src/main/kotlin/io/github/projectmapk/jackson/module/kogera/deser/valueInstantiator/creator/MethodValueCreator.kt

@@ -7,7 +7,6 @@ import io.github.projectmapk.jackson.module.kogera.deser.valueInstantiator.argum
 import io.github.projectmapk.jackson.module.kogera.deser.valueInstantiator.argumentBucket.BucketGenerator
 import io.github.projectmapk.jackson.module.kogera.deser.valueInstantiator.calcMaskSize
 import io.github.projectmapk.jackson.module.kogera.getDeclaredMethodBy
-import io.github.projectmapk.jackson.module.kogera.hasVarargParam
 import kotlinx.metadata.KmFunction
 import java.lang.reflect.Method
 
@@ -33,7 +32,7 @@ internal class MethodValueCreator<T>(
         val rawTypes = method.parameterTypes.asList()
         bucketGenerator = BucketGenerator(
             rawTypes,
-            kmParameters.hasVarargParam(),
+            valueParameters,
             kmParameters.mapToConverters(rawTypes, cache)
         )
     }

src/main/kotlin/io/github/projectmapk/jackson/module/kogera/deser/valueInstantiator/creator/ValueParameter.kt

@@ -11,6 +11,7 @@ internal class ValueParameter(param: KmValueParameter) {
     val type: KmType = param.type
     val isOptional: Boolean = param.declaresDefaultValue
     val isNullable: Boolean = type.isNullable
+    val isVararg: Boolean = param.varargElementType != null
     val isGenericType: Boolean = type.classifier is KmClassifier.TypeParameter
 
     // TODO: Formatting into a form that is easy to understand as an error message with reference to KParameter

src/test/kotlin/io/github/projectmapk/jackson/module/kogera/deser/valueInstantiator/argumentBucket/ArgumentBucketTest.kt

@@ -1,9 +1,9 @@
 package io.github.projectmapk.jackson.module.kogera.deser.valueInstantiator.argumentBucket
 
 import io.github.projectmapk.jackson.module.kogera.ValueClassUnboxConverter
+import io.github.projectmapk.jackson.module.kogera.deser.valueInstantiator.creator.ValueParameter
 import io.mockk.every
 import io.mockk.mockk
-import org.junit.jupiter.api.Assertions.assertArrayEquals
 import org.junit.jupiter.api.Assertions.assertEquals
 import org.junit.jupiter.api.Assertions.assertFalse
 import org.junit.jupiter.api.Assertions.assertTrue
@@ -15,11 +15,20 @@ private class ArgumentBucketTest {
         every { this@mockk[any()] } returns null
     }
 
+    fun mockValueParameter(mockVararg: Boolean = false, mockOptional: Boolean = false) = mockk<ValueParameter> {
+        every { isVararg } returns mockVararg
+        every { isOptional } returns mockOptional
+    }
+
     @Nested
     inner class BucketGeneratorTest {
         @Test
         fun basic() {
-            val generator = BucketGenerator((0..31).map { String::class.java }, false, mockConverters)
+            val generator = BucketGenerator(
+                (0..31).map { String::class.java },
+                (0..31).map { mockValueParameter() },
+                mockConverters
+            )
             val result = generator.generate()
 
             assertEquals(32, result.valueParameterSize)
@@ -41,7 +50,7 @@ private class ArgumentBucketTest {
                     Long::class.java,
                     Double::class.java
                 ),
-                false,
+                (1..8).map { mockValueParameter() },
                 mockConverters
             )
             val result = generator.generate()
@@ -58,52 +67,43 @@ private class ArgumentBucketTest {
 
         @Test
         fun maskSize() {
-            val generator = BucketGenerator((0..32).map { String::class.java }, false, mockConverters)
+            val generator = BucketGenerator(
+                (0..32).map { String::class.java },
+                (0..32).map { mockValueParameter() },
+                mockConverters
+            )
             val result = generator.generate()
 
             assertEquals(33, result.valueParameterSize)
             assertTrue(result.arguments.all { it == null })
             assertEquals(2, result.masks.size)
             assertEquals(-1, result.masks[0])
-            assertEquals(-1, result.masks[1])
+            assertEquals(1, result.masks[1])
         }
 
-        @Test
-        fun hasVararg() {
-            val primitiveGenerator = BucketGenerator(listOf(IntArray::class.java), true, mockConverters)
-            val primitiveResult = primitiveGenerator.generate()
-
-            assertEquals(1, primitiveResult.valueParameterSize)
-            assertArrayEquals(intArrayOf(), primitiveResult.arguments[0] as IntArray)
-            assertEquals(1, primitiveResult.masks.size)
-            assertEquals(-1, primitiveResult.masks[0])
-
-            val objectGenerator = BucketGenerator(listOf(Array<Int>::class.java), true, mockConverters)
-            val objectResult = objectGenerator.generate()
-            assertEquals(1, objectResult.valueParameterSize)
-            @Suppress("UNCHECKED_CAST")
-            assertArrayEquals(emptyArray<Int>(), objectResult.arguments[0] as Array<Int>)
-            assertEquals(1, objectResult.masks.size)
-            assertEquals(-1, objectResult.masks[0])
-        }
+        // vararg is verified with an integration test.
     }
 
     @Test
     fun test() {
-        val generator = BucketGenerator((0..32).map { String::class.java }, false, mockConverters)
+        val generator = BucketGenerator(
+            (0..32).map { String::class.java },
+            (0..32).map { mockValueParameter() },
+            mockConverters
+        )
         val sut = generator.generate()
 
         sut[0] = "0"
         sut[32] = "32"
 
         assertEquals(-2, sut.masks[0])
-        assertEquals(-2, sut.masks[1])
+        assertEquals(0, sut.masks[1])
         assertFalse(sut.isFullInitialized)
 
         (1..31).forEach { sut[it] = it.toString() }
 
         assertEquals(0, sut.masks[0])
-        assertEquals(-2, sut.masks[1])
+        assertEquals(0, sut.masks[1])
         (0..32).forEach { assertEquals(it.toString(), sut.arguments[it]) }
         assertTrue(sut.isFullInitialized)
     }
@@ -115,7 +115,11 @@ private class ArgumentBucketTest {
     fun unboxTest() {
         @Suppress("UNCHECKED_CAST")
         val converter = ValueClassUnboxConverter(V::class.java) as ValueClassUnboxConverter<Any>
-        val generator = BucketGenerator(listOf(Int::class.java, V::class.java), false, listOf(converter, null))
+        val generator = BucketGenerator(
+            listOf(Int::class.java, V::class.java),
+            (1..2).map { mockValueParameter() },
+            listOf(converter, null)
+        )
         val bucket = generator.generate()
 
         bucket[0] = V(0)