diff --git a/packages/compiler/src/frontend/lowering/cjs-exports-object.ts b/packages/compiler/src/frontend/lowering/cjs-exports-object.ts index 5ff34a326..f06dde5a2 100644 --- a/packages/compiler/src/frontend/lowering/cjs-exports-object.ts +++ b/packages/compiler/src/frontend/lowering/cjs-exports-object.ts @@ -33,6 +33,7 @@ import { } from "../program.js"; import { cjsWholeExportValueOf, inlineProgramRequireOf } from "./cjs-inline-require.js"; import { isCjsModuleGlobal } from "./lower-node-module.js"; +import { bindingNeverReassigned } from "./binding-analysis.js"; import { DYN, type IrExpr, type IrGlobal } from "../../ir/ir.js"; /** True for the CommonJS wrapper's `exports` binding (not shadowed by any @@ -492,32 +493,68 @@ function memberAliasIn( name: string, depth: number, ): ts.Expression | null { + const member = memberWritesIn(lowerer, sf, name, depth); + // One write: a member written again would be a runtime slot. + return member !== null && member.writes === 1 ? member.value : null; +} + +/** The writes of member `name` in `sf` and in the modules it re-exports + * through `module.exports = require("./x")`, which share its exports + * object. `value` is the last write's value when the use site can read + * it again. Null when the module does not export a value whole. */ +function memberWritesIn( + lowerer: Lowerer, + sf: ts.SourceFile, + name: string, + depth: number, +): { value: ts.Expression | null; writes: number } | null { if (depth > 32 || sf.isDeclarationFile || !isCjsJsFile(sf, lowerer.program)) return null; const whole = cjsWholeExportValueOf(sf); if (whole === null) return null; - const inner = inlineProgramRequireOf(lowerer, whole.rhs); - if (inner !== null) return memberAliasIn(lowerer, inner.dep, name, depth + 1); - let found: ts.Expression | null = null; + let value: ts.Expression | null = null; let writes = 0; + const inner = inlineProgramRequireOf(lowerer, whole.rhs); + if (inner !== null) { + // The re-exported module runs first; this module's own writes follow. + const dep = memberWritesIn(lowerer, inner.dep, name, depth + 1); + if (dep === null) return null; + ({ value, writes } = dep); + } for (const stmt of sf.statements.nodes) { const cjs = cjsExportAssignmentOf(stmt); if (cjs?.kind !== "member" || !ts.isIdentifier(cjs.name) || cjs.name.text !== name) continue; writes++; if (cjsExportDiscardReason(stmt) !== null) return null; - let value: ts.Expression = cjs.value; - while (ts.isParenthesizedExpression(value)) value = value.expression; - // A binding of the module, or a primitive literal: reading either - // again at the use site observes the member's value. - if ( - ts.isIdentifier(value) || - ts.isStringLiteral(value) || - ts.isNumericLiteral(value) || - ts.isNoSubstitutionTemplateLiteral(value) || - value.kind === ts.SyntaxKind.TrueKeyword || - value.kind === ts.SyntaxKind.FalseKeyword - ) - found = value; + let written: ts.Expression = cjs.value; + while (ts.isParenthesizedExpression(written)) written = written.expression; + value = rereadableMemberValue(lowerer, stmt, written) ? written : null; } - // One write: a member written again would be a runtime slot. - return writes === 1 ? found : null; + return { value, writes }; +} + +/** True when reading `value` again at the use site observes what the + * member write `stmt` stored: a primitive literal, or a module binding + * declared before the write and never reassigned. */ +function rereadableMemberValue( + lowerer: Lowerer, + stmt: ts.Statement, + value: ts.Expression, +): boolean { + if ( + ts.isStringLiteral(value) || + ts.isNumericLiteral(value) || + ts.isNoSubstitutionTemplateLiteral(value) || + value.kind === ts.SyntaxKind.TrueKeyword || + value.kind === ts.SyntaxKind.FalseKeyword + ) + return true; + if (!ts.isIdentifier(value)) return false; + const sym = lowerer.resolveValueSymbol(value); + if (sym === null || sym.declarations.length !== 1) return false; + const decl = sym.declarations[0]!; + if (decl.getSourceFile() !== stmt.getSourceFile()) return false; + // A function declaration is initialized when the module starts; any + // other binding must be initialized before the write stores it. + if (!ts.isFunctionDeclaration(decl) && decl.end > stmt.pos) return false; + return bindingNeverReassigned(lowerer, sym, decl); } diff --git a/packages/compiler/src/frontend/lowering/lower-stmts.ts b/packages/compiler/src/frontend/lowering/lower-stmts.ts index 5fddb31d5..c9392db8c 100644 --- a/packages/compiler/src/frontend/lowering/lower-stmts.ts +++ b/packages/compiler/src/frontend/lowering/lower-stmts.ts @@ -6922,7 +6922,7 @@ export function lowerExprStatement(lowerer: Lowerer, expr: ts.Expression): IrStm lowerer.unsupported( "SC1090", expr.left, - "rebinding 'exports' (exports = module.exports = value): members attached through 'exports' afterwards are properties of that value, which the static export table does not model — write module.exports. = … and module.exports = value", + "rebinding 'exports' without module.exports (exports = value): members attached through 'exports' afterwards are properties of that value, which module.exports does not hold — write exports = module.exports = value", ); } const cjs = cjsExportAssignmentOf(stmtNode); diff --git a/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/6431-cjs-exports-rebinding/main.cjs.json b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/6431-cjs-exports-rebinding/main.cjs.json index 25277f4a5..8fc4a2acc 100644 --- a/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/6431-cjs-exports-rebinding/main.cjs.json +++ b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/6431-cjs-exports-rebinding/main.cjs.json @@ -1,6 +1,7 @@ { "order": [ "/tests/corpus/6431-cjs-exports-rebinding/lib.cjs", + "/tests/corpus/6431-cjs-exports-rebinding/extended.cjs", "/tests/corpus/6431-cjs-exports-rebinding/main.cjs" ], "diags": [] diff --git a/packages/compiler/test/ts7/baselines/order-parity/tests/diagnostics/cjs-member-alias-writes/main.ts.json b/packages/compiler/test/ts7/baselines/order-parity/tests/diagnostics/cjs-member-alias-writes/main.ts.json new file mode 100644 index 000000000..bd3ba799d --- /dev/null +++ b/packages/compiler/test/ts7/baselines/order-parity/tests/diagnostics/cjs-member-alias-writes/main.ts.json @@ -0,0 +1,6 @@ +{ + "order": [ + "/tests/diagnostics/cjs-member-alias-writes/main.ts" + ], + "diags": [] +} diff --git a/tests/corpus/6431-cjs-exports-rebinding/extended.cjs b/tests/corpus/6431-cjs-exports-rebinding/extended.cjs new file mode 100644 index 000000000..d07438ee9 --- /dev/null +++ b/tests/corpus/6431-cjs-exports-rebinding/extended.cjs @@ -0,0 +1,3 @@ +"use strict"; +module.exports = require("./lib.cjs"); +module.exports.extra = "added"; diff --git a/tests/corpus/6431-cjs-exports-rebinding/main.cjs b/tests/corpus/6431-cjs-exports-rebinding/main.cjs index 5c52ff687..cfeec742b 100644 --- a/tests/corpus/6431-cjs-exports-rebinding/main.cjs +++ b/tests/corpus/6431-cjs-exports-rebinding/main.cjs @@ -4,3 +4,6 @@ const format = require("./lib.cjs"); const { shout, version } = require("./lib.cjs"); console.log(format("a"), shout("b"), version); console.log(typeof format, typeof shout); +// A module that re-exports another module's exports and adds a member. +const { extra, shout: loud } = require("./extended.cjs"); +console.log(extra, loud("c")); diff --git a/tests/diagnostics/cjs-member-alias-writes/main.ts b/tests/diagnostics/cjs-member-alias-writes/main.ts new file mode 100644 index 000000000..d3723c41a --- /dev/null +++ b/tests/diagnostics/cjs-member-alias-writes/main.ts @@ -0,0 +1,6 @@ +// A member export is read through its binding only when that binding still +// holds the exported value: a member that holds a reassigned binding, or one +// that both a module and its re-exporter write, is refused. +import { counter, version } from "metrics"; +console.log(counter); +console.log(version); diff --git a/tests/diagnostics/cjs-member-alias-writes/node_modules/metrics/index.d.ts b/tests/diagnostics/cjs-member-alias-writes/node_modules/metrics/index.d.ts new file mode 100644 index 000000000..c9c639246 --- /dev/null +++ b/tests/diagnostics/cjs-member-alias-writes/node_modules/metrics/index.d.ts @@ -0,0 +1,6 @@ +declare function metrics(): string; +declare namespace metrics { + const counter: number; + const version: string; +} +export = metrics; diff --git a/tests/diagnostics/cjs-member-alias-writes/node_modules/metrics/index.js b/tests/diagnostics/cjs-member-alias-writes/node_modules/metrics/index.js new file mode 100644 index 000000000..7449f0e8a --- /dev/null +++ b/tests/diagnostics/cjs-member-alias-writes/node_modules/metrics/index.js @@ -0,0 +1,3 @@ +"use strict"; +module.exports = require("./lib.js"); +module.exports.version = "2.0.0"; diff --git a/tests/diagnostics/cjs-member-alias-writes/node_modules/metrics/lib.js b/tests/diagnostics/cjs-member-alias-writes/node_modules/metrics/lib.js new file mode 100644 index 000000000..913fe8bd1 --- /dev/null +++ b/tests/diagnostics/cjs-member-alias-writes/node_modules/metrics/lib.js @@ -0,0 +1,8 @@ +"use strict"; +let counter = 1; +exports = module.exports = function metrics() { + return "metrics"; +}; +exports.counter = counter; +exports.version = "1.0.0"; +counter = 5; diff --git a/tests/diagnostics/cjs-member-alias-writes/node_modules/metrics/package.json b/tests/diagnostics/cjs-member-alias-writes/node_modules/metrics/package.json new file mode 100644 index 000000000..f97fe551f --- /dev/null +++ b/tests/diagnostics/cjs-member-alias-writes/node_modules/metrics/package.json @@ -0,0 +1,6 @@ +{ + "name": "metrics", + "version": "1.0.0", + "main": "index.js", + "types": "index.d.ts" +} diff --git a/tests/harness/__snapshots__/cjs-member-alias-writes/main.ts.txt b/tests/harness/__snapshots__/cjs-member-alias-writes/main.ts.txt new file mode 100644 index 000000000..1682e9bfb --- /dev/null +++ b/tests/harness/__snapshots__/cjs-member-alias-writes/main.ts.txt @@ -0,0 +1,13 @@ +cjs-member-alias-writes/main.ts:5:13 - error SC1090: the reference to 'counter' (a binding form with no lowering) is not supported yet + + 4 | import { counter, version } from "metrics"; + 5 | console.log(counter); + | ^~~~~~~ + 6 | console.log(version); + +cjs-member-alias-writes/main.ts:6:13 - error SC1090: the reference to 'version' (a binding form with no lowering) is not supported yet + + 5 | console.log(counter); + 6 | console.log(version); + | ^~~~~~~ + 7 | \ No newline at end of file diff --git a/tests/harness/errors.test.ts b/tests/harness/errors.test.ts index 26dfd3a5c..8c60a1f81 100644 --- a/tests/harness/errors.test.ts +++ b/tests/harness/errors.test.ts @@ -1174,7 +1174,7 @@ console.log(keys(exports)); [ "exports-rebinding", `console.log("before"); -exports = module.exports = function hash(x) { +exports = function hash(x) { return x; }; exports.extra = 1; @@ -1182,7 +1182,7 @@ exports.extra = 1; {}, "before\n", 2, - "rebinding 'exports' \\(exports = module\\.exports = value\\)", + "rebinding 'exports' without module\\.exports \\(exports = value\\)", ], [ "require-computed",