From 59eb0d282048bd15e5f5cdec8c3b8a9bbe98d3d9 Mon Sep 17 00:00:00 2001 From: Eric Minish Date: Fri, 30 Oct 2026 05:59:02 -0500 Subject: [PATCH] fix(resolution): an import no longer lands on a same-named method of an exported object An exported object literal's methods (`export const d = { f() {} }`) are indexed as exported nodes named `f` (qualified name `d::f`), and the per-file export index kept the first exported node per name. So `import { f } from './m'` could link to `d.f` instead of the module's own `export function f`, and which one won depended only on declaration order. A JS/TS member, a node whose qualified name differs from its name, now ranks behind the module's own bindings wherever an import is matched to an export: a named import (the module's own `f`, then its export clause, then its re-exports, then a member), an export clause's local name, `export default f` and `module.exports = f`. A member is never dropped: `export const { f } = d` binds `f` with no node of its own, and `d::f` is its only answer. The default-import guess no longer picks a member. Validation on main de0a4eb3 (Windows 11): - Red/green: 7 of the 13 new tests fail with main's import-resolver.ts; all 13 pass with the fix, with the kernel and with CODEGRAPH_KERNEL=0. - A/B against main: express, vuejs/core and excalidraw are byte-identical, as are the flask and gin controls. codegraph's own source retargets 195 sites, each from a `hashNavigation` method to the exported function of the same name in ui/src/lib/navigation.ts, the issue's case. vscode's base and editor retarget 328 sites: 321 imports and calls of `join`, `basename`, `dirname` and 5 other path functions move from the `win32` object's methods to path.ts's own exports (`export const join = (platformIsWin32 ? win32.join : posix.join)`), and 7 `marked.lexer` and `marked.parse` calls move from a `_Tokenizer` field and a `_Parser` static method to the `lexer` and `parse` that marked.js exports. No edge is added or removed. - Index time unchanged: median of 3 interleaved runs on vscode's base and editor, 16.6 s on main and 17.2 s with the fix. Original PR #2990 by @inth3shadows. Fixes #2945 Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 1 + .../import-object-member-namesake.test.ts | 249 ++++++++++++++++++ src/resolution/import-resolver.ts | 55 +++- 3 files changed, 296 insertions(+), 9 deletions(-) create mode 100644 __tests__/import-object-member-namesake.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 4424d474f..489b96871 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -321,6 +321,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - A React Router route written as `element: ` or `Component: Outlet`, or whose `lazy` loader hands back `{ Component: Outlet }`, now counts as rendering nothing of its own, as a route with no element does and as `}>` already did. Before, `Outlet` was taken for the route's page and for the layout of the routes inside it, which linked nothing, and a loader returning `{ Component: Outlet }` made the module it loads for its `loader` the layout of the routes inside: in Medusa's admin dashboard, a product's detail page was drawn around its variants' pages. Such a route is still a route at its own address, inside the layouts around it, and a page inside it at the same address still takes the address over. Re-index React projects after upgrading. - React Router routes are now read from every table a route-building function returns, an early `if (!user) return [];` and each branch alike, and never from a list that a function written inside it returns, like a `.forEach` callback's `return []` or a helper's menu. A function that returns another's table, as in `return buildRoutes()`, is followed too, and so is a table behind `useMemo`, like `useMemo(() => getRoutes(user), [user])` or `useMemo(getRoutes, [])`. Before, the first `return [` anywhere in the function was taken for the table, so an empty guard or a callback's list could hide an app's routes entirely, or a menu could be read in their place. Re-index React projects after upgrading. - In JavaScript and TypeScript, an import written `import { default as AppRoot } from './routes/app/root'`, as in bulletproof-react's router, now counts as the module's default import, the same as `import AppRoot from './routes/app/root'`. Before, it was read as an import of an export named `default`, which no module has, so a call, a route or a JSX attribute that used `AppRoot` was matched by its name alone: it could link to another file's `AppRoot`, or to nothing when the default export has a name of its own. Svelte, Vue and Astro script blocks are read the same way. +- In JavaScript and TypeScript, an import now links to what the module itself exports under that name, a function, an `export { … }` clause, an `export default`, `module.exports` or a re-export, rather than to a method of the same name in an exported object like `export const hashDriver = { symbolHref(id) {…} }`. Before, whichever came first in the file won, so a module's own `symbolHref` could look unused while the object's method collected its callers, and vscode's `import { join } from 'vs/base/common/path'` reached the Windows `join` instead of the module's own export. Re-index JavaScript and TypeScript projects after upgrading. Thanks @inth3shadows for the report and the fix. (#2945) - In React, a component's JSX no longer links it to a class or component that only shares a name with one of its type arguments or with a variable of its own. A type in angle brackets, like `Document` in ` items={…} />`, `User` in `useState()` or `Entry` in a generic `(…) =>` component, is no longer read as a tag. A tag naming a variable or parameter the component sets itself, like `` after `const Content = isDropdown ? DropdownMenu.SubContent : ContextMenu.SubContent` or `` in `widgets.map((Widget) => )`, now links to nothing, unless the component declares a component of that name inside itself. Before, these linked to an unrelated class or component elsewhere in the repository: outline's document lists showed up among the callers of its `Document` model class, and its menus among the callers of the command bar's `Content`, so `codegraph_explore`, callers and impact followed renders that never happen. Re-index React projects after upgrading. - In React, Svelte, Astro and Vue, a component name used without an import, with several definitions in the project, now prefers one in the using file's own folder or a folder below it, compared folder by folder. Before, a folder whose name only starts the same way counted too, so `Widget` in `src/ui/Card.tsx` could link to `src/ui-kit/Widget.tsx` instead of `src/ui/Widget.tsx`, and a file at the project root, like `App.tsx`, linked to whichever definition came first instead of to none. Re-index affected projects after upgrading. Thanks @Mubashirrrr. - In React, a tag for a component a function destructures from its props or from an object, like `({ Header, Footer, children }) =>` in refine's layouts or `const { Sidebar, Footer } = excalidrawLib` in excalidraw's example app, now links to nothing, as a tag naming a variable the function sets itself already did, unless the function declares a component of that name inside itself. A function nested in the one that sets or destructures the name, like a `renderSidebar` helper inside the component, reads the tag the same way. A name destructured from `require(…)` or `import(…)` still links to that module's export. Before, these tags were matched by their name alone, so refine's layouts showed up among the callers of other examples' `Header` and `Footer` components, and `codegraph_explore`, callers and impact followed renders that never happen. Re-index React projects after upgrading. diff --git a/__tests__/import-object-member-namesake.test.ts b/__tests__/import-object-member-namesake.test.ts new file mode 100644 index 000000000..a759f9dfd --- /dev/null +++ b/__tests__/import-object-member-namesake.test.ts @@ -0,0 +1,249 @@ +/** + * An import names one of the module's own bindings, never a method of an + * object in it. Extraction marks an exported object literal's methods + * exported too, and the per-file export index kept whichever came first, so + * `import { f }` could land on `d.f` instead of the module's `export function + * f`, or instead of what the module's export clause, re-export or default + * export names (#2945). + */ +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'; + +const roots: string[] = []; +const graphs: CodeGraph[] = []; +afterEach(() => { + for (const cg of graphs.splice(0)) cg.close(); + for (const r of roots.splice(0)) fs.rmSync(r, { recursive: true, force: true }); +}); + +async function indexProject(files: Record): Promise { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-import-member-namesake-')); + roots.push(root); + for (const [rel, content] of Object.entries(files)) { + fs.mkdirSync(path.dirname(path.join(root, rel)), { recursive: true }); + fs.writeFileSync(path.join(root, rel), content); + } + const cg = await CodeGraph.init(root, { index: true }); + graphs.push(cg); + return cg; +} + +/** `::` for the calls leaving `file`'s function `fn`. */ +function callsFrom(cg: CodeGraph, file: string, fn: string): string[] { + const src = cg.getNodesInFile(file).find((n) => n.name === fn)!; + return cg + .getOutgoingEdgesFrom([src.id]) + .filter((e) => e.kind === 'calls') + .map((e) => { + const t = cg.getNode(e.target); + return `${t?.filePath}:${t?.qualifiedName}:${t?.startLine}`; + }) + .sort(); +} + +const WALK = `import { f } from './m'; +export function walk() { return f(); } +`; + +describe('a named import whose module has an object with a method of that name', () => { + it('links the exported function when an exported object with that method comes first', async () => { + const cg = await indexProject({ + 'm.ts': `export interface Driver { f(id: string): string; } +export const d: Driver = { + f(id) { return '#' + id; }, +}; +let driver: Driver = d; +export function f(id: string): string { return driver.f(id); } +`, + 'walk.ts': WALK, + }); + expect(callsFrom(cg, 'walk.ts', 'walk')).toEqual(['m.ts:f:6']); + }); + + it('links the exported function when the object comes after it', async () => { + const cg = await indexProject({ + 'm.ts': `export function f(): string { return '#'; } +export const d = { + f() { return f(); }, +}; +`, + 'walk.ts': WALK, + }); + expect(callsFrom(cg, 'walk.ts', 'walk')).toEqual(['m.ts:f:1']); + }); + + it('links what a local export clause names', async () => { + const cg = await indexProject({ + 'm.ts': `export const d = { f() {} }; +function f() {} +function impl() {} +export { f, impl as g }; +`, + 'walk.ts': `import { f, g } from './m'; +export function walk() { f(); g(); } +`, + }); + expect(callsFrom(cg, 'walk.ts', 'walk')).toEqual(['m.ts:f:2', 'm.ts:impl:3']); + }); + + it('follows a re-export past an exported object with that method', async () => { + const cg = await indexProject({ + 'm.ts': `export const d = { f() {} }; +export { f } from './other'; +`, + 'other.ts': `export function f() {} +`, + 'walk.ts': WALK, + }); + expect(callsFrom(cg, 'walk.ts', 'walk')).toEqual(['other.ts:f:1']); + }); + + it('links a method the module exports by destructuring its object', async () => { + const cg = await indexProject({ + 'm.ts': `export const d = { f() {} }; +export const { f } = d; +`, + 'walk.ts': `import { f } from './m'; +export function walk() { f(); } +`, + }); + expect(callsFrom(cg, 'walk.ts', 'walk')).toEqual(['m.ts:d::f:1']); + }); + + it('links destructuring exports however their pattern, annotation and object are written', async () => { + const cg = await indexProject({ + 'm.ts': `export const d = { f() {}, g() {}, h() {}, k() {}, inner: { b: 1 } }; +export const { f }: { f: () => void } = d; +export const { + g, // the g + inner: { b }, + h = make(1, 2), +} = d; +export const { k } = d as typeof d; +function make(a: number, b: number) { return () => a + b; } +`, + 'n.ts': `export const e = { p() {}, q() {}, r() {} }; +export const { p, q }: { p: () => void; q: () => void } = e; +export const { r = '}' } = (e); +`, + 'walk.ts': `import { f, g, h, k } from './m'; +import { p, q, r } from './n'; +export function walk() { f(); g(); h(); k(); p(); q(); r(); } +`, + }); + expect(callsFrom(cg, 'walk.ts', 'walk')).toEqual([ + 'm.ts:d::f:1', 'm.ts:d::g:1', 'm.ts:d::h:1', 'm.ts:d::k:1', + 'n.ts:e::p:1', 'n.ts:e::q:1', 'n.ts:e::r:1', + ]); + }); + + it('links a method pulled out of an object and exported by a clause, a default or module.exports', async () => { + const cg = await indexProject({ + 'm.ts': `export type Api = { f(): void }; +export const Api = { f() {} }; +export const { f } = Api; +`, + 'n.ts': `const d = { g() {}, h() {} }; +const { g, h } = d; +export { g, h as hh }; +`, + 'o.ts': `const d = { k() {} }; +const { k } = d; +export default k; +`, + 'p.js': `const o = { q() {} }; +const { q } = o; +module.exports = q; +`, + 'walk.ts': `import { f } from './m'; +import { g, hh } from './n'; +import K from './o'; +const Q = require('./p'); +export function walk() { f(); g(); hh(); K(); Q(); } +`, + }); + expect(callsFrom(cg, 'walk.ts', 'walk')).toEqual([ + 'm.ts:Api::f:2', 'n.ts:d::g:1', 'n.ts:d::h:1', 'o.ts:d::k:1', 'p.js:o::q:1', + ]); + }); + + it("does not answer an import with a class's method", async () => { + const cg = await indexProject({ + 'm.ts': `export class C { render() {} } +`, + 'walk.ts': `import { render } from './m'; +export function walk() { return render(); } +`, + }); + expect(callsFrom(cg, 'walk.ts', 'walk')).toEqual([]); + }); + + it('links a namespace import\'s member to the exported function, not the object\'s method', async () => { + const cg = await indexProject({ + 'm.ts': `export const d = { f() {} }; +export function f() {} +`, + 'walk.ts': `import * as m from './m'; +export function walk() { return m.f(); } +`, + }); + expect(callsFrom(cg, 'walk.ts', 'walk')).toEqual(['m.ts:f:2']); + }); + + it('still links an object method called through its object', async () => { + const cg = await indexProject({ + 'm.ts': `export const d = { f() {} }; +`, + 'walk.ts': `import { d } from './m'; +export function walk() { return d.f(); } +`, + }); + expect(callsFrom(cg, 'walk.ts', 'walk')).toEqual(['m.ts:d::f:1']); + }); +}); + +describe('a default import', () => { + it("links what `export default f` names, not an object's method declared first", async () => { + const cg = await indexProject({ + 'm.ts': `const d = { f() { return 1; } }; +function f() { return d; } +export default f; +`, + 'walk.ts': `import X from './m'; +export function walk() { return X(); } +`, + }); + expect(callsFrom(cg, 'walk.ts', 'walk')).toEqual(['m.ts:f:2']); + }); + + it("links what `module.exports = g` names, not an object's method declared first", async () => { + const cg = await indexProject({ + 'm.js': `const o = { g() { return 1; } }; +function g() { return o; } +module.exports = g; +`, + 'walk.js': `const G = require('./m'); +function walk() { return G(); } +module.exports = { walk }; +`, + }); + expect(callsFrom(cg, 'walk.js', 'walk')).toEqual(['m.js:g:2']); + }); + + it("does not guess an exported object's method", async () => { + const cg = await indexProject({ + 'm.ts': `export const d = { helper() {} }; +declare function memo(c: T): T; +function Card() {} +export default memo(Card); +`, + 'walk.ts': `import X from './m'; +export function walk() { return X(); } +`, + }); + expect(callsFrom(cg, 'walk.ts', 'walk')).toEqual([]); + }); +}); diff --git a/src/resolution/import-resolver.ts b/src/resolution/import-resolver.ts index 6e8546832..d1492238b 100644 --- a/src/resolution/import-resolver.ts +++ b/src/resolution/import-resolver.ts @@ -132,6 +132,14 @@ interface FileExportIndex { * `byName`. `undefined` until first read through {@link exportedByName}. */ clauseAliases?: Map; + /** + * Exported members of something else in the file (`export const d = { f() + * {} }` → `d::f`) by name: no import's first answer, only its last, after + * the file's own bindings, export clause and re-exports (#2945). That last + * answer is what reaches `d.f` through `export const { f } = d`, whose + * binding has no node of its own. + */ + members: Map; } const DEFAULT_BINDING_KINDS = new Set(['function', 'class', 'component', 'constant', 'variable']); @@ -239,9 +247,9 @@ function statedDefaultExport(filePath: string, idx: FileExportIndex, context: Re const to = lineAt(value.nameAt); const node = value.declared ? named.find((n) => DEFAULT_DECLARATION_KINDS.has(n.kind) && n.startLine >= from && n.startLine <= to) - : named + : moduleBindingsFirst(filePath, named .filter((n) => DEFAULT_BINDING_KINDS.has(n.kind)) - .sort((a, b) => a.startLine - b.startLine || a.startColumn - b.startColumn)[0]; + .sort((a, b) => a.startLine - b.startLine || a.startColumn - b.startColumn))[0]; if (node) { stated = node; break; @@ -279,14 +287,19 @@ function getFileExportIndex(filePath: string, context: ResolutionContext): FileE } let idx = perFile.get(filePath); if (!idx) { - idx = { byName: new Map(), defaultComponent: undefined, defaultFnClass: undefined }; + idx = { byName: new Map(), defaultComponent: undefined, defaultFnClass: undefined, members: new Map() }; // Python has no export keyword: every module-level def is importable, and // the extractor never flags one isExported. Without this a Python named // import could only ever resolve through the bare-name matcher. const exported = PYTHON_MODULE_FILE.test(filePath) ? context.getNodesInFile(filePath).filter((n) => n.isExported || isPythonModuleLevelDef(n)) : context.getExportedNodesInFile?.(filePath) ?? context.getNodesInFile(filePath).filter((n) => n.isExported); + const jsFamily = JS_FAMILY_FILE.test(filePath); for (const n of exported) { + if (jsFamily && n.qualifiedName !== n.name) { + if (!idx.members.has(n.name)) idx.members.set(n.name, n); + continue; + } if (!idx.byName.has(n.name)) idx.byName.set(n.name, n); if (idx.defaultComponent === undefined && n.kind === 'component') idx.defaultComponent = n; if (idx.defaultFnClass === undefined && (n.kind === 'function' || n.kind === 'class')) idx.defaultFnClass = n; @@ -296,6 +309,19 @@ function getFileExportIndex(filePath: string, context: ResolutionContext): FileE return idx; } +/** + * `nodes` with the members of something else in their JS/TS file (an + * object's method `d::f`) moved behind the module's own bindings. Extraction + * marks such a member exported, so it can share a name with what an import, + * export clause or default export means (#2945); it stays the last answer + * because `const { f } = d` gives the binding `f` no node but `d::f`. + */ +function moduleBindingsFirst(filePath: string, nodes: Node[]): Node[] { + if (!JS_FAMILY_FILE.test(filePath)) return nodes; + const own = nodes.filter((n) => n.qualifiedName === n.name); + return own.length === nodes.length ? nodes : [...own, ...nodes.filter((n) => n.qualifiedName !== n.name)]; +} + /** The file's nodes named `name`, in `getNodesInFile` order. */ function nodesInFileNamed(filePath: string, name: string, context: ResolutionContext): Node[] { return context.getNodesInFileNamed?.(filePath, name) ?? context.getNodesInFile(filePath).filter((n) => n.name === name); @@ -337,13 +363,14 @@ function exportedByName(filePath: string, idx: FileExportIndex, name: string, co // Bind names introduced by a local export clause to their declarations, so // an importer asking for the renamed name gets the real symbol instead of // falling through to the name-matcher (which cannot cross the rename). - // The declaration is the file's first node of that name, exported or not. + // The declaration is the file's first node of that name, exported or not, + // a member of something else in it only when nothing else has the name. idx.clauseAliases = new Map(); const content = context.readFile(filePath); if (content && content.includes('export')) { for (const { exportedName, localName } of extractLocalExportAliases(content)) { if (idx.byName.has(exportedName) || idx.clauseAliases.has(exportedName)) continue; - const decl = nodesInFileNamed(filePath, localName, context)[0]; + const decl = moduleBindingsFirst(filePath, nodesInFileNamed(filePath, localName, context))[0]; if (decl) idx.clauseAliases.set(exportedName, decl); } } @@ -4081,9 +4108,9 @@ function findExportedSymbolWalk( const next = resolveImportPath(commonJs.source, filePath, language, context); if (next) return findExportedSymbol(next, want, language, context, visited, depth + 1); } else if (commonJs?.name) { - const bound = nodesInFileNamed(filePath, commonJs.name, context) + const bound = moduleBindingsFirst(filePath, nodesInFileNamed(filePath, commonJs.name, context) .filter((n) => DEFAULT_BINDING_KINDS.has(n.kind)) - .sort((a, b) => a.startLine - b.startLine || a.startColumn - b.startColumn)[0]; + .sort((a, b) => a.startLine - b.startLine || a.startColumn - b.startColumn))[0]; if (bound) return bound; } } else if (want.isNamespace && want.memberName) { @@ -4112,7 +4139,7 @@ function findExportedSymbolWalk( // 2. Re-export hit: the file forwards the symbol to another module. const reExports = context.getReExports?.(filePath, language) ?? []; - if (reExports.length === 0) return undefined; + if (reExports.length === 0) return exportedMember(exportIndex, want); // Look for explicit `export { want } from './other'` (with optional rename) — for // `ns.member` through `import * as ns`, the member is the name wanted. @@ -4167,7 +4194,17 @@ function findExportedSymbolWalk( } } - return undefined; + return exportedMember(exportIndex, want); +} + +/** + * The last answer for a named import: a member of something in the file, as + * `export const { f } = d` exports `d::f`, once nothing the file declares or + * forwards is the name (#2945). + */ +function exportedMember(idx: FileExportIndex, want: { isDefault: boolean; isNamespace: boolean; exportedName: string; memberName: string | null }): Node | undefined { + const name = want.isDefault ? null : want.isNamespace ? want.memberName : want.exportedName; + return name ? idx.members.get(name) : undefined; } /** Node kinds that own static members reachable as `Container.member`. */