From 06262e5e3fead2be9171ec9fdd54373faa62a7a3 Mon Sep 17 00:00:00 2001 From: David Blass Date: Tue, 31 Oct 2023 16:36:53 -0400 Subject: [PATCH] fix: ensure optional paths are not used as discriminants (#870) --- .../.changeset/unlucky-kangaroos-divide.md | 5 ++ dev/test/discriminate.test.ts | 73 +++++++++++++++++++ src/nodes/compose.ts | 12 ++- src/nodes/discriminate.ts | 6 +- src/nodes/rules/props.ts | 8 +- 5 files changed, 101 insertions(+), 3 deletions(-) create mode 100644 dev/configs/.changeset/unlucky-kangaroos-divide.md diff --git a/dev/configs/.changeset/unlucky-kangaroos-divide.md b/dev/configs/.changeset/unlucky-kangaroos-divide.md new file mode 100644 index 0000000000..42f5816ac3 --- /dev/null +++ b/dev/configs/.changeset/unlucky-kangaroos-divide.md @@ -0,0 +1,5 @@ +--- +"arktype": patch +--- + +Fix an issue where optional paths could be used as discriminants diff --git a/dev/test/discriminate.test.ts b/dev/test/discriminate.test.ts index 75c519f14b..9fc0d33d1c 100644 --- a/dev/test/discriminate.test.ts +++ b/dev/test/discriminate.test.ts @@ -77,6 +77,79 @@ describe("discriminate", () => { ] ]) }) + it("doesn't discriminate optional key", () => { + const a = type({ + direction: "'forward' | 'backward'", + "operator?": "'by'" + }) + + const b = type({ + duration: "'s' | 'min' | 'h'", + operator: "'to'" + }) + + const c = type([a, "|", b]) + attest(c.flat).snap([ + ["domain", "object"], + [ + "branches", + [ + [ + [ + "requiredProp", + [ + "direction", + [ + ["domain", "string"], + [ + "switch", + { + path: [], + kind: "value", + cases: { + "'forward'": [], + "'backward'": [] + } + } + ] + ] + ] + ], + ["optionalProp", ["operator", [["value", "by"]]]] + ], + [ + [ + "requiredProp", + [ + "duration", + [ + ["domain", "string"], + [ + "switch", + { + path: [], + kind: "value", + cases: { + "'s'": [], + "'min'": [], + "'h'": [] + } + } + ] + ] + ] + ], + ["requiredProp", ["operator", [["value", "to"]]]] + ] + ] + ] + ]) + attest( + c.allows({ + direction: "forward" + }) + ).equals(true) + }) it("undiscriminatable", () => { const t = getPlaces().type([ diff --git a/src/nodes/compose.ts b/src/nodes/compose.ts index b040b49abd..57ce6c9055 100644 --- a/src/nodes/compose.ts +++ b/src/nodes/compose.ts @@ -112,6 +112,8 @@ export type DisjointKind = keyof DisjointKinds export class IntersectionState { path = new Path() + lOptional = false + rOptional = false domain: Domain | undefined #disjoints: DisjointsByPath = {} @@ -126,7 +128,13 @@ export class IntersectionState { l: DisjointKinds[kind]["l"], r: DisjointKinds[kind]["r"] ): Empty { - this.#disjoints[`${this.path}`] = { kind, l, r } + this.#disjoints[`${this.path}`] = { + kind, + l, + r, + lOptional: this.lOptional, + rOptional: this.rOptional + } return empty } } @@ -135,6 +143,8 @@ export type DisjointsByPath = Record export type DisjointContext = { kind: kind + lOptional: boolean + rOptional: boolean } & DisjointKinds[kind] const empty = Symbol("empty") diff --git a/src/nodes/discriminate.ts b/src/nodes/discriminate.ts index 311eb78559..8bbb79bb2b 100644 --- a/src/nodes/discriminate.ts +++ b/src/nodes/discriminate.ts @@ -244,10 +244,14 @@ const calculateDiscriminants = ( // https://github.com/arktypeio/arktype/issues/593 continue } - const { l, r, kind } = intersectionState.disjoints[path] + const { l, r, kind, lOptional, rOptional } = + intersectionState.disjoints[path] if (!isKeyOf(kind, discriminantKinds)) { continue } + if (lOptional || rOptional) { + continue + } const lSerialized = serializeDefinitionIfAllowed(kind, l) const rSerialized = serializeDefinitionIfAllowed(kind, r) if (lSerialized === undefined || rSerialized === undefined) { diff --git a/src/nodes/rules/props.ts b/src/nodes/rules/props.ts index 372fa3c3b2..a41edd8903 100644 --- a/src/nodes/rules/props.ts +++ b/src/nodes/rules/props.ts @@ -145,9 +145,15 @@ const propKeysIntersection = composeKeyedIntersection( return l } context.path.push(propKey) + const previousLOptional = context.lOptional + const previousROptional = context.rOptional + context.lOptional ||= isOptional(l) + context.rOptional ||= isOptional(r) const result = nodeIntersection(propToNode(l), propToNode(r), context) + const resultIsOptional = context.lOptional && context.rOptional + context.rOptional = previousROptional + context.lOptional = previousLOptional context.path.pop() - const resultIsOptional = isOptional(l) && isOptional(r) if (isDisjoint(result) && resultIsOptional) { // If an optional key has an empty intersection, the type can // still be satisfied as long as the key is not included. Set