Address review feedback

packages/compiler/src/frontend/shape-inference/static-objects.ts

@@ -149,7 +149,8 @@ function literalMemberFunction(fn: Node): { stmt: ts.Statement; path: string } |
 /** The text of a primitive literal an object literal itself gives `key`
  * (`{ K: 1 }`, `{ "k": "v" }`, `-1`, `true`, `null`), or null. */
 function ownLiteralMember(literal: Node, key: string): string | null {
-  if (!ts.isObjectLiteralExpression(literal)) return null;
+  // `__proto__: v` in a literal sets the prototype and makes no property.
+  if (!ts.isObjectLiteralExpression(literal) || key === "__proto__") return null;
   let found: string | null = null;
   for (const m of literal.properties.nodes) {
     const name = (m as { name?: Node }).name;
@@ -695,35 +696,43 @@ export function planStaticObjectRewrites(
    * one of which it initializes, optionally followed by member reads. A
    * require inside a function, a condition or another expression runs only
    * when that does. */
-  const requiresWhenRun = (sf: ts.SourceFile, st: ts.Statement, target: ts.SourceFile): boolean => {
-    const values: ts.Expression[] = [];
-    if (ts.isVariableStatement(st)) {
-      for (const d of st.declarationList.declarations.nodes)
-        if (d.initializer !== undefined) values.push(d.initializer);
-    } else if (ts.isExpressionStatement(st)) {
-      const e = st.expression;
-      values.push(
-        ts.isBinaryExpression(e) && e.operatorToken.kind === ts.SyntaxKind.EqualsToken
-          ? e.right
-          : e,
+  /** The values a top-level statement computes: its declarations'
+   * initializers, or its expression (an assignment's right side). */
+  const statementValues = (st: ts.Statement): ts.Expression[] | null => {
+    if (ts.isVariableStatement(st))
+      return st.declarationList.declarations.nodes.flatMap((d) =>
+        d.initializer !== undefined ? [d.initializer] : [],
       );
-    }
-    return values.some((v) => {
-      let e = v;
-      for (;;) {
-        if (ts.isParenthesizedExpression(e)) e = e.expression;
-        else if (ts.isPropertyAccessExpression(e) && e.questionDotToken === undefined)
-          e = e.expression;
-        else break;
-      }
-      return (
-        ts.isCallExpression(e) &&
-        ts.isIdentifier(e.expression) &&
-        e.expression.text === "require" &&
-        requiredFiles(sf, e).includes(target)
-      );
-    });
+    if (!ts.isExpressionStatement(st)) return null;
+    const e = st.expression;
+    return [
+      ts.isBinaryExpression(e) &&
+      e.operatorToken.kind === ts.SyntaxKind.EqualsToken &&
+      ts.isIdentifier(e.left)
+        ? e.right
+        : e,
+    ];
   };
+  /** The require call `v` is, followed by member reads, or null. */
+  const requireValue = (v: ts.Expression): ts.CallExpression | null => {
+    let e = v;
+    for (;;) {
+      if (ts.isParenthesizedExpression(e)) e = e.expression;
+      else if (ts.isPropertyAccessExpression(e) && e.questionDotToken === undefined)
+        e = e.expression;
+      else break;
+    }
+    return ts.isCallExpression(e) &&
+      ts.isIdentifier(e.expression) &&
+      e.expression.text === "require"
+      ? e
+      : null;
+  };
+  const requiresWhenRun = (sf: ts.SourceFile, st: ts.Statement, target: ts.SourceFile): boolean =>
+    (statementValues(st) ?? []).some((v) => {
+      const call = requireValue(v);
+      return call !== null && requiredFiles(sf, call).includes(target);
+    });
   const moduleBindings = new Map<string, string>();
   const exportAdds = new Map<ts.SourceFile, Set<string>>();
   const crossFileReference = (
@@ -756,6 +765,14 @@ export function planStaticObjectRewrites(
       return false;
     })();
     if (!inFunction && stmts.indexOf(top) <= at) return null;
+    // A function may run before the binding below is initialized if the
+    // statement that loads the module, or one before it, runs other code
+    // (`var m = require("lib"), x = f();`): then its reads stay as written.
+    if (inFunction) {
+      const values = statementValues(stmts[at]!);
+      const onlyLoads = values !== null && values.every((v) => requireValue(v) !== null);
+      if (!onlyLoads || !stmts.slice(0, at).every(definitionOnly)) return null;
+    }
     const key = `${sf.fileName}\0${defining.fileName}`;
     let binding = moduleBindings.get(key);
     if (binding === undefined) {

tests/fixtures/npm/cases/static-objects-order/js-main.cjs

@@ -3,8 +3,11 @@
 const receivers = require("static-objects-order/receivers.js");
 const lazy = require("static-objects-order/lazy-user.js");
 const vec = require("static-objects-order/vec-user.js");
+const early = require("static-objects-order/early-user.js");
 
 console.log("start");
 console.log(receivers.ns.run("add"), receivers.ns.run("sub"), receivers.ns.reads());
+console.log(receivers.ns.proto());
 console.log(lazy.greet("a"), lazy.greet("b"));
 console.log(vec.len2(3, 4));
+console.log(early.early());

tests/fixtures/npm/cases/static-objects-order/main.ts

@@ -3,8 +3,11 @@
 import { ns } from "static-objects-order/receivers.js";
 import { greet } from "static-objects-order/lazy-user.js";
 import { len2 } from "static-objects-order/vec-user.js";
+import { early } from "static-objects-order/early-user.js";
 
 console.log("start");
 console.log(ns.run("add"), ns.run("sub"), ns.reads());
+console.log(ns.proto());
 console.log(greet("a"), greet("b"));
 console.log(len2(3, 4));
+console.log(early());

tests/fixtures/npm/cases/static-objects-order/node_modules/static-objects-order/early-user.d.ts

@@ -0,0 +1 @@
+export declare function early(): string;

tests/fixtures/npm/cases/static-objects-order/node_modules/static-objects-order/early-user.js

@@ -0,0 +1,9 @@
+// A function called in the statement that requires lazy.js, after m is
+// assigned and before the statement ends.
+var m = require("./lazy"),
+  early = (function (n) {
+    return m.lz.hello(n);
+  })("early");
+exports.early = function () {
+  return early + " " + m.lz.hello("late");
+};

tests/fixtures/npm/cases/static-objects-order/node_modules/static-objects-order/lazy-user.js

@@ -3,6 +3,7 @@
 if (process.env["STATIC_OBJECTS_ORDER_EARLY"] === "1") require("./lazy");
 console.log("lazy-user.js before its require");
 var m = require("./lazy");
+var hi = m.lz.hello("top");
 exports.greet = function (n) {
-  return m.lz.hello(n);
+  return hi + " " + n;
 };

tests/fixtures/npm/cases/static-objects-order/node_modules/static-objects-order/receivers.js

@@ -22,4 +22,9 @@ ns.run = function (op) {
 ns.reads = function () {
   return reads;
 };
+// `__proto__: null` in a literal sets the prototype: no own property.
+ns.Proto = { __proto__: null, A: 1 };
+ns.proto = function () {
+  return String(ns.Proto.__proto__) + " " + ns.Proto.A;
+};
 module.exports = { ns: ns };

tests/harness/static-objects-order.test.ts

@@ -2,9 +2,10 @@
  * rewrites keep what the program evaluates and when its modules load. With
  * the shape overlay on, the npm case static-objects-order (through its
  * JavaScript consumer, js-main.cjs) keeps receivers with effects as
- * written, binds a module where the program loads it, never adds a hoisted
- * class to an exports literal that runs before the class's declaration,
- * and prints what Node prints. A module in a require cycle through another
+ * written, binds a module where the program loads it and only for reads
+ * that run after the binding, never adds a hoisted class to an exports
+ * literal that runs before the class's declaration, does not fold a
+ * literal's `__proto__`, and prints what Node prints. A module in a require cycle through another
  * module is not read through its exports from the cycle. The rewritten
  * files are read from the overlay's debug output
  * (SCRIPTC_DEBUG_SHAPE_OVERLAY). */
@@ -50,12 +51,17 @@ async function compileWithOverlay(entry: string, name: string) {
   }
 }
 
-/** The text the static-object rewrites give `file` in the debug output. */
-function rewritten(debug: string, file: string): string {
+/** The text the rewrites give `file` in the debug output, if any. */
+function rewrittenIfAny(debug: string, file: string): string | undefined {
   const parts = debug.split(/^\[(?:static-objects|shape-overlay)\] /m);
-  const part = parts.find((p) => p.startsWith(`${file}\n`));
-  expect(part, `no rewrite of ${file}`).toBeDefined();
-  return part!.slice(file.length + 1);
+  return parts.find((p) => p.startsWith(`${file}\n`))?.slice(file.length + 1);
+}
+
+/** The text the rewrites give `file` in the debug output. */
+function rewritten(debug: string, file: string): string {
+  const text = rewrittenIfAny(debug, file);
+  expect(text, `no rewrite of ${file}`).toBeDefined();
+  return text!;
 }
 
 test(
@@ -82,6 +88,9 @@ test(
     expect(rewritten(debug, join(pkg, "lazy-user.js"))).toMatch(
       /var m = require\("\.\/lazy"\);\nconst __ns_mod_\w+ = require\("\.\/lazy"\);/,
     );
+    // early-user.js calls a function in the statement that loads lazy.js:
+    // a binding after that statement is not initialized when it runs.
+    expect(rewrittenIfAny(debug, join(pkg, "early-user.js")) ?? "").not.toContain("__ns_mod_");
     const vec = rewritten(debug, join(pkg, "vec.js"));
     expect(vec).toContain("class __ns_geo_Vec");
     expect(vec).toContain("module.exports = { geo: geo };");