Просмотр исходного кода

fix(codemode): align string, array, and Date behavior (#37775)

Aiden Cline 3 недель назад
Родитель
Сommit
391cfbcfe7

+ 6 - 7
packages/codemode/interpreter-support.md

@@ -91,8 +91,8 @@ ultimate source of truth.
       like JS.
 - [x] Tool references and detached `Promise` statics are rejected as callbacks with a hint to wrap them in an
       arrow function.
-- [ ] Stop automatically awaiting promise-returning string replacers; match JavaScript's synchronous callback-result
-      coercion.
+- [x] Promise-returning string replacers are coerced synchronously to `"[object Promise]"`, like JavaScript; they are
+      not automatically awaited.
 - [x] The optional `thisArg` of iteration methods is accepted and ignored: CodeMode functions have no `this`, so
       ignoring it matches JS arrow-function semantics exactly.
 - [ ] `this` in non-arrow CodeMode functions and callbacks.
@@ -199,8 +199,8 @@ ultimate source of truth.
 - [ ] `Array.prototype.toSpliced`.
 - [ ] Canonical array/string index parsing: a key such as `"01"` must remain an ordinary property key rather than
       aliasing index `1`.
-- [ ] `Array.prototype.sort` and `toSorted` must preserve trailing holes; they currently turn holes into own
-      `undefined` elements.
+- [x] `Array.prototype.sort` preserves trailing holes, while `toSorted` densifies holes into `undefined` elements,
+      like JavaScript.
 
 ## Strings
 
@@ -267,10 +267,9 @@ ultimate source of truth.
 - [x] Date values serialize to ISO strings; invalid dates serialize to `null`.
 - [ ] Date setters.
 - [ ] `Date.prototype.toUTCString` and its `toGMTString` alias.
-- [ ] Native one-argument Date coercion; unsupported boolean/object inputs currently become invalid dates instead of
-      being coerced.
+- [x] Native one-argument Date coercion for supported values, including booleans, null, arrays, and plain objects.
 - [ ] Native Date loose-equality and default primitive-coercion semantics.
-- [ ] Native `RangeError` branding for invalid `toISOString()` calls.
+- [x] Native `RangeError` branding for invalid `toISOString()` calls.
 
 ## Regular expressions
 

+ 16 - 9
packages/codemode/src/interpreter/methods.ts

@@ -428,16 +428,14 @@ const invokeStringReplacer = <R>(
     let end = 0
     for (const match of matches) {
       const replacement = yield* apply(match.args)
-      const resolved =
-        args[1] instanceof CodeModeFunction && args[1].async && replacement instanceof CodeModePromise
-          ? yield* runner.settlePromise(replacement)
-          : replacement
       // Error values are branded plain objects; boundedData would strip the brand before coercion.
       output.push(
         value.slice(end, match.offset),
-        errorBrandName(resolved)
-          ? coerceToString(resolved)
-          : coerceToString(boundedData(resolved, `String.${name} replacer result`)),
+        replacement instanceof CodeModePromise
+          ? "[object Promise]"
+          : errorBrandName(replacement)
+            ? coerceToString(replacement)
+            : coerceToString(boundedData(replacement, `String.${name} replacer result`)),
       )
       end = match.offset + match.match.length
     }
@@ -664,11 +662,20 @@ const invokeArrayMethod = <R>(
       return Effect.succeed(target.flat(optNumber(args[0], "depth") ?? 1))
     case "reverse":
       return Effect.succeed(target.reverse())
-    case "sort":
+    case "sort": {
+      const length = target.length
+      const holeCount = Array.from({ length }, (_, index) => Object.hasOwn(target, index)).filter((own) => !own).length
+      const itemCount = length - holeCount
       return Effect.map(sortArray(runner, target, args[0], "Array.sort", node), (sorted) => {
-        target.splice(0, target.length, ...sorted)
+        sorted.slice(0, itemCount).forEach((item, index) => {
+          target[index] = item
+        })
+        Array.from({ length: holeCount }, (_, index) => itemCount + index).forEach((index) => {
+          Reflect.deleteProperty(target, index)
+        })
         return target
       })
+    }
     case "toSorted":
       return sortArray(runner, target, args[0], "Array.toSorted", node)
     case "toReversed":

+ 28 - 8
packages/codemode/src/interpreter/runtime.ts

@@ -1095,7 +1095,7 @@ export class Interpreter<R> {
         const args = yield* self.evaluateCallArguments(argNodes)
         switch (name) {
           case "Date":
-            return self.constructDate(args)
+            return yield* self.constructDate(args, node)
           case "RegExp":
             return self.constructRegExp(args, node)
           case "Map":
@@ -1133,17 +1133,37 @@ export class Interpreter<R> {
     )
   }
 
-  private constructDate(args: Array<unknown>): CodeModeDate {
-    if (args.length === 0) return new CodeModeDate(Date.now())
+  private constructDate(args: Array<unknown>, node: AstNode): Effect.Effect<CodeModeDate, unknown, R> {
+    if (args.length === 0) return Effect.succeed(new CodeModeDate(Date.now()))
     if (args.length === 1) {
       const arg = args[0]
-      if (arg instanceof CodeModeDate) return new CodeModeDate(arg.time)
-      if (typeof arg === "number") return new CodeModeDate(new Date(arg).getTime())
-      if (typeof arg === "string") return new CodeModeDate(Date.parse(arg))
-      return new CodeModeDate(Number.NaN)
+      if (arg instanceof CodeModeDate) return Effect.succeed(new CodeModeDate(arg.time))
+      return Effect.map(this.toDatePrimitive(arg, node), (value) =>
+        typeof value === "string"
+          ? new CodeModeDate(Date.parse(value))
+          : new CodeModeDate(new Date(coerceToNumber(value)).getTime()),
+      )
     }
     const parts = args.map((arg) => coerceToNumber(arg))
-    return new CodeModeDate(new Date(...(parts as [number, number])).getTime())
+    return Effect.succeed(new CodeModeDate(new Date(...(parts as [number, number])).getTime()))
+  }
+
+  private toDatePrimitive(value: unknown, node: AstNode): Effect.Effect<unknown, unknown, R> {
+    if (value === null || (typeof value !== "object" && typeof value !== "function")) return Effect.succeed(value)
+    const object = value as Record<string, unknown>
+    const self = this
+    return Effect.gen(function* () {
+      if (Object.hasOwn(object, "valueOf") && typeofValue(object.valueOf) === "function") {
+        const result = yield* self.runner.invokeCallable(object.valueOf, [], node)
+        if (result === null || (typeof result !== "object" && typeof result !== "function")) return result
+      }
+      if (!Object.hasOwn(object, "toString")) return coerceToString(value)
+      if (typeofValue(object.toString) === "function") {
+        const result = yield* self.runner.invokeCallable(object.toString, [], node)
+        if (result === null || (typeof result !== "object" && typeof result !== "function")) return result
+      }
+      throw new InterpreterRuntimeError("Cannot convert object to primitive value.", node).as("TypeError")
+    })
   }
 
   private constructRegExp(args: Array<unknown>, node: AstNode): CodeModeRegExp {

+ 1 - 1
packages/codemode/src/stdlib/date.ts

@@ -45,7 +45,7 @@ export const invokeDateMethod = (value: CodeModeDate, name: string, node: AstNod
     case "valueOf":
       return value.time
     case "toISOString":
-      if (!Number.isFinite(value.time)) throw new InterpreterRuntimeError("Invalid time value.", node)
+      if (!Number.isFinite(value.time)) throw new InterpreterRuntimeError("Invalid time value.", node).as("RangeError")
       return hosted.toISOString()
     case "toJSON":
       return Number.isFinite(value.time) ? hosted.toISOString() : null

+ 51 - 4
packages/codemode/test/callbacks.test.ts

@@ -131,10 +131,6 @@ describe("constructors callable without new, like JS", () => {
     expect((await error(`try { Array(-1) } catch (e) { throw Error(e.name) }`)).message).toContain("RangeError")
   })
 
-  test("sort densifies trailing holes into undefined (documented divergence)", async () => {
-    expect(await value(`return Array(2).sort().map(() => 1)`)).toEqual([1, 1])
-  })
-
   test("returned sparse arrays normalize holes to null at the host boundary", async () => {
     expect(await value(`return Array(3)`)).toEqual([null, null, null])
   })
@@ -153,6 +149,57 @@ describe("constructors callable without new, like JS", () => {
 })
 
 describe("sort accepts the unified callback set", () => {
+  test("sort preserves trailing holes while toSorted densifies them", async () => {
+    expect(
+      await value(`
+        const defaultSorted = [2, , 1]
+        const compared = [2, , 1]
+        const copied = defaultSorted.toSorted()
+        defaultSorted.sort()
+        compared.sort((a, b) => a - b)
+        return [
+          Object.hasOwn(defaultSorted, 2),
+          Object.hasOwn(compared, 2),
+          Object.hasOwn(copied, 2),
+        ]
+      `),
+    ).toEqual([false, false, true])
+
+    expect(await value(`const values = [2, undefined, 1]; values.sort(); return Object.hasOwn(values, 2)`)).toBe(true)
+  })
+
+  test("sort writes its snapshot without discarding comparator length mutations", async () => {
+    expect(
+      await value(`
+        const values = [3, 2, 1]
+        let first = true
+        values.sort((a, b) => {
+          if (first) {
+            first = false
+            values.push("kept")
+          }
+          return a - b
+        })
+        return values
+      `),
+    ).toEqual([1, 2, 3, "kept"])
+
+    expect(
+      await value(`
+        const values = [3, , 1, , 2]
+        let first = true
+        values.sort((a, b) => {
+          if (first) {
+            first = false
+            values.splice(0)
+          }
+          return a - b
+        })
+        return { values, owns: values.map((_, index) => Object.hasOwn(values, index)) }
+      `),
+    ).toEqual({ values: [1, 2, 3], owns: [true, true, true] })
+  })
+
   test("sort and toSorted take built-in comparators", async () => {
     expect(await value(`return [0, 1, 0].sort(Boolean)`)).toEqual([0, 0, 1])
     expect(await value(`return [0, 1, 0].toSorted(Boolean)`)).toEqual([0, 0, 1])

+ 63 - 7
packages/codemode/test/stdlib.test.ts

@@ -1,3 +1,14 @@
+/*
+ * Portions adapted from Test262 at revision 250f204f23a9249ff204be2baec29600faae7b75:
+ * - test/built-ins/Date/value-to-primitive-result-non-string-prim.js
+ * - test/built-ins/Date/value-to-primitive-result-string.js
+ *
+ * CodeMode does not support Symbol.toPrimitive, so these cases exercise the same Date-constructor primitive-result
+ * handling through supported own valueOf and toString functions.
+ *
+ * Copyright (C) 2016 the V8 project authors. All rights reserved.
+ * Test262 portions are governed by the BSD license in LICENSE.test262.
+ */
 import { describe, expect, test } from "bun:test"
 import { Effect, Schema } from "effect"
 import { CodeMode, Tool } from "../src/index.js"
@@ -55,6 +66,52 @@ describe("Date", () => {
     expect(await value(`return Date.parse("2024-01-02T03:04:05.000Z")`)).toBe(1704164645000)
   })
 
+  test("one-argument construction coerces supported values like JavaScript", async () => {
+    expect(
+      await value(`return [new Date(true).getTime(), new Date(false).getTime(), new Date(null).getTime()]`),
+    ).toEqual([1, 0, 0])
+    expect(await value(`return Number.isNaN(new Date(undefined).getTime())`)).toBe(true)
+    expect(await value(`return Number.isNaN(new Date([]).getTime())`)).toBe(true)
+    expect(await value(`return new Date(["1970-01-01T00:00:00.000Z"]).getTime()`)).toBe(0)
+    expect(await value(`return Number.isNaN(new Date({}).getTime())`)).toBe(true)
+  })
+
+  test("one-argument construction uses valueOf then toString for objects", async () => {
+    expect(
+      await value(`
+        const calls = []
+        const number = { valueOf: () => 8 }
+        const text = {
+          valueOf: () => { calls.push("valueOf"); return {} },
+          toString: () => { calls.push("toString"); return "2016-06-05T18:40:00.000Z" },
+        }
+        return [new Date(number).getTime(), new Date(text).getTime(), calls]
+      `),
+    ).toEqual([8, 1465152000000, ["valueOf", "toString"]])
+
+    expect(
+      await value(`
+        const values = [
+          { valueOf: () => undefined },
+          { valueOf: () => true },
+          { valueOf: () => false },
+          { valueOf: () => null },
+        ]
+        return values.map((item) => new Date(item).getTime())
+      `),
+    ).toEqual([null, 1, 0, 0])
+
+    expect(
+      await value(`
+        try {
+          new Date({ valueOf: () => ({}), toString: () => ({}) })
+        } catch (error) {
+          return error.name
+        }
+      `),
+    ).toBe("TypeError")
+  })
+
   test("date arithmetic and comparison use the time value", async () => {
     expect(await value(`const a = new Date(1000); const b = new Date(3000); return b - a`)).toBe(2000)
     expect(await value(`const a = new Date(1000); const b = new Date(3000); return a < b`)).toBe(true)
@@ -74,9 +131,9 @@ describe("Date", () => {
     expect(await value(`return new Date("garbage").toJSON()`)).toBeNull()
   })
 
-  test("toISOString on an invalid date is a catchable error", async () => {
-    expect(await value(`try { new Date("garbage").toISOString(); return "no" } catch { return "caught" }`)).toBe(
-      "caught",
+  test("toISOString on an invalid date throws RangeError", async () => {
+    expect(await value(`try { new Date("garbage").toISOString() } catch (error) { return error.name }`)).toBe(
+      "RangeError",
     )
   })
 
@@ -198,7 +255,7 @@ describe("RegExp", () => {
     ).toBe("7null[object Object]")
   })
 
-  test("function replacers can await effectful tool calls", async () => {
+  test("promise-returning string replacers are coerced synchronously", async () => {
     const decorate = Tool.make({
       description: "Decorate a string",
       input: Schema.String,
@@ -211,7 +268,7 @@ describe("RegExp", () => {
         code: `return "a1b22".replace(/\\d+/g, async (match) => await tools.host.decorate(match))`,
       }),
     )
-    expect(result.ok && result.value).toBe("a[1]b[22]")
+    expect(result.ok && result.value).toBe("a[object Promise]b[object Promise]")
 
     const missingAwait = await Effect.runPromise(
       CodeMode.execute({
@@ -219,8 +276,7 @@ describe("RegExp", () => {
         code: `return "a1".replace(/\\d/, (match) => tools.host.decorate(match))`,
       }),
     )
-    expect(!missingAwait.ok && missingAwait.error.kind).toBe("InvalidDataValue")
-    expect(!missingAwait.ok && missingAwait.error.message).toContain("un-awaited Promise")
+    expect(missingAwait.ok && missingAwait.value).toBe("a[object Promise]")
   })
 
   test("replaceAll without the g flag is a catchable error", async () => {