From 2647251d3368e50113e6f5a11c82e0d082ae3ebc Mon Sep 17 00:00:00 2001 From: Drew Heavner Date: Tue, 10 Sep 2024 10:31:29 -0400 Subject: [PATCH] Fixed naive toClassName function that was breaking FQ names (#49) --- CHANGELOG.md | 4 + .../circuit/CircuitInjectSymbolProcessor.kt | 11 ++- .../r0adkll/kimchi/circuit/util/UiFactory.kt | 8 +- .../kimchi/circuit/UiFunctionFactoryTest.kt | 94 ++++++++++++++++++- .../r0adkll/kimchi/util/KotlinPoetUtils.kt | 3 - .../kimchi/util/ksp/FunctionDeclarations.kt | 16 +++- .../kimchi/util/ksp/KSValueArgument.kt | 6 +- .../com/r0adkll/kimchi/util/ksp/KspUtil.kt | 4 + .../MergeComponentSymbolProcessor.kt | 1 - .../kimchi/util/kotlinpoet/Bindings.kt | 4 +- .../r0adkll/kimchi/util/ksp/BindingType.kt | 5 +- .../processors/ContributedBindingTest.kt | 33 +++++++ 12 files changed, 160 insertions(+), 29 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f898c3b..2bf1694 100644 --- 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 index afda317..89609db 100644 --- 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 index a6cf72c..0e0fb94 100644 --- 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 index 305cab5..a85837e 100644 --- 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,6 +18,7 @@ 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 @@ -25,12 +26,11 @@ 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 { + 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 { + 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 index 5e53680..8acc203 100644 --- 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.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 index c469548..2e9f633 100644 --- 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 index 2fdc732..c7154c2 100644 --- 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,8 +4,8 @@ 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 @@ -13,7 +13,7 @@ import com.squareup.kotlinpoet.ClassName public val KSValueArgument.valueAsClassName: ClassName? get() = value ?.let { it as? KSType } - ?.declaration + ?.classDeclaration ?.toClassName() /** @@ -23,5 +23,5 @@ public val KSValueArgument.valueAsClassNameList: List? 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 index e890712..cbc0d9f 100644 --- 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 index 8a993e7..949ea06 100644 --- 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 index 1a6a363..ab0127f 100644 --- 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 index 8e3ef07..e1c9a49 100644 --- 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() } diff --git a/compiler/src/test/kotlin/com/r0adkll/kimchi/processors/ContributedBindingTest.kt b/compiler/src/test/kotlin/com/r0adkll/kimchi/processors/ContributedBindingTest.kt index 470bb70..10f9e12 100644 --- a/compiler/src/test/kotlin/com/r0adkll/kimchi/processors/ContributedBindingTest.kt +++ b/compiler/src/test/kotlin/com/r0adkll/kimchi/processors/ContributedBindingTest.kt @@ -343,4 +343,37 @@ class ContributedBindingTest { } } } + + @Test + fun `contributed binding with nested type compiles`() { + compileKimchiWithTestSources( + """ + package kimchi + + import me.tatarka.inject.annotations.Inject + import com.r0adkll.kimchi.annotations.ContributesBinding + + interface Parent { + interface Binding + } + + @ContributesBinding(TestScope::class) + @Inject + class RealBinding : Parent.Binding + """.trimIndent(), + workingDir = workingDir, + ) { + val binding = kotlinClass("kimchi.Parent\$Binding") + val realBinding = kotlinClass("kimchi.RealBinding") + + expectThat(mergedTestComponent) + .declaredProperties() + .withFirst { + hasReceiverOf(realBinding) + hasReturnTypeOf(binding) + getter() + .hasAnnotation(Provides::class) + } + } + } }