KSP followups from #1393 (#1448)

* Rename to unwrapTypeAliasInternal + simplify * Move down isAnyNullable * Make dynamic explicit * Clean up supertypes doc and filtering * Switch to invoke extensions * Just best guess the annotation * Clean up redundant sequence and use a regular loop * element -> type * supertypes -> superclasses * Spotless * Fix copyright * Add multiple messages check * Link issue Co-authored-by: Jesse Wilson <jesse@swank.ca>
2025-10-18 23:49:21 +08:00 · 2021-12-05 20:34:37 -05:00
parent f57d7200f3
commit fb5dd08168
8 changed files with 85 additions and 102 deletions
--- a/kotlin/codegen/src/main/java/com/squareup/moshi/kotlin/codegen/api/typeAliasUnwrapping.kt
+++ b/kotlin/codegen/src/main/java/com/squareup/moshi/kotlin/codegen/api/typeAliasUnwrapping.kt
@@ -17,6 +17,7 @@ package com.squareup.moshi.kotlin.codegen.api

 import com.squareup.kotlinpoet.AnnotationSpec
 import com.squareup.kotlinpoet.ClassName
+import com.squareup.kotlinpoet.Dynamic
 import com.squareup.kotlinpoet.LambdaTypeName
 import com.squareup.kotlinpoet.ParameterizedTypeName
 import com.squareup.kotlinpoet.TypeName
@@ -26,52 +27,35 @@ import com.squareup.kotlinpoet.tag
 import com.squareup.kotlinpoet.tags.TypeAliasTag
 import java.util.TreeSet

-internal fun TypeName.unwrapTypeAliasReal(): TypeName {
+private fun TypeName.unwrapTypeAliasInternal(): TypeName? {
  return tag<TypeAliasTag>()?.abbreviatedType?.let { unwrappedType ->
-    // If any type is nullable, then the whole thing is nullable
-    var isAnyNullable = isNullable
    // Keep track of all annotations across type levels. Sort them too for consistency.
    val runningAnnotations = TreeSet<AnnotationSpec>(compareBy { it.toString() }).apply {
      addAll(annotations)
    }
    val nestedUnwrappedType = unwrappedType.unwrapTypeAlias()
    runningAnnotations.addAll(nestedUnwrappedType.annotations)
-    isAnyNullable = isAnyNullable || nestedUnwrappedType.isNullable
+    // If any type is nullable, then the whole thing is nullable
+    val isAnyNullable = isNullable || nestedUnwrappedType.isNullable
    nestedUnwrappedType.copy(nullable = isAnyNullable, annotations = runningAnnotations.toList())
-  } ?: this
+  }
 }

 internal fun TypeName.unwrapTypeAlias(): TypeName {
  return when (this) {
-    is ClassName -> unwrapTypeAliasReal()
+    is ClassName -> unwrapTypeAliasInternal() ?: this
    is ParameterizedTypeName -> {
-      if (TypeAliasTag::class in tags) {
-        unwrapTypeAliasReal()
-      } else {
-        deepCopy(TypeName::unwrapTypeAlias)
-      }
+      unwrapTypeAliasInternal() ?: deepCopy(TypeName::unwrapTypeAlias)
    }
    is TypeVariableName -> {
-      if (TypeAliasTag::class in tags) {
-        unwrapTypeAliasReal()
-      } else {
-        deepCopy(transform = TypeName::unwrapTypeAlias)
-      }
+      unwrapTypeAliasInternal() ?: deepCopy(transform = TypeName::unwrapTypeAlias)
    }
    is WildcardTypeName -> {
-      if (TypeAliasTag::class in tags) {
-        unwrapTypeAliasReal()
-      } else {
-        deepCopy(TypeName::unwrapTypeAlias)
-      }
+      unwrapTypeAliasInternal() ?: deepCopy(TypeName::unwrapTypeAlias)
    }
    is LambdaTypeName -> {
-      if (TypeAliasTag::class in tags) {
-        unwrapTypeAliasReal()
-      } else {
-        deepCopy(TypeName::unwrapTypeAlias)
-      }
+      unwrapTypeAliasInternal() ?: deepCopy(TypeName::unwrapTypeAlias)
    }
-    else -> throw UnsupportedOperationException("Type '${javaClass.simpleName}' is illegal. Only classes, parameterized types, wildcard types, or type variables are allowed.")
+    Dynamic -> throw UnsupportedOperationException("Type '${javaClass.simpleName}' is illegal. Only classes, parameterized types, wildcard types, or type variables are allowed.")
  }
 }
--- a/kotlin/codegen/src/main/java/com/squareup/moshi/kotlin/codegen/apt/AppliedType.kt
+++ b/kotlin/codegen/src/main/java/com/squareup/moshi/kotlin/codegen/apt/AppliedType.kt
@@ -15,10 +15,16 @@
 */
 package com.squareup.moshi.kotlin.codegen.apt

+import com.squareup.kotlinpoet.ClassName
+import com.squareup.kotlinpoet.DelicateKotlinPoetApi
+import com.squareup.kotlinpoet.asClassName
+import javax.lang.model.element.ElementKind.CLASS
 import javax.lang.model.element.TypeElement
 import javax.lang.model.type.DeclaredType
 import javax.lang.model.util.Types

+private val OBJECT_CLASS = ClassName("java.lang", "Object")
+
 /**
 * A concrete type like `List<String>` with enough information to know how to resolve its type
 * variables.
@@ -27,8 +33,9 @@ internal class AppliedType private constructor(
  val element: TypeElement,
  private val mirror: DeclaredType
 ) {
-  /** Returns all supertypes of this, recursively. Includes both interface and class supertypes. */
-  fun supertypes(
+  /** Returns all supertypes of this, recursively. Only [CLASS] is used as we can't really use other types. */
+  @OptIn(DelicateKotlinPoetApi::class)
+  fun superclasses(
    types: Types,
    result: LinkedHashSet<AppliedType> = LinkedHashSet()
  ): LinkedHashSet<AppliedType> {
@@ -36,8 +43,14 @@ internal class AppliedType private constructor(
    for (supertype in types.directSupertypes(mirror)) {
      val supertypeDeclaredType = supertype as DeclaredType
      val supertypeElement = supertypeDeclaredType.asElement() as TypeElement
-      val appliedSupertype = AppliedType(supertypeElement, supertypeDeclaredType)
-      appliedSupertype.supertypes(types, result)
+      if (supertypeElement.kind != CLASS) {
+        continue
+      } else if (supertypeElement.asClassName() == OBJECT_CLASS) {
+        // Don't load properties for java.lang.Object.
+        continue
+      }
+      val appliedSuperclass = AppliedType(supertypeElement, supertypeDeclaredType)
+      appliedSuperclass.superclasses(types, result)
    }
    return result
  }
@@ -45,7 +58,7 @@ internal class AppliedType private constructor(
  override fun toString() = mirror.toString()

  companion object {
-    fun get(typeElement: TypeElement): AppliedType {
+    operator fun invoke(typeElement: TypeElement): AppliedType {
      return AppliedType(typeElement, typeElement.asType() as DeclaredType)
    }
  }
--- a/kotlin/codegen/src/main/java/com/squareup/moshi/kotlin/codegen/apt/metadata.kt
+++ b/kotlin/codegen/src/main/java/com/squareup/moshi/kotlin/codegen/apt/metadata.kt
@@ -52,7 +52,6 @@ import java.lang.annotation.RetentionPolicy
 import javax.annotation.processing.Messager
 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
@@ -63,7 +62,6 @@ import javax.tools.Diagnostic.Kind.WARNING
 private val JSON_QUALIFIER = JsonQualifier::class.java
 private val JSON = Json::class.asClassName()
 private val TRANSIENT = Transient::class.asClassName()
-private val OBJECT_CLASS = ClassName("java.lang", "Object")
 private val VISIBILITY_MODIFIERS = setOf(
  KModifier.INTERNAL,
  KModifier.PRIVATE,
@@ -206,7 +204,7 @@ internal fun targetType(

  val kotlinApi = cachedClassInspector.toTypeSpec(kmClass)
  val typeVariables = kotlinApi.typeVariables
-  val appliedType = AppliedType.get(element)
+  val appliedType = AppliedType(element)

  val constructor = primaryConstructor(element, kotlinApi, elements, messager)
  if (constructor == null) {
@@ -230,28 +228,24 @@ internal fun targetType(
  val properties = mutableMapOf<String, TargetProperty>()

  val resolvedTypes = mutableListOf<ResolvedTypeMapping>()
-  val superTypes = appliedType.supertypes(types)
-    .filterNot { supertype ->
-      supertype.element.asClassName() == OBJECT_CLASS || // Don't load properties for java.lang.Object.
-        supertype.element.kind != ElementKind.CLASS // Don't load properties for interface types.
-    }
-    .onEach { supertype ->
-      if (supertype.element.getAnnotation(Metadata::class.java) == null) {
+  val superclass = appliedType.superclasses(types)
+    .onEach { superclass ->
+      if (superclass.element.getAnnotation(Metadata::class.java) == null) {
        messager.printMessage(
          ERROR,
-          "@JsonClass can't be applied to $element: supertype $supertype is not a Kotlin type",
+          "@JsonClass can't be applied to $element: supertype $superclass is not a Kotlin type",
          element
        )
        return null
      }
    }
-    .associateWithTo(LinkedHashMap()) { supertype ->
+    .associateWithTo(LinkedHashMap()) { superclass ->
      // Load the kotlin API cache into memory eagerly so we can reuse the parsed APIs
-      val api = if (supertype.element == element) {
+      val api = if (superclass.element == element) {
        // We've already parsed this api above, reuse it
        kotlinApi
      } else {
-        cachedClassInspector.toTypeSpec(supertype.element)
+        cachedClassInspector.toTypeSpec(superclass.element)
      }

      val apiSuperClass = api.superclass
@@ -268,7 +262,7 @@ internal fun targetType(
        // Then when we look at Bar<T> later, we'll look up to the descendent Foo and extract its
        // materialized type from there.
        //
-        val superSuperClass = supertype.element.superclass as DeclaredType
+        val superSuperClass = superclass.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
@@ -285,7 +279,7 @@ internal fun targetType(
      return@associateWithTo api
    }

-  for ((localAppliedType, supertypeApi) in superTypes.entries) {
+  for ((localAppliedType, supertypeApi) in superclass.entries) {
    val appliedClassName = localAppliedType.element.asClassName()
    val supertypeProperties = declaredProperties(
      constructor = constructor,
--- a/kotlin/codegen/src/main/java/com/squareup/moshi/kotlin/codegen/ksp/AppliedType.kt
+++ b/kotlin/codegen/src/main/java/com/squareup/moshi/kotlin/codegen/ksp/AppliedType.kt
@@ -35,8 +35,8 @@ internal class AppliedType private constructor(
  val typeName: TypeName = type.toClassName()
 ) {

-  /** Returns all supertypes of this, recursively. Includes both interface and class supertypes. */
-  fun supertypes(
+  /** Returns all super classes of this, recursively. Only [CLASS] is used as we can't really use other types. */
+  fun superclasses(
    resolver: Resolver,
  ): LinkedHashSet<AppliedType> {
    val result: LinkedHashSet<AppliedType> = LinkedHashSet()
@@ -63,7 +63,7 @@ internal class AppliedType private constructor(
  override fun toString() = type.qualifiedName!!.asString()

  companion object {
-    fun get(type: KSClassDeclaration): AppliedType {
+    operator fun invoke(type: KSClassDeclaration): AppliedType {
      return AppliedType(type)
    }
  }
--- a/kotlin/codegen/src/main/java/com/squareup/moshi/kotlin/codegen/ksp/JsonClassSymbolProcessorProvider.kt
+++ b/kotlin/codegen/src/main/java/com/squareup/moshi/kotlin/codegen/ksp/JsonClassSymbolProcessorProvider.kt
@@ -27,8 +27,8 @@ import com.google.devtools.ksp.symbol.KSAnnotated
 import com.google.devtools.ksp.symbol.KSDeclaration
 import com.google.devtools.ksp.symbol.KSFile
 import com.squareup.kotlinpoet.AnnotationSpec
+import com.squareup.kotlinpoet.ClassName
 import com.squareup.kotlinpoet.ksp.addOriginatingKSFile
-import com.squareup.kotlinpoet.ksp.toClassName
 import com.squareup.kotlinpoet.ksp.writeTo
 import com.squareup.moshi.JsonClass
 import com.squareup.moshi.kotlin.codegen.api.AdapterGenerator
@@ -66,54 +66,47 @@ private class JsonClassSymbolProcessor(

  override fun process(resolver: Resolver): List<KSAnnotated> {
    val generatedAnnotation = generatedOption?.let {
-      val annotationType = resolver.getClassDeclarationByName(resolver.getKSNameFromString(it))
-        ?: run {
-          logger.error("Generated annotation type doesn't exist: $it")
-          return emptyList()
-        }
-      AnnotationSpec.builder(annotationType.toClassName())
+      AnnotationSpec.builder(ClassName.bestGuess(it))
        .addMember("value = [%S]", JsonClassSymbolProcessor::class.java.canonicalName)
        .addMember("comments = %S", "https://github.com/square/moshi")
        .build()
    }

-    resolver.getSymbolsWithAnnotation(JSON_CLASS_NAME)
-      .asSequence()
-      .forEach { type ->
-        // For the smart cast
-        if (type !is KSDeclaration) {
-          logger.error("@JsonClass can't be applied to $type: must be a Kotlin class", type)
-          return@forEach
-        }
-
-        val jsonClassAnnotation = type.findAnnotationWithType<JsonClass>() ?: return@forEach
-
-        val generator = jsonClassAnnotation.generator
-
-        if (generator.isNotEmpty()) return@forEach
-
-        if (!jsonClassAnnotation.generateAdapter) return@forEach
-
-        val originatingFile = type.containingFile!!
-        val adapterGenerator = adapterGenerator(logger, resolver, type) ?: return emptyList()
-        try {
-          val preparedAdapter = adapterGenerator
-            .prepare(generateProguardRules) { spec ->
-              spec.toBuilder()
-                .apply {
-                  generatedAnnotation?.let(::addAnnotation)
-                }
-                .addOriginatingKSFile(originatingFile)
-                .build()
-            }
-          preparedAdapter.spec.writeTo(codeGenerator, aggregating = false)
-          preparedAdapter.proguardConfig?.writeTo(codeGenerator, originatingFile)
-        } catch (e: Exception) {
-          logger.error(
-            "Error preparing ${type.simpleName.asString()}: ${e.stackTrace.joinToString("\n")}"
-          )
-        }
+    for (type in resolver.getSymbolsWithAnnotation(JSON_CLASS_NAME)) {
+      // For the smart cast
+      if (type !is KSDeclaration) {
+        logger.error("@JsonClass can't be applied to $type: must be a Kotlin class", type)
+        continue
      }
+
+      val jsonClassAnnotation = type.findAnnotationWithType<JsonClass>() ?: continue
+
+      val generator = jsonClassAnnotation.generator
+
+      if (generator.isNotEmpty()) continue
+
+      if (!jsonClassAnnotation.generateAdapter) continue
+
+      val originatingFile = type.containingFile!!
+      val adapterGenerator = adapterGenerator(logger, resolver, type) ?: return emptyList()
+      try {
+        val preparedAdapter = adapterGenerator
+          .prepare(generateProguardRules) { spec ->
+            spec.toBuilder()
+              .apply {
+                generatedAnnotation?.let(::addAnnotation)
+              }
+              .addOriginatingKSFile(originatingFile)
+              .build()
+          }
+        preparedAdapter.spec.writeTo(codeGenerator, aggregating = false)
+        preparedAdapter.proguardConfig?.writeTo(codeGenerator, originatingFile)
+      } catch (e: Exception) {
+        logger.error(
+          "Error preparing ${type.simpleName.asString()}: ${e.stackTrace.joinToString("\n")}"
+        )
+      }
+    }
    return emptyList()
  }

--- a/kotlin/codegen/src/main/java/com/squareup/moshi/kotlin/codegen/ksp/TargetTypes.kt
+++ b/kotlin/codegen/src/main/java/com/squareup/moshi/kotlin/codegen/ksp/TargetTypes.kt
@@ -53,7 +53,7 @@ import com.squareup.moshi.kotlin.codegen.api.TargetProperty
 import com.squareup.moshi.kotlin.codegen.api.TargetType
 import com.squareup.moshi.kotlin.codegen.api.unwrapTypeAlias

-/** Returns a target type for `element`, or null if it cannot be used with code gen. */
+/** Returns a target type for [type] or null if it cannot be used with code gen. */
 internal fun targetType(
  type: KSDeclaration,
  resolver: Resolver,
@@ -89,7 +89,7 @@ internal fun targetType(
    sourceTypeHint = type.qualifiedName!!.asString()
  )
  val typeVariables = type.typeParameters.map { it.toTypeVariableName(classTypeParamsResolver) }
-  val appliedType = AppliedType.get(type)
+  val appliedType = AppliedType(type)

  val constructor = primaryConstructor(resolver, type, classTypeParamsResolver, logger)
    ?: run {
@@ -108,12 +108,12 @@ internal fun targetType(
  val properties = mutableMapOf<String, TargetProperty>()

  val originalType = appliedType.type
-  for (supertype in appliedType.supertypes(resolver)) {
-    val classDecl = supertype.type
+  for (superclass in appliedType.superclasses(resolver)) {
+    val classDecl = superclass.type
    if (!classDecl.isKotlinClass()) {
      logger.error(
        """
-        @JsonClass can't be applied to $type: supertype $supertype is not a Kotlin type.
+        @JsonClass can't be applied to $type: supertype $superclass is not a Kotlin type.
        Origin=${classDecl.origin}
        Annotations=${classDecl.annotations.joinToString(prefix = "[", postfix = "]") { it.shortName.getShortName() }}
        """.trimIndent(),
--- a/kotlin/codegen/src/test/java/com/squareup/moshi/kotlin/codegen/ksp/JsonClassSymbolProcessorTest.kt
+++ b/kotlin/codegen/src/test/java/com/squareup/moshi/kotlin/codegen/ksp/JsonClassSymbolProcessorTest.kt
@@ -430,8 +430,7 @@ class JsonClassSymbolProcessorTest {
    )
    assertThat(result.exitCode).isEqualTo(KotlinCompilation.ExitCode.COMPILATION_ERROR)
    assertThat(result.messages).contains("property a is not visible")
-    // TODO we throw eagerly currently and don't collect
-//    assertThat(result.messages).contains("property c is not visible")
+    assertThat(result.messages).contains("property c is not visible")
  }

  @Test
--- a/kotlin/tests/src/test/kotlin/com/squareup/moshi/kotlin/DualKotlinTest.kt
+++ b/kotlin/tests/src/test/kotlin/com/squareup/moshi/kotlin/DualKotlinTest.kt
@@ -257,7 +257,7 @@ class DualKotlinTest {
    val result = adapter.fromJson(testJson)!!
    assertThat(result.i).isEqualTo(6)

-    // TODO doesn't work yet.
+    // TODO doesn't work yet. https://github.com/square/moshi/issues/1170
    //  need to invoke the constructor_impl$default static method, invoke constructor with result
 //    val testEmptyJson =
 //      """{}"""