Check the stores of for-in and for-of heads before converting a constructor

A `for (k in o)` or `for (k of a)` whose head is an expression stores into
it on every iteration. The strict-mode check of constructor and method
bodies now treats such a head like the target of `=`: a property store
not made through `this`, a store into a read-only global, and a store into
an undeclared name keep the constructor function, since sloppy mode drops
or completes those stores where a class body throws.

The static-objects-order fixture gains loops.js, whose methods store from
a for-in head into an undeclared name and from a for-of head into a
property of a string.

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

@@ -480,11 +480,18 @@ function strictSafe(checker: ts.TypeChecker, fn: ts.FunctionExpression): boolean
     const counts =
       (ts.isPrefixUnaryExpression(n) || ts.isPostfixUnaryExpression(n)) &&
       (n.operator === ts.SyntaxKind.PlusPlusToken || n.operator === ts.SyntaxKind.MinusMinusToken);
+    // `for (k in o)` and `for (k of a)` store into an expression head on
+    // every iteration, like `k = …`.
+    const loopHead =
+      (ts.isForInStatement(n) || ts.isForOfStatement(n)) &&
+      !ts.isVariableDeclarationList(n.initializer)
+        ? (n.initializer as ts.Expression)
+        : undefined;
     const target = assigns
       ? (n as ts.BinaryExpression).left
       : counts
         ? (n as ts.PrefixUnaryExpression).operand
-        : undefined;
+        : loopHead;
     // A store that sloppy mode drops (into a primitive, a frozen object or a
     // read-only property) throws in strict mode: only stores through `this`,
     // a fresh or method receiver object, are known to behave the same.
@@ -501,15 +508,13 @@ function strictSafe(checker: ts.TypeChecker, fn: ts.FunctionExpression): boolean
       )
         safe = false;
     }
-    if (
-      ts.isBinaryExpression(n) &&
-      n.operatorToken.kind >= ts.SyntaxKind.FirstAssignment &&
-      n.operatorToken.kind <= ts.SyntaxKind.LastAssignment &&
-      ts.isIdentifier(n.left)
-    ) {
+    // A store into an undeclared name creates a global in sloppy mode and
+    // throws a ReferenceError in strict mode.
+    const stored = assigns ? (n as ts.BinaryExpression).left : loopHead;
+    if (stored !== undefined && ts.isIdentifier(stored)) {
       let declared = false;
       try {
-        const s = checker.getSymbolAtLocation(n.left);
+        const s = checker.getSymbolAtLocation(stored);
         declared = s !== undefined && s.declarations.length > 0;
       } catch {
         declared = false;

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

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

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

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

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

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

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

@@ -0,0 +1,9 @@
+var loops = require("./loops");
+module.exports = {
+  // The methods are not called: scriptc refuses their loop heads (SC1090).
+  loopStores: function () {
+    var bag = new loops.shapes.Bag(2);
+    var cell = new loops.shapes.Cell("cell");
+    return bag.size + " " + cell.label;
+  },
+};

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

@@ -0,0 +1,20 @@
+// Methods that store from a for-in or for-of head where sloppy mode and
+// strict mode differ: into an undeclared name (sloppy mode creates a
+// global) and into a property of a string (sloppy mode drops the store).
+var shapes = {};
+shapes.Bag = function (size) {
+  this.size = size;
+};
+shapes.Bag.prototype.lastKey = function () {
+  for (bagLastKey in { first: 1, last: 2 });
+  return bagLastKey + " " + this.size;
+};
+shapes.Cell = function (label) {
+  this.label = label;
+};
+shapes.Cell.prototype.lastItem = function () {
+  var label = this.label;
+  for (label.item of [1, 2, 3]);
+  return typeof label.item;
+};
+module.exports = { shapes: shapes };

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

@@ -6,7 +6,8 @@
  * 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
+ * keeps constructors whose methods store from for-in and for-of heads where
+ * strict mode throws, 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). */
@@ -103,6 +104,12 @@ test(
     expect(rewrittenIfAny(debug, join(pkg, "coerce.js")) ?? "").not.toContain(
       "class __ns_shapes_Box",
     );
+    // loops.js's methods store from for-in and for-of heads into an
+    // undeclared name and into a property of a string, which throws in a
+    // class body.
+    const loops = rewrittenIfAny(debug, join(pkg, "loops.js")) ?? "";
+    expect(loops).not.toContain("class __ns_shapes_Bag");
+    expect(loops).not.toContain("class __ns_shapes_Cell");
     const vec = rewritten(debug, join(pkg, "vec.js"));
     expect(vec).toContain("class __ns_geo_Vec");
     expect(vec).toContain("module.exports = { geo: geo };");