Treat optional members an index signature covers as keys, erase 'as never', and keep fromEntries objects asserted to records (#1496)

packages/compiler/src/frontend/lowering/expressions/narrowing.ts

@@ -1187,6 +1187,56 @@ function classInstanceOrAbsent(lowerer: Lowerer, type: IrType): boolean {
   );
 }
 
+/** `Object.fromEntries(...) as T` with T a declared record (the generic
+ * `transform<T extends Record<string, unknown>>(t: T): T` idiom): the
+ * entries build a pure index-signature record, which has no layout in
+ * common with T's struct. Node keeps the object itself, with exactly the
+ * keys and the order the entries gave it. The result of fromEntries is a
+ * fresh object nothing else references, so the checked-dynamic bridge,
+ * which keeps keys and their order the same way `JSON.parse(text) as T`
+ * does, cannot be told apart from the original object; a value outside
+ * T throws the catchable TypeError of every checked cast. The entries'
+ * values must be scalars or checked-dynamic values, which the bridge
+ * passes by value or by identity; native arrays and records would be
+ * copied. Other sources may be aliased, where a rebuilt object would sever
+ * mutation, so they keep the exact-shape fence. */
+function freshEntriesRecordAssertion(
+  lowerer: Lowerer,
+  expr: ts.AsExpression | ts.TypeAssertion,
+  inner: IrExpr,
+  target: IrType | null,
+): IrExpr | null {
+  if (inner.type.kind !== "record" || target?.kind !== "record") return null;
+  const source = lowerer.shapes.get(inner.type.shapeId);
+  const dest = lowerer.shapes.get(target.shapeId);
+  if (!source?.indexValue || source.tuple || source.fields.length > 0) return null;
+  if (!dest || dest.tuple || dest.indexValue || shapeHasAccessorSlots(dest)) return null;
+  // Native arrays and records in the entries would be copied on the way
+  // through the checked-dynamic bridge, severing their identity: only
+  // slots of scalars or checked-dynamic values pass.
+  if (extractionCopiesReference(lowerer, source.indexValue)) return null;
+  let operand: ts.Expression = expr.expression;
+  while (ts.isParenthesizedExpression(operand)) operand = operand.expression;
+  if (
+    !ts.isCallExpression(operand) ||
+    !ts.isPropertyAccessExpression(operand.expression) ||
+    operand.expression.name.text !== "fromEntries" ||
+    !lowerer.isStdlibGlobal(operand.expression.expression, "Object")
+  )
+    return null;
+  if (!lowerer.dynConvertible(inner.type)) return null;
+  if (
+    !canDynCheckTo(
+      target,
+      (id) => lowerer.shapes.get(id),
+      (id) => lowerer.unions.get(id),
+    )
+  )
+    return null;
+  const loc = locOf(expr);
+  return { kind: "dynCheck", value: lowerer.coerceToExpected(inner, DYN), type: target, loc };
+}
+
 /** `e as T` and the old-style assertion `<T>e` — one node shape (both
  * carry `.type` and `.expression`), one lowering. */
 export function lowerAsExpression(
@@ -1324,6 +1374,8 @@ export function lowerAsExpression(
     ) {
       return lowerer.coerceInto(expr, inner, target);
     }
+    const rebuilt = freshEntriesRecordAssertion(lowerer, expr, inner, target);
+    if (rebuilt) return rebuilt;
     // `job as Job & { startedAt: number }` on a record: the asserted
     // record refines the stored one, so the assertion keeps the object
     // (a width copy would sever writes through the asserted view).
@@ -1377,6 +1429,12 @@ export function lowerAsExpression(
     return inner; // erasure, unchanged
   }
 
+  // `value as never` is the escape hatch that makes an unknown value fit
+  // any slot (`tagSuccess(value as never)`). No value has type never, so
+  // checking against its placeholder layout would reject every value.
+  // Erase it instead: the consuming slot applies the same validated
+  // extraction an unknown value gets wherever the checker admits it.
+  if (expr.type.kind === ts.SyntaxKind.NeverKeyword) return inner;
   const targetTs = lowerer.checker.getTypeFromTypeNode(expr.type);
   const target = lowerer.mapTypeOf(targetTs) ?? dynFallbackType(lowerer, expr.type, targetTs);
   if (!target) lowerer.badType(expr.type, targetTs);

packages/compiler/src/frontend/type-mapper.ts

@@ -6238,6 +6238,8 @@ function mapRecordType(widened: ts.Type, ctx: TypeMapperCtx): IrType | null {
       (inner.indexValue === undefined || inner.indexValue.kind === "dyn")
     )
       return DYN;
+    const covered = coveredOptionalFieldsShape(widened, inner, ctx);
+    if (covered !== null) return { kind: "record", shapeId: covered };
     const merged = dictSpreadLiteralShape(widened, inner, ctx);
     if (merged === "dyn") return DYN;
     if (merged !== null) return { kind: "record", shapeId: merged };
@@ -6251,6 +6253,51 @@ function mapRecordType(widened: ts.Type, ctx: TypeMapperCtx): IrType | null {
   }
 }
 
+/** An index signature already describes every OPTIONAL declared member
+ * whose type is its slot type: @types/node's `ProcessEnv extends
+ * Dict<string> { TZ?: string }` and every spread of it, or a declared
+ * environment `{ CI?: string } & Dictionary<string>`. At runtime such a
+ * member is just another key, so the shape interns as the pure
+ * index-signature record. Its slot admits undefined, as the optional
+ * members do (`env.CI = undefined` keeps the key). A hybrid layout would
+ * give each member a struct slot, which enumerates before the overflow
+ * keys (Node keeps one insertion order) and makes `delete` and spread
+ * answer differently for declared and undeclared keys. Required members,
+ * and members typed otherwise than the slot (whose reads the slot type
+ * could not give back), keep the hybrid shape. */
+function coveredOptionalFieldsShape(
+  widened: ts.Type,
+  parts: RecordShapeParts,
+  ctx: TypeMapperCtx,
+): string | null {
+  const slot = parts.indexValue;
+  if (!slot || parts.fields.length === 0) return null;
+  const { checker, shapes, unions } = ctx;
+  // The members' common type must be the slot plus undefined. Read it off
+  // the members (already interned) rather than building it: a hybrid that
+  // does not qualify must not intern anything new.
+  const optionalSlot = parts.fields[0]!.type;
+  if (!parts.fields.every((field) => typeEquals(field.type, optionalSlot))) return null;
+  if (optionalSlot.kind !== "union" || unions.armTag(optionalSlot.unionId, UNDEFINED_T) < 0)
+    return null;
+  const memberArms = unions.get(optionalSlot.unionId)?.arms ?? [];
+  const slotArms =
+    slot.kind === "union" ? (unions.get(slot.unionId)?.arms ?? []) : isUnitType(slot) ? [] : [slot];
+  const definedArms = memberArms.filter((arm): boolean => arm.kind !== "undefinedT");
+  const slotDefined = slotArms.filter((arm): boolean => arm.kind !== "undefinedT");
+  if (
+    definedArms.length === 0 ||
+    definedArms.length !== slotDefined.length ||
+    !slotDefined.every((arm) => definedArms.some((other) => typeEquals(arm, other)))
+  )
+    return null;
+  if (!isSupportedIndexValue(optionalSlot)) return null;
+  const props = checker.getPropertiesOfType(widened);
+  if (props.length !== parts.fields.length) return null;
+  if (!props.every((p) => (p.flags & ts.SymbolFlags.Optional) !== 0)) return null;
+  return shapes.intern([], false, optionalSlot, []);
+}
+
 /** The type of an object literal that spreads an index-signature record
  * (`{ ...tags, bucket: b, status }`, the metric-tags idiom). The checker
  * drops the signature and types the literal by its explicit members only,

packages/compiler/test/ts7/baselines/order-parity/tests/corpus/6465-optional-members-over-index-signature.ts.json

@@ -0,0 +1,6 @@
+{
+ "order": [
+  "<repo>/tests/corpus/6465-optional-members-over-index-signature.ts"
+ ],
+ "diags": []
+}

packages/compiler/test/ts7/baselines/order-parity/tests/corpus/6466-never-assertion-argument.ts.json

@@ -0,0 +1,6 @@
+{
+ "order": [
+  "<repo>/tests/corpus/6466-never-assertion-argument.ts"
+ ],
+ "diags": []
+}

packages/compiler/test/ts7/baselines/order-parity/tests/corpus/6467-fromentries-record-assertion.ts.json

@@ -0,0 +1,6 @@
+{
+ "order": [
+  "<repo>/tests/corpus/6467-fromentries-record-assertion.ts"
+ ],
+ "diags": []
+}

packages/compiler/test/ts7/baselines/order-parity/tests/diagnostics/fromentries-aliased-entries.ts.json

@@ -0,0 +1,6 @@
+{
+ "order": [
+  "<repo>/tests/diagnostics/fromentries-aliased-entries.ts"
+ ],
+ "diags": []
+}

tests/corpus/6465-optional-members-over-index-signature.ts

@@ -0,0 +1,73 @@
+// An index signature whose value admits undefined already describes an
+// optional member of the same type (`interface Env extends Dict<string>
+// { TZ?: string }`, @types/node's ProcessEnv): such a record is one key
+// set in insertion order, so spread, delete, `in` and every enumeration
+// treat the declared member like any other key.
+interface Env {
+  TZ?: string;
+  [key: string]: string | undefined;
+}
+
+const base: Env = { A: "1", TZ: "UTC", B: "2" };
+const env = { ...base };
+env.C = "3";
+for (const key of ["A", "TZ", "MISSING"]) {
+  delete env[key];
+}
+console.log(Object.entries(env), Object.keys(base));
+const probe = (key: string): boolean => key in env;
+console.log(probe("TZ"), probe("B"), env.TZ, env.B);
+env.TZ = "Europe/Berlin";
+console.log(JSON.stringify(env), env);
+
+const extended: Env = { ...base, D: "4" };
+delete extended.TZ;
+extended.TZ = "Asia/Tokyo";
+console.log(extended, Object.keys(extended).join(","));
+
+// A declared member written after another key comes after it.
+interface Scores {
+  best?: number;
+  [name: string]: number | undefined;
+}
+const scores: Scores = {};
+scores["x"] = 1;
+scores.best = 2;
+console.log(Object.keys(scores), JSON.stringify(scores));
+
+// The process environment has the same shape; a child environment built
+// from it keeps the parent's keys in order.
+process.env.SCX_FIRST = "first";
+process.env.SCX_DROP = "drop";
+process.env.SCX_LAST = "last";
+const DROP = ["SCX_DROP", "SCX_NOT_SET"];
+function childEnv(): string[] {
+  const runEnv = { ...process.env };
+  runEnv.SCX_ADDED = "added";
+  for (const key of DROP) {
+    delete runEnv[key];
+  }
+  runEnv.SCX_FIRST = runEnv.SCX_FIRST ? `${runEnv.SCX_FIRST}!` : "none";
+  return Object.entries(runEnv)
+    .filter(([key]) => key.startsWith("SCX_"))
+    .map(([key, value]) => `${key}=${value || ""}`);
+}
+console.log(childEnv());
+
+// A record of declared optional members over a slot without undefined is
+// the same key set: merging it over the process environment keeps both.
+type Dictionary<T> = { [key: string]: T };
+type BuildEnv = { CI?: string; DEBUG?: string } & Dictionary<string>;
+function merged(buildEnv: BuildEnv): string[] {
+  const env = { ...process.env, ...buildEnv };
+  return [
+    String(env.CI === "1" || env.SCX_FIRST === "first"),
+    `${env.DEBUG ?? "-"}`,
+    Object.keys(env)
+      .filter((key) => key.startsWith("SCX_") || key === "CI" || key === "DEBUG")
+      .join(","),
+  ];
+}
+const buildEnv: BuildEnv = { SCX_BUILD: "b", CI: "1" };
+buildEnv.DEBUG = "yes";
+console.log(merged(buildEnv), Object.keys(buildEnv), buildEnv.CI, buildEnv["SCX_BUILD"]);

tests/corpus/6466-never-assertion-argument.ts

@@ -0,0 +1,31 @@
+// `value as never` makes an unknown value fit any parameter: the argument
+// is checked against the parameter's type like any unknown value the
+// checker admits there, and a typed value passes unchanged.
+type Out = { routes: { src: string; dest?: string }[]; count: number };
+
+function show(tags: Record<string, unknown>): void {
+  console.log(JSON.stringify(tags));
+}
+
+function report(tag: (result: Out) => Record<string, unknown>, value: unknown): void {
+  show(tag(value as never));
+}
+
+const describe = (r: Out): Record<string, unknown> => ({ count: r.count, first: r.routes[0]?.src });
+report(describe, JSON.parse('{"routes":[{"src":"/a"},{"src":"/b","dest":"/c"}],"count":2}'));
+
+// Through a promise of unknown, the way a wrapper tags an async result.
+function later(value: Out): Promise<unknown> {
+  return Promise.resolve(value);
+}
+later({ routes: [], count: 7 }).then((value) => {
+  show(describe(value as never));
+});
+
+// A typed value keeps its identity through the assertion.
+function bump(target: Out): void {
+  target.count++;
+}
+const typed: Out = { routes: [{ src: "/x" }], count: 1 };
+bump(typed as never);
+console.log(typed.count);

tests/corpus/6467-fromentries-record-assertion.ts

@@ -0,0 +1,43 @@
+// `Object.fromEntries(...) as T` for a declared record T keeps exactly
+// the keys the entries created, in their order, as Node keeps the object
+// itself, as long as the entries hold scalars or checked-dynamic values.
+type Tags = { "error.type": string; "error.msg": string; "error.cause"?: string };
+
+function scrub(input: string): string {
+  return input.replace(/secret/g, "***");
+}
+
+function redact(tags: Tags): Tags {
+  return Object.fromEntries(
+    Object.entries(tags).map(([key, value]) => [key, typeof value === "string" ? scrub(value) : value]),
+  ) as Tags;
+}
+
+const e = new Error("a secret here");
+const plain = redact({ "error.type": e.name, "error.msg": e.message });
+console.log(plain, Object.keys(plain), "error.cause" in plain, plain["error.msg"].length);
+const caused = redact({ "error.type": "TypeError", "error.msg": "m", "error.cause": "secret cause" });
+console.log(caused, JSON.stringify(caused));
+
+// Keys the record does not declare stay with the object.
+type Point = { x: number; y: number };
+const p = Object.fromEntries([
+  ["y", 2],
+  ["x", 1],
+  ["z", 3],
+]) as Point;
+console.log(p, p.x + p.y, Object.keys(p), JSON.stringify(p));
+
+// Entries held as unknown keep their identity: the object's members are
+// the same array and record, and writes through either reach both.
+type Holder = { list: number[]; point: { x: number } };
+const list = [1, 2];
+const point = { x: 1 };
+const entries: [string, unknown][] = [
+  ["list", list],
+  ["point", point],
+];
+const held = Object.fromEntries(entries) as Holder;
+held.list.push(3);
+held.point.x = 5;
+console.log(list.length, point.x, held.list === list, held.point === point);

tests/diagnostics/fromentries-aliased-entries.ts

@@ -0,0 +1,13 @@
+// `Object.fromEntries(...) as T` keeps the object only when the entries'
+// values are scalars or checked-dynamic values. Native arrays and records
+// in the entries would be copied, severing their identity, so the
+// assertion keeps the exact-shape fence (corpus 6467 covers the cases that
+// lower).
+type Scores = { a: number[]; b: number[] };
+const shared = [10];
+const scores = Object.fromEntries([
+  ["a", shared],
+  ["b", shared],
+]) as Scores;
+scores.a.push(11);
+console.log(shared.length, scores.b.length);

tests/harness/__snapshots__/fromentries-aliased-entries.ts.txt

@@ -0,0 +1,8 @@
+fromentries-aliased-entries.ts:8:16 - error SC2002: record shapes must match exactly or width-coerce: expected '{ a: number[]; b: number[] }', got '{ [key: string]: number[] }' — 'a' is not a declared field of the source, and the source's index signature could hold it at runtime (a completed undefined would drop that value)
+
+  7 | const shared = [10];
+  8 | const scores = Object.fromEntries([
+    |                ^~~~~~~~~~~~~~~~~~~~
+  9 |   ["a", shared],
+
+  hint: width subtyping compiles as a copy (each expected field copied off the source; extra fields drop) — this pair is outside the copy's rules; build a literal with exactly the expected fields instead

tests/harness/corpus-costs/6465-optional-members-over-index-signature.ts.json

@@ -0,0 +1 @@
+{ "plain": 3225, "san": 26787 }

tests/harness/corpus-costs/6466-never-assertion-argument.ts.json

@@ -0,0 +1 @@
+{ "plain": 2895, "san": 23565 }

tests/harness/corpus-costs/6467-fromentries-record-assertion.ts.json

@@ -0,0 +1 @@
+{ "plain": 3093, "san": 22400 }