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 8f7e406..d6be437 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 @@ -18,7 +18,6 @@ package com.squareup.moshi.kotlin.codegen.api import com.squareup.kotlinpoet.ARRAY import com.squareup.kotlinpoet.AnnotationSpec import com.squareup.kotlinpoet.CodeBlock -import com.squareup.kotlinpoet.CodeBlock.Companion import com.squareup.kotlinpoet.FileSpec import com.squareup.kotlinpoet.FunSpec import com.squareup.kotlinpoet.INT @@ -74,6 +73,9 @@ internal class AdapterGenerator( // Because we generate redundant `out` variance for some generics and there's no way // for us to know when it's redundant. "REDUNDANT_PROJECTION", + // Because we may generate redundant explicit types for local vars with default values. + // Example: 'var fooSet: Boolean = false' + "RedundantExplicitType", // NameAllocator will just add underscores to differentiate names, which Kotlin doesn't // like for stylistic reasons. "LocalVariableName" @@ -478,8 +480,10 @@ internal class AdapterGenerator( localConstructorProperty ) } else { - // Standard constructor call. Can omit generics as they're inferred - result.addCode("«%L%T(", returnOrResultAssignment, originalTypeName.rawType()) + // Standard constructor call. Don't omit generics for parameterized types even if they can be + // inferred, as calculating the right condition for inference exceeds the value gained from + // being less pedantic. + result.addCode("«%L%T(", returnOrResultAssignment, originalTypeName) } for (input in components.filterIsInstance()) { 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 87a2201..08c630f 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 @@ -18,8 +18,10 @@ package com.squareup.moshi.kotlin.codegen import com.squareup.kotlinpoet.AnnotationSpec import com.squareup.kotlinpoet.ClassName import com.squareup.kotlinpoet.KModifier +import com.squareup.kotlinpoet.ParameterizedTypeName import com.squareup.kotlinpoet.TypeName import com.squareup.kotlinpoet.TypeSpec +import com.squareup.kotlinpoet.TypeVariableName import com.squareup.kotlinpoet.asClassName import com.squareup.kotlinpoet.asTypeName import com.squareup.kotlinpoet.metadata.ImmutableKmConstructor @@ -53,6 +55,7 @@ import javax.lang.model.element.AnnotationMirror import javax.lang.model.element.Element import javax.lang.model.element.ElementKind import javax.lang.model.element.TypeElement +import javax.lang.model.type.DeclaredType import javax.lang.model.util.Elements import javax.lang.model.util.Types import javax.tools.Diagnostic.Kind.ERROR @@ -192,6 +195,8 @@ internal fun targetType(messager: Messager, } val properties = mutableMapOf() + + val resolvedTypes = mutableListOf() val superTypes = appliedType.supertypes(types) .filterNot { supertype -> supertype.element.asClassName() == OBJECT_CLASS || // Don't load properties for java.lang.Object. @@ -207,15 +212,53 @@ internal fun targetType(messager: Messager, } .associateWithTo(LinkedHashMap()) { supertype -> // Load the kotlin API cache into memory eagerly so we can reuse the parsed APIs - if (supertype.element == element) { + val api = if (supertype.element == element) { // We've already parsed this api above, reuse it kotlinApi } else { cachedClassInspector.toTypeSpec(supertype.element) } + + val apiSuperClass = api.superclass + if (apiSuperClass is ParameterizedTypeName) { + // + // This extends a typed generic superclass. We want to construct a mapping of the + // superclass typevar names to their materialized types here. + // + // class Foo extends Bar + // class Bar + // + // We will store {Foo : {T : [String]}}. + // + // Then when we look at Bar later, we'll look up to the descendent Foo and extract its + // materialized type from there. + // + val superSuperClass = supertype.element.superclass as DeclaredType + + // Convert to an element and back to wipe the typed generics off of this + val untyped = superSuperClass.asElement().asType().asTypeName() as ParameterizedTypeName + resolvedTypes += ResolvedTypeMapping( + target = untyped.rawType, + args = untyped.typeArguments.asSequence() + .cast() + .map(TypeVariableName::name) + .zip(apiSuperClass.typeArguments.asSequence()) + .associate { it } + ) + } + + return@associateWithTo api } - for (supertypeApi in superTypes.values) { - val supertypeProperties = declaredProperties(constructor, supertypeApi) + + for ((localAppliedType, supertypeApi) in superTypes.entries) { + val appliedClassName = localAppliedType.element.asClassName() + val supertypeProperties = declaredProperties( + constructor = constructor, + kotlinApi = supertypeApi, + allowedTypeVars = typeVariables.toSet(), + currentClass = appliedClassName, + resolvedTypes = resolvedTypes + ) for ((name, property) in supertypeProperties) { properties.putIfAbsent(name, property) } @@ -243,16 +286,71 @@ internal fun targetType(messager: Messager, visibility = resolvedVisibility) } +/** + * Represents a resolved raw class to type arguments where [args] are a map of the parent type var + * name to its resolved [TypeName]. + */ +private data class ResolvedTypeMapping(val target: ClassName, val args: Map) + +private fun resolveTypeArgs( + targetClass: ClassName, + propertyType: TypeName, + resolvedTypes: List, + allowedTypeVars: Set, + entryStartIndex: Int = resolvedTypes.indexOfLast { it.target == targetClass } +): TypeName { + val unwrappedType = propertyType.unwrapTypeAlias() + + if (unwrappedType !is TypeVariableName) { + return unwrappedType + } else if (entryStartIndex == -1) { + return unwrappedType + } + + val targetMappingIndex = resolvedTypes[entryStartIndex] + val targetMappings = targetMappingIndex.args + + // Try to resolve the real type of this property based on mapped generics in the subclass. + // We need to us a non-nullable version for mapping since we're just mapping based on raw java + // type vars, but then can re-copy nullability back if it is found. + val resolvedType = targetMappings[unwrappedType.name] + ?.copy(nullable = unwrappedType.isNullable) + ?: unwrappedType + + return when { + resolvedType !is TypeVariableName -> resolvedType + entryStartIndex != 0 -> { + // We need to go deeper + resolveTypeArgs(targetClass, resolvedType, resolvedTypes, allowedTypeVars, entryStartIndex - 1) + } + resolvedType.copy(nullable = false) in allowedTypeVars -> { + // This is a generic type in the top-level declared class. This is fine to leave in because + // this will be handled by the `Type` array passed in at runtime. + resolvedType + } + else -> error("Could not find $resolvedType in $resolvedTypes. Also not present in allowable top-level type vars $allowedTypeVars") + } +} + /** Returns the properties declared by `typeElement`. */ @KotlinPoetMetadataPreview private fun declaredProperties( constructor: TargetConstructor, - kotlinApi: TypeSpec + kotlinApi: TypeSpec, + allowedTypeVars: Set, + currentClass: ClassName, + resolvedTypes: List ): Map { val result = mutableMapOf() for (initialProperty in kotlinApi.propertySpecs) { - val property = initialProperty.toBuilder(type = initialProperty.type.unwrapTypeAlias()).build() + val resolvedType = resolveTypeArgs( + targetClass = currentClass, + propertyType = initialProperty.type, + resolvedTypes = resolvedTypes, + allowedTypeVars = allowedTypeVars + ) + val property = initialProperty.toBuilder(type = resolvedType).build() val name = property.name val parameter = constructor.parameters[name] result[name] = TargetProperty( @@ -380,3 +478,10 @@ internal val TypeElement.metadata: Metadata return getAnnotation(Metadata::class.java) ?: throw IllegalStateException("Not a kotlin type! $this") } + +private fun Sequence<*>.cast(): Sequence { + return map { + @Suppress("UNCHECKED_CAST") + it as E + } +} diff --git a/kotlin/tests/src/test/kotlin/com/squareup/moshi/kotlin/codegen/ComplexGenericsInheritanceTest.kt b/kotlin/tests/src/test/kotlin/com/squareup/moshi/kotlin/codegen/ComplexGenericsInheritanceTest.kt new file mode 100644 index 0000000..a3a16f8 --- /dev/null +++ b/kotlin/tests/src/test/kotlin/com/squareup/moshi/kotlin/codegen/ComplexGenericsInheritanceTest.kt @@ -0,0 +1,132 @@ +@file:Suppress("UNUSED", "UNUSED_PARAMETER") + +package com.squareup.moshi.kotlin.codegen + +import com.squareup.moshi.JsonClass +import com.squareup.moshi.Moshi +import com.squareup.moshi.kotlin.reflect.adapter +import org.assertj.core.api.Assertions.assertThat +import org.intellij.lang.annotations.Language +import org.junit.Test + +@ExperimentalStdlibApi +class ComplexGenericsInheritanceTest { + + private val moshi = Moshi.Builder().build() + + @Test + fun simple() { + val adapter = moshi.adapter() + + @Language("JSON") + val json = """{"data":{"name":"foo"},"data2":"bar","data3":"baz"}""" + + val instance = adapter.fromJson(json)!! + val testInstance = PersonResponse().apply { + data = Person("foo") + } + assertThat(instance).isEqualTo(testInstance) + assertThat(adapter.toJson(instance)).isEqualTo(json) + } + + @Test + fun nested() { + val adapter = moshi.adapter() + + @Language("JSON") + val json = """{"data":{"name":"foo"},"data2":"bar","data3":"baz"}""" + + val instance = adapter.fromJson(json)!! + val testInstance = NestedPersonResponse().apply { + data = Person("foo") + } + assertThat(instance).isEqualTo(testInstance) + assertThat(adapter.toJson(instance)).isEqualTo(json) + } + + @Test + fun untyped() { + val adapter = moshi.adapter>() + + @Language("JSON") + val json = """{"data":{"name":"foo"},"data2":"bar","data3":"baz"}""" + + val instance = adapter.fromJson(json)!! + val testInstance = UntypedNestedPersonResponse().apply { + data = Person("foo") + } + assertThat(instance).isEqualTo(testInstance) + assertThat(adapter.toJson(instance)).isEqualTo(json) + } + + @Test + fun complex() { + val adapter = moshi.adapter>>() + + @Language("JSON") + val json = """{"layer4E":{"name":"layer4E"},"layer4F":{"data":{"name":"layer4F"},"data2":"layer4F","data3":"layer4F"},"layer3C":[1,2,3],"layer3D":"layer3D","layer2":"layer2","layer1":"layer1"}""" + + val instance = adapter.fromJson(json)!! + val testInstance = Layer4( + layer4E = Person("layer4E"), + layer4F = UntypedNestedPersonResponse().apply { + data = Person("layer4F") + data2 = "layer4F" + data3 = "layer4F" + } + ).apply { + layer3C = listOf(1, 2, 3) + layer3D = "layer3D" + layer2 = "layer2" + layer1 = "layer1" + } + assertThat(instance).isEqualTo(testInstance) + assertThat(adapter.toJson(testInstance)).isEqualTo(json) + } +} + +open class ResponseWithSettableProperty { + var data: T? = null + var data2: R? = null + var data3: R? = null +} + +interface Personable + +@JsonClass(generateAdapter = true) +data class Person(val name: String) : Personable + +@JsonClass(generateAdapter = true) +data class PersonResponse( + val extra: String? = null) : ResponseWithSettableProperty() + +abstract class NestedResponse : ResponseWithSettableProperty() + +@JsonClass(generateAdapter = true) +data class NestedPersonResponse(val extra: String? = null) : NestedResponse() + +@JsonClass(generateAdapter = true) +data class UntypedNestedPersonResponse( + val extra: String? = null +) : NestedResponse() + +interface LayerInterface + +abstract class Layer1 { + var layer1: A? = null +} + +abstract class Layer2 : Layer1(), LayerInterface { + var layer2: B? = null +} + +abstract class Layer3 : Layer2() { + var layer3C: C? = null + var layer3D: D? = null +} + +@JsonClass(generateAdapter = true) +data class Layer4( + val layer4E: E, + val layer4F: F? = null +) : Layer3, String>(), LayerInterface