diff --git a/core/src/commonMain/kotlin/dev/androidbroadcast/featured/ConfigValues.kt b/core/src/commonMain/kotlin/dev/androidbroadcast/featured/ConfigValues.kt index 4788966..ab8df1e 100644 --- a/core/src/commonMain/kotlin/dev/androidbroadcast/featured/ConfigValues.kt +++ b/core/src/commonMain/kotlin/dev/androidbroadcast/featured/ConfigValues.kt @@ -91,6 +91,7 @@ public class ConfigValues( } if (remoteValue != null) return remoteValue + @Suppress("HardcodedFlagValue") // intentional: this IS the provider fallback path return ConfigValue(param.defaultValue, ConfigValue.Source.DEFAULT) } diff --git a/docs/superpowers/plans/2026-03-22-agp9-migration.md b/docs/superpowers/plans/2026-03-22-agp9-migration.md new file mode 100644 index 0000000..0abfc60 --- /dev/null +++ b/docs/superpowers/plans/2026-03-22-agp9-migration.md @@ -0,0 +1,916 @@ +# AGP 9 Migration Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Migrate the Featured KMP project from AGP 8.12.0 + Gradle 8.14.3 to AGP 9.1.0 + Gradle 9.1.0 with full (non-legacy) AGP 9 support in a single PR. + +**Architecture:** Issue 1 lays the foundation (version bumps + temporary legacy flag). Issues 2–5 migrate KMP module groups in parallel worktrees. Issue 6 removes the temporary flag, rewrites `gradle.properties` with all explicit properties, and verifies the full build. + +**Tech Stack:** Kotlin 2.3.10, AGP 9.1.0, Gradle 9.1.0, Kotlin Multiplatform, SKIE 0.10.10, Compose Multiplatform 1.9.3 + +**Spec:** `docs/superpowers/specs/2026-03-22-agp9-migration-design.md` + +--- + +## Parallelisation Map + +``` +Task 1: Foundation (sequential — all other tasks depend on this) + ├── Task 2: core, featured-testing, featured-platform + ├── Task 3: featured-compose, featured-debug-ui + ├── Task 4: datastore-provider, featured-registry + └── Task 5: sample → androidApp restructuring + └── Task 6: Final cleanup + full verification +``` + +Tasks 2–5 run in **parallel worktrees** off the Task 1 branch. Task 6 runs after all four merge. + +--- + +## GitHub Issues to Create (before starting implementation) + +Create one GitHub issue per task below using `gh issue create`. Each issue becomes the PR description for its worktree branch. + +| Issue | Title | Assignee | +|---|---|---| +| Task 1 | `build: upgrade Gradle 9.1.0 + AGP 9.1.0 + Dokka 2.1.0 foundation` | — | +| Task 2 | `build: migrate core, featured-testing, featured-platform to AGP 9 KMP plugin` | — | +| Task 3 | `build: migrate featured-compose, featured-debug-ui to AGP 9 KMP plugin` | — | +| Task 4 | `build: migrate datastore-provider, featured-registry to AGP 9 KMP plugin` | — | +| Task 5 | `build: extract androidApp module from sample for AGP 9 compliance` | — | +| Task 6 | `build: finalize AGP 9 migration — remove legacy flags, explicit gradle.properties` | — | + +--- + +## Task 1: Foundation — Version Bumps + Temporary Compatibility Flag + +**Branch:** `feat/agp9-migration` off `main` + +**Files to modify:** +- `gradle/wrapper/gradle-wrapper.properties` +- `gradle/libs.versions.toml` +- `build.gradle.kts` (root) +- `gradle.properties` + +--- + +- [ ] **Step 1.1: Create the feature branch** + +```bash +git checkout main && git pull +git checkout -b feat/agp9-migration +``` + +- [ ] **Step 1.2: Upgrade the Gradle wrapper** + +Edit `gradle/wrapper/gradle-wrapper.properties` — change the `distributionUrl` line: + +```properties +distributionUrl=https\://services.gradle.org/distributions/gradle-9.1.0-bin.zip +``` + +- [ ] **Step 1.3: Upgrade AGP, Dokka, and add the KMP library plugin alias** + +Edit `gradle/libs.versions.toml`: + +```toml +# Change these two lines: +agp = "9.1.0" +dokka = "2.1.0" + +# Add this line in the [plugins] section: +androidKmpLibrary = { id = "com.android.kotlin.multiplatform.library", version.ref = "agp" } +``` + +- [ ] **Step 1.4: Register the new plugin in the root build file** + +Edit `build.gradle.kts` (root). In the `plugins {}` block, add **after** the existing +`alias(libs.plugins.androidLibrary) apply false` line — do not remove any existing entries: + +```kotlin +alias(libs.plugins.androidKmpLibrary) apply false +``` + +- [ ] **Step 1.5: Add parallel build and temporary compatibility flags to gradle.properties** + +Edit `gradle.properties` — append: + +```properties +# Build performance +org.gradle.parallel=true + +# AGP 9 migration — temporary: removed in Task 6 once all modules are migrated +android.enableLegacyVariantApi=true +``` + +> ⚠️ Do NOT set `android.builtInKotlin=false`. The new `com.android.kotlin.multiplatform.library` +> plugin requires `builtInKotlin=true` (the AGP 9 default). The existing `kotlinAndroid` plugin +> on android-only modules is compatible with `builtInKotlin=true` — redundant, not conflicting. + +- [ ] **Step 1.6: Verify Dokka 2.1 property compatibility** + +Check whether `org.jetbrains.dokka.experimental.gradle.pluginMode=V1Enabled` is still valid in +Dokka 2.1 by running: + +```bash +./gradlew :core:dokkaHtml --dry-run 2>&1 | head -40 +``` + +If you see an error about `V1Enabled` being an unknown value, remove the property from +`gradle.properties`. If `dokkaHtml` configures cleanly, keep it. + +- [ ] **Step 1.7: Verify project configures** + +```bash +./gradlew help 2>&1 | tail -20 +``` + +Expected: `BUILD SUCCESSFUL`. Configuration-time errors must be fixed before continuing. +Compilation failures in individual modules are expected at this point and will be fixed in Tasks 2–5. + +- [ ] **Step 1.8: Commit** + +```bash +git add gradle/wrapper/gradle-wrapper.properties \ + gradle/libs.versions.toml \ + build.gradle.kts \ + gradle.properties +git commit -m "build: upgrade Gradle 9.1.0 + AGP 9.1.0 + Dokka 2.1.0 — migration foundation" +``` + +--- + +## Task 2: Migrate `core`, `featured-testing`, `featured-platform` + +**Branch:** `feat/agp9-task2-core-group` off `feat/agp9-migration` (Task 1 branch) + +**Files to modify:** +- `core/build.gradle.kts` +- `featured-testing/build.gradle.kts` +- `featured-platform/build.gradle.kts` + +**⚠️ SKIE risk:** `core` applies `alias(libs.plugins.skie)`. If SKIE 0.10.10 does not support +`com.android.kotlin.multiplatform.library`, the `core` migration will fail at compile time. +Follow the contingency steps below. + +--- + +### Migrate `featured-testing` + +- [ ] **Step 2.1: Apply DSL migration to `featured-testing/build.gradle.kts`** + +Replace the plugin declaration and remove the `android {}` block: + +```kotlin +// BEFORE (plugins block): +alias(libs.plugins.androidLibrary) +alias(libs.plugins.kotlinMultiplatform) + +// AFTER (plugins block): +alias(libs.plugins.androidKmpLibrary) +alias(libs.plugins.kotlinMultiplatform) +``` + +Move `android {}` configuration into `kotlin { androidLibrary {} }` and remove the old blocks: + +```kotlin +// REMOVE the entire android { } block +// REMOVE androidTarget { } from inside kotlin { } + +// ADD inside kotlin { } replacing androidTarget { }: +androidLibrary { + namespace = "dev.androidbroadcast.featured.testing" + compileSdk = libs.versions.android.compileSdk.get().toInt() + minSdk = libs.versions.android.minSdk.get().toInt() + compilerOptions { + jvmTarget.set(JvmTarget.JVM_21) + } +} +``` + +Rename test dependency if present: +```kotlin +// androidUnitTestImplementation(...) → androidHostTestImplementation(...) +``` + +- [ ] **Step 2.2: Verify `featured-testing` compiles** + +```bash +./gradlew :featured-testing:compileKotlinAndroid 2>&1 | tail -30 +``` + +Expected: `BUILD SUCCESSFUL` + +### Migrate `featured-platform` + +- [ ] **Step 2.3: Apply DSL migration to `featured-platform/build.gradle.kts`** + +Same pattern as `featured-testing` — swap plugin alias, collapse `android {}` into +`kotlin { androidLibrary {} }`, rename any `androidUnitTestImplementation` to +`androidHostTestImplementation`. + +- [ ] **Step 2.4: Verify `featured-platform` compiles** + +```bash +./gradlew :featured-platform:compileKotlinAndroid 2>&1 | tail -30 +``` + +Expected: `BUILD SUCCESSFUL` + +### Migrate `core` (SKIE module) + +- [ ] **Step 2.5: Apply DSL migration to `core/build.gradle.kts`** + +Swap plugin alias and collapse blocks — same pattern as above. The `skie {}` configuration block +and the `zipXCFramework` task remain unchanged. + +```kotlin +// BEFORE (plugins block): +alias(libs.plugins.kotlinMultiplatform) +alias(libs.plugins.androidLibrary) +alias(libs.plugins.skie) +... + +// AFTER (plugins block): +alias(libs.plugins.kotlinMultiplatform) +alias(libs.plugins.androidKmpLibrary) +alias(libs.plugins.skie) +... +``` + +Replace `androidTarget { compilerOptions { } }` with `androidLibrary { ... }` inside `kotlin {}`, +and remove the `android {}` block: + +```kotlin +kotlin { + explicitApi() + + androidLibrary { + namespace = "dev.androidbroadcast.featured.core" + compileSdk = libs.versions.android.compileSdk.get().toInt() + minSdk = libs.versions.android.minSdk.get().toInt() + compilerOptions { + jvmTarget.set(JvmTarget.JVM_21) + } + } + + val xcf = XCFramework("FeaturedCore") + listOf(iosX64(), iosArm64(), iosSimulatorArm64()).forEach { iosTarget -> + iosTarget.binaries.framework { + baseName = "FeaturedCore" + isStatic = true + xcf.add(this) + } + } + + jvm() + + sourceSets { /* unchanged */ } +} +// Remove android { } block entirely +// Keep skie { } block unchanged +``` + +- [ ] **Step 2.6: Test SKIE compatibility — verify `core` compiles** + +```bash +./gradlew :core:compileKotlinAndroid 2>&1 | tail -40 +``` + +**If `BUILD SUCCESSFUL`:** SKIE is compatible. Continue. + +**If build fails with a SKIE-related error** (e.g., `SKIEPlugin`, `co.touchlab.skie`, plugin +application error): apply the contingency — revert `core/build.gradle.kts` to its pre-migration +state and add this comment at the top: + +```kotlin +// TODO: Migrate to androidKmpLibrary once SKIE supports AGP 9. +// Tracking: https://github.com/touchlab/SKIE/issues (search for AGP 9 support) +// Blocked by: co.touchlab.skie:gradle-plugin:0.10.10 incompatible with +// com.android.kotlin.multiplatform.library +``` + +Then run again to confirm the contingency builds: +```bash +./gradlew :core:compileKotlinAndroid 2>&1 | tail -20 +``` + +- [ ] **Step 2.7: Run all tests for this group** + +```bash +./gradlew :core:allTests :featured-testing:allTests :featured-platform:allTests 2>&1 | tail -30 +``` + +Expected: `BUILD SUCCESSFUL`. Use `allTests` rather than `test` — some KMP modules may not have +a JVM target and therefore no plain `test` task. + +- [ ] **Step 2.8: Commit** + +```bash +git add core/build.gradle.kts featured-testing/build.gradle.kts featured-platform/build.gradle.kts +git commit -m "build: migrate core, featured-testing, featured-platform to AGP 9 KMP plugin" +``` + +--- + +## Task 3: Migrate `featured-compose`, `featured-debug-ui` + +**Branch:** `feat/agp9-task3-compose-group` off `feat/agp9-migration` (Task 1 branch) + +**Files to modify:** +- `featured-compose/build.gradle.kts` +- `featured-debug-ui/build.gradle.kts` + +--- + +- [ ] **Step 3.1: Apply DSL migration to `featured-compose/build.gradle.kts`** + +Swap plugin alias (`androidLibrary` → `androidKmpLibrary`), collapse `android {}` into +`kotlin { androidLibrary {} }`. This module uses Compose Multiplatform — source set structure +and dependencies are unchanged; only the plugin and DSL block change. + +- [ ] **Step 3.2: Verify `featured-compose` compiles** + +```bash +./gradlew :featured-compose:compileKotlinAndroid 2>&1 | tail -30 +``` + +Expected: `BUILD SUCCESSFUL`. If Compose-specific errors appear, verify the Compose source sets +(`commonMain`, `androidMain`) are still declared correctly inside `kotlin { sourceSets { } }`. + +- [ ] **Step 3.3: Apply DSL migration to `featured-debug-ui/build.gradle.kts`** + +Same pattern as `featured-compose`. + +- [ ] **Step 3.4: Verify `featured-debug-ui` compiles** + +```bash +./gradlew :featured-debug-ui:compileKotlinAndroid 2>&1 | tail -30 +``` + +Expected: `BUILD SUCCESSFUL` + +- [ ] **Step 3.5: Run tests** + +```bash +./gradlew :featured-compose:allTests :featured-debug-ui:allTests 2>&1 | tail -20 +``` + +Expected: `BUILD SUCCESSFUL` + +- [ ] **Step 3.6: Commit** + +```bash +git add featured-compose/build.gradle.kts featured-debug-ui/build.gradle.kts +git commit -m "build: migrate featured-compose, featured-debug-ui to AGP 9 KMP plugin" +``` + +--- + +## Task 4: Migrate `datastore-provider`, `featured-registry` + +**Branch:** `feat/agp9-task4-data-group` off `feat/agp9-migration` (Task 1 branch) + +**Files to modify:** +- `datastore-provider/build.gradle.kts` +- `featured-registry/build.gradle.kts` + +**⚠️ SKIE risk:** `datastore-provider` applies SKIE. Same contingency as `core` in Task 2. + +--- + +- [ ] **Step 4.1: Apply DSL migration to `featured-registry/build.gradle.kts`** + +Swap plugin alias, collapse blocks. No SKIE involved — straightforward. + +- [ ] **Step 4.2: Verify `featured-registry` compiles** + +```bash +./gradlew :featured-registry:compileKotlinAndroid 2>&1 | tail -20 +``` + +Expected: `BUILD SUCCESSFUL` + +- [ ] **Step 4.3: Apply DSL migration to `datastore-provider/build.gradle.kts`** + +Swap plugin alias, collapse blocks. Keep the `skie {}` configuration block unchanged. + +- [ ] **Step 4.4: Test SKIE compatibility — verify `datastore-provider` compiles** + +```bash +./gradlew :datastore-provider:compileKotlinAndroid 2>&1 | tail -40 +``` + +**If `BUILD SUCCESSFUL`:** SKIE compatible — continue. + +**If SKIE-related failure:** revert `datastore-provider/build.gradle.kts` and add the same +contingency comment as in Task 2 Step 2.6. + +- [ ] **Step 4.5: Run tests** + +```bash +./gradlew :datastore-provider:allTests :featured-registry:allTests 2>&1 | tail -20 +``` + +Expected: `BUILD SUCCESSFUL` + +- [ ] **Step 4.6: Commit** + +```bash +git add datastore-provider/build.gradle.kts featured-registry/build.gradle.kts +git commit -m "build: migrate datastore-provider, featured-registry to AGP 9 KMP plugin" +``` + +--- + +## Task 5: Extract `androidApp` Module from `sample` + +**Branch:** `feat/agp9-task5-sample-split` off `feat/agp9-migration` (Task 1 branch) + +**Files to create:** +- `androidApp/build.gradle.kts` +- `androidApp/src/main/AndroidManifest.xml` (copied from `sample/src/androidMain/`) +- `androidApp/src/main/kotlin/dev/androidbroadcast/featured/SampleApplication.kt` (moved) +- `androidApp/src/main/res/**` (moved — full icon/resource tree) + +**Files to modify:** +- `settings.gradle.kts` (register `:androidApp` **first** — before writing build.gradle.kts) +- `sample/build.gradle.kts` + +**Files unchanged:** `iosApp/` and `Package.swift` — the iOS framework is produced by the KMP +`sample` module which keeps its iOS targets. No path changes. + +**⚠️ SKIE risk:** `sample/build.gradle.kts` applies `alias(libs.plugins.skie)`. When `sample` +switches from `androidApplication` to `androidKmpLibrary`, SKIE encounters the same +compatibility risk as `core` and `datastore-provider`. Apply the identical contingency if needed. + +--- + +- [ ] **Step 5.1: Audit all Android-specific files in `sample/src/androidMain/`** + +```bash +find sample/src/androidMain -type f | sort +``` + +Expected output (verified against actual project): +``` +sample/src/androidMain/AndroidManifest.xml +sample/src/androidMain/kotlin/dev/androidbroadcast/featured/SampleApplication.kt +sample/src/androidMain/res/drawable-v24/ic_launcher_foreground.xml +sample/src/androidMain/res/drawable/ic_launcher_background.xml +sample/src/androidMain/res/mipmap-anydpi-v26/ic_launcher.xml +sample/src/androidMain/res/mipmap-anydpi-v26/ic_launcher_round.xml +sample/src/androidMain/res/mipmap-hdpi/ic_launcher.png +sample/src/androidMain/res/mipmap-hdpi/ic_launcher_round.png +sample/src/androidMain/res/mipmap-mdpi/ic_launcher.png +sample/src/androidMain/res/mipmap-mdpi/ic_launcher_round.png +sample/src/androidMain/res/mipmap-xhdpi/ic_launcher.png +sample/src/androidMain/res/mipmap-xhdpi/ic_launcher_round.png +sample/src/androidMain/res/mipmap-xxhdpi/ic_launcher.png +sample/src/androidMain/res/mipmap-xxhdpi/ic_launcher_round.png +sample/src/androidMain/res/mipmap-xxxhdpi/ic_launcher.png +sample/src/androidMain/res/mipmap-xxxhdpi/ic_launcher_round.png +sample/src/androidMain/res/values/strings.xml +``` + +If the output differs, adjust subsequent copy steps to match actual files. + +- [ ] **Step 5.2: Create the `androidApp` directory structure** + +```bash +mkdir -p androidApp/src/main/kotlin/dev/androidbroadcast/featured +``` + +- [ ] **Step 5.3: Register `androidApp` in `settings.gradle.kts` (must be done before Step 5.5)** + +Add to the include list in `settings.gradle.kts`: + +```kotlin +include(":androidApp") +``` + +Gradle evaluates all subproject build files only after `settings.gradle.kts` is fully parsed. +`:androidApp` must be registered here before its `build.gradle.kts` can be evaluated. + +- [ ] **Step 5.4: Copy all Android-specific files to `androidApp`** + +```bash +# Manifest +cp sample/src/androidMain/AndroidManifest.xml \ + androidApp/src/main/AndroidManifest.xml + +# Kotlin source +cp sample/src/androidMain/kotlin/dev/androidbroadcast/featured/SampleApplication.kt \ + androidApp/src/main/kotlin/dev/androidbroadcast/featured/SampleApplication.kt + +# Resources (entire tree) +cp -r sample/src/androidMain/res androidApp/src/main/res +``` + +Do not delete the originals yet — confirm the new module compiles first (Step 5.8). + +- [ ] **Step 5.5: Create `androidApp/build.gradle.kts`** + +Note: dependencies use `project(":")` notation to match the existing project convention. +`targetSdk` is explicitly set here (the original `sample` relied on the AGP default; +`androidApp` declares it explicitly as best practice for application modules). + +```kotlin +// Set to true (or pass -PhasFirebase=true) when google-services.json is present. +val hasFirebase = + project.findProperty("hasFirebase") == "true" || + rootProject.file("sample/google-services.json").exists() + +plugins { + alias(libs.plugins.androidApplication) + alias(libs.plugins.kotlinAndroid) + alias(libs.plugins.composeMultiplatform) + alias(libs.plugins.composeCompiler) + alias(libs.plugins.composeHotReload) +} + +kotlin { + jvmToolchain(21) +} + +android { + namespace = "dev.androidbroadcast.featured" + compileSdk = libs.versions.android.compileSdk.get().toInt() + + defaultConfig { + applicationId = "dev.androidbroadcast.featured" + minSdk = libs.versions.android.minSdk.get().toInt() + targetSdk = libs.versions.android.targetSdk.get().toInt() + versionCode = 1 + versionName = "1.0" + buildConfigField("boolean", "HAS_FIREBASE", "$hasFirebase") + } + + buildFeatures { + buildConfig = true + } + + packaging { + resources { + excludes += "/META-INF/{AL2.0,LGPL2.1}" + } + } + + buildTypes { + getByName("release") { + isMinifyEnabled = false + } + } + + compileOptions { + sourceCompatibility = JavaVersion.VERSION_21 + targetCompatibility = JavaVersion.VERSION_21 + } +} + +dependencies { + implementation(project(":sample")) + + implementation(compose.preview) + implementation(libs.androidx.activity.compose) + implementation(project(":featured-compose")) + implementation(project(":featured-platform")) + implementation(project(":sharedpreferences-provider")) + + debugImplementation(compose.uiTooling) + debugImplementation(project(":featured-debug-ui")) + + if (hasFirebase) { + implementation(platform(libs.firebase.bom)) + implementation(libs.firebase.config) + implementation(project(":firebase-provider")) + } +} +``` + +- [ ] **Step 5.6: Strip Android-specific content from `sample/build.gradle.kts`** + +**Plugin changes:** +```kotlin +// Remove: +alias(libs.plugins.androidApplication) // replaced by androidKmpLibrary +alias(libs.plugins.composeHotReload) // moved to androidApp +alias(libs.plugins.skie) // see SKIE risk below + +// Add: +alias(libs.plugins.androidKmpLibrary) +``` + +Keep: `kotlinMultiplatform`, `composeMultiplatform`, `composeCompiler`. + +**⚠️ SKIE on `sample`:** removing `alias(libs.plugins.skie)` is safe only if the iOS framework +(`FeaturedSampleApp`) does not rely on SKIE-generated Swift interop. If the iosApp Swift code +calls any SKIE-bridged API from `sample`, retain SKIE and test the SKIE contingency path below. + +**Replace `android {}` block** with `kotlin { androidLibrary {} }`: +```kotlin +androidLibrary { + namespace = "dev.androidbroadcast.featured.sample" + compileSdk = libs.versions.android.compileSdk.get().toInt() + minSdk = libs.versions.android.minSdk.get().toInt() + compilerOptions { + jvmTarget.set(JvmTarget.JVM_21) + } +} +``` + +**Remove** from `kotlin { sourceSets { androidMain.dependencies { } } }` the dependencies that +moved to `androidApp`: +- `compose.preview` +- `libs.androidx.activity.compose` +- `project(":featured-compose")` +- `project(":featured-platform")` +- `project(":sharedpreferences-provider")` +- Firebase entries + +**Preserve** the following — they belong to the shared KMP module: +- `jvm()` target and `jvmMain.dependencies { }` block (compose.desktop, coroutinesSwing) +- `compose.desktop { application { } }` block +- `iosX64()`, `iosArm64()`, `iosSimulatorArm64()` targets with `FeaturedSampleApp` framework +- `commonMain.dependencies { }` + +**Remove** the top-level `dependencies { debugImplementation(...) }` block — moved to `androidApp`. + +- [ ] **Step 5.7: Test SKIE compatibility on `sample` (if SKIE was kept)** + +If `alias(libs.plugins.skie)` was retained in `sample/build.gradle.kts`: + +```bash +./gradlew :sample:compileKotlinAndroid 2>&1 | tail -40 +``` + +**If failure with SKIE error:** apply contingency — revert `sample` to `androidApplication` + +old DSL; the `androidApp` restructuring cannot proceed while SKIE blocks the new plugin. +Document with: +```kotlin +// TODO: Switch sample to androidKmpLibrary once SKIE supports AGP 9. +// Tracking: https://github.com/touchlab/SKIE/issues +``` + +- [ ] **Step 5.8: Verify `androidApp` assembles** + +```bash +./gradlew :androidApp:assembleDebug 2>&1 | tail -40 +``` + +Expected: `BUILD SUCCESSFUL`. + +If resource errors appear (missing launcher icons), verify Step 5.4 copied the full `res/` tree. + +- [ ] **Step 5.9: Verify `sample` JVM desktop target still builds** + +```bash +./gradlew :sample:jvmJar 2>&1 | tail -20 +``` + +Expected: `BUILD SUCCESSFUL`. Confirms the `compose.desktop` and `jvmMain` content was preserved. + +- [ ] **Step 5.10: Remove original Android-specific files from `sample/src/androidMain/`** + +Only after Steps 5.8 and 5.9 both succeed: + +```bash +rm sample/src/androidMain/kotlin/dev/androidbroadcast/featured/SampleApplication.kt +rm sample/src/androidMain/AndroidManifest.xml +rm -r sample/src/androidMain/res + +# Confirm the androidApp still assembles without the originals +./gradlew :androidApp:assembleDebug 2>&1 | tail -10 +``` + +Expected: `BUILD SUCCESSFUL` + +- [ ] **Step 5.11: Commit** + +```bash +git add androidApp/ sample/build.gradle.kts settings.gradle.kts +git commit -m "build: extract androidApp module from sample for AGP 9 KMP app compliance" +``` + +--- + +## Task 6: Final Cleanup + Explicit `gradle.properties` + Full Verification + +**Branch:** merge Tasks 2–5 into `feat/agp9-migration`, then continue on that branch. + +**Files to modify:** +- `gradle.properties` (full rewrite) + +--- + +- [ ] **Step 6.1: Merge all parallel task branches into `feat/agp9-migration`** + +```bash +git checkout feat/agp9-migration +git merge feat/agp9-task2-core-group +git merge feat/agp9-task3-compose-group +git merge feat/agp9-task4-data-group +git merge feat/agp9-task5-sample-split +``` + +Resolve any merge conflicts (unlikely — modules are independent files). + +- [ ] **Step 6.2: Rewrite `gradle.properties` with all explicit properties** + +Replace the entire file content with: + +```properties +# Kotlin +kotlin.code.style=official +kotlin.daemon.jvmargs=-Xmx3072M + +# Gradle +org.gradle.jvmargs=-Xmx4096M -Dfile.encoding=UTF-8 +org.gradle.configuration-cache=true +org.gradle.caching=true +org.gradle.parallel=true + +# Android — General +android.useAndroidX=true +android.nonTransitiveRClass=true +android.uniquePackageNames=true +android.enableJetifier=false +android.sdk.defaultTargetSdkToCompileSdkIfUnset=true + +# Android — DSL & Kotlin +android.builtInKotlin=true +android.newDsl=true + +# Android — Dependencies +android.dependency.useConstraints=false + +# Android — Build Features +android.defaults.buildfeatures.resvalues=false + +# Android — Testing +android.default.androidx.test.runner=true +android.onlyEnableUnitTestForTheTestedBuildType=true + +# Android — R8 / ProGuard +android.enableR8.fullMode=true +android.r8.optimizedResourceShrinking=true +android.r8.strictInputValidation=true +android.proguard.failOnMissingFiles=true + +# Publishing +VERSION_NAME=0.1.0-SNAPSHOT + +# Dokka +org.jetbrains.dokka.experimental.gradle.pluginMode=V1Enabled +``` + +> This replaces the temporary `android.enableLegacyVariantApi=true` and `org.gradle.parallel=true` +> (already added in Task 1) with the complete, permanent set of explicit properties. + +- [ ] **Step 6.3: Verify project configures cleanly** + +```bash +./gradlew help 2>&1 | tail -10 +``` + +Expected: `BUILD SUCCESSFUL` with no warnings about unknown or deprecated properties. +If `android.defaults.buildfeatures.resvalues` is unrecognised in AGP 9.1.0, remove it, +then re-run `./gradlew help` to confirm the project still configures cleanly. + +- [ ] **Step 6.4: Run the full build** + +```bash +./gradlew build 2>&1 | tail -50 +``` + +Expected: `BUILD SUCCESSFUL`. Fix any compilation errors before continuing. + +Common failure patterns and fixes: +- **"Unresolved reference: androidUnitTestImplementation"** — rename to `androidHostTestImplementation` in the affected module +- **SKIE error on `core` or `datastore-provider`** — apply the contingency from Task 2 / Task 4 +- **`enableLegacyVariantApi` error** — a plugin still uses legacy variant API; add `android.enableLegacyVariantApi=true` back with an inline comment naming the plugin + +- [ ] **Step 6.5: Run all tests** + +```bash +./gradlew allTests 2>&1 | tail -30 +``` + +Expected: `BUILD SUCCESSFUL`. All test suites pass. Use `allTests` rather than `test` — KMP +modules without a JVM target have no plain `test` task. + +- [ ] **Step 6.6: Run spotless check** + +```bash +./gradlew spotlessCheck 2>&1 | tail -20 +``` + +If violations found: `./gradlew spotlessApply` then re-run check. + +- [ ] **Step 6.7: Run binary compatibility check** + +```bash +./gradlew apiCheck 2>&1 | tail -20 +``` + +Expected: `BUILD SUCCESSFUL`. Public API must not have changed. + +- [ ] **Step 6.8: Commit** + +```bash +git add gradle.properties +git commit -m "build: finalize AGP 9 migration — remove legacy flag, explicit gradle.properties" +``` + +- [ ] **Step 6.9: Open the pull request** + +```bash +gh pr create \ + --base main \ + --title "build: migrate to AGP 9.1.0 + Gradle 9.1.0" \ + --body "$(cat <<'EOF' +## Summary + +- Upgrades Android Gradle Plugin 8.12.0 → 9.1.0 +- Upgrades Gradle wrapper 8.14.3 → 9.1.0 +- Upgrades Dokka 2.0.0 → 2.1.0 +- Migrates all KMP library modules to `com.android.kotlin.multiplatform.library` plugin with unified `androidLibrary {}` DSL +- Extracts `androidApp` module from `sample` as required by AGP 9 for KMP apps +- Rewrites `gradle.properties` with all AGP 9 properties explicitly declared +- Removes all legacy compatibility flags (full AGP 9 support, no legacy mode) + +## Test plan + +- [ ] `./gradlew build` passes +- [ ] `./gradlew allTests` passes +- [ ] `./gradlew spotlessCheck` passes +- [ ] `./gradlew apiCheck` passes +- [ ] Android sample app assembles: `./gradlew :androidApp:assembleDebug` +- [ ] iOS framework builds (if Xcode available): `./gradlew :core:assembleXCFramework` + +## Notes + +If SKIE 0.10.10 is incompatible with the new plugin, `core` and/or `datastore-provider` +retain `com.android.library` temporarily with a `// TODO` comment and tracking issue link. +EOF +)" +``` + +--- + +## Reference: Canonical DSL Migration Pattern + +For every KMP+Android library module the transformation is: + +**Before:** +```kotlin +plugins { + alias(libs.plugins.kotlinMultiplatform) + alias(libs.plugins.androidLibrary) // ← change this + // ... other plugins unchanged +} + +kotlin { + androidTarget { // ← replace with androidLibrary {} + @OptIn(ExperimentalKotlinGradlePluginApi::class) + compilerOptions { + jvmTarget.set(JvmTarget.JVM_21) + } + } + // ... other targets unchanged +} + +android { // ← remove entire block + namespace = "..." + compileSdk = libs.versions.android.compileSdk.get().toInt() + defaultConfig { + minSdk = libs.versions.android.minSdk.get().toInt() + } + compileOptions { + sourceCompatibility = JavaVersion.VERSION_21 + targetCompatibility = JavaVersion.VERSION_21 + } +} +``` + +**After:** +```kotlin +plugins { + alias(libs.plugins.kotlinMultiplatform) + alias(libs.plugins.androidKmpLibrary) // ← new plugin + // ... other plugins unchanged +} + +kotlin { + androidLibrary { // ← replaces androidTarget {} + android {} + namespace = "..." + compileSdk = libs.versions.android.compileSdk.get().toInt() + minSdk = libs.versions.android.minSdk.get().toInt() + compilerOptions { + jvmTarget.set(JvmTarget.JVM_21) + } + } + // ... other targets unchanged +} +// android {} block is gone +``` diff --git a/docs/superpowers/plans/2026-03-23-behind-flag-annotation.md b/docs/superpowers/plans/2026-03-23-behind-flag-annotation.md new file mode 100644 index 0000000..1c63363 --- /dev/null +++ b/docs/superpowers/plans/2026-03-23-behind-flag-annotation.md @@ -0,0 +1,1022 @@ +# @BehindFlag / @AssumesFlag Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Add `@BehindFlag` and `@AssumesFlag` annotations to `core` and two Detekt rules to `featured-detekt-rules` that enforce flag-guarded usage at lint time. + +**Architecture:** Two SOURCE-retention annotations in `core` mark code that must be used behind a flag (`@BehindFlag`) and scopes that take responsibility for the check (`@AssumesFlag`). Two Detekt rules enforce this: `InvalidFlagReference` (PSI-only, validates flag name strings in same file) and `UncheckedFlagAccess` (type resolution via BindingContext, cross-file call-site validation). + +**Tech Stack:** Kotlin, Detekt 1.23.8 (`detekt-api`, `detekt-test`), KMP `core` module, JVM `featured-detekt-rules` module. + +--- + +## File Map + +| File | Action | Purpose | +|---|---|---| +| `core/src/commonMain/kotlin/dev/androidbroadcast/featured/BehindFlag.kt` | Create | `@BehindFlag` annotation | +| `core/src/commonMain/kotlin/dev/androidbroadcast/featured/AssumesFlag.kt` | Create | `@AssumesFlag` annotation | +| `featured-detekt-rules/src/main/kotlin/dev/androidbroadcast/featured/detekt/InvalidFlagReference.kt` | Create | PSI-only rule — validates flag name strings | +| `featured-detekt-rules/src/main/kotlin/dev/androidbroadcast/featured/detekt/UncheckedFlagAccess.kt` | Create | Type-resolution rule — validates call sites cross-file | +| `featured-detekt-rules/src/main/kotlin/dev/androidbroadcast/featured/detekt/FeaturedRuleSetProvider.kt` | Modify | Register 2 new rules + update KDoc | +| `featured-detekt-rules/src/test/kotlin/dev/androidbroadcast/featured/detekt/InvalidFlagReferenceTest.kt` | Create | Tests for `InvalidFlagReference` | +| `featured-detekt-rules/src/test/kotlin/dev/androidbroadcast/featured/detekt/UncheckedFlagAccessTest.kt` | Create | Tests for `UncheckedFlagAccess` | + +--- + +## Task 1: `@BehindFlag` annotation + +**Files:** +- Create: `core/src/commonMain/kotlin/dev/androidbroadcast/featured/BehindFlag.kt` + +- [ ] **Step 1: Create `BehindFlag.kt`** + +```kotlin +package dev.androidbroadcast.featured + +/** + * Marks a function, class, or property that must only be used inside a valid feature-flag + * context — i.e., where the named flag has been checked before execution. + * + * ## Intended workflow + * + * 1. **Declare the flag** — introduce a `ConfigParam` annotated with `@LocalFlag` or + * `@RemoteFlag`. The `flagName` here must match the exact Kotlin property name. + * 2. **Annotate guarded code** — place `@BehindFlag("flagName")` on every function, class, + * or property that must only run when the flag is active. + * 3. **Guard call sites** — wrap every call site in an `if`/`when` that checks the flag, + * or annotate the containing function/class with `@AssumesFlag("flagName")`. + * 4. **Let Detekt enforce it** — the `UncheckedFlagAccess` rule (requires + * `detektWithTypeResolution`) reports any call site that lacks a valid guard. + * + * ## Usage + * + * ```kotlin + * @LocalFlag + * val newCheckout = ConfigParam("new_checkout", defaultValue = false) + * + * @BehindFlag("newCheckout") + * fun NewCheckoutScreen() { ... } + * + * // Call site must be guarded: + * if (configValues[newCheckout]) { + * NewCheckoutScreen() + * } + * ``` + * + * This annotation has [AnnotationRetention.SOURCE] retention — zero runtime overhead. + * + * @see AssumesFlag + */ +@Target(AnnotationTarget.CLASS, AnnotationTarget.FUNCTION, AnnotationTarget.PROPERTY) +@Retention(AnnotationRetention.SOURCE) +public annotation class BehindFlag( + /** + * The name of the Kotlin property (declared with `@LocalFlag` or `@RemoteFlag`) + * that guards this declaration. Must match the exact property name, e.g. `"newCheckout"`. + * + * Validated by the `InvalidFlagReference` Detekt rule within the same file. + */ + val flagName: String, +) +``` + +- [ ] **Step 2: Verify it compiles** + +```bash +./gradlew :core:compileKotlinJvm +``` +Expected: `BUILD SUCCESSFUL` + +- [ ] **Step 3: Commit** + +```bash +git add core/src/commonMain/kotlin/dev/androidbroadcast/featured/BehindFlag.kt +git commit -m "feat(core): add @BehindFlag annotation" +``` + +--- + +## Task 2: `@AssumesFlag` annotation + +**Files:** +- Create: `core/src/commonMain/kotlin/dev/androidbroadcast/featured/AssumesFlag.kt` + +- [ ] **Step 1: Create `AssumesFlag.kt`** + +```kotlin +package dev.androidbroadcast.featured + +/** + * Marks a function or class that takes explicit responsibility for ensuring the named feature + * flag is checked before execution reaches this scope. + * + * ## Purpose + * + * When a function or class always runs within a guarded context but cannot express that guard + * directly in its own body (e.g., a navigation host that conditionally renders flag-gated + * screens), annotate it with `@AssumesFlag` to suppress `UncheckedFlagAccess` warnings for + * call sites of `@BehindFlag`-annotated code inside this scope. + * + * ## Scope + * + * - On a **function**: suppresses warnings inside the function body. + * - On a **class**: suppresses warnings inside member functions and `init` blocks. + * Companion object members are **not** covered — they are a separate scope. + * + * ## ⚠️ Escape hatch + * + * This annotation is **not verified**. The Detekt rule trusts the annotation without + * checking that an actual flag guard exists inside the scope. Misuse silently bypasses + * `UncheckedFlagAccess`. Use it only when the calling context genuinely guarantees the + * flag is checked. + * + * ## Usage + * + * ```kotlin + * @AssumesFlag("newCheckout") + * fun CheckoutNavHost(configValues: ConfigValues) { + * // This function is only called when newCheckout is enabled upstream. + * NewCheckoutScreen() // no UncheckedFlagAccess warning here + * } + * ``` + * + * This annotation has [AnnotationRetention.SOURCE] retention — zero runtime overhead. + * + * @see BehindFlag + */ +@Target(AnnotationTarget.CLASS, AnnotationTarget.FUNCTION) +@Retention(AnnotationRetention.SOURCE) +public annotation class AssumesFlag( + /** + * The name of the feature flag property this scope guarantees is checked before execution. + * Must match the `flagName` of the corresponding `@BehindFlag` declaration. + */ + val flagName: String, +) +``` + +- [ ] **Step 2: Verify it compiles** + +```bash +./gradlew :core:compileKotlinJvm +``` +Expected: `BUILD SUCCESSFUL` + +- [ ] **Step 3: Commit** + +```bash +git add core/src/commonMain/kotlin/dev/androidbroadcast/featured/AssumesFlag.kt +git commit -m "feat(core): add @AssumesFlag annotation" +``` + +--- + +## Task 3: `InvalidFlagReference` rule + +PSI-only rule. No type resolution needed. Uses the same `rule.lint()` test pattern as all +existing rules in this module. + +**Files:** +- Create: `featured-detekt-rules/src/main/kotlin/dev/androidbroadcast/featured/detekt/InvalidFlagReference.kt` +- Create: `featured-detekt-rules/src/test/kotlin/dev/androidbroadcast/featured/detekt/InvalidFlagReferenceTest.kt` + +- [ ] **Step 1: Write the failing tests** + +```kotlin +// featured-detekt-rules/src/test/kotlin/dev/androidbroadcast/featured/detekt/InvalidFlagReferenceTest.kt +package dev.androidbroadcast.featured.detekt + +import io.gitlab.arturbosch.detekt.test.lint +import org.junit.Test +import kotlin.test.assertEquals + +class InvalidFlagReferenceTest { + private val rule = InvalidFlagReference() + + @Test + fun `no finding when BehindFlag matches LocalFlag property in same file`() { + val findings = rule.lint(""" + import dev.androidbroadcast.featured.BehindFlag + import dev.androidbroadcast.featured.LocalFlag + + @LocalFlag + val newCheckout = ConfigParam("new_checkout", false) + + @BehindFlag("newCheckout") + fun NewCheckoutScreen() {} + """.trimIndent()) + assertEquals(0, findings.size) + } + + @Test + fun `no finding when BehindFlag matches RemoteFlag property in same file`() { + val findings = rule.lint(""" + import dev.androidbroadcast.featured.BehindFlag + import dev.androidbroadcast.featured.RemoteFlag + + @RemoteFlag + val remoteFeature = ConfigParam("remote_feature", false) + + @BehindFlag("remoteFeature") + fun RemoteFeatureScreen() {} + """.trimIndent()) + assertEquals(0, findings.size) + } + + @Test + fun `reports finding when BehindFlag has typo in flag name`() { + val findings = rule.lint(""" + import dev.androidbroadcast.featured.BehindFlag + import dev.androidbroadcast.featured.LocalFlag + + @LocalFlag + val newCheckout = ConfigParam("new_checkout", false) + + @BehindFlag("newChekout") + fun NewCheckoutScreen() {} + """.trimIndent()) + assertEquals(1, findings.size) + } + + @Test + fun `reports finding when AssumesFlag references unknown flag on function`() { + // @LocalFlag must be present so knownFlags is non-empty; "unknown" is not in it + val findings = rule.lint(""" + import dev.androidbroadcast.featured.AssumesFlag + import dev.androidbroadcast.featured.LocalFlag + + @LocalFlag + val realFlag = ConfigParam("real_flag", false) + + @AssumesFlag("unknown") + fun SomeContainer() {} + """.trimIndent()) + assertEquals(1, findings.size) + } + + @Test + fun `reports finding when AssumesFlag references unknown flag on class`() { + // @LocalFlag must be present so knownFlags is non-empty; "unknown" is not in it + val findings = rule.lint(""" + import dev.androidbroadcast.featured.AssumesFlag + import dev.androidbroadcast.featured.LocalFlag + + @LocalFlag + val realFlag = ConfigParam("real_flag", false) + + @AssumesFlag("unknown") + class SomeViewModel {} + """.trimIndent()) + assertEquals(1, findings.size) + } + + @Test + fun `no finding when flag registry is in a different file`() { + // No @LocalFlag or @RemoteFlag in this snippet — rule must not false-positive + val findings = rule.lint(""" + import dev.androidbroadcast.featured.BehindFlag + + @BehindFlag("newCheckout") + fun NewCheckoutScreen() {} + """.trimIndent()) + assertEquals(0, findings.size) + } + + @Test + fun `no finding when BehindFlag annotation appears before LocalFlag declaration`() { + // Two-pass must handle ordering correctly + val findings = rule.lint(""" + import dev.androidbroadcast.featured.BehindFlag + import dev.androidbroadcast.featured.LocalFlag + + @BehindFlag("newCheckout") + fun NewCheckoutScreen() {} + + @LocalFlag + val newCheckout = ConfigParam("new_checkout", false) + """.trimIndent()) + assertEquals(0, findings.size) + } +} +``` + +- [ ] **Step 2: Run tests — verify they fail** + +```bash +./gradlew :featured-detekt-rules:test --tests "*.InvalidFlagReferenceTest" +``` +Expected: compilation error — `InvalidFlagReference` does not exist yet. + +- [ ] **Step 3: Create `InvalidFlagReference.kt`** + +```kotlin +// featured-detekt-rules/src/main/kotlin/dev/androidbroadcast/featured/detekt/InvalidFlagReference.kt +package dev.androidbroadcast.featured.detekt + +import io.gitlab.arturbosch.detekt.api.CodeSmell +import io.gitlab.arturbosch.detekt.api.Config +import io.gitlab.arturbosch.detekt.api.Debt +import io.gitlab.arturbosch.detekt.api.Entity +import io.gitlab.arturbosch.detekt.api.Issue +import io.gitlab.arturbosch.detekt.api.Rule +import io.gitlab.arturbosch.detekt.api.Severity +import org.jetbrains.kotlin.psi.KtAnnotationEntry +import org.jetbrains.kotlin.psi.KtFile +import org.jetbrains.kotlin.psi.KtProperty +import org.jetbrains.kotlin.psi.psiUtil.collectDescendantsOfType + +/** + * Warns when `@BehindFlag` or `@AssumesFlag` references a flag name that has no matching + * `@LocalFlag` or `@RemoteFlag` property in the same file. + * + * This catches typos in `flagName` at lint time. If the flag registry lives in a different + * file, the rule produces no warning (no false positives). + * + * **Non-compliant:** + * ```kotlin + * @BehindFlag("newChekout") // typo + * fun NewCheckoutScreen() {} + * ``` + * + * **Compliant:** + * ```kotlin + * @LocalFlag + * val newCheckout = ConfigParam("new_checkout", false) + * + * @BehindFlag("newCheckout") + * fun NewCheckoutScreen() {} + * ``` + */ +public class InvalidFlagReference( + config: Config = Config.empty, +) : Rule(config) { + + override val issue: Issue = Issue( + id = "InvalidFlagReference", + severity = Severity.Warning, + description = "@BehindFlag or @AssumesFlag references an unknown flag name.", + debt = Debt.FIVE_MINS, + ) + + override fun visitFile(file: KtFile) { + // Pass 1: collect @LocalFlag / @RemoteFlag property names in this file + val knownFlags = file.collectDescendantsOfType() + .filter { property -> + property.annotationEntries.any { + it.shortName?.asString() in setOf("LocalFlag", "RemoteFlag") + } + } + .mapNotNull { it.name } + .toSet() + + // No local flag declarations — nothing to validate against, skip to avoid false positives + if (knownFlags.isEmpty()) return + + // Pass 2: validate @BehindFlag / @AssumesFlag annotation arguments + file.collectDescendantsOfType() + .filter { it.shortName?.asString() in setOf("BehindFlag", "AssumesFlag") } + .forEach { annotation -> + val flagName = annotation.valueArguments + .firstOrNull() + ?.getArgumentExpression() + ?.text + ?.trim('"') + ?: return@forEach + + if (flagName !in knownFlags) { + report( + CodeSmell( + issue = issue, + entity = Entity.from(annotation), + message = "Flag name '$flagName' does not match any @LocalFlag or " + + "@RemoteFlag property in this file.", + ) + ) + } + } + } +} +``` + +- [ ] **Step 4: Run tests — verify they pass** + +```bash +./gradlew :featured-detekt-rules:test --tests "*.InvalidFlagReferenceTest" +``` +Expected: `BUILD SUCCESSFUL`, all tests green. + +- [ ] **Step 5: Commit** + +```bash +git add featured-detekt-rules/src/main/kotlin/dev/androidbroadcast/featured/detekt/InvalidFlagReference.kt \ + featured-detekt-rules/src/test/kotlin/dev/androidbroadcast/featured/detekt/InvalidFlagReferenceTest.kt +git commit -m "feat(detekt): add InvalidFlagReference rule" +``` + +--- + +## Task 4: `UncheckedFlagAccess` rule + +Requires type resolution. Tests use `rule.lintWithContext(env, code)` instead of `rule.lint()`. +`createEnvironment()` and `lintWithContext` are both in `detekt-test` 1.23.8 — no extra +dependency needed. + +**Files:** +- Create: `featured-detekt-rules/src/main/kotlin/dev/androidbroadcast/featured/detekt/UncheckedFlagAccess.kt` +- Create: `featured-detekt-rules/src/test/kotlin/dev/androidbroadcast/featured/detekt/UncheckedFlagAccessTest.kt` + +- [ ] **Step 1: Write the failing tests** + +```kotlin +// featured-detekt-rules/src/test/kotlin/dev/androidbroadcast/featured/detekt/UncheckedFlagAccessTest.kt +package dev.androidbroadcast.featured.detekt + +import io.gitlab.arturbosch.detekt.test.createEnvironment +import io.gitlab.arturbosch.detekt.test.lintWithContext +import org.junit.Test +import kotlin.test.assertEquals + +class UncheckedFlagAccessTest { + private val rule = UncheckedFlagAccess() + private val env = createEnvironment() + + // ── No findings ────────────────────────────────────────────────────────── + + @Test + fun `no finding for direct if check with bare flag reference`() { + val findings = rule.lintWithContext(env, """ + import dev.androidbroadcast.featured.BehindFlag + + @BehindFlag("newCheckout") + fun newCheckoutScreen() {} + + fun host(newCheckout: Boolean) { + if (newCheckout) { newCheckoutScreen() } + } + """.trimIndent()) + assertEquals(0, findings.size) + } + + @Test + fun `no finding for if check with array access pattern`() { + val findings = rule.lintWithContext(env, """ + import dev.androidbroadcast.featured.BehindFlag + + @BehindFlag("newCheckout") + fun newCheckoutScreen() {} + + fun host(configValues: Map, newCheckout: Any) { + if (configValues[newCheckout] == true) { newCheckoutScreen() } + } + """.trimIndent()) + assertEquals(0, findings.size) + } + + @Test + fun `no finding for if check with dot-qualified flag reference`() { + val findings = rule.lintWithContext(env, """ + import dev.androidbroadcast.featured.BehindFlag + + @BehindFlag("newCheckout") + fun newCheckoutScreen() {} + + class Flags { val newCheckout: Boolean = false } + + fun host(featureFlags: Flags) { + if (featureFlags.newCheckout) { newCheckoutScreen() } + } + """.trimIndent()) + assertEquals(0, findings.size) + } + + @Test + fun `no finding for when check with flag name in condition`() { + val findings = rule.lintWithContext(env, """ + import dev.androidbroadcast.featured.BehindFlag + + @BehindFlag("newCheckout") + fun newCheckoutScreen() {} + + fun host(newCheckout: Boolean) { + when { + newCheckout -> newCheckoutScreen() + } + } + """.trimIndent()) + assertEquals(0, findings.size) + } + + @Test + fun `no finding for call inside BehindFlag function same flag`() { + val findings = rule.lintWithContext(env, """ + import dev.androidbroadcast.featured.BehindFlag + + @BehindFlag("newCheckout") + fun newCheckoutScreen() {} + + @BehindFlag("newCheckout") + fun newCheckoutHost() { newCheckoutScreen() } + """.trimIndent()) + assertEquals(0, findings.size) + } + + @Test + fun `no finding for call inside AssumesFlag function same flag`() { + val findings = rule.lintWithContext(env, """ + import dev.androidbroadcast.featured.BehindFlag + import dev.androidbroadcast.featured.AssumesFlag + + @BehindFlag("newCheckout") + fun newCheckoutScreen() {} + + @AssumesFlag("newCheckout") + fun checkoutNavHost() { newCheckoutScreen() } + """.trimIndent()) + assertEquals(0, findings.size) + } + + @Test + fun `no finding for call inside AssumesFlag class member function`() { + val findings = rule.lintWithContext(env, """ + import dev.androidbroadcast.featured.BehindFlag + import dev.androidbroadcast.featured.AssumesFlag + + @BehindFlag("newCheckout") + fun newCheckoutScreen() {} + + @AssumesFlag("newCheckout") + class CheckoutContainer { + fun render() { newCheckoutScreen() } + } + """.trimIndent()) + assertEquals(0, findings.size) + } + + @Test + fun `no finding when BehindFlag is silent with BindingContext empty`() { + // Rule must not crash when run without type resolution + val findings = UncheckedFlagAccess().lint(""" + import dev.androidbroadcast.featured.BehindFlag + + @BehindFlag("newCheckout") + fun newCheckoutScreen() {} + + fun host() { newCheckoutScreen() } + """.trimIndent()) + assertEquals(0, findings.size) + } + + // ── Findings ───────────────────────────────────────────────────────────── + + @Test + fun `reports finding for call at top level without context`() { + val findings = rule.lintWithContext(env, """ + import dev.androidbroadcast.featured.BehindFlag + + @BehindFlag("newCheckout") + fun newCheckoutScreen() {} + + fun host() { newCheckoutScreen() } + """.trimIndent()) + assertEquals(1, findings.size) + } + + @Test + fun `reports finding for call inside BehindFlag function with different flag`() { + val findings = rule.lintWithContext(env, """ + import dev.androidbroadcast.featured.BehindFlag + + @BehindFlag("newCheckout") + fun newCheckoutScreen() {} + + @BehindFlag("otherFeature") + fun otherHost() { newCheckoutScreen() } + """.trimIndent()) + assertEquals(1, findings.size) + } + + @Test + fun `reports finding for call inside AssumesFlag function with different flag`() { + val findings = rule.lintWithContext(env, """ + import dev.androidbroadcast.featured.BehindFlag + import dev.androidbroadcast.featured.AssumesFlag + + @BehindFlag("newCheckout") + fun newCheckoutScreen() {} + + @AssumesFlag("otherFeature") + fun otherHost() { newCheckoutScreen() } + """.trimIndent()) + assertEquals(1, findings.size) + } + + @Test + fun `reports finding for constructor call without context`() { + val findings = rule.lintWithContext(env, """ + import dev.androidbroadcast.featured.BehindFlag + + @BehindFlag("newCheckout") + class NewCheckoutViewModel + + fun host() { val vm = NewCheckoutViewModel() } + """.trimIndent()) + assertEquals(1, findings.size) + } + + @Test + fun `no finding for constructor call inside valid if`() { + val findings = rule.lintWithContext(env, """ + import dev.androidbroadcast.featured.BehindFlag + + @BehindFlag("newCheckout") + class NewCheckoutViewModel + + fun host(newCheckout: Boolean) { + if (newCheckout) { val vm = NewCheckoutViewModel() } + } + """.trimIndent()) + assertEquals(0, findings.size) + } + + @Test + fun `reports finding for companion object member calling BehindFlag code despite class AssumesFlag`() { + val findings = rule.lintWithContext(env, """ + import dev.androidbroadcast.featured.BehindFlag + import dev.androidbroadcast.featured.AssumesFlag + + @BehindFlag("newCheckout") + fun newCheckoutScreen() {} + + @AssumesFlag("newCheckout") + class CheckoutContainer { + companion object { + fun create() { newCheckoutScreen() } // companion is excluded + } + } + """.trimIndent()) + assertEquals(1, findings.size) + } + + @Test + fun `reports finding for lambda capturing BehindFlag call`() { + val findings = rule.lintWithContext(env, """ + import dev.androidbroadcast.featured.BehindFlag + + @BehindFlag("newCheckout") + fun newCheckoutScreen() {} + + fun host() { + val action = { newCheckoutScreen() } // lambda is not a guarded context + } + """.trimIndent()) + assertEquals(1, findings.size) + } + + @Test + fun `reports finding for BehindFlag property access without context`() { + val findings = rule.lintWithContext(env, """ + import dev.androidbroadcast.featured.BehindFlag + + @BehindFlag("newCheckout") + val checkoutConfig: String = "config" + + fun host() { + val value = checkoutConfig // unguarded access + } + """.trimIndent()) + assertEquals(1, findings.size) + } + + @Test + fun `reports finding when call site has no guard — same compilation unit`() { + // NOTE: Detekt 1.23.8 lintWithContext accepts a single String only. + // True cross-file detection (declaration in module A, call in module B) cannot be + // unit-tested here. Verify cross-file behavior manually by: + // 1. Adding @BehindFlag to a function in :core or any other module + // 2. Calling it without a guard in :sample or :androidApp + // 3. Running: ./gradlew detektWithTypeResolutionCommonMain + // (or the target-specific variant for the call-site module) + // Expected: UncheckedFlagAccess warning reported for the call site. + val findings = rule.lintWithContext(env, """ + import dev.androidbroadcast.featured.BehindFlag + + @BehindFlag("newCheckout") + fun newCheckoutScreen() {} + + fun host() { newCheckoutScreen() } + """.trimIndent()) + assertEquals(1, findings.size) + } +} +``` + +- [ ] **Step 2: Run tests — verify they fail** + +```bash +./gradlew :featured-detekt-rules:test --tests "*.UncheckedFlagAccessTest" +``` +Expected: compilation error — `UncheckedFlagAccess` does not exist yet. + +- [ ] **Step 3: Create `UncheckedFlagAccess.kt`** + +```kotlin +// featured-detekt-rules/src/main/kotlin/dev/androidbroadcast/featured/detekt/UncheckedFlagAccess.kt +package dev.androidbroadcast.featured.detekt + +import io.gitlab.arturbosch.detekt.api.CodeSmell +import io.gitlab.arturbosch.detekt.api.Config +import io.gitlab.arturbosch.detekt.api.Debt +import io.gitlab.arturbosch.detekt.api.Entity +import io.gitlab.arturbosch.detekt.api.Issue +import io.gitlab.arturbosch.detekt.api.Rule +import io.gitlab.arturbosch.detekt.api.Severity +import org.jetbrains.kotlin.com.intellij.psi.PsiElement +import org.jetbrains.kotlin.com.intellij.psi.util.PsiTreeUtil +import org.jetbrains.kotlin.descriptors.DeclarationDescriptor +import org.jetbrains.kotlin.name.FqName +import org.jetbrains.kotlin.name.Name +import org.jetbrains.kotlin.psi.KtCallExpression +import org.jetbrains.kotlin.psi.KtClass +import org.jetbrains.kotlin.psi.KtIfExpression +import org.jetbrains.kotlin.psi.KtNameReferenceExpression +import org.jetbrains.kotlin.psi.KtNamedFunction +import org.jetbrains.kotlin.psi.KtObjectDeclaration +import org.jetbrains.kotlin.psi.KtWhenEntry +import org.jetbrains.kotlin.resolve.BindingContext +import org.jetbrains.kotlin.resolve.calls.util.getResolvedCall + +private val BEHIND_FLAG_FQN = FqName("dev.androidbroadcast.featured.BehindFlag") +private val ASSUMES_FLAG_FQN = FqName("dev.androidbroadcast.featured.AssumesFlag") +private val FLAG_NAME_PARAM = Name.identifier("flagName") + +/** + * Warns when a `@BehindFlag("X")`-annotated function, constructor, or property is used outside + * a valid feature-flag context. + * + * **Requires type resolution.** Run via `./gradlew detektWithTypeResolution` (or the + * target-specific variant for KMP: `detektWithTypeResolutionCommonMain`, etc.). + * When run without type resolution (`BindingContext.EMPTY`), the rule silently skips all + * checks to avoid false positives. + * + * **Valid contexts** (checked by walking up the PSI tree from the call site): + * - Enclosing `if`/`when` whose condition references the flag by name. + * - Enclosing function or class annotated `@BehindFlag("X")` for the same flag. + * - Enclosing function or class annotated `@AssumesFlag("X")` for the same flag. + * + * **Non-compliant:** + * ```kotlin + * @BehindFlag("newCheckout") + * fun NewCheckoutScreen() { ... } + * + * fun host() { NewCheckoutScreen() } // missing flag guard + * ``` + * + * **Compliant:** + * ```kotlin + * if (configValues[newCheckout]) { NewCheckoutScreen() } + * ``` + */ +public class UncheckedFlagAccess( + config: Config = Config.empty, +) : Rule(config) { + + override val issue: Issue = Issue( + id = "UncheckedFlagAccess", + severity = Severity.Warning, + description = "@BehindFlag-annotated code used outside a feature-flag guard.", + debt = Debt.TWENTY_MINS, + ) + + override fun visitCallExpression(expression: KtCallExpression) { + super.visitCallExpression(expression) + if (bindingContext == BindingContext.EMPTY) return + + val descriptor = expression.getResolvedCall(bindingContext) + ?.resultingDescriptor ?: return + val flagName = descriptor.behindFlagName() ?: return + + if (!expression.isInValidFlagContext(flagName)) { + report(CodeSmell( + issue = issue, + entity = Entity.from(expression), + message = "Call to '${descriptor.name}' is not guarded by flag '$flagName'. " + + "Wrap in if/when checking '$flagName', or annotate the containing scope " + + "with @BehindFlag(\"$flagName\") or @AssumesFlag(\"$flagName\").", + )) + } + } + + override fun visitNameReferenceExpression(expression: KtNameReferenceExpression) { + super.visitNameReferenceExpression(expression) + if (bindingContext == BindingContext.EMPTY) return + // Skip references that are the callee of a call expression — handled by visitCallExpression + if (expression.parent is KtCallExpression) return + + val target = bindingContext[BindingContext.REFERENCE_TARGET, expression] ?: return + val flagName = target.behindFlagName() ?: return + + if (!expression.isInValidFlagContext(flagName)) { + report(CodeSmell( + issue = issue, + entity = Entity.from(expression), + message = "Access to '${expression.getReferencedName()}' is not guarded by " + + "flag '$flagName'.", + )) + } + } + + // ── Helpers ────────────────────────────────────────────────────────────── + + private fun DeclarationDescriptor.behindFlagName(): String? = + annotations.findAnnotation(BEHIND_FLAG_FQN) + ?.allValueArguments + ?.get(FLAG_NAME_PARAM) + ?.value as? String + + private fun PsiElement.isInValidFlagContext(flagName: String): Boolean { + var node: PsiElement? = parent + while (node != null) { + when { + // if (...flagName...) { call() } + node is KtIfExpression && node.condition.containsFlagReference(flagName) -> return true + + // when { flagName -> { call() } } + node is KtWhenEntry && node.conditions.any { cond -> + cond.containsFlagReference(flagName) + } -> return true + + // Enclosing function with @BehindFlag("X") or @AssumesFlag("X") + node is KtNamedFunction && node.hasGuardAnnotation(flagName) -> return true + + // Enclosing class with @BehindFlag("X") or @AssumesFlag("X") + // but NOT if we crossed a companion object boundary + node is KtClass && node.hasGuardAnnotation(flagName) -> return true + + // Crossed into a companion object — class annotation does not cover this scope + node is KtObjectDeclaration && node.isCompanion() -> return false + } + node = node.parent + } + return false + } + + private fun PsiElement?.containsFlagReference(flagName: String): Boolean { + if (this == null) return false + return PsiTreeUtil.findChildrenOfType(this, KtNameReferenceExpression::class.java) + .any { it.getReferencedName() == flagName } + } + + private fun KtNamedFunction.hasGuardAnnotation(flagName: String): Boolean = + annotationEntries.any { it.matchesGuard(flagName) } + + private fun KtClass.hasGuardAnnotation(flagName: String): Boolean = + annotationEntries.any { it.matchesGuard(flagName) } + + private fun org.jetbrains.kotlin.psi.KtAnnotationEntry.matchesGuard(flagName: String): Boolean { + val name = shortName?.asString() ?: return false + if (name !in setOf("BehindFlag", "AssumesFlag")) return false + val value = valueArguments.firstOrNull() + ?.getArgumentExpression() + ?.text + ?.trim('"') ?: return false + return value == flagName + } +} +``` + +- [ ] **Step 4: Run tests — verify they pass** + +```bash +./gradlew :featured-detekt-rules:test --tests "*.UncheckedFlagAccessTest" +``` +Expected: `BUILD SUCCESSFUL`, all tests green. + +- [ ] **Step 5: Commit** + +```bash +git add featured-detekt-rules/src/main/kotlin/dev/androidbroadcast/featured/detekt/UncheckedFlagAccess.kt \ + featured-detekt-rules/src/test/kotlin/dev/androidbroadcast/featured/detekt/UncheckedFlagAccessTest.kt +git commit -m "feat(detekt): add UncheckedFlagAccess rule with type resolution" +``` + +--- + +## Task 5: Register rules and update `FeaturedRuleSetProvider` + +**Files:** +- Modify: `featured-detekt-rules/src/main/kotlin/dev/androidbroadcast/featured/detekt/FeaturedRuleSetProvider.kt` + +- [ ] **Step 1: Update `FeaturedRuleSetProvider.kt`** + +Replace the entire file with: + +```kotlin +package dev.androidbroadcast.featured.detekt + +import io.gitlab.arturbosch.detekt.api.Config +import io.gitlab.arturbosch.detekt.api.RuleSet +import io.gitlab.arturbosch.detekt.api.RuleSetProvider + +/** + * Registers the Featured custom Detekt rules under the `featured` rule set id. + * + * To enable in your project, add the artifact to Detekt's classpath and include + * the rule set in your `detekt.yml`: + * + * ```yaml + * featured: + * ExpiredFeatureFlag: + * active: true + * HardcodedFlagValue: + * active: true + * InvalidFlagReference: + * active: true + * MissingFlagAnnotation: + * active: true + * UncheckedFlagAccess: + * active: true # requires detektWithTypeResolution task + * ``` + * + * Note: `UncheckedFlagAccess` requires the `detektWithTypeResolution` Gradle task. + * It silently skips analysis when run under the plain `detekt` task. + */ +public class FeaturedRuleSetProvider : RuleSetProvider { + override val ruleSetId: String = "featured" + + override fun instance(config: Config): RuleSet = + RuleSet( + id = ruleSetId, + rules = listOf( + ExpiredFeatureFlagRule(config), + HardcodedFlagValueRule(config), + InvalidFlagReference(config), + MissingFlagAnnotationRule(config), + UncheckedFlagAccess(config), + ), + ) +} +``` + +- [ ] **Step 2: Run all detekt-rules tests** + +```bash +./gradlew :featured-detekt-rules:test +``` +Expected: `BUILD SUCCESSFUL`, all tests green. + +- [ ] **Step 3: Commit** + +```bash +git add featured-detekt-rules/src/main/kotlin/dev/androidbroadcast/featured/detekt/FeaturedRuleSetProvider.kt +git commit -m "feat(detekt): register InvalidFlagReference and UncheckedFlagAccess rules" +``` + +--- + +## Task 6: Update `detekt.yml` and run full check + +**Files:** +- Modify: project-root `detekt.yml` (wherever it lives — check `grep -r "ExpiredFeatureFlag" .`) + +- [ ] **Step 1: Find and update `detekt.yml`** + +```bash +grep -r "ExpiredFeatureFlag" /Users/krozov/dev/projects/Featured --include="*.yml" -l +``` + +Add under the `featured:` block: +```yaml + InvalidFlagReference: + active: true + UncheckedFlagAccess: + active: true # requires detektWithTypeResolution +``` + +- [ ] **Step 2: Run spotless + full build** + +```bash +./gradlew spotlessApply +./gradlew :core:build :featured-detekt-rules:build +``` +Expected: `BUILD SUCCESSFUL` + +- [ ] **Step 3: Run API compatibility check** + +```bash +./gradlew :core:apiCheck :featured-detekt-rules:apiCheck +``` +If it fails with "API dump is missing", run: +```bash +./gradlew :core:apiDump :featured-detekt-rules:apiDump +``` +Then re-check. Commit the updated `.api` files. + +- [ ] **Step 4: Final commit** + +```bash +git add core/api/ featured-detekt-rules/api/ +# Add detekt.yml if it was changed +git add $(git diff --name-only | grep detekt.yml) +git commit -m "chore: update detekt.yml and API dumps for @BehindFlag/@AssumesFlag" +``` diff --git a/docs/superpowers/plans/2026-03-23-featured-lint-rules.md b/docs/superpowers/plans/2026-03-23-featured-lint-rules.md new file mode 100644 index 0000000..f94195a --- /dev/null +++ b/docs/superpowers/plans/2026-03-23-featured-lint-rules.md @@ -0,0 +1,563 @@ +# featured-lint-rules Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Create a new `:featured-lint-rules` Gradle module with an Android Lint `HardcodedFlagValueDetector` that uses full UAST type resolution to flag direct `.defaultValue` access on `ConfigParam` with zero false positives. + +**Architecture:** Plain JVM module (`kotlinJvm` plugin), depends on `lint-api` as `compileOnly`. Detection uses `USimpleNameReferenceExpression` visitor with `JavaEvaluator.extendsClass` for precise type checking. Registry wired via JAR manifest attribute `Lint-Registry-v2`. + +**Tech Stack:** Kotlin, `com.android.tools.lint:lint-api:32.1.0`, `com.android.tools.lint:lint-tests:32.1.0`, JUnit 4 (via `lint-tests` transitive dep) + +--- + +## Pre-flight: Create a worktree + +Before touching any code, run: +```bash +git worktree add .worktrees/lint-rules -b feat/featured-lint-rules +cd .worktrees/lint-rules +``` +All subsequent work happens inside `.worktrees/lint-rules/`. + +--- + +## File Map + +| Action | Path | Responsibility | +|---|---|---| +| Modify | `gradle/libs.versions.toml` | Add `lint` version + `lint.api` / `lint.tests` library aliases | +| Modify | `settings.gradle.kts` | Register `:featured-lint-rules` module | +| Create | `featured-lint-rules/build.gradle.kts` | Module build config | +| Create | `featured-lint-rules/src/main/kotlin/dev/androidbroadcast/featured/lint/FeaturedIssueRegistry.kt` | `IssueRegistry` — declares vendor, api, minApi, issue list | +| Create | `featured-lint-rules/src/main/kotlin/dev/androidbroadcast/featured/lint/HardcodedFlagValueDetector.kt` | Detector + `ISSUE` companion | +| Create | `featured-lint-rules/src/test/kotlin/dev/androidbroadcast/featured/lint/HardcodedFlagValueDetectorTest.kt` | Full test suite | + +--- + +## Task 1: Add `lint` to the version catalog + +**Files:** +- Modify: `gradle/libs.versions.toml` + +- [ ] **Step 1: Add version and library entries** + +Open `gradle/libs.versions.toml`. In the `[versions]` block, add: +```toml +lint = "32.1.0" +``` +> Formula: `lint_major = agp_major + 23`. AGP is `9.1.0` → lint is `32.1.0`. +> If AGP patch ever bumps (e.g. 9.2.0), lint must move to 32.2.0 in lockstep. + +In the `[libraries]` block, add: +```toml +lint-api = { module = "com.android.tools.lint:lint-api", version.ref = "lint" } +lint-tests = { module = "com.android.tools.lint:lint-tests", version.ref = "lint" } +``` + +- [ ] **Step 2: Verify the catalog parses** + +```bash +./gradlew help --quiet +``` +Expected: no error about unresolved version refs. + +- [ ] **Step 3: Commit** + +```bash +git add gradle/libs.versions.toml +git commit -m "chore: add lint-api and lint-tests to version catalog" +``` + +--- + +## Task 2: Scaffold the module + +**Files:** +- Modify: `settings.gradle.kts` +- Create: `featured-lint-rules/build.gradle.kts` + +- [ ] **Step 1: Register module in settings** + +In `settings.gradle.kts`, append after the last `include(...)` line: +```kotlin +include(":featured-lint-rules") +``` + +- [ ] **Step 2: Create `build.gradle.kts`** + +Create `featured-lint-rules/build.gradle.kts` with: +```kotlin +plugins { + alias(libs.plugins.kotlinJvm) + alias(libs.plugins.bcv) + alias(libs.plugins.mavenPublish) +} + +kotlin { + explicitApi() + jvmToolchain(21) +} + +dependencies { + compileOnly(libs.lint.api) + testImplementation(libs.lint.tests) + testImplementation(libs.kotlin.testJunit) +} + +tasks.jar { + manifest { + attributes("Lint-Registry-v2" to "dev.androidbroadcast.featured.lint.FeaturedIssueRegistry") + } +} + +mavenPublishing { + publishToMavenCentral(com.vanniktech.maven.publish.SonatypeHost.CENTRAL_PORTAL) + signAllPublications() + coordinates( + groupId = "dev.androidbroadcast.featured", + artifactId = "featured-lint-rules", + ) + pom { + name.set("Featured Lint Rules") + description.set("Custom Android Lint rules for Featured – enforce correct feature flag usage") + inceptionYear.set("2024") + url.set("https://github.com/AndroidBroadcast/Featured") + licenses { + license { + name.set("The Apache Software License, Version 2.0") + url.set("https://www.apache.org/licenses/LICENSE-2.0.txt") + distribution.set("repo") + } + } + developers { + developer { + id.set("androidbroadcast") + name.set("Kirill Rozov") + url.set("https://github.com/androidbroadcast") + } + } + scm { + url.set("https://github.com/AndroidBroadcast/Featured") + connection.set("scm:git:git://github.com/AndroidBroadcast/Featured.git") + developerConnection.set("scm:git:ssh://git@github.com/AndroidBroadcast/Featured.git") + } + } +} +``` + +- [ ] **Step 3: Verify module is recognized** + +```bash +./gradlew :featured-lint-rules:help --quiet +``` +Expected: task runs without "Project ':featured-lint-rules' not found". + +- [ ] **Step 4: Commit** + +```bash +git add settings.gradle.kts featured-lint-rules/build.gradle.kts +git commit -m "chore: scaffold :featured-lint-rules module" +``` + +--- + +## Task 3: Write the failing tests + +**Files:** +- Create: `featured-lint-rules/src/test/kotlin/dev/androidbroadcast/featured/lint/HardcodedFlagValueDetectorTest.kt` + +The `lint-tests` library provides `LintDetectorTest` as the base class. You override `getDetector()` and `getIssues()`, then call `lint().files(...).run().expect(...)`. + +**Key constraint:** the type resolver needs a `ConfigParam` stub on the classpath. Without it, `evaluator.extendsClass` cannot resolve the type and the detector fires nothing. Every test that should produce a warning must include `configParamStub` in its `.files(...)` list. + +- [ ] **Step 1: Create the test file** + +```kotlin +package dev.androidbroadcast.featured.lint + +import com.android.tools.lint.checks.infrastructure.LintDetectorTest +import com.android.tools.lint.detector.api.Detector +import com.android.tools.lint.detector.api.Issue +import org.junit.Test + +class HardcodedFlagValueDetectorTest : LintDetectorTest() { + + override fun getDetector(): Detector = HardcodedFlagValueDetector() + + override fun getIssues(): List = listOf(HardcodedFlagValueDetector.ISSUE) + + // Minimal stub — primary constructor of the real ConfigParam is internal, + // so we provide a simplified version with matching val defaultValue: T. + // T : Any matches the real non-nullable upper bound. + private val configParamStub = kotlin( + """ + package dev.androidbroadcast.featured + class ConfigParam(val key: String, val defaultValue: T) + """, + ).indented() + + @Test + fun `reports defaultValue access on ConfigParam parameter`() { + lint() + .files( + configParamStub, + kotlin( + """ + import dev.androidbroadcast.featured.ConfigParam + + fun check(param: ConfigParam) { + if (param.defaultValue) println("on") + } + """, + ).indented(), + ) + .run() + .expectWarningCount(1) + } + + @Test + fun `reports defaultValue access on ConfigParam local variable`() { + lint() + .files( + configParamStub, + kotlin( + """ + import dev.androidbroadcast.featured.ConfigParam + + fun check() { + val flag = ConfigParam("flag", false) + println(flag.defaultValue) + } + """, + ).indented(), + ) + .run() + .expectWarningCount(1) + } + + @Test + fun `reports defaultValue on chained receiver`() { + lint() + .files( + configParamStub, + kotlin( + """ + import dev.androidbroadcast.featured.ConfigParam + + class Flags { + val darkMode = ConfigParam("dark_mode", false) + } + + fun check(flags: Flags) { + println(flags.darkMode.defaultValue) + } + """, + ).indented(), + ) + .run() + .expectWarningCount(1) + } + + @Test + fun `does not report defaultValue on String receiver`() { + lint() + .files( + configParamStub, + kotlin( + """ + fun check(s: String) { + println(s.defaultValue) + } + """, + ).indented(), + ) + .run() + .expectClean() + } + + @Test + fun `does not report access to other ConfigParam properties`() { + lint() + .files( + configParamStub, + kotlin( + """ + import dev.androidbroadcast.featured.ConfigParam + + fun check(param: ConfigParam) { + println(param.key) + } + """, + ).indented(), + ) + .run() + .expectClean() + } + + @Test + fun `does not report correct usage via ConfigValues`() { + lint() + .files( + configParamStub, + kotlin( + """ + import dev.androidbroadcast.featured.ConfigParam + + class ConfigValues { + operator fun get(param: ConfigParam): T = param.defaultValue + } + + fun check(configValues: ConfigValues, flag: ConfigParam) { + val enabled = configValues[flag] + println(enabled) + } + """, + ).indented(), + ) + .run() + .expectClean() + } + + @Test + fun `does not report when no ConfigParam stub on classpath`() { + // Sanity: without the stub, the type is unresolvable — detector stays silent. + lint() + .files( + kotlin( + """ + fun check(x: Any) { + println(x.defaultValue) + } + """, + ).indented(), + ) + .run() + .expectClean() + } +} +``` + +- [ ] **Step 2: Run tests — verify they fail to compile (detector not yet created)** + +```bash +./gradlew :featured-lint-rules:test 2>&1 | tail -20 +``` +Expected: compilation error — `HardcodedFlagValueDetector` not found. + +- [ ] **Step 3: Commit the failing tests** + +```bash +git add featured-lint-rules/src/test/ +git commit -m "test: add failing tests for HardcodedFlagValueDetector" +``` + +--- + +## Task 4: Implement `FeaturedIssueRegistry` (minimal — enough to compile) + +**Files:** +- Create: `featured-lint-rules/src/main/kotlin/dev/androidbroadcast/featured/lint/FeaturedIssueRegistry.kt` + +The registry must exist before the detector, because `lint-tests` loads the registry to find issues. We'll reference `HardcodedFlagValueDetector.ISSUE` which we create in Task 5. + +- [ ] **Step 1: Create the registry** + +```kotlin +package dev.androidbroadcast.featured.lint + +import com.android.tools.lint.client.api.IssueRegistry +import com.android.tools.lint.client.api.Vendor +import com.android.tools.lint.detector.api.CURRENT_API + +public class FeaturedIssueRegistry : IssueRegistry() { + + override val issues = listOf(HardcodedFlagValueDetector.ISSUE) + + override val api: Int = CURRENT_API + + // minApi = 10 allows AGP consumers on older lint hosts to load the registry. + // Setting it to CURRENT_API would silently drop all rules for older hosts. + override val minApi: Int = 10 + + override val vendor: Vendor = Vendor( + vendorName = "Featured", + feedbackUrl = "https://github.com/AndroidBroadcast/Featured/issues", + ) +} +``` + +- [ ] **Step 2: Commit** + +```bash +git add featured-lint-rules/src/main/kotlin/dev/androidbroadcast/featured/lint/FeaturedIssueRegistry.kt +git commit -m "chore: add FeaturedIssueRegistry scaffold" +``` + +--- + +## Task 5: Implement `HardcodedFlagValueDetector` + +**Files:** +- Create: `featured-lint-rules/src/main/kotlin/dev/androidbroadcast/featured/lint/HardcodedFlagValueDetector.kt` + +**Key UAST facts:** +- In Kotlin, `param.defaultValue` is a **property access**, not a function call. UAST models it as `USimpleNameReferenceExpression` inside a `UQualifiedReferenceExpression`. +- `UCallExpression` would only fire for Java-style `getDefaultValue()` — not what we want. +- `evaluator.extendsClass` takes a `PsiClass`, not a `PsiType`. You must unwrap: `(type as? PsiClassType)?.resolve()`. + +- [ ] **Step 1: Create the detector** + +```kotlin +package dev.androidbroadcast.featured.lint + +import com.android.tools.lint.client.api.UElementHandler +import com.android.tools.lint.detector.api.Category +import com.android.tools.lint.detector.api.Detector +import com.android.tools.lint.detector.api.Implementation +import com.android.tools.lint.detector.api.Issue +import com.android.tools.lint.detector.api.JavaContext +import com.android.tools.lint.detector.api.Scope +import com.android.tools.lint.detector.api.Severity +import com.intellij.psi.PsiClassType +import org.jetbrains.uast.UElement +import org.jetbrains.uast.UQualifiedReferenceExpression +import org.jetbrains.uast.USimpleNameReferenceExpression + +public class HardcodedFlagValueDetector : Detector(), Detector.UastScanner { + + public companion object { + public val ISSUE: Issue = Issue.create( + id = "HardcodedFlagValue", + briefDescription = "Accessing `ConfigParam.defaultValue` directly bypasses providers", + explanation = """ + Accessing `defaultValue` directly bypasses any local or remote provider \ + overrides, making the flag effectively hardcoded. \ + Use `ConfigValues` to read the live value instead. + """, + category = Category.CORRECTNESS, + priority = 6, + severity = Severity.WARNING, + implementation = Implementation( + HardcodedFlagValueDetector::class.java, + Scope.JAVA_FILE_SCOPE, + ), + ) + + private const val CONFIG_PARAM_FQN = "dev.androidbroadcast.featured.ConfigParam" + private const val DEFAULT_VALUE_PROPERTY = "defaultValue" + } + + override fun getApplicableUastTypes(): List> = + listOf(USimpleNameReferenceExpression::class.java) + + override fun createUastHandler(context: JavaContext): UElementHandler = + object : UElementHandler() { + override fun visitSimpleNameReferenceExpression(node: USimpleNameReferenceExpression) { + // Only care about references named "defaultValue" + if (node.identifier != DEFAULT_VALUE_PROPERTY) return + + // Must be the selector of a qualified expression: receiver.defaultValue + val parent = node.uastParent as? UQualifiedReferenceExpression ?: return + + // Resolve the receiver's type + val receiverType = parent.receiver.getExpressionType() as? PsiClassType ?: return + val receiverClass = receiverType.resolve() ?: return + + // Fire only when the receiver is ConfigParam or a subclass + if (!context.evaluator.extendsClass(receiverClass, CONFIG_PARAM_FQN, false)) return + + context.report( + issue = ISSUE, + scope = node, + location = context.getLocation(node), + message = "Accessing `defaultValue` directly on a `ConfigParam` bypasses " + + "provider overrides. Use `ConfigValues` to read the live value instead.", + ) + } + } +} +``` + +- [ ] **Step 2: Run the tests** + +```bash +./gradlew :featured-lint-rules:test +``` +Expected: all 6 tests pass, BUILD SUCCESSFUL. + +If tests fail: +- "expectWarningCount(1) but got 0" → type resolution failed; verify `configParamStub` is included in the failing test's `.files(...)`. +- "expectClean() but got 1 warning" → check that the receiver type is correctly unwrapped before calling `extendsClass`. + +- [ ] **Step 3: Commit** + +```bash +git add featured-lint-rules/src/main/ +git commit -m "feat: implement HardcodedFlagValueDetector with UAST type resolution" +``` + +--- + +## Task 6: Verify BCV API dump and full build + +Binary Compatibility Validator (`bcv`) tracks the public API surface. This module's public API is `HardcodedFlagValueDetector`, `HardcodedFlagValueDetector.ISSUE`, and `FeaturedIssueRegistry`. + +- [ ] **Step 1: Generate the API dump** + +```bash +./gradlew :featured-lint-rules:apiDump +``` +Expected: creates `featured-lint-rules/api/featured-lint-rules.api`. + +- [ ] **Step 2: Check the generated dump looks correct** + +```bash +cat featured-lint-rules/api/featured-lint-rules.api +``` +Expected: contains `HardcodedFlagValueDetector` and `FeaturedIssueRegistry` class entries. + +- [ ] **Step 3: Run the full module check** + +```bash +./gradlew :featured-lint-rules:check +``` +Expected: tests pass, API check passes, spotless passes. + +If spotless fails: run `./gradlew :featured-lint-rules:spotlessApply` then re-run `:check`. + +- [ ] **Step 4: Commit the API dump** + +```bash +git add featured-lint-rules/api/ +git commit -m "chore: add BCV API dump for featured-lint-rules" +``` + +--- + +## Task 7: Final integration smoke test + +Verify the JAR manifest is written correctly — this is what Lint uses to discover the registry. + +- [ ] **Step 1: Build the JAR and inspect the manifest** + +```bash +./gradlew :featured-lint-rules:jar +unzip -p featured-lint-rules/build/libs/featured-lint-rules-*.jar META-INF/MANIFEST.MF +``` +Expected output contains: +``` +Lint-Registry-v2: dev.androidbroadcast.featured.lint.FeaturedIssueRegistry +``` + +- [ ] **Step 2: Final commit** + +```bash +git add featured-lint-rules/ +git commit -m "feat: add :featured-lint-rules module with HardcodedFlagValue Lint check" +``` + +--- + +## Done + +At this point `:featured-lint-rules` is a working, tested, BCV-tracked Gradle module ready for publication. The next steps (porting remaining rules, adding `LintFix` suggestions) are tracked as out-of-scope in the design spec. diff --git a/docs/superpowers/specs/2026-03-22-agp9-migration-design.md b/docs/superpowers/specs/2026-03-22-agp9-migration-design.md new file mode 100644 index 0000000..6e10e40 --- /dev/null +++ b/docs/superpowers/specs/2026-03-22-agp9-migration-design.md @@ -0,0 +1,354 @@ +# AGP 9 Migration Design + +**Date:** 2026-03-22 +**Target:** Single PR — full AGP 9 support, no legacy mode as end state + +--- + +## Context + +The Featured project is a Kotlin Multiplatform (KMP) configuration management library supporting +Android, iOS (via SKIE), and JVM. + +### Current versions + +| Component | Current | Target | +|---|---|---| +| Android Gradle Plugin | 8.12.0 | 9.1.0 | +| Gradle wrapper | 8.14.3 | 9.1.0 | +| Kotlin | 2.3.10 | 2.3.10 (unchanged) | +| Dokka | 2.0.0 | 2.1.0 | +| SKIE | 0.10.10 | 0.10.10 (unchanged, compatibility risk — see below) | +| Compose Multiplatform | 1.9.3 | 1.9.3 (unchanged) | + +### Module inventory + +**KMP + Android library modules** (require plugin migration): +- `core` ⚠️ SKIE applied +- `featured-testing`, `featured-platform` +- `featured-compose`, `featured-debug-ui` +- `datastore-provider` ⚠️ SKIE applied +- `featured-registry` + +**Android-only library modules** (no plugin change — keep `com.android.library`): +- `firebase-provider` (uses Firebase BOM) +- `sharedpreferences-provider` + +**KMP sample app** (requires module restructuring): +- `sample` + +**Unaffected modules** (no Android dependency): +- `javaprefs-provider`, `nsuserdefaults-provider`, `featured-bom`, + `featured-detekt-rules`, `featured-gradle-plugin` + +--- + +## Migration strategy + +The PR lands full AGP 9 support. `android.enableLegacyVariantApi=true` is used **temporarily** +in Issue 1 so all parallel agents work in a compiling codebase — it is removed in Issue 6 before +the PR is merged. `android.builtInKotlin` is NOT set to false at any point: the +`com.android.kotlin.multiplatform.library` plugin requires `builtInKotlin=true` (the AGP 9 +default) to function. + +Exception: if a specific plugin (e.g. SKIE) cannot be migrated, document the blocker with an +inline comment and retain the minimal flag scoped to that module only. + +--- + +## Known risk: SKIE compatibility + +`co.touchlab.skie:gradle-plugin:0.10.10` is the latest stable release. SKIE hooks into the +Kotlin compiler plugin API and the Android Gradle Plugin lifecycle. Its compatibility with +`com.android.kotlin.multiplatform.library` (the AGP 9 KMP library plugin) has **not been +confirmed**. + +SKIE is applied to `core` (Issue 2) and `datastore-provider` (Issue 4). + +**Contingency:** if SKIE fails to initialise with the new plugin, those two modules retain +`com.android.library` + `androidTarget { }` and document the blocker inline: +```kotlin +// TODO: Migrate to com.android.kotlin.multiplatform.library once SKIE supports AGP 9 +// Tracking: https://github.com/touchlab/SKIE/issues/??? +``` +All other modules proceed with full migration. + +--- + +## Issue breakdown + +### Issue 1 — Foundation (sequential, must complete first) + +All other issues branch from this one. + +**Changes:** + +| File | Change | +|---|---| +| `gradle/wrapper/gradle-wrapper.properties` | `8.14.3` → `9.1.0` | +| `gradle/libs.versions.toml` | `agp = "8.12.0"` → `"9.1.0"` | +| `gradle/libs.versions.toml` | `dokka = "2.0.0"` → `"2.1.0"` | +| `gradle/libs.versions.toml` | Add `androidKmpLibrary` plugin alias | +| `build.gradle.kts` (root) | Add `alias(libs.plugins.androidKmpLibrary) apply false` | +| `gradle.properties` | Add `org.gradle.parallel=true` | +| `gradle.properties` | Add temporary `android.enableLegacyVariantApi=true` | + +**New version catalog entry:** +```toml +androidKmpLibrary = { id = "com.android.kotlin.multiplatform.library", version.ref = "agp" } +``` + +**Note on `android.builtInKotlin`:** do NOT set it to `false`. The AGP 9 default is `true`, +which is required by the new KMP library plugin. Android-only modules that still declare +`org.jetbrains.kotlin.android` are compatible with `builtInKotlin=true` — the plugin becomes +redundant rather than conflicting. + +**Verify:** run `./gradlew help` (configuration only) to confirm the project configures without +errors after version bumps. Build failures in module compilation are expected at this stage — they +will be resolved by Issues 2–5. + +**Dokka 2.1 note:** confirm that `org.jetbrains.dokka.experimental.gradle.pluginMode=V1Enabled` +remains a valid property in Dokka 2.1 (the property name and accepted values changed between +major Dokka releases). If not valid, update or remove the property as part of this issue. + +--- + +### Issues 2–5 — KMP module migrations (parallel, depend on Issue 1) + +Each issue migrates its module group from the old two-block DSL to the new unified `androidLibrary` +DSL. Android-only modules (`firebase-provider`, `sharedpreferences-provider`) are not in scope. + +#### Canonical DSL migration pattern + +**Before:** +```kotlin +plugins { + alias(libs.plugins.androidLibrary) // com.android.library + alias(libs.plugins.kotlinMultiplatform) +} + +android { + namespace = "dev.androidbroadcast.featured.example" + compileSdk = libs.versions.android.compileSdk.get().toInt() + defaultConfig { + minSdk = libs.versions.android.minSdk.get().toInt() + } + compileOptions { + sourceCompatibility = JavaVersion.VERSION_21 + targetCompatibility = JavaVersion.VERSION_21 + } +} + +kotlin { + androidTarget { + compilerOptions { + jvmTarget.set(JvmTarget.JVM_21) + } + } +} +``` + +**After:** +```kotlin +plugins { + alias(libs.plugins.androidKmpLibrary) // com.android.kotlin.multiplatform.library + alias(libs.plugins.kotlinMultiplatform) +} + +kotlin { + androidLibrary { + namespace = "dev.androidbroadcast.featured.example" + compileSdk = libs.versions.android.compileSdk.get().toInt() + minSdk = libs.versions.android.minSdk.get().toInt() + compilerOptions { + jvmTarget.set(JvmTarget.JVM_21) + } + } +} +``` + +Also rename test dependency configuration in every migrated module: +```kotlin +// Before +androidUnitTestImplementation(...) +// After +androidHostTestImplementation(...) +``` + +#### Issue 2 — `core`, `featured-testing`, `featured-platform` + +No Compose dependencies. Straightforward plugin + DSL swap. + +⚠️ **`core` carries SKIE.** Apply the SKIE contingency if the build fails after migration: +retain `com.android.library` + `androidTarget { }` for `core` only, document the blocker inline, +and proceed with `featured-testing` and `featured-platform`. + +#### Issue 3 — `featured-compose`, `featured-debug-ui` + +Same plugin + DSL swap. These modules carry Compose Multiplatform dependencies — verify Compose +source sets compile correctly after migration. + +#### Issue 4 — `datastore-provider`, `featured-registry` + +Same plugin + DSL swap. + +⚠️ **`datastore-provider` carries SKIE.** Apply the SKIE contingency if the build fails: +retain the old plugin for `datastore-provider` only, document inline, proceed with +`featured-registry`. + +#### Issue 5 — `sample` module restructuring + +AGP 9 requires separating the Android entry point from shared KMP code for application modules. + +**New structure:** +``` +sample/ # Shared KMP code — keeps existing KMP plugins, composeHotReload removed + src/commonMain/ + src/iosMain/ + ... + +androidApp/ # New module — Android entry point only + src/main/ + AndroidManifest.xml + kotlin/ + MainActivity.kt (moved from sample/src/androidMain/) + build.gradle.kts # com.android.application + org.jetbrains.kotlin.android + # + composeMultiplatform + composeCompiler + composeHotReload +``` + +**`iosApp/` is unaffected.** It references the XCFramework produced by the `sample` KMP module, +which continues to live in `sample/`. The XCFramework artifact name and path do not change. +`Package.swift` (if present at repo root) does not require updates. + +**`composeHotReload` moves to `androidApp/`.** It is a JVM/Android-only feature; remove it from +`sample/build.gradle.kts` and add it to `androidApp/build.gradle.kts`. + +`androidApp` declares a dependency on `sample`: +```kotlin +dependencies { + implementation(project(":sample")) +} +``` + +Add `androidApp` to `settings.gradle.kts`. + +--- + +### Issue 6 — Final cleanup + verification (sequential, depends on Issues 2–5) + +#### Remove temporary flag + +Remove from `gradle.properties`: +```properties +android.enableLegacyVariantApi=true +``` + +#### Android-only modules: built-in Kotlin + +`firebase-provider` and `sharedpreferences-provider` retain `org.jetbrains.kotlin.android` for +now — it is compatible with `android.builtInKotlin=true` (redundant, not conflicting) and +removing it would require migrating `kotlin { explicitApi() }` and `jvmToolchain(21)` to AGP +equivalents, which is out of scope for this PR. This is acceptable cleanup for a future PR. + +#### Firebase BOM and `android.dependency.useConstraints=false` + +`firebase-provider` uses `platform(libs.firebase.bom)`. Setting `useConstraints=false` disables +AGP's automatic constraint injection between configurations but does not affect how BOM-based +version resolution works within the module's own dependency graph. Firebase BOM resolution is +handled by Gradle's platform dependency mechanism, which is independent of AGP constraints. +This setting is safe for `firebase-provider`. + +#### Final `gradle.properties` + +Replace the entire file with explicit declarations — no implicit defaults: + +```properties +# Kotlin +kotlin.code.style=official +kotlin.daemon.jvmargs=-Xmx3072M + +# Gradle +org.gradle.jvmargs=-Xmx4096M -Dfile.encoding=UTF-8 +org.gradle.configuration-cache=true +org.gradle.caching=true +org.gradle.parallel=true + +# Android — General +android.useAndroidX=true +android.nonTransitiveRClass=true +android.uniquePackageNames=true +android.enableJetifier=false +android.sdk.defaultTargetSdkToCompileSdkIfUnset=true + +# Android — DSL & Kotlin +android.builtInKotlin=true +android.newDsl=true + +# Android — Dependencies +android.dependency.useConstraints=false + +# Android — Build Features +android.defaults.buildfeatures.resvalues=false + +# Android — Testing +android.default.androidx.test.runner=true +android.onlyEnableUnitTestForTheTestedBuildType=true + +# Android — R8 / ProGuard +android.enableR8.fullMode=true +android.r8.optimizedResourceShrinking=true +android.r8.strictInputValidation=true +android.proguard.failOnMissingFiles=true + +# Publishing +VERSION_NAME=0.1.0-SNAPSHOT + +# Dokka +org.jetbrains.dokka.experimental.gradle.pluginMode=V1Enabled +``` + +#### Build verification + +Run `./gradlew build`. Fix any errors. If a specific plugin blocks full migration, document the +blocker with an inline comment and keep only the minimum required flag scoped to that plugin. + +All tests must pass before the PR is raised. + +--- + +## Parallelisation plan + +``` +Issue 1 (Foundation — version bumps + temporary legacy flag) + ├── Issue 2 (core ⚠️SKIE, featured-testing, featured-platform) + ├── Issue 3 (featured-compose, featured-debug-ui) + ├── Issue 4 (datastore-provider ⚠️SKIE, featured-registry) + └── Issue 5 (sample restructuring → androidApp extraction) + └── Issue 6 (Final cleanup + gradle.properties + verification) +``` + +Issues 2–5 run in parallel worktrees after Issue 1 merges into the feature branch. +Issue 6 runs after Issues 2–5 are all merged. + +--- + +## Properties reference + +| Property | AGP 8 default | AGP 9 default | Value chosen | Reason | +|---|---|---|---|---| +| `android.builtInKotlin` | `false` | `true` | `true` | Required by new KMP plugin; Kotlin 2.3.10 via existing KGP classpath | +| `android.newDsl` | `false` | `true` | `true` | Full migration; no legacy DSL | +| `android.uniquePackageNames` | `false` | `true` | `true` | All modules already have unique namespaces | +| `android.enableLegacyVariantApi` | `false` | `false` | not set | Removed before merge; no legacy variant API used | +| `android.dependency.useConstraints` | `true` | `false` | `false` | Library; let consuming app decide; Firebase BOM unaffected | +| `android.onlyEnableUnitTestForTheTestedBuildType` | `false` | `true` | `true` | Reduces test task count across 10+ modules | +| `android.sdk.defaultTargetSdkToCompileSdkIfUnset` | `false` | `true` | `true` | Safe; all modules already declare explicit values | +| `android.defaults.buildfeatures.resvalues` | `true` | removed | `false` | No module uses resValues | +| `android.nonTransitiveRClass` | — | — | `true` | Already set; modern practice | +| `android.useAndroidX` | — | — | `true` | Already set | +| `android.enableJetifier` | — | — | `false` | Using AndroidX directly; no support libs | +| `android.enableR8.fullMode` | — | `true` | `true` | Full R8 optimisation | +| `android.r8.optimizedResourceShrinking` | `false` | `true` | `true` | Better resource shrinking | +| `android.r8.strictInputValidation` | `false` | `true` | `true` | Catch broken keep rules early | +| `android.proguard.failOnMissingFiles` | `false` | `true` | `true` | Fail fast on missing ProGuard files | +| `android.default.androidx.test.runner` | `false` | `true` | `true` | Standard AndroidX test runner | diff --git a/docs/superpowers/specs/2026-03-23-behind-flag-annotation-design.md b/docs/superpowers/specs/2026-03-23-behind-flag-annotation-design.md new file mode 100644 index 0000000..2778870 --- /dev/null +++ b/docs/superpowers/specs/2026-03-23-behind-flag-annotation-design.md @@ -0,0 +1,322 @@ +# Design: `@BehindFlag` / `@AssumesFlag` — Feature Flag Guard Annotations + +**Date:** 2026-03-23 +**Status:** Draft +**Modules affected:** `core`, `featured-detekt-rules` + +--- + +## Problem + +There is no static mechanism to enforce that code intended to run only when a feature flag is +enabled is actually guarded by a flag check at every call site. A developer can introduce a new +screen, class, or function behind a flag on their branch, but nothing prevents a colleague from +calling that code without checking the flag first — the mistake is invisible until runtime. + +In a KMP project, call sites and annotated declarations are routinely in different files and +modules. A same-file-only rule would cover fewer than 10% of real violations. + +--- + +## Goals + +1. Provide a way to annotate code that must only be used when a specific feature flag is active. +2. Provide a way to declare that a calling context already guarantees the flag is checked. +3. Catch violations at lint time (Detekt with type resolution), cross-file and cross-module. +4. Validate that flag name strings refer to real declared flags, catching typos at lint time. +5. Zero runtime overhead — SOURCE retention annotations only. + +--- + +## Non-Goals + +- Transitive call-chain analysis across arbitrary depth (only direct syntactic context). +- Tracking method calls on an already-constructed instance of an annotated class. +- Lambda / functional type analysis. +- Secondary constructor guarding (primary constructor only). +- Runtime enforcement. +- Detecting renamed boolean helpers (`if (isNewCheckoutEnabled())`) — only direct + `KtNameReferenceExpression` matching `flagName` in conditions is matched. + +--- + +## Annotations (`core` module) + +### `@BehindFlag` + +Marks a function, class, or property that must only be used inside a valid feature-flag context. + +```kotlin +@Target(AnnotationTarget.CLASS, AnnotationTarget.FUNCTION, AnnotationTarget.PROPERTY) +@Retention(AnnotationRetention.SOURCE) +public annotation class BehindFlag( + /** + * The name of the Kotlin property (declared with `@LocalFlag` or `@RemoteFlag`) + * that guards this declaration. Must match the exact property name, e.g. `"newCheckout"`. + * + * Validated by the [InvalidFlagReference] Detekt rule. + * + * See also: [AssumesFlag] + */ + val flagName: String, +) +``` + +**KDoc model:** Follow the style of `@ExpiresAt` — single-paragraph description, intended +workflow as a numbered list, a usage code block, `See also` cross-reference to `@AssumesFlag`. + +**Coverage note:** Annotation classes with no body produce no coverable lines and do not +affect the `core` module's ≥90% koverVerify requirement. + +--- + +### `@AssumesFlag` + +Marks a function or class that takes explicit responsibility for ensuring the named flag is +checked before execution. Call sites of `@BehindFlag("X")` code inside an `@AssumesFlag("X")` +scope are not warned by `UncheckedFlagAccess`. + +```kotlin +@Target(AnnotationTarget.CLASS, AnnotationTarget.FUNCTION) +@Retention(AnnotationRetention.SOURCE) +public annotation class AssumesFlag( + /** + * The name of the feature flag property this scope guarantees is checked before execution. + * Must match the `flagName` of the corresponding `@BehindFlag` declaration. + * + * **Escape hatch.** No automated verification that the flag is actually checked inside + * the scope. The developer asserts correctness. Misuse silently bypasses + * [UncheckedFlagAccess]. + * + * See also: [BehindFlag] + */ + val flagName: String, +) +``` + +**Scope of `@AssumesFlag` on CLASS:** covers member functions and `init` blocks. +Companion object members are excluded (separate `KtObjectDeclaration` in PSI). + +--- + +## Detekt Rules (`featured-detekt-rules` module) + +### `UncheckedFlagAccess` — requires type resolution + +**Severity:** Warning +**Debt:** `Debt.TWENTY_MINS` + +Fires when a call to `@BehindFlag("X")`-annotated code is found outside a valid context. + +#### Why type resolution is required + +The annotated declaration and its call site are typically in different files or modules in a +KMP project. PSI-only analysis can only match by name heuristic within a single file, which +covers fewer than 10% of real violations. `BindingContext` resolves any call expression to its +exact declaration, enabling precise cross-file, cross-module detection with no false positives +from name collisions. + +#### Gradle setup + +`UncheckedFlagAccess` must run under `detektWithTypeResolution` (not the plain `detekt` task). +For KMP projects, each compilation target has its own task: + +``` +./gradlew detektWithTypeResolutionAndroidMain +./gradlew detektWithTypeResolutionCommonMain +./gradlew detektWithTypeResolutionJvmMain +``` + +The rule must guard against running without type resolution. If +`bindingContext == BindingContext.EMPTY`, skip all checks and log a warning: + +```kotlin +override fun visitCallExpression(expression: KtCallExpression) { + if (bindingContext == BindingContext.EMPTY) return + // ... +} +``` + +#### Detection algorithm + +For each `KtCallExpression` and `KtNameReferenceExpression`: + +1. Resolve to declaration via `BindingContext`: + ```kotlin + val descriptor = expression.getResolvedCall(bindingContext) + ?.resultingDescriptor ?: return + ``` +2. Look up `@BehindFlag` on the descriptor: + ```kotlin + val annotation = descriptor.annotations + .findAnnotation(FqName("dev.androidbroadcast.featured.BehindFlag")) ?: return + val flagName = annotation.allValueArguments[Name.identifier("flagName")] + ?.value as? String ?: return + ``` +3. Walk up the PSI tree from the call site to find a valid context (see table below). +4. No valid context found → `report(CodeSmell(...))`. + +#### Valid contexts + +| Context | How it's detected | +|---|---| +| Direct `if` check | An enclosing `KtIfExpression` condition subtree contains a `KtNameReferenceExpression` with `getReferencedName() == flagName` (via `PsiTreeUtil.findChildrenOfType`) | +| Direct `when` check | An enclosing `KtWhenEntry` condition subtree contains a `KtNameReferenceExpression` with `getReferencedName() == flagName` | +| Propagated flag context | An enclosing `KtNamedFunction` or `KtClass` has `@BehindFlag("X")` with the same `flagName` (checked via PSI annotation entries — no BindingContext needed for the container) | +| Explicit assumption | An enclosing `KtNamedFunction` or `KtClass` has `@AssumesFlag("X")` with the same `flagName` (PSI annotation entries) | + +Condition matching scans the full condition subtree. This correctly handles all common patterns: + +```kotlin +if (newCheckout) { ... } // bare reference ✅ +if (configValues[newCheckout]) { ... } // array access ✅ +if (configValues.get(newCheckout)) { ... } // call argument ✅ +if (featureFlags.newCheckout) { ... } // dot-qualified ✅ +if (isNewCheckoutEnabled()) { ... } // renamed helper ❌ out of scope +``` + +#### Scope by annotation target + +| Target | What is checked | +|---|---| +| `FUNCTION` | Every `KtCallExpression` resolving to the annotated function | +| `CLASS` | Every `KtCallExpression` resolving to the primary constructor of the annotated class | +| `PROPERTY` | Every `KtNameReferenceExpression` resolving to the annotated property (via `BindingContext.REFERENCE_TARGET`) | + +Method calls on instances of an annotated class are **not** checked. +Secondary constructors are **not** checked. + +--- + +### `InvalidFlagReference` + +**Severity:** Warning +**Debt:** `Debt.FIVE_MINS` + +Fires when `@BehindFlag("X")` or `@AssumesFlag("X")` references a flag name with no matching +`@LocalFlag` or `@RemoteFlag` property in the same file. + +This rule is **PSI-only** (no type resolution required) and runs under the plain `detekt` task. +Cross-module flag registry validation is out of scope. Teams that co-locate flag declarations +with annotated code get full validation; others get no false positives. + +#### Algorithm (two-pass, per-file) + +```kotlin +override fun visitFile(file: KtFile) { + // Pass 1: collect @LocalFlag / @RemoteFlag property names in this file + val knownFlags = file.collectDescendantsOfType() + .filter { it.hasAnnotation("LocalFlag") || it.hasAnnotation("RemoteFlag") } + .mapNotNull { it.name } + .toSet() + + // Pass 2: validate @BehindFlag / @AssumesFlag annotation arguments + file.collectDescendantsOfType() + .filter { it.shortName?.asString() in setOf("BehindFlag", "AssumesFlag") } + .forEach { annotation -> + val flagName = annotation.valueArguments + .firstOrNull()?.getArgumentExpression()?.text?.trim('"') ?: return@forEach + if (flagName !in knownFlags) report(CodeSmell(...)) + } +} +``` + +Two passes avoid ordering sensitivity (annotation before declaration in same file). + +--- + +## `FeaturedRuleSetProvider` updates + +Register both new rules in `FeaturedRuleSetProvider.instance(config: Config)`. Update the +KDoc to include both rules in the `detekt.yml` example block. Note in the KDoc that +`UncheckedFlagAccess` requires `detektWithTypeResolution`. + +--- + +## Corner Cases + +| Situation | Behaviour | +|---|---| +| `@BehindFlag("A")` called inside `@BehindFlag("B")` context | ❌ Warning — different flags | +| Lambda: `val f = { NewCheckoutScreen() }` | ❌ Warning — not a guarded context | +| `@AssumesFlag` without actual flag check inside | ✅ No warning — escape hatch | +| `@AssumesFlag` on CLASS — member functions / `init` | ✅ No warning | +| `@AssumesFlag` on CLASS — companion object members | ❌ Warning — excluded from scope | +| Annotated class subclassed / interface implemented | Annotation not inherited | +| Secondary constructor of `@BehindFlag` class | Not checked — out of scope | +| `UncheckedFlagAccess` run without type resolution | Skips silently — logs warning | +| Call site and declaration in different files / modules | ✅ Detected via BindingContext | + +--- + +## File Map + +``` +core/ + src/commonMain/kotlin/dev/androidbroadcast/featured/ + BehindFlag.kt ← new + AssumesFlag.kt ← new + +featured-detekt-rules/ + src/main/kotlin/dev/androidbroadcast/featured/detekt/ + UncheckedFlagAccess.kt ← new (requires type resolution) + InvalidFlagReference.kt ← new (PSI-only) + FeaturedRuleSetProvider.kt ← register 2 new rules + update KDoc + + src/test/kotlin/dev/androidbroadcast/featured/detekt/ + UncheckedFlagAccessTest.kt ← new + InvalidFlagReferenceTest.kt ← new +``` + +--- + +## detekt.yml additions + +```yaml +featured: + UncheckedFlagAccess: + active: true # requires detektWithTypeResolution task + InvalidFlagReference: + active: true # runs under plain detekt task +``` + +--- + +## Testing Requirements + +**`UncheckedFlagAccessTest`** + +Tests use `LintTestRule` with `KotlinCoreEnvironment` to enable type resolution. + +| Scenario | Expected | +|---|---| +| `if (newCheckout)` before call | No finding | +| `if (configValues[newCheckout])` before call | No finding | +| `if (featureFlags.newCheckout)` before call | No finding | +| `when` with flag name in condition | No finding | +| Call inside `@BehindFlag("X")` function, same flag | No finding | +| Call inside `@AssumesFlag("X")` function, same flag | No finding | +| Call inside `@AssumesFlag("X")` class body, same flag | No finding | +| `@BehindFlag` code calling other `@BehindFlag` code, same flag | No finding | +| Declaration in file A, call site in file B (cross-file) | Finding | +| Call at top level, no context | Finding | +| Call inside `@BehindFlag("Y")`, different flag | Finding | +| Call inside `@AssumesFlag("Y")`, different flag | Finding | +| Lambda: `val f = { NewCheckoutScreen() }` | Finding | +| `@BehindFlag` on class: constructor called without context | Finding | +| `@BehindFlag` on class: constructor inside valid `if` | No finding | +| `@BehindFlag` on property: access without context | Finding | +| `@AssumesFlag` on class: companion object member calls flagged code | Finding | +| Rule run with `BindingContext.EMPTY` (no type resolution) | No finding, no crash | + +**`InvalidFlagReferenceTest`** + +| Scenario | Expected | +|---|---| +| `@BehindFlag("newCheckout")` + `@LocalFlag val newCheckout` same file | No finding | +| `@BehindFlag("newChekout")` typo, correct property present | Finding | +| `@AssumesFlag("unknown")` on function | Finding | +| `@AssumesFlag("unknown")` on class | Finding | +| `@BehindFlag("remoteFlag")` + `@RemoteFlag val remoteFlag` same file | No finding | +| Flag registry in a different file, no local declaration | No finding — no false positive | +| `@BehindFlag` appears before `@LocalFlag` declaration in same file | No finding — two-pass | diff --git a/docs/superpowers/specs/2026-03-23-featured-lint-rules-design.md b/docs/superpowers/specs/2026-03-23-featured-lint-rules-design.md new file mode 100644 index 0000000..e250b0c --- /dev/null +++ b/docs/superpowers/specs/2026-03-23-featured-lint-rules-design.md @@ -0,0 +1,206 @@ +# Design Spec: `featured-lint-rules` — Android Lint Pilot + +**Date:** 2026-03-23 +**Status:** Approved +**Scope:** Pilot — `HardcodedFlagValue` detector only + +--- + +## Background + +The project already ships `featured-detekt-rules` with three rules: +`MissingFlagAnnotation`, `HardcodedFlagValue`, and `ExpiredFeatureFlag`. + +All Detekt rules rely on **name-based heuristics** because Detekt runs without +full type resolution. The comment in `PsiExtensions.kt` calls this out explicitly: + +> "Detection is heuristic (name-based) because Detekt rules run without full +> type resolution in the default lint mode." + +Android Lint runs UAST with the full compiled classpath, enabling exact type +checks via `JavaEvaluator`. The goal is a parallel `featured-lint-rules` module +that provides the same rules with zero false positives. + +The pilot implements `HardcodedFlagValue` — the rule most affected by Detekt's +heuristic limitation (any `x.defaultValue` on any simple name fires). + +--- + +## Code Sharing Between Detekt and Lint + +Detekt and Android Lint use **different AST models** — Detekt works on raw +Kotlin PSI (`KtElement` hierarchy), while Lint works on UAST (`UElement` +hierarchy). Detection logic cannot be shared. + +What can be shared: +- **Issue IDs** — `"HardcodedFlagValue"` must be identical in both so that + `@Suppress("HardcodedFlagValue")` works for both tools. This is enforced by + convention (same string literal), not by a shared constant module. Given the + small number of rules, a dedicated shared module adds complexity without + meaningful benefit. +- **Test fixture strings** — both test suites use inline Kotlin snippets as + strings. These could theoretically live in a shared `testFixtures` source set, + but again, the added module boundary is not worth it for a handful of strings. + +**Decision:** no shared module. Duplication is intentional and minimal. + +--- + +## Module Structure + +New Gradle module `:featured-lint-rules`, mirroring `:featured-detekt-rules`: + +``` +featured-lint-rules/ +├── build.gradle.kts +└── src/ + ├── main/kotlin/dev/androidbroadcast/featured/lint/ + │ ├── HardcodedFlagValueDetector.kt + │ └── FeaturedIssueRegistry.kt + └── test/kotlin/dev/androidbroadcast/featured/lint/ + └── HardcodedFlagValueDetectorTest.kt +``` + +`settings.gradle.kts` gains `include(":featured-lint-rules")`. + +### Build configuration + +- Plugins: `alias(libs.plugins.kotlinJvm)`, `alias(libs.plugins.bcv)`, + `alias(libs.plugins.mavenPublish)` — same set as `:featured-detekt-rules` +- Dependencies: + - `compileOnly(libs.lint.api)` — `com.android.tools.lint:lint-api:32.1.0` + - `testImplementation(libs.lint.tests)` — `com.android.tools.lint:lint-tests:32.1.0` +- Version catalog: add `lint` version alias `32.1.0`. + Formula: lint_major = agp_major + 23, patch tracks AGP patch exactly + (AGP 9.1.0 → lint 32.1.0; if AGP bumps to 9.2.0, lint must move to 32.2.0). +- `explicitApi()` + `jvmToolchain(21)` matching the rest of the project +- `mavenPublishing` block with `artifactId = "featured-lint-rules"` +- The `com.android.lint` Gradle plugin is **not required** — the JAR manifest + entry `Lint-Registry-v2` is written manually in `build.gradle.kts` via a + `tasks.jar { manifest { ... } }` block, which is the standard approach for + non-Android modules. + +--- + +## Detector Design: `HardcodedFlagValueDetector` + +### Issue definition + +| Field | Value | +|---|---| +| ID | `HardcodedFlagValue` | +| Category | `Correctness` | +| Severity | `WARNING` | +| Priority | 6 / 10 | +| Brief description | Accessing `ConfigParam.defaultValue` directly bypasses providers | +| Explanation | Accessing `defaultValue` directly bypasses any local or remote provider overrides, making the flag effectively hardcoded. Use `ConfigValues` to read the live value. | + +The ID matches the Detekt rule so `@Suppress("HardcodedFlagValue")` silences +both tools with one annotation. + +### Detection logic + +In Kotlin, `param.defaultValue` is a **property access**, not a function call. +UAST models it as a `USimpleNameReferenceExpression` with identifier +`"defaultValue"` inside a `UQualifiedReferenceExpression`, **not** as a +`UCallExpression`. Using `UCallExpression` would only catch explicit Java-style +`getDefaultValue()` calls, which no Kotlin caller would ever write. + +Correct approach — `SourceCodeScanner` with: + +``` +getApplicableUastTypes() = listOf(USimpleNameReferenceExpression::class.java) + +visitSimpleNameReferenceExpression(node: USimpleNameReferenceExpression) + └─ node.identifier == "defaultValue"? → proceed + └─ parent is UQualifiedReferenceExpression? → get receiver expression + └─ val psiType = receiver.getExpressionType() as? PsiClassType ?: return + └─ val psiClass = psiType.resolve() ?: return + └─ evaluator.extendsClass(psiClass, "dev.androidbroadcast.featured.ConfigParam", false) + └─ context.report(ISSUE, node, context.getLocation(node), message) +``` + +This handles both simple receivers (`param.defaultValue`) and chained receivers +(`flags.darkMode.defaultValue`) identically — in both cases the visitor fires +on the `defaultValue` name node, and the receiver is resolved via +`getExpressionType()` on the parent `UQualifiedReferenceExpression`'s receiver. + +### Message + +``` +Accessing 'defaultValue' directly on a ConfigParam bypasses provider overrides. +Use ConfigValues to read the live value instead. +``` + +### `FeaturedIssueRegistry` + +```kotlin +class FeaturedIssueRegistry : IssueRegistry() { + override val issues = listOf(HardcodedFlagValueDetector.ISSUE) + override val api = CURRENT_API + // minApi must be lower than api to allow older AGP consumers to load the registry. + // 10 is the stable minimum that supports the Vendor API. + override val minApi = 10 + override val vendor = Vendor( + vendorName = "Featured", + feedbackUrl = "https://github.com/AndroidBroadcast/Featured/issues", + ) +} +``` + +JAR manifest entry (in `build.gradle.kts`): +```kotlin +tasks.jar { + manifest { + attributes("Lint-Registry-v2" to "dev.androidbroadcast.featured.lint.FeaturedIssueRegistry") + } +} +``` + +--- + +## Test Design + +Uses `lint-tests` (`LintDetectorTest` base class). + +Every test provides a `ConfigParam` stub so the type resolver can resolve the +type. The stub must declare `T : Any` (non-nullable upper bound matching the +real class) and include `val defaultValue: T`. The real `ConfigParam` primary +constructor is `internal`, so the stub omits the full constructor and uses a +minimal form sufficient for type-checking: + +```kotlin +private val configParamStub = kotlin(""" + package dev.androidbroadcast.featured + class ConfigParam(val key: String, val defaultValue: T) +""").indented() +``` + +Tests that call `ConfigParam(key = "x", defaultValue = false)` must ensure +the stub constructor matches. If constructor shape diverges from the real API, +prefer testing via local variable declarations with explicit types: +```kotlin +val flag: ConfigParam = TODO() +flag.defaultValue +``` + +### Test cases + +| # | Scenario | Expected | +|---|---|---| +| 1 | `someParam.defaultValue` where `someParam: ConfigParam` | 1 warning | +| 2 | `someParam.defaultValue` where `someParam: String` | 0 warnings | +| 3 | `configValues[flag]` (correct usage via `ConfigValues`) | 0 warnings | +| 4 | Chained: `flags.darkMode.defaultValue` where `darkMode: ConfigParam` | 1 warning | +| 5 | `defaultValue` on a `ConfigParam` method parameter | 1 warning | +| 6 | `defaultValue` accessed on a local variable of unrelated type | 0 warnings | + +--- + +## Out of Scope (this spec) + +- Porting `MissingFlagAnnotation` and `ExpiredFeatureFlag` to Lint +- QuickFix / `LintFix` suggestions +- Baseline file integration + +These will be addressed in follow-up specs once the pilot module is validated. diff --git a/featured-lint-rules/api/featured-lint-rules.api b/featured-lint-rules/api/featured-lint-rules.api new file mode 100644 index 0000000..8adcb8c --- /dev/null +++ b/featured-lint-rules/api/featured-lint-rules.api @@ -0,0 +1,19 @@ +public final class dev/androidbroadcast/featured/lint/FeaturedIssueRegistry : com/android/tools/lint/client/api/IssueRegistry { + public fun ()V + public fun getApi ()I + public fun getIssues ()Ljava/util/List; + public fun getMinApi ()I + public fun getVendor ()Lcom/android/tools/lint/client/api/Vendor; +} + +public final class dev/androidbroadcast/featured/lint/HardcodedFlagValueDetector : com/android/tools/lint/detector/api/Detector, com/android/tools/lint/detector/api/Detector$UastScanner { + public static final field Companion Ldev/androidbroadcast/featured/lint/HardcodedFlagValueDetector$Companion; + public fun ()V + public fun createUastHandler (Lcom/android/tools/lint/detector/api/JavaContext;)Lcom/android/tools/lint/client/api/UElementHandler; + public fun getApplicableUastTypes ()Ljava/util/List; +} + +public final class dev/androidbroadcast/featured/lint/HardcodedFlagValueDetector$Companion { + public final fun getISSUE ()Lcom/android/tools/lint/detector/api/Issue; +} + diff --git a/featured-lint-rules/build.gradle.kts b/featured-lint-rules/build.gradle.kts new file mode 100644 index 0000000..a489429 --- /dev/null +++ b/featured-lint-rules/build.gradle.kts @@ -0,0 +1,58 @@ +plugins { + alias(libs.plugins.kotlinJvm) + alias(libs.plugins.bcv) + alias(libs.plugins.mavenPublish) +} + +kotlin { + explicitApi() + jvmToolchain(21) +} + +dependencies { + compileOnly(libs.lint.api) + // lint-tests does not transitively expose lint-api on the test classpath + testImplementation(libs.lint.api) + testImplementation(libs.lint.tests) + testImplementation(libs.kotlin.testJunit) +} + +tasks.jar { + manifest { + attributes("Lint-Registry-v2" to "dev.androidbroadcast.featured.lint.FeaturedIssueRegistry") + } +} + +mavenPublishing { + publishToMavenCentral(com.vanniktech.maven.publish.SonatypeHost.CENTRAL_PORTAL) + signAllPublications() + coordinates( + groupId = "dev.androidbroadcast.featured", + artifactId = "featured-lint-rules", + ) + pom { + name.set("Featured Lint Rules") + description.set("Custom Android Lint rules for Featured – enforce correct feature flag usage") + inceptionYear.set("2024") + url.set("https://github.com/AndroidBroadcast/Featured") + licenses { + license { + name.set("The Apache Software License, Version 2.0") + url.set("https://www.apache.org/licenses/LICENSE-2.0.txt") + distribution.set("repo") + } + } + developers { + developer { + id.set("androidbroadcast") + name.set("Kirill Rozov") + url.set("https://github.com/androidbroadcast") + } + } + scm { + url.set("https://github.com/AndroidBroadcast/Featured") + connection.set("scm:git:git://github.com/AndroidBroadcast/Featured.git") + developerConnection.set("scm:git:ssh://git@github.com/AndroidBroadcast/Featured.git") + } + } +} diff --git a/featured-lint-rules/src/main/kotlin/dev/androidbroadcast/featured/lint/FeaturedIssueRegistry.kt b/featured-lint-rules/src/main/kotlin/dev/androidbroadcast/featured/lint/FeaturedIssueRegistry.kt new file mode 100644 index 0000000..8b9fb31 --- /dev/null +++ b/featured-lint-rules/src/main/kotlin/dev/androidbroadcast/featured/lint/FeaturedIssueRegistry.kt @@ -0,0 +1,29 @@ +package dev.androidbroadcast.featured.lint + +import com.android.tools.lint.client.api.IssueRegistry +import com.android.tools.lint.client.api.Vendor +import com.android.tools.lint.detector.api.CURRENT_API +import com.android.tools.lint.detector.api.Issue + +/** + * Registers all Featured Android Lint rules with the Lint infrastructure. + * + * Discovered via the `Lint-Registry-v2` JAR manifest attribute. + * [minApi] is set to 10 (not [CURRENT_API]) so the registry loads on older AGP/Lint + * hosts without silently dropping all rules. + */ +public class FeaturedIssueRegistry : IssueRegistry() { + override val issues: List = listOf(HardcodedFlagValueDetector.ISSUE) + + override val api: Int = CURRENT_API + + // minApi = 10 allows AGP consumers on older lint hosts to load the registry. + // Setting it to CURRENT_API would silently drop all rules for older hosts. + override val minApi: Int = 10 + + override val vendor: Vendor = + Vendor( + vendorName = "Featured", + feedbackUrl = "https://github.com/AndroidBroadcast/Featured/issues", + ) +} diff --git a/featured-lint-rules/src/main/kotlin/dev/androidbroadcast/featured/lint/HardcodedFlagValueDetector.kt b/featured-lint-rules/src/main/kotlin/dev/androidbroadcast/featured/lint/HardcodedFlagValueDetector.kt new file mode 100644 index 0000000..617c3a9 --- /dev/null +++ b/featured-lint-rules/src/main/kotlin/dev/androidbroadcast/featured/lint/HardcodedFlagValueDetector.kt @@ -0,0 +1,75 @@ +package dev.androidbroadcast.featured.lint + +import com.android.tools.lint.client.api.UElementHandler +import com.android.tools.lint.detector.api.Category +import com.android.tools.lint.detector.api.Detector +import com.android.tools.lint.detector.api.Implementation +import com.android.tools.lint.detector.api.Issue +import com.android.tools.lint.detector.api.JavaContext +import com.android.tools.lint.detector.api.Scope +import com.android.tools.lint.detector.api.Severity +import com.intellij.psi.PsiClassType +import org.jetbrains.uast.UElement +import org.jetbrains.uast.UQualifiedReferenceExpression +import org.jetbrains.uast.USimpleNameReferenceExpression + +public class HardcodedFlagValueDetector : + Detector(), + Detector.UastScanner { + public companion object { + public val ISSUE: Issue = + Issue.create( + id = "HardcodedFlagValue", + briefDescription = "Accessing `ConfigParam.defaultValue` directly bypasses providers", + explanation = """ + Accessing `defaultValue` directly bypasses any local or remote provider \ + overrides, making the flag effectively hardcoded. \ + Use `ConfigValues` to read the live value instead. + """, + category = Category.CORRECTNESS, + priority = 6, + severity = Severity.WARNING, + implementation = + Implementation( + HardcodedFlagValueDetector::class.java, + Scope.JAVA_FILE_SCOPE, + ), + ) + + private const val CONFIG_PARAM_FQN = "dev.androidbroadcast.featured.ConfigParam" + private const val DEFAULT_VALUE_PROPERTY = "defaultValue" + } + + override fun getApplicableUastTypes(): List> = listOf(USimpleNameReferenceExpression::class.java) + + override fun createUastHandler(context: JavaContext): UElementHandler = + object : UElementHandler() { + override fun visitSimpleNameReferenceExpression(node: USimpleNameReferenceExpression) { + // Only care about references named "defaultValue" + if (node.identifier != DEFAULT_VALUE_PROPERTY) return + + // Must be the selector of a qualified expression: receiver.defaultValue + // Guard: node must be the selector, not the receiver, to avoid false positives + // when a ConfigParam-typed variable is itself named "defaultValue" and used as a + // receiver (e.g. `defaultValue.key`). + val parent = node.uastParent as? UQualifiedReferenceExpression ?: return + if (parent.selector != node) return + + // Resolve the receiver's type + val receiverType = parent.receiver.getExpressionType() as? PsiClassType ?: return + val receiverClass = receiverType.resolve() ?: return + + // Fire only when the receiver is ConfigParam or a subclass + if (!context.evaluator.extendsClass(receiverClass, CONFIG_PARAM_FQN, false)) return + + context.report( + issue = ISSUE, + scope = node, + location = context.getLocation(node), + message = + "Accessing `defaultValue` directly on a `ConfigParam` bypasses " + + "provider overrides. Use `ConfigValues` to read the live value instead.", + ) + } + } +} diff --git a/featured-lint-rules/src/test/kotlin/dev/androidbroadcast/featured/lint/HardcodedFlagValueDetectorTest.kt b/featured-lint-rules/src/test/kotlin/dev/androidbroadcast/featured/lint/HardcodedFlagValueDetectorTest.kt new file mode 100644 index 0000000..658f086 --- /dev/null +++ b/featured-lint-rules/src/test/kotlin/dev/androidbroadcast/featured/lint/HardcodedFlagValueDetectorTest.kt @@ -0,0 +1,191 @@ +package dev.androidbroadcast.featured.lint + +import com.android.tools.lint.checks.infrastructure.LintDetectorTest +import com.android.tools.lint.detector.api.Detector +import com.android.tools.lint.detector.api.Issue +import org.junit.Test +import org.junit.runner.RunWith +import org.junit.runners.JUnit4 + +@RunWith(JUnit4::class) +class HardcodedFlagValueDetectorTest : LintDetectorTest() { + override fun getDetector(): Detector = HardcodedFlagValueDetector() + + override fun getIssues(): List = listOf(HardcodedFlagValueDetector.ISSUE) + + // Minimal stub — primary constructor of the real ConfigParam is internal, + // so we provide a simplified version with matching val defaultValue: T. + // T : Any matches the real non-nullable upper bound. + private val configParamStub = + kotlin( + """ + package dev.androidbroadcast.featured + class ConfigParam(val key: String, val defaultValue: T) + """, + ).indented() + + @Test + fun `reports defaultValue access on ConfigParam parameter`() { + lint() + .files( + configParamStub, + kotlin( + """ + import dev.androidbroadcast.featured.ConfigParam + + fun check(param: ConfigParam) { + if (param.defaultValue) println("on") + } + """, + ).indented(), + ).run() + .expectWarningCount(1) + } + + @Test + fun `reports defaultValue access on ConfigParam local variable`() { + lint() + .files( + configParamStub, + kotlin( + """ + import dev.androidbroadcast.featured.ConfigParam + + fun check(flag: ConfigParam) { + val value: Boolean = flag.defaultValue + println(value) + } + """, + ).indented(), + ).run() + .expectWarningCount(1) + } + + @Test + fun `reports defaultValue on chained receiver`() { + lint() + .files( + configParamStub, + kotlin( + """ + import dev.androidbroadcast.featured.ConfigParam + + class Flags { + val darkMode = ConfigParam("dark_mode", false) + } + + fun check(flags: Flags) { + println(flags.darkMode.defaultValue) + } + """, + ).indented(), + ).run() + .expectWarningCount(1) + } + + @Test + fun `does not report defaultValue on String receiver`() { + lint() + .files( + configParamStub, + kotlin( + """ + fun check(s: String) { + println(s.defaultValue) + } + """, + ).indented(), + ).run() + .expectClean() + } + + @Test + fun `does not report access to other ConfigParam properties`() { + lint() + .files( + configParamStub, + kotlin( + """ + import dev.androidbroadcast.featured.ConfigParam + + fun check(param: ConfigParam) { + println(param.key) + } + """, + ).indented(), + ).run() + .expectClean() + } + + @Test + fun `does not report correct usage via ConfigValues`() { + // Tests that configValues[flag] at the call site does not trigger the rule. + // The get() body uses TODO() so it contains no defaultValue access — this + // isolates the call-site behavior from the implementation behavior. + // (The real ConfigValues uses @Suppress("HardcodedFlagValue") on its internal + // defaultValue access — see ConfigValues.kt.) + lint() + .files( + configParamStub, + kotlin( + """ + import dev.androidbroadcast.featured.ConfigParam + + class ConfigValues { + operator fun get(param: ConfigParam): T = TODO() + } + + fun check(configValues: ConfigValues, flag: ConfigParam) { + val enabled = configValues[flag] + println(enabled) + } + """, + ).indented(), + ).run() + .expectClean() + } + + @Test + fun `does not report defaultValue on unrelated type with same property name`() { + // A class named 'Wrapper' that also has a 'defaultValue' property must not fire. + // Confirms the detector is gated on type identity, not just property name. + lint() + .files( + kotlin( + """ + class Wrapper(val defaultValue: Boolean) + + fun check(w: Wrapper) { + println(w.defaultValue) + } + """, + ).indented(), + ).run() + .expectClean() + } + + @Test + fun `does not report when ConfigParam-typed variable named defaultValue is used as receiver`() { + // Regression test: a variable/property of type ConfigParam that happens to be *named* + // "defaultValue" must not trigger the rule when it is used as a receiver + // (e.g. `defaultValue.key`). The detector visits all USimpleNameReferenceExpressions + // named "defaultValue", so without an explicit selector-position guard it would resolve + // the receiver's type as ConfigParam and fire incorrectly. + lint() + .files( + configParamStub, + kotlin( + """ + import dev.androidbroadcast.featured.ConfigParam + + val defaultValue: ConfigParam = ConfigParam("flag", false) + + fun check() { + println(defaultValue.key) + } + """, + ).indented(), + ).run() + .expectClean() + } +} diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 7c4cdd5..d260e31 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -32,6 +32,7 @@ mavenPublish = "0.32.0" dokka = "2.1.0" detekt = "1.23.8" configcat = "5.1.0" +lint = "32.1.0" # Must equal AGP version + 23.0.0 (e.g., AGP 9.1.0 → Lint 32.1.0) [libraries] androidx-core-ktx = { module = "androidx.core:core-ktx", version.ref = "androidx-core" } @@ -70,6 +71,9 @@ detekt-test = { module = "io.gitlab.arturbosch.detekt:detekt-test", version.ref configcat-kotlinSdk = { module = "com.configcat:configcat-kotlin-client", version.ref = "configcat" } +lint-api = { module = "com.android.tools.lint:lint-api", version.ref = "lint" } +lint-tests = { module = "com.android.tools.lint:lint-tests", version.ref = "lint" } + [plugins] androidApplication = { id = "com.android.application", version.ref = "agp" } androidLibrary = { id = "com.android.library", version.ref = "agp" } diff --git a/settings.gradle.kts b/settings.gradle.kts index 2048268..ab2e51f 100644 --- a/settings.gradle.kts +++ b/settings.gradle.kts @@ -53,3 +53,4 @@ include(":featured-platform") include(":featured-bom") include(":featured-detekt-rules") include(":configcat-provider") +include(":featured-lint-rules")