mirror of
https://github.com/colbymchenry/codegraph.git
synced 2026-10-21 16:57:09 +08:00
fix(resolution): an unexported ES module binding is not reachable cross-file (#2162)
The sealed-module rule only covered modules exporting nothing. Per symbol
now: a JS/TS top-level binding declared by its own statement (function,
const/let/var, class, interface, type, enum) in an ES module is reachable
from another file only if it's exported — the `export` keyword (node flag or
its line), an `export { a as b }` list, `export default x` or
`export default { a, b }`. Classic scripts, CommonJS, `declare global`,
`.d.ts`, qualified members, object-literal members (zustand store actions)
and `proto.x = function x()` are exempt. Applied at the reachability gate,
so the winner is rejected rather than another promoted.
A/B (edges removed / added): zod -229 (package `z` imports → a local `z`),
excalidraw -66 (DOM `Element` → a test helper's type), kit -50, typeorm -47
(`import { ColumnMetadata } from "typeorm"` → a fixture's local), trpc -26,
hono -8, axios -1; express byte-identical.
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -14,6 +14,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
|
||||
|
||||
### Fixes
|
||||
|
||||
- In JavaScript and TypeScript, a module's top-level function, variable or type that the module doesn't export is no longer linked from other files. Every form of export still counts: the `export` keyword, `export { … }` lists, `export default`, and a default-exported object. On typeorm, test fixtures' local `ColumnMetadata` used to capture `import { ColumnMetadata } from "typeorm"`; on sveltekit, an unexported `resolve` collected other files' `resolve(…)` calls; on zod, a local `z` collected the `zod` package imports.
|
||||
- Flow-typed JavaScript files (those with `@flow` in their header comment) are now parsed like TypeScript, so type annotations like `render(): React.Node` no longer cut classes short. This mostly affects React Native code: on React Native's own libraries the graph gained over 8,000 links, and component classes keep their methods. Files without the pragma are unchanged. Re-index after upgrading.
|
||||
- In Scala, a call reached by its name alone now stays in reach. A name that the enclosing function binds as a parameter or local (`f(true)` for `f: A => B`) is that local. A call later in a chain, like `fa.iterator.map(…)`, reaches only a member of what its receiver is named after. A plain call reaches only the enclosing class and its supertypes (an anonymous subclass's base included), a companion, or an imported object. On cats, parameter calls like `f(a)` used to land on a case class's field `f` over 400 times, and chained `.map(…)` on one ops class's `map`.
|
||||
- A link made by a framework or bridge resolver now records which one made it (`metadata.framework`, such as `swift-objc-bridge` or `react-native-bridge`), as the README describes. Before, only synthesized event and view channels were named.
|
||||
|
||||
@@ -0,0 +1,77 @@
|
||||
/**
|
||||
* A top-level binding an ES module doesn't export can't be named from another
|
||||
* file. sveltekit's `generate_manifest.js` keeps an unexported `resolve` that
|
||||
* other files' `resolve(…)` calls went to; typeorm's test inputs' local
|
||||
* `ColumnMetadata` took `import { ColumnMetadata } from "typeorm"`. Exported
|
||||
* forms all still count: the `export` keyword, `export { a as b }`,
|
||||
* `export default x`, `export default { a, b }`; so do classic scripts,
|
||||
* CommonJS files and functions assigned onto a prototype.
|
||||
*/
|
||||
import { describe, it, expect, afterAll, beforeAll } from 'vitest';
|
||||
import * as fs from 'fs';
|
||||
import * as os from 'os';
|
||||
import * as path from 'path';
|
||||
import { CodeGraph } from '../src';
|
||||
|
||||
let root = '';
|
||||
let cg: CodeGraph;
|
||||
|
||||
beforeAll(async () => {
|
||||
root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-esm-unexported-'));
|
||||
const files: Record<string, string> = {
|
||||
'package.json': JSON.stringify({ name: 'app', type: 'module' }),
|
||||
'src/manifest.js': `import { join } from 'node:path';
|
||||
function resolve(event) { return join(event); }
|
||||
export function manifest() { return resolve('x'); }
|
||||
`,
|
||||
'src/adapters.js': `import { a } from './a.js';
|
||||
function getAdapter(name) { return name; }
|
||||
export default { getAdapter };
|
||||
`,
|
||||
'src/listed.js': `import { a } from './a.js';
|
||||
function helper() { return 1; }
|
||||
export { helper };
|
||||
`,
|
||||
'src/proto.js': `import { a } from './a.js';
|
||||
function Params() {}
|
||||
const prototype = Params.prototype;
|
||||
prototype.render = function render() { return ''; };
|
||||
export default Params;
|
||||
`,
|
||||
'src/a.js': `export const a = 1;
|
||||
`,
|
||||
'src/use.js': `import { helper } from './listed.js';
|
||||
import Params from './proto.js';
|
||||
export async function handle(event) {
|
||||
await resolve(event);
|
||||
getAdapter('xhr');
|
||||
helper();
|
||||
new Params().render();
|
||||
}
|
||||
`,
|
||||
};
|
||||
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);
|
||||
}
|
||||
cg = await CodeGraph.init(root, { index: true });
|
||||
});
|
||||
|
||||
afterAll(() => {
|
||||
cg?.close();
|
||||
if (root) fs.rmSync(root, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
describe('unexported ES module bindings', () => {
|
||||
it('are not reached from another file; every export form still is', () => {
|
||||
const handle = cg.getNodesInFile('src/use.js').find((n) => n.name === 'handle')!;
|
||||
const targets = cg
|
||||
.getOutgoingEdges(handle.id)
|
||||
.filter((e) => e.kind === 'calls')
|
||||
.map((e) => `${cg.getNode(e.target)!.filePath}:${cg.getNode(e.target)!.name}`);
|
||||
expect(targets).not.toContain('src/manifest.js:resolve');
|
||||
expect(targets).toContain('src/adapters.js:getAdapter');
|
||||
expect(targets).toContain('src/listed.js:helper');
|
||||
expect(targets).toContain('src/proto.js:render');
|
||||
});
|
||||
});
|
||||
@@ -781,10 +781,68 @@ function isCrossFileReachable(
|
||||
return (
|
||||
candidate.filePath === ref.filePath ||
|
||||
!ESM_FAMILY.has(candidate.language) ||
|
||||
!isSealedModule(candidate.filePath, context)
|
||||
(!isSealedModule(candidate.filePath, context) && !isUnexportedModuleBinding(candidate, context))
|
||||
);
|
||||
}
|
||||
|
||||
const ESM_BINDING_KINDS: ReadonlySet<string> = new Set(['function', 'variable', 'constant', 'class', 'interface', 'type_alias', 'enum', 'component']);
|
||||
const ESM_EXPORT_LISTS = new WeakMap<ResolutionContext, Map<string, { module: boolean; names: Set<string> }>>();
|
||||
|
||||
/**
|
||||
* Whether `candidate` is a top-level binding of an ES module that the module
|
||||
* doesn't export — declared without `export` and absent from its `export { … }`
|
||||
* / `export default x` lists. No other file can name it. The sealed-module
|
||||
* rule above covers files that export nothing; this is the same boundary per
|
||||
* symbol: sveltekit's `generate_manifest.js` keeps an unexported `resolve`
|
||||
* that twenty other files' `resolve(…)` calls went to. Classic scripts,
|
||||
* CommonJS, `declare global` and `.d.ts` files, members of a class or
|
||||
* namespace (qualified names), names a default-exported object literal lists,
|
||||
* and anything not declared by a statement of its own (an object literal's
|
||||
* member, `proto.x = function x() {}`) are exempt.
|
||||
*/
|
||||
function isUnexportedModuleBinding(candidate: Node, context: ResolutionContext): boolean {
|
||||
if (candidate.isExported || !ESM_BINDING_KINDS.has(candidate.kind)) return false;
|
||||
if (candidate.qualifiedName.includes('::') || /\.d\.[cm]?ts$/.test(candidate.filePath)) return false;
|
||||
let memo = ESM_EXPORT_LISTS.get(context);
|
||||
if (!memo) ESM_EXPORT_LISTS.set(context, (memo = new Map()));
|
||||
let info = memo.get(candidate.filePath);
|
||||
if (!info) {
|
||||
const source = context.readFile(candidate.filePath) ?? '';
|
||||
const code = blankStringContents(stripCommentsForRegex(source, 'typescript'));
|
||||
const module = (HAS_IMPORT_STATEMENT.test(code) || HAS_ESM_EXPORT.test(code)) &&
|
||||
!HAS_CJS_EXPORT.test(source) && !/\bdeclare\s+global\b/.test(code);
|
||||
const names = new Set<string>();
|
||||
if (module) {
|
||||
for (const m of source.matchAll(/^[ \t]*export\s+(?:type\s+)?\{([^}]*)\}/gm)) {
|
||||
for (const item of m[1]!.split(',')) {
|
||||
const local = item.trim().replace(/^type\s+/, '').split(/\s+as\s+/)[0]!.trim();
|
||||
if (local) names.add(local);
|
||||
}
|
||||
}
|
||||
for (const m of source.matchAll(/^[ \t]*export\s+(?:default|=)\s+([A-Za-z_$][\w$]*)\s*;?\s*$/gm)) names.add(m[1]!);
|
||||
// `export default { getAdapter, adapters: known }` exposes its shorthand and value names.
|
||||
for (const m of code.matchAll(/^[ \t]*export\s+default\s+\{([^}]*)\}/gm)) {
|
||||
for (const item of m[1]!.split(',')) {
|
||||
const value = item.includes(':') ? item.split(':').pop()! : item;
|
||||
const id = /^\s*([A-Za-z_$][\w$]*)\s*$/.exec(value)?.[1];
|
||||
if (id) names.add(id);
|
||||
}
|
||||
}
|
||||
}
|
||||
info = { module, names };
|
||||
memo.set(candidate.filePath, info);
|
||||
}
|
||||
if (!info.module || info.names.has(candidate.name)) return false;
|
||||
const line = (context.getFileLines?.(candidate.filePath) ?? context.readFile(candidate.filePath)?.split('\n'))?.[candidate.startLine - 1] ?? '';
|
||||
// Its own line says `export` (a node's flag can miss a form), or it is
|
||||
// `prototype.toString = function toString() {…}`, reached through instances.
|
||||
if (/^\s*export\b/.test(line)) return false;
|
||||
// Only a declaration statement is a module binding: an object literal's
|
||||
// member (a zustand store action `setZipUri: (v) => set(…)`) is reached
|
||||
// through the object, and so is `proto.x = function x() {}`.
|
||||
return /^\s*(?:declare\s+)?(?:async\s+)?(?:function\*?|const|let|var|(?:abstract\s+)?class|interface|type|enum)\s/.test(line);
|
||||
}
|
||||
|
||||
const LUA_LOCALS = new WeakMap<ResolutionContext, Map<string, boolean>>();
|
||||
|
||||
/** Whether a Lua variable or function is declared `local` (`local x = …`, `local function f`, `local a, x = …`). */
|
||||
@@ -3709,6 +3767,7 @@ export function clearNameMatcherMemos(context: ResolutionContext): void {
|
||||
CSHARP_STATIC_USINGS.delete(context);
|
||||
SCALA_SUPERS.delete(context);
|
||||
SCALA_IMPORTS.delete(context);
|
||||
ESM_EXPORT_LISTS.delete(context);
|
||||
LUA_LOCALS.delete(context);
|
||||
PHP_FILE_SCOPES.delete(context);
|
||||
JAVA_STATIC_IMPORTS.delete(context);
|
||||
|
||||
Reference in New Issue
Block a user