mirror of
https://github.com/colbymchenry/codegraph.git
synced 2026-11-14 16:52:09 +08:00
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 main01e0ac3a(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 main4eb8c2a5, http4s, scalachess and serilog were identical too. - Cost (mihon, median of 3 interleaved indexes, on main4eb8c2a5): 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 <noreply@anthropic.com>
This commit is contained in:
committed by
Colby Mchenry
co-authored by
Claude Opus 5.5
parent
01e0ac3aa7
commit
13f983db5a
@@ -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.
|
||||
|
||||
@@ -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, Any>): 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({
|
||||
|
||||
@@ -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()
|
||||
|
||||
|
||||
@@ -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() }` —
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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<ResolutionContext, Map<string, boolean>>();
|
||||
|
||||
/** Kinds that own members and carry a visibility of their own. */
|
||||
const OWNER_KINDS = new Set<string>(['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);
|
||||
|
||||
Reference in New Issue
Block a user