Read member aliases only while the binding holds the exported value

A member export resolves to its binding only when the binding is declared
before the write and never reassigned, so a consumer reads the value the
member stored. A module that re-exports another module's exports through
module.exports = require("./x") now counts its own writes after the
re-exported module's: a member it adds resolves, and a member both modules
write stays refused. The exports rebinding refusal and its test now cover
the form that stays refused, exports = value without module.exports.

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

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.<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/test/ts7/baselines/order-parity/tests/corpus/6431-cjs-exports-rebinding/main.cjs.json

@@ -1,6 +1,7 @@
 {
  "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/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/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"));

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

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