Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: ensure optional paths are not used as discriminants #870

Merged
merged 1 commit into from
Oct 31, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading