From a95580557df67c8d1d341bef1b3dacb24f30acd4 Mon Sep 17 00:00:00 2001 From: Colby McHenry Date: Sun, 1 Nov 2026 03:03:26 -0600 Subject: [PATCH] fix(kotlin): a Multiplatform import reaches what the caller compiles against 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 4499d007: 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 --- CHANGELOG.md | 1 + .../kotlin-expect-actual-callers.test.ts | 128 ++++++++++++++++++ src/resolution/import-resolver.ts | 54 +++++++- 3 files changed, 178 insertions(+), 5 deletions(-) create mode 100644 __tests__/kotlin-expect-actual-callers.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 106f2518a..26902887f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -342,6 +342,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - In Kotlin, a call on a library type you import, like Jetpack Compose's `Modifier.pad()`, now links to the extension your project declares on that type, `fun Modifier.pad()`, when the calling file can see it: in the same package, or imported by name or with `.*`. The library type's own methods, like `Modifier.fillMaxSize()`, still never link to a project method that shares the name. Re-index Kotlin projects after upgrading. Thanks @danusha2345. - In Kotlin, when two packages each declare a class with the same name, a call on a value of that type now links to the class the file imports, even when that class lives in a file named for something else, like `FooConverter` in `Converters.kt`. Before, it went to whichever of the two came first. Java calls on such a class, like okhttp's Java tests calling the `mockwebserver3.MockWebServer` they import, now link through the type their variable is declared with (#1942). Re-index Kotlin projects after upgrading. Thanks @arttttt. - In Kotlin and Java, a call through an import of a class written in the other language now links to that class: Kotlin's `JUtil.make()`, `JUtil.Nested.deep()` or `Outer.Inner()` on an imported Java class, and Java's `KUtil.INSTANCE.kmake()` on a Kotlin object, `Factory.Companion.create()` on a companion object or `UtilsKt.helper()` on a Kotlin file's top-level function, whose import now links that file. Before, the import was only matched against files in the calling file's own language, so the call went to a same-named class in another package, or to nothing. A Kotlin extension declared in another package, like `fun Modes.hidden()`, is now reached only from a file that imports it or its package. Re-index Kotlin and Java projects after upgrading. Thanks @mixxer. +- In Kotlin Multiplatform projects, a call through an import now links to what the caller compiles against: common code keeps the `expect`, while Java, a plain Kotlin module and a platform's source sets reach that platform's `actual`, since an `expect` doesn't exist in JVM bytecode. Before, timber's Java sample calling `Timber.plant(…)` or extending `Timber.Tree` landed on the `commonMain` declaration. Thanks @inth3shadows for the report. Thanks @johnhldiver-beep. (#3033) - In Java, Kotlin and C#, a supertype written through its enclosing type, like `class Named extends Outer.Inner`, `implements Api.Listener`, `new Api.Listener() { … }`, Kotlin's `class K : Outer.Inner()` or C#'s `class N : Outer.IListener`, now links to that nested type when the file can name the outer type, and a method a class inherits through a nested type, like a builder's `build()` from its `AbstractBuilder`, is that type's rather than a namesake's. Before, Java and C# left it unlinked, a Java anonymous class kept only the last name, and Kotlin linked the outer type instead. A Kotlin object expression, like `object : Listener { … }` passed as a callback or held in a property, is now indexed as an anonymous class, as Java's `new Listener() { … }` is: its overrides are its own methods, so a call through the interface reaches them and `codegraph_explore` follows a callback into its body. Re-index Java, Kotlin and C# projects after upgrading. Thanks @mixxer. - In JavaScript and TypeScript, a `super.run()` call now links to the method of the class's parent, or of the nearest ancestor that has it, like three.js's `super.copy(source)` reaching `Object3D.copy`, so `codegraph_explore`'s call path and impact follow an override into the method it builds on. Before, such a call inside a method of the same name was dropped, and elsewhere it went to whatever method had the name, often the class's own or a sibling class's in the same file. A call from a static method reaches the parent's static method, and a call to a parent from outside your project, like `HTMLElement`, `Set` or `Promise`, links nothing. Re-index JavaScript and TypeScript projects after upgrading. Thanks @Puma7 for the report and the pull request. (#2497) - In JavaScript and TypeScript, a named class expression, like `register(class Widget extends Base {…})`, `customElements.define('x-card', class XCard extends HTMLElement {…})` or `const Panel = class PanelImpl extends Base {…}`, is now indexed as a class with its own methods and the class it extends, so calls between its methods and its `super` calls link as a class declaration's do. Before, its methods were free functions or weren't indexed at all, and it extended nothing. Its name stays bound inside the expression alone, as JavaScript has it: `new Widget()` elsewhere links nothing, a constant, class or interface of the same name around it keeps its links, and code outside never reaches its methods by a guess. A class a CommonJS module exports, like webpack's `module.exports = class MultiCompiler {…}` or `exports.Store = class Store {…}`, is reached through `require` as a class declaration is, along with its static methods and the methods called on its instances. Re-index JavaScript and TypeScript projects after upgrading. Thanks @Puma7 for the report and the pull request. (#2499) diff --git a/__tests__/kotlin-expect-actual-callers.test.ts b/__tests__/kotlin-expect-actual-callers.test.ts new file mode 100644 index 000000000..70f366838 --- /dev/null +++ b/__tests__/kotlin-expect-actual-callers.test.ts @@ -0,0 +1,128 @@ +/** + * A Kotlin Multiplatform `expect` and its `actual`s share one qualified name, so + * a reference through an import had to choose, and the nearest declaration with + * the `expect` on a tie suits only common code (#3033). What a caller reaches is + * what its compilation sees: common code the `expect`; a platform's source sets + * that platform's `actual`, or one of a target they build on; Java, and a plain + * Kotlin module, JVM bytecode, in which no `expect` exists and no `iosMain` lives. + */ +import { describe, it, expect, afterAll, beforeAll } from 'vitest'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { CodeGraph } from '../src'; + +const CALLER = (pkg: string, fn: string): string => `package ${pkg}\n\nimport a.Util\n\nfun ${fn}() = Util.go()\n`; + +const FILES: Record = { + 'lib/src/commonMain/kotlin/a/Util.kt': 'package a\n\nexpect object Util {\n fun go()\n}\n', + 'lib/src/androidMain/kotlin/a/Util.kt': 'package a\n\nactual object Util {\n @JvmStatic\n actual fun go() {}\n}\n', + 'lib/src/iosMain/kotlin/a/Util.kt': 'package a\n\nactual object Util {\n actual fun go() {}\n}\n', + // timber's shape: a class with a nested type and a companion. + 'lib/src/commonMain/kotlin/t/Timber.kt': 'package t\n\nexpect class Timber {\n class Tree\n companion object {\n fun plant(tree: Tree)\n }\n}\n', + 'lib/src/androidMain/kotlin/t/Timber.kt': 'package t\n\nactual class Timber {\n actual class Tree\n actual companion object {\n @JvmStatic\n actual fun plant(tree: Tree) {}\n }\n}\n', + // Only one target that isn't the JVM has an `actual`. + 'lib/src/commonMain/kotlin/n/OnlyNative.kt': 'package n\n\nexpect object OnlyNative {\n fun go()\n}\n', + 'lib/src/iosMain/kotlin/n/OnlyNative.kt': 'package n\n\nactual object OnlyNative {\n actual fun go() {}\n}\n', + // okhttp's shape: an intermediate source set declares the `expect`, and two JVM targets have an `actual`. + 'lib/src/commonJvmAndroid/kotlin/s/Version.kt': 'package s\n\nexpect object Version {\n fun name(): String\n}\n', + 'lib/src/androidMain/kotlin/s/Version.android.kt': 'package s\n\nactual object Version {\n actual fun name() = "android"\n}\n', + 'lib/src/jvmMain/kotlin/s/Version.jvm.kt': 'package s\n\nactual object Version {\n actual fun name() = "jvm"\n}\n', + 'samples/survey/src/main/kotlin/e/Clients.kt': 'package e\n\nimport s.Version\n\nfun jvmSample() = Version.name()\n', + 'android-test/src/main/kotlin/e/Screen.kt': 'package e\n\nimport s.Version\n\nfun androidApp() = Version.name()\n', + 'lib/src/commonJvmAndroid/kotlin/e/Shared.kt': 'package e\n\nimport s.Version\n\nfun sharedCode() = Version.name()\n', + 'app/src/main/java/b/App.java': 'package b;\n\nimport a.Util;\nimport t.Timber;\n\nclass App {\n void run() { Util.go(); }\n void grow() { Timber.plant(new Timber.Tree()); }\n}\n\nclass Log extends Timber.Tree {\n}\n', + 'app/src/main/java/b/Native.java': 'package b;\n\nimport n.OnlyNative;\n\nclass Native {\n void run() { OnlyNative.go(); }\n}\n', + 'app/src/main/kotlin/d/Plain.kt': CALLER('d', 'plainModule'), + 'lib/src/commonMain/kotlin/c/Common.kt': CALLER('c', 'inCommonMain'), + 'lib/src/commonTest/kotlin/c/CommonTest.kt': CALLER('c', 'inCommonTest'), + 'lib/src/androidMain/kotlin/c/Android.kt': CALLER('c', 'inAndroidMain'), + 'lib/src/androidUnitTest/kotlin/c/AndroidTest.kt': CALLER('c', 'inAndroidUnitTest'), + 'lib/src/iosMain/kotlin/c/Ios.kt': CALLER('c', 'inIosMain'), + 'lib/src/iosArm64Main/kotlin/c/Arm.kt': CALLER('c', 'inIosArm64Main'), + 'lib/src/nativeMain/kotlin/c/Native.kt': CALLER('c', 'inNativeMain'), +}; + +let root = ''; +let cg: CodeGraph; + +beforeAll(async () => { + root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-kmp-callers-')); + for (const [rel, text] of Object.entries(FILES)) { + fs.mkdirSync(path.dirname(path.join(root, rel)), { recursive: true }); + fs.writeFileSync(path.join(root, rel), text); + } + cg = await CodeGraph.init(root, { index: true }); +}, 60_000); + +afterAll(() => { + cg?.close(); + if (root) fs.rmSync(root, { recursive: true, force: true }); +}); + +/** The files a function or method calls `callee` in. */ +function callsFrom(name: string, callee: string): string[] { + const from = [...cg.getNodesByKind('function'), ...cg.getNodesByKind('method')].find((n) => n.name === name); + expect(from, name).toBeDefined(); + return cg.getOutgoingEdges(from!.id).filter((e) => e.kind === 'calls').map((e) => cg.getNode(e.target)!) + .filter((n) => n.name === callee).map((n) => n.filePath).sort(); +} + +const COMMON = 'lib/src/commonMain/kotlin/a/Util.kt'; +const ANDROID = 'lib/src/androidMain/kotlin/a/Util.kt'; +const IOS = 'lib/src/iosMain/kotlin/a/Util.kt'; + +describe('a Kotlin Multiplatform import reaches what the caller compiles against (#3033)', () => { + it('takes a Java call to the JVM `actual`', () => { + expect(callsFrom('run', 'go')).toContain(ANDROID); + expect(callsFrom('run', 'go')).not.toContain(COMMON); + expect(callsFrom('grow', 'plant')).toEqual(['lib/src/androidMain/kotlin/t/Timber.kt']); + }); + + it('takes a Java `extends` through the type to the `actual`\'s nested type', () => { + const log = cg.getNodesByKind('class').find((n) => n.name === 'Log')!; + const supers = cg.getOutgoingEdges(log.id).filter((e) => e.kind === 'extends').map((e) => cg.getNode(e.target)!); + expect(supers.map((n) => `${n.name} ${n.filePath}`)).toEqual(['Tree lib/src/androidMain/kotlin/t/Timber.kt']); + }); + + it('takes the import of a Java file to the `actual` as well', () => { + const imported = cg.getNodesInFile('app/src/main/java/b/App.java').find((n) => n.kind === 'namespace')!; + const targets = cg.getOutgoingEdges(imported.id).filter((e) => e.kind === 'imports').map((e) => cg.getNode(e.target)!); + expect(targets.map((n) => n.filePath).sort()).toEqual([ANDROID, 'lib/src/androidMain/kotlin/t/Timber.kt']); + }); + + it('takes Java to the one `actual` there is, when no JVM target has one', () => { + const native = cg.getNodesInFile('app/src/main/java/b/Native.java').find((n) => n.name === 'run')!; + const targets = cg.getOutgoingEdges(native.id).filter((e) => e.kind === 'calls').map((e) => cg.getNode(e.target)!.filePath); + expect(targets).toContain('lib/src/iosMain/kotlin/n/OnlyNative.kt'); + }); + + it('takes a plain Kotlin module to the JVM `actual`', () => { + expect(callsFrom('plainModule', 'go')).toEqual([ANDROID]); + }); + + it('keeps common code on the `expect`', () => { + expect(callsFrom('inCommonMain', 'go')).toEqual([COMMON]); + expect(callsFrom('inCommonTest', 'go')).toEqual([COMMON]); + }); + + it('takes a platform\'s source sets to that platform\'s `actual`', () => { + expect(callsFrom('inAndroidMain', 'go')).toEqual([ANDROID]); + expect(callsFrom('inAndroidUnitTest', 'go')).toEqual([ANDROID]); + expect(callsFrom('inIosMain', 'go')).toEqual([IOS]); + }); + + it('takes a plain module to the JVM target its path names, and Android only by name', () => { + expect(callsFrom('jvmSample', 'name')).toEqual(['lib/src/jvmMain/kotlin/s/Version.jvm.kt']); + expect(callsFrom('androidApp', 'name')).toEqual(['lib/src/androidMain/kotlin/s/Version.android.kt']); + }); + + it('keeps a common source set of another name on the `expect`', () => { + expect(callsFrom('sharedCode', 'name')).toEqual(['lib/src/commonJvmAndroid/kotlin/s/Version.kt']); + }); + + it('takes a target to the `actual` of a target it builds on, and an intermediate set to the `expect`', () => { + expect(callsFrom('inIosArm64Main', 'go')).toEqual([IOS]); + expect(callsFrom('inNativeMain', 'go')).toEqual([COMMON]); + }); +}); diff --git a/src/resolution/import-resolver.ts b/src/resolution/import-resolver.ts index 861af26e9..4772cfe96 100644 --- a/src/resolution/import-resolver.ts +++ b/src/resolution/import-resolver.ts @@ -2732,10 +2732,9 @@ export function resolveJvmImport( // FQN across source sets (commonMain / androidMain / appleMain). Taking the // first candidate let a single platform `actual` absorb every common-side // import, so the `expect` (the canonical API a commonMain file imports) - // looked unused. Prefer the candidate CLOSEST to the importing file by - // directory proximity — a commonMain import resolves to the commonMain - // declaration — with the `expect` side as a tiebreak. - const best = candidates.length === 1 ? candidates[0]! : pickClosestJvmCandidate(candidates, ref.filePath); + // looked unused. A commonMain import resolves to the commonMain declaration, + // a platform's to its `actual` (pickJvmDeclaration). + const best = candidates.length === 1 ? candidates[0]! : pickJvmDeclaration(candidates, ref); return { original: ref, targetNodeId: best.id, @@ -2795,6 +2794,51 @@ function pickClosestJvmCandidate(candidates: Node[], fromPath: string): Node { return best; } +/** The source set a file is in: `androidMain` of `lib/src/androidMain/kotlin/a/Util.kt`; null outside any `src//`. */ +function kmpSourceSet(filePath: string): string | null { + return /(?:^|\/)src\/([^/]+)\//.exec(filePath)?.[1] ?? null; +} + +/** What a source set builds for: `android` of `androidMain` and `androidUnitTest`, `common` of `commonTest`. */ +function kmpBuildTarget(set: string): string { + return set.replace(/(?:Main|(?:Unit|Instrumented|Device|Host)?Test)$/, ''); +} + +/** Kotlin Multiplatform targets that build for the JVM. */ +const KMP_JVM_TARGET = /^(?:android|jvm|desktop)/; + +/** + * Which of a declaration's same-named Kotlin Multiplatform declarations a JVM + * reference reaches. A file compiled for common code sees the `expect`, so + * commonMain and commonTest keep the nearest declaration, the `expect` on a + * tie. Anything built for a platform sees the `actual`: Kotlin of that target's + * source sets its own (an androidUnitTest the androidMain one) or that of a + * target it builds on (`iosArm64Main` from `iosMain`), and with neither the + * `expect`, as an intermediate source set does. Java, and a Kotlin module with + * no source sets of its own, run against JVM bytecode, where no `expect` exists: + * timber's Java sample calling `Timber.plant(…)` is the `androidMain` `actual`, + * never an `iosMain` one. Of the JVM's targets the one its path names, else not + * Android's: okhttp's samples are `jvmMain`'s, its `android-test` `androidMain`'s. + */ +function pickJvmDeclaration(declarations: Node[], ref: UnresolvedRef): Node { + const actuals = declarations.filter((n) => n.decorators?.includes('actual')); + if (actuals.length === 0 || actuals.length === declarations.length) return pickClosestJvmCandidate(declarations, ref.filePath); + const set = kmpSourceSet(ref.filePath); + const target = set === null ? null : kmpBuildTarget(set); + const targetOf = (n: Node): string => kmpBuildTarget(kmpSourceSet(n.filePath) ?? ''); + if (target?.startsWith('common') || (target === null && ref.language !== 'java')) return pickClosestJvmCandidate(declarations, ref.filePath); + const own = actuals.filter((n) => targetOf(n) === target); + if (own.length > 0) return pickClosestJvmCandidate(own, ref.filePath); + if (ref.language === 'java' || target === 'main' || target === 'test') { + const jvm = actuals.filter((n) => KMP_JVM_TARGET.test(targetOf(n))); + const android = /android/i.test(ref.filePath); + const named = jvm.filter((n) => targetOf(n).startsWith('android') === android); + return pickClosestJvmCandidate(named.length > 0 ? named : jvm.length > 0 ? jvm : actuals, ref.filePath); + } + const above = actuals.filter((n) => targetOf(n) !== '' && target!.startsWith(targetOf(n))); + return pickClosestJvmCandidate(above.length > 0 ? above : declarations, ref.filePath); +} + /** * PHP scoped calls are encoded as "Alias.method" by both extractors. A use * mapping names a namespace, not a filesystem path, so resolve the receiver @@ -4138,7 +4182,7 @@ function resolveJavaImportedReference( const callOfFunction = matchesBare && ref.referenceKind === 'calls' && named.some((n) => n.kind === 'function' || n.kind === 'method'); if (declarations.length > 0 && !callOfFunction) { - const owner = declarations.length === 1 ? declarations[0]! : pickClosestJvmCandidate(declarations, ref.filePath); + const owner = declarations.length === 1 ? declarations[0]! : pickJvmDeclaration(declarations, ref); const target = jvmImportedMember(owner, memberPath, ref, context); // Of a member's overloads, the one the call's argument count fits: // `ItemStack.Hash.of(stack, registries)`, not `of(stack)`.