diff --git a/.chronus/changes/mictaylor-json-schema-discriminator-circular-ref-fix-2025-10-21-6-44-36.md b/.chronus/changes/mictaylor-json-schema-discriminator-circular-ref-fix-2025-10-21-6-44-36.md new file mode 100644 index 00000000000..eb81b45a43f --- /dev/null +++ b/.chronus/changes/mictaylor-json-schema-discriminator-circular-ref-fix-2025-10-21-6-44-36.md @@ -0,0 +1,7 @@ +--- +changeKind: fix +packages: + - "@typespec/json-schema" +--- + +avoid circular references in discriminated unions with polymorphic-models-strategy \ No newline at end of file diff --git a/packages/json-schema/src/json-schema-emitter.ts b/packages/json-schema/src/json-schema-emitter.ts index ba40f96c1ae..8fd9e715d55 100644 --- a/packages/json-schema/src/json-schema-emitter.ts +++ b/packages/json-schema/src/json-schema-emitter.ts @@ -113,13 +113,22 @@ export class JsonSchemaEmitter extends TypeEmitter, JSONSche return this.#createDiscriminatedUnionDeclaration(model, name, discriminator, strategy); } + // Check if base model is using discriminated union strategy + // If so, don't use allOf (to avoid circular references), inline properties instead + const shouldInlineBase = + model.baseModel && this.#isBaseUsingDiscriminatedUnion(model.baseModel); + const schema = this.#initializeSchema(model, name, { type: "object", - properties: this.emitter.emitModelProperties(model), - required: this.#requiredModelProperties(model), + properties: shouldInlineBase + ? this.#getAllModelProperties(model) + : this.emitter.emitModelProperties(model), + required: shouldInlineBase + ? this.#getAllRequiredModelProperties(model) + : this.#requiredModelProperties(model), }); - if (model.baseModel) { + if (model.baseModel && !shouldInlineBase) { const allOf = new ArrayBuilder(); allOf.push(this.emitter.emitTypeReference(model.baseModel)); setProperty(schema, "allOf", allOf); @@ -131,6 +140,16 @@ export class JsonSchemaEmitter extends TypeEmitter, JSONSche return this.#createDeclaration(model, name, schema); } + #isBaseUsingDiscriminatedUnion(model: Model): boolean { + const discriminator = getDiscriminator(this.emitter.getProgram(), model); + const strategy = this.emitter.getOptions()["polymorphic-models-strategy"]; + return ( + (strategy === "oneOf" || strategy === "anyOf") && + !!discriminator && + model.derivedModels.length > 0 + ); + } + modelLiteral(model: Model): EmitterOutput { const schema = new ObjectBuilder({ type: "object", @@ -187,6 +206,76 @@ export class JsonSchemaEmitter extends TypeEmitter, JSONSche return requiredProps.length > 0 ? requiredProps : undefined; } + // Get all required properties including inherited ones (for when base uses discriminated union) + #getAllRequiredModelProperties(model: Model): string[] | undefined { + const requiredProps: string[] = []; + const visited = new Set(); + + const collectRequired = (m: Model) => { + if (visited.has(m)) return; + visited.add(m); + + // Collect from base first + if (m.baseModel) { + collectRequired(m.baseModel); + } + + // Then collect from this model + for (const prop of m.properties.values()) { + if (!prop.optional && !requiredProps.includes(prop.name)) { + requiredProps.push(prop.name); + } + } + + // Add discriminator property if defined on this model + const discriminator = getDiscriminator(this.emitter.getProgram(), m); + if ( + discriminator && + !m.properties.has(discriminator.propertyName) && + !requiredProps.includes(discriminator.propertyName) + ) { + requiredProps.push(discriminator.propertyName); + } + }; + + collectRequired(model); + return requiredProps.length > 0 ? requiredProps : undefined; + } + + // Get all properties including inherited ones (for when base uses discriminated union) + #getAllModelProperties(model: Model): EmitterOutput { + const props = new ObjectBuilder(); + const visited = new Set(); + + const collectProperties = (m: Model) => { + if (visited.has(m)) return; + visited.add(m); + + // Collect from base first + if (m.baseModel) { + collectProperties(m.baseModel); + } + + // Then collect from this model (overriding base if needed) + for (const [name, prop] of m.properties) { + const result = this.emitter.emitModelProperty(prop); + setProperty(props, name, result); + } + + // Add discriminator property if defined on this model and not already added + const discriminator = getDiscriminator(this.emitter.getProgram(), m); + if (discriminator && !(discriminator.propertyName in props)) { + setProperty(props, discriminator.propertyName, { + type: "string", + description: `Discriminator property for ${m.name}.`, + }); + } + }; + + collectProperties(model); + return props; + } + modelProperties(model: Model): EmitterOutput { const props = new ObjectBuilder(); @@ -697,7 +786,9 @@ export class JsonSchemaEmitter extends TypeEmitter, JSONSche variants.push(derivedRef); } - // Include base model properties and the discriminated union + // For discriminated unions, emit the oneOf/anyOf with base model properties + // Since derived models inline properties (not using allOf), we can include + // base properties here without creating circular references const schema = this.#initializeSchema(model, name, { type: "object", properties: this.emitter.emitModelProperties(model), diff --git a/packages/json-schema/test/discriminator.test.ts b/packages/json-schema/test/discriminator.test.ts index 52a8264063f..246f6499c3d 100644 --- a/packages/json-schema/test/discriminator.test.ts +++ b/packages/json-schema/test/discriminator.test.ts @@ -232,10 +232,10 @@ describe("discriminated union with polymorphic-models-strategy option", () => { deepStrictEqual(petSchema.oneOf[0], { $ref: "Cat.json" }); deepStrictEqual(petSchema.oneOf[1], { $ref: "Dog.json" }); - // Should have base model properties along with oneOf + // Pet schema should contain both oneOf and base properties strictEqual(petSchema.type, "object"); - ok(petSchema.properties, "Should have properties"); - strictEqual(petSchema.properties.name.type, "string"); + ok(petSchema.properties, "Pet should have properties"); + strictEqual(petSchema.properties.name.type, "string", "Pet should have name property"); deepStrictEqual(petSchema.properties.kind, { type: "string", description: "Discriminator property for Pet.", @@ -271,13 +271,17 @@ describe("discriminated union with polymorphic-models-strategy option", () => { ok(catSchema, "Cat schema should exist"); strictEqual(catSchema.properties.kind.const, "cat"); strictEqual(catSchema.properties.meow.type, "integer"); - deepStrictEqual(catSchema.allOf, [{ $ref: "Pet.json" }]); + // Cat should include base properties (name) inline to avoid circular refs + strictEqual(catSchema.properties.name.type, "string"); + strictEqual(catSchema.allOf, undefined, "Should not have allOf to avoid circular refs"); const dogSchema = schemas["Dog.json"]; ok(dogSchema, "Dog schema should exist"); strictEqual(dogSchema.properties.kind.const, "dog"); strictEqual(dogSchema.properties.bark.type, "string"); - deepStrictEqual(dogSchema.allOf, [{ $ref: "Pet.json" }]); + // Dog should include base properties (name) inline to avoid circular refs + strictEqual(dogSchema.properties.name.type, "string"); + strictEqual(dogSchema.allOf, undefined, "Should not have allOf to avoid circular refs"); }); it("works with multiple levels of inheritance", async () => { @@ -308,7 +312,9 @@ describe("discriminated union with polymorphic-models-strategy option", () => { const dogSchema = schemas["Dog.json"]; ok(dogSchema, "Dog schema should exist"); - deepStrictEqual(dogSchema.allOf, [{ $ref: "Pet.json" }]); + // Dog should inline Pet properties, not use allOf + strictEqual(dogSchema.properties.name.type, "string"); + strictEqual(dogSchema.allOf, undefined, "Should not have allOf to avoid circular refs"); }); it("does not emit oneOf when option is ignore", async () => { @@ -381,10 +387,10 @@ describe("discriminated union with polymorphic-models-strategy option", () => { deepStrictEqual(petSchema.anyOf[0], { $ref: "Cat.json" }); deepStrictEqual(petSchema.anyOf[1], { $ref: "Dog.json" }); - // Should have base model properties along with anyOf + // Pet schema should contain both anyOf and base properties strictEqual(petSchema.type, "object"); - ok(petSchema.properties, "Should have properties"); - strictEqual(petSchema.properties.name.type, "string"); + ok(petSchema.properties, "Pet should have properties"); + strictEqual(petSchema.properties.name.type, "string", "Pet should have name property"); deepStrictEqual(petSchema.properties.kind, { type: "string", description: "Discriminator property for Pet.",