Skip to content

fix(resolution): a caller of an imported expect/actual reaches the actual its source set holds (#3033) - #3136

Closed
johnhldiver-beep wants to merge 1 commit into
colbymchenry:mainfrom
johnhldiver-beep:fix/3033-kotlin-expect-actual
Closed

johnhldiver-beep wants to merge 1 commit into
colbymchenry:mainfrom
johnhldiver-beep:fix/3033-kotlin-expect-actual

Conversation

@johnhldiver-beep

Copy link
Copy Markdown

Problem

A Java call to a Kotlin Multiplatform object links the commonMain
expect declaration instead of the platform actual. An expect does not
exist in JVM bytecode, so a Java caller can only ever reach the actual.
Regression after #3023: at 01e0ac3 the same call linked the androidMain
actual. Real case: JakeWharton/timber's Timber.plant / Timber.tag
moved from androidMain into a commonMain expect.

Fixes #3033.

Root cause

resolveJavaImportedReference (the JVM import resolution #3023 added)
reads the imported declaration's fully qualified name through
jvmImportedDeclarations and picks its owner with pickClosestJvmCandidate.
That rule was written for the common-side Kotlin import ref case and
prefers expect when the evidence is tied. A Java caller from
app/src/main/java shares no source set with either declaration, so the
tie-break fires and picks expect.

Fix

In src/resolution/import-resolver.ts (+29 lines):

  • kmpSourceSet reads the src/<set>/ path segment, so a declaration
    knows which source set holds it.
  • jvmExpectActualPick: when the candidates for one declaration span
    expect and actual, the pick becomes caller-aware — a Java caller
    (JVM bytecode holds no expect) and a Kotlin caller outside
    commonMain take the actual (the one from the caller's own source
    set when several exist, otherwise by proximity); a commonMain
    caller keeps the expect.
  • The import itself (resolveJvmImport) still keeps its common-side
    resolution; only the owner pick changes.

No public API touched; no unrelated code reformatted.

Regression tests

New __tests__/kotlin-expect-actual.test.ts, 5 end-to-end cases: the
issue's Util.go repro (Java caller → androidMain actual), timber's
Timber.plant (companion member of the actual), the issue's
extends Timber.Tree (the actual's nested type), a Kotlin androidMain
caller in another package (own source set's actual), and a commonMain
caller keeping the expect.

Red-to-green on main: the two Java-caller cases failed before the fix
and pass after; the two Kotlin-caller cases already passed on main
and still pass. After the fix: 5/5.

Verification (disk- and log-verified)

  • npm run build green; npx tsc --noEmit clean after every edit.
  • JVM + Kotlin suites post-fix: jvm-cross-language-imports (the fix(jvm): a call through an import reaches the Java or Kotlin class it names #3023
    regression surface) + java/* + kotlin/* all green (51 + 51).
  • Full suite on base 2008e2f: 9856 passed / 8 failed / 439 skipped.
    All 8 failures are environmental or load flakes, proven unrelated to
    this diff: the 6 known ones from earlier the same day (gitignore x4,
    busiest-symbol x1, vitest-worker-flags x1, ui-package collect — all
    reproduced on bare main), plus mcp-daemon.test.ts x2 (socket-timing
    tests) which pass 24/24 in isolation on BOTH the branch and bare
    main.
  • Self-review caught and removed two leftover DBG console.error lines
    and a stray blank line; re-verified after cleanup (tsc, build,
    expect-actual + cross-import suites 13/13).
  • Upstream did not move during the work (base 2008e2f still current at
    filing); no rebase needed. One commit, 3 files, +167/-1.

RFC note

An RFC comment was posted on issue #3033 before implementation
(comment 6538257515, 2026-10-31) describing the reproduction, the root
cause in src/resolution/import-resolver.ts, and the planned fix. No
maintainer reply arrived after a full review round, so this PR is filed
for review with the RFC on record.

Risk

Callers that previously (wrongly) linked expect now link actual.
commonMain callers are unchanged by design. The change is gated to the
expect/actual owner pick only; ordinary Java/Kotlin import resolution is
untouched, and the #3023 regression surface stays green.

…tual its source set holds (colbymchenry#3033)

resolveJavaImportedReference (colbymchenry#3023) reads an imported Kotlin
Multiplatform declaration by qualified name, which returns BOTH the
commonMain expect and the platform actuals, and picks the owner with
pickClosestJvmCandidate — a rule written for a commonSide import ref,
which prefers an expect on a directory-proximity tie. A Java caller in
an app module shares no directory segments with either, so the tie
fired and the call landed on the expect — a declaration JVM bytecode
never contains — instead of the actual: timber's Java sample calling
Timber.plant(...) or extending Timber.Tree reached the commonMain
declarations.

The pick is now caller-aware: a Java caller, and a Kotlin caller in a
platform source set, take the actuals — the one in the caller's own
source set when there are several, proximity among them otherwise —
while a commonMain caller keeps the expect its compile sees. The
import-ref itself (resolveJvmImport) keeps the commonSide reading.
Kotlin callers in a source set the library holds were already right
through proximity; the tie-break was what betrayed callers outside it.
colbymchenry added a commit that referenced this pull request Nov 1, 2026
…against (#3191)

A Kotlin Multiplatform `expect` and its `actual`s share one qualified name,
and a reference through an import took the nearest declaration, the `expect`
on a tie. That suits common code only. PR #3023 sent Java's calls through this
pick, so `Util.go()` from `app/src/main/java`, which 1.6.2 linked to the
`androidMain` `actual` by a name guess, reached the `commonMain` `expect`, and
so did `extends Timber.Tree`, which 1.6.2 left unresolved. An `expect` isn't in
JVM bytecode, so a Java caller can never reach one. Kotlin in `androidUnitTest`
or a plain Android module went the same way.

The fix: pickJvmDeclaration chooses by what the caller compiles against, for
the owner of an imported reference and for the import itself. Common source
sets (`commonMain`, `commonTest`, okhttp's `commonJvmAndroid`) keep the nearest
declaration. A platform's source sets take that platform's `actual`, so
`androidUnitTest` takes `androidMain`'s, or that of a target they build on
(`iosArm64Main` from `iosMain`), and otherwise the `expect`, as an intermediate
set like `nativeMain` sees. Java and a plain Kotlin module take a JVM target's
`actual`, never `iosMain`'s: the one their path names, else not Android's, so
okhttp's samples reach `jvmMain` and its `android-test` `androidMain`.

This reimplements the idea of PR #3136 by @johnhldiver-beep, who found the
cause. Beyond it: `commonTest` stays on the `expect` (that PR's check named
`commonMain` alone), `androidUnitTest` finds `androidMain`, Java never lands on
a native target's `actual`, and the import edge agrees with the calls.

Validation:
- kotlin-expect-actual-callers.test.ts, 10 cases built from the issue's repro
  and timber's and okhttp's shapes: 8 fail on main, all pass.
- 1.6.2 run first: the Java cases link the `actual` by a 0.65 guess, so this is
  a regression from #3023, as the issue says.
- A/B against main 4499d00: okhttp 9 imports and references move from the
  `expect` in `commonJvmAndroid` to the platform `actual`, each to the target
  its module builds for; nowinandroid, mihon and every Java repo unchanged,
  having no `expect`. Controls flask and express identical.
- CPU, mihon, median of 3 interleaved: 20.02 to 19.86 s. Resolution only.

Fixes #3033

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@colbymchenry

Copy link
Copy Markdown
Owner

Thanks @johnhldiver-beep, you found the cause of #3033: a caller should reach the actual its own source set builds against, not the nearest declaration. We landed that in #3191 and took it a little further, so commonTest stays on the expect, androidUnitTest finds androidMain, and a Java caller never lands on a native actual. You're credited in the changelog, so I'm closing this in favor of #3191.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Kotlin Multiplatform: a Java call to a Kotlin object links the commonMain expect instead of the platform actual

2 participants