Add enum-typed flags to Gradle DSL (#162)#185
Merged
Conversation
Adds `FlagContainer.enum(key, typeFqn, default)` declarator. Enum flags round-trip through FlagSpec/ResolveFlagsTask as the fully-qualified class name stored in the `type` field. LocalFlagEntry.isEnum detects enum types via presence of '.' in the type string. Generator behaviour: - ConfigParamGenerator: emits ConfigParam<com.example.Foo> with default expressed as FQN.CONSTANT_NAME. - ExtensionFunctionGenerator: emits get…() extension returning the raw enum type (same as non-Boolean primitives). - ProguardRulesGenerator: skips enum flags — values are resolved at runtime and cannot be assumed at build time. - IosConstValGenerator: skips enum flags — const val only supports primitives and String. Integration test fixture: adds CheckoutVariant enum source so assembleRelease compiles generated ConfigParam<CheckoutVariant>. assertContainsAssumevaluesBlock now also asserts enum flags are absent from ProGuard output.
Review Summary by QodoAdd enum-typed flags to Gradle DSL with full generation pipeline support
WalkthroughsDescription• Add enum(key, typeFqn, default) DSL declarator to FlagContainer • Enum flags round-trip through generation pipeline with FQN type support • Generators handle enums: ConfigParam uses FQN syntax, extension functions use get…() pattern • ProGuard and iOS generators intentionally skip enums (runtime values, const val limitations) • Comprehensive test coverage including integration test with fixture enum Diagramflowchart LR
DSL["FlagContainer.enum<br/>DSL declarator"]
FlagSpec["FlagSpec stores<br/>FQN type + default"]
ConfigParam["ConfigParamGenerator<br/>emits FQN.CONSTANT syntax"]
Extension["ExtensionFunctionGenerator<br/>emits get…() pattern"]
ProGuard["ProguardRulesGenerator<br/>skips enums"]
iOS["IosConstValGenerator<br/>skips enums"]
DSL --> FlagSpec
FlagSpec --> ConfigParam
FlagSpec --> Extension
FlagSpec --> ProGuard
FlagSpec --> iOS
File Changes1. featured-gradle-plugin/src/main/kotlin/dev/androidbroadcast/featured/gradle/FlagContainer.kt
|
Code Review by Qodo
1. iOS includes remote flags
|
There was a problem hiding this comment.
Pull request overview
Adds first-class support for enum-typed local flags in the Gradle DSL and threads the new type through code generation, while explicitly excluding enums from generators that can’t safely/meaningfully support them (ProGuard assumevalues, iOS const vals).
Changes:
- Added
enum(key, typeFqn, default)toFlagContainerand persisted enum type FQN + default constant through descriptors. - Updated generators to handle enum defaults (
TypeFqn.CONSTANT) and to exclude enums from ProGuard rules and iOSconst valoutput. - Extended unit/integration tests and the Android fixture to cover enum end-to-end behavior.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| featured-gradle-plugin/src/main/kotlin/dev/androidbroadcast/featured/gradle/FlagContainer.kt | Adds the public enum(...) DSL declarator for enum-typed flags. |
| featured-gradle-plugin/src/main/kotlin/dev/androidbroadcast/featured/gradle/LocalFlagEntry.kt | Introduces isEnum helper used by downstream generators. |
| featured-gradle-plugin/src/main/kotlin/dev/androidbroadcast/featured/gradle/ConfigParamGenerator.kt | Emits enum default values as TypeFqn.CONSTANT. |
| featured-gradle-plugin/src/main/kotlin/dev/androidbroadcast/featured/gradle/IosConstValGenerator.kt | Skips enum-typed entries when generating iOS expect/actual const val output. |
| featured-gradle-plugin/src/main/kotlin/dev/androidbroadcast/featured/gradle/ProguardRulesGenerator.kt | Documents intentional exclusion of enums from -assumevalues. |
| featured-gradle-plugin/src/main/kotlin/dev/androidbroadcast/featured/gradle/FlagEntryUtils.kt | Clarifies that enum flags use get… extension naming. |
| featured-gradle-plugin/src/test/kotlin/dev/androidbroadcast/featured/gradle/FlagContainerTest.kt | Tests enum storage, configure block behavior, and descriptor round-trip contents. |
| featured-gradle-plugin/src/test/kotlin/dev/androidbroadcast/featured/gradle/ConfigParamGeneratorTest.kt | Tests enum generic type emission and TypeFqn.CONSTANT default syntax. |
| featured-gradle-plugin/src/test/kotlin/dev/androidbroadcast/featured/gradle/ExtensionFunctionGeneratorTest.kt | Tests get… extension generation for enum flags. |
| featured-gradle-plugin/src/test/kotlin/dev/androidbroadcast/featured/gradle/ProguardRulesGeneratorTest.kt | Tests that enum flags are excluded from ProGuard rules. |
| featured-gradle-plugin/src/test/kotlin/dev/androidbroadcast/featured/gradle/IosConstValGeneratorTest.kt | Tests that enum flags are skipped in iOS const-val generation. |
| featured-gradle-plugin/src/test/kotlin/dev/androidbroadcast/featured/gradle/FeaturedPluginIntegrationTest.kt | Ensures fixture enum flag does not appear in ProGuard output. |
| featured-gradle-plugin/src/test/fixtures/android-project/build.gradle.kts | Declares an enum flag in the fixture DSL. |
| featured-gradle-plugin/src/test/fixtures/android-project/src/main/kotlin/dev/androidbroadcast/featured/testapp/CheckoutVariant.kt | Adds fixture enum type used by the new DSL flag. |
Comment on lines
+100
to
+103
| ) { | ||
| _flags += FlagSpec(key = key, defaultValue = default, type = typeFqn).apply(configure) | ||
| } | ||
|
|
Comment on lines
32
to
40
| public fun generate(entries: List<LocalFlagEntry>): String { | ||
| if (entries.isEmpty()) return "" | ||
| val eligible = entries.filterNot { it.isEnum } | ||
| if (eligible.isEmpty()) return "" | ||
| return buildString { | ||
| appendLine(HEADER) | ||
| appendLine("package $GENERATED_PACKAGE") | ||
| appendLine() | ||
| entries.forEach { entry -> | ||
| eligible.forEach { entry -> | ||
| appendLine(actualDeclaration(entry)) |
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+33
to
+34
| val eligible = entries.filterNot { it.isEnum } | ||
| if (eligible.isEmpty()) return "" |
There was a problem hiding this comment.
1. Ios includes remote flags 🐞 Bug ≡ Correctness
IosConstValGenerator filters out enum-typed entries but still generates expect/actual declarations for remote flags, freezing remote defaults into compile-time constants. Because the iOS generation task reads ResolveFlagsTask output containing both local and remote flags, this can incorrectly eliminate runtime-configurable behavior on iOS.
Agent Prompt
### Issue description
`GenerateIosConstValTask` reads the unified flags file produced by `ResolveFlagsTask` (which includes both local and remote flags). `IosConstValGenerator` currently only filters out enums, so remote flags still end up as `expect val` / `actual const val`, incorrectly turning runtime-resolved remote values into compile-time constants.
### Issue Context
Other generators explicitly treat local and remote differently (e.g., ProGuard rules are local-only). iOS const-val generation should similarly operate on local flags only.
### Fix Focus Areas
- featured-gradle-plugin/src/main/kotlin/dev/androidbroadcast/featured/gradle/FeaturedPlugin.kt[45-56]
- featured-gradle-plugin/src/main/kotlin/dev/androidbroadcast/featured/gradle/FeaturedPlugin.kt[117-132]
- featured-gradle-plugin/src/main/kotlin/dev/androidbroadcast/featured/gradle/ResolveFlagsTask.kt[53-60]
- featured-gradle-plugin/src/main/kotlin/dev/androidbroadcast/featured/gradle/ResolveFlagsTask.kt[64-69]
- featured-gradle-plugin/src/main/kotlin/dev/androidbroadcast/featured/gradle/ScanResultParser.kt[60-71]
- featured-gradle-plugin/src/main/kotlin/dev/androidbroadcast/featured/gradle/IosConstValGenerator.kt[32-64]
### Suggested change
- In both `generate` and `generateExpect`, change eligibility to:
- `val eligible = entries.filter { it.isLocal && !it.isEnum }`
- Add/update tests to include a remote flag and assert it is absent from iOS output.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Contributor
Author
Code reviewFound 2 issues:
Featured/featured-gradle-plugin/CLAUDE.md Lines 12 to 20 in 74ab3d8 🤖 Generated with Claude Code - If this code review was useful, please react with 👍. Otherwise, react with 👎. |
- Require '.' in typeFqn so unqualified names fail at configuration time rather than as a generated-code compile error. - Add enum example to featured-gradle-plugin/CLAUDE.md DSL section.
20 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add `enum(key, typeFqn, default)` declarator to `FlagContainer` with full round-trip support through the generation pipeline.
New public DSL surface
```kotlin
featured {
localFlags {
enum(
key = "checkout_variant",
typeFqn = "com.example.CheckoutVariant",
default = "LEGACY",
) {
description = "Checkout flow variant"
category = "Checkout"
}
}
}
```
Generator behaviour
iOS generator
Enum flags are silently skipped by `IosConstValGenerator` in both `generate` (iosMain actual) and `generateExpect` (commonMain expect). The class KDoc documents this constraint. No structural decision is deferred — the behaviour is fully defined and tested.
Test coverage
New/extended tests:
Fixture source: `CheckoutVariant.kt` added to the Android test fixture so `assembleRelease` can compile the generated `ConfigParam`.
Closes #162