mirror of
https://github.com/colbymchenry/codegraph.git
synced 2026-10-21 16:57:09 +08:00
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.<field>` 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.<field>` 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 <noreply@anthropic.com>
This commit is contained in:
@@ -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 `<PaginatedList<Document> items={…} />` or excalidraw's `<DropdownMenuItemContentRadio<"contain" | "overlap"> …>`, 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, `<RegisterPage />` 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 `<Sidebar>` 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
|
||||
|
||||
|
||||
@@ -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.<field>` 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 `<file>: <refName> -> <target file>::<target qualified name>`. */
|
||||
async function callsOf(files: Record<string, string>): Promise<string[]> {
|
||||
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.<field>` 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<void>())`, 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 = <T>(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);
|
||||
});
|
||||
@@ -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: `<file>\0<type name as written>` → tsImportedTypeFile's answer. */
|
||||
const TS_IMPORTED_TYPE_FILES = new WeakMap<ResolutionContext, Map<string, string | null>>();
|
||||
|
||||
/**
|
||||
* 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;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user