diff --git a/packages/compiler/src/frontend/lowering/cjs-exports-object.ts b/packages/compiler/src/frontend/lowering/cjs-exports-object.ts index 5ea4001fc..f06dde5a2 100644 --- a/packages/compiler/src/frontend/lowering/cjs-exports-object.ts +++ b/packages/compiler/src/frontend/lowering/cjs-exports-object.ts @@ -22,13 +22,18 @@ import * as ts from "../ts7/adapter.js"; import type { Lowerer } from "./lowerer.js"; import { cjsExportAssignmentOf, + cjsExportDiscardReason, cjsExportTargetLiteral, isCjsJsFile, isModuleExportsAccess, isRequireStatement, locOf, + npmStaticDepSf7, + resolveImport, } 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 @@ -428,3 +433,128 @@ export function cjsExportCallIgnoresThis( return false; }); } + +/** The value behind a binding of a CommonJS member export (an `import { + * name }` or a top-level `const { name } = require(...)`) written once as + * `module.exports. = ` or `exports. = `, when the + * module exports a value whole and the member is a property added to it + * (`exports = module.exports = Client; exports.StatsD = Client`). The + * checker gives such a binding a synthesized property symbol, so the + * module's statements answer. The value is a binding of the exporting + * module or a primitive literal, which the use site reads again. Follows + * `module.exports = require("./x")` re-exports. Null for any other binding. */ +export function cjsMemberAliasValueOf(lowerer: Lowerer, ref: ts.Identifier): ts.Expression | null { + const sym = lowerer.checker.getSymbolAtLocation(ref); + if (!sym) return null; + const decls = sym.declarations; + // `import { StatsD } from "pkg"` + const specifier = sym.flags & ts.SymbolFlags.Alias ? decls.find(ts.isImportSpecifier) : undefined; + if (specifier !== undefined) { + const importDecl = specifier.parent?.parent?.parent; + if ( + importDecl === undefined || + !ts.isImportDeclaration(importDecl) || + !ts.isStringLiteral(importDecl.moduleSpecifier) + ) + return null; + const spec = importDecl.moduleSpecifier.text; + const from = importDecl.getSourceFile(); + const dep = + resolveImport(lowerer.program, from, spec, "import") ?? + npmStaticDepSf7(lowerer.program, from, spec, "import"); + if (dep === null) return null; + return memberAliasIn(lowerer, dep, (specifier.propertyName ?? specifier.name).text, 0); + } + // `const { shout } = require("./lib")` at a module's top level + const element = decls.find(ts.isBindingElement); + if (element === undefined || element.dotDotDotToken || !ts.isIdentifier(element.name)) + return null; + const pattern = element.parent; + const declaration = pattern?.parent; + if ( + pattern === undefined || + !ts.isObjectBindingPattern(pattern) || + declaration === undefined || + !ts.isVariableDeclaration(declaration) || + declaration.initializer === undefined || + (ts.getCombinedNodeFlags(declaration) & ts.NodeFlags.Const) === 0 + ) + return null; + const key = element.propertyName ?? element.name; + if (!ts.isIdentifier(key)) return null; + const req = inlineProgramRequireOf(lowerer, declaration.initializer); + if (req === null) return null; + return memberAliasIn(lowerer, req.dep, key.text, 0); +} + +function memberAliasIn( + lowerer: Lowerer, + sf: ts.SourceFile, + 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; + 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 written: ts.Expression = cjs.value; + while (ts.isParenthesizedExpression(written)) written = written.expression; + value = rereadableMemberValue(lowerer, stmt, written) ? written : 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/expressions/identifiers.ts b/packages/compiler/src/frontend/lowering/expressions/identifiers.ts index 5a8169300..2ee56573c 100644 --- a/packages/compiler/src/frontend/lowering/expressions/identifiers.ts +++ b/packages/compiler/src/frontend/lowering/expressions/identifiers.ts @@ -1,4 +1,5 @@ /* Identifier references in expression position: lowerExprInner's identifier branch and its helpers. */ +import { cjsMemberAliasValueOf } from "../cjs-exports-object.js"; import * as ts from "../../ts7/adapter.js"; import * as posix from "node:path/posix"; import { type Lowerer, PoisonError } from "../lowerer.js"; @@ -859,6 +860,11 @@ export function lowerIdentifierExpr(lowerer: Lowerer, expr: ts.Identifier, loc: lowerer.checker.getSymbolAtLocation(expr), ); } + // An import of a CommonJS member export whose value is a binding of the + // exporting module (`module.exports = Client; module.exports.StatsD = + // Client`): the member holds that binding's value. + const memberAlias = cjsMemberAliasValueOf(lowerer, expr); + if (memberAlias !== null) return lowerer.lowerExpr(memberAlias); // Preflight guarantees no unresolved identifiers; anything else here // is a blocked declaration's binding (the SC2004 cascade) or a // binding form we don't model yet. diff --git a/packages/compiler/src/frontend/lowering/lower-stmts.ts b/packages/compiler/src/frontend/lowering/lower-stmts.ts index e1ff31018..e530a57f7 100644 --- a/packages/compiler/src/frontend/lowering/lower-stmts.ts +++ b/packages/compiler/src/frontend/lowering/lower-stmts.ts @@ -6929,12 +6929,13 @@ export function lowerExprStatement(lowerer: Lowerer, expr: ts.Expression): IrStm if ( ts.isBinaryExpression(expr) && expr.operatorToken.kind === ts.SyntaxKind.EqualsToken && - isCjsExportsIdentifier(lowerer, expr.left) + isCjsExportsIdentifier(lowerer, expr.left) && + cjsExportAssignmentOf(stmtNode)?.kind !== "table" ) { 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/src/frontend/program.ts b/packages/compiler/src/frontend/program.ts index d7c2ba66a..906d901fe 100644 --- a/packages/compiler/src/frontend/program.ts +++ b/packages/compiler/src/frontend/program.ts @@ -5967,10 +5967,15 @@ export function isModuleExportsAccess(expr: ts.Expression): expr is ts.PropertyA * - member: `exports.f = ` / `module.exports.f = `. * Null for everything else. Callers gate on top-level position in a JS * module file — these forms anywhere else keep their generic fences. */ -export function cjsExportAssignmentOf( - stmt: ts.Statement, -): - | { kind: "table"; obj: ts.ObjectLiteralExpression | null; expr: ts.BinaryExpression } +export function cjsExportAssignmentOf(stmt: ts.Statement): + | { + kind: "table"; + obj: ts.ObjectLiteralExpression | null; + expr: ts.BinaryExpression; + /** `exports = module.exports = value`: the `exports` binding names + * the new value too, so later `exports.f =` members attach to it. */ + rebindsExports?: true; + } | { kind: "member"; name: ts.MemberName; value: ts.Expression; expr: ts.BinaryExpression } | null { if (!ts.isExpressionStatement(stmt)) return null; @@ -5979,6 +5984,20 @@ export function cjsExportAssignmentOf( if (isModuleExportsAccess(e.left)) { return { kind: "table", obj: ts.isObjectLiteralExpression(e.right) ? e.right : null, expr: e }; } + if (ts.isIdentifier(e.left) && e.left.text === "exports") { + let inner: ts.Expression = e.right; + while (ts.isParenthesizedExpression(inner)) inner = inner.expression; + if ( + ts.isBinaryExpression(inner) && + inner.operatorToken.kind === ts.SyntaxKind.EqualsToken && + isModuleExportsAccess(inner.left) && + // `exports = module.exports = {}` is the fresh-table reset, which the + // export walk handles on its own. + !ts.isObjectLiteralExpression(inner.right) + ) { + return { kind: "table", obj: null, expr: inner, rebindsExports: true }; + } + } if (ts.isPropertyAccessExpression(e.left) && !e.left.questionDotToken) { const recv = e.left.expression; const isExports = ts.isIdentifier(recv) && recv.text === "exports"; @@ -6110,6 +6129,8 @@ export function cjsExportDiscardReason(stmt: ts.Statement): string | null { return "export members attached before a 'module.exports =' assignment (Node replaces the export object — importers never see this member; attach it to module.exports AFTER the table)"; } const left = mine.expr.left as ts.PropertyAccessExpression; + const last = cjsExportAssignmentOf(stmts[lastTable]!); + if (last?.kind === "table" && last.rebindsExports === true) return null; return ts.isIdentifier(left.expression) && left.expression.text === "exports" ? "'exports.f =' after a 'module.exports =' assignment ('exports' still references the replaced object — Node discards this member; write module.exports.f instead)" : null; diff --git a/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/6430-npm-constructor-function-class/main.ts.json b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/6430-npm-constructor-function-class/main.ts.json new file mode 100644 index 000000000..76bd8e813 --- /dev/null +++ b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/6430-npm-constructor-function-class/main.ts.json @@ -0,0 +1,6 @@ +{ + "order": [ + "/tests/corpus/6430-npm-constructor-function-class/main.ts" + ], + "diags": [] +} 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 new file mode 100644 index 000000000..8fc4a2acc --- /dev/null +++ b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/6431-cjs-exports-rebinding/main.cjs.json @@ -0,0 +1,8 @@ +{ + "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/6430-npm-constructor-function-class/main.ts b/tests/corpus/6430-npm-constructor-function-class/main.ts new file mode 100644 index 000000000..6098c1e19 --- /dev/null +++ b/tests/corpus/6430-npm-constructor-function-class/main.ts @@ -0,0 +1,19 @@ +// Constructing a CommonJS constructor-function "class" imported by a named +// member alias (`exports = module.exports = Client; exports.StatsD = +// Client`) through a re-exporting package index: prototype methods, a child +// client linked with util.inherits, and a callback-style close. +import { StatsD } from "metrics-client"; + +const client = new StatsD({ prefix: "app." }); +client.onSent((line: string) => console.log("event", line)); +console.log(client.increment("boot")); +console.log(client.gauge("heap", 42)); +console.log(typeof client.increment, typeof client.onSent); +const child = client.childClient({ prefix: "db." }); +console.log(child.increment("query", 3)); +console.log(child.gauge("pool", 1), child.sent.length, client.sent.length); +const bare = new StatsD(); +console.log(bare.increment("x"), bare.sent.length); +client.close((error: Error | null, count: number) => { + console.log("closed", error, count); +}); diff --git a/tests/corpus/6430-npm-constructor-function-class/node_modules/metrics-client/index.d.ts b/tests/corpus/6430-npm-constructor-function-class/node_modules/metrics-client/index.d.ts new file mode 100644 index 000000000..c53bdd8ae --- /dev/null +++ b/tests/corpus/6430-npm-constructor-function-class/node_modules/metrics-client/index.d.ts @@ -0,0 +1,14 @@ +export interface ClientOptions { + prefix?: string; + suffix?: string; +} +export declare class StatsD { + constructor(options?: ClientOptions); + prefix: string; + sent: string[]; + onSent(listener: (line: string) => void): this; + increment(stat: string, value?: number): string; + gauge(stat: string, value: number): string; + close(callback: (error: Error | null, count: number) => void): void; + childClient(options?: ClientOptions): StatsD; +} diff --git a/tests/corpus/6430-npm-constructor-function-class/node_modules/metrics-client/index.js b/tests/corpus/6430-npm-constructor-function-class/node_modules/metrics-client/index.js new file mode 100644 index 000000000..eb7384718 --- /dev/null +++ b/tests/corpus/6430-npm-constructor-function-class/node_modules/metrics-client/index.js @@ -0,0 +1 @@ +module.exports = require("./lib/client"); diff --git a/tests/corpus/6430-npm-constructor-function-class/node_modules/metrics-client/lib/client.js b/tests/corpus/6430-npm-constructor-function-class/node_modules/metrics-client/lib/client.js new file mode 100644 index 000000000..7dd5968c9 --- /dev/null +++ b/tests/corpus/6430-npm-constructor-function-class/node_modules/metrics-client/lib/client.js @@ -0,0 +1,50 @@ +"use strict"; +// A constructor-function "class" in the style of statsd clients: a function +// expression, prototype methods, a child client linked with util.inherits, +// and the whole export plus a named alias. +const util = require("util"); + +const Client = function (options) { + const opts = options || {}; + this.prefix = opts.prefix || ""; + this.suffix = opts.suffix || ""; + this.sent = []; + this.listeners = []; +}; + +Client.prototype.send = function (stat, value, type) { + const line = this.prefix + stat + this.suffix + ":" + value + "|" + type; + this.sent.push(line); + for (const listener of this.listeners) listener(line); + return line; +}; +Client.prototype.onSent = function (listener) { + this.listeners.push(listener); + return this; +}; +Client.prototype.increment = function (stat, value) { + return this.send(stat, value === undefined ? 1 : value, "c"); +}; +Client.prototype.gauge = function (stat, value) { + return this.send(stat, value, "g"); +}; +Client.prototype.close = function (callback) { + const count = this.sent.length; + setImmediate(() => callback(null, count)); +}; +Client.prototype.childClient = function (options) { + return new ChildClient(this, options); +}; + +const ChildClient = function (parent, options) { + Client.call(this, { + prefix: parent.prefix + ((options && options.prefix) || ""), + suffix: parent.suffix, + }); + this.parent = parent; +}; +util.inherits(ChildClient, Client); + +exports = module.exports = Client; +exports.StatsD = Client; +exports.ChildClient = ChildClient; diff --git a/tests/corpus/6430-npm-constructor-function-class/node_modules/metrics-client/package.json b/tests/corpus/6430-npm-constructor-function-class/node_modules/metrics-client/package.json new file mode 100644 index 000000000..af4ede9ee --- /dev/null +++ b/tests/corpus/6430-npm-constructor-function-class/node_modules/metrics-client/package.json @@ -0,0 +1 @@ +{ "name": "metrics-client", "version": "1.0.0", "main": "index.js", "types": "index.d.ts" } 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/lib.cjs b/tests/corpus/6431-cjs-exports-rebinding/lib.cjs new file mode 100644 index 000000000..52b6b8c9b --- /dev/null +++ b/tests/corpus/6431-cjs-exports-rebinding/lib.cjs @@ -0,0 +1,10 @@ +"use strict"; +function format(value) { + return "[" + value + "]"; +} +function shout(value) { + return String(value).toUpperCase(); +} +exports = module.exports = format; +exports.shout = shout; +exports.version = "1.2.3"; diff --git a/tests/corpus/6431-cjs-exports-rebinding/main.cjs b/tests/corpus/6431-cjs-exports-rebinding/main.cjs new file mode 100644 index 000000000..cfeec742b --- /dev/null +++ b/tests/corpus/6431-cjs-exports-rebinding/main.cjs @@ -0,0 +1,9 @@ +// `exports = module.exports = value` rebinds the wrapper's exports binding +// too, so members attached through `exports` afterwards are exported. +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 eea541912..1f3d9729b 100644 --- a/tests/harness/errors.test.ts +++ b/tests/harness/errors.test.ts @@ -1412,7 +1412,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; @@ -1420,7 +1420,7 @@ exports.extra = 1; {}, "before\n", 2, - "rebinding 'exports' \\(exports = module\\.exports = value\\)", + "rebinding 'exports' without module\\.exports \\(exports = value\\)", ], [ "require-computed",