Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce compose.ui dependency and add Modifier to Ui.Content() #422

Merged
merged 17 commits into from
Feb 2, 2023
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ com.squareup.anvil:annotations
javax.inject:javax.inject
org.jetbrains.compose.runtime:runtime-saveable
org.jetbrains.compose.runtime:runtime
org.jetbrains.compose.ui:ui
org.jetbrains.kotlin:kotlin-bom
org.jetbrains.kotlin:kotlin-stdlib-jdk7
org.jetbrains.kotlin:kotlin-stdlib-jdk8
Expand Down
22 changes: 22 additions & 0 deletions circuit-codegen-annotations/dependencies/jvmRuntimeClasspath.txt
Original file line number Diff line number Diff line change
@@ -1,10 +1,30 @@
com.google.dagger:dagger
com.squareup.anvil:annotations
javax.inject:javax.inject
org.jetbrains.compose.animation:animation-core-desktop
org.jetbrains.compose.animation:animation-core
org.jetbrains.compose.animation:animation-desktop
org.jetbrains.compose.animation:animation
org.jetbrains.compose.foundation:foundation-desktop
org.jetbrains.compose.foundation:foundation-layout-desktop
org.jetbrains.compose.foundation:foundation-layout
org.jetbrains.compose.foundation:foundation
org.jetbrains.compose.runtime:runtime-desktop
org.jetbrains.compose.runtime:runtime-saveable-desktop
org.jetbrains.compose.runtime:runtime-saveable
org.jetbrains.compose.runtime:runtime
org.jetbrains.compose.ui:ui-desktop
org.jetbrains.compose.ui:ui-geometry-desktop
org.jetbrains.compose.ui:ui-geometry
org.jetbrains.compose.ui:ui-graphics-desktop
org.jetbrains.compose.ui:ui-graphics
org.jetbrains.compose.ui:ui-text-desktop
org.jetbrains.compose.ui:ui-text
org.jetbrains.compose.ui:ui-unit-desktop
org.jetbrains.compose.ui:ui-unit
org.jetbrains.compose.ui:ui-util-desktop
org.jetbrains.compose.ui:ui-util
org.jetbrains.compose.ui:ui
org.jetbrains.kotlin:kotlin-bom
org.jetbrains.kotlin:kotlin-stdlib-jdk7
org.jetbrains.kotlin:kotlin-stdlib-jdk8
Expand All @@ -14,4 +34,6 @@ org.jetbrains.kotlinx:atomicfu
org.jetbrains.kotlinx:kotlinx-coroutines-bom
org.jetbrains.kotlinx:kotlinx-coroutines-core-jvm
org.jetbrains.kotlinx:kotlinx-coroutines-core
org.jetbrains.skiko:skiko-awt
org.jetbrains.skiko:skiko
org.jetbrains:annotations
2 changes: 0 additions & 2 deletions circuit-codegen/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,5 @@ dependencies {
implementation(libs.dagger)
implementation(libs.kotlinpoet)
implementation(libs.kotlinpoet.ksp)
implementation(projects.circuit)
implementation(projects.circuitCodegenAnnotations)
implementation(libs.anvil.annotations)
}
9 changes: 0 additions & 9 deletions circuit-codegen/dependencies/runtimeClasspath.txt
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,10 @@ com.squareup.anvil:annotations
com.squareup:kotlinpoet-ksp
com.squareup:kotlinpoet
javax.inject:javax.inject
org.jetbrains.compose.runtime:runtime-desktop
org.jetbrains.compose.runtime:runtime-saveable-desktop
org.jetbrains.compose.runtime:runtime-saveable
org.jetbrains.compose.runtime:runtime
org.jetbrains.kotlin:kotlin-bom
org.jetbrains.kotlin:kotlin-reflect
org.jetbrains.kotlin:kotlin-stdlib-common
org.jetbrains.kotlin:kotlin-stdlib-jdk7
org.jetbrains.kotlin:kotlin-stdlib-jdk8
org.jetbrains.kotlin:kotlin-stdlib
org.jetbrains.kotlinx:atomicfu-jvm
org.jetbrains.kotlinx:atomicfu
org.jetbrains.kotlinx:kotlinx-coroutines-bom
org.jetbrains.kotlinx:kotlinx-coroutines-core-jvm
org.jetbrains.kotlinx:kotlinx-coroutines-core
org.jetbrains:annotations
1 change: 1 addition & 0 deletions circuit-codegen/gradle.properties
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
POM_ARTIFACT_ID=circuit-codegen
POM_NAME=Circuit (Codegen)
POM_DESCRIPTION=Circuit (Codegen)
circuit.noCompose=true
Original file line number Diff line number Diff line change
Expand Up @@ -23,16 +23,9 @@ import com.google.devtools.ksp.symbol.KSDeclaration
import com.google.devtools.ksp.symbol.KSFunctionDeclaration
import com.google.devtools.ksp.symbol.KSType
import com.google.devtools.ksp.symbol.Visibility
import com.slack.circuit.CircuitContext
import com.slack.circuit.CircuitUiState
import com.slack.circuit.Navigator
import com.slack.circuit.Presenter
import com.slack.circuit.Screen
import com.slack.circuit.ScreenUi
import com.slack.circuit.Ui
import com.slack.circuit.codegen.annotations.CircuitInject
import com.squareup.anvil.annotations.ContributesMultibinding
import com.squareup.kotlinpoet.AnnotationSpec
import com.squareup.kotlinpoet.ClassName
import com.squareup.kotlinpoet.CodeBlock
import com.squareup.kotlinpoet.FileSpec
import com.squareup.kotlinpoet.FunSpec
Expand All @@ -45,7 +38,6 @@ import com.squareup.kotlinpoet.STAR
import com.squareup.kotlinpoet.TypeName
import com.squareup.kotlinpoet.TypeSpec
import com.squareup.kotlinpoet.asClassName
import com.squareup.kotlinpoet.asTypeName
import com.squareup.kotlinpoet.joinToCode
import com.squareup.kotlinpoet.ksp.addOriginatingKSFile
import com.squareup.kotlinpoet.ksp.toClassName
Expand All @@ -55,9 +47,17 @@ import dagger.assisted.AssistedFactory
import javax.inject.Inject
import javax.inject.Provider

private val CIRCUIT_INJECT_ANNOTATION = CircuitInject::class.java.canonicalName
private val CIRCUIT_PRESENTER = Presenter::class.java.canonicalName
private val CIRCUIT_UI = Ui::class.java.canonicalName
private val MODIFIER = ClassName("androidx.compose.ui", "Modifier")
private val CIRCUIT_INJECT_ANNOTATION =
ClassName("com.slack.circuit.codegen.annotations", "CircuitInject")
private val CIRCUIT_PRESENTER = ClassName("com.slack.circuit", "Presenter")
ZacSweers marked this conversation as resolved.
Show resolved Hide resolved
private val CIRCUIT_PRESENTER_FACTORY = CIRCUIT_PRESENTER.nestedClass("Factory")
private val CIRCUIT_UI = ClassName("com.slack.circuit", "Ui")
private val CIRCUIT_UI_FACTORY = CIRCUIT_UI.nestedClass("Factory")
private val CIRCUIT_UI_STATE = ClassName("com.slack.circuit", "CircuitUiState")
private val SCREEN = ClassName("com.slack.circuit", "Screen")
private val NAVIGATOR = ClassName("com.slack.circuit", "Navigator")
private val CIRCUIT_CONTEXT = ClassName("com.slack.circuit", "CircuitContext")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: the IDE is complaining that all of these either don't match a regex or contain an "_" in the name. FACTORY is ok which I assume is because it's a const. Can we suppress or rename these to address the warnings?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the warning? Mine is fine with it

image

private const val FACTORY = "Factory"

@AutoService(SymbolProcessorProvider::class)
Expand All @@ -68,9 +68,11 @@ public class CircuitSymbolProcessorProvider : SymbolProcessorProvider {
}

private class CircuitSymbols private constructor(resolver: Resolver) {
val circuitUiState = resolver.loadKSType<CircuitUiState>()
val screen = resolver.loadKSType<Screen>()
val navigator = resolver.loadKSType<Navigator>()
val modifier = resolver.loadKSType(MODIFIER.canonicalName)
val circuitUiState = resolver.loadKSType(CIRCUIT_UI_STATE.canonicalName)
val screen = resolver.loadKSType(SCREEN.canonicalName)
val navigator = resolver.loadKSType(NAVIGATOR.canonicalName)

companion object {
fun create(resolver: Resolver): CircuitSymbols? {
@Suppress("SwallowedException")
Expand All @@ -83,24 +85,23 @@ private class CircuitSymbols private constructor(resolver: Resolver) {
}
}

private inline fun <reified T> Resolver.loadKSType(): KSType {
return loadOptionalKSType<T>()
?: error("Could not find ${T::class.java.canonicalName} in classpath")
}
private fun Resolver.loadKSType(name: String?): KSType =
ZacSweers marked this conversation as resolved.
Show resolved Hide resolved
loadOptionalKSType(name) ?: error("Could not find $name in classpath")

private inline fun <reified T> Resolver.loadOptionalKSType(): KSType? {
return getClassDeclarationByName(getKSNameFromString(T::class.java.canonicalName))
?.asType(emptyList())
internal fun Resolver.loadOptionalKSType(name: String?): KSType? {
ZacSweers marked this conversation as resolved.
Show resolved Hide resolved
if (name == null) return null
return getClassDeclarationByName(getKSNameFromString(name))?.asType(emptyList())
}

private class CircuitSymbolProcessor(
private val logger: KSPLogger,
private val codeGenerator: CodeGenerator
private val codeGenerator: CodeGenerator,
) : SymbolProcessor {

override fun process(resolver: Resolver): List<KSAnnotated> {
val symbols = CircuitSymbols.create(resolver) ?: return emptyList()
resolver.getSymbolsWithAnnotation(CIRCUIT_INJECT_ANNOTATION).forEach { annotatedElement ->
resolver.getSymbolsWithAnnotation(CIRCUIT_INJECT_ANNOTATION.canonicalName).forEach {
annotatedElement ->
when (annotatedElement) {
is KSClassDeclaration -> {
generateFactory(annotatedElement, InstantiationType.CLASS, symbols)
Expand All @@ -121,12 +122,12 @@ private class CircuitSymbolProcessor(
private fun generateFactory(
annotatedElement: KSAnnotated,
instantiationType: InstantiationType,
symbols: CircuitSymbols
symbols: CircuitSymbols,
) {
val circuitInjectAnnotation =
annotatedElement.annotations.first {
it.annotationType.resolve().declaration.qualifiedName?.asString() ==
CIRCUIT_INJECT_ANNOTATION
CIRCUIT_INJECT_ANNOTATION.canonicalName
}
val screenKSType = circuitInjectAnnotation.arguments[0].value as KSType
val screenIsObject =
Expand Down Expand Up @@ -185,12 +186,12 @@ private class CircuitSymbolProcessor(
val packageName: String,
val factoryType: FactoryType,
val constructorParams: List<ParameterSpec>,
val codeBlock: CodeBlock
val codeBlock: CodeBlock,
)

/** Computes the data needed to generate a factory. */
// Detekt and ktfmt don't agree on whether or not the rectangle rule makes for readable code.
@Suppress("ComplexMethod", "LongMethod")
@Suppress("ComplexMethod", "LongMethod", "ReturnCount")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking ahead: I think we should refactor this method (and maybe others in this file). It's pretty long and unwieldy right now :-\

@OptIn(KspExperimental::class)
private fun computeFactoryData(
annotatedElement: KSAnnotated,
Expand Down Expand Up @@ -232,34 +233,72 @@ private class CircuitSymbolProcessor(
assistedParams
)
FactoryType.UI -> {
// State param is optional
val stateParam =
fd.parameters.singleOrNull { parameter ->
symbols.circuitUiState.isAssignableFrom(parameter.type.resolve())
}
if (stateParam == null) {
CodeBlock.of(
"%M<%T>·{·%M(%L)·}",
MemberName("com.slack.circuit", "ui"),
CircuitUiState::class.java,
MemberName(packageName, name),
assistedParams
)
} else {
val block =
if (assistedParams.isEmpty()) {
CodeBlock.of("")
} else {
CodeBlock.of(",·%L", assistedParams)

// Modifier param is required
val modifierParam =
fd.parameters.singleOrNull { parameter ->
symbols.modifier.isAssignableFrom(parameter.type.resolve())
}
?: run {
logger.error("UI composable functions must have a Modifier parameter!", fd)
return null
}
CodeBlock.of(
"%M<%T>·{·state·->·%M(%L·=·state%L)·}",
MemberName("com.slack.circuit", "ui"),
stateParam.type.resolve().toTypeName(),
MemberName(packageName, name),
stateParam.name!!.getShortName(),
block
)
}

/*
Diagram of what goes into generating a function!
- State parameter is _optional_ and can be omitted if it's static state.
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note for the future: this is matching the previous behavior, but also... does this kind of situation even need to use circuit vs just being a regular composable function? I think it only matters if you have a presenter with no state but can receive UI events, which we currently don't support doing without a state.

- When omitted, the argument becomes _ and the param is omitted entirely.
- <StateType> is either the State or CircuitUiState if no state param is used.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thought: I could see someone reading the generated code and getting confused by a param of type CircuitUiState. I'm wondering if it might make sense to create object NoState : CircuitUiState and use it here instead.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unless we promote NoState to be a public and reusable type, I don't think it makes much of a difference. I have wondered if theres room for a class StaticState<UiEvent : CircuitUiEvent>(val eventSink: (UiEvent) -> Unit) that could be useful

- Modifier parameter is required.
- Assisted parameters can be 0 or more extra supported assisted parameters.

Optional state param
Optional state arg │
│ │ Required modifier param
│ Req modifier arg │ │
┌─── ui function │ │ │ │ Any assisted params
│ │ │ Composable │ │ │
│ State type │ │ │ │ │ │
│ │ │ │ │ │ │ │
│ │ │ │ │ │ │ │
└──────┴───── ──┴── ───┴──── ──┴───── ───────┴───── ────────┴────────── ────────┴────────
ui<StateType> { state, modifier -> Function(state = state, modifier = modifier, <assisted params>) }
────────────────────────────────────────────────────────────────────────────────────────────────────

Diagram generated with asciiflow. You can make new ones or edit with this link.
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

open to good url shorteners that aren't ephemeral!

https://asciiflow.com/#/share/eJzVVM1KxDAQfpVhTgr1IizLlt2CCF4F9ZhLdGclkKbd%2FMCW0rfwcXwan8SsWW27sd0qXixzmDTffN83bSY1Kp4TpspJmaDkFWlMsWa4Y5guZvOEYeWzy%2FnCZ5Z21i8Ywg%2Be29KKQnEJxnJLUHLNc8bUKIjr55jo7eXVR1R62Dplo4Xc0dYJTWvIi7XYCNIDnnpVvqjF9%2BzF2sFlsBvCCdg49bTvsVcx4vtb2u7ySlXAjRHG%2BlY%2BOjBBdYSqza6LvCwMf5Q0VS%2FqDjq%2FzVYlDfV1zPMrpWHSf6w1JeDz4HfejGCH9ybrTQT%2BUan%2FEk4s7%2Fdj%2F%2BAPUQZ1uAOSdtwuMrg5TM9ZuB9WEWb1lSawPBqL7Bwahg027%2Byjz8s%3D)
*/

@Suppress("IfThenToElvis") // The elvis is less readable here
val stateType =
if (stateParam == null) CIRCUIT_UI_STATE else stateParam.type.resolve().toTypeName()
val stateArg = if (stateParam == null) "_" else "state"
val stateParamBlock =
if (stateParam == null) CodeBlock.of("")
else CodeBlock.of("%L·=·state,·", stateParam.name!!.getShortName())
val modifierParamBlock =
CodeBlock.of("%L·=·modifier", modifierParam.name!!.getShortName())
val assistedParamsBlock =
if (assistedParams.isEmpty()) {
CodeBlock.of("")
} else {
CodeBlock.of(",·%L", assistedParams)
}
CodeBlock.of(
"%M<%T>·{·%L,·modifier·->·%M(%L%L%L)·}",
MemberName("com.slack.circuit", "ui"),
stateType,
stateArg,
MemberName(packageName, name),
stateParamBlock,
modifierParamBlock,
assistedParamsBlock
)
}
}
}
Expand Down Expand Up @@ -291,8 +330,8 @@ private class CircuitSymbolProcessor(
.getAllSuperTypes()
.mapNotNull {
when (it.declaration.qualifiedName?.asString()) {
CIRCUIT_UI -> FactoryType.UI
CIRCUIT_PRESENTER -> FactoryType.PRESENTER
CIRCUIT_UI.canonicalName -> FactoryType.UI
CIRCUIT_PRESENTER.canonicalName -> FactoryType.PRESENTER
else -> null
}
}
Expand Down Expand Up @@ -361,7 +400,7 @@ private fun KSFunctionDeclaration.assistedParameters(
symbols: CircuitSymbols,
logger: KSPLogger,
screenType: KSType,
allowNavigator: Boolean
allowNavigator: Boolean,
): CodeBlock {
return buildSet {
for (param in parameters) {
Expand Down Expand Up @@ -410,22 +449,17 @@ private fun KSType.isInstanceOf(type: KSType): Boolean {
private fun TypeSpec.Builder.buildUiFactory(
originatingSymbol: KSAnnotated,
screenBranch: CodeBlock,
instantiationCodeBlock: CodeBlock
instantiationCodeBlock: CodeBlock,
): TypeSpec {
return addSuperinterface(Ui.Factory::class)
return addSuperinterface(CIRCUIT_UI_FACTORY)
.addFunction(
FunSpec.builder("create")
.addModifiers(KModifier.OVERRIDE)
.addParameter("screen", Screen::class)
.addParameter("context", CircuitContext::class)
.returns(ScreenUi::class.asClassName().copy(nullable = true))
.addParameter("screen", SCREEN)
.addParameter("context", CIRCUIT_CONTEXT)
.returns(CIRCUIT_UI.parameterizedBy(STAR).copy(nullable = true))
.beginControlFlow("return·when·(screen)")
.addStatement(
"%L·->·%T(%L)",
screenBranch,
ScreenUi::class.asTypeName(),
instantiationCodeBlock
)
.addStatement("%L·->·%L", screenBranch, instantiationCodeBlock)
.addStatement("else·->·null")
.endControlFlow()
.build()
Expand All @@ -437,7 +471,7 @@ private fun TypeSpec.Builder.buildUiFactory(
private fun TypeSpec.Builder.buildPresenterFactory(
originatingSymbol: KSAnnotated,
screenBranch: CodeBlock,
instantiationCodeBlock: CodeBlock
instantiationCodeBlock: CodeBlock,
): TypeSpec {
// The TypeSpec below will generate something similar to the following.
// public class AboutPresenterFactory : Presenter.Factory {
Expand All @@ -452,14 +486,14 @@ private fun TypeSpec.Builder.buildPresenterFactory(
// }
// }

return addSuperinterface(Presenter.Factory::class)
return addSuperinterface(CIRCUIT_PRESENTER_FACTORY)
.addFunction(
FunSpec.builder("create")
.addModifiers(KModifier.OVERRIDE)
.addParameter("screen", Screen::class)
.addParameter("navigator", Navigator::class)
.addParameter("context", CircuitContext::class)
.returns(Presenter::class.asClassName().parameterizedBy(STAR).copy(nullable = true))
.addParameter("screen", SCREEN)
.addParameter("navigator", NAVIGATOR)
.addParameter("context", CIRCUIT_CONTEXT)
.returns(CIRCUIT_PRESENTER.parameterizedBy(STAR).copy(nullable = true))
.beginControlFlow("return when (screen)")
.addStatement("%L·->·%L", screenBranch, instantiationCodeBlock)
.addStatement("else·->·null")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ app.cash.turbine:turbine
com.google.guava:listenablefuture
org.jetbrains.compose.runtime:runtime-saveable
org.jetbrains.compose.runtime:runtime
org.jetbrains.compose.ui:ui
org.jetbrains.kotlin:kotlin-bom
org.jetbrains.kotlin:kotlin-stdlib-jdk7
org.jetbrains.kotlin:kotlin-stdlib-jdk8
Expand Down
Loading