Check for Error accessors after definitions over object-typed targets

The reflection gate turned on only for an untyped or Error-shaped
defineProperty target, so a helper taking `target: object` (or `{}`,
`Object`, a weak type, or a type parameter) installed a message or name
accessor that typed Error reads then skipped, returning the old slot
value. The gate now turns on for any target an Error may flow into: an
untyped or generic type, a type Error is assignable to, or an
Error-shaped type, alone or in a union or intersection.

packages/compiler/src/frontend/lowering/lower-calls.ts

@@ -269,7 +269,7 @@ const ZLIB_STREAM_METHODS: ReadonlySet<string> = new Set(["flush", "params", "cl
 
 /** `Object.defineProperty(target, ...)`, `Object.defineProperties`,
  * `Reflect.defineProperty`, and `target.__defineGetter__/__defineSetter__`
- * over an untyped or Error-shaped target can install an accessor over an
+ * over a target that may hold an Error can install an accessor over an
  * error's `name` or `message` (error-methods.ts). */
 function noteErrorAccessorReflection(lowerer: Lowerer, expr: ts.CallExpression): void {
   const callee = expr.expression;
@@ -282,15 +282,28 @@ function noteErrorAccessorReflection(lowerer: Lowerer, expr: ts.CallExpression):
         ? callee.expression
         : undefined;
   if (!target) return;
-  const type = lowerer.checker.getTypeAtLocation(target);
-  if (
-    type.flags & (ts.TypeFlags.Any | ts.TypeFlags.Unknown) ||
-    (lowerer.checker.getPropertyOfType(type, "message") &&
-      lowerer.checker.getPropertyOfType(type, "name"))
-  )
+  const error = lowerer.checker.resolveName("Error", ts.SymbolFlags.Type, target, false);
+  const errorType = error ? lowerer.checker.getDeclaredTypeOfSymbol(error) : null;
+  if (mayHoldError(lowerer, lowerer.checker.getTypeAtLocation(target), errorType))
     lowerer.errorAccessorReflection = locOf(expr);
 }
 
+/** An untyped or generic type, a type Error is assignable to (`object`,
+ * `{}`, `Object`, `{ message?: string }`), or
+ * an Error-shaped type with both `name` and `message`, alone or in a
+ * union or intersection. */
+function mayHoldError(lowerer: Lowerer, type: ts.Type, errorType: ts.Type | null): boolean {
+  if (type.flags & (ts.TypeFlags.Any | ts.TypeFlags.Unknown | ts.TypeFlags.Instantiable))
+    return true;
+  if (type.flags & (ts.TypeFlags.Union | ts.TypeFlags.Intersection))
+    return ts.constituentTypes(type).some((part) => mayHoldError(lowerer, part, errorType));
+  return (
+    (lowerer.checker.getPropertyOfType(type, "message") !== undefined &&
+      lowerer.checker.getPropertyOfType(type, "name") !== undefined) ||
+    (errorType !== null && lowerer.checker.isTypeAssignableTo(errorType, type))
+  );
+}
+
 export function lowerCall(lowerer: Lowerer, expr: ts.CallExpression): IrExpr {
   const loc = locOf(expr);
   if (lowerer.errorAccessorReflection === null) noteErrorAccessorReflection(lowerer, expr);

packages/compiler/test/ts7/baselines/order-parity/tests/corpus/7263-error-accessor-object-helper.ts.json

@@ -0,0 +1,6 @@
+{
+ "order": [
+  "<repo>/tests/corpus/7263-error-accessor-object-helper.ts"
+ ],
+ "diags": []
+}

tests/corpus/7263-error-accessor-object-helper.ts

@@ -0,0 +1,54 @@
+// An accessor installed over an Error's own `message` and `name` by
+// helpers whose target parameter is typed `object`, `{}` or generic:
+// typed reads, writes and String() go through the accessors.
+class ValidationError extends Error {}
+
+function defineLazyMessage(target: object, compute: () => string): void {
+  let cached: string | undefined;
+  Object.defineProperty(target, "message", {
+    get() {
+      cached ??= compute();
+      return cached;
+    },
+    set(value: string) {
+      cached = `set:${value}`;
+    },
+    enumerable: true,
+    configurable: true,
+  });
+}
+
+function defineName(target: {}, name: string): void {
+  Object.defineProperty(target, "name", { get: () => name, configurable: true });
+}
+
+function defineFixedMessage<T extends object>(target: T, message: string): T {
+  Object.defineProperty(target, "message", { get: () => message, configurable: true });
+  return target;
+}
+
+let computed = 0;
+const e = new ValidationError("orig");
+defineLazyMessage(e, () => {
+  computed++;
+  return "too small";
+});
+console.log(e.message, computed);
+console.log(String(e), computed);
+e.message = "typed";
+console.log(e.message, Object.keys(e).join());
+
+const named = new ValidationError("bad input");
+defineName(named, "ZodError");
+console.log(named.name, String(named));
+const fixed = defineFixedMessage(new ValidationError("orig"), "fixed");
+console.log(fixed.message, String(fixed));
+
+const plain = new Error("start");
+defineLazyMessage(plain, () => "computed");
+console.log(plain.message, `${plain}`);
+try {
+  throw plain;
+} catch (caught) {
+  console.log(caught instanceof Error && caught.message);
+}

tests/harness/corpus-costs/7263-error-accessor-object-helper.ts.json

@@ -0,0 +1 @@
+{ "plain": 2600, "san": 4000 }

tests/harness/error-accessor-reflection.test.ts

@@ -0,0 +1,77 @@
+/* Reflection that can install an accessor over an error's `name` or
+ * `message`: a defineProperty call over a target that may hold an Error
+ * turns on the accessor checks of typed Error reads and writes
+ * (error-methods.ts), whatever the target parameter's declared type. */
+import { createHash } from "node:crypto";
+import { mkdirSync, readFileSync, writeFileSync } from "node:fs";
+import { join } from "node:path";
+import { expect, test } from "vitest";
+import { compile } from "@scriptc/compiler";
+
+const repoRoot = join(import.meta.dirname, "../..");
+const cacheDir = join(repoRoot, "node_modules/.cache/scriptc-tests");
+
+async function emittedIr(source: string): Promise<string> {
+  const key = createHash("sha256").update(source).digest("hex").slice(0, 16);
+  const outDir = join(cacheDir, `error-accessor-${key}`);
+  mkdirSync(outDir, { recursive: true });
+  const file = join(outDir, "main.ts");
+  writeFileSync(file, source);
+  const result = await compile(file, {
+    outPath: join(outDir, "main.ir"),
+    outDir,
+    outputKind: "ir",
+  });
+  if (!result.ok)
+    throw new Error(result.diagnostics.map((d) => `${d.code}: ${d.message}`).join("\n"));
+  return readFileSync(result.artifact.path, "utf8");
+}
+
+const program = (
+  param: string,
+  define = "Object.defineProperty",
+): string => `class E extends Error {}
+function install${param} {
+  ${define}(target, "message", { get: () => "lazy", configurable: true });
+}
+const e = new E("orig");
+install(e);
+console.log(e.message);
+`;
+
+const checksAccessors = async (source: string): Promise<boolean> =>
+  (await emittedIr(source)).includes("error.ownAccessor");
+
+test.each([
+  ["object", "(target: object)"],
+  ["{}", "(target: {})"],
+  ["Object", "(target: Object)"],
+  ["a weak type", "(target: { message?: string })"],
+  ["{ message: string }", "(target: { message: string })"],
+  ["a type parameter", "<T>(target: T)"],
+  ["a constrained type parameter", "<T extends object>(target: T)"],
+  ["object | undefined", "(target: object | undefined)"],
+  ["Error", "(target: Error)"],
+  ["unknown", "(target: unknown)"],
+])("a definition over %s checks for accessors", async (_, param) => {
+  expect(await checksAccessors(program(param))).toBe(true);
+});
+
+test("Reflect.defineProperty over object checks for accessors", async () => {
+  expect(await checksAccessors(program("(target: object)", "Reflect.defineProperty"))).toBe(true);
+});
+
+test("a definition over a type that cannot hold an Error leaves typed reads plain", async () => {
+  const source = `class E extends Error {}
+class Box {
+  id = 0;
+}
+function install(target: Box) {
+  Object.defineProperty(target, "extra", { get: () => 1, configurable: true });
+}
+install(new Box());
+const e = new E("orig");
+console.log(e.message);
+`;
+  expect(await checksAccessors(source)).toBe(false);
+});