Allow using default values for union property types & description bei…

…ng dropped (#2896)

fix #2894 Cannot use union variant as default value
fix #2895 Description lost on union of primitive types

.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

.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

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);
   }

packages/json-schema/src/json-schema-emitter.ts

@@ -217,6 +217,8 @@ export class JsonSchemaEmitter extends TypeEmitter<Record<string, any>, JSONSche
             });
       case "EnumMember":
         return defaultType.value ?? defaultType.name;
+      case "UnionVariant":
+        return this.#getDefaultValue(type, defaultType.type);
       default:
         reportDiagnostic(program, {
           code: "invalid-default",

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",
+      });
+    });
   });
 });

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 {

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);

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<OpenAPI3Schema> = 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<OpenAPI3Schema> {
     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<string, any> = {};
@@ -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<OpenAPI3Schema> = 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<OpenAPI3Schema>(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<Record<string, unknown>>,
     pathUp: Scope<Record<string, unknown>>[],
@@ -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,
+      });
+  }
+}

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",