Fixed naive toClassName function that was breaking FQ names (#49)

r0adkll · Sep 10, 2024 · 2647251 · 2647251
1 parent 0db50f5
commit 2647251
Show file tree

Hide file tree

Showing 12 changed files with 160 additions and 29 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -21,6 +21,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Other Notes & Contributions
 -->
 
+### Fixed
+
+- Fixed naive `toClassName()` function that broke on nested classes - [https://github.com/r0adkll/kimchi/pull/48]
+
 ## [0.3.0] - 2024-09-09
 
 ### Fixed

diff --git a/circuit/compiler/src/main/kotlin/com/r0adkll/kimchi/circuit/CircuitInjectSymbolProcessor.kt b/circuit/compiler/src/main/kotlin/com/r0adkll/kimchi/circuit/CircuitInjectSymbolProcessor.kt
@@ -23,11 +23,12 @@ import com.r0adkll.kimchi.util.buildConstructor
 import com.r0adkll.kimchi.util.buildFile
 import com.r0adkll.kimchi.util.capitalized
 import com.r0adkll.kimchi.util.kotlinpoet.toParameterSpec
+import com.r0adkll.kimchi.util.ksp.asMemberName
 import com.r0adkll.kimchi.util.ksp.directReturnTypeIs
+import com.r0adkll.kimchi.util.ksp.findActualType
 import com.r0adkll.kimchi.util.ksp.hasAnnotation
 import com.r0adkll.kimchi.util.ksp.implements
 import com.r0adkll.kimchi.util.ksp.returnTypeIs
-import com.r0adkll.kimchi.util.toClassName
 import com.squareup.kotlinpoet.AnnotationSpec
 import com.squareup.kotlinpoet.ClassName
 import com.squareup.kotlinpoet.CodeBlock
@@ -221,7 +222,7 @@ class CircuitInjectSymbolProcessor(
       ?.mapNotNull { parameter ->
         if (parameter.hasAnnotation(Assisted::class)) {
           // Validate that injected type is allowed type
-          val parameterTypeClassName = parameter.type.resolve().declaration.toClassName()
+          val parameterTypeClassName = parameter.type.findActualType().toClassName()
           if (parameterTypeClassName in allowedAssistedTypes) {
             ParameterSpec(parameter.name!!.asString(), parameterTypeClassName)
           } else {
@@ -362,9 +363,9 @@ class CircuitInjectSymbolProcessor(
                   .beginControlFlow("return when(screen)")
                   .beginControlFlow("is %T ->", annotation.screen)
                   .addStatement(
-                    "%M { %T() }",
+                    "%M { %M() }",
                     MemberNames.CircuitPresenterOf,
-                    element.toClassName(),
+                    element.asMemberName(),
                   )
                   .endControlFlow()
                   .addStatement("else -> null")
@@ -403,7 +404,7 @@ class CircuitInjectSymbolProcessor(
       ?.mapNotNull { parameter ->
         if (parameter.hasAnnotation(Assisted::class)) {
           // Validate that injected type is allowed type
-          val parameterTypeClassName = parameter.type.resolve().declaration.toClassName()
+          val parameterTypeClassName = parameter.type.findActualType().toClassName()
           if (parameterTypeClassName in allowedAssistedTypes) {
             ParameterSpec(parameter.name!!.asString(), parameterTypeClassName)
           } else {

diff --git a/circuit/compiler/src/main/kotlin/com/r0adkll/kimchi/circuit/util/UiFactory.kt b/circuit/compiler/src/main/kotlin/com/r0adkll/kimchi/circuit/util/UiFactory.kt
@@ -3,12 +3,13 @@
 package com.r0adkll.kimchi.circuit.util
 
 import com.google.devtools.ksp.symbol.KSFunctionDeclaration
+import com.r0adkll.kimchi.util.ksp.asMemberName
 import com.r0adkll.kimchi.util.ksp.findActualType
 import com.r0adkll.kimchi.util.ksp.findParameterThatImplements
 import com.r0adkll.kimchi.util.ksp.implements
-import com.r0adkll.kimchi.util.toClassName
 import com.squareup.kotlinpoet.ClassName
 import com.squareup.kotlinpoet.CodeBlock
+import com.squareup.kotlinpoet.ksp.toClassName
 
 fun CodeBlock.Builder.addUiFactoryCreateStatement(
   element: KSFunctionDeclaration,
@@ -39,11 +40,12 @@ fun CodeBlock.Builder.addUiFactoryCreateStatement(
   // [CircuitUiState] type parameter.
   val stateClassName = stateClassParameter?.type?.findActualType()?.toClassName()
     ?: ClassNames.Circuit.UiState
+
   addStatement(
-    "%M<%T> { state, modifier -> %T(%L) }",
+    "%M<%T> { state, modifier -> %M(%L) }",
     MemberNames.CircuitUi,
     stateClassName,
-    element.toClassName(),
+    element.asMemberName(),
     callParameters.joinToString(),
   )
 }
diff --git a/circuit/compiler/src/test/kotlin/com/r0adkll/kimchi/circuit/UiFunctionFactoryTest.kt b/circuit/compiler/src/test/kotlin/com/r0adkll/kimchi/circuit/UiFunctionFactoryTest.kt
@@ -18,19 +18,19 @@ import me.tatarka.inject.annotations.Inject
 import me.tatarka.inject.annotations.IntoSet
 import me.tatarka.inject.annotations.Provides
 import org.junit.jupiter.api.Test
+import org.junit.jupiter.api.io.CleanupMode
 import org.junit.jupiter.api.io.TempDir
 import strikt.api.expectThat
 import strikt.assertions.isEqualTo
 import strikt.assertions.isNotNull
 
 class UiFunctionFactoryTest {
 
-  @TempDir
+  @TempDir(cleanup = CleanupMode.NEVER)
   lateinit var workingDir: File
 
   @Test
   fun `ui composable function generates factory and contributed component`() {
-    println(workingDir.absolutePath)
     compileKimchiWithTestSources(
       """
         package kimchi
@@ -70,7 +70,6 @@ class UiFunctionFactoryTest {
 
   @Test
   fun `ui composable function for static screen generates factory and contributed component`() {
-    println(workingDir.absolutePath)
     compileKimchiWithTestSources(
       """
         package kimchi
@@ -110,7 +109,6 @@ class UiFunctionFactoryTest {
 
   @Test
   fun `ui composable function with injected parameters injects into factory`() {
-    println(workingDir.absolutePath)
     compileKimchiWithTestSources(
       """
         package kimchi
@@ -144,7 +142,6 @@ class UiFunctionFactoryTest {
 
   @Test
   fun `ui composable function with injected typealias parameters injects into factory`() {
-    println(workingDir.absolutePath)
     compileKimchiWithTestSources(
       """
         package kimchi
@@ -175,4 +172,91 @@ class UiFunctionFactoryTest {
         .isTypeOf(injectedComposable)
     }
   }
+
+  @Test
+  fun `ui composable function with nested screen reference compiles`() {
+    compileKimchiWithTestSources(
+      """
+        package kimchi
+
+        import androidx.compose.runtime.Composable
+        import androidx.compose.ui.Modifier
+        import com.r0adkll.kimchi.circuit.annotations.CircuitInject
+        import com.slack.circuit.runtime.screen.Screen
+
+        data object Screens {
+          data object TestScreen : Screen
+        }
+
+        @CircuitInject(Screens.TestScreen::class, TestScope::class)
+        @Composable
+        fun TestUi(
+          state: TestUiState,
+          modifier: Modifier = Modifier,
+        ) { }
+      """.trimIndent(),
+      workingDir = workingDir,
+    ) {
+      val factory = kotlinClass("kimchi.TestUiUiFactory")
+      expectThat(factory)
+        .hasAnnotation(Inject::class)
+        .implements(Ui.Factory::class)
+
+      val component = kotlinClass("kimchi.TestUiUiFactoryComponent")
+      expectThat(component)
+        .withAnnotation<ContributesTo> {
+          get { scope } isEqualTo testScope
+        }
+        .withFunction("bindTestUiUiFactory") {
+          hasAnnotation(IntoSet::class)
+          hasAnnotation(Provides::class)
+          parameter(1)
+            .isTypeOf(factory)
+          hasReturnType(Ui.Factory::class)
+        }
+    }
+  }
+
+  @Test
+  fun `nested ui composable function compiles`() {
+    println(workingDir.absolutePath)
+    compileKimchiWithTestSources(
+      """
+        package kimchi
+
+        import androidx.compose.runtime.Composable
+        import androidx.compose.ui.Modifier
+        import com.r0adkll.kimchi.circuit.annotations.CircuitInject
+        import com.slack.circuit.runtime.screen.Screen
+
+        object TestUiScreen {
+          @CircuitInject(TestScreen::class, TestScope::class)
+          @Composable
+          fun TestUi(
+            state: TestUiState,
+            modifier: Modifier = Modifier,
+          ) { }
+        }
+      """.trimIndent(),
+      workingDir = workingDir,
+    ) {
+      val factory = kotlinClass("kimchi.TestUiUiFactory")
+      expectThat(factory)
+        .hasAnnotation(Inject::class)
+        .implements(Ui.Factory::class)
+
+      val component = kotlinClass("kimchi.TestUiUiFactoryComponent")
+      expectThat(component)
+        .withAnnotation<ContributesTo> {
+          get { scope } isEqualTo testScope
+        }
+        .withFunction("bindTestUiUiFactory") {
+          hasAnnotation(IntoSet::class)
+          hasAnnotation(Provides::class)
+          parameter(1)
+            .isTypeOf(factory)
+          hasReturnType(Ui.Factory::class)
+        }
+    }
+  }
 }
diff --git a/compiler-utils/src/main/kotlin/com/r0adkll/kimchi/util/KotlinPoetUtils.kt b/compiler-utils/src/main/kotlin/com/r0adkll/kimchi/util/KotlinPoetUtils.kt
@@ -2,7 +2,6 @@
 // SPDX-License-Identifier: Apache-2.0
 package com.r0adkll.kimchi.util
 
-import com.google.devtools.ksp.symbol.KSDeclaration
 import com.squareup.kotlinpoet.ClassName
 import com.squareup.kotlinpoet.FileSpec
 import com.squareup.kotlinpoet.FunSpec
@@ -45,8 +44,6 @@ public fun FunSpec.Companion.buildConstructor(
   .apply(builder)
   .build()
 
-public fun KSDeclaration.toClassName(): ClassName = ClassName(packageName.asString(), simpleName.asString())
-
 public fun <T> T.applyIf(predicate: Boolean, block: T.() -> Unit): T {
   return if (predicate) apply(block) else this
 }
diff --git a/compiler-utils/src/main/kotlin/com/r0adkll/kimchi/util/ksp/FunctionDeclarations.kt b/compiler-utils/src/main/kotlin/com/r0adkll/kimchi/util/ksp/FunctionDeclarations.kt
@@ -2,12 +2,13 @@
 // SPDX-License-Identifier: Apache-2.0
 package com.r0adkll.kimchi.util.ksp
 
+import com.google.devtools.ksp.closestClassDeclaration
 import com.google.devtools.ksp.getAllSuperTypes
 import com.google.devtools.ksp.symbol.KSClassDeclaration
 import com.google.devtools.ksp.symbol.KSFunctionDeclaration
 import com.google.devtools.ksp.symbol.KSValueParameter
-import com.r0adkll.kimchi.util.toClassName
 import com.squareup.kotlinpoet.ClassName
+import com.squareup.kotlinpoet.MemberName
 import com.squareup.kotlinpoet.asClassName
 import com.squareup.kotlinpoet.ksp.toClassName
 import kotlin.reflect.KClass
@@ -31,7 +32,7 @@ public fun KSValueParameter.implements(className: ClassName): Boolean {
     // Check if the direct type implements the passed className
     if (classDecl.toClassName() == className) return true
     return classDecl.getAllSuperTypes()
-      .any { it.declaration.toClassName() == className }
+      .any { it.classDeclaration.toClassName() == className }
   }
   return false
 }
@@ -44,7 +45,7 @@ public fun KSFunctionDeclaration.returnTypeIs(className: ClassName): Boolean {
   return returnType
     ?.findActualType()
     ?.getAllSuperTypes()
-    ?.any { it.declaration.toClassName() == className } == true
+    ?.any { it.classDeclaration.toClassName() == className } == true
 }
 
 public fun KSFunctionDeclaration.directReturnTypeIs(clazz: KClass<*>): Boolean {
@@ -56,3 +57,12 @@ public fun KSFunctionDeclaration.directReturnTypeIs(className: ClassName): Boole
     ?.findActualType()
     ?.toClassName() == className
 }
+
+public fun KSFunctionDeclaration.asMemberName(): MemberName {
+  val parentClass = closestClassDeclaration()
+  return if (parentClass != null) {
+    MemberName(parentClass.toClassName(), simpleName.asString())
+  } else {
+    MemberName(packageName.asString(), simpleName.asString())
+  }
+}
diff --git a/compiler-utils/src/main/kotlin/com/r0adkll/kimchi/util/ksp/KSValueArgument.kt b/compiler-utils/src/main/kotlin/com/r0adkll/kimchi/util/ksp/KSValueArgument.kt
@@ -4,16 +4,16 @@ package com.r0adkll.kimchi.util.ksp
 
 import com.google.devtools.ksp.symbol.KSType
 import com.google.devtools.ksp.symbol.KSValueArgument
-import com.r0adkll.kimchi.util.toClassName
 import com.squareup.kotlinpoet.ClassName
+import com.squareup.kotlinpoet.ksp.toClassName
 
 /**
  * Get the arguments value as a ClassName, if possible
  */
 public val KSValueArgument.valueAsClassName: ClassName?
   get() = value
     ?.let { it as? KSType }
-    ?.declaration
+    ?.classDeclaration
     ?.toClassName()
 
 /**
@@ -23,5 +23,5 @@ public val KSValueArgument.valueAsClassNameList: List<ClassName>?
   get() = value
     ?.let { it as? List<*> }
     ?.mapNotNull {
-      (it as? KSType)?.declaration?.toClassName()
+      (it as? KSType)?.classDeclaration?.toClassName()
     }
diff --git a/compiler-utils/src/main/kotlin/com/r0adkll/kimchi/util/ksp/KspUtil.kt b/compiler-utils/src/main/kotlin/com/r0adkll/kimchi/util/ksp/KspUtil.kt
@@ -6,6 +6,7 @@ import com.google.devtools.ksp.symbol.ClassKind
 import com.google.devtools.ksp.symbol.KSAnnotated
 import com.google.devtools.ksp.symbol.KSAnnotation
 import com.google.devtools.ksp.symbol.KSClassDeclaration
+import com.google.devtools.ksp.symbol.KSType
 import com.google.devtools.ksp.symbol.KSTypeAlias
 import com.google.devtools.ksp.symbol.KSTypeReference
 import com.squareup.kotlinpoet.ClassName
@@ -63,5 +64,8 @@ public fun KSTypeReference.findActualType(): KSClassDeclaration {
   }
 }
 
+public val KSType.classDeclaration: KSClassDeclaration
+  get() = declaration as KSClassDeclaration
+
 public val KSClassDeclaration.isInterface: Boolean
   get() = this.classKind == ClassKind.INTERFACE
diff --git a/compiler/src/main/kotlin/com/r0adkll/kimchi/processors/MergeComponentSymbolProcessor.kt b/compiler/src/main/kotlin/com/r0adkll/kimchi/processors/MergeComponentSymbolProcessor.kt
@@ -32,7 +32,6 @@ import com.r0adkll.kimchi.util.ksp.findQualifier
 import com.r0adkll.kimchi.util.ksp.hasAnnotation
 import com.r0adkll.kimchi.util.ksp.hasCompanionObject
 import com.r0adkll.kimchi.util.ksp.isInterface
-import com.r0adkll.kimchi.util.toClassName
 import com.squareup.kotlinpoet.AnnotationSpec
 import com.squareup.kotlinpoet.ClassName
 import com.squareup.kotlinpoet.FileSpec

diff --git a/compiler/src/main/kotlin/com/r0adkll/kimchi/util/kotlinpoet/Bindings.kt b/compiler/src/main/kotlin/com/r0adkll/kimchi/util/kotlinpoet/Bindings.kt
@@ -4,7 +4,6 @@ package com.r0adkll.kimchi.util.kotlinpoet
 
 import com.google.devtools.ksp.symbol.ClassKind
 import com.google.devtools.ksp.symbol.KSClassDeclaration
-import com.google.devtools.ksp.symbol.KSDeclaration
 import com.r0adkll.kimchi.annotations.ContributesMultibinding
 import com.r0adkll.kimchi.util.buildFun
 import com.r0adkll.kimchi.util.buildProperty
@@ -13,7 +12,6 @@ import com.r0adkll.kimchi.util.ksp.findMapKey
 import com.r0adkll.kimchi.util.ksp.findQualifier
 import com.r0adkll.kimchi.util.ksp.hasAnnotation
 import com.r0adkll.kimchi.util.ksp.pairTypeOf
-import com.r0adkll.kimchi.util.toClassName
 import com.squareup.kotlinpoet.ClassName
 import com.squareup.kotlinpoet.FunSpec
 import com.squareup.kotlinpoet.PropertySpec
@@ -135,7 +133,7 @@ private fun TypeSpec.Builder.addProvidesFunction(
 
 private fun TypeSpec.Builder.addMappingProvidesFunction(
   boundClass: KSClassDeclaration,
-  boundType: KSDeclaration,
+  boundType: KSClassDeclaration,
   mapKey: Any,
   isBindable: Boolean,
 ) {

diff --git a/compiler/src/main/kotlin/com/r0adkll/kimchi/util/ksp/BindingType.kt b/compiler/src/main/kotlin/com/r0adkll/kimchi/util/ksp/BindingType.kt
@@ -3,7 +3,6 @@
 package com.r0adkll.kimchi.util.ksp
 
 import com.google.devtools.ksp.symbol.KSClassDeclaration
-import com.google.devtools.ksp.symbol.KSDeclaration
 import com.google.devtools.ksp.symbol.KSType
 import com.r0adkll.kimchi.util.KimchiException
 import com.squareup.kotlinpoet.asTypeName
@@ -12,7 +11,7 @@ import kotlin.reflect.KClass
 
 fun KSClassDeclaration.findBindingTypeFor(
   bindingAnnotation: KClass<*>,
-): KSDeclaration {
+): KSClassDeclaration {
   // 1) Check if there is an explicit type defined by the binding annotation
   val annotation = findAnnotation(bindingAnnotation)
     ?: error("Unable to find annotation, ${bindingAnnotation.simpleName}, on class ${simpleName.asString()}")
@@ -21,7 +20,7 @@ fun KSClassDeclaration.findBindingTypeFor(
   val defaultTypeArgument = annotation.defaultArgumentAt(BOUND_TYPE_NAME, BOUND_TYPE_POSITIONAL_INDEX)
 
   return if (boundTypeArgument != null && boundTypeArgument.value != defaultTypeArgument?.value) {
-    (boundTypeArgument.value as KSType).declaration
+    (boundTypeArgument.value as KSType).declaration as KSClassDeclaration
   } else {
     val superTypeCount = superTypes
       .filterNot { it.toTypeName() == Any::class.asTypeName() }