Enforce module boundaries from the build - #78
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
The previous two changes removed 14 stale module dependencies and routed the remaining three through
:domain. Nothing stops all of that from growing back — the layering has never been checked by anything, so a singleimplementation(projects.data.local)added in review would silently undo it.Both reference projects this architecture is modelled on have exactly this gap: the
api/implsplit they are built around is held up by convention alone, and one of them has already drifted. This adds the missing mechanism while the graph is clean, so the baseline records real coupling rather than accumulated noise.What changed
asl.module.boundariesplugin on the root project, contributing averifyModuleBoundariestask that fails the build on a forbidden module dependency.apimodule may depend only on:domain.asl.android.featureconvention applied by all seven feature modules.verifyModuleBoundariesand the convention-plugin tests in the static-analysis job.Correctness details that matter
Collection is lazy, not eager. A subproject's
dependencies { }block has not run while the root project is configuring. Reading the graph duringapplywould observe an empty list and pass no matter what was declared — a check that always succeeds is worse than no check. The edges are read through a provider that is realised after every project is evaluated.Only Strings cross into the task. Holding a
Project,ConfigurationorDependencypast configuration time is incompatible with the configuration cache, which this build has enabled.ProjectDependency.pathis used becausedependencyProjectwas removed in Gradle 9.Every production configuration is checked, not just
implementation. Otherwise the rule is evadable by declaring the same edge asapi,compileOnly,runtimeOnlyor a variant-specific configuration. Test and tooling configurations are exempt on purpose, so a feature may still exercise a real implementation from its own tests.Why the feature convention is small
It applies the Android/Compose defaults and adds the Koin BOM and
:domain— things every feature needs by definition. Capabilities stay in each feature's build file.Eight dependencies currently appear in all seven feature modules, which looks like an argument for centralising more. It is not:
:feature:buildrundeclares Compose UI, Material andkoin-androidx-composewhile containing no@Composableat all. That commonality is copy-paste, and moving it into a convention would cement it and quietly grant those dependencies to modules that never wanted them.Behavior
No behavior change. One new verification task, one convention plugin, and seven build files that declare the same dependencies through a different route.
Scope
Not included: removing
:feature:buildrun's unused Compose dependencies, and thedata.local/data.buildsystemsplit packages. Both are separate changes.The baseline is not a place to add things. It has five entries, each corresponding to a real feature-to-feature edge, and a test asserts that every entry still maps to an actual violation so a stale entry cannot sit there unnoticed.
Tests
The check was verified to actually fail, not merely to pass. Injecting
implementation(projects.feature.git)andimplementation(projects.data.local)into:feature:onboardingproduced both expected violations with actionable messages; the file was then restored. Without that step, a check that silently observes nothing looks identical to a passing one.Configuration cache was verified across two consecutive runs — entry stored, then reused.
Verification
From a clean worktree at the branch tip:
./gradlew test detekt verifyModuleBoundaries :app:assembleDebug— BUILD SUCCESSFUL./gradlew -p build-logic test— BUILD SUCCESSFUL, 11 tests