Enforce module boundaries from the build

.github/workflows/ci_verify_build.yml

@@ -11,7 +11,7 @@ concurrency:
 
 jobs:
   static-analysis:
-    name: Detekt
+    name: Static analysis
     runs-on: ubuntu-latest
     timeout-minutes: 20
 
@@ -35,6 +35,12 @@ jobs:
       - name: Run detekt
         run: ./gradlew detekt --no-daemon --stacktrace
 
+      - name: Verify module boundaries
+        run: ./gradlew verifyModuleBoundaries --no-daemon --stacktrace
+
+      - name: Test the convention plugins
+        run: ./gradlew -p build-logic test --no-daemon --stacktrace
+
       - name: Upload detekt reports
         if: failure()
         uses: actions/upload-artifact@v4

build-logic/conventions/build.gradle.kts

@@ -11,6 +11,11 @@ repositories {
 dependencies {
     implementation("com.android.tools.build:gradle:9.2.1")
     implementation("org.jetbrains.kotlin:kotlin-gradle-plugin:2.2.10")
+    testImplementation(kotlin("test"))
+}
+
+tasks.withType<Test>().configureEach {
+    useJUnitPlatform()
 }
 
 gradlePlugin {
@@ -27,6 +32,14 @@ gradlePlugin {
             id = "asl.android.library.compose"
             implementationClass = "AslAndroidComposeLibraryConventionPlugin"
         }
+        register("aslAndroidFeature") {
+            id = "asl.android.feature"
+            implementationClass = "AslAndroidFeatureConventionPlugin"
+        }
+        register("aslModuleBoundaries") {
+            id = "asl.module.boundaries"
+            implementationClass = "AslModuleBoundariesConventionPlugin"
+        }
         register("aslKotlinLibrary") {
             id = "asl.kotlin.library"
             implementationClass = "AslKotlinLibraryConventionPlugin"

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

@@ -58,3 +58,23 @@ private fun org.gradle.api.plugins.ExtensionContainer.configureAndroidDefaults()
         testOptions.unitTests.isReturnDefaultValues = true
     }
 }
+
+/**
+ * Shared setup for `:feature:*` modules.
+ *
+ * Deliberately limited to things every feature needs by definition — the Android/Compose defaults,
+ * the dependency-version BOMs, and the domain contracts. Capabilities stay in each feature's own
+ * build file: not every feature wants navigation, paging or a datastore, and centralising those
+ * would quietly grant them to modules that never asked. `:feature:buildrun`, for instance, contains
+ * no composables at all.
+ */
+class AslAndroidFeatureConventionPlugin : Plugin<Project> {
+    override fun apply(target: Project) = with(target) {
+        pluginManager.apply("asl.android.library.compose")
+        val libs = extensions.getByType<VersionCatalogsExtension>().named("libs")
+        dependencies {
+            add("implementation", platform(libs.findLibrary("koin-bom").get()))
+            add("implementation", project(":domain"))
+        }
+    }
+}

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

@@ -0,0 +1,120 @@
+import org.gradle.api.DefaultTask
+import org.gradle.api.GradleException
+import org.gradle.api.Plugin
+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.SetProperty
+import org.gradle.api.tasks.Input
+import org.gradle.api.tasks.OutputFile
+import org.gradle.api.tasks.TaskAction
+import org.gradle.kotlin.dsl.register
+
+/**
+ * Fails the build when a module dependency crosses a boundary the architecture forbids.
+ *
+ * Applied to the root project only, because it has to see every subproject's declared dependencies.
+ *
+ * Two details matter for correctness:
+ *
+ * 1. Collection happens in `projectsEvaluated`, not in [apply]. A subproject's `dependencies {}`
+ *    block has not run yet while the root project is being configured, so capturing earlier would
+ *    silently observe an empty graph and pass no matter what.
+ * 2. Only Strings are captured into the task's inputs. Holding a `Project`, `Configuration` or
+ *    `Dependency` past configuration time is not allowed with the configuration cache, which this
+ *    build has enabled.
+ */
+class AslModuleBoundariesConventionPlugin : Plugin<Project> {
+    override fun apply(target: Project) {
+        require(target == target.rootProject) {
+            "asl.module.boundaries applies to the root project; it inspects the whole module graph."
+        }
+
+        target.tasks.register<VerifyModuleBoundariesTask>(TASK_NAME) {
+            group = "verification"
+            description = "Checks that module dependencies respect the architecture boundaries."
+            baseline.set(MODULE_BOUNDARY_BASELINE)
+            report.set(target.layout.buildDirectory.file("reports/module-boundaries/result.txt"))
+            // Lazy on purpose: a subproject's dependencies { } block has not run while the root
+            // project is configuring, so reading the graph eagerly would observe nothing and pass
+            // 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() })
+        }
+    }
+
+    private fun Project.collectProjectEdges(): List<String> = allprojects
+        .flatMap { project ->
+            project.configurations.flatMap { configuration ->
+                configuration.dependencies
+                    .withType(ProjectDependency::class.java)
+                    // ProjectDependency.path is the Gradle 9 accessor; dependencyProject was removed.
+                    .map { dependency -> "${project.path}|${configuration.name}|${dependency.path}" }
+            }
+        }
+        .distinct()
+        .sorted()
+
+    private companion object {
+        const val TASK_NAME = "verifyModuleBoundaries"
+    }
+}
+
+abstract class VerifyModuleBoundariesTask : DefaultTask() {
+
+    @get:Input
+    abstract val edges: ListProperty<String>
+
+    @get:Input
+    abstract val baseline: SetProperty<String>
+
+    @get:OutputFile
+    abstract val report: RegularFileProperty
+
+    @TaskAction
+    fun verify() {
+        val parsed = edges.get().map { encoded ->
+            val (from, configuration, to) = encoded.split("|", limit = 3)
+            ModuleEdge(from = from, configuration = configuration, to = to)
+        }
+        val allowed = baseline.get()
+        val violations = findBoundaryViolations(parsed, allowed)
+
+        val stillPresent = parsed
+            .filter { isProductionConfiguration(it.configuration) }
+            .map { "${it.from} -> ${it.to}" }
+            .toSet()
+        val staleBaseline = allowed - stillPresent
+
+        val summary = buildString {
+            appendLine("checked ${parsed.size} project dependencies")
+            appendLine("baseline entries: ${allowed.size} (${staleBaseline.size} no longer present)")
+            appendLine("violations: ${violations.size}")
+        }
+        report.get().asFile.apply { parentFile.mkdirs() }.writeText(summary)
+
+        if (staleBaseline.isNotEmpty()) {
+            logger.lifecycle(
+                "Module boundary baseline has ${staleBaseline.size} stale entr" +
+                    "${if (staleBaseline.size == 1) "y" else "ies"} that can now be deleted:",
+            )
+            staleBaseline.sorted().forEach { logger.lifecycle("  $it") }
+        }
+
+        if (violations.isNotEmpty()) {
+            throw GradleException(
+                buildString {
+                    appendLine("Module boundary violations (${violations.size}):")
+                    appendLine()
+                    violations.sortedBy { it.edge.toString() }.forEach { appendLine(it.render()) }
+                    appendLine()
+                    append(
+                        "These edges are not in the baseline. Either route the dependency through a " +
+                            "contract, or move it to a test configuration if only tests need it.",
+                    )
+                },
+            )
+        }
+    }
+}

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

@@ -0,0 +1,93 @@
+/**
+ * Pure module-boundary rules, deliberately free of Gradle types so they can be unit tested directly.
+ *
+ * A [ModuleEdge] is one declared project-to-project dependency. The Gradle plugin collects the edges
+ * and hands them here; everything about *whether an edge is allowed* lives in this file.
+ */
+data class ModuleEdge(
+    val from: String,
+    val configuration: String,
+    val to: String,
+) {
+    override fun toString(): String = "$from --($configuration)--> $to"
+}
+
+data class BoundaryViolation(
+    val edge: ModuleEdge,
+    val rule: String,
+    val explanation: String,
+) {
+    fun render(): String = "  ${edge.from} -> ${edge.to}  [${edge.configuration}]\n      $rule: $explanation"
+}
+
+/**
+ * Configurations that carry production code. Test-only and tooling configurations are exempt: a
+ * feature may legitimately exercise a real data implementation from its own tests.
+ */
+fun isProductionConfiguration(name: String): Boolean {
+    val lower = name.lowercase()
+    if ("test" in lower) return false
+    val toolingPrefixes = listOf("ksp", "kapt", "detekt", "lint", "annotationprocessor", "compiler")
+    return toolingPrefixes.none { lower.startsWith(it) }
+}
+
+private fun isFeature(path: String) = path.startsWith(":feature:")
+private fun isApiModule(path: String) = path.endsWith(":api")
+private fun isData(path: String) = path.startsWith(":data:")
+
+/**
+ * @param baseline edges that already violate a rule and are being burned down. An edge listed here
+ *   is reported as an allowed exception; an edge NOT listed fails the build immediately, so the
+ *   graph can only improve.
+ */
+fun findBoundaryViolations(
+    edges: List<ModuleEdge>,
+    baseline: Set<String> = emptySet(),
+): List<BoundaryViolation> = edges
+    .filter { isProductionConfiguration(it.configuration) }
+    .mapNotNull { edge -> violationFor(edge) }
+    .filterNot { "${it.edge.from} -> ${it.edge.to}" in baseline }
+
+private fun violationFor(edge: ModuleEdge): BoundaryViolation? = when {
+    isFeature(edge.from) && isApiModule(edge.from) && edge.to != ":domain" ->
+        BoundaryViolation(
+            edge,
+            "api-module-scope",
+            "A feature api module is a contract: it may only depend on :domain, so that depending on " +
+                "a contract never drags in an implementation.",
+        )
+
+    isFeature(edge.from) && !isApiModule(edge.from) && isFeature(edge.to) && !isApiModule(edge.to) ->
+        BoundaryViolation(
+            edge,
+            "no-feature-to-feature",
+            "Features must not depend on each other's implementations. Depend on ${edge.to}:api, or " +
+                "have :app pass the collaboration in.",
+        )
+
+    isFeature(edge.from) && isData(edge.to) ->
+        BoundaryViolation(
+            edge,
+            "no-feature-to-data",
+            "Features must reach data through a :domain contract; :app binds the implementation. Use " +
+                "testImplementation if only the tests need the real implementation.",
+        )
+
+    isData(edge.from) && isFeature(edge.to) ->
+        BoundaryViolation(
+            edge,
+            "no-data-to-feature",
+            "The data layer must not depend on a feature; that inverts the layering.",
+        )
+
+    else -> null
+}
+
+/** Edges that are allowed for now and expected to be removed. Nothing may be added to this list. */
+val MODULE_BOUNDARY_BASELINE: Set<String> = setOf(
+    ":feature:editor -> :feature:buildrun",
+    ":feature:editor -> :feature:git",
+    ":feature:editor -> :feature:terminal",
+    ":feature:projects -> :feature:git",
+    ":feature:settings -> :feature:git",
+)

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

@@ -0,0 +1,109 @@
+import kotlin.test.Test
+import kotlin.test.assertEquals
+import kotlin.test.assertFalse
+import kotlin.test.assertTrue
+
+class ModuleBoundaryRulesTest {
+
+    private fun edge(from: String, to: String, configuration: String = "implementation") =
+        ModuleEdge(from = from, configuration = configuration, to = to)
+
+    @Test
+    fun `feature depending on another feature is a violation`() {
+        val violations = findBoundaryViolations(listOf(edge(":feature:a", ":feature:b")))
+        assertEquals(1, violations.size)
+        assertEquals("no-feature-to-feature", violations.single().rule)
+    }
+
+    @Test
+    fun `feature depending on another feature's api module is allowed`() {
+        assertTrue(findBoundaryViolations(listOf(edge(":feature:a", ":feature:b:api"))).isEmpty())
+    }
+
+    @Test
+    fun `feature depending on a data module is a violation`() {
+        assertEquals("no-feature-to-data", findBoundaryViolations(listOf(edge(":feature:a", ":data:local"))).single().rule)
+    }
+
+    @Test
+    fun `data depending on a feature is a violation`() {
+        assertEquals("no-data-to-feature", findBoundaryViolations(listOf(edge(":data:local", ":feature:a"))).single().rule)
+    }
+
+    @Test
+    fun `an api module may only depend on domain`() {
+        assertTrue(findBoundaryViolations(listOf(edge(":feature:a:api", ":domain"))).isEmpty())
+        assertEquals(
+            "api-module-scope",
+            findBoundaryViolations(listOf(edge(":feature:a:api", ":core:common"))).single().rule,
+        )
+    }
+
+    @Test
+    fun `allowed edges produce no violations`() {
+        val edges = listOf(
+            edge(":feature:a", ":domain"),
+            edge(":feature:a", ":designsystem"),
+            edge(":feature:a", ":core:common"),
+            edge(":data:local", ":domain"),
+            edge(":app", ":feature:a"),
+            edge(":app", ":data:local"),
+        )
+        assertTrue(findBoundaryViolations(edges).isEmpty())
+    }
+
+    // ---- configuration scoping ------------------------------------------------------------
+
+    @Test
+    fun `test configurations are exempt so tests may use a real implementation`() {
+        val edges = listOf(
+            edge(":feature:a", ":data:local", "testImplementation"),
+            edge(":feature:a", ":feature:b", "testImplementation"),
+            edge(":feature:a", ":data:local", "androidTestImplementation"),
+        )
+        assertTrue(findBoundaryViolations(edges).isEmpty())
+    }
+
+    @Test
+    fun `non-implementation production configurations are still checked`() {
+        // The rule must not be evadable by declaring the same edge on another production configuration.
+        for (configuration in listOf("api", "compileOnly", "runtimeOnly", "debugImplementation", "releaseApi")) {
+            val violations = findBoundaryViolations(listOf(edge(":feature:a", ":feature:b", configuration)))
+            assertEquals(1, violations.size, "expected $configuration to be checked")
+        }
+    }
+
+    @Test
+    fun `tooling configurations are exempt`() {
+        for (configuration in listOf("ksp", "kapt", "detektPlugins", "lintChecks", "annotationProcessor")) {
+            assertFalse(isProductionConfiguration(configuration), "$configuration should be exempt")
+        }
+    }
+
+    // ---- baseline -------------------------------------------------------------------------
+
+    @Test
+    fun `a baselined edge is tolerated but an unlisted one is not`() {
+        val edges = listOf(edge(":feature:a", ":feature:b"), edge(":feature:c", ":feature:d"))
+        val violations = findBoundaryViolations(edges, baseline = setOf(":feature:a -> :feature:b"))
+        assertEquals(1, violations.size)
+        assertEquals(":feature:c", violations.single().edge.from)
+    }
+
+    @Test
+    fun `the shipped baseline covers exactly the known feature to feature edges`() {
+        val edges = MODULE_BOUNDARY_BASELINE.map { entry ->
+            val (from, to) = entry.split(" -> ")
+            edge(from, to)
+        }
+        assertTrue(
+            findBoundaryViolations(edges, MODULE_BOUNDARY_BASELINE).isEmpty(),
+            "every baseline entry must silence its own edge",
+        )
+        assertEquals(
+            MODULE_BOUNDARY_BASELINE.size,
+            findBoundaryViolations(edges).size,
+            "every baseline entry must correspond to a real violation, so stale entries cannot hide",
+        )
+    }
+}