Skip to content

Commit

Permalink
fix: ensure optional paths are not used as discriminants (#870)
Browse files Browse the repository at this point in the history
  • Loading branch information
ssalbdivad authored Oct 31, 2023
1 parent e0e6f7b commit 06262e5
Show file tree
Hide file tree
Showing 5 changed files with 101 additions and 3 deletions.
5 changes: 5 additions & 0 deletions dev/configs/.changeset/unlucky-kangaroos-divide.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"arktype": patch
---

Fix an issue where optional paths could be used as discriminants
73 changes: 73 additions & 0 deletions dev/test/discriminate.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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([
Expand Down
12 changes: 11 additions & 1 deletion src/nodes/compose.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,8 @@ export type DisjointKind = keyof DisjointKinds

export class IntersectionState {
path = new Path()
lOptional = false
rOptional = false
domain: Domain | undefined
#disjoints: DisjointsByPath = {}

Expand All @@ -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
}
}
Expand All @@ -135,6 +143,8 @@ export type DisjointsByPath = Record<string, DisjointContext>

export type DisjointContext<kind extends DisjointKind = DisjointKind> = {
kind: kind
lOptional: boolean
rOptional: boolean
} & DisjointKinds[kind]

const empty = Symbol("empty")
Expand Down
6 changes: 5 additions & 1 deletion src/nodes/discriminate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
8 changes: 7 additions & 1 deletion src/nodes/rules/props.ts
Original file line number Diff line number Diff line change
Expand Up @@ -145,9 +145,15 @@ const propKeysIntersection = composeKeyedIntersection<PropsRule>(
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
Expand Down

0 comments on commit 06262e5

Please sign in to comment.