Construct CommonJS constructor functions imported through a member alias (#1453)

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.<name> = <value>` or `exports.<name> = <value>`, 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);
+}

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.

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.<name> = … 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);

packages/compiler/src/frontend/program.ts

@@ -5967,10 +5967,15 @@ export function isModuleExportsAccess(expr: ts.Expression): expr is ts.PropertyA
  *   - member: `exports.f = <expr>` / `module.exports.f = <expr>`.
  * 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;

packages/compiler/test/ts7/baselines/order-parity/tests/corpus/6430-npm-constructor-function-class/main.ts.json

@@ -0,0 +1,6 @@
+{
+ "order": [
+  "<repo>/tests/corpus/6430-npm-constructor-function-class/main.ts"
+ ],
+ "diags": []
+}

packages/compiler/test/ts7/baselines/order-parity/tests/corpus/6431-cjs-exports-rebinding/main.cjs.json

@@ -0,0 +1,8 @@
+{
+ "order": [
+  "<repo>/tests/corpus/6431-cjs-exports-rebinding/lib.cjs",
+  "<repo>/tests/corpus/6431-cjs-exports-rebinding/extended.cjs",
+  "<repo>/tests/corpus/6431-cjs-exports-rebinding/main.cjs"
+ ],
+ "diags": []
+}

packages/compiler/test/ts7/baselines/order-parity/tests/diagnostics/cjs-member-alias-writes/main.ts.json

@@ -0,0 +1,6 @@
+{
+ "order": [
+  "<repo>/tests/diagnostics/cjs-member-alias-writes/main.ts"
+ ],
+ "diags": []
+}

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);
+});

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;
+}

tests/corpus/6430-npm-constructor-function-class/node_modules/metrics-client/index.js

@@ -0,0 +1 @@
+module.exports = require("./lib/client");

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;

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" }

tests/corpus/6431-cjs-exports-rebinding/extended.cjs

@@ -0,0 +1,3 @@
+"use strict";
+module.exports = require("./lib.cjs");
+module.exports.extra = "added";

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";

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"));

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);

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;

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";

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;

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"
+}

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 | 

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",