|
| 1 | +# Kotlin support — deferred follow-ups |
| 2 | + |
| 3 | +**Status:** deferred — captured for a follow-up PR. Surfaced during review of the |
| 4 | +[#918](https://github.com/flamingock/flamingock-java/issues/918) Kotlin-support fix (auto-apply |
| 5 | +`org.jetbrains.kotlin.kapt` when Kotlin JVM is detected, register `flamingock-processor` under |
| 6 | +the `kapt` configuration, harden the reflective Kotlin-compile-task arg wiring). |
| 7 | + |
| 8 | +Two related concerns are intentionally **not** addressed in the bug-fix PR so the patch release |
| 9 | +stays focused. They are described here in enough detail that the next PR doesn't have to |
| 10 | +re-derive the analysis. |
| 11 | + |
| 12 | +--- |
| 13 | + |
| 14 | +## 1. Kapt auto-apply: opt-out and KSP roadmap |
| 15 | + |
| 16 | +### Current behaviour |
| 17 | + |
| 18 | +`FlamingockPlugin.apply()` reacts to `org.jetbrains.kotlin.jvm` being present and |
| 19 | +unconditionally applies `org.jetbrains.kotlin.kapt` (via `FlamingockConstants.KAPT_PLUGIN_ID`) |
| 20 | +unless it is already applied. There is no opt-out. `DependencyConfigurator.configure()` then |
| 21 | +adds `flamingock-processor` (and `mongock-support` when Mongock support is enabled) under the |
| 22 | +`kapt` configuration alongside `annotationProcessor`. |
| 23 | + |
| 24 | +### Why this is fine for now |
| 25 | + |
| 26 | +The plugin's stated positioning is zero-boilerplate (`"id 'io.flamingock'"` and the user is |
| 27 | +done). Kotlin users hitting #918 were dropping into raw kapt configuration and getting it |
| 28 | +wrong; auto-apply removes that failure mode. As long as `flamingock-processor` ships only as a |
| 29 | +`javax.annotation.processing.Processor`, **kapt is the only path** for running it against |
| 30 | +Kotlin sources — KSP cannot run a `javax.annotation.processing.Processor` directly. |
| 31 | + |
| 32 | +### Why it deserves an opt-out |
| 33 | + |
| 34 | +- **Build cost.** Kapt generates Java stubs from Kotlin sources and runs the AP against the |
| 35 | + stubs. This is slower than KSP and historically blocks some Kotlin language features (e.g. |
| 36 | + certain inline-class scenarios) from being visible to the AP. |
| 37 | +- **Long-term direction.** KSP is Google/JetBrains' recommended replacement for kapt for new |
| 38 | + annotation processing in Kotlin projects. Teams that have moved their stack to KSP will |
| 39 | + resent kapt being silently force-applied. |
| 40 | +- **Composability.** Users who deliberately don't want kapt — for instance, they have their |
| 41 | + own kapt setup, or they want to register the processor under `ksp` once that's supported — |
| 42 | + have no way to disable the auto-apply today. |
| 43 | + |
| 44 | +### Deferred items |
| 45 | + |
| 46 | +**a) Add an extension toggle.** Shape suggestion: |
| 47 | + |
| 48 | +```kotlin |
| 49 | +flamingock { |
| 50 | + // Default: "kapt" (current behaviour, no breaking change). |
| 51 | + // Future values: "ksp" (once the processor is ported), "none" (caller wires it themselves). |
| 52 | + kotlinAnnotationProcessor = "kapt" |
| 53 | +} |
| 54 | +``` |
| 55 | + |
| 56 | +A simpler `autoApplyKapt: Boolean` is acceptable if KSP support is genuinely far off; the |
| 57 | +three-valued enum is preferred because it makes the eventual KSP migration a value change |
| 58 | +rather than an option deprecation. |
| 59 | + |
| 60 | +Default must remain the current behaviour (`"kapt"` or `autoApplyKapt = true`) so existing |
| 61 | +users see no change. |
| 62 | + |
| 63 | +Implementation touches `FlamingockExtension`, `FlamingockPlugin.apply()` (gate the |
| 64 | +`project.plugins.withId(KOTLIN_JVM_PLUGIN_ID) { … }` block on the new value), and |
| 65 | +`DependencyConfigurator.configure()` (skip the `kapt` dependency registration when the toggle |
| 66 | +is `"none"`). |
| 67 | + |
| 68 | +**b) KSP port of `flamingock-processor`.** Larger, separate piece of work touching |
| 69 | +`core/flamingock-processor/`. Not every javax.annotation.processing API has a direct KSP |
| 70 | +equivalent — `RoundDiscovery`, the `Filer`-based incremental cache in `FlamingockMetadataStore`, |
| 71 | +and the SPI-file roundtrip in `MetadataModuleIdentity` all need careful porting. Worth a |
| 72 | +scoping issue before implementation to enumerate the API gaps. The auto-apply toggle above |
| 73 | +should land first so the KSP work has somewhere to plug in. |
| 74 | + |
| 75 | +--- |
| 76 | + |
| 77 | +## 2. Mixed Java+Kotlin modules: duplicate same-name default stages |
| 78 | + |
| 79 | +### Mechanism |
| 80 | + |
| 81 | +When a single module has **both** Java and Kotlin sources annotated with Flamingock, the |
| 82 | +processor runs **twice** per build — once under `compileJava` (via the `annotationProcessor` |
| 83 | +configuration) and once under `kaptKotlin` (via the `kapt` configuration). The two invocations |
| 84 | +happen with **different `CLASS_OUTPUT` directories**: |
| 85 | + |
| 86 | +- kapt: `build/tmp/kapt3/classes/<sourceSet>/` |
| 87 | +- javac: `build/classes/java/<sourceSet>/` |
| 88 | + |
| 89 | +`MetadataModuleIdentity.resolve()` |
| 90 | +(`core/flamingock-processor/src/main/java/io/flamingock/core/processor/util/MetadataModuleIdentity.java`, |
| 91 | +the `readExistingProviderFqn` path) reads the SPI registration from `CLASS_OUTPUT` to discover |
| 92 | +or generate a per-module suffix. Because the two `CLASS_OUTPUT` dirs are disjoint, the two |
| 93 | +invocations each find no existing SPI file and each generate their own random 8-hex suffix. |
| 94 | +End result: |
| 95 | + |
| 96 | +- kapt writes `META-INF/services/io.flamingock.internal.common.core.metadata.FlamingockMetadataProvider` |
| 97 | + pointing at `…Impl_aaaa1111`, plus `META-INF/flamingock/metadata_aaaa1111.json`. |
| 98 | +- javac writes the same SPI filename pointing at `…Impl_bbbb2222`, plus |
| 99 | + `META-INF/flamingock/metadata_bbbb2222.json`. |
| 100 | + |
| 101 | +Both end up in the packaged JAR. At runtime, |
| 102 | +`MetadataLoader.loadAggregated()` |
| 103 | +(`core/flamingock-core-commons/src/main/java/io/flamingock/internal/common/core/metadata/MetadataLoader.java`) |
| 104 | +uses `ServiceLoader` to discover **both** providers. |
| 105 | + |
| 106 | +### Runtime aggregation behaviour |
| 107 | + |
| 108 | +In `MetadataLoader.CompositePipelineBuilder.buildFrom()`: |
| 109 | + |
| 110 | +- **System stages**: `mergeSystem()` id-deduplicates and logs at DEBUG. |
| 111 | +- **Legacy stages**: `collapseLegacyStagesByName()` + `mergeSameNameLegacyStages()` group by |
| 112 | + name and id-deduplicate the change set per group. |
| 113 | +- **Default stages**: the loop in `buildFrom()` adds every default stage from every provider |
| 114 | + into `defaultStages`. The duplicate-name detection logs a **WARN** (`"Duplicate stage name |
| 115 | + '{}' across modules — proceeding with both."`) but **keeps both copies**. There is no |
| 116 | + id-dedup at the default-stage level. |
| 117 | + |
| 118 | +For the kapt + javac case in a single module, both metadata files contain the **same** |
| 119 | +default-stage structure (both derive it from the same `@EnableFlamingock`). Within those |
| 120 | +stages, each AP only contributes elements visible to its own compiler (kapt: Kotlin sources; |
| 121 | +javac: Java sources). So the composite ends up with the same default stage appearing **twice** |
| 122 | +— one populated with the module's Kotlin changes, the other with its Java changes. |
| 123 | + |
| 124 | +### Why this is messy but not catastrophic |
| 125 | + |
| 126 | +- **No double-execution of the same change.** kapt's metadata has Kotlin changes, javac's has |
| 127 | + Java changes; they don't overlap. |
| 128 | +- **Audit and reporting are messy.** The execution report and the audit log will reference the |
| 129 | + same stage name twice with different change subsets each time, which diverges from what the |
| 130 | + user declared in `@EnableFlamingock` and is confusing to read. |
| 131 | +- **A WARN fires every startup.** Currently the WARN is genuinely useful for catching |
| 132 | + multi-module configuration mistakes; a fix should keep that signal alive while suppressing |
| 133 | + it for the legitimate kapt+javac case. |
| 134 | + |
| 135 | +### Solution directions |
| 136 | + |
| 137 | +**(a) Merge-layer dedup (recommended).** Extend `MetadataLoader.CompositePipelineBuilder` to |
| 138 | +id-union same-name default stages, mirroring `mergeSameNameLegacyStages`. Concretely: refactor |
| 139 | +`mergeSameNameLegacyStages` and `collapseLegacyStagesByName` into a stage-type-agnostic helper |
| 140 | +(`collapseStagesByName(List<PreviewStage>)` returning one collapsed stage per name); call it |
| 141 | +for both `defaultStages` and `legacyStages`. Preserve the duplicate-name WARN for the |
| 142 | +deduplicated-but-still-suspicious case where the contents weren't fully aligned across modules |
| 143 | +(e.g. different `sourcesPackage` on stages of the same name) so genuine misconfigurations are |
| 144 | +still surfaced. |
| 145 | + |
| 146 | +Pros: one fix lives in one place (commons), reuses an existing pattern, robust to any future |
| 147 | +multi-provider scenario (not just kapt+javac). Cons: weakens the current invariant that |
| 148 | +"default stages with the same name across modules is suspicious" — mitigated by keeping the |
| 149 | +WARN for content mismatches. |
| 150 | + |
| 151 | +**(b) Shared `CLASS_OUTPUT` / shared module identity (rejected).** Make kapt and javac in the |
| 152 | +same module reuse the same suffix. Conceptually cleaner — one metadata file end-to-end — but |
| 153 | +the build-side mechanics are fragile: Gradle's task ordering across compilers, incremental |
| 154 | +recompilation scenarios, and clean-build orderings all become surface area for this to drift. |
| 155 | +The merge-layer fix achieves the same end result without that fragility. |
| 156 | + |
| 157 | +### Verification fixture |
| 158 | + |
| 159 | +A test fixture under `flamingock-gradle-plugin/src/test/resources/` (or as an integration test |
| 160 | +under `core/flamingock-core-commons/`) exercising a module with at least one `@Change` in Java |
| 161 | +and one in Kotlin, both under the same `@EnableFlamingock` stage. Assertions: |
| 162 | + |
| 163 | +1. Build succeeds, producing two `META-INF/flamingock/metadata_*.json` files in the JAR (this |
| 164 | + is expected — the fix is at the aggregator, not the build). |
| 165 | +2. After `MetadataLoader.loadAggregated()`, the composite pipeline contains exactly one |
| 166 | + default stage with that name, and its change list contains both changes (one from each |
| 167 | + language). |
| 168 | +3. No `"Duplicate stage name … proceeding with both."` WARN is emitted when the contents are |
| 169 | + compatible. |
| 170 | + |
| 171 | +--- |
| 172 | + |
| 173 | +## Tracking |
| 174 | + |
| 175 | +When the follow-up PR is filed, please open two issues referencing this document: |
| 176 | + |
| 177 | +1. **Kotlin annotation processing: add opt-out toggle and scope KSP port** — implements |
| 178 | + Section 1's deferred items (a) and (b). |
| 179 | +2. **MetadataLoader: dedupe same-name default stages across providers** — implements |
| 180 | + Section 2's solution direction (a). |
0 commit comments