Reject default arguments on composable contract members

build-logic/conventions/src/main/kotlin/ApiContractRules.kt

@@ -0,0 +1,106 @@
+/**
+ * Rules for contract (`:*:api`) modules, kept free of Gradle types so they can be unit tested.
+ *
+ * The rule here exists because of a real crash. `GitPanelApi.Panel` was declared with default
+ * argument values. For a `@Composable` interface member the Compose compiler emits a
+ * `ComposeDefaultImpls.<name>$default` bridge that invokes the abstract method through a synthesised
+ * signature. While the interface and its implementation shared a module that resolved; once the
+ * interface moved to its own api module and the implementation stayed behind, the bridge no longer
+ * matched the override. It compiled, passed detekt, passed every unit test, and threw
+ * `AbstractMethodError` the first time the panel was composed.
+ *
+ * Nothing on the JVM side can see that, so it is checked at the source level instead.
+ */
+data class ApiContractViolation(
+    val file: String,
+    val line: Int,
+    val declaration: String,
+) {
+    fun render(): String =
+        "  $file:$line\n      `$declaration` is a @Composable interface member with default argument " +
+            "values. Across a module boundary the generated defaults bridge does not match the " +
+            "implementation, which fails at runtime with AbstractMethodError. Require every argument."
+}
+
+/**
+ * @param sources file path to file contents. Only contract modules should be passed in.
+ */
+fun findApiContractViolations(sources: Map<String, String>): List<ApiContractViolation> =
+    sources.flatMap { (path, text) -> violationsIn(path, text) }
+
+private fun violationsIn(path: String, text: String): List<ApiContractViolation> {
+    val found = mutableListOf<ApiContractViolation>()
+    val lines = text.lines()
+    var interfaceDepth = -1
+    var braceDepth = 0
+    var composableSeen = false
+
+    var index = 0
+    while (index < lines.size) {
+        val raw = lines[index]
+        val line = raw.substringBefore("//").trim()
+
+        if (interfaceDepth < 0 && Regex("""\binterface\s+\w+""").containsMatchIn(line)) {
+            interfaceDepth = braceDepth
+        }
+        if (line.startsWith("@Composable")) composableSeen = true
+
+        if (composableSeen && interfaceDepth >= 0 && Regex("""\bfun\s+\w+""").containsMatchIn(line)) {
+            val (signature, consumed) = readSignature(lines, index)
+            if (hasDefaultArgument(signature)) {
+                val name = Regex("""\bfun\s+(\w+)""").find(signature)?.groupValues?.get(1).orEmpty()
+                found += ApiContractViolation(path, index + 1, "fun $name(...)")
+            }
+            index += consumed
+            composableSeen = false
+            continue
+        }
+
+        if (line.isNotEmpty() && !line.startsWith("@")) {
+            braceDepth += line.count { it == '{' } - line.count { it == '}' }
+            if (interfaceDepth >= 0 && braceDepth <= interfaceDepth) interfaceDepth = -1
+        }
+        index++
+    }
+    return found
+}
+
+/** Collects a possibly multi-line signature up to the closing paren of its parameter list. */
+private fun readSignature(lines: List<String>, start: Int): Pair<String, Int> {
+    val builder = StringBuilder()
+    var depth = 0
+    var consumed = 0
+    var opened = false
+    for (i in start until lines.size) {
+        val line = lines[i].substringBefore("//")
+        builder.append(line).append('\n')
+        consumed++
+        depth += line.count { it == '(' } - line.count { it == ')' }
+        if (line.contains('(')) opened = true
+        if (opened && depth <= 0) break
+    }
+    return builder.toString() to consumed
+}
+
+/** True when the parameter list contains a top-level `=`, i.e. a default value. */
+private fun hasDefaultArgument(signature: String): Boolean {
+    val params = signature.substringAfter('(', "").substringBeforeLast(')', "")
+    var depth = 0
+    var index = 0
+    while (index < params.length) {
+        when (params[index]) {
+            '(', '<', '[' -> depth++
+            ')', '>', ']' -> depth--
+            '-' -> if (index + 1 < params.length && params[index + 1] == '>') index++ // lambda arrow
+            '=' -> {
+                val next = params.getOrNull(index + 1)
+                // `=` that is not part of `==`, `>=`, `<=` or `->`
+                if (depth == 0 && next != '=' && params.getOrNull(index - 1) !in listOf('!', '<', '>', '=')) {
+                    return true
+                }
+            }
+        }
+        index++
+    }
+    return false
+}

build-logic/conventions/src/main/kotlin/AslModuleBoundariesConventionPlugin.kt

@@ -5,6 +5,7 @@ import org.gradle.api.Project
 import org.gradle.api.artifacts.ProjectDependency
 import org.gradle.api.file.RegularFileProperty
 import org.gradle.api.provider.ListProperty
+import org.gradle.api.provider.MapProperty
 import org.gradle.api.provider.SetProperty
 import org.gradle.api.tasks.Input
 import org.gradle.api.tasks.OutputFile
@@ -41,9 +42,21 @@ class AslModuleBoundariesConventionPlugin : Plugin<Project> {
             // regardless of what is declared. The provider is realised once every project is
             // evaluated, and only Strings are stored, which keeps it configuration-cache safe.
             edges.set(target.provider { target.collectProjectEdges() })
+            contractSources.set(target.provider { target.collectContractSources() })
         }
     }
 
+    /** Kotlin sources of every contract module, keyed by a repo-relative path. */
+    private fun Project.collectContractSources(): Map<String, String> = allprojects
+        .filter { it.path.endsWith(":api") }
+        .flatMap { project ->
+            project.file("src/main").walkTopDown()
+                .filter { it.isFile && it.extension == "kt" }
+                .map { it.relativeTo(rootDir).path to it.readText() }
+                .toList()
+        }
+        .toMap()
+
     private fun Project.collectProjectEdges(): List<String> = allprojects
         .flatMap { project ->
             project.configurations.flatMap { configuration ->
@@ -69,6 +82,9 @@ abstract class VerifyModuleBoundariesTask : DefaultTask() {
     @get:Input
     abstract val baseline: SetProperty<String>
 
+    @get:Input
+    abstract val contractSources: MapProperty<String, String>
+
     @get:OutputFile
     abstract val report: RegularFileProperty
 
@@ -102,6 +118,17 @@ abstract class VerifyModuleBoundariesTask : DefaultTask() {
             staleBaseline.sorted().forEach { logger.lifecycle("  $it") }
         }
 
+        val contractViolations = findApiContractViolations(contractSources.get())
+        if (contractViolations.isNotEmpty()) {
+            throw GradleException(
+                buildString {
+                    appendLine("Contract module violations (${contractViolations.size}):")
+                    appendLine()
+                    contractViolations.forEach { appendLine(it.render()) }
+                },
+            )
+        }
+
         if (violations.isNotEmpty()) {
             throw GradleException(
                 buildString {

build-logic/conventions/src/test/kotlin/ApiContractRulesTest.kt

@@ -0,0 +1,137 @@
+import kotlin.test.Test
+import kotlin.test.assertEquals
+import kotlin.test.assertTrue
+
+class ApiContractRulesTest {
+
+    private fun violations(source: String) = findApiContractViolations(mapOf("Api.kt" to source))
+
+    @Test
+    fun `composable interface member with a default argument is a violation`() {
+        val found = violations(
+            """
+            interface PanelApi {
+                @Composable
+                fun Panel(id: String, onClose: () -> Unit = {})
+            }
+            """.trimIndent(),
+        )
+        assertEquals(1, found.size)
+        assertTrue(found.single().declaration.contains("Panel"))
+    }
+
+    @Test
+    fun `the real regression is caught across multiple lines`() {
+        // The exact shape that shipped an AbstractMethodError.
+        val found = violations(
+            """
+            interface GitPanelApi {
+                @Composable
+                fun Panel(
+                    projectId: String,
+                    onClose: () -> Unit,
+                    onOpenDiff: (String, GitDiffTarget) -> Unit = { _, _ -> },
+                    onOpenHistory: () -> Unit = {},
+                )
+            }
+            """.trimIndent(),
+        )
+        assertEquals(1, found.size)
+    }
+
+    @Test
+    fun `composable interface member without defaults is fine`() {
+        val found = violations(
+            """
+            interface PanelApi {
+                @Composable
+                fun Panel(
+                    projectId: String,
+                    onClose: () -> Unit,
+                    onOpenDiff: (String, GitDiffTarget) -> Unit,
+                )
+            }
+            """.trimIndent(),
+        )
+        assertTrue(found.isEmpty())
+    }
+
+    // ---- cases that must NOT be reported --------------------------------------------------
+
+    @Test
+    fun `a non-composable interface member may keep its defaults`() {
+        // Plain interface methods do not go through the Compose defaults bridge, so they are safe.
+        val found = violations(
+            """
+            interface BuildRunApi {
+                suspend fun clear(buildId: String? = null)
+            }
+            """.trimIndent(),
+        )
+        assertTrue(found.isEmpty())
+    }
+
+    @Test
+    fun `a composable that is not an interface member may keep its defaults`() {
+        val found = violations(
+            """
+            @Composable
+            fun Standalone(text: String, modifier: Modifier = Modifier) { }
+            """.trimIndent(),
+        )
+        assertTrue(found.isEmpty())
+    }
+
+    @Test
+    fun `a data class default value is not mistaken for one`() {
+        val found = violations(
+            """
+            data class BuildClientMeta(
+                val projectId: String,
+                val autoLaunchAfterInstall: Boolean = true,
+            )
+            """.trimIndent(),
+        )
+        assertTrue(found.isEmpty())
+    }
+
+    @Test
+    fun `a class implementing the interface may keep its overrides`() {
+        val found = violations(
+            """
+            class PanelApiImpl : PanelApi {
+                @Composable
+                override fun Panel(id: String, onClose: () -> Unit) { }
+            }
+            """.trimIndent(),
+        )
+        assertTrue(found.isEmpty())
+    }
+
+    @Test
+    fun `comparison operators in a default expression are not read as defaults`() {
+        val found = violations(
+            """
+            interface PanelApi {
+                @Composable
+                fun Panel(id: String, onClose: () -> Unit)
+                fun compare(a: Int, b: Int): Boolean
+            }
+            """.trimIndent(),
+        )
+        assertTrue(found.isEmpty())
+    }
+
+    @Test
+    fun `each offending declaration is reported once with its file and line`() {
+        val found = findApiContractViolations(
+            mapOf(
+                "A.kt" to "interface A {\n    @Composable\n    fun X(a: Int = 1)\n}",
+                "B.kt" to "interface B {\n    @Composable\n    fun Y(b: Int)\n}",
+            ),
+        )
+        assertEquals(1, found.size)
+        assertEquals("A.kt", found.single().file)
+        assertEquals(3, found.single().line)
+    }
+}