fix: address Copilot review round 13 — cross-keyword contradictions, own-property required check
- rootUnion: keywords the two declarations state SEPARATELY can contradict too.
A merged `const` is now checked against its own `type` and `enum`, so
root `x: { type: "string" }` under branch `x: { const: 1 }` declines
instead of seeding an immutable 1 into a string field. An integer const still
satisfies a `number` type — the one direction JSON Schema widens.
- TUI missingRequiredFields tests `Object.hasOwn` first: an argument legally
named `constructor` resolved to the inherited one and read as supplied, and
the call went out without it.
Signed-off-by: cliffhall <cliff@futurescale.com>
This commit is contained in:
parent
41be9e7aa8
commit
df1e39e623
4 changed files with 105 additions and 6 deletions
|
|
@ -808,6 +808,17 @@ describe("schemaToForm", () => {
|
|||
).toEqual([]);
|
||||
});
|
||||
|
||||
it("does not mistake an inherited property for a supplied argument", () => {
|
||||
// `constructor` is a legal argument name; reading it off the prototype
|
||||
// would report it as present and send the call without it.
|
||||
const schema = {
|
||||
type: "object",
|
||||
properties: { constructor: { type: "string" } },
|
||||
required: ["constructor"],
|
||||
};
|
||||
expect(missingRequiredFields(schema, {})).toEqual(["constructor"]);
|
||||
});
|
||||
|
||||
it("checks the root's own required fields when there is no union", () => {
|
||||
const schema = {
|
||||
type: "object",
|
||||
|
|
|
|||
|
|
@ -462,6 +462,10 @@ export function missingRequiredFields(
|
|||
? base
|
||||
: branches[selectedBranchIndex(base, branches, rawValues)]!.schema;
|
||||
return (effective.required ?? []).filter((name) => {
|
||||
// `hasOwn` first: an argument legally named `constructor` would otherwise
|
||||
// resolve to the inherited one and read as supplied, and the call would go
|
||||
// out without it.
|
||||
if (!Object.hasOwn(decoded, name)) return true;
|
||||
const value = decoded[name];
|
||||
return value === undefined || value === "";
|
||||
});
|
||||
|
|
|
|||
|
|
@ -358,6 +358,46 @@ describe("resolveRootUnion", () => {
|
|||
expect(branches).toHaveLength(2);
|
||||
});
|
||||
|
||||
it("declines a branch whose const its root type rejects", () => {
|
||||
// The two share no keyword, yet nothing satisfies both — and the merged
|
||||
// declaration would seed an immutable `1` into a string field.
|
||||
const { branches } = resolveRootUnion({
|
||||
type: "object",
|
||||
properties: { x: { type: "string" } },
|
||||
anyOf: [
|
||||
{ type: "object", properties: { x: { const: 1 } } },
|
||||
{ type: "object", properties: { other: { type: "string" } } },
|
||||
] as unknown[],
|
||||
});
|
||||
expect(branches).toEqual([]);
|
||||
});
|
||||
|
||||
it("declines a branch whose const its root enum excludes", () => {
|
||||
const { branches } = resolveRootUnion({
|
||||
type: "object",
|
||||
properties: { x: { enum: ["a", "b"] } },
|
||||
anyOf: [
|
||||
{ type: "object", properties: { x: { const: "c" } } },
|
||||
{ type: "object", properties: { other: { type: "string" } } },
|
||||
] as unknown[],
|
||||
});
|
||||
expect(branches).toEqual([]);
|
||||
});
|
||||
|
||||
it("accepts a const its root type and enum admit", () => {
|
||||
const { branches } = resolveRootUnion({
|
||||
type: "object",
|
||||
properties: { x: { type: "number", enum: [1, 2] } },
|
||||
anyOf: [
|
||||
// An integer const satisfies a `number` type — the one direction JSON
|
||||
// Schema widens.
|
||||
{ type: "object", properties: { x: { const: 1 } } },
|
||||
{ type: "object", properties: { other: { type: "string" } } },
|
||||
] as unknown[],
|
||||
});
|
||||
expect(branches).toHaveLength(2);
|
||||
});
|
||||
|
||||
it("declines a union whose branch contradicts the root's type for a field", () => {
|
||||
// `string` under a base `number` describes a value that cannot exist, so
|
||||
// flattening it would render one type and accept what the schema rejects.
|
||||
|
|
|
|||
|
|
@ -224,12 +224,56 @@ function conflicts(baseProperty: unknown, branchProperty: unknown): boolean {
|
|||
}
|
||||
const left = a as Record<string, unknown>;
|
||||
const right = b as Record<string, unknown>;
|
||||
return Object.keys(right).some(
|
||||
(keyword) =>
|
||||
!ANNOTATION_KEYWORDS.has(keyword) &&
|
||||
keyword in left &&
|
||||
!sameValue(left[keyword], right[keyword]),
|
||||
);
|
||||
if (
|
||||
Object.keys(right).some(
|
||||
(keyword) =>
|
||||
!ANNOTATION_KEYWORDS.has(keyword) &&
|
||||
keyword in left &&
|
||||
!sameValue(left[keyword], right[keyword]),
|
||||
)
|
||||
) {
|
||||
return true;
|
||||
}
|
||||
// Keywords the two sides state *separately* can still contradict each other:
|
||||
// root `{ type: "string" }` under branch `{ const: 1 }` shares no keyword at
|
||||
// all, yet nothing satisfies both — and the merged declaration would seed an
|
||||
// immutable `1` into a field the schema rejects.
|
||||
return !constSatisfiesSiblings({ ...left, ...right });
|
||||
}
|
||||
|
||||
/** The JSON type name of a value, in JSON Schema's vocabulary. */
|
||||
function jsonTypeOf(value: unknown): string {
|
||||
if (value === null) return "null";
|
||||
if (Array.isArray(value)) return "array";
|
||||
if (typeof value === "number") {
|
||||
return Number.isInteger(value) ? "integer" : "number";
|
||||
}
|
||||
return typeof value;
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether a merged declaration's `const` is one its own `type` and `enum` admit.
|
||||
*
|
||||
* `const` names the single value the schema accepts, so a sibling that excludes
|
||||
* it leaves nothing satisfiable. An `integer` const satisfies a `number` type,
|
||||
* which is the one direction JSON Schema widens.
|
||||
*/
|
||||
function constSatisfiesSiblings(schema: Record<string, unknown>): boolean {
|
||||
if (!("const" in schema)) return true;
|
||||
const value = schema.const;
|
||||
const { type, enum: allowed } = schema;
|
||||
const actual = jsonTypeOf(value);
|
||||
const admits = (name: unknown) =>
|
||||
name === actual || (name === "number" && actual === "integer");
|
||||
if (typeof type === "string" && !admits(type)) return false;
|
||||
if (Array.isArray(type) && !type.some(admits)) return false;
|
||||
if (
|
||||
Array.isArray(allowed) &&
|
||||
!allowed.some((member) => sameValue(member, value))
|
||||
) {
|
||||
return false;
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
|||
Loading…
Reference in a new issue