Address review feedback

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

@@ -864,10 +864,46 @@ export function planStaticObjectRewrites(
     if (recv.size !== 1 || !constantAtoms.has([...recv][0]!)) return false;
     return pureReceiver(target.expression, target);
   };
+  /** Whether evaluating `n` itself may run code of the program: converting
+   * an operand to a primitive or a property key (an arithmetic, relational or
+   * loose-equality operator, a compound assignment, `++`, a template
+   * substitution, a computed key) calls its valueOf, toString or
+   * Symbol.toPrimitive; `in`, `instanceof` and `delete` run traps and
+   * Symbol.hasInstance; spreading and destructuring run iterators and
+   * getters; a class runs its computed keys, static initializers and blocks. */
+  const runsImplicitly = (n: Node): boolean => {
+    if (ts.isBinaryExpression(n)) {
+      const op = n.operatorToken.kind;
+      if (op === ts.SyntaxKind.EqualsToken)
+        return ts.isObjectLiteralExpression(n.left) || ts.isArrayLiteralExpression(n.left);
+      return (
+        op !== ts.SyntaxKind.EqualsEqualsEqualsToken &&
+        op !== ts.SyntaxKind.ExclamationEqualsEqualsToken &&
+        op !== ts.SyntaxKind.AmpersandAmpersandToken &&
+        op !== ts.SyntaxKind.BarBarToken &&
+        op !== ts.SyntaxKind.QuestionQuestionToken &&
+        op !== ts.SyntaxKind.CommaToken
+      );
+    }
+    if (ts.isPrefixUnaryExpression(n)) return n.operator !== ts.SyntaxKind.ExclamationToken;
+    return (
+      ts.isPostfixUnaryExpression(n) ||
+      ts.isDeleteExpression(n) ||
+      ts.isAwaitExpression(n) ||
+      ts.isYieldExpression(n) ||
+      ts.isTemplateExpression(n) ||
+      ts.isSpreadElement(n) ||
+      ts.isSpreadAssignment(n) ||
+      ts.isComputedPropertyName(n) ||
+      ts.isObjectBindingPattern(n) ||
+      ts.isArrayBindingPattern(n) ||
+      ts.isClassExpression(n)
+    );
+  };
   /** Whether a statement only defines things: function declarations, and
    * declarations and stores whose evaluation runs no code outside function
    * bodies (no call, no read that could run a getter, no store that could run
-   * a setter). */
+   * a setter, no implicit conversion or other implicit call). */
   const definitionOnly = (stmt: ts.Statement): boolean => {
     if (ts.isFunctionDeclaration(stmt)) return true;
     if (!ts.isExpressionStatement(stmt) && !ts.isVariableStatement(stmt)) return false;
@@ -875,7 +911,12 @@ export function planStaticObjectRewrites(
     const visit = (n: Node): void => {
       if (runs) return;
       if (ts.isFunctionLike(n)) return;
-      if (ts.isCallExpression(n) || ts.isNewExpression(n) || ts.isTaggedTemplateExpression(n))
+      if (
+        ts.isCallExpression(n) ||
+        ts.isNewExpression(n) ||
+        ts.isTaggedTemplateExpression(n) ||
+        runsImplicitly(n)
+      )
         runs = true;
       if (ts.isPropertyAccessExpression(n) || ts.isElementAccessExpression(n)) {
         const p = n.parent;

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

@@ -4,6 +4,7 @@ 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");
+const coerce = require("static-objects-order/coerce-user.js");
 
 console.log("start");
 console.log(receivers.ns.run("add"), receivers.ns.run("sub"), receivers.ns.reads());
@@ -12,3 +13,4 @@ console.log(receivers.ns.early());
 console.log(lazy.greet("a"), lazy.greet("b"));
 console.log(vec.len2(3, 4));
 console.log(early.early());
+console.log(coerce.area(3));

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

@@ -4,6 +4,7 @@ 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";
+import { area } from "static-objects-order/coerce-user.js";
 
 console.log("start");
 console.log(ns.run("add"), ns.run("sub"), ns.reads());
@@ -12,3 +13,4 @@ console.log(ns.early());
 console.log(greet("a"), greet("b"));
 console.log(len2(3, 4));
 console.log(early());
+console.log(area(3));

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

@@ -0,0 +1 @@
+export declare function area(w: number): string;

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

@@ -0,0 +1,6 @@
+var coerce = require("./coerce");
+module.exports = {
+  area: function (w) {
+    return coerce.seen + " " + new coerce.shapes.Box(w).area();
+  },
+};

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

@@ -0,0 +1,17 @@
+// A conversion between the constructor function and its method runs code
+// (Probe's toString) that reads the prototype before the method is stored.
+var shapes = {};
+class Probe {
+  toString() {
+    return shapes.Box.prototype.area === undefined ? "no area yet" : "area";
+  }
+}
+var probe = new Probe();
+shapes.Box = function (w) {
+  this.w = w;
+};
+var seen = "" + probe;
+shapes.Box.prototype.area = function () {
+  return this.w * this.w;
+};
+module.exports = { shapes: shapes, seen: seen };

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

@@ -97,6 +97,12 @@ test(
     // 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_");
+    // coerce.js converts an object with a toString method between its
+    // constructor and the method: the conversion runs before the method exists.
+    expect(debug).toContain("code runs between the constructor and its methods");
+    expect(rewrittenIfAny(debug, join(pkg, "coerce.js")) ?? "").not.toContain(
+      "class __ns_shapes_Box",
+    );
     const vec = rewritten(debug, join(pkg, "vec.js"));
     expect(vec).toContain("class __ns_geo_Vec");
     expect(vec).toContain("module.exports = { geo: geo };");