fix(downgrader): loosen a oneOf or not that reaches a lost restriction through a kept $ref in 3.1 to 3.0

packages/downgrader/README.md

@@ -197,8 +197,8 @@ Accepts the default OAS dialect, which adds `discriminator`, `xml`, `externalDoc
 | An empty [`enum`][js-enum], which 3.0 forbids                                                                                                                                                                     | `allOf: [{ not: {} }]`, which also rejects every value.                                                                                                                                                        |
 | Keywords that 3.0 does not define                                                                                                                                                                                 | `x-` extensions of the same name, since the 3.0 Schema Object allows no others. An existing extension of that name wins.                                                                                       |
 | [`nullable`][3.0-schema-nullable], a 3.0 keyword                                                                                                                                                                  | Kept beside a single `type`, where it means in 3.0 what its author meant, and the schema counts as having lost a restriction. Removed otherwise.                                                               |
-| [`not`][js-not] over a schema that lost a restriction above                                                                                                                                                       | Removed. Negating the looser schema would reject values the original accepts. A keyword that restricts nothing, such as `propertyNames: { type: "string" }`, or a tuple converted exactly, does not count.     |
-| [`oneOf`][js-one-of] with a branch that lost a restriction above                                                                                                                                                  | `anyOf`. Looser branches may overlap, and then match more than one.                                                                                                                                            |
+| [`not`][js-not] over a schema that lost a restriction above, even through a `$ref`                                                                                                                                | Removed. Negating the looser schema would reject values the original accepts. A keyword that restricts nothing, such as `propertyNames: { type: "string" }`, or a tuple converted exactly, does not count.     |
+| [`oneOf`][js-one-of] with a branch that lost a restriction above, even through a `$ref`                                                                                                                           | `anyOf`, even where `const` tags keep the branches apart. Looser branches may overlap, and then match more than one.                                                                                           |
 
 [js-type]: https://json-schema.org/draft/2020-12/json-schema-validation#name-type
 [js-const]: https://json-schema.org/draft/2020-12/json-schema-validation#name-const
@@ -236,7 +236,7 @@ Accepts the default OAS dialect, which adds `discriminator`, `xml`, `externalDoc
 
 #### Limitations
 
-- A `not` or `oneOf` that reaches a schema that lost a restriction through a `$ref` kept in the output, such as one to `components.schemas`, or around a cycle of objects, can still reject values the original accepts.
+- Around a cycle, a `$ref` or object leading back into a schema still being converted does not count what that schema lost, so a `not` or `oneOf` over it can still reject values the original accepts. Which `$ref` or object misses it depends on which schema of the cycle converts first.
 - `$ref`s to an `$anchor`, or written relative to an `$id`, are left as written and dangle. Every `$ref` is read as a JSON Pointer from the document root, and one into `$defs` is recognized by its `/$defs/` segment.
 - Recursion through `$defs` is cut to `{}` after one level. Where several `$ref`s enter the same cycle, the first copy made is reused, so a later one can be cut sooner.
 - Older drafts are not supported. `definitions` is read as `$defs`, but other keywords that only draft-07 or 2019-09 define, such as `dependencies` and `additionalItems`, become extensions unconverted, so `$ref`s into them dangle, and array-form `items` passes through. Convert such schemas to 2020-12 first.

packages/downgrader/src/v3.1-to-v3.0.ts

@@ -151,10 +151,13 @@ const SCHEMA_FIELDS = defineFields({
   required: (item) => (Array.isArray(item) && item.length === 0 ? DROP : clone(item)),
 })
 
+// A bare `$ref` kept as written, copied as it is.
+const KEPT_REF_FIELDS = defineFields({})
+
 const PARAMETER_FIELDS = defineFields({
   content: convertContent,
   examples: map(convertReference),
-  schema: convertSchema,
+  schema: convertTopSchema,
 })
 
 const ENCODING_FIELDS = defineFields({
@@ -164,7 +167,7 @@ const ENCODING_FIELDS = defineFields({
 const MEDIA_TYPE_FIELDS = defineFields({
   encoding: map((item, ctx) => convertObject(item, ctx, ENCODING_FIELDS)),
   examples: map(convertReference),
-  schema: convertSchema,
+  schema: convertTopSchema,
 })
 
 const REQUEST_BODY_FIELDS = defineFields({
@@ -202,7 +205,7 @@ const COMPONENTS_FIELDS = defineFields({
   pathItems: DROP,
   requestBodies: map(refOr(convertRequestBody)),
   responses: map(refOr(convertResponse)),
-  schemas: map(convertSchema),
+  schemas: map(convertTopSchema),
   securitySchemes: map((item, ctx) =>
     schemeType(item, ctx) === 'mutualTLS' ? DROP : convertReference(item, ctx),
   ),
@@ -256,14 +259,44 @@ function refOr(convert: Convert): Convert {
   return self
 }
 
+// A `$ref` kept as written accepts whatever its target does, so it lost a
+// restriction if the converted target did. The target converts once, for
+// `components.schemas` too, but one still being converted around a cycle has
+// not lost any yet.
+function hasLooseTarget(ref: string, ctx: Context): boolean {
+  const target = resolvePointer(ref, ctx)
+  if (!isRecord(target)) {
+    return false
+  }
+  // `convertSchema` makes a new object for each bare `$ref`, so one kept as
+  // written converts here instead, cached like any other object, so that a
+  // chain of them is followed once, and a cycle of them ends.
+  const next = getRef(target)
+  return isLoose(
+    next !== undefined && isBareRef(target) && !isInlined(next, ctx)
+      ? convertObject(target, ctx, KEPT_REF_FIELDS, finishKeptRef)
+      : convertSchema(target, ctx),
+  )
+}
+
 function convertSchemaRef(ref: string, ctx: Context): unknown {
   if (!isInlined(ref, ctx)) {
-    return { $ref: ref }
+    return hasLooseTarget(ref, ctx) ? loosened({ $ref: ref }) : { $ref: ref }
   }
   const out = inline(ref, ctx, convertSchema)
   return out === DROP ? loosened({}) : out
 }
 
+function finishKeptRef(
+  out: Record<string, unknown>,
+  schema: Record<string, unknown>,
+  ctx: Context,
+): void {
+  if (hasLooseTarget(schema.$ref as string, ctx)) {
+    LOOSE.add(out)
+  }
+}
+
 // 3.0 `items` applies one schema to every item, so a tuple becomes an array
 // whose items match any of its item schemas, and those of the items after
 // them, unless `items: false` or `maxItems` allows none. `items: false`
@@ -543,6 +576,15 @@ function convertSchema(value: unknown, ctx: Context): unknown {
   return convertObject(value, ctx, SCHEMA_FIELDS, finishSchema)
 }
 
+// Whether a schema lost a restriction matters only to a schema holding it, so
+// a bare `$ref` kept as written where none does skips converting its target.
+function convertTopSchema(value: unknown, ctx: Context): unknown {
+  const ref = getRef(value)
+  return ref !== undefined && isBareRef(value as Record<string, unknown>) && !isInlined(ref, ctx)
+    ? { $ref: ref }
+    : convertSchema(value, ctx)
+}
+
 function convertParameter(value: unknown, ctx: Context): unknown {
   return convertObject(value, ctx, PARAMETER_FIELDS)
 }
@@ -646,7 +688,7 @@ export function downgradeSpecV31ToV30(spec: OpenAPIV3_1.OpenAPIObject): OpenAPIV
 export function downgradeSchemaV31ToV30<T = unknown>(
   schema: OpenAPIV3_1.SchemaObject<T>,
 ): OpenAPIV3_0.ReferenceObject | OpenAPIV3_0.SchemaObject<T> {
-  return downgrade(schema, convertSchema) as
+  return downgrade(schema, convertTopSchema) as
     | OpenAPIV3_0.ReferenceObject
     | OpenAPIV3_0.SchemaObject<T>
 }

packages/downgrader/tests/v3.1-to-v3.0.test.ts

@@ -789,6 +789,107 @@ describe('downgradeSchemaV31ToV30', () => {
         }),
       ).toEqual({})
     })
+
+    describe('through a $ref kept in the output', () => {
+      const ref = (name: string) => ({ $ref: `#/components/schemas/${name}` })
+      // As oRPC emits a z.tuple of two items: in 3.0 each item matches either type.
+      const pair = (first: string, second: string) =>
+        ({
+          type: 'array',
+          prefixItems: [{ type: first }, { type: second }],
+          items: false,
+          minItems: 2,
+          maxItems: 2,
+        }) as OpenAPIV3_1.SchemaObject
+
+      it('includes a named tuple or map whose items lost one', () => {
+        // As oRPC emits z.xor over named z.tuples, or over named z.maps.
+        const map = (key: string, value: string) =>
+          ({
+            'type': 'array',
+            'items': {
+              type: 'array',
+              prefixItems: [{ type: key }, { type: value }],
+              maxItems: 2,
+              minItems: 2,
+            },
+            'x-native-type': 'map',
+          }) as OpenAPIV3_1.SchemaObject
+        const out = convert({
+          components: {
+            schemas: {
+              Tuples: { oneOf: [ref('StrNum'), ref('NumStr')] },
+              Maps: { oneOf: [ref('StrNumMap'), ref('NumStrMap')] },
+              NotTuple: { not: { ...ref('StrNum'), description: 'Not a pair' } },
+              Points: { oneOf: [ref('Point'), { type: 'string' }] },
+              StrNum: pair('string', 'number'),
+              NumStr: pair('number', 'string'),
+              StrNumMap: map('string', 'number'),
+              NumStrMap: map('number', 'string'),
+              Point: pair('number', 'number'),
+            },
+          },
+        })
+        const schemas = out.components?.schemas ?? {}
+        expect(schemas.Tuples).toEqual({ anyOf: [ref('StrNum'), ref('NumStr')] })
+        expect(schemas.Maps).toEqual({ anyOf: [ref('StrNumMap'), ref('NumStrMap')] })
+        expect(schemas.NotTuple).toEqual({})
+        // A tuple converted exactly lost nothing.
+        expect(schemas.Points).toEqual({ oneOf: [ref('Point'), { type: 'string' }] })
+      })
+
+      it('includes a discriminated union over named branches, as it does inline', () => {
+        // Its const tags keep the branches exclusive, so anyOf means the same.
+        const branch = (kind: string, first: string, second: string) =>
+          ({
+            type: 'object',
+            properties: { kind: { type: 'string', const: kind }, pair: pair(first, second) },
+            required: ['kind', 'pair'],
+          }) as OpenAPIV3_1.SchemaObject
+        const a = branch('a', 'string', 'number')
+        const b = branch('b', 'number', 'string')
+        const out = convert({
+          components: {
+            schemas: {
+              Named: { oneOf: [ref('A'), ref('B')] },
+              Inline: { oneOf: [a, b] },
+              A: a,
+              B: b,
+            },
+          },
+        })
+        const { A, B, Inline, Named } = out.components?.schemas ?? {}
+        expect(Named).toEqual({ anyOf: [ref('A'), ref('B')] })
+        expect(Inline).toEqual({ anyOf: [A, B] })
+      })
+
+      it('follows a chain of bare $refs, and ends a cycle of them', () => {
+        const out = convert({
+          components: {
+            schemas: {
+              Chain: { oneOf: [ref('Alias'), { type: 'string' }] },
+              Alias: ref('AliasOfAlias'),
+              AliasOfAlias: ref('StrNum'),
+              StrNum: pair('string', 'number'),
+              Cycle: { oneOf: [ref('A'), ref('Self')], not: ref('B') },
+              A: ref('B'),
+              B: ref('A'),
+              Self: ref('Self'),
+            },
+          },
+        })
+        const schemas = out.components?.schemas ?? {}
+        expect(schemas.Chain).toEqual({ anyOf: [ref('Alias'), { type: 'string' }] })
+        expect(schemas.Cycle).toEqual({ oneOf: [ref('A'), ref('Self')], not: ref('B') })
+        expect([schemas.Alias, schemas.AliasOfAlias, schemas.A, schemas.B, schemas.Self]).toEqual([
+          ref('AliasOfAlias'),
+          ref('StrNum'),
+          ref('B'),
+          ref('A'),
+          ref('Self'),
+        ])
+      })
+    })
   })
 
   it('drops an empty required, which 3.0 forbids', () => {