fix(kotlin): a Multiplatform import reaches what the caller compiles against

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)

__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<string, string> = {
+  '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]);
+  });
+});

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/<set>/`. */
+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)`.