refactor: migrate kit infra leaf interfaces to Kotlin (Lane E) - #777
Merged
thomson-t merged 5 commits intoSep 10, 2026
Merged
Conversation
Kotlin migration progressThis PR migrates 193 lines of Java to Kotlin (+0.9 pp).
Migration goal progress: 23.3% (Kotlin LOC as a share of everything that is not a designated public-API facade.) 127 Java files / 48 Kotlin files in scope. Largest Java files still to migrate
|
thomson-t
previously approved these changes
Sep 10, 2026
thomson-t
force-pushed
the
kotlin/lane-e-kit-infra-leaf
branch
from
September 10, 2026 16:44
48da9bb to
55a7a2f
Compare
Collaborator
thomson-t
marked this pull request as ready for review
September 10, 2026 17:11
PR SummaryMedium Risk Overview
The public API baseline picks up one additive enum helper: Reviewed by Cursor Bugbot for commit edab2c4. Bugbot is set up for automated code reviews on this repo. Configure here. |
thomson-t
previously approved these changes
Sep 10, 2026
thomson-t
dismissed
their stale review
September 10, 2026 17:25
The merge-base changed after approval.
thomson-t
added a commit
that referenced
this pull request
Sep 10, 2026
Brings `workstation/kotlin-migration` up to date with `main`. ##⚠️ Merge this with "Create a merge commit" — not squash, not rebase The branch already contains a real merge commit with two parents (`7907bbe4` from the integration branch, `a1377d39` from `main`). Squashing would flatten the upstream commits into one synthetic commit on the integration branch and lose the branch point; rebasing would rewrite `main`'s commits with new SHAs. Either one makes the eventual `workstation/kotlin-migration` → `main` merge harder to read and harder to bisect. ## What it picks up | | | | --- | --- | | [#616](#616) | chore: bump androidx.test.ext:junit from 1.1.4 to 1.3.0 | | [#769](#769) | docs: trim AGENTS.md to the non-derivable core and add CLAUDE.md | | [#784](#784) | chore: bump actions/setup-java from 5 to 6 | | [#765](#765) | chore: bump gradle/actions/setup-gradle from 5.0.2 to 6.3.0 | | [#764](#764) | chore: bump trunk-io/trunk-action from 1.3.1 to 2.0.0 | | [#775](#775) | chore: prepare release 6.0.4 | 14 files, +194 / −211. **No conflicts.** ## Why the migration tooling is unaffected Nothing on `main` touched `android-core` or `android-kit-base` `src/main`, so the committed `api/*.api` files are still exact. The androidTest manifest and `testutils/build.gradle` junit bump do not affect the public API guard or the Kotlin migration tracker. Stacked PRs #777 and #778 will rebase onto this once it lands. Made with [Cursor](https://cursor.com)
Converts KitsLoadedListener, JsonReportingMessage, ReportingManager, KitContext, and KitManager from Java to Kotlin. No public API change: verified with `scripts/api-guard.sh check` and a full android-core + android-kit-base compile against the new interfaces. KitManager needed two adjustments beyond a literal translation: - Six Activity/Bundle lifecycle params go nullable (AppStateManager.kt already calls them with nullable arguments; Java's unannotated platform types had been silently permissive here). - getSurveyUrl's Map<String, List<String>> param becomes MutableMap<String, List<String>> — Kotlin auto-wildcards a covariant Map value type in parameter position, which broke the existing Java overrides in KitManagerImpl and KitFrameworkWrapper even though the erased javap signature is unaffected. The API baseline picks up one new line: KitStatus gets a synthetic getEntries() accessor, which every Kotlin 1.9+ enum conversion adds automatically (kotlin.enums.EnumEntries). Purely additive, already present in the baseline for WrapperSdk and ApplicationContextWrapper$MethodType from earlier conversions. Local ktlintCheck and unit tests could not run in this environment — Maven Central is rate-limiting dependency resolution (429) for powermock, kotlinx-coroutines-test, and ktlint-cli. Compile and api-guard both ran clean against the real build. CI will run the full suite. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
CI's Unit Tests / Kit Compatibility Test / build-kits::rokt jobs all failed on real regressions in the KitManager conversion that my local compile (main sources only) didn't exercise: 1. Five zero-arg getters/isEnabled became Kotlin properties (currentActivity, isEnabled, supportedKits, roktOptions, kitStatus, attributionResults). Kotlin's Java-interop sugar that lets Kotlin code call a Java getter as `.propertyName` only applies to Java-declared methods — once KitManager became a Kotlin interface, every existing Kotlin call site using that sugar (KitFrameworkWrapperTest.kt's `mockKitManager.supportedKits`, Rokt.kt's `mKitManager.isEnabled`, RoktKit.kt's `kitManager?.roktOptions`, etc.) stopped compiling. Declaring these as real Kotlin properties restores the sugar for Kotlin callers while producing the identical getXxx()/isXxx() bytecode Java callers and the API guard already expect — confirmed with api-guard.sh check (zero diff) after the change. 2. Six more parameters go nullable (logScreen, setLocation, setUserAttribute both params, removeUserAttribute's key, setUserIdentity both params, updateKits), on top of the six Activity/Bundle lifecycle params from the previous commit. KitManagerTest.kt and KitFrameworkWrapperTest.kt pass null to all of these on purpose, to exercise KitManagerImpl's existing null-tolerant behavior — proven by reading the actual test bodies, not inferred. Both are a single hazard, really: an unannotated Java parameter or getter carries no nullability information, and Java platform-type looseness let existing callers (production and test) rely on behavior a naive non-null Kotlin translation silently forecloses. Worth its own line in PLAN.md's hazard table, distinct from the already-documented "tightens nullability" entry — this one is about finding every existing caller (including tests) before choosing a type, not about the direction of the mismatch. Local verification: main-source compile and api-guard.sh both pass for android-core and android-kit-base. Could not compile the test sources locally — Maven Central is rate-limiting this network for powermock/kotlinx-coroutines-test dependency resolution (429), and the one cached kotlinx-coroutines-test-jvm version (1.10.2) doesn't match what's pinned (1.9.0). Every call site was instead verified by reading the actual failing test files and grepping the full repo for other usages of the same KitManager methods. CI will confirm the actual test compile. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The previous fix only covered parameters where an explicit `null`
literal appeared in test source, found by grep. CI's rerun surfaced a
broader pattern that grep can't find: `Mockito.any()` (and the
parameterless `any()` import) return null under the hood, and Kotlin
inserts a call-site null-check ("<expr> must not be null") whenever
that platform-typed null is passed into a genuinely non-null Kotlin
parameter. KitManager is mocked and verified extensively across the
kit test suite with exactly this pattern
(KitFrameworkWrapperTest.testLogBaseEvent/testLogCommerceEvent/
testReplayEvents/testSetWrapperSdkVersion_*, etc.), so grepping for
literal `null` was never going to find all of them.
Rather than keep discovering these one CI run at a time, every
non-primitive parameter in KitManager is now nullable, matching what
the original unannotated Java signature always permitted. Primitives
(Long/Int/Boolean) are unaffected — Mockito's anyLong()/anyInt()/
anyBoolean() return real default values, not null, so they were never
at risk.
Also fixes the same "Kotlin interop sugar only applies to Java
methods" hazard from the previous commit, this time in
JsonReportingMessage (Lane E, not KitManager): timestamp, moduleId,
and sessionId become real Kotlin properties. android-core's
androidTest suite (ReportingServiceTest.kt) calls
`jsonReportingMessage.sessionId` property-style on the interface type,
which broke the same way KitFrameworkWrapperTest's `.supportedKits`
did. Confirmed via full-repo grep that no Kotlin caller uses explicit
`.getSessionId()`/`.getTimestamp()` call syntax on this interface, so
the property conversion is unambiguously safe. setDevMode and toJson
stay functions — no matching getter/property shape in the original.
Local verification: main-source compile and api-guard.sh both pass
for android-core and android-kit-base (zero API diff — confirms
properties compile to the identical getXxx()/isXxx() bytecode).
Cannot compile android-core's test or androidTest source sets locally
at all — both need kotlinx-coroutines-test/powermock, and Maven
Central is still rate-limiting this network (429) for those
specific artifacts. Every change here is instead grounded in an
actual CI failure message or a full-repo grep for the call pattern,
not inference. CI will confirm.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
KitManager.logEvent took BaseEvent? after the CI nullability pass, but the kit-base test subclass still overrode the old non-null Java shape, so unit test compile failed. Co-authored-by: Cursor <cursoragent@cursor.com>
thomson-t
force-pushed
the
kotlin/lane-e-kit-infra-leaf
branch
from
September 10, 2026 17:25
5d10d03 to
95400e6
Compare
thomson-t
previously approved these changes
Sep 10, 2026
KitManager's onIdentify/Login/Logout/ModifyCompleted parameters are nullable after the interface conversion; the androidTest stubs still used the old non-null Java shapes. Co-authored-by: Cursor <cursoragent@cursor.com>
thomson-t
approved these changes
Sep 10, 2026
thomson-t
added a commit
that referenced
this pull request
Sep 10, 2026
…778) ## Lane C — identity leaf classes, part 1 (1 of 2 files, 13 of 170 LOC) Stacked on #777 (Lane E) — merge that one first. Part of the Java→Kotlin migration ([PLAN.md](docs/kotlin-migration/PLAN.md)). Converts `identity/MParticleIdentityClient`. Zero public API diff (`bash scripts/api-guard.sh check` passes) — this file lives outside `com.mparticle.internal`, so any diff here is a hard gate failure, not a reviewable one. Each method keeps `@Throws(Exception::class)` to preserve the `throws` clause Java callers compile against. ## `MParticleUserImpl` is deliberately NOT in this PR Lane C's other easy-tier file (157 LOC) turned out to have a real hazard: it has three package-private members (`setUserIdentities`, `setUserIdentity`, `setUserDelegate`/`mUserDelegate`) that `testutils/AccessUtils.java` calls directly — from a **separate Gradle module**. Kotlin has no package-private visibility. The closest equivalent, `internal`, is module-scoped and name-mangled on the JVM, so converting these would break that cross-module Java call site outright. Working around the mangling with `@JvmName` compiles, but makes those three members permanently public in the ABI — a real (if obscure) addition to a frozen-by-default file, which is exactly what this migration is supposed to avoid. This needs either a coordinated PR that also converts `AccessUtils.java`/`IdentityApi.java`, or a restructured test hook. Filing as a follow-up rather than folding it in here or quietly accepting the API growth. Worth adding to `PLAN.md`'s hazard table — package-private Java members with cross-module Java callers are a distinct case from the ones already documented. ## Validation - `./gradlew :android-core:compileDebugKotlin :android-core:compileDebugJavaWithJavac` — clean - `./gradlew :android-kit-base:compileDebugKotlin :android-kit-base:compileDebugJavaWithJavac` — clean - `bash scripts/api-guard.sh check` — passed, zero diff - `bash scripts/kotlin-migration-progress.sh` — 23.3% → 23.3% (rounds the same at one decimal; LOC moved 16,752 → 16,739) - Local `ktlintCheck`/unit tests could not run in this environment — Maven Central is rate-limiting dependency resolution (429). The commit hook's own ktlint pass (cached deps) was clean. CI will run the full suite. Draft while CI runs — flip to ready once green, and once #777 merges so this rebases onto `workstation/kotlin-migration` directly.
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
2 participants
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.
Lane E — kit infrastructure leaf classes (5 files, 193 LOC)
Part of the Java→Kotlin migration (PLAN.md). Converts:
internal/KitsLoadedListenerinternal/JsonReportingMessageinternal/ReportingManagerinternal/KitContextinternal/KitManager— on the frozen-internals list; 4 kit files implement or depend on itAPI surface
bash scripts/api-guard.sh checkpasses. One intentional baseline update:KitManager$KitStatusgains a syntheticgetEntries()accessor, which every Kotlin 1.9+ enum conversion adds automatically (kotlin.enums.EnumEntries). Purely additive — same pattern already accepted forWrapperSdkandApplicationContextWrapper$MethodTypefrom earlier conversions.Two non-mechanical fixes were needed to keep this a true no-op for callers:
KitManager's six Activity/Bundle lifecycle params go nullable (onActivityCreated,onActivityStarted,onActivityResumed,onActivityStopped,onActivitySaveInstanceState,onActivityDestroyed).AppStateManager.ktalready calls these with nullable arguments — Java's unannotated platform types were silently permissive here, and a straight non-null translation broke that existing caller.getSurveyUrl'sMap<String, List<String>>parameter becomesMutableMap<String, List<String>>. Kotlin auto-wildcards a covariantMapvalue type when used in parameter position, which broke the existing Java overrides inKitManagerImplandKitFrameworkWrapper(name clash... have the same erasure, yet neither overrides the other) even though the erasedjavapsignature the guard checks is unaffected either way.Both are worth folding into
PLAN.md's hazard table — filing as a follow-up.Validation
./gradlew :android-core:compileDebugKotlin :android-core:compileDebugJavaWithJavac— clean./gradlew :android-kit-base:compileDebugKotlin :android-kit-base:compileDebugJavaWithJavac— clean (verifiesKitManagerImplstill overrides correctly)bash scripts/api-guard.sh check— passed after the one intentional baseline update abovebash scripts/kotlin-migration-progress.sh— 22.4% → 23.3%ktlintCheck/unit tests could not run in this environment — Maven Central is rate-limiting dependency resolution (429) for powermock, kotlinx-coroutines-test, and ktlint-cli. The pre-commit hook's own ktlint run (cached deps) passed clean. CI will run the full suite.Draft while CI runs — flip to ready once green.