From da8673ed5f218825e60e46644a71496517dfa22e Mon Sep 17 00:00:00 2001 From: Scott Olcott Date: Mon, 28 Sep 2026 15:14:36 -0600 Subject: [PATCH 1/5] Mark model and screen states as Compose @Immutable and @Stable Adds the `androidx.compose.runtime:runtime-annotation` dependency to the `model` and `presenter` modules to explicitly mark UI-bound data classes for Compose: - Annotates `Continent`, `Country`, `CountryDetail`, and `Language` models with `@Immutable`. - Annotates `CountryDetailScreen.State` and `CountryListScreen.State` with `@Immutable`. - Annotates `SearchAndFilterScreen.State` with `@Stable`. Also includes minor cleanup, such as removing a redundant type argument in `SearchAndFilterPresenter` and suppressing a version inspection in `libs.versions.toml`. --- gradle/libs.versions.toml | 2 ++ model/build.gradle.kts | 4 ++++ .../kotlin/io/github/solcott/countries/model/Continent.kt | 3 ++- .../kotlin/io/github/solcott/countries/model/Country.kt | 6 +++++- presenter/build.gradle.kts | 1 + .../solcott/countries/presenter/CountryDetailScreen.kt | 2 ++ .../github/solcott/countries/presenter/CountryListScreen.kt | 2 ++ .../solcott/countries/presenter/SearchAndFilterPresenter.kt | 2 +- .../solcott/countries/presenter/SearchAndFilterScreen.kt | 2 ++ 9 files changed, 21 insertions(+), 3 deletions(-) diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 42df5ab..050f2fb 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -7,6 +7,7 @@ metro = "1.4.5" apollo = "5.2.0" apollo-normalized-cache = "1.0.9" # What normalized-cache-sqlite depends on — the web driver and the npm worker must agree with it. +#noinspection NewerVersionAvailable sqldelight = "2.1.0" ktfmt = "0.27.0" @@ -122,6 +123,7 @@ detekt-compose-rules = { module = "io.nlopez.compose.rules:detekt", version = "0 # Compose Multiplatform. `runtime` is a thin alias onto androidx.compose.runtime, which is already # multiplatform; `foundation` is the real multiplatform build, since androidx's is Android-only. compose-runtime = { module = "org.jetbrains.compose.runtime:runtime", version.ref = "composeMultiplatform" } +compose-runtime-annotations = { module = "androidx.compose.runtime:runtime-annotation", version.ref = "composeUi" } compose-runtime-saveable = { module = "org.jetbrains.compose.runtime:runtime-saveable", version.ref = "composeMultiplatform" } compose-foundation = { module = "org.jetbrains.compose.foundation:foundation", version.ref = "composeMultiplatform" } compose-ui = { module = "org.jetbrains.compose.ui:ui", version.ref = "composeMultiplatform" } diff --git a/model/build.gradle.kts b/model/build.gradle.kts index e6741ea..1766a9c 100644 --- a/model/build.gradle.kts +++ b/model/build.gradle.kts @@ -2,3 +2,7 @@ plugins { id("kmp-library") alias(libs.plugins.kmp.parcelize) } + +kotlin { + sourceSets { commonMain { dependencies { implementation(libs.compose.runtime.annotations) } } } +} diff --git a/model/src/commonMain/kotlin/io/github/solcott/countries/model/Continent.kt b/model/src/commonMain/kotlin/io/github/solcott/countries/model/Continent.kt index 781bd4c..b6f86be 100644 --- a/model/src/commonMain/kotlin/io/github/solcott/countries/model/Continent.kt +++ b/model/src/commonMain/kotlin/io/github/solcott/countries/model/Continent.kt @@ -1,6 +1,7 @@ package io.github.solcott.countries.model +import androidx.compose.runtime.Immutable import io.github.solcott.kmp.parcelize.Parcelable import io.github.solcott.kmp.parcelize.Parcelize -@Parcelize data class Continent(val code: String, val name: String) : Parcelable +@Immutable @Parcelize data class Continent(val code: String, val name: String) : Parcelable diff --git a/model/src/commonMain/kotlin/io/github/solcott/countries/model/Country.kt b/model/src/commonMain/kotlin/io/github/solcott/countries/model/Country.kt index f6a1bfb..a53078f 100644 --- a/model/src/commonMain/kotlin/io/github/solcott/countries/model/Country.kt +++ b/model/src/commonMain/kotlin/io/github/solcott/countries/model/Country.kt @@ -1,6 +1,9 @@ package io.github.solcott.countries.model +import androidx.compose.runtime.Immutable + /** Summary of a country, as shown in the list screen. */ +@Immutable data class Country( val code: String, val name: String, @@ -10,6 +13,7 @@ data class Country( ) /** Full detail for a single country, as shown in the detail screen. */ +@Immutable data class CountryDetail( val code: String, val name: String, @@ -22,4 +26,4 @@ data class CountryDetail( val languages: List, ) -data class Language(val code: String, val name: String) +@Immutable data class Language(val code: String, val name: String) diff --git a/presenter/build.gradle.kts b/presenter/build.gradle.kts index f20fe65..aa74801 100644 --- a/presenter/build.gradle.kts +++ b/presenter/build.gradle.kts @@ -58,6 +58,7 @@ kotlin { // alias onto it. foundation is not, hence the Compose Multiplatform build — it is what // provides TextFieldState. implementation(libs.compose.runtime) + implementation(libs.compose.runtime.annotations) implementation(libs.compose.runtime.saveable) implementation(libs.compose.foundation) implementation(libs.androidx.compose.runtime.retain) diff --git a/presenter/src/commonMain/kotlin/io/github/solcott/countries/presenter/CountryDetailScreen.kt b/presenter/src/commonMain/kotlin/io/github/solcott/countries/presenter/CountryDetailScreen.kt index 40508d1..b790652 100644 --- a/presenter/src/commonMain/kotlin/io/github/solcott/countries/presenter/CountryDetailScreen.kt +++ b/presenter/src/commonMain/kotlin/io/github/solcott/countries/presenter/CountryDetailScreen.kt @@ -1,5 +1,6 @@ package io.github.solcott.countries.presenter +import androidx.compose.runtime.Immutable import com.slack.circuit.runtime.CircuitUiEvent import com.slack.circuit.runtime.CircuitUiState import com.slack.circuit.runtime.screen.Screen @@ -13,6 +14,7 @@ import io.github.solcott.uistate.LoadStatus @CircuitSerializable(AppScope::class) data class CountryDetailScreen(val code: String) : Screen { + @Immutable data class State( val content: ContentState = ContentState(data = null), @Redacted val eventSink: (Event) -> Unit, diff --git a/presenter/src/commonMain/kotlin/io/github/solcott/countries/presenter/CountryListScreen.kt b/presenter/src/commonMain/kotlin/io/github/solcott/countries/presenter/CountryListScreen.kt index 09ab659..ecb0a33 100644 --- a/presenter/src/commonMain/kotlin/io/github/solcott/countries/presenter/CountryListScreen.kt +++ b/presenter/src/commonMain/kotlin/io/github/solcott/countries/presenter/CountryListScreen.kt @@ -1,5 +1,6 @@ package io.github.solcott.countries.presenter +import androidx.compose.runtime.Immutable import com.slack.circuit.runtime.CircuitUiEvent import com.slack.circuit.runtime.CircuitUiState import com.slack.circuit.runtime.screen.Screen @@ -13,6 +14,7 @@ import io.github.solcott.uistate.ContentState @CircuitSerializable(AppScope::class) data object CountryListScreen : Screen { + @Immutable data class State( val countriesState: ContentState>, /** diff --git a/presenter/src/commonMain/kotlin/io/github/solcott/countries/presenter/SearchAndFilterPresenter.kt b/presenter/src/commonMain/kotlin/io/github/solcott/countries/presenter/SearchAndFilterPresenter.kt index 88fa165..796fa74 100644 --- a/presenter/src/commonMain/kotlin/io/github/solcott/countries/presenter/SearchAndFilterPresenter.kt +++ b/presenter/src/commonMain/kotlin/io/github/solcott/countries/presenter/SearchAndFilterPresenter.kt @@ -46,7 +46,7 @@ class SearchAndFilterPresenter(private val continentRepository: ContinentReposit // error instead of the list — so this presenter is not composed at all. Retrying puts the list // on screen, which composes the header for the first time and runs this fresh. val continentsState = - produceRetainedContentState(initial = emptyList()) { + produceRetainedContentState(initial = emptyList()) { continentRepository.continentsAsFlow().distinctUntilChanged() } diff --git a/presenter/src/commonMain/kotlin/io/github/solcott/countries/presenter/SearchAndFilterScreen.kt b/presenter/src/commonMain/kotlin/io/github/solcott/countries/presenter/SearchAndFilterScreen.kt index a7e5810..ff5603b 100644 --- a/presenter/src/commonMain/kotlin/io/github/solcott/countries/presenter/SearchAndFilterScreen.kt +++ b/presenter/src/commonMain/kotlin/io/github/solcott/countries/presenter/SearchAndFilterScreen.kt @@ -1,6 +1,7 @@ package io.github.solcott.countries.presenter import androidx.compose.foundation.text.input.TextFieldState +import androidx.compose.runtime.Stable import com.slack.circuit.subcircuit.SubCircuitOuterEvent import com.slack.circuit.subcircuit.SubCircuitUiEvent import com.slack.circuit.subcircuit.SubCircuitUiState @@ -28,6 +29,7 @@ import io.github.solcott.uistate.ContentState */ data object SearchAndFilterScreen : SubScreen { + @Stable data class State( val nameStartsWithText: TextFieldState, val continentsState: ContentState>, From e4b221960a9e4d943388206cbe3d4dd5b70b3268 Mon Sep 17 00:00:00 2001 From: Scott Olcott Date: Mon, 28 Sep 2026 15:15:01 -0600 Subject: [PATCH 2/5] Remove Countries Xcode scheme Deletes the shared Xcode scheme `Countries.xcscheme` from `iosApp/Countries.xcodeproj/xcshareddata/xcschemes/`. --- .../xcshareddata/xcschemes/Countries.xcscheme | 98 ------------------- 1 file changed, 98 deletions(-) delete mode 100644 iosApp/Countries.xcodeproj/xcshareddata/xcschemes/Countries.xcscheme diff --git a/iosApp/Countries.xcodeproj/xcshareddata/xcschemes/Countries.xcscheme b/iosApp/Countries.xcodeproj/xcshareddata/xcschemes/Countries.xcscheme deleted file mode 100644 index 1649bca..0000000 --- a/iosApp/Countries.xcodeproj/xcshareddata/xcschemes/Countries.xcscheme +++ /dev/null @@ -1,98 +0,0 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - From 13b8f3bdfd91fe0d4e61149e414237c7cca7c3bf Mon Sep 17 00:00:00 2001 From: Scott Olcott Date: Mon, 28 Sep 2026 15:24:44 -0600 Subject: [PATCH 3/5] Read countriesState from the @Immutable screen state in CountriesList Passing ContentState> as its own parameter gave CountriesList runtime stability: ContentState is a generic library type and List is an interface. The value was already on CountryListScreen.State, which is @Immutable, so read it from there instead of whitelisting List. Co-Authored-By: Claude Opus 5.5 --- .../github/solcott/countries/ui/CountryListUi.kt | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/ui/src/commonMain/kotlin/io/github/solcott/countries/ui/CountryListUi.kt b/ui/src/commonMain/kotlin/io/github/solcott/countries/ui/CountryListUi.kt index 8502d65..2759041 100644 --- a/ui/src/commonMain/kotlin/io/github/solcott/countries/ui/CountryListUi.kt +++ b/ui/src/commonMain/kotlin/io/github/solcott/countries/ui/CountryListUi.kt @@ -73,7 +73,7 @@ fun CountryListUi(state: CountryListScreen.State, modifier: Modifier = Modifier) message = error.toUserMessage(), onRetry = { state.eventSink(CountryListScreen.Event.Retry) }, ) - else -> CountriesList(state, countriesState, Modifier.fillMaxSize()) + else -> CountriesList(state, Modifier.fillMaxSize()) } } } @@ -91,11 +91,8 @@ internal fun listPaneColor() = else MaterialTheme.colorScheme.surface @Composable -private fun CountriesList( - state: CountryListScreen.State, - countriesState: ContentState>, - modifier: Modifier = Modifier, -) { +private fun CountriesList(state: CountryListScreen.State, modifier: Modifier = Modifier) { + val countriesState = state.countriesState val skin = LocalAppSkin.current LazyColumn(modifier = modifier.fillMaxSize().imePadding()) { stickyHeader { @@ -323,8 +320,7 @@ private fun CountriesListPreview() { countriesState = loadedState(previewCountries), selectedCountryCode = previewCountries.first().code, eventSink = {}, - ), - countriesState = loadedState(previewCountries), + ) ) } } @@ -375,8 +371,7 @@ private fun CountriesListDesktopSkinPreview() { countriesState = loadedState(previewCountries), selectedCountryCode = previewCountries.first().code, eventSink = {}, - ), - countriesState = loadedState(previewCountries), + ) ) } } From 03e8435ea81bdd236b55578fa4fe3be3bb43c300 Mon Sep 17 00:00:00 2001 From: Scott Olcott Date: Mon, 28 Sep 2026 16:00:43 -0600 Subject: [PATCH 4/5] Add `@ReadOnlyComposable` to `listPaneColor` and remove unused opt-in - Annotate `listPaneColor()` in `CountryListUi.kt` with `@ReadOnlyComposable` to optimize its execution, as it only reads composition locals. - Remove the unnecessary `@OptIn(ExperimentalMaterial3AdaptiveApi::class)` from `countriesPaneDirective` in `ListDetailNavDecoration.kt`. --- .../kotlin/io/github/solcott/countries/ui/CountryListUi.kt | 2 ++ .../io/github/solcott/countries/ui/ListDetailNavDecoration.kt | 1 - 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/ui/src/commonMain/kotlin/io/github/solcott/countries/ui/CountryListUi.kt b/ui/src/commonMain/kotlin/io/github/solcott/countries/ui/CountryListUi.kt index 2759041..69d47dc 100644 --- a/ui/src/commonMain/kotlin/io/github/solcott/countries/ui/CountryListUi.kt +++ b/ui/src/commonMain/kotlin/io/github/solcott/countries/ui/CountryListUi.kt @@ -24,6 +24,7 @@ import androidx.compose.material3.MaterialTheme import androidx.compose.material3.Text import androidx.compose.runtime.Composable import androidx.compose.runtime.CompositionLocalProvider +import androidx.compose.runtime.ReadOnlyComposable import androidx.compose.runtime.getValue import androidx.compose.runtime.remember import androidx.compose.ui.Alignment @@ -86,6 +87,7 @@ fun CountryListUi(state: CountryListScreen.State, modifier: Modifier = Modifier) * paints exactly what was there before. */ @Composable +@ReadOnlyComposable internal fun listPaneColor() = if (LocalAppSkin.current.sidebarTinted) MaterialTheme.colorScheme.surfaceContainer else MaterialTheme.colorScheme.surface diff --git a/ui/src/commonMain/kotlin/io/github/solcott/countries/ui/ListDetailNavDecoration.kt b/ui/src/commonMain/kotlin/io/github/solcott/countries/ui/ListDetailNavDecoration.kt index 5117fff..5780393 100644 --- a/ui/src/commonMain/kotlin/io/github/solcott/countries/ui/ListDetailNavDecoration.kt +++ b/ui/src/commonMain/kotlin/io/github/solcott/countries/ui/ListDetailNavDecoration.kt @@ -29,7 +29,6 @@ import io.github.solcott.countries.ui.theme.LocalAppSkin * exposes neither — and which quietly resets `shouldAutoFocusCurrentDestination` to true on the way * through, so a `copy()` anywhere after this would undo it. */ -@OptIn(ExperimentalMaterial3AdaptiveApi::class) internal fun countriesPaneDirective(calculated: PaneScaffoldDirective): PaneScaffoldDirective = PaneScaffoldDirective( maxHorizontalPartitions = calculated.maxHorizontalPartitions, From bd4ea705b4fef4bf038891d54474027ef6bcaa7a Mon Sep 17 00:00:00 2001 From: Scott Olcott Date: Mon, 28 Sep 2026 16:02:22 -0600 Subject: [PATCH 5/5] Add a Compose stability check to fail on regressions Enable the Compose compiler's metrics and reports via a new `compose.stability` convention plugin, and add a `ComposeStabilityCheckTask` to enforce them. The task reads the generated reports and fails the build if it finds an `unstable` class or an unskippable restartable composable that isn't explicitly accepted in the new `config/compose/` allowlists. Apply the check across all Compose modules (`app`, `apple`, `desktop`, `presenter`, `shared-compose`, `ui`, and `web`). Alongside this, clean up the Kotlin Compose compiler plugin application to use a version catalog alias (`libs.plugins.kotlin.compose`) rather than a root `buildscript` classpath block. --- app/build.gradle.kts | 3 +- apple/build.gradle.kts | 3 +- build-logic/build.gradle.kts | 5 +- .../main/kotlin/compose.stability.gradle.kts | 50 ++++ .../countries/build/ComposeReportTarget.kt | 53 +++++ .../build/ComposeStabilityCheckTask.kt | 218 ++++++++++++++++++ build.gradle.kts | 14 +- config/compose/unskippable-composables.txt | 9 + config/compose/unstable-classes.txt | 18 ++ desktop/build.gradle.kts | 3 +- gradle/libs.versions.toml | 1 + presenter/build.gradle.kts | 3 +- shared-compose/build.gradle.kts | 3 +- ui/build.gradle.kts | 3 +- web/build.gradle.kts | 3 +- 15 files changed, 366 insertions(+), 23 deletions(-) create mode 100644 build-logic/src/main/kotlin/compose.stability.gradle.kts create mode 100644 build-logic/src/main/kotlin/io/github/solcott/countries/build/ComposeReportTarget.kt create mode 100644 build-logic/src/main/kotlin/io/github/solcott/countries/build/ComposeStabilityCheckTask.kt create mode 100644 config/compose/unskippable-composables.txt create mode 100644 config/compose/unstable-classes.txt diff --git a/app/build.gradle.kts b/app/build.gradle.kts index b9b393c..38de076 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -1,8 +1,9 @@ plugins { id("app") - id("org.jetbrains.kotlin.plugin.compose") + alias(libs.plugins.kotlin.compose) alias(libs.plugins.metro) id("formatting") + id("compose.stability") } android { diff --git a/apple/build.gradle.kts b/apple/build.gradle.kts index 983651a..25e6269 100644 --- a/apple/build.gradle.kts +++ b/apple/build.gradle.kts @@ -18,7 +18,7 @@ plugins { id("org.jetbrains.kotlin.multiplatform") // Molecule recomposes `presenter.present()`, which is a @Composable call, so this module needs // the Compose compiler even though it renders nothing. - id("org.jetbrains.kotlin.plugin.compose") + alias(libs.plugins.kotlin.compose) // So createGraph() resolves, exactly as in :app, :web and :desktop. The graph itself, // and every contribution to it, is aggregated on :shared's compile classpath — not here. alias(libs.plugins.metro) @@ -26,6 +26,7 @@ plugins { // it does NOT touch equals or hashCode, so it is not what keeps the event sinks out of the // equality that lets `StateFlow` conflate a frame — see `EventSink` in AppleUiState.kt for that. alias(libs.plugins.redacted) + id("compose.stability") } // Matches the `import CountriesKit` in the Swift sources. Changing it means changing both. diff --git a/build-logic/build.gradle.kts b/build-logic/build.gradle.kts index 473b94b..0591ee4 100644 --- a/build-logic/build.gradle.kts +++ b/build-logic/build.gradle.kts @@ -14,11 +14,10 @@ dependencies { compileOnly(libs.plugins.dependency.sorter.toDep()) compileOnly(libs.plugins.detekt.toDep()) + compileOnly(libs.plugins.kotlin.compose.toDep()) compileOnly(libs.plugins.ktfmt.toDep()) - detektPlugins( - libs.detekt.compose.rules - ) // required in order to use same detekt.yml as main project + detektPlugins(libs.detekt.compose.rules) } kotlin { jvmToolchain(25) } diff --git a/build-logic/src/main/kotlin/compose.stability.gradle.kts b/build-logic/src/main/kotlin/compose.stability.gradle.kts new file mode 100644 index 0000000..4c1046a --- /dev/null +++ b/build-logic/src/main/kotlin/compose.stability.gradle.kts @@ -0,0 +1,50 @@ +import io.github.solcott.countries.build.ComposeStabilityCheckTask +import io.github.solcott.countries.build.composeReportCompileTask +import org.jetbrains.kotlin.compose.compiler.gradle.ComposeCompilerGradlePluginExtension + +// Turns on the Compose compiler's own metrics and reports and registers the check that reads them. +// Gated on the compiler plugin rather than applying it, the same way kmp.library reacts to +// `org.jetbrains.compose` -- the six Compose modules each declare their own alias. +// +// The destinations are always set rather than hidden behind a `-P` flag, so `composeStabilityCheck` +// is a plain `dependsOn` with nothing to remember on CI. The cost is three small text files per +// compilation. +plugins.withId("org.jetbrains.kotlin.plugin.compose") { + // Not `build/compose`: the Compose Multiplatform plugin already unpacks the skiko runtime there. + val composeDirectory = layout.buildDirectory.dir("reports/compose") + val reportsDirectory = composeDirectory.map { it.dir("reports") } + + configure { + reportsDestination = reportsDirectory + metricsDestination = composeDirectory.map { it.dir("metrics") } + } + + // The Compose plugin hands `reportsDestination` to the compiler as an *input* option, so a + // compile task whose reports have been deleted still counts as up to date and quietly writes + // nothing -- the check would then fail with "no reports found" on a perfectly healthy tree. + // Declaring the directory as an output of the one task the check reads models what the task + // actually produces and fixes that. It buys a second thing too: after a full `build`, where + // every target writes over the shared directory, this task is out of date again and re-runs, so + // it stays the last writer and the check stays deterministic. + afterEvaluate { composeReportCompileTask().configure { outputs.dir(reportsDirectory) } } + + tasks.register("composeStabilityCheck") { + group = "verification" + description = "Fails on Compose stability regressions reported by the Compose compiler." + + // Resolved lazily: Android compilations do not exist when this plugin is applied. + dependsOn(provider { composeReportCompileTask() }) + + reports.from( + reportsDirectory.map { + it.asFileTree.matching { include("*-classes.txt", "*-composables.txt") } + } + ) + unstableClassAllowlist = + rootProject.layout.projectDirectory.file("config/compose/unstable-classes.txt") + unskippableComposableAllowlist = + rootProject.layout.projectDirectory.file("config/compose/unskippable-composables.txt") + moduleName = project.name + summary = composeDirectory.map { it.file("stability-check.txt") } + } +} diff --git a/build-logic/src/main/kotlin/io/github/solcott/countries/build/ComposeReportTarget.kt b/build-logic/src/main/kotlin/io/github/solcott/countries/build/ComposeReportTarget.kt new file mode 100644 index 0000000..1b46698 --- /dev/null +++ b/build-logic/src/main/kotlin/io/github/solcott/countries/build/ComposeReportTarget.kt @@ -0,0 +1,53 @@ +package io.github.solcott.countries.build + +import org.gradle.api.Project +import org.gradle.api.tasks.TaskProvider +import org.jetbrains.kotlin.gradle.dsl.KotlinMultiplatformExtension +import org.jetbrains.kotlin.gradle.dsl.KotlinProjectExtension +import org.jetbrains.kotlin.gradle.dsl.KotlinSingleTargetExtension +import org.jetbrains.kotlin.gradle.plugin.KotlinPlatformType + +/** + * The one compile task whose Compose reports the stability check reads. + * + * `reportsDestination` is a project-level setting the Compose plugin hands to every compilation + * verbatim -- unlike `metricsDestination`, it gets no per-target subdirectory -- and a KMP module's + * targets share a Kotlin module name (`ui/build/classes/kotlin/jvm/main` and `.../android/main` + * both hold `Recipes_ui.kotlin_module`). So all six targets write the same `Recipes_ui-classes.txt` + * and the last one to run wins. Depending on exactly one compile task leaves a single writer, which + * is what makes `composeStabilityCheck` deterministic. Which target hardly matters: stability + * inference for `commonMain` is identical on all of them, so prefer the JVM for compile speed. + */ +internal fun Project.composeReportCompileTask(): TaskProvider<*> { + val kotlin = + extensions.findByName("kotlin") as? KotlinProjectExtension + ?: error( + "No Kotlin extension in :$name -- compose.stability needs one to find a compilation." + ) + val targets = + when (kotlin) { + is KotlinMultiplatformExtension -> kotlin.targets.toList() + is KotlinSingleTargetExtension<*> -> listOf(kotlin.target) + else -> emptyList() + } + val target = + PLATFORM_PREFERENCE.firstNotNullOfOrNull { platform -> + targets.firstOrNull { it.platformType == platform } + } ?: error("No Kotlin target in :$name can produce Compose reports.") + val compilation = + target.compilations.findByName("main") + // KotlinAndroidTarget names its compilations after build types, so there is no "main". + ?: target.compilations.findByName("debug") + ?: error("Target ${target.name} in :$name has neither a main nor a debug compilation.") + return compilation.compileTaskProvider +} + +/** Metadata is deliberately absent: it compiles no bodies, so the compiler reports nothing. */ +private val PLATFORM_PREFERENCE = + listOf( + KotlinPlatformType.jvm, + KotlinPlatformType.androidJvm, + KotlinPlatformType.js, + KotlinPlatformType.wasm, + KotlinPlatformType.native, + ) diff --git a/build-logic/src/main/kotlin/io/github/solcott/countries/build/ComposeStabilityCheckTask.kt b/build-logic/src/main/kotlin/io/github/solcott/countries/build/ComposeStabilityCheckTask.kt new file mode 100644 index 0000000..82efc18 --- /dev/null +++ b/build-logic/src/main/kotlin/io/github/solcott/countries/build/ComposeStabilityCheckTask.kt @@ -0,0 +1,218 @@ +package io.github.solcott.countries.build + +import java.io.File +import org.gradle.api.DefaultTask +import org.gradle.api.GradleException +import org.gradle.api.file.ConfigurableFileCollection +import org.gradle.api.file.RegularFileProperty +import org.gradle.api.provider.Property +import org.gradle.api.tasks.CacheableTask +import org.gradle.api.tasks.Input +import org.gradle.api.tasks.InputFile +import org.gradle.api.tasks.InputFiles +import org.gradle.api.tasks.Optional +import org.gradle.api.tasks.OutputFile +import org.gradle.api.tasks.PathSensitive +import org.gradle.api.tasks.PathSensitivity +import org.gradle.api.tasks.TaskAction + +/** + * Turns the Compose compiler's own stability reports into a pass or a fail. + * + * Reads the `-classes.txt` and `-composables.txt` files the compiler writes to `reportsDestination` + * and fails on two things: + * * a class the compiler inferred as `unstable` -- every recomposition re-runs any composable that + * takes one; + * * a `restartable` composable that is not `skippable`. Strong skipping is on by default in Kotlin + * 2.4, so what survives is genuine: inline functions, non-`Unit` returns, and + * `@NonSkippableComposable`. + * + * Both are overridable through an allowlist file, one `:` per line. The module + * prefix is what lets each project's copy of this task tell its own entries from another module's. + */ +@CacheableTask +abstract class ComposeStabilityCheckTask : DefaultTask() { + + /** The `*-classes.txt` and `*-composables.txt` files written to `reportsDestination`. */ + @get:InputFiles + @get:PathSensitive(PathSensitivity.NONE) + abstract val reports: ConfigurableFileCollection + + /** Classes allowed to stay unstable. */ + @get:InputFile + @get:Optional + @get:PathSensitive(PathSensitivity.NONE) + abstract val unstableClassAllowlist: RegularFileProperty + + /** Restartable composables allowed to stay unskippable. */ + @get:InputFile + @get:Optional + @get:PathSensitive(PathSensitivity.NONE) + abstract val unskippableComposableAllowlist: RegularFileProperty + + /** The project name, which is the prefix this task's allowlist entries must carry. */ + @get:Input abstract val moduleName: Property + + @get:OutputFile abstract val summary: RegularFileProperty + + @TaskAction + fun check() { + val reportFiles = reports.files.filter { it.isFile } + if (reportFiles.isEmpty()) { + throw GradleException( + "No Compose reports found for :${moduleName.get()}. The compiler writes them only when " + + "composeCompiler.reportsDestination is set -- check the compose.stability convention " + + "plugin is applied to this project." + ) + } + + val unstableClasses = + reportFiles.filter { it.name.endsWith(CLASSES_SUFFIX) }.flatMap(::parseUnstableClasses) + val unskippable = + reportFiles + .filter { it.name.endsWith(COMPOSABLES_SUFFIX) } + .flatMap(::parseUnskippableComposables) + + val allowedClasses = readAllowlist(unstableClassAllowlist.orNull?.asFile) + val allowedComposables = readAllowlist(unskippableComposableAllowlist.orNull?.asFile) + + warnAboutStaleEntries(allowedClasses, unstableClasses, unstableClassAllowlist.orNull?.asFile) + warnAboutStaleEntries( + allowedComposables, + unskippable, + unskippableComposableAllowlist.orNull?.asFile, + ) + + val classFailures = unstableClasses.filterNot { it.name in allowedClasses } + val composableFailures = unskippable.filterNot { it.name in allowedComposables } + + writeSummary(reportFiles, unstableClasses, unskippable, classFailures, composableFailures) + + if (classFailures.isNotEmpty() || composableFailures.isNotEmpty()) { + throw GradleException( + buildFailureMessage(classFailures, composableFailures).also { logger.error(it) } + ) + } + } + + private fun buildFailureMessage( + classFailures: List, + composableFailures: List, + ): String = buildString { + val module = moduleName.get() + appendLine("Compose stability check failed for :$module.") + if (classFailures.isNotEmpty()) { + appendLine() + appendLine(" Unstable classes (${classFailures.size}):") + classFailures.forEach { appendLine(" ${it.name} [${it.report.name}]") } + appendLine( + " Make every property a val of a stable type, or annotate the class @Immutable " + + "(nothing ever changes) or @Stable (mutable properties are backed by snapshot state)." + ) + appendLine(" To accept one, add to ${allowlistPath(unstableClassAllowlist.orNull?.asFile)}:") + classFailures.forEach { appendLine(" $module:${it.name}") } + } + if (composableFailures.isNotEmpty()) { + appendLine() + appendLine(" Restartable but not skippable (${composableFailures.size}):") + composableFailures.forEach { appendLine(" ${it.name} [${it.report.name}]") } + appendLine( + " Every unskippable composable re-runs on each recomposition of its parent. Make its " + + "parameters stable, or accept it below with a comment saying why." + ) + appendLine( + " To accept one, add to ${allowlistPath(unskippableComposableAllowlist.orNull?.asFile)}:" + ) + composableFailures.forEach { appendLine(" $module:${it.name}") } + } + } + + private fun warnAboutStaleEntries(allowed: Set, found: List, file: File?) { + // Only ever a warning: the reports cover one target, so an entry for a class declared in, say, + // iosMain legitimately goes unmatched when the check reads the JVM report. + val foundNames = found.mapTo(mutableSetOf()) { it.name } + val stale = allowed - foundNames + if (stale.isNotEmpty()) { + logger.warn( + "Compose stability allowlist for :${moduleName.get()} has ${stale.size} entries that no " + + "longer appear in the report (${stale.sorted().joinToString()}). Remove them from " + + "${allowlistPath(file)} unless they are declared in a source set this target does not " + + "compile." + ) + } + } + + private fun writeSummary( + reportFiles: List, + unstableClasses: List, + unskippable: List, + classFailures: List, + composableFailures: List, + ) { + val out = summary.get().asFile + out.parentFile.mkdirs() + out.writeText( + buildString { + appendLine("Compose stability -- :${moduleName.get()}") + appendLine("reports read: ${reportFiles.joinToString { it.name }}") + appendLine( + "unstable classes: ${unstableClasses.size} (${classFailures.size} not allowlisted)" + ) + unstableClasses.forEach { appendLine(" ${it.name}") } + appendLine( + "restartable, not skippable: ${unskippable.size} " + + "(${composableFailures.size} not allowlisted)" + ) + unskippable.forEach { appendLine(" ${it.name}") } + } + ) + } + + /** + * Entries for this module, with the `:` prefix stripped. Others are another's problem. + */ + private fun readAllowlist(file: File?): Set { + if (file == null || !file.isFile) return emptySet() + val prefix = "${moduleName.get()}:" + return file + .readLines() + .map { it.substringBefore('#').trim() } + .filter { it.startsWith(prefix) } + .mapTo(mutableSetOf()) { it.removePrefix(prefix).trim() } + } + + private fun parseUnstableClasses(report: File): List = + report.readLines().mapNotNull { line -> + // Entries start at column 0; properties inside a class body are indented. + UNSTABLE_CLASS.find(line)?.let { Finding(it.groupValues[1], report) } + } + + private fun parseUnskippableComposables(report: File): List = + report.readLines().mapNotNull { line -> + val match = COMPOSABLE.find(line) ?: return@mapNotNull null + val flags = match.groupValues[1].trim().split(WHITESPACE) + // `runtime`/`readonly`/inline functions are not restartable and cost nothing to skip. + if (RESTARTABLE !in flags || SKIPPABLE in flags) return@mapNotNull null + Finding(match.groupValues[2], report) + } + + private fun allowlistPath(file: File?): String = file?.invariantSeparatorsPath ?: "the allowlist" + + private data class Finding(val name: String, val report: File) + + private companion object { + const val CLASSES_SUFFIX = "-classes.txt" + const val COMPOSABLES_SUFFIX = "-composables.txt" + const val RESTARTABLE = "restartable" + const val SKIPPABLE = "skippable" + + val UNSTABLE_CLASS = Regex("""^unstable class ([^\s{]+)""") + // e.g. `restartable skippable scheme("[androidx.compose.ui.UiComposable]") fun RecipeGrid(`. + // Names come out fully qualified and may carry angle brackets -- a property getter reads + // `fun com.scottolcott.recipe.domain.()` -- so the optional type-parameter + // group excludes `-` rather than matching any `<...>`, which would swallow those. + val COMPOSABLE = + Regex("""^([a-z ]*(?:scheme\("[^"]*"\)\s*)?)fun\s+(?:<[\w, ]+>\s+)?([^(\s]+)\(""") + val WHITESPACE = Regex("""\s+""") + } +} diff --git a/build.gradle.kts b/build.gradle.kts index cd5cfc6..a7223d6 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -1,20 +1,7 @@ -import com.ncorti.ktfmt.gradle.KtfmtExtension import org.gradle.buildconfiguration.tasks.UpdateDaemonJvm import org.jetbrains.kotlin.gradle.targets.js.testing.KotlinJsTest import org.jetbrains.kotlin.gradle.targets.js.webpack.KotlinWebpack -// AGP 9.x has built-in Kotlin support, pinned to an older Kotlin Gradle Plugin (KGP) version. -// Metro 1.3.2 needs Kotlin >= 2.3.20 and Compose needs a matching compiler plugin, so we override -// the built-in KGP (and provide the Compose compiler plugin) on the buildscript classpath. -// The Kotlin-family plugins are then applied in each module via `id(...)` with no version, -// picking up these classpath versions. This is the migration path recommended in AGP 9.0's notes. -buildscript { - dependencies { - classpath(libs.kotlin.gradle.plugin) - classpath(libs.kotlin.compose.compiler.plugin) - } -} - plugins { alias(libs.plugins.android.application) apply false alias(libs.plugins.android.library) apply false @@ -23,6 +10,7 @@ plugins { alias(libs.plugins.ktfmt) apply false alias(libs.plugins.kmp.parcelize) apply false alias(libs.plugins.compose.multiplatform) apply false + alias(libs.plugins.kotlin.compose) apply false } // Pins the Gradle daemon's JVM. `./gradlew updateDaemonJvm` writes the criteria to diff --git a/config/compose/unskippable-composables.txt b/config/compose/unskippable-composables.txt new file mode 100644 index 0000000..7439f86 --- /dev/null +++ b/config/compose/unskippable-composables.txt @@ -0,0 +1,9 @@ +# Composables reported as `restartable` but not `skippable` that we accept anyway. +# +# One `:` per line; `#` starts a comment. The compiler reports fully qualified names, +# so copy them verbatim from the failure message. The module prefix scopes the entry to that +# project's `composeStabilityCheck`. +# +# Strong skipping has been on by default since Kotlin 2.4, so an unskippable composable is no +# longer just "has an unstable parameter" -- what lands here is genuinely unskippable: a non-`Unit` +# return, an inline function, or `@NonSkippableComposable`. Each entry needs a comment saying which. diff --git a/config/compose/unstable-classes.txt b/config/compose/unstable-classes.txt new file mode 100644 index 0000000..34af630 --- /dev/null +++ b/config/compose/unstable-classes.txt @@ -0,0 +1,18 @@ +# Classes the Compose compiler infers as `unstable` that we accept anyway. +# +# One `:` per line; `#` starts a comment. The compiler reports fully qualified names, +# so copy them verbatim from the failure message. The module prefix scopes the entry to that +# project's `composeStabilityCheck`, so entries for other modules are ignored rather than mistaken +# for stale ones. +# +# Every entry needs a comment saying why the class cannot be made stable. An unstable class makes +# every composable that takes one re-run on each recomposition, so the bar is "the alternative is +# worse", not "this was quicker". The usual fixes come first: make the properties `val`s of stable +# types, swap `List` for `ImmutableList`, or annotate `@Immutable` (nothing ever changes) or +# `@Stable` (mutable properties are backed by snapshot state). See CLAUDE.md, *Compose stability*. + +# Android framework entry points, both full of mutable state the framework owns. Neither is ever a +# composable parameter -- MainActivity calls `setContent { CountriesApp(circuit, subCircuit, ...) }` +# and passes the two Circuit instances, not itself. Annotating either @Stable would be a lie. +app:io.github.solcott.countries.CountriesApplication +app:io.github.solcott.countries.MainActivity \ No newline at end of file diff --git a/desktop/build.gradle.kts b/desktop/build.gradle.kts index 5fac3ad..437d53b 100644 --- a/desktop/build.gradle.kts +++ b/desktop/build.gradle.kts @@ -11,7 +11,7 @@ import org.jetbrains.compose.desktop.application.dsl.TargetFormat plugins { id("formatting") id("org.jetbrains.kotlin.jvm") - id("org.jetbrains.kotlin.plugin.compose") + alias(libs.plugins.kotlin.compose) // Brings the `compose.desktop` extension — the packaging tasks and the OS-classified runtime. alias(libs.plugins.compose.multiplatform) // So createGraph() resolves, exactly as in :app and :web. The graph itself, and @@ -26,6 +26,7 @@ plugins { // puts hot-reload-gradle-plugin on the buildscript classpath, so naming a version here fails with // "already on the classpath with an unknown version". Same rule as the Kotlin-family plugins. id("org.jetbrains.compose.hot-reload") + id("compose.stability") } // `Versions` reaches a module build script, not just the convention plugins: it ships in the same diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 050f2fb..b703ebe 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -213,6 +213,7 @@ apollo = { id = "com.apollographql.apollo", version.ref = "apollo" } kotlinx-serialization = { id = "org.jetbrains.kotlin.plugin.serialization", version.ref = "kotlin" } ktfmt = { id = "com.ncorti.ktfmt.gradle", version.ref = "ktfmt" } kmp-parcelize = { id = "io.github.solcott.kmp.parcelize", version.ref = "kmp-parcelize" } +kotlin-compose = { id = "org.jetbrains.kotlin.plugin.compose", version.ref = "kotlin" } # Needed even though dependencies are declared by coordinate: this plugin configures skiko's # npm/webpack packaging, which compose.foundation pulls in on js and wasmJs. compose-multiplatform = { id = "org.jetbrains.compose", version.ref = "composeMultiplatform" } diff --git a/presenter/build.gradle.kts b/presenter/build.gradle.kts index aa74801..9d57917 100644 --- a/presenter/build.gradle.kts +++ b/presenter/build.gradle.kts @@ -4,7 +4,7 @@ import org.jetbrains.kotlin.gradle.ExperimentalWasmDsl plugins { id("kmp-library") - id("org.jetbrains.kotlin.plugin.compose") + alias(libs.plugins.kotlin.compose) // Not for the `compose.*` dependency accessors — dependencies are declared by coordinate below. // This plugin configures skiko's npm/webpack packaging, which compose.foundation pulls in on // js and wasmJs. @@ -15,6 +15,7 @@ plugins { alias(libs.plugins.kotlinx.serialization) alias(libs.plugins.metro) alias(libs.plugins.redacted) + id("compose.stability") } kotlin { diff --git a/shared-compose/build.gradle.kts b/shared-compose/build.gradle.kts index b14322d..c533d2a 100644 --- a/shared-compose/build.gradle.kts +++ b/shared-compose/build.gradle.kts @@ -8,13 +8,14 @@ plugins { id("kmp-library") // Required by the Compose Multiplatform plugin below, not by anything in this module — there is // no @Composable here. CMP fails configuration without it. - id("org.jetbrains.kotlin.plugin.compose") + alias(libs.plugins.kotlin.compose) // Not for the `compose.*` dependency accessors — this module declares no Compose dependency of // its own. It configures skiko's npm/webpack packaging, which arrives here through `:ui`, and // which the browser *test* bundle below cannot load without it. Same reason as in `:presenter` // and `:ui`. alias(libs.plugins.compose.multiplatform) alias(libs.plugins.metro) + id("compose.stability") } kotlin { diff --git a/ui/build.gradle.kts b/ui/build.gradle.kts index b2edffa..312efb0 100644 --- a/ui/build.gradle.kts +++ b/ui/build.gradle.kts @@ -4,9 +4,10 @@ import org.jetbrains.kotlin.gradle.ExperimentalWasmDsl plugins { id("kmp-library") - id("org.jetbrains.kotlin.plugin.compose") + alias(libs.plugins.kotlin.compose) alias(libs.plugins.compose.multiplatform) alias(libs.plugins.metro) + id("compose.stability") } compose.resources { diff --git a/web/build.gradle.kts b/web/build.gradle.kts index fbebf11..9e9e03d 100644 --- a/web/build.gradle.kts +++ b/web/build.gradle.kts @@ -10,13 +10,14 @@ import org.jetbrains.kotlin.gradle.ExperimentalWasmDsl plugins { id("formatting") id("org.jetbrains.kotlin.multiplatform") - id("org.jetbrains.kotlin.plugin.compose") + alias(libs.plugins.kotlin.compose) // Required even though every dependency is declared by catalog coordinate: this is what // configures skiko's npm/webpack packaging, which compose.ui pulls in on js and wasmJs. alias(libs.plugins.compose.multiplatform) // So createGraph() resolves, exactly as in :app. The graph itself, and every // contribution to it, is aggregated on :shared-compose's compile classpath — not here. alias(libs.plugins.metro) + id("compose.stability") } kotlin {