From 9faf0a0b8a3598f056f873f4588fa4440c5fd7e0 Mon Sep 17 00:00:00 2001 From: "sungyong.an" Date: Mon, 28 Sep 2026 22:22:40 +0900 Subject: [PATCH 1/3] Add module rules baseline plugin Record module structure rule violations (hilt, impl, layer) in module-rules.txt with the moop.module.rules root plugin: moduleRulesBaseline writes the file and moduleRules fails when it is out of date. Run moduleRules in CI and check.sh. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Upxj1FvQeDpycupKdPQHUZ --- .github/workflows/build.yaml | 3 + CHECK.md | 10 ++ build-logic/module-rules/build.gradle.kts | 16 +++ .../src/main/kotlin/ModuleRulesPlugin.kt | 115 ++++++++++++++++++ .../src/test/kotlin/ModuleRulesPluginTest.kt | 38 ++++++ build-logic/settings.gradle.kts | 1 + build.gradle | 3 + check.sh | 18 ++- module-rules.txt | 4 + 9 files changed, 202 insertions(+), 6 deletions(-) create mode 100644 build-logic/module-rules/build.gradle.kts create mode 100644 build-logic/module-rules/src/main/kotlin/ModuleRulesPlugin.kt create mode 100644 build-logic/module-rules/src/test/kotlin/ModuleRulesPluginTest.kt create mode 100644 module-rules.txt diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml index 9ba983e1..b569d23e 100644 --- a/.github/workflows/build.yaml +++ b/.github/workflows/build.yaml @@ -37,6 +37,9 @@ jobs: - name: Check proguardShield run: ./gradlew proguardShieldFast + - name: Check moduleRules + run: ./gradlew moduleRules + - name: Check spotless run: ./gradlew spotlessCheck --init-script gradle/init.gradle.kts --stacktrace diff --git a/CHECK.md b/CHECK.md index 766a68e7..1d8364dc 100644 --- a/CHECK.md +++ b/CHECK.md @@ -23,6 +23,11 @@ ./gradlew proguardShieldFast ``` +- moduleRules +```bash +./gradlew moduleRules +``` + - spotless ```bash ./gradlew spotlessCheck --init-script gradle/init.gradle.kts @@ -50,6 +55,11 @@ ./gradlew proguardShieldFastBaseline ``` +- moduleRules +```bash +./gradlew moduleRulesBaseline +``` + - spotless ```bash ./gradlew spotlessApply --init-script gradle/init.gradle.kts diff --git a/build-logic/module-rules/build.gradle.kts b/build-logic/module-rules/build.gradle.kts new file mode 100644 index 00000000..f7210a33 --- /dev/null +++ b/build-logic/module-rules/build.gradle.kts @@ -0,0 +1,16 @@ +plugins { + `kotlin-dsl` +} + +dependencies { + testImplementation(libs.test.junit) +} + +gradlePlugin { + plugins { + register("moduleRules") { + id = "moop.module.rules" + implementationClass = "ModuleRulesPlugin" + } + } +} diff --git a/build-logic/module-rules/src/main/kotlin/ModuleRulesPlugin.kt b/build-logic/module-rules/src/main/kotlin/ModuleRulesPlugin.kt new file mode 100644 index 00000000..71a0847a --- /dev/null +++ b/build-logic/module-rules/src/main/kotlin/ModuleRulesPlugin.kt @@ -0,0 +1,115 @@ +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.ConfigurableFileCollection +import org.gradle.api.file.RegularFileProperty +import org.gradle.api.provider.ListProperty +import org.gradle.api.tasks.Input +import org.gradle.api.tasks.InputFiles +import org.gradle.api.tasks.Internal +import org.gradle.api.tasks.OutputFile +import org.gradle.api.tasks.TaskAction +import org.gradle.kotlin.dsl.register +import org.gradle.kotlin.dsl.withType +import java.io.File + +/** + * Records module rule violations in module-rules.txt: + * `moduleRulesBaseline` writes the file and `moduleRules` fails when it is out of date. + */ +class ModuleRulesPlugin : Plugin { + override fun apply(root: Project) { + val modules = root.subprojects.filter { it.buildFile.exists() } + val infoFiles = modules.map { module -> + module.tasks.register("moduleRulesInfo") { + violations.set( + module.provider { + findViolations( + path = module.path, + hasHilt = module.pluginManager.hasPlugin("dagger.hilt.android.plugin"), + dependencies = module.declaredDependencies(), + ) + }, + ) + outputFile.set(module.layout.buildDirectory.file("module-rules.txt")) + }.flatMap { it.outputFile } + } + val baseline = root.tasks.register("moduleRulesBaseline") { update = true } + root.tasks.register("moduleRules") { mustRunAfter(baseline) } + root.tasks.withType().configureEach { + group = "verification" + // Depending on the tasks by path makes configure-on-demand configure every module. + dependsOn(modules.map { "${it.path}:moduleRulesInfo" }) + violationFiles.from(infoFiles) + } + } +} + +/** Module paths from the declaration buckets; AGP copies them, and the module itself, into classpaths. */ +private fun Project.declaredDependencies(): Set = configurations + .filter { !it.isCanBeResolved && !it.isCanBeConsumed } + .flatMap { it.dependencies.withType(ProjectDependency::class.java) } + .map { it.path } + .toSet() + +/** Writes the violations of one module for the root tasks to collect. */ +abstract class ModuleRulesInfoTask : DefaultTask() { + @get:Input + abstract val violations: ListProperty + + @get:OutputFile + abstract val outputFile: RegularFileProperty + + @TaskAction + fun write() = outputFile.get().asFile.writeText(violations.get().joinToString("") { "$it\n" }) +} + +/** Writes ([update]) or checks module-rules.txt. */ +abstract class ModuleRulesTask : DefaultTask() { + @get:InputFiles + abstract val violationFiles: ConfigurableFileCollection + + @get:Input + var update = false + + @get:Internal + val baselineFile: File = project.file("module-rules.txt") + + @TaskAction + fun run() { + val actual = violationFiles.flatMap { it.readLines() }.sorted() + if (update) { + baselineFile.writeText(actual.joinToString("") { "$it\n" }) + return + } + val diff = baselineDiff(expected = baselineFile.takeIf { it.exists() }?.readLines().orEmpty(), actual = actual) + if (diff.isNotEmpty()) { + throw GradleException( + "Module rules changed in module-rules.txt:\n" + diff.joinToString("\n") + + "\n\nIf this is intended, re-baseline with ./gradlew moduleRulesBaseline", + ) + } + } +} + +/** + * The rule violations of the module at [path], one line each: Hilt outside the app and impl modules, + * an impl module dependency outside the app, and a dependency on a higher layer. + */ +internal fun findViolations(path: String, hasHilt: Boolean, dependencies: Set): List = buildList { + val isApp = path == ":app" + if (hasHilt && !isApp && !path.isImpl()) add("hilt: $path") + if (!isApp) dependencies.filter { it.isImpl() }.forEach { add("impl: $path -> $it") } + dependencies.filter { path.layer() >= 0 && it.layer() > path.layer() }.forEach { add("layer: $path -> $it") } +} + +internal fun baselineDiff(expected: List, actual: List): List = + (expected - actual.toSet()).map { "- $it" } + (actual - expected.toSet()).map { "+ $it" } + +private val LAYERS = listOf("core", "data", "feature", "app") + +private fun String.layer() = LAYERS.indexOf(split(':')[1]) + +private fun String.isImpl() = endsWith(":impl") diff --git a/build-logic/module-rules/src/test/kotlin/ModuleRulesPluginTest.kt b/build-logic/module-rules/src/test/kotlin/ModuleRulesPluginTest.kt new file mode 100644 index 00000000..992625a6 --- /dev/null +++ b/build-logic/module-rules/src/test/kotlin/ModuleRulesPluginTest.kt @@ -0,0 +1,38 @@ +import org.junit.Assert.assertEquals +import org.junit.Test + +class ModuleRulesPluginTest { + + @Test + fun findViolations_reportsHiltOutsideAppAndImplModules() { + assertEquals(listOf("hilt: :core:kotlin"), findViolations(":core:kotlin", hasHilt = true, dependencies = emptySet())) + assertEquals(emptyList(), findViolations(":app", hasHilt = true, dependencies = emptySet())) + assertEquals(emptyList(), findViolations(":feature:home:impl", hasHilt = true, dependencies = emptySet())) + } + + @Test + fun findViolations_reportsImplDependencyOutsideApp() { + assertEquals( + listOf("impl: :feature:home:api -> :feature:home:impl"), + findViolations(":feature:home:api", hasHilt = false, dependencies = setOf(":feature:home:impl")), + ) + assertEquals(emptyList(), findViolations(":app", hasHilt = false, dependencies = setOf(":feature:home:impl"))) + } + + @Test + fun findViolations_reportsDependencyOnHigherLayer() { + assertEquals( + listOf("layer: :core:datetime -> :data:model"), + findViolations(":core:datetime", hasHilt = false, dependencies = setOf(":data:model", ":core:kotlin", ":testing")), + ) + assertEquals(emptyList(), findViolations(":testing", hasHilt = false, dependencies = setOf(":feature:home:api"))) + } + + @Test + fun baselineDiff_listsRemovedThenAddedLines() { + assertEquals( + listOf("- hilt: :a", "+ hilt: :c"), + baselineDiff(expected = listOf("hilt: :a", "hilt: :b"), actual = listOf("hilt: :b", "hilt: :c")), + ) + } +} diff --git a/build-logic/settings.gradle.kts b/build-logic/settings.gradle.kts index e37e14e7..3a33eed3 100644 --- a/build-logic/settings.gradle.kts +++ b/build-logic/settings.gradle.kts @@ -12,3 +12,4 @@ dependencyResolutionManagement { include(":convention") include(":module-detector") +include(":module-rules") diff --git a/build.gradle b/build.gradle index 554dfe9b..62ba972d 100644 --- a/build.gradle +++ b/build.gradle @@ -1,4 +1,7 @@ plugins { + // Records module rule violations in module-rules.txt (moduleRules / moduleRulesBaseline). + id "moop.module.rules" + alias(libs.plugins.android.application) apply false alias(libs.plugins.android.library) apply false alias(libs.plugins.kotlin.jvm) apply false diff --git a/check.sh b/check.sh index 9c6e7770..f8e6b636 100755 --- a/check.sh +++ b/check.sh @@ -1,6 +1,6 @@ #!/bin/bash # Check script for PR submission -# Validates dependencies, merged manifest, ProGuard/R8 rules, code formatting, lint. +# Validates dependencies, merged manifest, ProGuard/R8 rules, module rules, code formatting, lint. # Exit immediately if any command fails set -e @@ -9,31 +9,37 @@ echo "Starting check validations..." echo "" # Verify dependency changes -echo "🔍 [1/5] Checking dependency guard..." +echo "🔍 [1/6] Checking dependency guard..." ./gradlew dependencyGuard echo "✓ Dependency guard check passed" echo "" # Verify merged manifest changes -echo "🔍 [2/5] Checking manifest shield..." +echo "🔍 [2/6] Checking manifest shield..." ./gradlew manifestShield echo "✓ Manifest shield check passed" echo "" # Verify ProGuard/R8 rule changes -echo "🔍 [3/5] Checking proguard shield..." +echo "🔍 [3/6] Checking proguard shield..." ./gradlew proguardShieldFast echo "✓ ProGuard shield check passed" echo "" +# Verify module structure rules against the baseline +echo "🔍 [4/6] Checking module rules..." +./gradlew moduleRules +echo "✓ Module rules check passed" +echo "" + # Verify code formatting -echo "🔍 [4/5] Checking code formatting..." +echo "🔍 [5/6] Checking code formatting..." ./gradlew spotlessCheck --init-script gradle/init.gradle.kts echo "✓ Code formatting check passed" echo "" # Static analysis and lint checks -echo "🔍 [5/5] Running lint checks..." +echo "🔍 [6/6] Running lint checks..." ./gradlew lintDebug echo "✓ Lint check passed" echo "" diff --git a/module-rules.txt b/module-rules.txt new file mode 100644 index 00000000..af2cda13 --- /dev/null +++ b/module-rules.txt @@ -0,0 +1,4 @@ +hilt: :core:kotlin +hilt: :feature:home:api +hilt: :testing +layer: :core:datetime -> :data:model From 1d881ecc74034573295306d1d29c12845fdffd0c Mon Sep 17 00:00:00 2001 From: "sungyong.an" Date: Mon, 28 Sep 2026 23:11:49 +0900 Subject: [PATCH 2/3] Tidy up the module rules plugin Apply moop.module.rules at the end of the root plugins block without a comment, and drop mustRunAfter and the missing-baseline fallback that nothing uses. Follow Gradle's task authoring guidance: declare cacheability (@UntrackedTask, @DisableCachingByDefault) and input normalization so strict validatePlugins passes, set update and baselineFile through lazy properties instead of Task.project, and describe the tasks. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Upxj1FvQeDpycupKdPQHUZ --- .../src/main/kotlin/ModuleRulesPlugin.kt | 36 +++++++++++++------ build.gradle | 4 +-- 2 files changed, 26 insertions(+), 14 deletions(-) diff --git a/build-logic/module-rules/src/main/kotlin/ModuleRulesPlugin.kt b/build-logic/module-rules/src/main/kotlin/ModuleRulesPlugin.kt index 71a0847a..aba0904e 100644 --- a/build-logic/module-rules/src/main/kotlin/ModuleRulesPlugin.kt +++ b/build-logic/module-rules/src/main/kotlin/ModuleRulesPlugin.kt @@ -6,14 +6,18 @@ import org.gradle.api.artifacts.ProjectDependency import org.gradle.api.file.ConfigurableFileCollection import org.gradle.api.file.RegularFileProperty import org.gradle.api.provider.ListProperty +import org.gradle.api.provider.Property import org.gradle.api.tasks.Input import org.gradle.api.tasks.InputFiles import org.gradle.api.tasks.Internal import org.gradle.api.tasks.OutputFile +import org.gradle.api.tasks.PathSensitive +import org.gradle.api.tasks.PathSensitivity import org.gradle.api.tasks.TaskAction +import org.gradle.api.tasks.UntrackedTask import org.gradle.kotlin.dsl.register import org.gradle.kotlin.dsl.withType -import java.io.File +import org.gradle.work.DisableCachingByDefault /** * Records module rule violations in module-rules.txt: @@ -36,13 +40,20 @@ class ModuleRulesPlugin : Plugin { outputFile.set(module.layout.buildDirectory.file("module-rules.txt")) }.flatMap { it.outputFile } } - val baseline = root.tasks.register("moduleRulesBaseline") { update = true } - root.tasks.register("moduleRules") { mustRunAfter(baseline) } + root.tasks.register("moduleRulesBaseline") { + description = "Writes the module rule violations to module-rules.txt." + update.set(true) + } + root.tasks.register("moduleRules") { + description = "Fails when the module rule violations differ from module-rules.txt." + update.set(false) + } root.tasks.withType().configureEach { group = "verification" // Depending on the tasks by path makes configure-on-demand configure every module. dependsOn(modules.map { "${it.path}:moduleRulesInfo" }) violationFiles.from(infoFiles) + baselineFile.set(root.layout.projectDirectory.file("module-rules.txt")) } } } @@ -54,7 +65,7 @@ private fun Project.declaredDependencies(): Set = configurations .map { it.path } .toSet() -/** Writes the violations of one module for the root tasks to collect. */ +@DisableCachingByDefault(because = "Writes a few lines computed from its inputs") abstract class ModuleRulesInfoTask : DefaultTask() { @get:Input abstract val violations: ListProperty @@ -66,25 +77,27 @@ abstract class ModuleRulesInfoTask : DefaultTask() { fun write() = outputFile.get().asFile.writeText(violations.get().joinToString("") { "$it\n" }) } -/** Writes ([update]) or checks module-rules.txt. */ +@UntrackedTask(because = "Checks or rewrites module-rules.txt in the source tree") abstract class ModuleRulesTask : DefaultTask() { @get:InputFiles + @get:PathSensitive(PathSensitivity.NONE) abstract val violationFiles: ConfigurableFileCollection @get:Input - var update = false + abstract val update: Property @get:Internal - val baselineFile: File = project.file("module-rules.txt") + abstract val baselineFile: RegularFileProperty @TaskAction fun run() { val actual = violationFiles.flatMap { it.readLines() }.sorted() - if (update) { - baselineFile.writeText(actual.joinToString("") { "$it\n" }) + val file = baselineFile.get().asFile + if (update.get()) { + file.writeText(actual.joinToString("") { "$it\n" }) return } - val diff = baselineDiff(expected = baselineFile.takeIf { it.exists() }?.readLines().orEmpty(), actual = actual) + val diff = baselineDiff(expected = file.readLines(), actual = actual) if (diff.isNotEmpty()) { throw GradleException( "Module rules changed in module-rules.txt:\n" + diff.joinToString("\n") + @@ -100,9 +113,10 @@ abstract class ModuleRulesTask : DefaultTask() { */ internal fun findViolations(path: String, hasHilt: Boolean, dependencies: Set): List = buildList { val isApp = path == ":app" + val layer = path.layer() if (hasHilt && !isApp && !path.isImpl()) add("hilt: $path") if (!isApp) dependencies.filter { it.isImpl() }.forEach { add("impl: $path -> $it") } - dependencies.filter { path.layer() >= 0 && it.layer() > path.layer() }.forEach { add("layer: $path -> $it") } + if (layer >= 0) dependencies.filter { it.layer() > layer }.forEach { add("layer: $path -> $it") } } internal fun baselineDiff(expected: List, actual: List): List = diff --git a/build.gradle b/build.gradle index 62ba972d..8d77f106 100644 --- a/build.gradle +++ b/build.gradle @@ -1,7 +1,4 @@ plugins { - // Records module rule violations in module-rules.txt (moduleRules / moduleRulesBaseline). - id "moop.module.rules" - alias(libs.plugins.android.application) apply false alias(libs.plugins.android.library) apply false alias(libs.plugins.kotlin.jvm) apply false @@ -13,6 +10,7 @@ plugins { alias(libs.plugins.manifestShield) apply false alias(libs.plugins.proguardShield) apply false alias(libs.plugins.baselineprofile) apply false + id "moop.module.rules" } apply from: "$rootDir/gradle/version.gradle" From c4cb29a1b95ad18c4d32a776ab2dd6fb0d315ba3 Mon Sep 17 00:00:00 2001 From: "sungyong.an" Date: Tue, 29 Sep 2026 08:31:06 +0900 Subject: [PATCH 3/3] Rename moduleRulesInfo to moduleRulesViolations The per-module task writes the module's rule violations, not module info, so name the task, its class and its output after that. Also move declaredDependencies into ModuleRulesPlugin, its only user. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Upxj1FvQeDpycupKdPQHUZ --- .../src/main/kotlin/ModuleRulesPlugin.kt | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/build-logic/module-rules/src/main/kotlin/ModuleRulesPlugin.kt b/build-logic/module-rules/src/main/kotlin/ModuleRulesPlugin.kt index aba0904e..5f93f0a5 100644 --- a/build-logic/module-rules/src/main/kotlin/ModuleRulesPlugin.kt +++ b/build-logic/module-rules/src/main/kotlin/ModuleRulesPlugin.kt @@ -26,8 +26,8 @@ import org.gradle.work.DisableCachingByDefault class ModuleRulesPlugin : Plugin { override fun apply(root: Project) { val modules = root.subprojects.filter { it.buildFile.exists() } - val infoFiles = modules.map { module -> - module.tasks.register("moduleRulesInfo") { + val moduleViolationFiles = modules.map { module -> + module.tasks.register("moduleRulesViolations") { violations.set( module.provider { findViolations( @@ -37,7 +37,7 @@ class ModuleRulesPlugin : Plugin { ) }, ) - outputFile.set(module.layout.buildDirectory.file("module-rules.txt")) + outputFile.set(module.layout.buildDirectory.file("module-rules-violations.txt")) }.flatMap { it.outputFile } } root.tasks.register("moduleRulesBaseline") { @@ -51,22 +51,22 @@ class ModuleRulesPlugin : Plugin { root.tasks.withType().configureEach { group = "verification" // Depending on the tasks by path makes configure-on-demand configure every module. - dependsOn(modules.map { "${it.path}:moduleRulesInfo" }) - violationFiles.from(infoFiles) + dependsOn(modules.map { "${it.path}:moduleRulesViolations" }) + violationFiles.from(moduleViolationFiles) baselineFile.set(root.layout.projectDirectory.file("module-rules.txt")) } } -} -/** Module paths from the declaration buckets; AGP copies them, and the module itself, into classpaths. */ -private fun Project.declaredDependencies(): Set = configurations - .filter { !it.isCanBeResolved && !it.isCanBeConsumed } - .flatMap { it.dependencies.withType(ProjectDependency::class.java) } - .map { it.path } - .toSet() + /** Module paths from the declaration buckets; AGP copies them, and the module itself, into classpaths. */ + private fun Project.declaredDependencies(): Set = configurations + .filter { !it.isCanBeResolved && !it.isCanBeConsumed } + .flatMap { it.dependencies.withType(ProjectDependency::class.java) } + .map { it.path } + .toSet() +} @DisableCachingByDefault(because = "Writes a few lines computed from its inputs") -abstract class ModuleRulesInfoTask : DefaultTask() { +abstract class ModuleRulesViolationsTask : DefaultTask() { @get:Input abstract val violations: ListProperty