From b64f4e14db0d9d52d1d2adfc1c73ec5e0a670099 Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Thu, 10 Apr 2025 08:24:36 -0700 Subject: [PATCH 1/2] Fix using enum values in extension --- packages/json-schema/src/decorators.ts | 46 ++++++++++ packages/json-schema/test/extension.test.ts | 46 +++++----- packages/openapi/src/decorators.ts | 42 ++++++++- packages/openapi3/test/extensions.test.ts | 81 +++++++++++++++++ packages/openapi3/test/openapi-output.test.ts | 91 +++---------------- 5 files changed, 202 insertions(+), 104 deletions(-) create mode 100644 packages/openapi3/test/extensions.test.ts diff --git a/packages/json-schema/src/decorators.ts b/packages/json-schema/src/decorators.ts index 72c97d654e1..9465932a0d8 100644 --- a/packages/json-schema/src/decorators.ts +++ b/packages/json-schema/src/decorators.ts @@ -6,11 +6,13 @@ import { type Namespace, type Program, type Scalar, + serializeValueAsJson, setTypeSpecNamespace, type Tuple, type Type, typespecTypeToJson, type Union, + type Value, } from "@typespec/compiler"; import { useStateMap, useStateSet } from "@typespec/compiler/utils"; import type { ValidatesRawJsonDecorator } from "../generated-defs/TypeSpec.JsonSchema.Private.js"; @@ -257,9 +259,53 @@ export const $extension: ExtensionDecorator = ( key: string, value: unknown, ) => { + if (!isTypeLike(value)) { + value = convertRemainingValuesToExtensions(context.program, value); + } setExtension(context.program, target, key, value); }; +// Workaround until we have a way to disable arg marshalling and just call serializeValueAsJson +// https://github.com/microsoft/typespec/issues/3570 +function convertRemainingValuesToExtensions(program: Program, value: unknown): unknown { + switch (typeof value) { + case "string": + case "number": + case "boolean": + return value; + case "object": + if (value === null) { + return null; + } + if (Array.isArray(value)) { + return value.map((x) => convertRemainingValuesToExtensions(program, x)); + } + + if (isTypeSpecValue(value)) { + return serializeValueAsJson(program, value, value.type); + } else { + const result: Record = {}; + for (const [key, val] of Object.entries(value)) { + if (val === undefined) { + continue; + } + result[key] = convertRemainingValuesToExtensions(program, val); + } + return result; + } + default: + return value; + } +} + +function isTypeLike(value: any): value is Type { + return typeof value === "object" && value !== null && isType(value); +} + +function isTypeSpecValue(value: object): value is Value { + return "entityKind" in value && value.entityKind === "Value"; +} + /** * Get extensions set via the `@extension` decorator on the given type * @param program TypeSpec program diff --git a/packages/json-schema/test/extension.test.ts b/packages/json-schema/test/extension.test.ts index f336b46b1ae..ab6641941bb 100644 --- a/packages/json-schema/test/extension.test.ts +++ b/packages/json-schema/test/extension.test.ts @@ -1,7 +1,7 @@ import type { DecoratorContext, Type } from "@typespec/compiler"; import { expectDiagnosticEmpty } from "@typespec/compiler/testing"; import assert from "assert"; -import { describe, it } from "vitest"; +import { describe, expect, it } from "vitest"; import { setExtension } from "../src/index.js"; import { emitSchema, emitSchemaWithDiagnostics } from "./utils.js"; @@ -150,30 +150,26 @@ it("handles Json-wrapped types", async () => { assert.deepStrictEqual(Foo.properties.x["x-string-literal"], "hi"); }); -// These tests are skipped - can enable if @extension is updated to support `valueof unknown` -it("handles values", async () => { - const schemas = await emitSchema(` - @extension("x-anon-model", #{ name: "foo" }) - @extension("x-nested-anon-model", #{ items: #[ #{foo: "bar" }]}) - @extension("x-tuple", #["foo"]) - model Foo { - @extension("x-bool-literal", true) - @extension("x-int-literal", 42) - @extension("x-string-literal", "hi") - @extension("x-null", null) - x: string; - } - `); - const Foo = schemas["Foo.json"]; - - assert.deepStrictEqual(Foo["x-anon-model"], { name: "foo" }); - assert.deepStrictEqual(Foo["x-nested-anon-model"], { items: [{ foo: "bar" }] }); - assert.deepStrictEqual(Foo["x-tuple"], ["foo"]); - - assert.deepStrictEqual(Foo.properties.x["x-bool-literal"], true); - assert.deepStrictEqual(Foo.properties.x["x-int-literal"], 42); - assert.deepStrictEqual(Foo.properties.x["x-string-literal"], "hi"); - assert.deepStrictEqual(Foo.properties.x["x-null"], null); +describe("values", () => { + it.each([ + ["string", `"foo"`, "foo"], + ["number", `42`, 42], + ["boolean", `true`, true], + ["null", `null`, null], + ["array", `#["foo", 42, true]`, ["foo", 42, true]], + ["object", `#{foo: "bar"}`, { foo: "bar" }], + ["enum value", "Direction.Up", "Up", "enum Direction {Up, Down}"], + ["enum value in object", "#{ dir: Direction.Up }", { dir: "Up" }, "enum Direction {Up, Down}"], + ])("%s", async (_, value, expected, extra?: string) => { + const schemas = await emitSchema( + ` + ${extra ?? ""} + @extension("x-custom", ${value}) + model Foo {} + `, + ); + expect(schemas["Foo.json"]["x-custom"]).toEqual(expected); + }); }); describe("setExtension", () => { diff --git a/packages/openapi/src/decorators.ts b/packages/openapi/src/decorators.ts index 130a06fea2d..b5c0c56f5bb 100644 --- a/packages/openapi/src/decorators.ts +++ b/packages/openapi/src/decorators.ts @@ -10,7 +10,9 @@ import { Namespace, Operation, Program, + serializeValueAsJson, Type, + Value, } from "@typespec/compiler"; import { useStateMap } from "@typespec/compiler/utils"; import * as http from "@typespec/http"; @@ -63,9 +65,47 @@ export const $extension: ExtensionDecorator = ( "OpenAPI extension value must be a value but was a type", context.getArgumentTarget(1), ); - setExtension(context.program, entity, extensionName as ExtensionKey, value); + const processed = convertRemainingValuesToExtensions(context.program, value); + setExtension(context.program, entity, extensionName as ExtensionKey, processed); }; +// Workaround until we have a way to disable arg marshalling and just call serializeValueAsJson +// https://github.com/microsoft/typespec/issues/3570 +function convertRemainingValuesToExtensions(program: Program, value: unknown): unknown { + switch (typeof value) { + case "string": + case "number": + case "boolean": + return value; + case "object": + if (value === null) { + return null; + } + if (Array.isArray(value)) { + return value.map((x) => convertRemainingValuesToExtensions(program, x)); + } + + if (isTypeSpecValue(value)) { + return serializeValueAsJson(program, value, value.type); + } else { + const result: Record = {}; + for (const [key, val] of Object.entries(value)) { + if (val === undefined) { + continue; + } + result[key] = convertRemainingValuesToExtensions(program, val); + } + return result; + } + default: + return value; + } +} + +function isTypeSpecValue(value: object): value is Value { + return "entityKind" in value && value.entityKind === "Value"; +} + /** * Set the OpenAPI info node on for the given service namespace. * @param program Program diff --git a/packages/openapi3/test/extensions.test.ts b/packages/openapi3/test/extensions.test.ts new file mode 100644 index 00000000000..6585d1d6e21 --- /dev/null +++ b/packages/openapi3/test/extensions.test.ts @@ -0,0 +1,81 @@ +import { ok, strictEqual } from "assert"; +import { describe, expect, it } from "vitest"; +import { openApiFor } from "./test-host.js"; + +describe("target", () => { + it("add to model", async () => { + const oapi = await openApiFor(` + @extension("x-model-extension", "foobar") + model Pet {} + `); + ok(oapi.components.schemas.Pet); + strictEqual(oapi.components.schemas.Pet["x-model-extension"], "foobar"); + }); + + it("operation", async () => { + const oapi = await openApiFor( + ` + @extension("x-operation-extension", "barbaz") + op list(): string[]; + `, + ); + ok(oapi.paths["/"].get); + strictEqual(oapi.paths["/"].get["x-operation-extension"], "barbaz"); + }); + + it("parameter component", async () => { + const oapi = await openApiFor( + ` + model Pet { + name: string; + } + model PetId { + @path + @extension("x-parameter-extension", "foobaz") + petId: string; + } + @route("/Pets") @get op get(... PetId): Pet; + `, + ); + ok(oapi.paths["/Pets/{petId}"].get); + strictEqual( + oapi.paths["/Pets/{petId}"].get.parameters[0]["$ref"], + "#/components/parameters/PetId", + ); + strictEqual(oapi.components.parameters.PetId.name, "petId"); + strictEqual(oapi.components.parameters.PetId["x-parameter-extension"], "foobaz"); + }); + + it("adds at root of document when on namespace", async () => { + const oapi = await openApiFor( + ` + @extension("x-namespace-extension", "foobar") + @service namespace Service {}; + `, + ); + + strictEqual(oapi["x-namespace-extension"], "foobar"); + }); +}); + +describe("extension value", () => { + it.each([ + ["string", `"foo"`, "foo"], + ["number", `42`, 42], + ["boolean", `true`, true], + ["null", `null`, null], + ["array", `#["foo", 42, true]`, ["foo", 42, true]], + ["object", `#{foo: "bar"}`, { foo: "bar" }], + ["enum value", "Direction.Up", "Up", "enum Direction {Up, Down}"], + ["enum value in object", "#{ dir: Direction.Up }", { dir: "Up" }, "enum Direction {Up, Down}"], + ])("%s", async (_, value, expected, extra?: string) => { + const oapi = await openApiFor( + ` + ${extra ?? ""} + @extension("x-custom", ${value}) + model Foo {} + `, + ); + expect(oapi.components.schemas.Foo["x-custom"]).toEqual(expected); + }); +}); diff --git a/packages/openapi3/test/openapi-output.test.ts b/packages/openapi3/test/openapi-output.test.ts index 1af3f1bf1a4..3410d5fef87 100644 --- a/packages/openapi3/test/openapi-output.test.ts +++ b/packages/openapi3/test/openapi-output.test.ts @@ -193,73 +193,9 @@ worksFor(["3.0.0", "3.1.0"], ({ oapiForModel, openApiFor, openapiWithOptions }) strictEqual(res.components.parameters["PetId.name"].deprecated, undefined); }); - describe("openapi3: extension decorator", () => { - it("adds an arbitrary extension to a model", async () => { - const oapi = await openApiFor(` - @extension("x-model-extension", "foobar") - model Pet { - name: string; - } - @get() op read(): Pet; - `); - ok(oapi.components.schemas.Pet); - strictEqual(oapi.components.schemas.Pet["x-model-extension"], "foobar"); - }); - - it("adds an arbitrary extension to an operation", async () => { - const oapi = await openApiFor( - ` - model Pet { - name: string; - } - @get() - @extension("x-operation-extension", "barbaz") - op list(): Pet[]; - `, - ); - ok(oapi.paths["/"].get); - strictEqual(oapi.paths["/"].get["x-operation-extension"], "barbaz"); - }); - - it("adds an arbitrary extension to a parameter", async () => { - const oapi = await openApiFor( - ` - model Pet { - name: string; - } - model PetId { - @path - @extension("x-parameter-extension", "foobaz") - petId: string; - } - @route("/Pets") - @get() - op get(... PetId): Pet; - `, - ); - ok(oapi.paths["/Pets/{petId}"].get); - strictEqual( - oapi.paths["/Pets/{petId}"].get.parameters[0]["$ref"], - "#/components/parameters/PetId", - ); - strictEqual(oapi.components.parameters.PetId.name, "petId"); - strictEqual(oapi.components.parameters.PetId["x-parameter-extension"], "foobaz"); - }); - - it("adds an extension to a namespace", async () => { - const oapi = await openApiFor( - ` - @extension("x-namespace-extension", "foobar") - @service namespace Service {}; - `, - ); - - strictEqual(oapi["x-namespace-extension"], "foobar"); - }); - - it("check format and pattern decorator on model", async () => { - const oapi = await openApiFor( - ` + it("check format and pattern decorator on model", async () => { + const oapi = await openApiFor( + ` model Pet extends PetId { @pattern("^[a-zA-Z0-9-]{3,24}$") name: string; @@ -274,17 +210,16 @@ worksFor(["3.0.0", "3.1.0"], ({ oapiForModel, openApiFor, openapiWithOptions }) @get() op get(... PetId): Pet; `, - ); - ok(oapi.paths["/Pets/{petId}"].get); - strictEqual( - oapi.paths["/Pets/{petId}"].get.parameters[0]["$ref"], - "#/components/parameters/PetId", - ); - strictEqual(oapi.components.parameters.PetId.name, "petId"); - strictEqual(oapi.components.schemas.Pet.properties.name.pattern, "^[a-zA-Z0-9-]{3,24}$"); - strictEqual(oapi.components.parameters.PetId.schema.format, "UUID"); - strictEqual(oapi.components.parameters.PetId.schema.pattern, "^[a-zA-Z0-9-]{3,24}$"); - }); + ); + ok(oapi.paths["/Pets/{petId}"].get); + strictEqual( + oapi.paths["/Pets/{petId}"].get.parameters[0]["$ref"], + "#/components/parameters/PetId", + ); + strictEqual(oapi.components.parameters.PetId.name, "petId"); + strictEqual(oapi.components.schemas.Pet.properties.name.pattern, "^[a-zA-Z0-9-]{3,24}$"); + strictEqual(oapi.components.parameters.PetId.schema.format, "UUID"); + strictEqual(oapi.components.parameters.PetId.schema.pattern, "^[a-zA-Z0-9-]{3,24}$"); }); describe("openapi3: useRef decorator", () => { From a720546d35f0fee3ebbadbb403cfbc99f8f13182 Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Thu, 10 Apr 2025 08:47:49 -0700 Subject: [PATCH 2/2] Create fix-enum-value-extensions-2025-3-10-15-26-22.md --- .../fix-enum-value-extensions-2025-3-10-15-26-22.md | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 .chronus/changes/fix-enum-value-extensions-2025-3-10-15-26-22.md diff --git a/.chronus/changes/fix-enum-value-extensions-2025-3-10-15-26-22.md b/.chronus/changes/fix-enum-value-extensions-2025-3-10-15-26-22.md new file mode 100644 index 00000000000..53527f56519 --- /dev/null +++ b/.chronus/changes/fix-enum-value-extensions-2025-3-10-15-26-22.md @@ -0,0 +1,9 @@ +--- +# Change versionKind to one of: internal, fix, dependencies, feature, deprecation, breaking +changeKind: fix +packages: + - "@typespec/json-schema" + - "@typespec/openapi" +--- + +Fix crash when using enum values in extension