diff --git a/packages/downgrader/src/v3.1-to-v3.0.ts b/packages/downgrader/src/v3.1-to-v3.0.ts index 207c925..2d99ccb 100644 --- a/packages/downgrader/src/v3.1-to-v3.0.ts +++ b/packages/downgrader/src/v3.1-to-v3.0.ts @@ -141,6 +141,9 @@ const refTargets = new Set() // holds it, also once the target it was cut at is no longer being inlined. const cutHolders = new Set() +// Pairs of null and a `$ref` that became the `$ref` alone, with their branches. +const collapsedPairs: [schema: Record, anyOf: unknown[]][] = [] + // The 3.0 keywords that apply only to objects, which `convertType` moves into // the `"object"` branch of a `type` list. const OBJECT_KEYWORDS = [ @@ -958,47 +961,6 @@ function admitsNull(schema: unknown): boolean { ) } -// A `$ref` whose target admits null, which a pair of null and the `$ref` -// becomes. A target that holds such a pair, beside annotations that 3.0 may -// drop, such as `$comment`, can become its own `$ref` in turn, so a loop of -// them, which in 3.1 admits only null, would become `$ref`s that match -// nothing. One that does not admit null keeps its pair. A pair nested in the -// target's `anyOf`, as `anyOf: [{ anyOf: [$ref, null] }, null]`, leads on too. -function refAdmitsNull(ref: string, ctx: Context): boolean { - const seen = new Set() - for (let next: string | undefined = ref; next !== undefined;) { - const target = resolvePointer(next, ctx) - if (!admitsNull(target)) { - return next !== ref - } - if (seen.has(next)) { - return false - } - seen.add(next) - const { anyOf } = target as Record - const others = Array.isArray(anyOf) ? nonNullBranches(anyOf) : [] - next = others.length === 1 ? getRef(others[0]) : undefined - } - return true -} - -// Met again, around a cycle of objects, a union adds nothing. -function nonNullBranches(branches: unknown[], seen = new Set()): unknown[] { - if (seen.has(branches)) { - return [] - } - seen.add(branches) - return branches.flatMap((branch) => { - if (acceptsNull(branch)) { - return [] - } - const nested = isRecord(branch) ? (branch.anyOf ?? branch.oneOf) : undefined - return Array.isArray(nested) && nested.some(acceptsNull) - ? nonNullBranches(nested, seen) - : [branch] - }) -} - // A `type` with no `nullable: true` beside it, in 3.1 or 3.0, rejects null. function rejectsNull(schema: unknown): boolean { if (!isRecord(schema) || schema.nullable === true) { @@ -1219,11 +1181,16 @@ function mergeNullable( const nullBranch = branches.find((branch) => isNullBranch(branch, out)) out.oneOf = branches[0] === other ? [other, nullBranch] : [nullBranch, other] delete out.anyOf - } else if (ref !== undefined && Object.keys(out).length === 1 && refAdmitsNull(ref, ctx)) { + } else if ( + ref !== undefined && + Object.keys(out).length === 1 && + admitsNull(resolvePointer(ref, ctx)) + ) { // GitHub writes a nullable reference this way to a schema that already // admits null, and Zod and Pydantic to a nullable one, which 3.0 writes as // the `$ref` alone. NSwag reads the pair as an empty class, and // oapi-codegen as a union. + collapsedPairs.push([out, out.anyOf as unknown[]]) delete out.anyOf out.$ref = ref } @@ -1769,6 +1736,23 @@ function convertDocument(value: unknown, ctx: Context): unknown { return convertObject(value, ctx, DOCUMENT_FIELDS, finishDocument) } +// A chain of bare `$ref`s from `schema` that comes back to it. +function leadsBack(schema: Record, ctx: Context): boolean { + const seen = new Set() + for (let next: unknown = schema; isRecord(next) && Object.keys(next).length === 1;) { + const ref = getRef(next) + if (ref === undefined || seen.has(next)) { + return false + } + seen.add(next) + next = resolvePointer(ref, ctx) + if (next === schema) { + return true + } + } + return false +} + function downgradeV30(root: unknown, convert: Convert): unknown { try { const ctx = createContext(root) @@ -1787,6 +1771,15 @@ function downgradeV30(root: unknown, convert: Convert): unknown { } } } + // A loop of such `$ref`s, which in 3.1 admits only null, would match + // nothing, so each keeps its pair. + if (collapsedPairs.length > 0) { + const outCtx = createContext(out) + for (const [schema, anyOf] of collapsedPairs.filter(([item]) => leadsBack(item, outCtx))) { + delete schema.$ref + schema.anyOf = anyOf + } + } return out } finally { // Cleared even when conversion throws, so nothing keeps the input alive. @@ -1794,6 +1787,7 @@ function downgradeV30(root: unknown, convert: Convert): unknown { unionTargets.clear() refTargets.clear() cutHolders.clear() + collapsedPairs.length = 0 LOOSE.clear() } } diff --git a/packages/downgrader/tests/v3.1-to-v3.0.test.ts b/packages/downgrader/tests/v3.1-to-v3.0.test.ts index 6d95410..73889a3 100644 --- a/packages/downgrader/tests/v3.1-to-v3.0.test.ts +++ b/packages/downgrader/tests/v3.1-to-v3.0.test.ts @@ -1339,8 +1339,9 @@ describe('downgradeSchemaV31ToV30', () => { A: { anyOf: [ref('B'), onlyNull] }, B: { anyOf: [ref('A'), onlyNull] }, }) + // One that leads into such a loop, without being on it, keeps the `$ref`. expect(schemas({ A: pair('B'), B: pair('C'), C: pair('B') })).toEqual({ - A: { anyOf: [ref('B'), onlyNull] }, + A: ref('B'), B: { anyOf: [ref('C'), onlyNull] }, C: { anyOf: [ref('B'), onlyNull] }, }) @@ -1370,12 +1371,13 @@ describe('downgradeSchemaV31ToV30', () => { A: { anyOf: [ref('B'), onlyNull] }, B: { anyOf: [ref('A'), onlyNull] }, }) + // A description that 3.0 keeps beside the pair breaks the loop. expect(schemas({ A: pair('B'), B: { description: 'B', ...pair('A') } })).toEqual({ - A: { anyOf: [ref('B'), onlyNull] }, + A: ref('B'), B: { description: 'B', anyOf: [ref('A'), onlyNull] }, }) - // So does a pair nested in the target's union. - const nested = { anyOf: [{ anyOf: [ref('A'), onlyNull] }, onlyNull] } + // So does a pair nested in the target's union, which keeps one pair. + const nested = { anyOf: [ref('A'), onlyNull] } expect(schemas({ A: { anyOf: [pair('A'), { type: 'null' }] } })).toEqual({ A: nested }) expect(schemas({ A: { anyOf: [{ oneOf: pair('A').anyOf }, { type: 'null' }] } })).toEqual({ A: nested, @@ -1390,13 +1392,17 @@ describe('downgradeSchemaV31ToV30', () => { A: ref('B'), B: { anyOf: [{ not: {} }, ref('A'), onlyNull] }, }) - // A cycle of objects ends the walk. - const cyclic: Record = { anyOf: [{ type: 'null' }] } - cyclic.anyOf.unshift({ anyOf: [cyclic, { type: 'null' }] }) - expect(schemas({ A: pair('B'), B: cyclic })?.A).toEqual(ref('B')) - // A $ref beside the cycle is still followed. - cyclic.anyOf[0].anyOf.push(ref('A')) - expect(schemas({ A: pair('B'), B: cyclic })?.A).toEqual({ anyOf: [ref('B'), onlyNull] }) + // As does a loop through a nested pair, or a duplicate `$ref`, or null + // written as `const`. + expect( + schemas({ + A: pair('B'), + B: { anyOf: [{ anyOf: [ref('A'), { const: null }] }, { type: 'null' }] }, + }), + ).toEqual({ A: { anyOf: [ref('B'), onlyNull] }, B: { anyOf: [ref('A'), onlyNull] } }) + expect(schemas({ A: { anyOf: [ref('A'), ref('A'), { type: 'null' }] } })).toEqual({ + A: { anyOf: [ref('A'), onlyNull] }, + }) }) it('merges a branch in full where a cycle meets it before its conversion is done', () => {