From 0e71390b9f7a0f45eb525c612c8c1eb78fe9b851 Mon Sep 17 00:00:00 2001 From: Kunal Roy Choudhury Date: Mon, 9 Nov 2026 21:29:09 +0530 Subject: [PATCH] fix(kotlin): type a member call's receiver and its implicit this Kotlin calls were resolved by name against functions only, so a call on a constructor property, a parameter, a typed or constructed local, an object or companion, or an unqualified member of the same class produced no edge, while the Java twin of each resolved. `callers FooService.handle` reported nothing for a service wired by constructor injection (#431). Bind `val`/`var` constructor parameters, function parameters, and typed or constructor-initialised properties and locals, scoped the way the walk scopes them. Read `this.repo.call()` as a field access, treat an UpperCamelCase receiver as the type itself (objects, companions), and let a lowercase bare call inside a type resolve to the member first and the top-level function otherwise, as Swift does. A lambda parameter, loop variable, destructured name, untyped local or function-typed parameter hides a field of the same name, `this@Outer` means the outer class, and a bare call inside `apply`, `run`, `with` or a `buildX` lambda is not read as a member of the enclosing class. --- src/graph/bindings.ts | 118 ++++++++++++++++++++- src/graph/extract.ts | 37 ++++++- test/graph-kotlin-members.test.ts | 168 ++++++++++++++++++++++++++++++ 3 files changed, 317 insertions(+), 6 deletions(-) create mode 100644 test/graph-kotlin-members.test.ts diff --git a/src/graph/bindings.ts b/src/graph/bindings.ts index f637ff449..4fb726460 100644 --- a/src/graph/bindings.ts +++ b/src/graph/bindings.ts @@ -19,14 +19,22 @@ export class FileBindings { private map = new Map(); set(scopePath: string, name: string, type: string): void { - this.map.set(`${scopePath}|${name}`, type); + const key = `${scopePath}|${name}`; + if (this.map.get(key) === "") return; + this.map.set(key, type); + } + + /** `name` is declared in this scope with a type we cannot read, so it hides any outer + * binding of the same name and nothing binds it again. */ + shadow(scopePath: string, name: string): void { + this.map.set(`${scopePath}|${name}`, ""); } /** Innermost-first: for scope ["a","b"] name "x", tries `a.b|x`, `a|x`, `|x`. */ lookup(scope: string[], name: string): string | null { for (let i = scope.length; i >= 0; i--) { const hit = this.map.get(`${scope.slice(0, i).join(".")}|${name}`); - if (hit) return hit; + if (hit !== undefined) return hit || null; } return null; } @@ -60,6 +68,7 @@ export function defName(node: SyntaxNode, lang: Language): string | null { } if (lang === "r") return rDefName(node); if (lang === "swift") return swiftDefName(node); + if (lang === "kotlin") return kotlinDefName(node); if (lang === "php") { const phpDefTypes = new Set([ "class_declaration", @@ -130,6 +139,28 @@ function swiftDefName(node: SyntaxNode): string | null { return null; } +/** Kotlin definitions that push a scope segment, mirroring extract.ts's describeKotlin: + * types and objects by their `type_identifier`, functions by their `simple_identifier`, + * a secondary constructor under its class, and a top-level property under its name. An + * unnamed `companion object` pushes nothing, so its members scope under the outer class. */ +function kotlinDefName(node: SyntaxNode): string | null { + if (node.type === "class_declaration" || node.type === "object_declaration" || node.type === "type_alias") { + return node.namedChildren.find((c) => c.type === "type_identifier")?.text ?? null; + } + if (node.type === "function_declaration") { + return node.namedChildren.find((c) => c.type === "simple_identifier")?.text ?? null; + } + if (node.type === "secondary_constructor") { + const owner = node.parent?.parent; + return owner ? kotlinDefName(owner) : null; + } + if (node.type === "property_declaration" && node.parent?.type === "source_file") { + const decl = node.namedChildren.find((c) => c.type === "variable_declaration"); + return decl?.namedChildren.find((c) => c.type === "simple_identifier")?.text ?? null; + } + return null; +} + const R_ASSIGN_OPS = new Set(["<-", "<<-", "="]); const R_RIGHT_ASSIGN_OPS = new Set(["->", "->>"]); @@ -223,7 +254,7 @@ export function resolveRecvType( // Swift type-member call `Animal.staticThing()`: an uppercase receiver with no // local binding is the type itself (Swift naming: types are UpperCamelCase, // values lowerCamelCase — and a shadowing binding was already tried above). - (ctx.lang === "swift" && /^[A-Z]/.test(receiver) ? receiver : undefined) ?? + ((ctx.lang === "swift" || ctx.lang === "kotlin") && /^[A-Z]/.test(receiver) ? receiver : undefined) ?? // Python type-member call `SomeClass.method()`: classmethods and staticmethods // are invoked on the class itself, so an uppercase receiver with no local // binding is the type. Same shape as Swift above, and PEP 8 gives the same @@ -253,6 +284,7 @@ function isClassNode(node: SyntaxNode, lang: Language): boolean { if (lang === "swift") { return node.type === "class_declaration" || node.type === "protocol_declaration"; } + if (lang === "kotlin") return node.type === "class_declaration" || node.type === "object_declaration"; return false; } @@ -323,6 +355,7 @@ function visit( else if (lang === "r") void 0; else if (lang === "java") handleJava(node, scope, classScope, bindings); else if (lang === "swift") handleSwift(node, scope, classScope, bindings); + else if (lang === "kotlin") handleKotlin(node, scope, classScope, bindings); else if (lang === "php") handlePhp(node, scope, bindings); else handleTs(node, scope, classScope, bindings, aliases); @@ -405,6 +438,85 @@ function handleSwift( if (isField) bindings.set(target, `self.${name}`, type); } +/** Kotlin variable->type bindings from the syntax-local clues: a `val`/`var` constructor + * parameter (a field), a function parameter, and a `val`/`var` with a declared type or a + * constructor-call initializer (`val repo = Repo()`), as a field in a class body or a + * local elsewhere. A plain constructor parameter (no `val`/`var`) is not visible to the + * methods, so it binds nothing. */ +function handleKotlin( + node: SyntaxNode, + scope: string[], + classScope: string | null, + bindings: FileBindings, +): void { + const scopePath = scope.join("."); + const typeOf = (n: SyntaxNode) => + kotlinTypeName(n.namedChildren.find((c) => c.type === "user_type" || c.type === "nullable_type")); + if (node.type === "class_parameter") { + if (!node.namedChildren.some((c) => c.type === "binding_pattern_kind")) return; + const name = node.namedChildren.find((c) => c.type === "simple_identifier")?.text; + const type = typeOf(node); + if (!name || !type) return; + const target = classScope ?? scopePath; + bindings.set(target, name, type); + bindings.set(target, `self.${name}`, type); + return; + } + if (node.type === "parameter") { + const name = node.namedChildren.find((c) => c.type === "simple_identifier")?.text; + if (!name) return; + const type = typeOf(node); + if (type) bindings.set(scopePath, name, type); + else bindings.shadow(scopePath, name); + return; + } + if (node.type === "variable_declaration" && KOTLIN_LOOSE_DECLARATIONS.has(node.parent?.type ?? "")) { + // A lambda parameter, a `for` variable or a destructured name: it hides a field of the + // same name, and its type is only known when written out. + const name = node.namedChildren.find((c) => c.type === "simple_identifier")?.text; + if (!name) return; + const type = typeOf(node); + if (type) bindings.set(scopePath, name, type); + else bindings.shadow(scopePath, name); + return; + } + if (node.type !== "property_declaration") return; + const decl = node.namedChildren.find((c) => c.type === "variable_declaration"); + const name = decl?.namedChildren.find((c) => c.type === "simple_identifier")?.text; + if (!decl || !name) return; + const isField = node.parent?.type === "class_body"; + const target = isField ? (classScope ?? scopePath) : scopePath; + const type = typeOf(decl) ?? kotlinCtorTypeName(node.namedChildren.find((c) => c.type === "call_expression")); + if (!type) { + bindings.shadow(target, name); + return; + } + bindings.set(target, name, type); + if (isField) bindings.set(target, `self.${name}`, type); +} + +const KOTLIN_LOOSE_DECLARATIONS = new Set(["lambda_parameters", "for_statement", "multi_variable_declaration"]); + +/** A Kotlin type node's bare name: a `user_type`'s LAST type_identifier (a qualified + * `a.b.Repo` binds as `Repo`; generic arguments live in nested nodes), unwrapping a + * nullable (`Repo?`). Function types, collections and the like bind nothing. */ +function kotlinTypeName(node: SyntaxNode | null | undefined): string | null { + if (!node) return null; + if (node.type === "nullable_type") return kotlinTypeName(node.namedChildren.find((c) => c.type === "user_type")); + if (node.type !== "user_type") return null; + return node.namedChildren.filter((c) => c.type === "type_identifier").at(-1)?.text ?? null; +} + +/** The type built by a constructor call (`Repo()` -> `Repo`), or null when the callee is + * not a bare UpperCamelCase name: a lowercase callee is a function whose return type a + * single-file pass cannot know. */ +function kotlinCtorTypeName(call: SyntaxNode | null | undefined): string | null { + if (call?.type !== "call_expression") return null; + const fn = call.namedChildren[0]; + if (fn?.type !== "simple_identifier") return null; + return /^[A-Z]/.test(fn.text) ? fn.text : null; +} + /** A Swift type node's bare name: a `user_type`'s LAST type_identifier (so a * module-qualified `Foundation.Date` binds as `Date`, generic arguments live in * nested nodes and never leak in), unwrapping one level of optional (`Animal?`). diff --git a/src/graph/extract.ts b/src/graph/extract.ts index b18a56c01..f746c10d5 100644 --- a/src/graph/extract.ts +++ b/src/graph/extract.ts @@ -775,7 +775,7 @@ function walk(node: SyntaxNode, ctx: WalkCtx, out: NodeV1[], edges: RawEdge[], m // resolve.ts's class/struct/enum fallback instead — extension nodes (kind // "module") can never false-match it. const swiftImplicitSelf = - ctx.lang === "swift" && + (ctx.lang === "swift" || (ctx.lang === "kotlin" && !kotlinInReceiverLambda(node))) && !callee.viaMember && !callee.kinds && ctx.enclosingClass && @@ -1175,6 +1175,22 @@ function isFunctionBoundary(node: SyntaxNode): boolean { ); } +const KOTLIN_RECEIVER_SCOPES = new Set(["apply", "run", "with", "buildString", "buildList", "buildMap", "buildSet"]); + +/** Is this Kotlin call inside a lambda whose implicit receiver is another object + * (`list.apply { add(x) }`, `with(sb) { append(x) }`)? A bare call there reaches that + * receiver's member, which this pass cannot read, so it must not read as the enclosing class's. */ +function kotlinInReceiverLambda(node: SyntaxNode): boolean { + for (let n = node.parent; n && n.type !== "function_declaration"; n = n.parent) { + if (n.type !== "lambda_literal") continue; + let callee = n.parent?.parent?.parent?.namedChildren[0]; + if (callee?.type === "call_expression") callee = callee.namedChildren[0]; + if (callee?.type === "navigation_expression") callee = callee.namedChildren.at(-1)?.namedChildren[0]; + if (callee?.type === "simple_identifier" && KOTLIN_RECEIVER_SCOPES.has(callee.text)) return true; + } + return false; +} + /** A direct invocation already emits a stronger `calls` edge. Java names the callee * in a `name` field (there is no `function` field on `method_invocation`), so both * spellings count. */ @@ -2395,8 +2411,23 @@ if (lang === "kotlin") { if (!name) return null; if (receiver?.type === "simple_identifier") return { name: name.text, viaMember: true, receiver: receiver.text }; - if (receiver?.type === "this_expression" || receiver?.type === "super_expression") - return { name: name.text, viaMember: true, receiver: receiver.type === "this_expression" ? "this" : "super" }; + if (receiver?.type === "this_expression" || receiver?.type === "super_expression") { + // `this@Outer` is the enclosing class `Outer`, not the innermost one. + const label = receiver.namedChildren.find((c) => c.type === "type_identifier"); + const self = receiver.type === "this_expression" ? "this" : "super"; + return { name: name.text, viaMember: true, receiver: label ? label.text : self }; + } + if (receiver?.type === "navigation_expression") { + // `this.repo.save()` — one hop off `this` is a field access and binds like TS's + // `this.x`; deeper chains and call-result receivers carry no confident clue. + const field = receiver.namedChildren + .find((c) => c.type === "navigation_suffix") + ?.namedChildren.find((c) => c.type === "simple_identifier"); + const owner = receiver.namedChildren[0]; + if (owner?.type === "this_expression" && owner.namedChildren.length === 0 && field) + return { name: name.text, viaMember: true, receiver: `this.${field.text}` }; + } + return { name: name.text, viaMember: true }; } return null; } diff --git a/test/graph-kotlin-members.test.ts b/test/graph-kotlin-members.test.ts new file mode 100644 index 000000000..6f9c7655b --- /dev/null +++ b/test/graph-kotlin-members.test.ts @@ -0,0 +1,168 @@ +/** + * Kotlin member calls as call edges (#431). + * + * A Kotlin service reaches its collaborators through a constructor property + * (`class Bar(private val foo: FooService)`), a function parameter, a typed or + * constructed local, an object/companion (`Utils.helper()`), or just its own + * members (`other()`). None of those had a receiver type or an enclosing owner, so + * `callers FooService.handle` reported nothing while the Java twin found them all. + * + * What these pin: each receiver shape lands on the owning type's method; a bare + * call to a top-level function still resolves as a function; and a name that is no + * member of the receiver's type resolves to nothing rather than to a lookalike. + */ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { mkdirSync, writeFileSync } from "node:fs"; +import { dirname, join } from "node:path"; +import { buildGraph } from "../src/graph/build.js"; +import { readGraph, wiringPath } from "../src/graph/write.js"; +import { tmpRepo } from "./helpers.js"; +import type { GraphV1 } from "../src/graph/types.js"; + +async function buildKotlin(files: Record): Promise { + const root = tmpRepo("kotlin-members-"); + for (const [rel, src] of Object.entries(files)) { + const file = join(root, rel); + mkdirSync(dirname(file), { recursive: true }); + writeFileSync(file, src); + } + await buildGraph(root, { reuse: false }); + return readGraph(wiringPath(join(root, "graft")))!; +} + +const calls = (g: GraphV1, source: string) => + g.edges.filter((e) => e.relation === "calls" && e.source === source).map((e) => e.target).sort(); + +const FOO_KT = `package app + +class FooService { + fun handle(x: Int): Int = x + fun audit() {} +} + +class Other { + fun handle(x: Int): Int = x +} + +object Utils { + fun helper(x: Int): Int = x +} + +class Registry { + companion object { + fun create(): Registry = Registry() + } +} + +fun topLevel() {} +`; + +const BAR_KT = `package app + +class Bar(private val foo: FooService, val other: Other, plain: FooService) { + private val made = FooService() + private val typed: FooService? = null + private val unknown = build() + + fun viaProperty() { foo.handle(1) } + fun viaThis() { this.foo.handle(2) } + fun viaField() { made.audit(); typed?.audit() } + fun viaParam(p: FooService) { p.handle(3) } + fun viaLocal() { + val local = FooService() + local.handle(4) + val declared: Other = build() + declared.handle(5) + } + fun viaObject() { Utils.helper(6); Registry.create() } + fun implicitThis() { sibling(); topLevel() } + fun sibling() {} + fun nonMember() { foo.nope(); unknown.handle(7); plain.handle(8) } +} +`; + +test("kotlin members: property, parameter, local, object and implicit-this calls resolve to the owner's method", async () => { + const g = await buildKotlin({ "Foo.kt": FOO_KT, "Bar.kt": BAR_KT }); + assert.deepEqual(calls(g, "Bar.kt#Bar.viaProperty"), ["Foo.kt#FooService.handle"]); + assert.deepEqual(calls(g, "Bar.kt#Bar.viaThis"), ["Foo.kt#FooService.handle"]); + assert.deepEqual(calls(g, "Bar.kt#Bar.viaField"), ["Foo.kt#FooService.audit"]); + assert.deepEqual(calls(g, "Bar.kt#Bar.viaParam"), ["Foo.kt#FooService.handle"]); + assert.deepEqual(calls(g, "Bar.kt#Bar.viaLocal"), ["Foo.kt#FooService", "Foo.kt#FooService.handle", "Foo.kt#Other.handle"]); + assert.deepEqual(calls(g, "Bar.kt#Bar.viaObject"), ["Foo.kt#Registry.create", "Foo.kt#Utils.helper"]); + assert.deepEqual(calls(g, "Bar.kt#Bar.implicitThis"), ["Bar.kt#Bar.sibling", "Foo.kt#topLevel"]); +}); + +test("kotlin members: an unknown member, an unbound receiver and a plain constructor parameter resolve to nothing", async () => { + const g = await buildKotlin({ "Foo.kt": FOO_KT, "Bar.kt": BAR_KT }); + assert.deepEqual(calls(g, "Bar.kt#Bar.nonMember"), []); +}); + +const SHADOW_KT = `package app + +class Shadow(private val foo: FooService, private val other: Other) { + fun lambdaParam(xs: List) { xs.forEach { foo -> foo.handle(1) } } + fun untypedLocal() { + val foo = build() + foo.handle(2) + } + fun forVariable(xs: List) { for (foo in xs) foo.handle(3) } + fun destructured(pair: Pair) { + val (foo, _) = pair + foo.handle(4) + } + fun opaqueParam(foo: (Int) -> Int) { foo.handle(5) } + fun typedLambdaParam(xs: List) { xs.forEach { foo: Other -> foo.handle(6) } } + fun stillTheField() { foo.handle(7); other.handle(8) } + fun viaThis(xs: List) { xs.forEach { foo -> this.foo.handle(9) } } +} +`; + +test("kotlin members: a lambda parameter, an untyped local, a loop variable or a destructured name hides the field of that name", async () => { + const g = await buildKotlin({ "Foo.kt": FOO_KT, "Shadow.kt": SHADOW_KT }); + assert.deepEqual(calls(g, "Shadow.kt#Shadow.lambdaParam"), []); + assert.deepEqual(calls(g, "Shadow.kt#Shadow.untypedLocal"), []); + assert.deepEqual(calls(g, "Shadow.kt#Shadow.forVariable"), []); + assert.deepEqual(calls(g, "Shadow.kt#Shadow.destructured"), []); + assert.deepEqual(calls(g, "Shadow.kt#Shadow.opaqueParam"), []); + assert.deepEqual(calls(g, "Shadow.kt#Shadow.typedLambdaParam"), ["Foo.kt#Other.handle"]); + assert.deepEqual(calls(g, "Shadow.kt#Shadow.stillTheField"), ["Foo.kt#FooService.handle", "Foo.kt#Other.handle"]); + assert.deepEqual(calls(g, "Shadow.kt#Shadow.viaThis"), ["Foo.kt#FooService.handle"]); +}); + +test("kotlin members: a bare call inside another object's receiver lambda is not the enclosing class's member", async () => { + const g = await buildKotlin({ + "Cart.kt": `package app + +class Cart { + fun add(x: Int) {} + fun viaApply() { mutableListOf().apply { add(1) } } + fun viaWith(sb: StringBuilder) { with(sb) { add(2) } } + fun viaBuilder() = buildList { add(3) } + fun viaForEach(xs: List) { xs.forEach { add(it) } } +} +`, + }); + assert.deepEqual(calls(g, "Cart.kt#Cart.viaApply"), []); + assert.deepEqual(calls(g, "Cart.kt#Cart.viaWith"), []); + assert.deepEqual(calls(g, "Cart.kt#Cart.viaBuilder"), []); + assert.deepEqual(calls(g, "Cart.kt#Cart.viaForEach"), ["Cart.kt#Cart.add"]); +}); + +test("kotlin members: this@Outer reaches the outer class, not the inner one", async () => { + const g = await buildKotlin({ + "Outer.kt": `package app + +class Outer { + fun save() {} + inner class Inner { + fun save() {} + fun viaOuter() { this@Outer.save() } + fun viaSelf() { this.save() } + } +} +`, + }); + assert.deepEqual(calls(g, "Outer.kt#Outer.Inner.viaOuter"), ["Outer.kt#Outer.save"]); + assert.deepEqual(calls(g, "Outer.kt#Outer.Inner.viaSelf"), ["Outer.kt#Outer.Inner.save"]); +});