Fix OpenAPI3 type property should always be set when nullable propert…

…y is present (microsoft#4672) microsoft#4584 --------- Co-authored-by: Kyle Zhang <v-zhanh@microsoft.com>
swatkatz · Nov 5, 2024 · 7b5fc74 · 7b5fc74
1 parent 2cf780b
commit 7b5fc74
Show file tree

Hide file tree

Showing 3 changed files with 72 additions and 3 deletions.
diff --git a/.chronus/changes/unionRefNullable-2024-9-10-14-48-35.md b/.chronus/changes/unionRefNullable-2024-9-10-14-48-35.md
@@ -0,0 +1,7 @@
+---
+changeKind: fix
+packages:
+  - "@typespec/openapi3"
+---
+
+OpenAPI3 type property should always be set when nullable property is present.
diff --git a/packages/openapi3/src/schema-emitter.ts b/packages/openapi3/src/schema-emitter.ts
@@ -554,7 +554,7 @@ export class OpenAPI3SchemaEmitter extends TypeEmitter<
           (schema instanceof Placeholder || "$ref" in schema) &&
           !(type && shouldInline(program, type))
         ) {
-          if (type && type.kind === "Model") {
+          if (type && (type.kind === "Model" || type.kind === "Scalar")) {
             return new ObjectBuilder({
               type: "object",
               allOf: B.array([schema]),
@@ -573,6 +573,17 @@ export class OpenAPI3SchemaEmitter extends TypeEmitter<
       }
     };
 
+    const checkMerge = (schemaMembers: { schema: any; type: Type | null }[]): boolean => {
+      if (nullable) {
+        for (const m of schemaMembers) {
+          if (m.schema instanceof Placeholder || "$ref" in m.schema) {
+            return true;
+          }
+        }
+      }
+      return false;
+    };
+
     if (schemaMembers.length === 0) {
       if (nullable) {
         // This union is equivalent to just `null` but OA3 has no way to specify
@@ -589,13 +600,14 @@ export class OpenAPI3SchemaEmitter extends TypeEmitter<
       return wrapWithObjectBuilder(schemaMembers[0], { mergeUnionWideConstraints: true });
     }
 
+    const isMerge = checkMerge(schemaMembers);
     const schema: OpenAPI3Schema = {
       [ofType]: schemaMembers.map((m) =>
-        wrapWithObjectBuilder(m, { mergeUnionWideConstraints: false }),
+        wrapWithObjectBuilder(m, { mergeUnionWideConstraints: isMerge }),
       ),
     };
 
-    if (nullable) {
+    if (!isMerge && nullable) {
       schema.nullable = true;
     }
 

diff --git a/packages/openapi3/test/union-schema.test.ts b/packages/openapi3/test/union-schema.test.ts
@@ -527,4 +527,54 @@ describe("openapi3: union type", () => {
       strictEqual(variant.description, undefined);
     }
   });
+
+  it("type property should always be set when nullable property is present", async () => {
+    const openApi = await openApiFor(`
+      scalar MyStr extends string;
+      model Foo {};
+      model A {
+        x: MyStr |Foo| null;
+      }
+      `);
+    deepStrictEqual(openApi.components.schemas.A.properties, {
+      x: {
+        anyOf: [
+          {
+            type: "object",
+            allOf: [{ $ref: "#/components/schemas/MyStr" }],
+            nullable: true,
+          },
+          {
+            type: "object",
+            allOf: [{ $ref: "#/components/schemas/Foo" }],
+            nullable: true,
+          },
+        ],
+      },
+    });
+  });
+
+  it("scalar type property should always be set when nullable property is present", async () => {
+    const openApi = await openApiFor(`
+        model Foo {};
+        model A {
+          x: Foo |string| null;
+        }
+        `);
+    deepStrictEqual(openApi.components.schemas.A.properties, {
+      x: {
+        anyOf: [
+          {
+            type: "object",
+            allOf: [{ $ref: "#/components/schemas/Foo" }],
+            nullable: true,
+          },
+          {
+            type: "string",
+            nullable: true,
+          },
+        ],
+      },
+    });
+  });
 });