From bc47acc77e1d2a57ae6fa00174bf61c9569405b4 Mon Sep 17 00:00:00 2001 From: Colby McHenry Date: Tue, 8 Sep 2026 19:20:25 +0000 Subject: [PATCH] fix(resolution): resolve this..() on the fields declared type (#1496) Land the six-file fix from upstream PR #1691 by danusha2345 (pr-1691 at 6d0e80d52ae953b22d615bd20c4a4c7758814e60), preserving wasm/native extraction parity and exclusive field-type resolution. Preserve coexistence with the #1566 Map/collection fix merged in #1790, including nested holder.values.get coverage and the unchanged #1566 Unreleased changelog bullet. EXTRACTION_VERSION remains unchanged. Align the existing chained-receiver regression with the fix: a declared service field calls its method, while an anonymous field type does not bind to unrelated same-named project functions. Verified on Linux with Node 22.19.0: - Rebuilt the native kernel and TypeScript/browser distribution. - Both backends change Outbox::send -> Outbox::send into Outbox::send -> Mailer::send, keep Relay::forward -> Mailer::send, and store no self-edges in the issue repro. - Wasm: 224 tests passed; native kernel: 255 tests passed, no skips. - All 10 #1566 resolution cases pass on each backend, plus all four nested-receiver extraction parity cases. --- CHANGELOG.md | 1 + __tests__/fixtures/kernel-parity/torture.tsx | 8 ++ __tests__/ts-chained-receiver.test.ts | 12 +- __tests__/ts-this-field-call.test.ts | 106 ++++++++++++++++ codegraph-kernel/src/tsjs/extractors.rs | 18 +++ src/extraction/tree-sitter.ts | 23 ++++ src/resolution/name-matcher.ts | 120 +++++++++++++++++++ 7 files changed, 287 insertions(+), 1 deletion(-) create mode 100644 __tests__/ts-this-field-call.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 1fd25a1b4..851f8e914 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -219,6 +219,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). #### Symbols, tests and the viewer +- TypeScript/JavaScript: a call through a field of the enclosing class — `this.mailer.send()` — now resolves on the field's declared type, so a delegating wrapper that shares the method's name no longer records itself as its own callee and `callers`, `impact` and trace stop lying on that shape. A field whose type is external or a builtin stays unresolved rather than guessed. Re-index after upgrading. (#1496) - TypeScript and JavaScript collection calls through local variables and their nested properties no longer link to unrelated project methods; re-index after upgrading. (#1566) - Objective-C headers now index in a project that has no `.m` file. A `.h` file is read as C from its name alone, and only later — once its contents are read — recognized as Objective-C; the grammar for that was never loaded up front, so the file failed with a parser error and nothing in it reached the index. Adding any `.m` file used to make the same header work, which is what made this look arbitrary. Thanks @Juddd. (#1628) diff --git a/__tests__/fixtures/kernel-parity/torture.tsx b/__tests__/fixtures/kernel-parity/torture.tsx index 007792e96..b7f4be749 100644 --- a/__tests__/fixtures/kernel-parity/torture.tsx +++ b/__tests__/fixtures/kernel-parity/torture.tsx @@ -213,6 +213,14 @@ new NS.Widget(makeArg()); new Map(); super_weird?.(); +// --- call through a field of the enclosing class (#1496) --------------------- +export class FieldDelegator { + constructor(private readonly mailer: { send(m: string): string }, private items: string[]) {} + send(msg: string): string { return this.mailer.send(msg); } + push(msg: string): void { this.items.push(msg); this.mailer.send(msg).trim(); } + direct(): void { this.send('x'); super.toString(); } +} + // --- const-bound functions inside a body (#1669) ----------------------------- export function NestedHandlers({ items, onPick }: { items: string[]; onPick: (a: unknown, b: unknown) => void }) { const handleClear = () => { onPick(null, null); }; diff --git a/__tests__/ts-chained-receiver.test.ts b/__tests__/ts-chained-receiver.test.ts index cdfa95be5..267aadb8c 100644 --- a/__tests__/ts-chained-receiver.test.ts +++ b/__tests__/ts-chained-receiver.test.ts @@ -43,7 +43,12 @@ beforeAll(async () => { 'export function viaGlobal(): string {\n' + ' return window.MyNs.ping();\n' + '}\n' + + 'export class PingService { ping(): string { return "service"; } }\n' + 'export class Runner {\n' + + ' constructor(private svc: PingService) {}\n' + + ' run(): string { return this.svc.ping(); }\n' + + '}\n' + + 'export class AnonymousRunner {\n' + ' constructor(private svc: { ping(): string }) {}\n' + ' run(): string { return this.svc.ping(); }\n' + '}\n' @@ -86,6 +91,11 @@ describe('TS/JS call through a host-global chain (#1707)', () => { it('keeps a chain rooted at a project value — window.MyNs.m() and this..m()', () => { const ping = fn('ping', 'service.ts').id; expect(callTargets(fn('viaGlobal', 'service.ts').id)).toContain(ping); - expect(callTargets(method('Runner::run').id)).toContain(ping); + expect(callTargets(method('Runner::run').id)).toEqual([method('PingService::ping').id]); + }); + + it('does not guess a same-named project target for an anonymous field type (#1496)', () => { + // Neither the top-level ping nor PingService::ping establishes what svc is. + expect(callTargets(method('AnonymousRunner::run').id)).toEqual([]); }); }); diff --git a/__tests__/ts-this-field-call.test.ts b/__tests__/ts-this-field-call.test.ts new file mode 100644 index 000000000..ce2f675d7 --- /dev/null +++ b/__tests__/ts-this-field-call.test.ts @@ -0,0 +1,106 @@ +/** + * A TS/JS call through a field of the enclosing class resolves on the field's + * declared type, never by bare name (#1496). + * + * `this.mailer.send(msg)` inside `Notifier.send()` used to be emitted as the + * bare `send`, which exact-matched the nearest same-named method — the + * calling method itself. The stored self-edge `Notifier::send → Notifier::send` + * made callers, callees, impact and trace silently wrong on exactly the + * shape a delegating wrapper takes. The identical call resolved correctly + * whenever the wrapper had any other name. + */ +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { CodeGraph } from '../src'; +import { initGrammars, loadAllGrammars } from '../src/extraction/grammars'; + +let dir: string; +let cg: CodeGraph; + +beforeAll(async () => { + await initGrammars(); + await loadAllGrammars(); + dir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-1496-')); + fs.mkdirSync(path.join(dir, 'src')); + const w = (rel: string, body: string) => fs.writeFileSync(path.join(dir, 'src', rel), body); + w('mailer.ts', 'export class Mailer {\n send(msg: string): string { return msg; }\n}\n'); + w( + 'notifier.ts', + "import { Mailer } from './mailer';\n" + + 'export class Notifier {\n' + + ' constructor(private readonly mailer: Mailer, private items: string[]) {}\n' + + ' send(msg: string): string { return this.mailer.send(msg); }\n' + + ' other(msg: string): string { return this.mailer.send(msg); }\n' + + ' push(msg: string): void { this.items.push(msg); }\n' + + '}\n' + ); + // Plain JS: the field's type is only known from its `new` initializer. + // (resolveMethodOnType matches within one language, so the JS wrapper gets a JS Mailer.) + w('legacy-mailer.js', 'class LegacyMailer {\n send(msg) { return msg; }\n}\nmodule.exports = { LegacyMailer };\n'); + w( + 'legacy.js', + "const { LegacyMailer } = require('./legacy-mailer');\n" + + 'class LegacyNotifier {\n' + + ' constructor() { this.mailer = new LegacyMailer(); }\n' + + ' send(msg) { return this.mailer.send(msg); }\n' + + '}\n' + + 'module.exports = { LegacyNotifier };\n' + ); + // A field typed as the type OF a value: an object literal used as a namespace. + w( + 'storage.ts', + 'export const DraftHubStorage = {\n' + + ' async get(key: string): Promise { return key; },\n' + + ' async getSettings(): Promise { return {}; },\n' + + '};\n' + ); + w( + 'keeper.ts', + "import { DraftHubStorage } from './storage';\n" + + 'export class Keeper {\n' + + ' constructor(private readonly storage: typeof DraftHubStorage) {}\n' + + ' async get(key: string): Promise { return this.storage.get(key); }\n' + + ' async settings(): Promise { return this.storage.getSettings(); }\n' + + '}\n' + ); + cg = CodeGraph.initSync(dir); + await cg.indexAll(); +}); + +afterAll(() => { + cg.destroy(); + fs.rmSync(dir, { recursive: true, force: true }); +}); + +const method = (qn: string) => cg.getNodesByKind('method').find((n) => n.qualifiedName === qn)!; +const calleesOf = (qn: string) => cg.getCallees(method(qn).id).map(({ node }) => node.qualifiedName).sort(); + +describe('this..() (#1496)', () => { + it('resolves on the field\'s declared type even when the wrapper shares the method name', () => { + expect(calleesOf('Notifier::send')).toEqual(['Mailer::send']); + expect(calleesOf('Notifier::other')).toEqual(['Mailer::send']); + // No self-edge anywhere. + const self = cg.getCallers(method('Notifier::send').id).some(({ node }) => node.id === method('Notifier::send').id); + expect(self).toBe(false); + }); + + it('reads a JS field initialized in the constructor', () => { + expect(calleesOf('LegacyNotifier::send')).toEqual(['LegacyMailer::send']); + }); + + it('leaves a builtin-typed field unresolved rather than guessing a same-named method', () => { + // `this.items.push()` — `string[]` names no project type; the wrapper `push` + // must not become its own callee. + expect(calleesOf('Notifier::push')).toEqual([]); + }); + + it('resolves a field typed `typeof ` onto the literal\'s member', () => { + // The members are bare-named functions inside the constant's extent (#1573). + expect(calleesOf('Keeper::settings')).toEqual(['getSettings']); + expect(calleesOf('Keeper::get')).toEqual(['get']); + const self = cg.getCallers(method('Keeper::get').id).some(({ node }) => node.id === method('Keeper::get').id); + expect(self).toBe(false); + }); +}); diff --git a/codegraph-kernel/src/tsjs/extractors.rs b/codegraph-kernel/src/tsjs/extractors.rs index c46495e51..34fd4a076 100644 --- a/codegraph-kernel/src/tsjs/extractors.rs +++ b/codegraph-kernel/src/tsjs/extractors.rs @@ -1185,6 +1185,11 @@ impl<'t> Walker<'t> { } else { callee_name = method_name.to_string(); } + } else if let Some(field) = receiver.and_then(|r| self.this_field_of(r)) { + // `this..()` — keep the field so the + // resolver can read its declared type (#1496). Mirrors + // TreeSitterExtractor.extractCall. + callee_name = format!("this.{field}.{method_name}"); } else if let Some(r) = receiver.filter(|r| r.kind() == "call_expression") { // Call receiver — `make().run()` (#1683): keep the inner // callee as `().`, or emit nothing when it @@ -1215,6 +1220,19 @@ impl<'t> Walker<'t> { // --- extractInstantiation ----------------------------------------------------------- + /// `this.` as a member_expression receiver → Some(field) (#1496). + fn this_field_of(&self, receiver: Node<'t>) -> Option { + if receiver.kind() != "member_expression" { + return None; + } + let object = receiver.child_by_field_name("object")?; + let property = receiver.child_by_field_name("property")?; + if object.kind() != "this" || property.kind() != "property_identifier" { + return None; + } + Some(self.text(property).to_string()) + } + /// The callee of a call-expression receiver when it is a plain identifier /// or member chain (`make`, `d.setdefault`), whitespace stripped (#1683). fn plain_inner_callee(&self, call: Node<'t>) -> Option { diff --git a/src/extraction/tree-sitter.ts b/src/extraction/tree-sitter.ts index 030607b92..cedf77145 100644 --- a/src/extraction/tree-sitter.ts +++ b/src/extraction/tree-sitter.ts @@ -4728,6 +4728,29 @@ export class TreeSitterExtractor { // scope keywords: such calls previously emitted a bare method // name, which either failed to resolve or resolved ambiguously. calleeName = `${getNodeText(receiver, this.source)}.${methodName}`; + } else if ( + (this.language === 'typescript' || + this.language === 'javascript' || + this.language === 'tsx' || + this.language === 'jsx') && + receiver && + receiver.type === 'member_expression' && + getChildByField(receiver, 'object')?.type === 'this' && + getChildByField(receiver, 'property')?.type === 'property_identifier' + ) { + // TS/JS call through a field of the enclosing class — + // `this.mailer.send()` (#1496). Keep the `this.` prefix: + // the resolver reads the field's declared type off the class's + // own declaration (`private mailer: Mailer`, `mailer = new + // Mailer()`) and resolves the method on THAT type — or leaves the + // ref unresolved when the type is external or unknown. Previously + // this collapsed to the bare method name, which exact-matched + // whichever same-named method was nearest — the calling method + // itself when the two share a name, a self-edge not in the + // source. Same discipline as Rust's `self.` (#1585). + // Mirrored in the kernel's extract_call (tsjs/extractors.rs). + const fieldName = getNodeText(getChildByField(receiver, 'property')!, this.source); + calleeName = `this.${fieldName}.${methodName}`; } else if ( (this.language === 'typescript' || this.language === 'javascript' || diff --git a/src/resolution/name-matcher.ts b/src/resolution/name-matcher.ts index 9f9b3b3e3..44c8fe3d9 100644 --- a/src/resolution/name-matcher.ts +++ b/src/resolution/name-matcher.ts @@ -2232,6 +2232,21 @@ export function matchMethodCall( return matchRustSelfFieldCall(objectOrClass!.slice('self.'.length), methodName!, ref, context); } + // TS/JS call through a field of the enclosing class — `this.mailer.send()`, + // emitted as `this.mailer.send` (#1496). Same discipline as the Rust branch + // above, and EXCLUSIVE for the same reason: the field's declared type off + // the class's own declaration, validated by resolveMethodOnType, or nothing. + // Letting the bare name through is how `this.mailer.send()` inside + // `Notifier.send()` resolved to the calling method itself — a self-edge the + // source does not contain — whenever the two shared a name. + if ( + (ref.language === 'typescript' || ref.language === 'javascript' || ref.language === 'tsx' || ref.language === 'jsx') && + dotMatch && + objectOrClass!.startsWith('this.') + ) { + return matchTsThisFieldCall(objectOrClass!.slice('this.'.length), methodName!, ref, context); + } + // Java/Kotlin: receiver may be a field whose name doesn't match the type by // Java naming convention (`userbo` → class `UserBO`, abbreviated). Look up // the field in the enclosing class to get its declared type, then resolve @@ -2618,6 +2633,111 @@ function matchRustSelfFieldCall( return null; } +/** + * Resolve a TS/JS `this..()` call (#1496) through the field's + * declared type, read off the ENCLOSING class's own declaration lines: + * a field or constructor-parameter property (`private mailer: Mailer`, + * `mailer?: Mailer`, `readonly mailer: Mailer`) or an initializer + * (`mailer = new Mailer()`, `this.mailer = new Mailer()`). The method is then + * VALIDATED on that type by resolveMethodOnType. Null — never a bare-name + * fallback — when the field is not declared there or its type is external, + * a builtin (`this.items.push()`) or not spelled out. + */ +function matchTsThisFieldCall( + field: string, + methodName: string, + ref: UnresolvedRef, + context: ResolutionContext, +): ResolvedRef | null { + if (!field || field.includes('.')) return null; + const caller = context.getNodeById?.(ref.fromNodeId); + if (!caller) return null; + const sep = caller.qualifiedName.lastIndexOf('::'); + if (sep <= 0) return null; // not inside a class + const owner = caller.qualifiedName.slice(0, sep).split('::').pop(); + if (!owner) return null; + + const owners = preferCallSiteFile(context.getNodesByName(owner), ref.filePath).filter( + (n) => (n.kind === 'class' || n.kind === 'component') && sameLanguageFamily(n.language, ref.language) + ); + const fieldEsc = field.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); + const patterns: Array<{ re: RegExp; valueType: boolean }> = [ + // `storage: typeof DraftHubStorage` — the type OF a value: an object + // literal used as a namespace. Its members are bare-named functions inside + // the constant's extent (#1573), so they are found by containment, not by + // `Type::method`. Tried first: the declared-type pattern below would + // otherwise capture the word `typeof`. + { + re: new RegExp(`\\b${fieldEsc}\\b\\s*[?!]?\\s*:\\s*(?:readonly\\s+)?typeof\\s+([A-Za-z_$][\\w.$]*)`), + valueType: true, + }, + // `private readonly mailer?: Mailer` — a class field or a constructor + // parameter property; the capture stops at `<`, `[` or `|`, so a generic + // or union type yields its head and resolveMethodOnType decides. + { + re: new RegExp(`\\b${fieldEsc}\\b\\s*[?!]?\\s*:\\s*(?:readonly\\s+)?([A-Za-z_$][\\w.$]*)`), + valueType: false, + }, + // `mailer = new Mailer()` / `this.mailer = new Mailer()` + { re: new RegExp(`\\b${fieldEsc}\\b\\s*=\\s*new\\s+([A-Za-z_$][\\w.$]*)`), valueType: false }, + ]; + for (const cls of owners) { + const source = context.readFile(cls.filePath); + if (!source) continue; + const declLines = source.split('\n').slice(Math.max(0, cls.startLine - 1), cls.endLine); + for (const rawLine of declLines) { + const line = rawLine.replace(/\/\/.*$/, '').replace(/\/\*.*?\*\//g, ''); + for (const { re, valueType } of patterns) { + const m = line.match(re); + if (!m || !m[1]) continue; + if (valueType) { + // The value's declaration may live in another file (it is imported); + // the call site's file is preferred when several share the name. + const holderName = m[1].split('.').pop()!; + const holders = preferCallSiteFile(context.getNodesByName(holderName), ref.filePath).filter( + (n) => (n.kind === 'constant' || n.kind === 'variable') && sameLanguageFamily(n.language, ref.language) + ); + for (const holder of holders) { + const hit = resolveObjectLiteralMember(holder, methodName, ref, context, 0.85, 'instance-method'); + if (hit) return hit; + } + return null; + } + // `ns.Mailer` → `Mailer`; a primitive or builtin names no project type. + const typeName = m[1].split('.').pop()!; + if (!/^[A-Z]/.test(typeName)) return null; + // Two apps in one repo may each declare a `UserService`. The bare-name + // path this replaces broke that tie by directory proximity, so keep the + // same signal: among the type's declarations of the method, prefer the + // one closest to the call site's directory (its own app), never index + // order. resolveMethodOnType still answers the single-declaration and + // supertype cases. + const declared = context + .getNodesByName(methodName) + .filter( + (n) => + n.kind === 'method' && + sameLanguageFamily(n.language, ref.language) && + (n.qualifiedName === `${typeName}::${methodName}` || n.qualifiedName.endsWith(`::${typeName}::${methodName}`)) + ); + if (declared.length > 1) { + const callDirs = ref.filePath.split('/').slice(0, -1); + const shared = (fp: string) => { + const dirs = fp.split('/').slice(0, -1); + let i = 0; + while (i < dirs.length && i < callDirs.length && dirs[i] === callDirs[i]) i++; + return i; + }; + const nearest = [...declared].sort((a, b) => shared(b.filePath) - shared(a.filePath) || a.filePath.localeCompare(b.filePath))[0]!; + return { original: ref, targetNodeId: nearest.id, confidence: 0.85, resolvedBy: 'instance-method' }; + } + return resolveMethodOnType(typeName, methodName, ref, context, 0.85, 'instance-method'); + } + } + } + return null; +} + /** * The one fallback a TS/JS/Python call-receiver chain keeps (#1683): a STORE * ACCESSOR. Zustand's `get()` inside the store factory and