Fix nullability not being preserved and clean up from shadowed names (#529)

* Add helper TypeName.asNullableIf extension

* Add missing nullability preservers to TypeResolver

* Fix shadowed names and add more missing nullable stuff

kotlin-codegen/compiler/src/main/java/com/squareup/moshi/TypeResolver.kt

@@ -35,15 +35,18 @@ open class TypeResolver {
       is ParameterizedTypeName -> {
         ParameterizedTypeName.get(
             typeName.rawType, *(typeName.typeArguments.map { resolve(it) }.toTypedArray()))
+            .asNullableIf(typeName.nullable)
       }
 
       is WildcardTypeName -> {
         when {
           typeName.lowerBounds.size == 1 -> {
             WildcardTypeName.supertypeOf(resolve(typeName.lowerBounds[0]))
+                .asNullableIf(typeName.nullable)
           }
           typeName.upperBounds.size == 1 -> {
             WildcardTypeName.subtypeOf(resolve(typeName.upperBounds[0]))
+                .asNullableIf(typeName.nullable)
           }
           else -> {
             throw IllegalArgumentException(
@@ -57,4 +60,4 @@ open class TypeResolver {
       else -> throw IllegalArgumentException("Unrepresentable type: $typeName")
     }
   }
-}
+}

kotlin-codegen/compiler/src/main/java/com/squareup/moshi/kotlintypes.kt

@@ -26,3 +26,7 @@ internal fun TypeName.rawType(): ClassName {
     else -> throw IllegalArgumentException("Cannot get raw type from $this")
   }
 }
+
+internal fun TypeName.asNullableIf(condition: Boolean): TypeName {
+  return if (condition) asNullable() else this
+}

kotlin-codegen/compiler/src/main/java/com/squareup/moshi/metadata.kt

@@ -72,15 +72,17 @@ internal fun Type.asTypeName(
   if (hasFlexibleUpperBound()) {
     return WildcardTypeName.subtypeOf(
         flexibleUpperBound.asTypeName(nameResolver, getTypeParameter, resolveAliases))
+        .asNullableIf(nullable)
   } else if (hasOuterType()) {
     return WildcardTypeName.supertypeOf(
         outerType.asTypeName(nameResolver, getTypeParameter, resolveAliases))
+        .asNullableIf(nullable)
   }
 
   val realType = when {
     hasTypeParameter() -> return getTypeParameter(typeParameter)
         .asTypeName(nameResolver, getTypeParameter, resolveAliases)
-        .let { if (nullable) it.asNullable() else it }
+        .asNullableIf(nullable)
     hasTypeParameterName() -> typeParameterName
     hasAbbreviatedType() && !resolveAliases -> abbreviatedType.typeAliasName
     else -> className
@@ -91,43 +93,36 @@ internal fun Type.asTypeName(
           .replace("/", "."))
 
   if (argumentList.isNotEmpty()) {
-    val remappedArgs: Array<TypeName> = argumentList.map {
+    val remappedArgs: Array<TypeName> = argumentList.map { argumentType ->
-      val projection = if (it.hasProjection()) {
+      val nullableProjection = if (argumentType.hasProjection()) {
-        it.projection
+        argumentType.projection
       } else null
-      if (it.hasType()) {
+      if (argumentType.hasType()) {
-        it.type.asTypeName(nameResolver, getTypeParameter, resolveAliases)
+        argumentType.type.asTypeName(nameResolver, getTypeParameter, resolveAliases)
-            .let { typeName ->
+            .let { argumentTypeName ->
-              projection?.let {
+              nullableProjection?.let { projection ->
-                when (it) {
+                when (projection) {
-                  Type.Argument.Projection.IN -> WildcardTypeName.supertypeOf(
+                  Type.Argument.Projection.IN -> WildcardTypeName.supertypeOf(argumentTypeName)
-                      typeName)
                   Type.Argument.Projection.OUT -> {
-                    if (typeName == ANY) {
+                    if (argumentTypeName == ANY) {
                       // This becomes a *, which we actually don't want here.
                       // List<Any> works with List<*>, but List<*> doesn't work with List<Any>
-                      typeName
+                      argumentTypeName
                     } else {
-                      WildcardTypeName.subtypeOf(typeName)
+                      WildcardTypeName.subtypeOf(argumentTypeName)
                     }
                   }
-                  Type.Argument.Projection.STAR -> WildcardTypeName.subtypeOf(
+                  Type.Argument.Projection.STAR -> WildcardTypeName.subtypeOf(ANY)
-                      ANY)
                   Type.Argument.Projection.INV -> TODO("INV projection is unsupported")
                 }
-              } ?: typeName
+              } ?: argumentTypeName
             }
       } else {
         WildcardTypeName.subtypeOf(ANY)
       }
     }.toTypedArray()
-    typeName = ParameterizedTypeName.get(
+    typeName = ParameterizedTypeName.get(typeName as ClassName, *remappedArgs)
-        typeName as ClassName, *remappedArgs)
   }
 
-  if (nullable) {
+  return typeName.asNullableIf(nullable)
-    typeName = typeName.asNullable()
-  }
-
-  return typeName
 }

kotlin-codegen/compiler/src/test/java/com/squareup/moshi/TypeResolverTest.kt

@@ -0,0 +1,36 @@
+package com.squareup.moshi
+
+import com.google.common.truth.Truth.assertThat
+import com.squareup.kotlinpoet.ParameterizedTypeName
+import com.squareup.kotlinpoet.WildcardTypeName
+import com.squareup.kotlinpoet.asClassName
+import org.junit.Test
+
+class TypeResolverTest {
+
+  private val resolver = TypeResolver()
+
+  @Test
+  fun ensureClassNameNullabilityIsPreserved() {
+    assertThat(resolver.resolve(Int::class.asClassName().asNullable()).nullable).isTrue()
+  }
+
+  @Test
+  fun ensureParameterizedNullabilityIsPreserved() {
+    val nullableTypeName = ParameterizedTypeName.get(
+        List::class.asClassName(),
+        String::class.asClassName())
+        .asNullable()
+
+    assertThat(resolver.resolve(nullableTypeName).nullable).isTrue()
+  }
+
+  @Test
+  fun ensureWildcardNullabilityIsPreserved() {
+    val nullableTypeName = WildcardTypeName.subtypeOf(List::class.asClassName())
+        .asNullable()
+
+    assertThat(resolver.resolve(nullableTypeName).nullable).isTrue()
+  }
+
+}