diff --git a/kotlin/codegen/src/main/java/com/squareup/moshi/kotlin/codegen/api/AdapterGenerator.kt b/kotlin/codegen/src/main/java/com/squareup/moshi/kotlin/codegen/api/AdapterGenerator.kt index 83b8c1e..8719662 100644 --- a/kotlin/codegen/src/main/java/com/squareup/moshi/kotlin/codegen/api/AdapterGenerator.kt +++ b/kotlin/codegen/src/main/java/com/squareup/moshi/kotlin/codegen/api/AdapterGenerator.kt @@ -28,6 +28,7 @@ import com.squareup.kotlinpoet.ParameterSpec import com.squareup.kotlinpoet.ParameterizedTypeName import com.squareup.kotlinpoet.ParameterizedTypeName.Companion.parameterizedBy import com.squareup.kotlinpoet.PropertySpec +import com.squareup.kotlinpoet.TypeName import com.squareup.kotlinpoet.TypeSpec import com.squareup.kotlinpoet.TypeVariableName import com.squareup.kotlinpoet.asClassName @@ -38,6 +39,9 @@ import com.squareup.moshi.JsonReader import com.squareup.moshi.JsonWriter import com.squareup.moshi.Moshi import com.squareup.moshi.internal.Util +import com.squareup.moshi.kotlin.codegen.api.FromJsonComponent.ParameterOnly +import com.squareup.moshi.kotlin.codegen.api.FromJsonComponent.ParameterProperty +import com.squareup.moshi.kotlin.codegen.api.FromJsonComponent.PropertyOnly import java.lang.reflect.Constructor import java.lang.reflect.Type @@ -60,6 +64,8 @@ internal class AdapterGenerator( private val className = target.typeName.rawType() private val visibility = target.visibility private val typeVariables = target.typeVariables + private val targetConstructorParams = target.constructor.parameters + .mapKeys { (_, param) -> param.index } private val nameAllocator = NameAllocator() private val adapterName = "${className.simpleNames.joinToString(separator = "_")}JsonAdapter" @@ -196,8 +202,34 @@ internal class AdapterGenerator( } } + val propertiesByIndex = propertyList.asSequence() + .filter { it.hasConstructorParameter } + .associateBy { it.target.parameterIndex } + val components = mutableListOf() + + // Add parameters (± properties) first, their index matters + for ((index, parameter) in targetConstructorParams) { + val property = propertiesByIndex[index] + if (property == null) { + components += ParameterOnly(parameter) + } else { + components += ParameterProperty(parameter, property) + } + } + + // Now add the remaining properties that aren't parameters + for (property in propertyList) { + if (property.target.parameterIndex in targetConstructorParams) { + continue // Already handled + } + if (property.isTransient) { + continue // We don't care about these outside of constructor parameters + } + components += PropertyOnly(property) + } + // Calculate how many masks we'll need. Round up if it's not evenly divisible by 32 - val propertyCount = propertyList.count { it.hasConstructorParameter } + val propertyCount = targetConstructorParams.size val maskCount = if (propertyCount == 0) { 0 } else { @@ -207,8 +239,9 @@ internal class AdapterGenerator( val maskNames = Array(maskCount) { index -> nameAllocator.newName("mask$index") } - val useConstructorDefaults = nonTransientProperties.any { it.hasConstructorDefault } - if (useConstructorDefaults) { + val useDefaultsConstructor = components.filterIsInstance() + .any { it.parameter.hasDefault } + if (useDefaultsConstructor) { // Initialize all our masks, defaulting to fully unset (-1) for (maskName in maskNames) { result.addStatement("var %L = -1", maskName) @@ -243,14 +276,21 @@ internal class AdapterGenerator( maskNameIndex++ } } - for (property in propertyList) { - if (property.isTransient) { - if (property.hasConstructorParameter) { - updateMaskIndexes() - constructorPropertyTypes += property.target.type.asTypeBlock() - } + + for (input in components) { + if (input is ParameterOnly || + (input is ParameterProperty && input.property.isTransient)) { + updateMaskIndexes() + constructorPropertyTypes += input.type.asTypeBlock() + continue + } else if (input is PropertyOnly && input.property.isTransient) { continue } + + // We've removed all parameter-only types by this point + val property = (input as PropertyComponent).property + + // Proceed as usual if (property.hasLocalIsPresentName || property.hasConstructorDefault) { result.beginControlFlow("%L ->", propertyIndex) if (property.delegateKey.nullable) { @@ -299,8 +339,6 @@ internal class AdapterGenerator( result.addStatement("%N.endObject()", readerParam) var separator = "\n" - val parameterProperties = propertyList.filter { it.hasConstructorParameter } - val useDefaultsConstructor = parameterProperties.any { it.hasDefault } val resultName = nameAllocator.newName("result") val hasNonConstructorProperties = nonTransientProperties.any { !it.hasConstructorParameter } @@ -352,25 +390,29 @@ internal class AdapterGenerator( result.addCode("«%L%T(", returnOrResultAssignment, originalTypeName) } - for (property in parameterProperties) { + for (input in components.filterIsInstance()) { result.addCode(separator) if (useDefaultsConstructor) { - if (property.isTransient) { + if (input is ParameterOnly || (input is ParameterProperty && input.property.isTransient)) { // We have to use the default primitive for the available type in order for // invokeDefaultConstructor to properly invoke it. Just using "null" isn't safe because // the transient type may be a primitive type. - result.addCode(property.target.type.rawType().defaultPrimitiveValue()) + result.addCode(input.type.rawType().defaultPrimitiveValue()) } else { - result.addCode("%N", property.localName) + result.addCode("%N", (input as ParameterProperty).property.localName) } - } else { + } else if (input !is ParameterOnly) { + val property = (input as ParameterProperty).property result.addCode("%N = %N", property.name, property.localName) } - if (!property.isTransient && property.isRequired) { - val missingPropertyBlock = - CodeBlock.of("%T.missingProperty(%S, %S, %N)", - MOSHI_UTIL, property.localName, property.jsonName, readerParam) - result.addCode(" ?: throw·%L", missingPropertyBlock) + if (input is PropertyComponent) { + val property = input.property + if (!property.isTransient && property.isRequired) { + val missingPropertyBlock = + CodeBlock.of("%T.missingProperty(%S, %S, %N)", + MOSHI_UTIL, property.localName, property.jsonName, readerParam) + result.addCode(" ?: throw·%L", missingPropertyBlock) + } } separator = ",\n" } @@ -430,3 +472,40 @@ internal class AdapterGenerator( return result.build() } } + +private interface PropertyComponent { + val property: PropertyGenerator + val type: TypeName +} +private interface ParameterComponent { + val parameter: TargetParameter + val type: TypeName +} + +/** + * Type hierarchy for describing fromJson() components. Specifically - parameters, properties, and + * parameter properties. All three of these scenarios participate in fromJson() parsing. + */ +private sealed class FromJsonComponent { + + abstract val type: TypeName + + data class ParameterOnly( + override val parameter: TargetParameter + ) : FromJsonComponent(), ParameterComponent { + override val type: TypeName = parameter.type + } + + data class PropertyOnly( + override val property: PropertyGenerator + ) : FromJsonComponent(), PropertyComponent { + override val type: TypeName = property.target.type + } + + data class ParameterProperty( + override val parameter: TargetParameter, + override val property: PropertyGenerator + ) : FromJsonComponent(), ParameterComponent, PropertyComponent { + override val type: TypeName = parameter.type + } +} diff --git a/kotlin/codegen/src/main/java/com/squareup/moshi/kotlin/codegen/api/TargetConstructor.kt b/kotlin/codegen/src/main/java/com/squareup/moshi/kotlin/codegen/api/TargetConstructor.kt index 1606807..742cb4b 100644 --- a/kotlin/codegen/src/main/java/com/squareup/moshi/kotlin/codegen/api/TargetConstructor.kt +++ b/kotlin/codegen/src/main/java/com/squareup/moshi/kotlin/codegen/api/TargetConstructor.kt @@ -19,7 +19,7 @@ import com.squareup.kotlinpoet.KModifier /** A constructor in user code that should be called by generated code. */ internal data class TargetConstructor( - val parameters: Map, + val parameters: LinkedHashMap, val visibility: KModifier ) { init { diff --git a/kotlin/codegen/src/main/java/com/squareup/moshi/kotlin/codegen/api/TargetParameter.kt b/kotlin/codegen/src/main/java/com/squareup/moshi/kotlin/codegen/api/TargetParameter.kt index 828f25c..d61b77c 100644 --- a/kotlin/codegen/src/main/java/com/squareup/moshi/kotlin/codegen/api/TargetParameter.kt +++ b/kotlin/codegen/src/main/java/com/squareup/moshi/kotlin/codegen/api/TargetParameter.kt @@ -16,11 +16,13 @@ package com.squareup.moshi.kotlin.codegen.api import com.squareup.kotlinpoet.AnnotationSpec +import com.squareup.kotlinpoet.TypeName /** A parameter in user code that should be populated by generated code. */ internal data class TargetParameter( val name: String, val index: Int, + val type: TypeName, val hasDefault: Boolean, val jsonName: String? = null, val qualifiers: Set? = null diff --git a/kotlin/codegen/src/main/java/com/squareup/moshi/kotlin/codegen/metadata.kt b/kotlin/codegen/src/main/java/com/squareup/moshi/kotlin/codegen/metadata.kt index cc8eea6..2f344a3 100644 --- a/kotlin/codegen/src/main/java/com/squareup/moshi/kotlin/codegen/metadata.kt +++ b/kotlin/codegen/src/main/java/com/squareup/moshi/kotlin/codegen/metadata.kt @@ -72,12 +72,13 @@ private fun Collection.visibility(): KModifier { internal fun primaryConstructor(kotlinApi: TypeSpec, elements: Elements): TargetConstructor? { val primaryConstructor = kotlinApi.primaryConstructor ?: return null - val parameters = mutableMapOf() + val parameters = LinkedHashMap() for ((index, parameter) in primaryConstructor.parameters.withIndex()) { val name = parameter.name parameters[name] = TargetParameter( name = name, index = index, + type = parameter.type, hasDefault = parameter.defaultValue != null, qualifiers = parameter.annotations.qualifiers(elements), jsonName = parameter.annotations.jsonName() diff --git a/kotlin/tests/src/test/kotlin/com/squareup/moshi/kotlin/DualKotlinTest.kt b/kotlin/tests/src/test/kotlin/com/squareup/moshi/kotlin/DualKotlinTest.kt index 6f79dbb..a85f00f 100644 --- a/kotlin/tests/src/test/kotlin/com/squareup/moshi/kotlin/DualKotlinTest.kt +++ b/kotlin/tests/src/test/kotlin/com/squareup/moshi/kotlin/DualKotlinTest.kt @@ -327,6 +327,7 @@ class DualKotlinTest(useReflection: Boolean) { @Test fun multipleConstructors() { val adapter = moshi.adapter() + //language=JSON assertThat(adapter.toJson(MultipleConstructorsB(6))).isEqualTo("""{"f":{"f":6},"b":6}""") @Language("JSON") @@ -342,6 +343,59 @@ class DualKotlinTest(useReflection: Boolean) { class MultipleConstructorsB(val f: MultipleConstructorsA = MultipleConstructorsA(5), val b: Int) { constructor(f: Int, b: Int = 6): this(MultipleConstructorsA(f), b) } + + @Test fun `multiple non-property parameters`() { + val adapter = moshi.adapter() + + @Language("JSON") + val testJson = """{"prop":7}""" + + assertThat(adapter.toJson(MultipleNonPropertyParameters(7))).isEqualTo(testJson) + + val result = adapter.fromJson(testJson)!! + assertThat(result.prop).isEqualTo(7) + } + + @JsonClass(generateAdapter = true) + class MultipleNonPropertyParameters( + val prop: Int, + param1: Int = 1, + param2: Int = 2 + ) { + init { + // Ensure the params always uses their default value + require(param1 == 1) + require(param2 == 2) + } + } + + // Tests the case of multiple parameters with no parameter properties. + @Test fun `only multiple non-property parameters`() { + val adapter = moshi.adapter() + + @Language("JSON") + val testJson = """{"prop":7}""" + + assertThat(adapter.toJson(OnlyMultipleNonPropertyParameters().apply { prop = 7 })) + .isEqualTo(testJson) + + val result = adapter.fromJson(testJson)!! + assertThat(result.prop).isEqualTo(7) + } + + @JsonClass(generateAdapter = true) + class OnlyMultipleNonPropertyParameters( + param1: Int = 1, + param2: Int = 2 + ) { + init { + // Ensure the params always uses their default value + require(param1 == 1) + require(param2 == 2) + } + + var prop: Int = 0 + } } // Has to be outside since inline classes are only allowed on top level diff --git a/kotlin/tests/src/test/kotlin/com/squareup/moshi/kotlin/codegen/MultipleMasksTest.kt b/kotlin/tests/src/test/kotlin/com/squareup/moshi/kotlin/codegen/MultipleMasksTest.kt index 8b95c0d..c31dcde 100644 --- a/kotlin/tests/src/test/kotlin/com/squareup/moshi/kotlin/codegen/MultipleMasksTest.kt +++ b/kotlin/tests/src/test/kotlin/com/squareup/moshi/kotlin/codegen/MultipleMasksTest.kt @@ -75,7 +75,7 @@ class MultipleMasks( val arg17: Long = 17, val arg18: Long = 18, val arg19: Long = 19, - val arg20: Long = 20, + @Suppress("UNUSED_PARAMETER") arg20: Long = 20, val arg21: Long = 21, val arg22: Long = 22, val arg23: Long = 23,