diff --git a/.changeset/hotfix-unions-handling-2024-1-8-1-20-37.md b/.changeset/hotfix-unions-handling-2024-1-8-1-20-37.md new file mode 100644 index 0000000000..fb0afab5e9 --- /dev/null +++ b/.changeset/hotfix-unions-handling-2024-1-8-1-20-37.md @@ -0,0 +1,7 @@ +--- +"@typespec/compiler": patch +"@typespec/json-schema": patch +"@typespec/openapi3": patch +--- + +Allow using default values for union property types diff --git a/.changeset/hotfix-unions-handling-2024-1-8-1-20-38.md b/.changeset/hotfix-unions-handling-2024-1-8-1-20-38.md new file mode 100644 index 0000000000..a53733a0bc --- /dev/null +++ b/.changeset/hotfix-unions-handling-2024-1-8-1-20-38.md @@ -0,0 +1,5 @@ +--- +"@typespec/openapi3": patch +--- + +Fix: union of primitive types that gets emitted as an `enum` keeps the description diff --git a/packages/compiler/src/core/checker.ts b/packages/compiler/src/core/checker.ts index 420bf9f829..42d8da6db2 100644 --- a/packages/compiler/src/core/checker.ts +++ b/packages/compiler/src/core/checker.ts @@ -3470,6 +3470,9 @@ export function createChecker(program: Program): Checker { const [valid] = isStringTemplateSerializable(type); return valid; } + if (type.kind === "UnionVariant") { + return isValueType(type.type); + } const valueTypes = new Set(["String", "Number", "Boolean", "EnumMember", "Tuple"]); return valueTypes.has(type.kind); } diff --git a/packages/json-schema/src/json-schema-emitter.ts b/packages/json-schema/src/json-schema-emitter.ts index 760547b246..6759e565bd 100644 --- a/packages/json-schema/src/json-schema-emitter.ts +++ b/packages/json-schema/src/json-schema-emitter.ts @@ -217,6 +217,8 @@ export class JsonSchemaEmitter extends TypeEmitter, JSONSche }); case "EnumMember": return defaultType.value ?? defaultType.name; + case "UnionVariant": + return this.#getDefaultValue(type, defaultType.type); default: reportDiagnostic(program, { code: "invalid-default", diff --git a/packages/json-schema/test/models.test.ts b/packages/json-schema/test/models.test.ts index d29f3985f7..ec3ceea650 100644 --- a/packages/json-schema/test/models.test.ts +++ b/packages/json-schema/test/models.test.ts @@ -323,5 +323,25 @@ describe("emitting models", () => { default: true, }); }); + + it("specify default value on union with variant", async () => { + const res = await emitSchema( + ` + model Foo { + optionalUnion?: MyUnion = MyUnion.a; + }; + + union MyUnion { + a: "a-value", + b: "b-value", + } + ` + ); + + deepStrictEqual(res["Foo.json"].properties.optionalUnion, { + $ref: "MyUnion.json", + default: "a-value", + }); + }); }); }); diff --git a/packages/json-schema/test/unions.test.ts b/packages/json-schema/test/unions.test.ts index 2f34dc1027..244fe2d63d 100644 --- a/packages/json-schema/test/unions.test.ts +++ b/packages/json-schema/test/unions.test.ts @@ -17,6 +17,18 @@ describe("emitting unions", () => { assert.deepStrictEqual(Foo.anyOf, [{ type: "string" }, { type: "boolean" }]); }); + it("union description carries over", async () => { + const schemas = await emitSchema(` + /** Foo doc */ + union Foo { + x: string; + y: boolean; + } + `); + + assert.strictEqual(schemas["Foo.json"].description, "Foo doc"); + }); + it("works with declarations with anonymous variants", async () => { const schemas = await emitSchema(` union Foo { diff --git a/packages/openapi3/src/openapi.ts b/packages/openapi3/src/openapi.ts index bef13007b3..8f85bc6134 100644 --- a/packages/openapi3/src/openapi.ts +++ b/packages/openapi3/src/openapi.ts @@ -23,11 +23,9 @@ import { getSummary, ignoreDiagnostics, interpolatePath, - isArrayModelType, isDeprecated, isGlobalNamespace, isNeverType, - isNullType, isSecret, isVoidType, listServices, @@ -89,7 +87,7 @@ import { buildVersionProjections } from "@typespec/versioning"; import { stringify } from "yaml"; import { getRef } from "./decorators.js"; import { FileType, OpenAPI3EmitterOptions, reportDiagnostic } from "./lib.js"; -import { OpenAPI3SchemaEmitter } from "./schema-emitter.js"; +import { getDefaultValue, OpenAPI3SchemaEmitter } from "./schema-emitter.js"; import { OpenAPI3Document, OpenAPI3Header, @@ -302,7 +300,7 @@ function createOAPIEmitter( } const variable: OpenAPI3ServerVariable = { - default: prop.default ? getDefaultValue(prop.type, prop.default) : "", + default: prop.default ? getDefaultValue(program, prop.type, prop.default) : "", description: getDoc(program, prop), }; @@ -1129,7 +1127,7 @@ function createOAPIEmitter( } const schema = applyEncoding(param, applyIntrinsicDecorators(param, typeSchema)); if (param.default) { - schema.default = getDefaultValue(param.type, param.default); + schema.default = getDefaultValue(program, param.type, param.default); } // Description is already provided in the parameter itself. delete schema.description; @@ -1323,49 +1321,6 @@ function createOAPIEmitter( return callSchemaEmitter(type, visibility) as any; } - function getDefaultValue(type: Type, defaultType: Type): any { - switch (defaultType.kind) { - case "String": - return defaultType.value; - case "Number": - return defaultType.value; - case "Boolean": - return defaultType.value; - case "Tuple": - compilerAssert( - type.kind === "Tuple" || (type.kind === "Model" && isArrayModelType(program, type)), - "setting tuple default to non-tuple value" - ); - - if (type.kind === "Tuple") { - return defaultType.values.map((defaultTupleValue, index) => - getDefaultValue(type.values[index], defaultTupleValue) - ); - } else { - return defaultType.values.map((defaultTuplevalue) => - getDefaultValue(type.indexer!.value, defaultTuplevalue) - ); - } - - case "Intrinsic": - return isNullType(defaultType) - ? null - : reportDiagnostic(program, { - code: "invalid-default", - format: { type: defaultType.kind }, - target: defaultType, - }); - case "EnumMember": - return defaultType.value ?? defaultType.name; - default: - reportDiagnostic(program, { - code: "invalid-default", - format: { type: defaultType.kind }, - target: defaultType, - }); - } - } - function attachExtensions(program: Program, type: Type, emitObject: any) { // Attach any OpenAPI extensions const extensions = getExtensions(program, type); diff --git a/packages/openapi3/src/schema-emitter.ts b/packages/openapi3/src/schema-emitter.ts index 992b7ee805..32a7d54994 100644 --- a/packages/openapi3/src/schema-emitter.ts +++ b/packages/openapi3/src/schema-emitter.ts @@ -347,7 +347,7 @@ export class OpenAPI3SchemaEmitter extends TypeEmitter< // Apply decorators on the property to the type's schema const additionalProps: Partial = this.#applyConstraints(prop, {}); if (prop.default) { - additionalProps.default = this.#getDefaultValue(prop.type, prop.default); + additionalProps.default = getDefaultValue(program, prop.type, prop.default); } if (isReadonlyProperty(program, prop)) { @@ -453,11 +453,11 @@ export class OpenAPI3SchemaEmitter extends TypeEmitter< return this.#createDeclaration(union, name, schema); } - #unionSchema(union: Union) { + #unionSchema(union: Union): ObjectBuilder { const program = this.emitter.getProgram(); if (union.variants.size === 0) { reportDiagnostic(program, { code: "empty-union", target: union }); - return {}; + return new ObjectBuilder({}); } const variants = Array.from(union.variants.values()); const literalVariantEnumByType: Record = {}; @@ -496,7 +496,7 @@ export class OpenAPI3SchemaEmitter extends TypeEmitter< // This union is equivalent to just `null` but OA3 has no way to specify // null as a value, so we throw an error. reportDiagnostic(program, { code: "union-null", target: union }); - return {}; + return new ObjectBuilder({}); } else { // completely empty union can maybe only happen with bugs? compilerAssert(false, "Attempting to emit an empty union"); @@ -505,33 +505,41 @@ export class OpenAPI3SchemaEmitter extends TypeEmitter< if (schemaMembers.length === 1) { // we can just return the single schema member after applying nullable - let schema = schemaMembers[0].schema; + const schema = schemaMembers[0].schema; const type = schemaMembers[0].type; + const additionalProps: Partial = this.#applyConstraints(union, {}); if (nullable) { - if (schema instanceof Placeholder || "$ref" in schema) { - // but we can't make a ref "nullable", so wrap in an allOf (for models) - // or oneOf (for all other types) + additionalProps.nullable = true; + } + + if (Object.keys(additionalProps).length === 0) { + return new ObjectBuilder(schema); + } else { + if ( + (schema instanceof Placeholder || "$ref" in schema) && + !(type && shouldInline(program, type)) + ) { if (type && type.kind === "Model") { - if (shouldInline(program, type)) { - const merged = new ObjectBuilder(schema); - merged.set("nullable", true); - return merged; - } else { - return B.object({ type: "object", allOf: B.array([schema]), nullable: true }); - } + return new ObjectBuilder({ + type: "object", + allOf: B.array([schema]), + ...additionalProps, + }); } else { - return B.object({ oneOf: B.array([schema]), nullable: true }); + return new ObjectBuilder({ oneOf: B.array([schema]), ...additionalProps }); } } else { - schema = { ...schema, nullable: true }; + const merged = new ObjectBuilder(schema); + for (const [key, value] of Object.entries(additionalProps)) { + merged.set(key, value); + } + return merged; } } - - return schema; } - const schema: any = { + const schema: OpenAPI3Schema = { [ofType]: schemaMembers.map((m) => m.schema), }; @@ -593,51 +601,6 @@ export class OpenAPI3SchemaEmitter extends TypeEmitter< } } - #getDefaultValue(type: Type, defaultType: Type): any { - const program = this.emitter.getProgram(); - - switch (defaultType.kind) { - case "String": - return defaultType.value; - case "Number": - return defaultType.value; - case "Boolean": - return defaultType.value; - case "Tuple": - compilerAssert( - type.kind === "Tuple" || (type.kind === "Model" && isArrayModelType(program, type)), - "setting tuple default to non-tuple value" - ); - - if (type.kind === "Tuple") { - return defaultType.values.map((defaultTupleValue, index) => - this.#getDefaultValue(type.values[index], defaultTupleValue) - ); - } else { - return defaultType.values.map((defaultTuplevalue) => - this.#getDefaultValue(type.indexer!.value, defaultTuplevalue) - ); - } - - case "Intrinsic": - return isNullType(defaultType) - ? null - : reportDiagnostic(program, { - code: "invalid-default", - format: { type: defaultType.kind }, - target: defaultType, - }); - case "EnumMember": - return defaultType.value ?? defaultType.name; - default: - reportDiagnostic(program, { - code: "invalid-default", - format: { type: defaultType.kind }, - target: defaultType, - }); - } - } - reference( targetDeclaration: Declaration>, pathUp: Scope>[], @@ -973,3 +936,48 @@ const B = { return builder; }, } as const; + +export function getDefaultValue(program: Program, type: Type, defaultType: Type): any { + switch (defaultType.kind) { + case "String": + return defaultType.value; + case "Number": + return defaultType.value; + case "Boolean": + return defaultType.value; + case "Tuple": + compilerAssert( + type.kind === "Tuple" || (type.kind === "Model" && isArrayModelType(program, type)), + "setting tuple default to non-tuple value" + ); + + if (type.kind === "Tuple") { + return defaultType.values.map((defaultTupleValue, index) => + getDefaultValue(program, type.values[index], defaultTupleValue) + ); + } else { + return defaultType.values.map((defaultTuplevalue) => + getDefaultValue(program, type.indexer!.value, defaultTuplevalue) + ); + } + + case "Intrinsic": + return isNullType(defaultType) + ? null + : reportDiagnostic(program, { + code: "invalid-default", + format: { type: defaultType.kind }, + target: defaultType, + }); + case "EnumMember": + return defaultType.value ?? defaultType.name; + case "UnionVariant": + return getDefaultValue(program, type, defaultType.type); + default: + reportDiagnostic(program, { + code: "invalid-default", + format: { type: defaultType.kind }, + target: defaultType, + }); + } +} diff --git a/packages/openapi3/test/models.test.ts b/packages/openapi3/test/models.test.ts index 1230cddf4b..a2052829db 100644 --- a/packages/openapi3/test/models.test.ts +++ b/packages/openapi3/test/models.test.ts @@ -204,6 +204,33 @@ describe("openapi3: models", () => { }, }); }); + + it("specify default value on union with variant", async () => { + const res = await oapiForModel( + "Foo", + ` + model Foo { + optionalUnion?: MyUnion = MyUnion.a; + }; + + union MyUnion { + a: "a-value", + b: "b-value", + } + ` + ); + + deepStrictEqual(res.schemas.Foo, { + type: "object", + properties: { + optionalUnion: { + allOf: [{ $ref: "#/components/schemas/MyUnion" }], + default: "a-value", + }, + }, + }); + }); + it("specify default value on nullable property", async () => { const res = await oapiForModel( "Foo", diff --git a/packages/openapi3/test/union-schema.test.ts b/packages/openapi3/test/union-schema.test.ts index 39a1d73358..d4a45e7aab 100644 --- a/packages/openapi3/test/union-schema.test.ts +++ b/packages/openapi3/test/union-schema.test.ts @@ -338,6 +338,21 @@ describe("openapi3: union type", () => { }); }); + it("supports description on unions that reduce to enums", async () => { + const res = await oapiForModel( + "Foo", + ` + @doc("FooUnion") + union Foo { + "a"; + "b"; + } + + ` + ); + strictEqual(res.schemas.Foo.description, "FooUnion"); + }); + it("supports summary on unions and union variants", async () => { const res = await oapiForModel( "Foo",