diff --git a/.github/workflows/ci_verify_build.yml b/.github/workflows/ci_verify_build.yml index c8d6cdd3..610dc58a 100644 --- a/.github/workflows/ci_verify_build.yml +++ b/.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 diff --git a/build-logic/conventions/build.gradle.kts b/build-logic/conventions/build.gradle.kts index e359ffa6..9dd61c3a 100644 --- a/build-logic/conventions/build.gradle.kts +++ b/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().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" diff --git a/build-logic/conventions/src/main/kotlin/AslConventionPlugins.kt b/build-logic/conventions/src/main/kotlin/AslConventionPlugins.kt index bb92e1f7..693f3235 100644 --- a/build-logic/conventions/src/main/kotlin/AslConventionPlugins.kt +++ b/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 { + override fun apply(target: Project) = with(target) { + pluginManager.apply("asl.android.library.compose") + val libs = extensions.getByType().named("libs") + dependencies { + add("implementation", platform(libs.findLibrary("koin-bom").get())) + add("implementation", project(":domain")) + } + } +} diff --git a/build-logic/conventions/src/main/kotlin/AslModuleBoundariesConventionPlugin.kt b/build-logic/conventions/src/main/kotlin/AslModuleBoundariesConventionPlugin.kt new file mode 100644 index 00000000..4ee671b1 --- /dev/null +++ b/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 { + 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(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 = 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 + + @get:Input + abstract val baseline: SetProperty + + @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.", + ) + }, + ) + } + } +} diff --git a/build-logic/conventions/src/main/kotlin/ModuleBoundaryRules.kt b/build-logic/conventions/src/main/kotlin/ModuleBoundaryRules.kt new file mode 100644 index 00000000..db7318b6 --- /dev/null +++ b/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, + baseline: Set = emptySet(), +): List = 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 = setOf( + ":feature:editor -> :feature:buildrun", + ":feature:editor -> :feature:git", + ":feature:editor -> :feature:terminal", + ":feature:projects -> :feature:git", + ":feature:settings -> :feature:git", +) diff --git a/build-logic/conventions/src/test/kotlin/ModuleBoundaryRulesTest.kt b/build-logic/conventions/src/test/kotlin/ModuleBoundaryRulesTest.kt new file mode 100644 index 00000000..06b0327e --- /dev/null +++ b/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", + ) + } +} diff --git a/build.gradle.kts b/build.gradle.kts index 1be3fd34..642f9759 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -9,6 +9,7 @@ plugins { alias(libs.plugins.kotlin.jvm) apply false alias(libs.plugins.kotlin.serialization) apply false alias(libs.plugins.detekt) apply false + id("asl.module.boundaries") } val composeRulesDetekt = libs.detekt.compose.rules diff --git a/feature/buildrun/build.gradle.kts b/feature/buildrun/build.gradle.kts index 262173a5..9d38aae9 100644 --- a/feature/buildrun/build.gradle.kts +++ b/feature/buildrun/build.gradle.kts @@ -1,11 +1,9 @@ -plugins { id("asl.android.library.compose") } +plugins { id("asl.android.feature") } android { namespace = "com.ahmadkharfan.androidstudiolite.feature.buildrun" } dependencies { - implementation(platform(libs.koin.bom)) implementation(platform(libs.androidx.compose.bom)) - implementation(projects.domain) implementation(libs.koin.android) implementation(libs.koin.androidx.compose) implementation(libs.androidx.compose.material3) diff --git a/feature/editor/build.gradle.kts b/feature/editor/build.gradle.kts index cc949c69..56ec7bb7 100644 --- a/feature/editor/build.gradle.kts +++ b/feature/editor/build.gradle.kts @@ -1,11 +1,9 @@ -plugins { id("asl.android.library.compose") } +plugins { id("asl.android.feature") } android { namespace = "com.ahmadkharfan.androidstudiolite.feature.editor" } dependencies { - implementation(platform(libs.koin.bom)) implementation(platform(libs.androidx.compose.bom)) - implementation(projects.domain) implementation(projects.core.common) implementation(projects.designsystem) implementation(projects.feature.buildrun) diff --git a/feature/git/build.gradle.kts b/feature/git/build.gradle.kts index dad3d367..20c03f97 100644 --- a/feature/git/build.gradle.kts +++ b/feature/git/build.gradle.kts @@ -1,11 +1,9 @@ -plugins { id("asl.android.library.compose") } +plugins { id("asl.android.feature") } android { namespace = "com.ahmadkharfan.androidstudiolite.feature.git" } dependencies { - implementation(platform(libs.koin.bom)) implementation(platform(libs.androidx.compose.bom)) - implementation(projects.domain) implementation(projects.core.common) implementation(projects.designsystem) implementation(libs.koin.android) diff --git a/feature/onboarding/build.gradle.kts b/feature/onboarding/build.gradle.kts index 9cc9c108..a3cfeb39 100644 --- a/feature/onboarding/build.gradle.kts +++ b/feature/onboarding/build.gradle.kts @@ -1,9 +1,7 @@ -plugins { id("asl.android.library.compose") } +plugins { id("asl.android.feature") } android { namespace = "com.ahmadkharfan.androidstudiolite.feature.onboarding" } dependencies { - implementation(platform(libs.koin.bom)) implementation(platform(libs.androidx.compose.bom)) - implementation(projects.domain) implementation(projects.core.common) implementation(projects.designsystem) implementation(libs.koin.android) diff --git a/feature/projects/build.gradle.kts b/feature/projects/build.gradle.kts index d8d02508..1cbc7059 100644 --- a/feature/projects/build.gradle.kts +++ b/feature/projects/build.gradle.kts @@ -1,9 +1,7 @@ -plugins { id("asl.android.library.compose") } +plugins { id("asl.android.feature") } android { namespace = "com.ahmadkharfan.androidstudiolite.feature.projects" } dependencies { - implementation(platform(libs.koin.bom)) implementation(platform(libs.androidx.compose.bom)) - implementation(projects.domain) implementation(projects.core.common) implementation(projects.designsystem) implementation(projects.feature.git) diff --git a/feature/settings/build.gradle.kts b/feature/settings/build.gradle.kts index ae19b76f..288cbec2 100644 --- a/feature/settings/build.gradle.kts +++ b/feature/settings/build.gradle.kts @@ -1,4 +1,4 @@ -plugins { id("asl.android.library.compose") } +plugins { id("asl.android.feature") } android { namespace = "com.ahmadkharfan.androidstudiolite.feature.settings" buildFeatures { buildConfig = true } @@ -8,9 +8,7 @@ android { } } dependencies { - implementation(platform(libs.koin.bom)) implementation(platform(libs.androidx.compose.bom)) - implementation(projects.domain) implementation(projects.core.common) implementation(projects.designsystem) implementation(projects.feature.git) diff --git a/feature/terminal/build.gradle.kts b/feature/terminal/build.gradle.kts index 718bcab1..bb8f3f73 100644 --- a/feature/terminal/build.gradle.kts +++ b/feature/terminal/build.gradle.kts @@ -1,9 +1,7 @@ -plugins { id("asl.android.library.compose") } +plugins { id("asl.android.feature") } android { namespace = "com.ahmadkharfan.androidstudiolite.feature.terminal" } dependencies { - implementation(platform(libs.koin.bom)) implementation(platform(libs.androidx.compose.bom)) - implementation(projects.domain) implementation(projects.core.common) implementation(projects.designsystem) implementation(libs.kotlinx.coroutines.android)