From 255808f2fc2e05db15d170d4f1134abb2b7e92f7 Mon Sep 17 00:00:00 2001 From: null Date: Wed, 19 Aug 2026 21:37:43 -0500 Subject: [PATCH] feat: a guard that every drawable has a night twin MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The light/dark pairing was held by a @Preview and nothing else. A preview fails no build and nothing runs it, and the KDoc on OnboardingPreviews.kt already said why that matters: "a set of eight where seven have a night variant looks completely fine in light mode." The failure mode is what makes this worth a guard. A missing drawable-night file does not crash, does not warn, and does not fall back to nothing — Android resolves the light drawable and draws it on a dark screen. The only other way to find it is to open that one screen in that one theme, which is how #44 was found and how it sat unnoticed until somebody looked. checkThemedDrawables walks both directions: a light asset with no night twin, and a night asset with no light one. The second is the same defect from the other side and renders as nothing rather than as the wrong picture. Exemptions are a named map with a reason each, rather than a narrowed scope. ic_launcher_monochrome is the only entry: the launcher tints it from the system palette, so a night copy would be a second source of truth for one shape. A scope that only listed today's eight illustrations would not cover tomorrow's, and the defect this guards against is a file somebody forgot. Proved four ways, because prove-guard.sh cannot drive this one — it replaces a string inside a file, and this guard's failure mode is a file that is not there, in a set that is all .webp. GUARDS.md gains section 9 for that class of guard, and the manual recipe from section 1 was run instead: - a night twin deleted -> exactly 1 violation, naming art_welcome - a dark-only asset added -> exactly 1 violation, naming art_orphan - both restored -> green, 64 resources across 5 folder pairs - roots pointed at a folder that does not exist -> "no drawables were found ... not a pass" The fourth is the one worth copying. Refusing to report a pass over an empty observation is itself a thing to prove: a guard that finds nothing and says "clean" is the failure GUARDS.md was written after, and two guards in this project have done exactly that. closes #42 Co-Authored-By: Claude Opus 5 (1M context) --- build.gradle.kts | 135 ++++++++++++++++++++++++++++++++++++ docs/architecture/GUARDS.md | 29 ++++++++ docs/architecture/README.md | 1 + 3 files changed, 165 insertions(+) diff --git a/build.gradle.kts b/build.gradle.kts index afa56d5..654aeed 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -447,11 +447,146 @@ tasks.register("checkNoHealthLogging") { } } +// --------------------------------------------------------------------------- +// Every themed drawable has a night twin — PRODUCT_PLAN.md §37 +// --------------------------------------------------------------------------- +// +// The theme swap is pure resource resolution, which is what makes it robust and +// also what makes a gap silent. A missing `drawable-night` file does not fail, +// does not warn, and renders the light illustration on a dark screen. Nobody +// sees it unless they open that one screen in that one theme. +// +// `OnboardingPreviews.kt` renders all eight in a light/dark pair for exactly +// this reason, and its own KDoc says why: "a set of eight where seven have a +// night variant looks completely fine in light mode." But a preview is not a +// test. It fails no build and nothing runs it. + +/** Resource roots whose light/dark pairing is enforced. */ +val themedResourceRoots: List = listOf( + "app/src/main/res", + "core/designsystem/src/main/res", +) + +/** + * Resource names that legitimately have no night twin, each with its reason. + * + * An exemption list rather than a narrower scope, because the rule should cover + * drawables added later by default — the failure this guards against is a file + * somebody forgot, and a scope that only names today's illustrations would not + * cover tomorrow's. + */ +val themedDrawableExemptions: Map = mapOf( + "ic_launcher_monochrome" to + "themed monochrome vector — the launcher tints it from the system palette, " + + "so a night copy would be a second source of truth for one shape", +) + +tasks.register("checkThemedDrawables") { + group = "verification" + description = "Every drawable must have a night twin of the same name, and vice versa." + + // Resolved at configuration time. Reaching for `layout` inside doLast + // captures the Project, which the configuration cache refuses to serialize. + val roots = themedResourceRoots.map { layout.projectDirectory.dir(it).asFile } + val repoRoot = layout.projectDirectory.asFile + val exemptions = themedDrawableExemptions + + doLast { + /** `drawable-hdpi` -> `drawable-night-hdpi`; `drawable` -> `drawable-night`. */ + fun nightNameOf(light: String): String = + if (light == "drawable") "drawable-night" else light.replaceFirst("drawable-", "drawable-night-") + + /** Resource name is the file name without its extension: art_welcome.webp -> art_welcome. */ + fun resourceNamesIn(dir: File): Set = + dir.listFiles() + ?.filter { it.isFile } + ?.map { it.name.substringBeforeLast(".") } + ?.toSet() + .orEmpty() + + val violations = mutableListOf() + var pairsChecked = 0 + var resourcesSeen = 0 + + roots.forEach { root -> + if (!root.isDirectory) return@forEach + + val drawableDirs = root.listFiles() + ?.filter { it.isDirectory && it.name.startsWith("drawable") } + // `/build/` and `/bin/` are outputs, not source — the same trap + // checkNoHealthLogging fell into on its first run. + ?.filterNot { it.path.contains("/build/") || it.path.contains("/bin/") } + .orEmpty() + + val lightDirs = drawableDirs.filterNot { it.name.contains("-night") } + val nightDirs = drawableDirs.filter { it.name.contains("-night") } + + lightDirs.forEach { lightDir -> + val nightDir = File(root, nightNameOf(lightDir.name)) + val light = resourceNamesIn(lightDir).filterNot { it in exemptions }.toSet() + val night = resourceNamesIn(nightDir) + resourcesSeen += light.size + pairsChecked++ + + (light - night).sorted().forEach { + violations += "$it has no night twin — expected " + + "${File(nightDir, it).relativeTo(repoRoot)}." + } + } + + // The other direction. A night asset with no light counterpart is + // the same defect seen from the other side, and it renders as + // nothing at all in light mode rather than as the wrong picture. + nightDirs.forEach { nightDir -> + val lightName = nightDir.name.replaceFirst("drawable-night", "drawable").ifEmpty { "drawable" } + val lightDir = File(root, if (lightName == "drawable-") "drawable" else lightName) + val night = resourceNamesIn(nightDir).filterNot { it in exemptions }.toSet() + val light = resourceNamesIn(lightDir) + resourcesSeen += night.size + + (night - light).sorted().forEach { + violations += "$it exists only in the dark set — expected " + + "${File(lightDir, it).relativeTo(repoRoot)}." + } + } + } + + // Never a silent pass. A renamed folder or a changed module path would + // otherwise report a clean build having compared nothing, which is how + // checkModuleBoundaries spent its first day green. + if (resourcesSeen == 0) { + throw GradleException( + "no drawables were found, so no pairing was checked. This is not a pass — " + + "check the paths in themedResourceRoots.", + ) + } + + if (violations.isNotEmpty()) { + logger.error("") + logger.error("Themed drawables must exist in both light and dark:") + logger.error("") + violations.forEach { logger.error(" - $it") } + logger.error("") + logger.error("A missing night asset does not fail at runtime. Android falls back to the") + logger.error("light drawable and renders it on a dark screen, so the only way to notice") + logger.error("is to open that screen in that theme. If a drawable genuinely needs no") + logger.error("night variant, add it to themedDrawableExemptions with the reason.") + throw GradleException("${violations.size} drawable(s) missing a light or dark counterpart.") + } + + logger.lifecycle( + "themed drawables: $resourcesSeen resource(s) across $pairsChecked folder pair(s), " + + "all paired.", + ) + } +} + // Wired into `check` so it runs with the tests rather than only when remembered. subprojects { tasks.matching { it.name == "check" }.configureEach { dependsOn(rootProject.tasks.named("checkModuleBoundaries")) if (project.path == ":app") dependsOn(rootProject.tasks.named("checkPermissions")) dependsOn(rootProject.tasks.named("checkNoHealthLogging")) + dependsOn(rootProject.tasks.named("checkThemedDrawables")) } } diff --git a/docs/architecture/GUARDS.md b/docs/architecture/GUARDS.md index b77aa7c..bc21cee 100644 --- a/docs/architecture/GUARDS.md +++ b/docs/architecture/GUARDS.md @@ -179,3 +179,32 @@ Two rules follow, and the first is the general one: The uncomfortable part: the documented proofs had been exiting 3 rather than 0 since they were written. Nobody had run them and read the last line. + +## 9. Some guards cannot be driven by `prove-guard.sh`, and must still be proved + +`scripts/prove-guard.sh` breaks a guard's target by **replacing a string inside a +file**. That covers every guard whose subject is code, which was all of them +until `checkThemedDrawables` — whose failure mode is a file that is *not there*. + +There is no string to replace in an absent file, and the drawables are `.webp`, +so there is no text in the present ones either. The tool simply does not reach +this class of guard. + +That is not permission to skip §1. It means running §1's manual recipe instead, +with the same standard — break exactly one thing, require exactly one failure, +restore, require green again — and writing down what was run. For +`checkThemedDrawables` that was four passes: + +```bash +# 1. a night twin is deleted -> 1 violation, naming art_welcome +# 2. a dark-only asset is added -> 1 violation, naming art_orphan +# 3. both restored -> green, 64 resources, 5 folder pairs +# 4. themedResourceRoots pointed at a +# folder that does not exist -> "no drawables were found … not a pass" +``` + +The fourth is the one worth copying. Every guard here refuses to report a pass +over an empty observation, and that refusal is itself a thing to prove — a guard +that finds nothing and says "clean" is the exact failure this document was +written after. + diff --git a/docs/architecture/README.md b/docs/architecture/README.md index d8f7fdf..4809273 100644 --- a/docs/architecture/README.md +++ b/docs/architecture/README.md @@ -417,6 +417,7 @@ menu is. | `scripts/prove-guard.sh` | breaks what a guard protects and requires the guard to go red | | `scripts/schema-guard.sh` | a Room entity may not change without the version changing with it — asks git, because Room overwrites the export during the build | | `checkNoHealthLogging` (root `build.gradle.kts`) | no logging call may exist in a module that can see a cycle date — §45. Strips comments and matches a call rather than the class, so the `Log.WARN` constant and the KDoc explaining the rule both stay legal | +| `checkThemedDrawables` (root `build.gradle.kts`) | every drawable has a `-night` twin of the same name, both directions. A missing night asset fails nothing at runtime — Android falls back to the light one and draws it on a dark screen — so the only other way to notice is to open that screen in that theme. Exemptions are a named map with reasons, not a narrowed scope | | `.githooks/` | pre-commit, commit-msg, post-commit — see [githooks/README.md](githooks/README.md) | ## What does not belong here