diff --git a/core/api/jvm/core.api b/core/api/jvm/core.api index 1628484..680fdaa 100644 --- a/core/api/jvm/core.api +++ b/core/api/jvm/core.api @@ -1,3 +1,11 @@ +public abstract interface annotation class dev/androidbroadcast/featured/AssumesFlag : java/lang/annotation/Annotation { + public abstract fun flagName ()Ljava/lang/String; +} + +public abstract interface annotation class dev/androidbroadcast/featured/BehindFlag : java/lang/annotation/Annotation { + public abstract fun flagName ()Ljava/lang/String; +} + public final class dev/androidbroadcast/featured/ConfigParam { public fun (Ljava/lang/String;Ljava/lang/Object;Lkotlin/reflect/KClass;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;)V public synthetic fun (Ljava/lang/String;Ljava/lang/Object;Lkotlin/reflect/KClass;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;ILkotlin/jvm/internal/DefaultConstructorMarker;)V diff --git a/core/src/commonMain/kotlin/dev/androidbroadcast/featured/AssumesFlag.kt b/core/src/commonMain/kotlin/dev/androidbroadcast/featured/AssumesFlag.kt new file mode 100644 index 0000000..2c09a10 --- /dev/null +++ b/core/src/commonMain/kotlin/dev/androidbroadcast/featured/AssumesFlag.kt @@ -0,0 +1,49 @@ +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, +) diff --git a/core/src/commonMain/kotlin/dev/androidbroadcast/featured/BehindFlag.kt b/core/src/commonMain/kotlin/dev/androidbroadcast/featured/BehindFlag.kt new file mode 100644 index 0000000..37d95fc --- /dev/null +++ b/core/src/commonMain/kotlin/dev/androidbroadcast/featured/BehindFlag.kt @@ -0,0 +1,47 @@ +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, +) 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/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/featured-detekt-rules/api/featured-detekt-rules.api b/featured-detekt-rules/api/featured-detekt-rules.api index 9a7983b..4972140 100644 --- a/featured-detekt-rules/api/featured-detekt-rules.api +++ b/featured-detekt-rules/api/featured-detekt-rules.api @@ -20,6 +20,14 @@ public final class dev/androidbroadcast/featured/detekt/HardcodedFlagValueRule : public fun visitDotQualifiedExpression (Lorg/jetbrains/kotlin/psi/KtDotQualifiedExpression;)V } +public final class dev/androidbroadcast/featured/detekt/InvalidFlagReference : io/gitlab/arturbosch/detekt/api/Rule { + public fun ()V + public fun (Lio/gitlab/arturbosch/detekt/api/Config;)V + public synthetic fun (Lio/gitlab/arturbosch/detekt/api/Config;ILkotlin/jvm/internal/DefaultConstructorMarker;)V + public fun getIssue ()Lio/gitlab/arturbosch/detekt/api/Issue; + public fun visit (Lorg/jetbrains/kotlin/psi/KtFile;)V +} + public final class dev/androidbroadcast/featured/detekt/MissingFlagAnnotationRule : io/gitlab/arturbosch/detekt/api/Rule { public fun ()V public fun (Lio/gitlab/arturbosch/detekt/api/Config;)V @@ -28,3 +36,12 @@ public final class dev/androidbroadcast/featured/detekt/MissingFlagAnnotationRul public fun visitProperty (Lorg/jetbrains/kotlin/psi/KtProperty;)V } +public final class dev/androidbroadcast/featured/detekt/UncheckedFlagAccess : io/gitlab/arturbosch/detekt/api/Rule { + public fun ()V + public fun (Lio/gitlab/arturbosch/detekt/api/Config;)V + public synthetic fun (Lio/gitlab/arturbosch/detekt/api/Config;ILkotlin/jvm/internal/DefaultConstructorMarker;)V + public fun getIssue ()Lio/gitlab/arturbosch/detekt/api/Issue; + public fun visitCallExpression (Lorg/jetbrains/kotlin/psi/KtCallExpression;)V + public fun visitSimpleNameExpression (Lorg/jetbrains/kotlin/psi/KtSimpleNameExpression;)V +} + diff --git a/featured-detekt-rules/src/main/kotlin/dev/androidbroadcast/featured/detekt/FeaturedRuleSetProvider.kt b/featured-detekt-rules/src/main/kotlin/dev/androidbroadcast/featured/detekt/FeaturedRuleSetProvider.kt index 5e90f19..67138fa 100644 --- a/featured-detekt-rules/src/main/kotlin/dev/androidbroadcast/featured/detekt/FeaturedRuleSetProvider.kt +++ b/featured-detekt-rules/src/main/kotlin/dev/androidbroadcast/featured/detekt/FeaturedRuleSetProvider.kt @@ -5,11 +5,16 @@ 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. + * Provides the `featured` Detekt rule set. * - * To enable in your project, add the artifact to Detekt's classpath and include - * the rule set in your `detekt.yml`: + * Rules: + * - [ExpiredFeatureFlagRule] — flags past their expiry date + * - [HardcodedFlagValueRule] — hardcoded boolean flag values + * - [MissingFlagAnnotationRule] — missing `@LocalFlag`/`@RemoteFlag` annotations + * - [InvalidFlagReference] — `@BehindFlag`/`@AssumesFlag` referencing an unknown flag name (PSI-only, runs under plain `detekt` task) + * - [UncheckedFlagAccess] — `@BehindFlag`-annotated code used outside a guard (requires `detektWithTypeResolution` task) * + * Example `detekt.yml`: * ```yaml * featured: * ExpiredFeatureFlag: @@ -18,7 +23,14 @@ import io.gitlab.arturbosch.detekt.api.RuleSetProvider * active: true * MissingFlagAnnotation: * active: true + * InvalidFlagReference: + * active: true # runs under plain detekt task + * UncheckedFlagAccess: + * active: true # requires detektWithTypeResolution task * ``` + * + * Note: [UncheckedFlagAccess] requires the `detektWithTypeResolution` Gradle task to resolve + * cross-file and cross-module references. It silently skips checks when run without type resolution. */ public class FeaturedRuleSetProvider : RuleSetProvider { override val ruleSetId: String = "featured" @@ -31,6 +43,8 @@ public class FeaturedRuleSetProvider : RuleSetProvider { ExpiredFeatureFlagRule(config), HardcodedFlagValueRule(config), MissingFlagAnnotationRule(config), + InvalidFlagReference(config), + UncheckedFlagAccess(config), ), ) } diff --git a/featured-detekt-rules/src/main/kotlin/dev/androidbroadcast/featured/detekt/InvalidFlagReference.kt b/featured-detekt-rules/src/main/kotlin/dev/androidbroadcast/featured/detekt/InvalidFlagReference.kt new file mode 100644 index 0000000..8172166 --- /dev/null +++ b/featured-detekt-rules/src/main/kotlin/dev/androidbroadcast/featured/detekt/InvalidFlagReference.kt @@ -0,0 +1,97 @@ +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.KtLiteralStringTemplateEntry +import org.jetbrains.kotlin.psi.KtProperty +import org.jetbrains.kotlin.psi.KtStringTemplateExpression +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 visit(root: KtFile) { + super.visit(root) + + // Pass 1: collect @LocalFlag / @RemoteFlag property names in this file + val knownFlags = + root + .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 + root + .collectDescendantsOfType() + .filter { it.shortName?.asString() in setOf("BehindFlag", "AssumesFlag") } + .forEach { annotation -> + val flagName = + annotation.valueArguments + .firstOrNull() + ?.getArgumentExpression() + ?.let { expr -> + val template = expr as? KtStringTemplateExpression ?: return@forEach + val entries = template.entries + if (entries.size != 1) return@forEach // skip string templates like "${someVar}" + (entries[0] as? KtLiteralStringTemplateEntry)?.text + } + ?: 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.", + ), + ) + } + } + } +} diff --git a/featured-detekt-rules/src/main/kotlin/dev/androidbroadcast/featured/detekt/UncheckedFlagAccess.kt b/featured-detekt-rules/src/main/kotlin/dev/androidbroadcast/featured/detekt/UncheckedFlagAccess.kt new file mode 100644 index 0000000..0435af0 --- /dev/null +++ b/featured-detekt-rules/src/main/kotlin/dev/androidbroadcast/featured/detekt/UncheckedFlagAccess.kt @@ -0,0 +1,228 @@ +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.descriptors.DeclarationDescriptorWithSource +import org.jetbrains.kotlin.name.FqName +import org.jetbrains.kotlin.name.Name +import org.jetbrains.kotlin.psi.KtAnnotationEntry +import org.jetbrains.kotlin.psi.KtCallExpression +import org.jetbrains.kotlin.psi.KtClassOrObject +import org.jetbrains.kotlin.psi.KtIfExpression +import org.jetbrains.kotlin.psi.KtLiteralStringTemplateEntry +import org.jetbrains.kotlin.psi.KtNameReferenceExpression +import org.jetbrains.kotlin.psi.KtNamedFunction +import org.jetbrains.kotlin.psi.KtObjectDeclaration +import org.jetbrains.kotlin.psi.KtProperty +import org.jetbrains.kotlin.psi.KtSimpleNameExpression +import org.jetbrains.kotlin.psi.KtStringTemplateExpression +import org.jetbrains.kotlin.psi.KtWhenEntry +import org.jetbrains.kotlin.resolve.BindingContext +import org.jetbrains.kotlin.resolve.calls.util.getResolvedCall +import org.jetbrains.kotlin.resolve.source.KotlinSourceElement + +private val BEHIND_FLAG_FQN = FqName("dev.androidbroadcast.featured.BehindFlag") +private val FLAG_NAME_PARAM = Name.identifier("flagName") +private const val BEHIND_FLAG_SHORT = "BehindFlag" +private const val ASSUMES_FLAG_SHORT = "AssumesFlag" + +/** + * Warns when a `@BehindFlag("X")`-annotated function or constructor is called 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 visitSimpleNameExpression(expression: KtSimpleNameExpression) { + super.visitSimpleNameExpression(expression) + if (bindingContext == BindingContext.EMPTY) return + // Skip callee expressions inside direct calls — handled by visitCallExpression. + // Callable references (this::fn) are intentionally NOT excluded here; accessing + // a @BehindFlag declaration via reference outside a guard is itself a violation. + if (expression.parent is KtCallExpression) return + + val descriptor = + bindingContext[BindingContext.REFERENCE_TARGET, expression] + ?: return + + val flagName = + descriptor.behindFlagNameViaDescriptor() + ?: descriptor.behindFlagNameViaPsi() + ?: return + + if (!expression.isInValidFlagContext(flagName)) { + report( + CodeSmell( + issue = issue, + entity = Entity.from(expression), + message = + "Access 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 visitCallExpression(expression: KtCallExpression) { + super.visitCallExpression(expression) + if (bindingContext == BindingContext.EMPTY) return + + val resolvedCall = expression.getResolvedCall(bindingContext) ?: return + val descriptor = resolvedCall.resultingDescriptor + + // Primary: resolve flag name via BindingContext (works cross-module in production). + // Fallback: read PSI annotation by short name (works in unit tests where the + // annotation class itself is not on the test classpath). + val flagName = + descriptor.behindFlagNameViaDescriptor() + ?: descriptor.behindFlagNameViaPsi() + ?: 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\").", + ), + ) + } + } + + // ── Annotation resolution ───────────────────────────────────────────────── + + /** Reads `@BehindFlag` from the descriptor's annotation list (requires annotation on classpath). */ + private fun DeclarationDescriptor.behindFlagNameViaDescriptor(): String? = + annotations + .findAnnotation(BEHIND_FLAG_FQN) + ?.allValueArguments + ?.get(FLAG_NAME_PARAM) + ?.value as? String + + /** + * Fallback: resolves the declaration's PSI source and reads `@BehindFlag` by short name. + * Used when the annotation class is not on the classpath (e.g. in unit tests). + */ + private fun DeclarationDescriptor.behindFlagNameViaPsi(): String? { + val psi = + ((this as? DeclarationDescriptorWithSource)?.source as? KotlinSourceElement)?.psi + ?: return null + return when (psi) { + is KtNamedFunction -> psi.annotationEntries.behindFlagName() + is KtClassOrObject -> psi.annotationEntries.behindFlagName() + is KtProperty -> psi.annotationEntries.behindFlagName() + else -> null + } + } + + private fun List.behindFlagName(): String? = + firstOrNull { it.shortName?.asString() == BEHIND_FLAG_SHORT } + ?.flagNameArgument() + + // ── PSI context validation ──────────────────────────────────────────────── + + 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/object with @BehindFlag("X") or @AssumesFlag("X") + // KtClassOrObject covers both `class` and `object`; companion objects are + // already short-circuited by the branch below, so exclude them here. + node is KtClassOrObject && !(node is KtObjectDeclaration && node.isCompanion()) && + 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 + // Check the element itself (e.g. bare `if (newCheckout)` where condition IS the reference) + if (this is KtNameReferenceExpression && this.getReferencedName() == flagName) return true + // Check all descendants + 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 KtClassOrObject.hasGuardAnnotation(flagName: String): Boolean = annotationEntries.any { it.matchesGuard(flagName) } + + private fun KtAnnotationEntry.matchesGuard(flagName: String): Boolean { + val name = shortName?.asString() ?: return false + if (name !in setOf(BEHIND_FLAG_SHORT, ASSUMES_FLAG_SHORT)) return false + return flagNameArgument() == flagName + } + + private fun KtAnnotationEntry.flagNameArgument(): String? { + val entries = + valueArguments + .firstOrNull() + ?.getArgumentExpression() + ?.let { it as? KtStringTemplateExpression } + ?.entries ?: return null + if (entries.size != 1) return null + return (entries[0] as? KtLiteralStringTemplateEntry)?.text + } +} diff --git a/featured-detekt-rules/src/test/kotlin/dev/androidbroadcast/featured/detekt/InvalidFlagReferenceTest.kt b/featured-detekt-rules/src/test/kotlin/dev/androidbroadcast/featured/detekt/InvalidFlagReferenceTest.kt new file mode 100644 index 0000000..318100a --- /dev/null +++ b/featured-detekt-rules/src/test/kotlin/dev/androidbroadcast/featured/detekt/InvalidFlagReferenceTest.kt @@ -0,0 +1,135 @@ +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) + } +} diff --git a/featured-detekt-rules/src/test/kotlin/dev/androidbroadcast/featured/detekt/UncheckedFlagAccessTest.kt b/featured-detekt-rules/src/test/kotlin/dev/androidbroadcast/featured/detekt/UncheckedFlagAccessTest.kt new file mode 100644 index 0000000..bb6947e --- /dev/null +++ b/featured-detekt-rules/src/test/kotlin/dev/androidbroadcast/featured/detekt/UncheckedFlagAccessTest.kt @@ -0,0 +1,384 @@ +package dev.androidbroadcast.featured.detekt + +import io.github.detekt.test.utils.createEnvironment +import io.gitlab.arturbosch.detekt.test.lint +import io.gitlab.arturbosch.detekt.test.lintWithContext +import org.junit.Test +import kotlin.test.assertEquals + +class UncheckedFlagAccessTest { + private val rule = UncheckedFlagAccess() + private val envWrapper = createEnvironment() + private val env = envWrapper.env + + // ── 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 body calling BehindFlag function`() { + val findings = + rule.lintWithContext( + env, + """ + import dev.androidbroadcast.featured.BehindFlag + + @BehindFlag("newCheckout") + fun newCheckoutScreen() {} + + val launcher: () -> Unit = { newCheckoutScreen() } + """.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 newCheckoutConfig: String = "config" + + fun host() { + val value = newCheckoutConfig + } + """.trimIndent(), + ) + assertEquals(1, findings.size) + } + + @Test + fun `no finding for call inside AssumesFlag object body same flag`() { + val findings = + rule.lintWithContext( + env, + """ + import dev.androidbroadcast.featured.BehindFlag + import dev.androidbroadcast.featured.AssumesFlag + + @BehindFlag("newCheckout") + fun newCheckoutScreen() {} + + @AssumesFlag("newCheckout") + object CheckoutFeature { + fun show() { newCheckoutScreen() } + } + """.trimIndent(), + ) + assertEquals(0, findings.size) + } + + @Test + fun `reports finding for callable reference to BehindFlag function without context`() { + val findings = + rule.lintWithContext( + env, + """ + import dev.androidbroadcast.featured.BehindFlag + + @BehindFlag("newCheckout") + fun newCheckoutScreen() {} + + val ref: () -> Unit = ::newCheckoutScreen + """.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) + } +}