diff --git a/packages/compiler/src/frontend/shape-inference/static-objects.ts b/packages/compiler/src/frontend/shape-inference/static-objects.ts index 25c0206c9..2b41c2f9d 100644 --- a/packages/compiler/src/frontend/shape-inference/static-objects.ts +++ b/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; diff --git a/tests/fixtures/npm/cases/static-objects-order/js-main.cjs b/tests/fixtures/npm/cases/static-objects-order/js-main.cjs index 56da826ad..070aed17b 100644 --- a/tests/fixtures/npm/cases/static-objects-order/js-main.cjs +++ b/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()); diff --git a/tests/fixtures/npm/cases/static-objects-order/main.ts b/tests/fixtures/npm/cases/static-objects-order/main.ts index fb0e64e76..ce25ea677 100644 --- a/tests/fixtures/npm/cases/static-objects-order/main.ts +++ b/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()); diff --git a/tests/fixtures/npm/cases/static-objects-order/node_modules/static-objects-order/loops-user.d.ts b/tests/fixtures/npm/cases/static-objects-order/node_modules/static-objects-order/loops-user.d.ts new file mode 100644 index 000000000..23e9cc62d --- /dev/null +++ b/tests/fixtures/npm/cases/static-objects-order/node_modules/static-objects-order/loops-user.d.ts @@ -0,0 +1 @@ +export declare function loopStores(): string; diff --git a/tests/fixtures/npm/cases/static-objects-order/node_modules/static-objects-order/loops-user.js b/tests/fixtures/npm/cases/static-objects-order/node_modules/static-objects-order/loops-user.js new file mode 100644 index 000000000..15c62a0bb --- /dev/null +++ b/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; + }, +}; diff --git a/tests/fixtures/npm/cases/static-objects-order/node_modules/static-objects-order/loops.js b/tests/fixtures/npm/cases/static-objects-order/node_modules/static-objects-order/loops.js new file mode 100644 index 000000000..ef5c2565b --- /dev/null +++ b/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 }; diff --git a/tests/harness/static-objects-order.test.ts b/tests/harness/static-objects-order.test.ts index 18fc58830..208048f18 100644 --- a/tests/harness/static-objects-order.test.ts +++ b/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 };");