fix(resolution): tighten the ESM export marker (Opus review of c5a4c2a6)

- `export { a } from './x'` / `export type { T } from './t'` re-export another
  module's names and no longer mark a same-named local exported (skip
  export_statements with a source clause).
- An `export` inside `namespace NS { … }` or `declare module 'x' { … }` is that
  container's export, not the file's: the walk now records the export and
  still stops false at the nesting boundary (internal_module, module,
  ambient_declaration added).
- The CommonJS sniff reads AST nodes (member/subscript expressions on
  `module.exports` / `exports`) instead of source text, so a comment or string
  mentioning module.exports no longer silences the file's verdicts.
- Tests for all three in esm-export-marker.test.ts.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015dSxjDEzuYDENrR314EiFc
This commit is contained in:
Abhinav Pandey 2026-09-05 14:31:14 +05:30
parent c5a4c2a63b
commit 042dfd86f6
No known key found for this signature in database
2 changed files with 93 additions and 6 deletions

View file

@ -44,8 +44,33 @@ export interface EsmExportEvidence {
readonly commonJs: boolean;
}
const CJS_EXPORT_ASSIGNMENT = /^\s*(module\.exports\b|exports\s*[.[]|this\.[A-Za-z_$][\w$]*\s*=)/;
const CJS_SURFACE = /\bmodule\.exports\b|\bexports\s*[.[=]/;
const CJS_EXPORT_ASSIGNMENT = /^\s*(this\.[A-Za-z_$][\w$]*\s*=)/;
/**
* Does the file touch a CommonJS export object anywhere `module.exports` or
* `exports.x` / `exports[x]` as an actual expression? Read from AST nodes,
* not source text, so a comment or string mentioning `module.exports` does not
* disable the file's verdicts.
*/
function hasCommonJsExportSurface(root: SyntaxNode): boolean {
for (const member of root.descendantsOfType('member_expression')) {
const object = member.childForFieldName('object');
if (object === null) continue;
if (object.type === 'identifier' && object.text === 'exports') return true;
if (
object.type === 'identifier' &&
object.text === 'module' &&
member.childForFieldName('property')?.text === 'exports'
) {
return true;
}
}
for (const sub of root.descendantsOfType('subscript_expression')) {
const object = sub.childForFieldName('object');
if (object?.type === 'identifier' && object.text === 'exports') return true;
}
return false;
}
/** Node types below which a declaration is nested, not module-level. */
const NESTING_BOUNDARIES: ReadonlySet<string> = new Set([
@ -60,7 +85,12 @@ const NESTING_BOUNDARIES: ReadonlySet<string> = new Set([
'generator_function',
'generator_function_declaration',
'method_definition',
// `export namespace NS { export function f() {} }` / `declare module 'x' {
// export function q(): void }`: an `export` inside these bodies is an export
// of the namespace/ambient module, not of the file.
'internal_module',
'module',
'ambient_declaration',
]);
/**
@ -75,11 +105,14 @@ export function collectEsmExportEvidence(
const namedLocals = new Set<string>();
// Any CommonJS export surface anywhere in the file — a direct `module.exports
// = …`, an alias (`const m = module.exports; m.x = …`), an `exports.x` — means
// "not under `export`" says nothing. Text-level on purpose: the alias forms
// are open-ended and a missed one would mark a real export private.
let commonJs = CJS_SURFACE.test(root.text);
// "not under `export`" says nothing.
let commonJs = hasCommonJsExportSurface(root);
for (const stmt of root.namedChildren) {
if (stmt.type === 'export_statement') {
// `export { a } from './x'` / `export type { T } from './t'` re-export
// ANOTHER module's names: they say nothing about a local `a`, and adding
// them here marked a private local of the same name exported.
if (stmt.childForFieldName('source') !== null) continue;
for (const child of stmt.namedChildren) {
if (child.type === 'export_clause') {
for (const spec of child.namedChildren) {
@ -128,8 +161,16 @@ export function esmExportVerdict(
// declaration itself (a `method_definition`, a `function_declaration`) must
// not count as its own nesting boundary.
let current: SyntaxNode | null = nameNode.parent;
// An `export` keyword is only a FILE-level export when the walk reaches the
// program without crossing a nesting boundary — one inside a namespace or
// ambient-module body is that container's export (see NESTING_BOUNDARIES).
let underExport = false;
while (current !== null && current.type !== 'program') {
if (current.type === 'export_statement') return true;
if (current.type === 'export_statement') {
underExport = true;
current = current.parent;
continue;
}
if (current.type === 'object') {
// `module.exports = { alpha() {} }`: the literal's own members are the
// module's exports. Any other object literal is a nesting boundary.
@ -141,6 +182,7 @@ export function esmExportVerdict(
}
current = current.parent;
}
if (underExport) return true;
if (evidence.commonJs) return undefined;
return evidence.namedLocals.has(nameNode.text);
}

View file

@ -104,6 +104,51 @@ describe('@declaration.is-exported (TypeScript emitter)', () => {
});
});
describe('@declaration.is-exported — Opus review follow-ups', () => {
it('a re-export FROM another module never marks a same-named local exported', () => {
const v = verdicts(
emitTsScopeCaptures,
"export { alpha } from './other';\nexport { beta as gamma } from './o';\nexport type { T } from './t';\nfunction alpha() {}\nfunction beta() {}\nfunction gamma() {}\ntype T = number;\nexport const keep = 1;\n",
'test.ts',
);
expect(v.alpha).not.toBe('true');
expect(v.beta).not.toBe('true');
expect(v.gamma).not.toBe('true');
expect(v.T).not.toBe('true');
expect(v.alpha).toBe('false');
expect(v.keep).toBe('true');
});
it('exports inside a namespace or ambient module body are not file-level exports', () => {
const v = verdicts(
emitTsScopeCaptures,
"export namespace NS { export function f() {} }\ndeclare module 'x' { export function q(): void; }\nexport function top() {}\n",
'test.ts',
);
expect(v.NS).toBe('true');
expect(v.f).not.toBe('true');
expect(v.q).not.toBe('true');
expect(v.top).toBe('true');
});
it('a comment or string mentioning module.exports does not silence the ESM verdicts', () => {
const v = verdicts(
emitTsScopeCaptures,
"// legacy: module.exports = api\nconst note = 'exports.x = 1';\nexport function a() {}\nfunction b() {}\n",
'test.ts',
);
expect(v.a).toBe('true');
expect(v.b).toBe('false');
// ...while a real alias of the export object still does.
const cjs = verdicts(
emitJsScopeCaptures,
'const m = module.exports;\nfunction b() {}\nm.b = b;\n',
'x.js',
);
expect(cjs.b).toBeUndefined();
});
});
describe('@declaration.is-exported (JavaScript emitter)', () => {
it('marks ESM declarations', () => {
const v = verdicts(emitJsScopeCaptures, ESM, 'test.js');