Respect event type nullability

If an event is declared as non-nullable then it is required and non-null. If an event is declared as nullable then it is still required but can be set to null. And, of course, nullable event can have a default of null. This brings it directly in line with property behavior. I'm not sure why we had the weird nullability behavior from before.
cashapp · Apr 18, 2023 · ba0f5a3 · ba0f5a3
1 parent c56178a
commit ba0f5a3
Show file tree

Hide file tree

Showing 12 changed files with 46 additions and 27 deletions.
diff --git a/...et/src/commonTest/kotlin/app/cash/redwood/protocol/widget/DiffConsumingNodeFactoryTest.kt b/...et/src/commonTest/kotlin/app/cash/redwood/protocol/widget/DiffConsumingNodeFactoryTest.kt
@@ -342,14 +342,14 @@ class DiffConsumingNodeFactoryTest {
     var onChange: ((String) -> Unit)? = null
       private set
 
-    override fun onChange(onChange: ((String) -> Unit)?) {
+    override fun onChange(onChange: (String) -> Unit) {
       this.onChange = onChange
     }
 
     var onChangeCustomType: ((Duration) -> Unit)? = null
       private set
 
-    override fun onChangeCustomType(onChangeCustomType: ((Duration) -> Unit)?) {
+    override fun onChangeCustomType(onChangeCustomType: (Duration) -> Unit) {
       this.onChangeCustomType = onChangeCustomType
     }
 

diff --git a/...ood-tooling-codegen/src/main/kotlin/app/cash/redwood/tooling/codegen/composeGeneration.kt b/...ood-tooling-codegen/src/main/kotlin/app/cash/redwood/tooling/codegen/composeGeneration.kt
@@ -107,7 +107,9 @@ internal fun generateComposable(
                 }
                 is Event -> {
                   ParameterSpec.builder(trait.name, trait.lambdaType)
-                    .defaultValue(trait.defaultExpression ?: "null")
+                    .apply {
+                      trait.defaultExpression?.let { defaultValue(it) }
+                    }
                     .build()
                 }
                 is Children -> {

diff --git a/...ing-codegen/src/main/kotlin/app/cash/redwood/tooling/codegen/composeProtocolGeneration.kt b/...ing-codegen/src/main/kotlin/app/cash/redwood/tooling/codegen/composeProtocolGeneration.kt
@@ -350,7 +350,7 @@ internal fun generateProtocolWidget(
               }
               is ProtocolEvent -> {
                 addProperty(
-                  PropertySpec.builder(trait.name, trait.lambdaType, PRIVATE)
+                  PropertySpec.builder(trait.name, trait.lambdaType.copy(nullable = true), PRIVATE)
                     .mutable()
                     .initializer("null")
                     .build(),
@@ -359,16 +359,24 @@ internal fun generateProtocolWidget(
                   FunSpec.builder(trait.name)
                     .addModifiers(OVERRIDE)
                     .addParameter(trait.name, trait.lambdaType)
-                    .addStatement("val %1NSet = %1N != null", trait.name)
-                    .beginControlFlow("if (%1NSet != (this.%1N != null))", trait.name)
-                    .addStatement(
-                      "this.state.append(%T(this.id, %T(%L), %M(%NSet)))",
-                      Protocol.PropertyDiff,
-                      Protocol.PropertyTag,
-                      trait.tag,
-                      KotlinxSerialization.JsonPrimitive,
-                      trait.name,
-                    )
+                    .apply {
+                      val newValue = if (trait.isNullable) {
+                        addStatement("val %1NSet = %1N != null", trait.name)
+                        beginControlFlow("if (%1NSet != (this.%1N != null))", trait.name)
+                        trait.name + "Set"
+                      } else {
+                        beginControlFlow("if (this.%1N == null)", trait.name)
+                        "true"
+                      }
+                      addStatement(
+                        "this.state.append(%T(this.id, %T(%L), %M(%L)))",
+                        Protocol.PropertyDiff,
+                        Protocol.PropertyTag,
+                        trait.tag,
+                        KotlinxSerialization.JsonPrimitive,
+                        newValue,
+                      )
+                    }
                     .endControlFlow()
                     .addStatement("this.%1N = %1N", trait.name)
                     .build(),

diff --git a/...oling-codegen/src/main/kotlin/app/cash/redwood/tooling/codegen/diffConsumingGeneration.kt b/...oling-codegen/src/main/kotlin/app/cash/redwood/tooling/codegen/diffConsumingGeneration.kt
@@ -264,7 +264,11 @@ internal fun generateDiffConsumingWidget(
                         )
                       }
                       nextControlFlow("else")
-                      addStatement("null")
+                      if (trait.isNullable) {
+                        addStatement("null")
+                      } else {
+                        addStatement("throw %T()", Stdlib.AssertionError)
+                      }
                       endControlFlow()
                       addStatement("widget.%1N(%1N)", trait.name)
                       endControlFlow()

diff --git a/redwood-tooling-codegen/src/main/kotlin/app/cash/redwood/tooling/codegen/sharedHelpers.kt b/redwood-tooling-codegen/src/main/kotlin/app/cash/redwood/tooling/codegen/sharedHelpers.kt
@@ -51,14 +51,16 @@ internal val FqType.flatName: String
 
 internal val Event.lambdaType: TypeName
   get() {
-    parameterType?.let { parameterType ->
-      return LambdaTypeName.get(null, parameterType.asTypeName(), returnType = UNIT)
-        .copy(nullable = true)
-    }
-    return noArgumentEventLambda
+    val lambda = parameterType
+      ?.let { parameterType ->
+        LambdaTypeName.get(null, parameterType.asTypeName(), returnType = UNIT)
+      }
+      ?: noArgumentEventLambda
+
+    return lambda.copy(nullable = isNullable)
   }
 
-private val noArgumentEventLambda = LambdaTypeName.get(returnType = UNIT).copy(nullable = true)
+private val noArgumentEventLambda = LambdaTypeName.get(returnType = UNIT)
 
 internal fun Schema.composePackage(host: Schema? = null): String {
   return if (host == null) {

diff --git a/...tooling-codegen/src/test/kotlin/app/cash/redwood/tooling/codegen/ComposeGenerationTest.kt b/...tooling-codegen/src/test/kotlin/app/cash/redwood/tooling/codegen/ComposeGenerationTest.kt
@@ -90,7 +90,7 @@ class ComposeGenerationTest {
     val fileSpec = generateComposable(schema, schema.widgets.single())
     assertThat(fileSpec.toString()).apply {
       contains("trait: String = \"test\"")
-      contains("onEvent: (() -> Unit)? = { error(\"test\") }")
+      contains("onEvent: () -> Unit = { error(\"test\") }")
       contains("block: @Composable () -> Unit = {}")
     }
   }

diff --git a/...tooling-codegen/src/test/kotlin/app/cash/redwood/tooling/codegen/TestingGenerationTest.kt b/...tooling-codegen/src/test/kotlin/app/cash/redwood/tooling/codegen/TestingGenerationTest.kt
@@ -98,15 +98,15 @@ class TestingGenerationTest {
       |    this.trait = trait
       |  }
       |
-      |  public override fun onEvent(onEvent: (() -> Unit)?): Unit {
+      |  public override fun onEvent(onEvent: () -> Unit): Unit {
       |    this.onEvent = onEvent
       |  }
       |
       |  public override fun snapshot(): TestingGenerationTestBasicWidgetValue =
       |      TestingGenerationTestBasicWidgetValue(
       |    layoutModifiers = layoutModifiers,
       |    trait = trait!!,
-      |    onEvent = onEvent,
+      |    onEvent = onEvent!!,
       |    block = block.map { it.`value`.snapshot() },
       |  )
       |}
@@ -119,7 +119,7 @@ class TestingGenerationTest {
       |public class TestingGenerationTestBasicWidgetValue(
       |  public override val layoutModifiers: LayoutModifier = LayoutModifier,
       |  public val trait: String = "test",
-      |  public val onEvent: (() -> Unit)? = { error("test") },
+      |  public val onEvent: () -> Unit = { error("test") },
       |  public val block: List<WidgetValue> = listOf(),
       |) : WidgetValue {
       |  public override val childrenLists: List<List<WidgetValue>>

diff --git a/redwood-tooling-schema/src/main/kotlin/app/cash/redwood/tooling/schema/schema.kt b/redwood-tooling-schema/src/main/kotlin/app/cash/redwood/tooling/schema/schema.kt
@@ -84,6 +84,7 @@ public interface Widget {
 
   public interface Event : Trait {
     public val parameterType: FqType?
+    public val isNullable: Boolean
   }
 
   public interface Children : Trait {

diff --git a/redwood-tooling-schema/src/main/kotlin/app/cash/redwood/tooling/schema/schemaClasses.kt b/redwood-tooling-schema/src/main/kotlin/app/cash/redwood/tooling/schema/schemaClasses.kt
@@ -94,6 +94,7 @@ internal data class ParsedProtocolEvent(
   override val tag: Int,
   override val name: String,
   override val parameterType: FqType?,
+  override val isNullable: Boolean,
   override val defaultExpression: String? = null,
   override val deprecation: ParsedDeprecation? = null,
 ) : ProtocolEvent

diff --git a/redwood-tooling-schema/src/main/kotlin/app/cash/redwood/tooling/schema/schemaParser.kt b/redwood-tooling-schema/src/main/kotlin/app/cash/redwood/tooling/schema/schemaParser.kt
@@ -229,6 +229,7 @@ private fun parseWidget(
             tag = property.tag,
             name = it.name!!,
             parameterType = arguments.singleOrNull()?.type?.toFqType(),
+            isNullable = it.type.isMarkedNullable,
             defaultExpression = defaultExpression,
             deprecation = deprecation,
           )

diff --git a/samples/emoji-search/schema/src/main/kotlin/com/example/redwood/emojisearch/schema.kt b/samples/emoji-search/schema/src/main/kotlin/com/example/redwood/emojisearch/schema.kt
@@ -47,7 +47,7 @@ data class TextInput(
   val hint: String,
   @Property(3)
   @Default("null")
-  val onChange: (TextFieldState) -> Unit,
+  val onChange: ((TextFieldState) -> Unit)?,
 )
 
 @Widget(2)

diff --git a/test-schema/src/main/kotlin/example/redwood/schema.kt b/test-schema/src/main/kotlin/example/redwood/schema.kt
@@ -66,7 +66,7 @@ public data class Text(
 @Widget(4)
 public data class Button(
   @Property(1) val text: String?,
-  @Property(2) val onClick: () -> Unit,
+  @Property(2) val onClick: (() -> Unit)?,
 )
 
 @Widget(5)