feat(codemode): hoist var to function scope (#48284)

packages/codemode/interpreter-support.md

@@ -44,14 +44,17 @@ ultimate source of truth.
 
 ## Bindings and destructuring
 
-- [x] `const`, `let`, and accepted `var` declarations.
+- [x] `const`, `let`, and `var` declarations.
 - [x] Object and array destructuring in declarations, parameters, assignment expressions, and `for...of` bindings.
 - [x] Nested patterns, defaults, elisions, and rest elements.
 - [x] Assignment to identifiers, plain-object fields, non-negative integer array indexes, and writable URL
       fields.
 - [x] Direct function declarations are hoisted in program and block statement lists.
 - [x] Parameter defaults observe a temporal dead zone for later parameters.
-- [ ] JavaScript-correct function scoping, hoisting, and redeclaration for accepted `var` declarations.
+- [x] `var` is function-scoped and hoisted: names declared anywhere in a function or program body, including loop
+      heads, blocks, `switch` cases, and `try`/`catch`, read as `undefined` before their statement runs; redeclaration
+      assigns the one binding; a same-named parameter keeps its argument; closures in parameter defaults see outer
+      names rather than body `var`s.
 - [x] Predeclare `let` and `const` bindings in every lexical scope, including program/block bodies, switch bodies, and
       loop headers, so reads before initialization and self- or cross-referential initializers observe the JavaScript
       temporal dead zone.

packages/codemode/src/interpreter/runtime.ts

@@ -170,6 +170,49 @@ const collectPatternNames = (pattern: Pattern, out: Array<string> = []): Array<s
   return out
 }
 
+// `var` names declared anywhere in a function body except inside nested functions, which own theirs.
+const collectVarNames = (
+  node: Statement | ModuleDeclaration | null | undefined,
+  out: Array<string> = [],
+): Array<string> => {
+  if (!node) return out
+  switch (node.type) {
+    case "VariableDeclaration":
+      if (node.kind === "var") for (const declaration of node.declarations) collectPatternNames(declaration.id, out)
+      break
+    case "BlockStatement":
+      for (const statement of node.body) collectVarNames(statement, out)
+      break
+    case "IfStatement":
+      collectVarNames(node.consequent, out)
+      collectVarNames(node.alternate, out)
+      break
+    case "ForStatement":
+      if (node.init?.type === "VariableDeclaration") collectVarNames(node.init, out)
+      collectVarNames(node.body, out)
+      break
+    case "ForInStatement":
+    case "ForOfStatement":
+      if (node.left.type === "VariableDeclaration") collectVarNames(node.left, out)
+      collectVarNames(node.body, out)
+      break
+    case "WhileStatement":
+    case "DoWhileStatement":
+    case "LabeledStatement":
+      collectVarNames(node.body, out)
+      break
+    case "SwitchStatement":
+      for (const item of node.cases) for (const statement of item.consequent) collectVarNames(statement, out)
+      break
+    case "TryStatement":
+      collectVarNames(node.block, out)
+      collectVarNames(node.handler?.body, out)
+      collectVarNames(node.finalizer, out)
+      break
+  }
+  return out
+}
+
 const loopDeclaration = (left: VariableDeclaration | Pattern, statement: "for...of" | "for...in") => {
   if (left.type !== "VariableDeclaration") return undefined
   const declaration = left.declarations.length === 1 ? left.declarations[0] : undefined
@@ -276,6 +319,7 @@ class Frame<R> {
     return Effect.gen(function* () {
       self.predeclareLexical(program.body)
       self.hoistFunctions(program.body)
+      self.hoistVars(program.body)
       let value: unknown = undefined
       for (const [index, statement] of program.body.entries()) {
         if (index === program.body.length - 1 && statement.type === "ExpressionStatement") {
@@ -399,6 +443,18 @@ class Frame<R> {
     }
   }
 
+  // Hoisted `var` bindings start undefined, or copy a same-named parameter. Function bodies hoist
+  // into their own scope above the parameters so closures in parameter defaults keep seeing outer names.
+  private hoistVars(statements: ReadonlyArray<Statement | ModuleDeclaration>, parameters?: Map<string, Binding>): void {
+    const scope = this.scopes.current()
+    for (const statement of statements) {
+      for (const name of collectVarNames(statement)) {
+        if (scope.has(name)) continue
+        scope.set(name, { mutable: true, value: parameters?.get(name)?.value, initialized: true })
+      }
+    }
+  }
+
   private predeclareLexical(statements: ReadonlyArray<Statement | ModuleDeclaration>): void {
     for (const statement of statements) {
       if (statement.type !== "VariableDeclaration") continue
@@ -589,10 +645,12 @@ class Frame<R> {
 
       const evaluateBody = (value: unknown) =>
         Effect.gen(function* () {
-          if (declared) {
+          if (declared?.lexical) {
             self.scopes.push()
-            if (declared.lexical) self.predeclarePattern(declared.pattern, declared.mutable, left)
-            yield* self.declarePattern(declared.pattern, value, declared.mutable, left, declared.lexical)
+            self.predeclarePattern(declared.pattern, declared.mutable, left)
+            yield* self.declarePattern(declared.pattern, value, declared.mutable, left, true)
+          } else if (declared) {
+            yield* self.assignPattern(declared.pattern, value, left)
           } else if (assignment) {
             yield* self.assignPattern(assignment, value, left)
           }
@@ -600,7 +658,7 @@ class Frame<R> {
         }).pipe(
           Effect.ensuring(
             Effect.sync(() => {
-              if (declared) self.scopes.pop()
+              if (declared?.lexical) self.scopes.pop()
             }),
           ),
         )
@@ -834,10 +892,12 @@ class Frame<R> {
 
       for (const key of keys) {
         const result = yield* Effect.gen(function* () {
-          if (declared) {
+          if (declared?.lexical) {
             self.scopes.push()
-            if (declared.lexical) self.predeclarePattern(declared.pattern, declared.mutable, left)
-            yield* self.declarePattern(declared.pattern, key, declared.mutable, left, declared.lexical)
+            self.predeclarePattern(declared.pattern, declared.mutable, left)
+            yield* self.declarePattern(declared.pattern, key, declared.mutable, left, true)
+          } else if (declared) {
+            yield* self.assignPattern(declared.pattern, key, left)
           } else if (assignmentName) {
             self.scopes.set(assignmentName, key, left)
           }
@@ -845,7 +905,7 @@ class Frame<R> {
         }).pipe(
           Effect.ensuring(
             Effect.sync(() => {
-              if (declared) self.scopes.pop()
+              if (declared?.lexical) self.scopes.pop()
             }),
           ),
         )
@@ -952,8 +1012,13 @@ class Frame<R> {
         }
 
         const init = declaration.init
+        // `var x` alone is a no-op: the binding was hoisted on function entry.
+        if (kind === "var") {
+          if (init) yield* self.assignPattern(declaration.id, yield* self.evaluateExpression(init), declaration)
+          continue
+        }
         const value = init ? yield* self.evaluateExpression(init) : undefined
-        yield* self.declarePattern(declaration.id, value, kind !== "const", declaration, kind !== "var")
+        yield* self.declarePattern(declaration.id, value, kind !== "const", declaration, true)
       }
     })
   }
@@ -1574,6 +1639,8 @@ class Frame<R> {
       }
 
       if (fn.body.type === "BlockStatement") {
+        invocation.scopes.push()
+        invocation.hoistVars(fn.body.body, paramScope)
         const result = yield* invocation.evaluateStatement(fn.body)
         return result.kind === "return" ? result.value : undefined
       }

packages/codemode/test/var-hoisting-test262.test.ts

@@ -0,0 +1,235 @@
+/**
+ * Portions adapted from Test262 at revision 250f204f23a9249ff204be2baec29600faae7b75:
+ * - test/language/statements/variable/S12.2_A1.js
+ * - test/language/statements/variable/S12.2_A3.js
+ * - test/language/statements/variable/S12.2_A6_T1.js
+ * - test/language/statements/variable/S12.2_A7.js
+ * - test/language/statements/variable/S12.2_A10.js
+ * - test/language/statements/variable/S12.2_A12.js
+ * - test/language/block-scope/shadowing/hoisting-var-declarations-out-of-blocks.js
+ * - test/language/block-scope/shadowing/catch-parameter-shadowing-var-variable.js
+ * - test/language/statements/for/head-var-bound-names-in-stmt.js
+ * - test/language/statements/function/scope-paramsbody-var-open.js
+ * - test/language/statements/function/scope-paramsbody-var-close.js
+ *
+ * Copyright 2009 the Sputnik authors. All rights reserved.
+ * Copyright (C) 2011, 2016 the V8 project authors. All rights reserved.
+ * Test262 portions are governed by the BSD license in LICENSE.test262.
+ *
+ * Files that observe `var` through `eval`, `this`, `delete`, or the global object (S12.2_A2, A5, A9,
+ * A11, `scope-*-none.js`, `scope-param-elem-*.js`) have no analogue here and are not ported.
+ */
+import { describe, expect, test } from "bun:test"
+import { Effect } from "effect"
+import { CodeMode } from "../src/index.js"
+
+const value = async (code: string) => {
+  const result = await Effect.runPromise(CodeMode.execute({ code, tools: {} }))
+  if (!result.ok) throw new Error(`expected success, got ${result.error.kind}: ${result.error.message}`)
+  return result.value
+}
+
+describe("var hoisting Test262 parity", () => {
+  test("test/language/statements/variable/S12.2_A1.js: use before declaration reads undefined", async () => {
+    expect(
+      await value(`
+        __x = __x
+        __y = __x ? "good fellow" : "liar"
+        __z = __z === __x ? 1 : 0
+        let unknown
+        try { __something__undefined = __something__undefined } catch (error) { unknown = error.name }
+        const before = [__y, __z, unknown]
+        var __x, __y = true, __z = __y ? "smeagol" : "golum"
+        return [...before, __y, __z]
+      `),
+    ).toEqual(["liar", 1, "ReferenceError", true, "smeagol"])
+  })
+
+  test("test/language/statements/variable/S12.2_A3.js: nested functions redeclare or assign", async () => {
+    expect(
+      await value(`
+        var __var = "OUT"
+        const inner = (function () {
+          var __var = "IN"
+          ;(function () { __var = "INNER_SPACE" })()
+          ;(function () { var __var = "INNER_SUN" })()
+          return __var
+        })()
+        const after = __var
+        const assigned = (function () {
+          __var = "IN"
+          ;(function () { __var = "INNERED" })()
+          ;(function () { var __var = "INNAGER" })()
+          return __var
+        })()
+        return [inner, after, assigned, __var]
+      `),
+    ).toEqual(["INNER_SPACE", "OUT", "INNERED", "INNERED"])
+  })
+
+  test("test/language/statements/variable/S12.2_A6_T1.js: var inside try and catch is hoisted", async () => {
+    expect(
+      await value(`
+        intry__var = intry__var
+        incatch__var = incatch__var
+        try { var intry__var } catch (e) { var incatch__var }
+        return [typeof intry__var, typeof incatch__var]
+      `),
+    ).toEqual(["undefined", "undefined"])
+  })
+
+  test("test/language/statements/variable/S12.2_A7.js: var after break inside for is hoisted", async () => {
+    expect(
+      await value(`
+        infor_var = infor_var
+        for (;;) { break; var infor_var }
+        return typeof infor_var
+      `),
+    ).toBe("undefined")
+  })
+
+  test("test/language/statements/variable/S12.2_A10.js: var in for head is hoisted", async () => {
+    expect(
+      await value(`
+        __ind = __ind
+        for (var __ind; ; ) { break }
+        return typeof __ind
+      `),
+    ).toBe("undefined")
+  })
+
+  test("test/language/statements/variable/S12.2_A12.js: var in do-while body is hoisted", async () => {
+    expect(
+      await value(`
+        x = x
+        do var x; while (false)
+        return typeof x
+      `),
+    ).toBe("undefined")
+  })
+
+  test("test/language/block-scope/shadowing/hoisting-var-declarations-out-of-blocks.js", async () => {
+    expect(
+      await value(`
+        function fn() {
+          { var x = 1; var y }
+          return [x, typeof y]
+        }
+        return fn()
+      `),
+    ).toEqual([1, "undefined"])
+  })
+
+  test("test/language/block-scope/shadowing/catch-parameter-shadowing-var-variable.js", async () => {
+    expect(
+      await value(`
+        function fn() {
+          var a = 1
+          let caught
+          try { throw "stuff3" } catch (a) { caught = a }
+          return [caught, a]
+        }
+        return fn()
+      `),
+    ).toEqual(["stuff3", 1])
+  })
+
+  test("test/language/statements/for/head-var-bound-names-in-stmt.js: redeclaring the head var in the body", async () => {
+    expect(
+      await value(`
+        var iterCount = 0
+        var first = true
+        for (var x; first; first = false) {
+          var x
+          iterCount += 1
+        }
+        return iterCount
+      `),
+    ).toBe(1)
+  })
+
+  test("test/language/statements/function/scope-paramsbody-var-open.js: parameter defaults see the outer var", async () => {
+    expect(
+      await value(`
+        var x = "outside"
+        var probeParams, probeBody
+        function f(_ = probeParams = function () { return x }) {
+          var x = "inside"
+          probeBody = function () { return x }
+        }
+        f()
+        return [probeParams(), probeBody()]
+      `),
+    ).toEqual(["outside", "inside"])
+  })
+
+  test("test/language/statements/function/scope-paramsbody-var-close.js: body var does not leak out", async () => {
+    expect(
+      await value(`
+        var probe
+        function f(_ = null) {
+          var x = "inside"
+          probe = function () { return x }
+        }
+        f()
+        var x = "outside"
+        return [probe(), x]
+      `),
+    ).toEqual(["inside", "outside"])
+  })
+})
+
+describe("var semantics beyond Test262", () => {
+  test("redeclaration and block-level var assign the one function-scoped binding", async () => {
+    expect(await value(`var a = 1; var a = 2; { var a = 3 } return a`)).toBe(3)
+    expect(await value(`var q = 1; { let q = 2 } return q`)).toBe(1)
+  })
+
+  test("var loop counters are shared by closures, let counters are not", async () => {
+    expect(
+      await value(`
+        const byVar = []
+        for (var i = 0; i < 3; i++) byVar.push(() => i)
+        const byLet = []
+        for (let j = 0; j < 3; j++) byLet.push(() => j)
+        return [byVar.map((f) => f()), byLet.map((f) => f())]
+      `),
+    ).toEqual([
+      [3, 3, 3],
+      [0, 1, 2],
+    ])
+  })
+
+  test("for...in and for...of var heads survive the loop", async () => {
+    expect(await value(`for (var k in { a: 1 }) {} for (var [p, q] of [[1, 2]]) {} return [k, p, q]`)).toEqual([
+      "a",
+      1,
+      2,
+    ])
+  })
+
+  test("var and function declarations of the same name share a binding", async () => {
+    expect(await value(`var fn = 1; function fn() {} return typeof fn`)).toBe("number")
+    expect(await value(`function fn() {} var fn; return typeof fn`)).toBe("function")
+    expect(await value(`function h() { var fn = 1; function fn() {} return typeof fn } return h()`)).toBe("number")
+  })
+
+  test("a var named after a parameter keeps the argument until assigned", async () => {
+    expect(await value(`function f(a) { var a; return a } return f(7)`)).toBe(7)
+    expect(await value(`function f(a) { var a = 2; return a } return f(7)`)).toBe(2)
+  })
+
+  test("var does not hoist across function boundaries", async () => {
+    expect(await value(`return [typeof b, (() => { var b = 1; return b })()]; var b`)).toEqual(["undefined", 1])
+    expect(
+      await value(
+        `function outer() { var o = 1; function inner() { var o = 2; return o } return [inner(), o] } return outer()`,
+      ),
+    ).toEqual([2, 1])
+  })
+
+  test("switch cases, labels, and generators hoist var", async () => {
+    expect(await value(`switch (1) { case 1: var s = 9 } label: { var lb = 1 } return [s, lb]`)).toEqual([9, 1])
+    expect(await value(`function* gen() { var t = 1; yield t; var t = 2; yield t } return [...gen()]`)).toEqual([1, 2])
+  })
+})