From ed50daa853aedeea1d644df779db93ada8479e80 Mon Sep 17 00:00:00 2001 From: Malte Ubl Date: Sun, 25 Oct 2026 11:01:57 -0700 Subject: [PATCH] Keep the whole union of an optional parameter that reaches a recursive placeholder An optional parameter typed by a union that is still being mapped gets a recursive placeholder: a union id whose arms arrive when the outer frame finishes. The function-type mapping then armed that placeholder with undefined, but the placeholder's arm list was still empty, so the result was a fresh union holding only `undefined`. The parameter lost its record arm, the same checker type mapped to two different layouts depending on where the mapping started, and later conversions failed (SC2001, SC2002, SC2003). This happens when an interface method takes an options object whose callbacks are typed by a subinterface of that same interface: `newSession(options?: { withSession?: (ctx: Replaced) => Promise })` with `interface Replaced extends CommandContext`. When the checker spelled the parameter with an undefined constituent, the placeholder already carries that arm, so the parameter keeps the placeholder. withUnitArm now refuses a pending placeholder instead of building the unit-only union. pi's ExtensionCommandContext is such an interface. It refused ExtensionAPI.registerCommand, and with it pi's main(). Co-Authored-By: Claude Opus 5.5 --- .../compiler/src/frontend/type-mapper.test.ts | 14 +++ packages/compiler/src/frontend/type-mapper.ts | 24 +++- ...recursive-optional-callback-params.ts.json | 6 + .../recursive-optional-callback-params.ts | 107 ++++++++++++++++++ ...recursive-optional-callback-params.ts.json | 1 + 5 files changed, 150 insertions(+), 2 deletions(-) create mode 100644 packages/compiler/test/ts7/baselines/order-parity/tests/corpus/recursive-optional-callback-params.ts.json create mode 100644 tests/corpus/recursive-optional-callback-params.ts create mode 100644 tests/harness/corpus-costs/recursive-optional-callback-params.ts.json diff --git a/packages/compiler/src/frontend/type-mapper.test.ts b/packages/compiler/src/frontend/type-mapper.test.ts index 163ba7b9e..6e7e339d6 100644 --- a/packages/compiler/src/frontend/type-mapper.test.ts +++ b/packages/compiler/src/frontend/type-mapper.test.ts @@ -53,6 +53,20 @@ describe("union arm lookup", () => { }); }); +describe("recursive union placeholders", () => { + test("refuse a unit arm until their arms are known", () => { + const unions = new UnionRegistry(); + const recursive = {} as Type; + const union = unions.recursiveRef(recursive, () => "Recursive"); + const placeholder: IrType = { kind: "union", unionId: union }; + expect(unions.isPendingPlaceholder(union)).toBe(true); + expect(withUndefinedArm(placeholder, unions)).toBeNull(); + unions.finalizeRecursive(recursive, [{ kind: "record", shapeId: "r1" }, UNDEFINED_T]); + expect(unions.isPendingPlaceholder(union)).toBe(false); + expect(withUndefinedArm(placeholder, unions)).toEqual(placeholder); + }); +}); + describe("nullable collection union builders", () => { test.each([mapOf(STRING, F64), setOf(STRING)])( "optional %j fields and generator results share the same union", diff --git a/packages/compiler/src/frontend/type-mapper.ts b/packages/compiler/src/frontend/type-mapper.ts index 17f9d0d11..63eed378d 100644 --- a/packages/compiler/src/frontend/type-mapper.ts +++ b/packages/compiler/src/frontend/type-mapper.ts @@ -717,6 +717,12 @@ export class UnionRegistry { return id !== undefined && this.pendingRec.has(id); } + /** True when `unionId` is a recursive placeholder whose arms are not + * known yet (its outer frame is still mapping, or failed). */ + isPendingPlaceholder(unionId: string): boolean { + return this.pendingRec.has(unionId); + } + /** Completes a recursive placeholder with its canonical arm list and * registers the structural key (first writer wins, like shapes). */ finalizeRecursive(t: ts.Type, arms: IrType[], discriminant?: IrUnionDiscriminant): string { @@ -5302,8 +5308,19 @@ function mapCallSignature(sig: ts.Signature, ctx: TypeMapperCtx): IrType | null if (pt) pt = jsOpenObjectType(decl, pt, ctx.shapes, ctx.unions); // Belt and braces for non-strict type worlds: an optional param's // ABI slot is always the undefined-armed union (strictNullChecks - // already spells it that way; arm it here if the world didn't). - if (optional && pt && pt.kind !== "void" && pt.kind !== "dyn") { + // already spells it that way; arm it here if the world didn't). A + // back-reference to a union still being mapped (`options?: { then?: + // (ctx: Sub) => void }` where Sub extends the interface declaring the + // method) is a recursive placeholder with no arms yet; when the checker + // spelled the parameter `X | undefined`, the placeholder receives that + // undefined arm when its frame finishes. + const pendingOptional = + pt?.kind === "union" && + ctx.unions.isPendingPlaceholder(pt.unionId) && + ts + .constituentTypes(checker.getTypeOfSymbol(p)) + .some((part) => (part.flags & ts.TypeFlags.Undefined) !== 0); + if (optional && pt && pt.kind !== "void" && pt.kind !== "dyn" && !pendingOptional) { const armed = withUndefinedArm(pt, ctx.unions); if (!armed) return null; pt = armed; @@ -6782,6 +6799,9 @@ export function withUnitArm( if (t.kind === "union") { const def = unions.get(t.unionId); if (!def) return null; + // A recursive placeholder's arms arrive when its outer frame finishes; + // extending its (still empty) arm list would drop every arm but the unit. + if (unions.isPendingPlaceholder(t.unionId)) return null; if (def.arms.some((a) => a.kind === kind)) return t; const arms = [...def.arms, unit]; // Date and Promise arms beside other data arms have no union home. diff --git a/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/recursive-optional-callback-params.ts.json b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/recursive-optional-callback-params.ts.json new file mode 100644 index 000000000..abf783f85 --- /dev/null +++ b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/recursive-optional-callback-params.ts.json @@ -0,0 +1,6 @@ +{ + "order": [ + "/tests/corpus/recursive-optional-callback-params.ts" + ], + "diags": [] +} diff --git a/tests/corpus/recursive-optional-callback-params.ts b/tests/corpus/recursive-optional-callback-params.ts new file mode 100644 index 000000000..0c81235d6 --- /dev/null +++ b/tests/corpus/recursive-optional-callback-params.ts @@ -0,0 +1,107 @@ +// An optional parameter whose type reaches back to the interface that +// declares the method keeps its whole union. A command context's +// `newSession(options?)` takes callbacks typed by a subinterface of the +// context, so mapping the context passes through the options union while +// that union is still being mapped; the parameter still holds the options +// record, not only `undefined`. + +interface CommandContext { + cwd: string; + newSession(options?: { + parent?: string; + withSession?: (ctx: ReplacedContext) => Promise; + }): Promise<{ cancelled: boolean }>; + fork( + entryId: string, + options?: { position?: "before" | "at"; withSession?: (ctx: ReplacedContext) => Promise }, + ): Promise<{ cancelled: boolean }>; +} + +interface ReplacedContext extends CommandContext { + depth: number; + send(content: string): Promise; +} + +interface Command { + description?: string; + handler: (args: string, ctx: CommandContext) => Promise; +} + +const log: string[] = []; +const commands = new Map(); + +function register(name: string, command: Command): void { + commands.set(name, command); +} + +register("new", { + description: "start a session", + handler: async (args, ctx) => { + const first = await ctx.newSession({ + parent: args, + withSession: async (next) => { + await next.send(`hello from ${next.cwd} at depth ${next.depth}`); + const nested = await next.newSession({ + withSession: async (inner) => { + await inner.send(`nested at depth ${inner.depth}`); + }, + }); + log.push(`nested cancelled=${nested.cancelled}`); + }, + }); + log.push(`first cancelled=${first.cancelled}`); + const bare = await ctx.newSession(); + log.push(`bare cancelled=${bare.cancelled}`); + }, +}); + +register("fork", { + handler: async (args, ctx) => { + const r = await ctx.fork(args, { + position: "at", + withSession: async (next) => next.send(`forked ${args} at depth ${next.depth}`), + }); + log.push(`fork cancelled=${r.cancelled}`); + const plain = await ctx.fork(args); + log.push(`plain fork cancelled=${plain.cancelled}`); + }, +}); + +function makeContext(depth: number): ReplacedContext { + return { + cwd: `/work/${depth}`, + depth, + newSession: async (options) => { + log.push(`newSession depth=${depth} parent=${options?.parent ?? "-"}`); + if (options?.withSession) { + await options.withSession(makeContext(depth + 1)); + return { cancelled: false }; + } + return { cancelled: true }; + }, + fork: async (entryId, options) => { + log.push(`fork ${entryId} position=${options?.position ?? "-"}`); + const callback = options?.withSession; + if (callback === undefined) return { cancelled: true }; + await callback(makeContext(depth + 1)); + return { cancelled: false }; + }, + send: async (content) => { + log.push(`send: ${content}`); + }, + }; +} + +async function run(): Promise { + const root: CommandContext = makeContext(0); + for (const [name, command] of commands) { + log.push(`run ${name} (${command.description ?? "no description"})`); + await command.handler(`entry-${name}`, root); + } + for (const line of log) console.log(line); +} + +run().catch((e: unknown) => { + console.log("failed", e instanceof Error ? e.message : String(e)); + process.exitCode = 1; +}); diff --git a/tests/harness/corpus-costs/recursive-optional-callback-params.ts.json b/tests/harness/corpus-costs/recursive-optional-callback-params.ts.json new file mode 100644 index 000000000..c33a4b32c --- /dev/null +++ b/tests/harness/corpus-costs/recursive-optional-callback-params.ts.json @@ -0,0 +1 @@ +{ "plain": 6000 }