fix: guard the surviving branch's own composition, via one shared predicate
Copilot round 10: one finding, the last position of the rule the previous two
rounds established. A *non-null* branch can carry its own `anyOf`, and the
hoist drops it — the branch's `anyOf` overwrites the wrapper's in the spread
and is then cleared, so `{ anyOf: [{ type: "string", anyOf: [{ const: "a" }] },
{ type: "null" }] }` widened into an unconstrained nullable string.
Rather than add a fourth inline condition, the four sites that ask "does this
schema compose something I cannot evaluate?" now share one predicate,
`hasUnevaluatedComposition` (an opaque applicator, or a nested `anyOf`):
- the surviving branch in the `anyOf` collapse (this round's finding),
- the null branch in `admitsNull`,
- the `type: [T, "null"]` collapse path,
- and, via `hasOpaqueApplicator`, the top of `admitsNull`.
Each of those was missed independently across rounds 8-10 for the same reason:
the guard existed but was spelled out inline at each site and drifted. Sharing
the predicate is the part that stops a fifth site from repeating it.
Signed-off-by: cliffhall <cliff@futurescale.com>
This commit is contained in:
parent
274a328abe
commit
562709a79a
2 changed files with 70 additions and 7 deletions
|
|
@ -450,6 +450,43 @@ describe("normalizeNullableUnion", () => {
|
|||
expect(normalizeNullableUnion(schema)).toBe(schema);
|
||||
});
|
||||
|
||||
// The hoist *drops* a nested union: the branch's `anyOf` overwrites the
|
||||
// wrapper's in the spread and is then cleared, so the constraint vanishes and
|
||||
// the field widens into an unconstrained nullable string.
|
||||
it("declines when the surviving branch carries its own anyOf", () => {
|
||||
const schema = {
|
||||
anyOf: [
|
||||
{ type: "string", anyOf: [{ const: "a" }] },
|
||||
{ type: "null" as const },
|
||||
],
|
||||
};
|
||||
expect(normalizeNullableUnion(schema)).toBe(schema);
|
||||
});
|
||||
|
||||
it("declines when the surviving branch carries an opaque applicator", () => {
|
||||
const withNot = {
|
||||
anyOf: [
|
||||
{ type: "string", not: { const: "a" } },
|
||||
{ type: "null" as const },
|
||||
],
|
||||
};
|
||||
expect(normalizeNullableUnion(withNot)).toBe(withNot);
|
||||
const withAllOf = {
|
||||
anyOf: [
|
||||
{ type: "string", allOf: [{ minLength: 2 }] },
|
||||
{ type: "null" as const },
|
||||
],
|
||||
};
|
||||
expect(normalizeNullableUnion(withAllOf)).toBe(withAllOf);
|
||||
const withOneOf = {
|
||||
anyOf: [
|
||||
{ type: "string", oneOf: [{ const: "a" }] },
|
||||
{ type: "null" as const },
|
||||
],
|
||||
};
|
||||
expect(normalizeNullableUnion(withOneOf)).toBe(withOneOf);
|
||||
});
|
||||
|
||||
it("declines when the wrapper carries an applicator the module cannot read", () => {
|
||||
const schema = {
|
||||
allOf: [{ minLength: 2 }],
|
||||
|
|
|
|||
|
|
@ -205,6 +205,26 @@ function hasOpaqueApplicator(schema: NullableUnionSchema): boolean {
|
|||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether a schema composes further constraints this module cannot evaluate —
|
||||
* {@link hasOpaqueApplicator}, **plus a nested `anyOf`**.
|
||||
*
|
||||
* This is the single test every position uses on a schema that is *not* the
|
||||
* union being recognized: the surviving branch, a null branch, and the
|
||||
* `type: [T, "null"]` schema. Each of those was missed independently at some
|
||||
* point — the guard existed but was spelled out inline and drifted between
|
||||
* sites — so they now share one predicate rather than three conditions that
|
||||
* have to be kept in step.
|
||||
*
|
||||
* A nested `anyOf` matters as much as the others because the hoist *drops* it:
|
||||
* the branch's `anyOf` overwrites the wrapper's in the spread and is then
|
||||
* cleared, so `{ anyOf: [{ type: "string", anyOf: [...] }, { type: "null" }] }`
|
||||
* would otherwise widen into an unconstrained nullable string.
|
||||
*/
|
||||
function hasUnevaluatedComposition(schema: NullableUnionSchema): boolean {
|
||||
return schema.anyOf !== undefined || hasOpaqueApplicator(schema);
|
||||
}
|
||||
|
||||
/**
|
||||
* What {@link stripNullEnumMembers} concluded about an `enum`.
|
||||
*
|
||||
|
|
@ -378,12 +398,12 @@ export function admitsNull(schema: NullableUnionSchema): boolean {
|
|||
// const: "x" }` is unsatisfiable, and its own applicators are as opaque
|
||||
// here as the wrapper's.
|
||||
const branchSchema = branch as NullableUnionSchema;
|
||||
// A branch that names null can still admit nothing: `{ type: "null",
|
||||
// const: "x" }` is unsatisfiable, and a nested union or applicator inside
|
||||
// it is as opaque here as one on the wrapper.
|
||||
return (
|
||||
!nullExcludedBySiblings(branchSchema) &&
|
||||
!hasOpaqueApplicator(branchSchema) &&
|
||||
// A nested union inside the branch is as opaque as the others: `{ type:
|
||||
// "null", anyOf: [{ type: "string" }] }` names null and admits nothing.
|
||||
branchSchema.anyOf === undefined
|
||||
!hasUnevaluatedComposition(branchSchema)
|
||||
);
|
||||
}) ?? false
|
||||
);
|
||||
|
|
@ -424,7 +444,14 @@ export function normalizeNullableUnion<T extends NullableUnionSchema>(
|
|||
|
||||
// Hoisting would silently drop a wrapper constraint the branch also
|
||||
// carries, so decline instead; see `branchDropsWrapperConstraint`.
|
||||
if (nullBranch && branch && !wrapperCarriesConstraints(schema)) {
|
||||
// The wrapper may only carry annotations, and the surviving branch may not
|
||||
// compose anything this module cannot evaluate — the hoist would drop it.
|
||||
if (
|
||||
nullBranch &&
|
||||
branch &&
|
||||
!wrapperCarriesConstraints(schema) &&
|
||||
!hasUnevaluatedComposition(branch as NullableUnionSchema)
|
||||
) {
|
||||
// A branch may carry an `enum` and no `type`; JSON Schema allows that, and
|
||||
// an all-string enum is unambiguously a string field. See isStringEnum for
|
||||
// why a non-string enum deliberately does not get the same treatment.
|
||||
|
|
@ -449,8 +476,7 @@ export function normalizeNullableUnion<T extends NullableUnionSchema>(
|
|||
// and the renderers ignore the other applicators — so a compound schema
|
||||
// like `{ type: ["string", "null"], anyOf: [{ const: "a" }] }` would be
|
||||
// widened into an unconstrained string field. Leave it for the JSON editor.
|
||||
schema.anyOf === undefined &&
|
||||
!hasOpaqueApplicator(schema)
|
||||
!hasUnevaluatedComposition(schema)
|
||||
) {
|
||||
const type = schema.type.find((member) => member !== "null");
|
||||
if (isRenderableType(type)) {
|
||||
|
|
|
|||
Loading…
Reference in a new issue