From 10e316d9a7d99a5f9cb075256fd403c05b63eccc Mon Sep 17 00:00:00 2001 From: Colby Mchenry Date: Mon, 19 Oct 2026 01:32:46 +0000 Subject: [PATCH] fix(ts): a typed call links to the method of the class the file imports (#2641) When several classes share a name, a TypeScript or JavaScript call on a typed value broke the tie only by the call site's own file, then took the first declaration indexed, and a `this.` call by folder nearness. On vscode, about 25,000 calls on classes vscode imports from its own `vs/base` and `vs/platform` reached the copies the Copilot extension vendors, 907 `Emitter` calls a copilot test fixture's copy, and 952 calls whose first match was a test fixture's class linked to nothing. The file declaring the class the call's file imports under the written name, read through re-exports, `import type`, namespace imports, default imports and `require`, is now passed to resolveMethodOnType, which picks that file's method among several, through the supertype walk too. `this.` calls check the import of the call's own file before folder nearness. Java, Kotlin and every other language are unchanged. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 1 + __tests__/ts-imported-type-receivers.test.ts | 180 +++++++++++++++++++ src/resolution/name-matcher.ts | 79 ++++++-- 3 files changed, 247 insertions(+), 13 deletions(-) create mode 100644 __tests__/ts-imported-type-receivers.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 98020a127..2e79cd420 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -163,6 +163,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - In React with TypeScript, a component that renders a generic component and passes it type arguments, like outline's ` items={…} />` or excalidraw's ` …>`, now links to the component it renders. Before, a tag followed by type arguments was skipped, so the generic component looked unused by the screens that render it, and `codegraph_explore`'s call paths, callers and impact stopped short of it. Re-index React projects after upgrading. - In React, a component that a file declares itself as a value is now what that file's JSX renders, whatever else in the repository shares its name. A lazily loaded page, like `const RegisterPage = Loadable(lazy(() => import('./pages/Register')))`, a `dynamic(() => import(…))` component or one loaded only in development (`IS_DEV ? lazy(…) : null`), links to the component its module exports, including the one its loader picks and one an `export default observer(Login)` wraps; a wrapped component like `const Settings = observer(function Settings() {…})`, `React.memo(Rows, areEqual)` or `forwardRef(ComboboxInner) as {…}` links to the function it wraps; and an alias like `const DialogPortal = DialogPrimitive.Portal` links to its own declaration. Before, `` was linked to any lone component of that name elsewhere, often in another app of the same repository, so `codegraph_explore`, callers and impact followed it into the wrong app while the page it really renders had no caller. React Router layouts declared this way follow the same rule, and a TSX type named like a value the file declares now stays in that file. Re-index React projects after upgrading. - In React, a JSX tag imported from one of your own modules now links to the component that module exports under that name, read the way a lazily loaded page's module already is. That includes going through an `index` file or barrel that passes the component on, like outline's `Sidebar/index.ts` with `import Sidebar from './App'; export default Sidebar` or its `Avatar/index.ts` with `const Avatar = AvatarWithHoverCard`, and past a wrapper such as `export default observer(AppSidebar)`, `React.memo(StaticCanvas, areEqual)` or mastodon's `const MemoizedBlurhash = memo(Blurhash)`. Before, a tag was matched by its name and the import only broke ties, so outline's `` linked to an unrelated styled `Sidebar` in a sibling file, a default import under another name linked to whatever other component its module exported, and an import through a barrel could land on another app's component of the same name. A tag whose module exports an unnamed component, like `export default () => …`, now links to nothing instead of to a namesake elsewhere. A lazily loaded React Router page exported as such a value or wrapper links to the component it holds too, the one with the markup. `codegraph sync` and the file watcher redraw these links when only the file that passes the component on changes. Re-index React projects after upgrading. +- In TypeScript and JavaScript, a call on a variable, parameter or field whose class the file imports, like `store.add(…)` with `store: DisposableStore`, now links to the method of the class that import names when other classes in the project share its name, such as a vendored copy, a test mock or another app's class. Before, it linked to whichever of them was indexed first, or to the one in the nearest folder: in vscode, calls on classes imported from vscode's own `vs/base` and `vs/platform` reached the copies the Copilot extension vendors or its test fixtures, in bitwarden the desktop app's `initService.init()` reached the browser extension's `InitService`, and in react-native `Keyboard` and `WebSocket` reached the Jest mock of `NativeEventEmitter`. A call whose first match was a test file's class, which then linked to nothing, now reaches the imported class too. The import is followed through re-exports, `import type`, namespace imports like `new async.Delayer()` and `require(…)`. Re-index TypeScript and JavaScript projects after upgrading. ## [1.6.2] - 2026-10-03 diff --git a/__tests__/ts-imported-type-receivers.test.ts b/__tests__/ts-imported-type-receivers.test.ts new file mode 100644 index 000000000..eabe582d2 --- /dev/null +++ b/__tests__/ts-imported-type-receivers.test.ts @@ -0,0 +1,180 @@ +/** + * A call on a TypeScript/JavaScript value whose type the file imports links + * to the method of the class the import names, when several classes share + * the name. + * + * Typed calls broke a tie between same-named classes only by the call site's + * own file, then took the first declaration indexed: on vscode, calls in + * `src/vs/` on a `DisposableStore` imported from `vs/base/common/lifecycle` + * reached copilot's vendored copy under `extensions/copilot/src/util/vs/`, + * and on an `Emitter` a copilot test fixture's. `this.` calls broke + * it by folder nearness, which took react-native's Jest mock of + * `NativeEventEmitter` over the class beside it. Java and Kotlin already + * followed the import (#314). + */ +import { describe, it, expect, afterEach } from 'vitest'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import CodeGraph from '../src/index'; + +let roots: string[] = []; +let graphs: CodeGraph[] = []; + +afterEach(() => { + for (const graph of graphs) graph.close(); + graphs = []; + for (const root of roots) fs.rmSync(root, { recursive: true, force: true }); + roots = []; +}); + +/** Index `files` and return each call edge as `: -> ::`. */ +async function callsOf(files: Record): Promise { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-ts-imported-recv-')); + roots.push(root); + for (const [file, text] of Object.entries(files)) { + fs.mkdirSync(path.dirname(path.join(root, file)), { recursive: true }); + fs.writeFileSync(path.join(root, file), text); + } + const graph = await CodeGraph.init(root, { index: true }); + graphs.push(graph); + const nodes = graph.getFiles().flatMap((f) => graph.getNodesInFile(f.path)); + const byId = new Map(nodes.map((n) => [n.id, n])); + return graph.getOutgoingEdgesFrom(nodes.map((n) => n.id), ['calls']) + .map((e) => { + const target = byId.get(e.target); + return `${byId.get(e.source)?.filePath}: ${e.metadata?.refName} -> ${target?.filePath}::${target?.qualifiedName}`; + }) + .sort(); +} + +/** Two classes named `Store`, each with the same methods. */ +const STORES = { + 'package.json': JSON.stringify({ name: 'app' }), + 'src/one/store.ts': + 'export class Base {\n reset() { return 1; }\n}\n' + + 'export class Store extends Base {\n add(v: number) { return v; }\n}\n', + 'src/two/store.ts': + 'export class Base {\n reset() { return 2; }\n}\n' + + 'export class Store extends Base {\n add(v: number) { return -v; }\n}\n', + 'src/two/index.ts': "export { Store } from './store';\n", +}; + +describe('a call on a TypeScript value whose type the file imports', () => { + it("links to the imported class's method, whichever of the same-named classes it is", async () => { + const calls = await callsOf({ + ...STORES, + 'src/use-one.ts': + "import { Store } from './one/store';\n" + + 'export function a(s: Store) {\n return s.add(1);\n}\n', + 'src/use-two.ts': + "import { Store } from './two/store';\n" + + 'export function b(s: Store) {\n return s.add(2);\n}\n', + }); + expect(calls.filter((c) => c.includes('s.add'))).toEqual([ + 'src/use-one.ts: s.add -> src/one/store.ts::Store::add', + 'src/use-two.ts: s.add -> src/two/store.ts::Store::add', + ]); + }, 60_000); + + it('follows the import through a re-export, a type-only import and a namespace import', async () => { + const calls = await callsOf({ + ...STORES, + 'src/use-barrel.ts': + "import { Store } from './two';\n" + + 'export function b(s: Store) {\n return s.add(2);\n}\n', + 'src/use-type.ts': + "import type { Store } from './two/store';\n" + + 'export function t(s: Store) {\n return s.add(3);\n}\n', + 'src/use-ns.ts': + "import * as two from './two/store';\n" + + 'export function n() {\n const s = new two.Store();\n return s.add(4);\n}\n', + }); + expect(calls.filter((c) => c.includes('s.add'))).toEqual([ + 'src/use-barrel.ts: s.add -> src/two/store.ts::Store::add', + 'src/use-ns.ts: s.add -> src/two/store.ts::Store::add', + 'src/use-type.ts: s.add -> src/two/store.ts::Store::add', + ]); + }, 60_000); + + it("links an inherited method to the base class declared beside the imported class", async () => { + const calls = await callsOf({ + ...STORES, + 'src/use-two.ts': + "import { Store } from './two/store';\n" + + 'export function b(s: Store) {\n return s.reset();\n}\n', + }); + expect(calls.filter((c) => c.includes('s.reset'))).toEqual(['src/use-two.ts: s.reset -> src/two/store.ts::Base::reset']); + }, 60_000); + + it("links a `this.` call to the method of the class its file imports", async () => { + // Both stores are as near the caller's directory, so nearness alone + // picks the first path, `src/one`. + const calls = await callsOf({ + ...STORES, + 'src/user.ts': + "import { Store } from './two/store';\n" + + 'export class User {\n' + + ' constructor(private readonly store: Store) {}\n' + + ' run() {\n return this.store.add(3);\n }\n' + + '}\n', + }); + expect(calls.filter((c) => c.includes('this.store.add'))).toEqual(['src/user.ts: this.store.add -> src/two/store.ts::Store::add']); + }, 60_000); + + it("follows the call's own file's import when the field is read off a same-named class elsewhere", async () => { + // vscode's copilot `MockPromptsService` declares its emitter as + // `this._register(new Emitter())`, which isn't read, so its field + // is read off the workbench's `MockPromptsService`, which imports the + // other `Emitter`. + const calls = await callsOf({ + ...STORES, + 'src/app/mock.ts': + "import { Store } from '../two/store';\n" + + 'const wrap = (value: T) => value;\n' + + 'export class Mock {\n' + + ' private readonly store = wrap(new Store());\n' + + ' run() {\n return this.store.add(1);\n }\n' + + '}\n', + 'src/other/mock.ts': + "import { Store } from '../one/store';\n" + + 'export class Mock {\n' + + ' private readonly store = new Store();\n' + + ' run() {\n return this.store.add(2);\n }\n' + + '}\n', + }); + expect(calls.filter((c) => c.includes('this.store.add'))).toEqual([ + 'src/app/mock.ts: this.store.add -> src/two/store.ts::Store::add', + 'src/other/mock.ts: this.store.add -> src/one/store.ts::Store::add', + ]); + }, 60_000); + + it('does the same in JavaScript, for `new` of an imported or required class', async () => { + const calls = await callsOf({ + 'package.json': JSON.stringify({ name: 'app' }), + 'lib/one/store.js': 'export class Store {\n add(v) { return v; }\n}\n', + 'lib/two/store.js': 'export class Store {\n add(v) { return -v; }\n}\n', + 'lib/use-new.js': + "import { Store } from './two/store';\n" + + 'export function n() {\n const s = new Store();\n return s.add(5);\n}\n', + 'lib/use-require.js': + "const { Store } = require('./two/store');\n" + + 'function r() {\n const s = new Store();\n return s.add(6);\n}\n' + + 'module.exports = { r };\n', + }); + expect(calls.filter((c) => c.includes('s.add'))).toEqual([ + 'lib/use-new.js: s.add -> lib/two/store.js::Store::add', + 'lib/use-require.js: s.add -> lib/two/store.js::Store::add', + ]); + }, 60_000); + + it("still prefers a class the call's own file declares", async () => { + const calls = await callsOf({ + ...STORES, + 'src/own.ts': + 'class Store {\n add(v: number) { return v * 2; }\n}\n' + + 'export function o(s: Store) {\n return s.add(7);\n}\n', + }); + expect(calls.filter((c) => c.includes('s.add'))).toEqual(['src/own.ts: s.add -> src/own.ts::Store::add']); + }, 60_000); +}); diff --git a/src/resolution/name-matcher.ts b/src/resolution/name-matcher.ts index 32d9b4041..5bb2175e8 100644 --- a/src/resolution/name-matcher.ts +++ b/src/resolution/name-matcher.ts @@ -8959,6 +8959,8 @@ export function resolveMethodOnType( * signal Java imports carry but the call site doesn't (#314). * For Go it is the project-relative directory of the package that * declares `typeName`, when that is known (resolveGoMethodInPackage). + * For TS/JS it is the file declaring the class the caller's file + * imports under that name (tsImportedTypeFile). */ preferredFqn?: string, /** Recursion guard for the supertype/conformance walk. */ @@ -9024,7 +9026,11 @@ export function resolveMethodOnType( return null; } - if (matches.length > 1 && preferredFqn && ref.language !== 'go') { + if (matches.length > 1 && preferredFqn && ESM_FAMILY.has(ref.language)) { + const chosen = matches.find((m) => m.filePath === preferredFqn); + if (chosen) return { original: ref, targetNodeId: chosen.id, confidence, resolvedBy }; + } + if (matches.length > 1 && preferredFqn && ref.language !== 'go' && !ESM_FAMILY.has(ref.language)) { const ext = ref.language === 'kotlin' ? '.kt' : '.java'; const fqnPath = preferredFqn.replace(/\./g, '/') + ext; const chosen = matches.find((m) => { @@ -9047,9 +9053,9 @@ export function resolveMethodOnType( // the definition in the CALL SITE's own file. Without this, every ambiguous // call collapses onto the first-indexed definition, so a call in `b/svc.cpp` // wrongly points at `a/svc.cpp` (#1079). This runs AFTER the `preferredFqn` - // block, so Java/Kotlin import disambiguation — whose target is intentionally - // in ANOTHER file (#314) — is unaffected: that block returns early whenever - // an import FQN pins the class. + // blocks, so Java/Kotlin and TS/JS import disambiguation — whose target is + // intentionally in ANOTHER file (#314) — is unaffected: those blocks return + // early whenever an import pins the class. if (ref.referenceKind === 'function_ref' && matches.length !== 1) return null; const ordered = preferCallSiteFile(cppVisibleFirst(matches, ref, context), ref.filePath); return { @@ -12756,6 +12762,42 @@ function importedFqnOf( return imports.find((i) => i.localName === typeName)?.source; } +/** Per-context memo: `\0` → tsImportedTypeFile's answer. */ +const TS_IMPORTED_TYPE_FILES = new WeakMap>(); + +/** + * The file declaring the type a TS/JS file imports under the name a + * declaration in it writes (`store: DisposableStore`, `new async.Delayer()`), + * or undefined when the file doesn't import that name. Several classes may + * share it, a vendored copy, a test fixture or another app's, and the import + * is what says which one the file means: on vscode, calls on a + * `DisposableStore` imported from `vs/base/common/lifecycle` reached + * copilot's vendored copy of that file, the first declaration indexed, and + * a call whose first declaration was a test fixture's linked to nothing. + */ +function tsImportedTypeFile(written: string, ref: UnresolvedRef, context: ResolutionContext): string | undefined { + const name = /^[A-Za-z_$][\w$]*(?:\.[A-Za-z_$][\w$]*)*/.exec(written.trim())?.[0]; + if (!name) return undefined; + let memo = TS_IMPORTED_TYPE_FILES.get(context); + if (!memo) { + memo = new Map(); + TS_IMPORTED_TYPE_FILES.set(context, memo); + } + const key = ref.filePath + '\0' + name; + let file = memo.get(key); + if (file === undefined) { + file = null; + const binding = name.split('.')[0]!; + if (context.getImportMappings?.(ref.filePath, ref.language)?.some((i) => i.localName === binding)) { + const imported = context.resolveImport?.({ ...ref, referenceName: name, referenceKind: 'references' }); + const node = imported ? context.getNodeById?.(imported.targetNodeId) : undefined; + if (node && node.kind !== 'file' && node.filePath !== ref.filePath) file = node.filePath; + } + memo.set(key, file); + } + return file ?? undefined; +} + /** * Java/Kotlin: infer a receiver's declared type by walking field declarations * in the class enclosing the call site. The field's `signature` is already in @@ -12924,6 +12966,7 @@ export function clearNameMatcherMemos(context: ResolutionContext): void { PYTHON_IMPORTED_FILES.delete(context); AWAITED_TYPE_MEMO.delete(context); AWAITED_FILES.delete(context); + TS_IMPORTED_TYPE_FILES.delete(context); C_STATIC_MEMO.delete(context); CPP_TYPE_DECLS.delete(context); CPP_INCLUDE_LINES.delete(context); @@ -14231,7 +14274,8 @@ export function matchMethodCall( return decl.asserted && resolveMethodOnType(decl.asserted.name, methodName!, ref, context, 0.9, 'instance-method', decl.asserted.dir); } // Java/Kotlin: when two classes share the simple name, the file's import - // pins WHICH one (#314); Go: the package that declares the type (#2323). + // pins WHICH one (#314); Go: the package that declares the type (#2323); + // TS/JS: the file declaring the class the file imports under that name. // Other languages disambiguate by call-site file. const importedFqn = ref.language === 'java' || ref.language === 'kotlin' @@ -14240,7 +14284,9 @@ export function matchMethodCall( .find((i) => i.localName === inferredType)?.source : ref.language === 'go' ? goDeclaredTypePackage(decl.raw, ref.filePath, context) - : undefined; + : ESM_FAMILY.has(ref.language) && !awaited + ? nmTimedT('mc-tsimport', ref, () => tsImportedTypeFile(decl.raw ?? inferredType!, ref, context)) + : undefined; const typedMatch = nmTimedT('mc-rmot', ref, () => resolveMethodOnType( inferredType, methodName!, @@ -15730,12 +15776,17 @@ function matchTsThisFieldCall( // `ns.Mailer` → `Mailer`; a primitive or builtin names no project type. const typeName = decl.type.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. + // Two apps in one repo may each declare a `UserService`. The call's file + // imports the one its field holds, so that one's method is the call's. + // Only the call's file: a field read off a same-named class elsewhere + // follows that file's imports, not this one's (vscode's two + // `MethodContextProvider`s import two `Symbols`). Without an import, the + // bare-name path this replaces broke the 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 imported = tsImportedTypeFile(decl.type, ref, context); const declared = context .getNodesByName(methodName) .filter( @@ -15745,6 +15796,8 @@ function matchTsThisFieldCall( (n.qualifiedName === `${typeName}::${methodName}` || n.qualifiedName.endsWith(`::${typeName}::${methodName}`)) ); if (declared.length > 1) { + const own = imported !== undefined ? declared.find((n) => n.filePath === imported) : undefined; + if (own) return { original: ref, targetNodeId: own.id, confidence: 0.85, resolvedBy: 'instance-method' }; const callDirs = ref.filePath.split('/').slice(0, -1); const shared = (fp: string) => { const dirs = fp.split('/').slice(0, -1); @@ -15755,7 +15808,7 @@ function matchTsThisFieldCall( 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 resolveMethodOnType(typeName, methodName, ref, context, 0.85, 'instance-method', imported); } return null; }