From 2aecd2ed40e7e18e859e3595811e914e0ddfa4af Mon Sep 17 00:00:00 2001 From: danusha2345 Date: Fri, 23 Oct 2026 17:58:09 -0500 Subject: [PATCH] fix(kotlin): a private property, and the members of a private class, stay in their file A Kotlin property was indexed without its visibility modifier, so the Kotlin file-local rule (#1745), which declines a private fun in another file, read every property as public: a call like token(...) could link to another file's @Volatile private var token, and JavaScript, Go or Python callers to a Kotlin test fixture's private val. A public member of a private class or object was reachable from other files too, though nothing outside its file can name it. Both extractors (wasm and the native kernel) now read a property's modifiers the way they read a function's, and a candidate inside a private Kotlin class or object is declined from another file. That second rule is Kotlin's alone: a Scala private[pkg] class, or one with a private constructor, is its package's, and a C# private nested class serves every file of its partial class. Applied to every language where private is file-local, as the PR had it, it removed 742 edges on http4s (right ones among them, like stream.receiveHeaders() on a private[h2] class H2Stream), 91 on scalachess and 10 on serilog (9 of them right typed edges). Validation: - cross-file-visibility.test.ts: 2 cases, red on main, green here; the kernel parity fixture gains a private val and an internal var. - Kernel parity (kotlin), 0 diffs: okhttp 588/619 files, mihon 891/940, nowinandroid 340/350, SystemUI's Kotlin 188/198 byte-identical, the same files deferred to wasm as with main's kernel; torture 1/1. - A/B against main 01e0ac3a (with the Kotlin receiver typing of #1933): okhttp -274, mihon -177, SystemUI -94, every one a call from another file onto a private Kotlin property or a member of a private class or object (okhttp's private inner AndroidQuery.enqueue and RealWebSocketTest's private TestStreams, mihon's private IntentFilter filter that collections' filter { } reached, SystemUI's private MyExecutor.execute); nothing added. nowinandroid, jextract, click and gin identical; against main 4eb8c2a5, http4s, scalachess and serilog were identical too. - Cost (mihon, median of 3 interleaved indexes, on main 4eb8c2a5): CPU 25.7 s on main vs 25.5 s, wall 8.2 s on both. Original PR #1761 by @danusha2345. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 1 + __tests__/cross-file-visibility.test.ts | 25 +++++++++++++ __tests__/fixtures/kernel-parity/torture.kt | 3 ++ codegraph-kernel/src/kotlin.rs | 8 +++-- src/extraction/languages/kotlin.ts | 35 ++++++++++-------- src/resolution/name-matcher.ts | 40 ++++++++++++++++++++- 6 files changed, 93 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f58218b4c7..8151941aa8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -255,6 +255,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - In Kotlin, a call on what one of your functions returns, like `repository().pending(1)` or `val repo = repository(); repo.pending(1)` with a `private fun repository(): RealRepo` in a test class, now links to the method of the type that function declares, including one the type inherits or an extension on it. Before, the chained call went to whichever `pending` the name matched first, like a test double's in another package, and the local was guessed from its name, which reached the interface. The function is found in the calling class, a class it inherits from or one around it, or at the top level, and a nested type like `Request.Builder` is that type, not another class's `Builder`. When the declared type has no such method, the call now links to nothing rather than a method of the same name elsewhere; a type from outside your project is matched by name as before. Re-index Kotlin projects after upgrading. Thanks @mindcipherrr for the report. (#2805) - In Kotlin, a call through a property now links to the method of the type the property is declared with, like `server.enqueue(…)` on a `lateinit var server: MockWebServer` or a call on a constructor property such as `class A(private val repo: Repo)`. So does a call through a chain like `engine.pump.drain()` or `Mode.ON.next()`, through a property one of your classes inherits, through a local set from a call like `val lease = HardwareLock.tryBegin()`, and through a local of the function around an anonymous `object : …`. Before, such a call went to an interface's method or to whichever method had the name: on okhttp, the tests' calls reached the deprecated `MockWebServer` instead of the one each file imports. A call on a type from outside your project, like `Regex` or a JDK class, no longer links to a same-named method of your own, unless your project extends that type with it. Re-index Kotlin projects after upgrading. Thanks @danusha2345. - In Kotlin, a `fun interface` is now indexed as an interface with its methods, like okhttp's `HttpLoggingInterceptor.Logger`. Before, it was read as a broken function, and when a comment followed it, the declaration after it went missing too, its members turning into loose functions. Re-index Kotlin projects after upgrading. Thanks @danusha2345. +- In Kotlin, a `private val` or `private var` property, and the members of a `private` class or object, no longer take calls from other files. Properties were indexed without their visibility, so a call like `token(...)` in one file could link to another class's `private var token`, and JavaScript, Go or Python callers could land on a Kotlin test's private property; a call to `now()` could also land on a method of a test's `private class Clock`. Re-index Kotlin projects after upgrading. Thanks @danusha2345. - 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) - In Go, a call through a struct field that holds a function, like prometheus's `d.refreshf(ctx)`, now reaches the functions put in that field, so `codegraph_explore` follows service discovery from `Discovery.Run` into each provider's `refresh` and names the line where each provider wires its method in, like `RefreshF: d.refresh`. The field can be set in a struct literal or by an assignment, copied from another struct's field (`refreshf: opts.RefreshF`), or given a method of an interface value, and two unrelated structs with a field of the same name, like `fn` or `handler`, stay apart. Before, such a flow stopped at the field. Re-index Go projects after upgrading. diff --git a/__tests__/cross-file-visibility.test.ts b/__tests__/cross-file-visibility.test.ts index 1cb0007c94..7dfe5e4706 100644 --- a/__tests__/cross-file-visibility.test.ts +++ b/__tests__/cross-file-visibility.test.ts @@ -96,6 +96,31 @@ describe('Kotlin: a private function is class- or file-local', () => { }); }); +describe('Kotlin: a private property is class- or file-local too', () => { + it('records the modifier on the property and declines a call onto it from another file', async () => { + project({ + 'WebRtcTransport.kt': 'class WebRtcTransport {\n @Volatile private var token = ""\n internal val shared = 1\n}\n', + 'CommandValidator.kt': 'class CommandValidator {\n fun validate(command: Map): String? {\n if (!token(command["sessionId"], 128)) return "bad"\n return null\n }\n}\n', + }); + expect(await calleesOf('validate')).not.toContain('WebRtcTransport.kt:token'); + const fields = cg!.getNodesByKind('field').filter((n) => n.filePath.endsWith('WebRtcTransport.kt')); + expect(fields.find((n) => n.name === 'token')?.visibility).toBe('private'); + expect(fields.find((n) => n.name === 'shared')?.visibility).toBe('internal'); + }); +}); + +describe('Kotlin: a member of a private class is not visible either', () => { + it('declines a call onto a public fun of another file\'s private class', async () => { + // `clock`'s type is unknown here, so only the method name picks a target. + // Not a test file: production code never reaches a test suite's symbol anyway. + project({ + 'Coordinator.kt': 'class Coordinator {\n private class Clock(var value: Long) { fun now() = value }\n}\n', + 'Gimbal.kt': 'class Gimbal {\n fun dispatchSpeed() {\n val clock = provide()\n val timestamp = clock.now()\n }\n}\n', + }); + expect(await calleesOf('dispatchSpeed')).not.toContain('Coordinator.kt:now'); + }); +}); + describe('Go: an unexported identifier is package-local', () => { it('does not resolve a call onto an unexported func in another package', async () => { project({ diff --git a/__tests__/fixtures/kernel-parity/torture.kt b/__tests__/fixtures/kernel-parity/torture.kt index e23349a8bc..a39e1ae69c 100644 --- a/__tests__/fixtures/kernel-parity/torture.kt +++ b/__tests__/fixtures/kernel-parity/torture.kt @@ -26,6 +26,9 @@ fun com.example.Qualified.qext() {} suspend fun suspender(): Unit { helper() } private internal fun visFn() {} +private val hiddenTop = 1 +internal var sharedTop = 2 + fun inferred() = helper() diff --git a/codegraph-kernel/src/kotlin.rs b/codegraph-kernel/src/kotlin.rs index fb4d4cdc52..5459e63c98 100644 --- a/codegraph-kernel/src/kotlin.rs +++ b/codegraph-kernel/src/kotlin.rs @@ -806,9 +806,11 @@ impl<'t> Walker<'t> { return Some(Vec::new()); }; // The `type`-field signature read is dead (zero fields) → signature - // undefined; NO docstring/visibility/isStatic — the modifiers merge in - // create_node still decorates expect/actual properties. - let row = self.create_node(kind, &name, node, Extra::default()); + // undefined; NO docstring/isStatic — the modifiers merge in + // create_node still decorates expect/actual properties. Visibility IS + // read: a `private val` is file-local to the resolver (#1731). + let extra = Extra { visibility: Some(self.visibility_of(node)), ..Extra::default() }; + let row = self.create_node(kind, &name, node, extra); // Walk the initializer ATTRIBUTED to the declared symbol (#693, the Go // fix, ported): without this the subtree is only fn-ref-scanned, so a // lambda / SAM / object initializer (`val cb = Runnable { target() }` — diff --git a/src/extraction/languages/kotlin.ts b/src/extraction/languages/kotlin.ts index 3e8397c54a..7024bd2087 100644 --- a/src/extraction/languages/kotlin.ts +++ b/src/extraction/languages/kotlin.ts @@ -2,6 +2,21 @@ import type { Node as SyntaxNode } from 'web-tree-sitter'; import { getNodeText } from '../tree-sitter-helpers'; import type { LanguageExtractor } from '../tree-sitter-types'; +/** The visibility a Kotlin declaration's `modifiers` child spells out; public by default. */ +function kotlinVisibility(node: { childCount: number; child(i: number): { type: string; text: string } | null }): 'public' | 'private' | 'protected' | 'internal' { + for (let i = 0; i < node.childCount; i++) { + const child = node.child(i); + if (child?.type === 'modifiers') { + const text = child.text; + if (text.includes('public')) return 'public'; + if (text.includes('private')) return 'private'; + if (text.includes('protected')) return 'protected'; + if (text.includes('internal')) return 'internal'; + } + } + return 'public'; +} + /** Kotlin return types that can't be a chained-call receiver (no class to chain on). */ const KOTLIN_NON_CLASS_RETURN = new Set(['Unit', 'Nothing']); @@ -300,7 +315,10 @@ export const kotlinExtractor: LanguageExtractor = { const sig = typeNode ? `${isVal ? 'val' : 'var'} ${name}: ${getNodeText(typeNode, ctx.source)}` : undefined; - const created = ctx.createNode(kind, name, node, { signature: sig }); + // `private val` / `internal var` at class or object scope: the modifier + // decides who can name it from another file (#1731's Kotlin rule reads + // `visibility`, and a property without one read as public). + const created = ctx.createNode(kind, name, node, { signature: sig, visibility: kotlinVisibility(node) }); // Walk the initializer ATTRIBUTED to the declared symbol (#693, the Go // fix, ported to Kotlin): the hook consumes this subtree, so without an // explicit walk a lambda / SAM / object initializer @@ -504,20 +522,7 @@ export const kotlinExtractor: LanguageExtractor = { } return sig; }, - getVisibility: (node) => { - // Check for visibility modifiers in Kotlin - for (let i = 0; i < node.childCount; i++) { - const child = node.child(i); - if (child?.type === 'modifiers') { - const text = child.text; - if (text.includes('public')) return 'public'; - if (text.includes('private')) return 'private'; - if (text.includes('protected')) return 'protected'; - if (text.includes('internal')) return 'internal'; - } - } - return 'public'; // Kotlin defaults to public - }, + getVisibility: (node) => kotlinVisibility(node), isStatic: (_node) => { // Kotlin doesn't have static, uses companion objects return false; diff --git a/src/resolution/name-matcher.ts b/src/resolution/name-matcher.ts index 9debc1ec29..f0013a3e67 100644 --- a/src/resolution/name-matcher.ts +++ b/src/resolution/name-matcher.ts @@ -3384,6 +3384,38 @@ function isSfcPrivate(n: Node, context: ResolutionContext): boolean { return true; } +/** Per-context memo: node id → "this member sits inside a `private` class/object". */ +const PRIVATE_OWNER_MEMO = new WeakMap>(); + +/** Kinds that own members and carry a visibility of their own. */ +const OWNER_KINDS = new Set(['class', 'interface', 'struct', 'enum', 'namespace']); + +/** + * Whether `candidate` is a member of a `private` type in its file — a Kotlin + * test's `private class Clock { fun now() = value }`: the method is public, + * the class is not, so nothing outside the file can name `now`. Found by + * line range among the file's own nodes, memoised per node. + */ +function isInsidePrivateType(candidate: Node, context: ResolutionContext): boolean { + let memo = PRIVATE_OWNER_MEMO.get(context); + if (!memo) { + memo = new Map(); + PRIVATE_OWNER_MEMO.set(context, memo); + } + const hit = memo.get(candidate.id); + if (hit !== undefined) return hit; + const inside = context.getNodesInFile(candidate.filePath).some( + (n) => + n.id !== candidate.id && + OWNER_KINDS.has(n.kind) && + n.visibility === 'private' && + n.startLine <= candidate.startLine && + n.endLine >= candidate.endLine + ); + memo.set(candidate.id, inside); + return inside; +} + /** * Whether Go lets `ref`'s file name the Go declaration `candidate`: an * exported name from anywhere, an unexported one only inside its own package, @@ -3525,7 +3557,12 @@ export function isVisibleAcrossFiles(candidate: Node, ref: UnresolvedRef, contex if (lang === 'dart') { return candidate.visibility !== 'private' || (ref.language === 'dart' && dartPrivateVisible(ref.filePath, candidate.filePath, context)); } - if (PRIVATE_IS_FILE_LOCAL.has(lang)) return candidate.visibility !== 'private'; + if (PRIVATE_IS_FILE_LOCAL.has(lang)) { + // Only Kotlin's private type keeps its members in its file: a Scala + // `private[pkg] class` (or one with a private constructor) is the package's, + // and a C# private nested class serves every file of its partial class. + return candidate.visibility !== 'private' && !(lang === 'kotlin' && isInsidePrivateType(candidate, context)); + } // An R test file runs in an environment of its own (testthat): its top-level // `c <- ggplot(…)` is not what the package's 2,843 `c(…)` calls mean. The // `helper-*.R` / `setup-*.R` files are sourced for every test, so theirs are shared. @@ -19425,6 +19462,7 @@ export function clearNameMatcherMemos(context: ResolutionContext): void { JAVA_STATIC_IMPORTS.delete(context); PY_IMPORTS.delete(context); PY_MODULE_LOCAL.delete(context); + PRIVATE_OWNER_MEMO.delete(context); SEALED_MODULES.delete(context); LOCAL_BINDING_MEMO.delete(context); LOCAL_BINDING_SITES.delete(context);