From 931ce4317c7370b7d4cbd19042d15464f61facb6 Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Thu, 29 Oct 2026 22:05:29 -0400 Subject: [PATCH] fix(analytics): default optional builder params to null The generated builders gave optional properties no default, so adding an optional property to an existing event broke every call site. Emit `= null` for nullable params, including the amount group's. Also order checkAnalyticsEventsPage after writeAnalyticsEventsPage so both can run in one invocation. --- .../src/main/kotlin/AnalyticsCatalogueConventionPlugin.kt | 4 +++- .../com/getcode/buildlogic/analytics/KotlinEmitter.kt | 5 ++++- .../getcode/buildlogic/analytics/AnalyticsCatalogueTest.kt | 6 +++--- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/build-logic/convention/src/main/kotlin/AnalyticsCatalogueConventionPlugin.kt b/build-logic/convention/src/main/kotlin/AnalyticsCatalogueConventionPlugin.kt index a7dec4ab57..57a85898bb 100644 --- a/build-logic/convention/src/main/kotlin/AnalyticsCatalogueConventionPlugin.kt +++ b/build-logic/convention/src/main/kotlin/AnalyticsCatalogueConventionPlugin.kt @@ -37,11 +37,13 @@ class AnalyticsCatalogueConventionPlugin : Plugin { val catalogueFile = layout.projectDirectory.file("events.toml") val pageFile = layout.projectDirectory.file("EVENTS.md") - tasks.register("writeAnalyticsEventsPage") { + val writePage = tasks.register("writeAnalyticsEventsPage") { catalogue.set(catalogueFile) page.set(pageFile) } tasks.register("checkAnalyticsEventsPage") { + // Both tasks touch EVENTS.md; without this, one invocation can check before it writes. + mustRunAfter(writePage) catalogue.set(catalogueFile) page.set(pageFile) fixCommand.set("./gradlew ${path.substringBeforeLast(':')}:writeAnalyticsEventsPage") diff --git a/build-logic/convention/src/main/kotlin/com/getcode/buildlogic/analytics/KotlinEmitter.kt b/build-logic/convention/src/main/kotlin/com/getcode/buildlogic/analytics/KotlinEmitter.kt index bff8f2814f..10f7f0d1b1 100644 --- a/build-logic/convention/src/main/kotlin/com/getcode/buildlogic/analytics/KotlinEmitter.kt +++ b/build-logic/convention/src/main/kotlin/com/getcode/buildlogic/analytics/KotlinEmitter.kt @@ -87,7 +87,10 @@ internal object KotlinEmitter { private fun builder(builder: Builder) = buildString { append(kdoc(listOfNotNull(builder.doc, builder.drift?.let { "DRIFT: $it" }), INDENT)) - val params = builder.params.joinToString { "${it.name}: ${it.kotlinType}" } + val params = builder.params.joinToString { param -> + // Optional means "omit when absent", so callers shouldn't have to pass null to say so. + "${param.name}: ${param.kotlinType}" + if (param.nullable) " = null" else "" + } val call = "event(${template(builder.event)})" if (builder.properties.isEmpty()) { appendLine("${INDENT}fun ${builder.name}($params): AnalyticsEvent = $call") diff --git a/build-logic/convention/src/test/kotlin/com/getcode/buildlogic/analytics/AnalyticsCatalogueTest.kt b/build-logic/convention/src/test/kotlin/com/getcode/buildlogic/analytics/AnalyticsCatalogueTest.kt index 02f2d83a5c..3c695e3f2b 100644 --- a/build-logic/convention/src/test/kotlin/com/getcode/buildlogic/analytics/AnalyticsCatalogueTest.kt +++ b/build-logic/convention/src/test/kotlin/com/getcode/buildlogic/analytics/AnalyticsCatalogueTest.kt @@ -93,7 +93,7 @@ class AnalyticsCatalogueTest { } @Test - fun optionalPropertiesPassNullThroughSoTheBuilderOmitsThem() { + fun optionalPropertiesDefaultToNullAndPassItThroughSoTheBuilderOmitsThem() { val source = generate( """ [lists.Kind.values] @@ -110,7 +110,7 @@ class AnalyticsCatalogueTest { """.trimIndent(), ).getValue("com/example/events/ThingEvents.kt") - assertContains(source, "fun thingHappened(kind: Kind?, error: String?, tier: Int?): AnalyticsEvent =") + assertContains(source, "fun thingHappened(kind: Kind? = null, error: String? = null, tier: Int? = null): AnalyticsEvent =") assertContains(source, "text(\"Kind\", kind?.value)") assertContains(source, "text(\"Error\", error)") assertContains(source, "number(\"Tier\", tier?.toDouble())") @@ -219,7 +219,7 @@ class AnalyticsCatalogueTest { assertContains( source, """ - fun grabbed(grabTimeMillis: Long?, state: State, amount: Amount?, error: String?): AnalyticsEvent = + fun grabbed(grabTimeMillis: Long? = null, state: State, amount: Amount? = null, error: String? = null): AnalyticsEvent = event("Grabbed") { number("Grab Time", grabTimeMillis) text("State", state.value)