From 04184bba9d592221c46ba1705a9a332d0c685dbf Mon Sep 17 00:00:00 2001 From: Travis Prescott Date: Thu, 27 Apr 2023 09:27:37 -0700 Subject: [PATCH 1/3] Issue error when versioned specs specify a single version with @service. Fixes #1866 (for real this time!) --- ...atibleDecoratorUsage_2023-04-27-16-38.json | 10 +++++ ...atibleDecoratorUsage_2023-04-27-16-29.json | 10 +++++ packages/openapi3/test/versioning.test.ts | 2 +- packages/versioning/src/lib.ts | 6 +++ packages/versioning/src/validate.ts | 13 +++++++ .../test/incompatible-versioning.test.ts | 37 +++++++++++++++++++ 6 files changed, 77 insertions(+), 1 deletion(-) create mode 100644 common/changes/@typespec/openapi3/versioning-IncompatibleDecoratorUsage_2023-04-27-16-38.json create mode 100644 common/changes/@typespec/versioning/versioning-IncompatibleDecoratorUsage_2023-04-27-16-29.json diff --git a/common/changes/@typespec/openapi3/versioning-IncompatibleDecoratorUsage_2023-04-27-16-38.json b/common/changes/@typespec/openapi3/versioning-IncompatibleDecoratorUsage_2023-04-27-16-38.json new file mode 100644 index 00000000000..d9a70151de5 --- /dev/null +++ b/common/changes/@typespec/openapi3/versioning-IncompatibleDecoratorUsage_2023-04-27-16-38.json @@ -0,0 +1,10 @@ +{ + "changes": [ + { + "packageName": "@typespec/openapi3", + "comment": "", + "type": "none" + } + ], + "packageName": "@typespec/openapi3" +} \ No newline at end of file diff --git a/common/changes/@typespec/versioning/versioning-IncompatibleDecoratorUsage_2023-04-27-16-29.json b/common/changes/@typespec/versioning/versioning-IncompatibleDecoratorUsage_2023-04-27-16-29.json new file mode 100644 index 00000000000..4f9b443fafc --- /dev/null +++ b/common/changes/@typespec/versioning/versioning-IncompatibleDecoratorUsage_2023-04-27-16-29.json @@ -0,0 +1,10 @@ +{ + "changes": [ + { + "packageName": "@typespec/versioning", + "comment": "Raise error if versioned spec specifies a single service version.", + "type": "none" + } + ], + "packageName": "@typespec/versioning" +} \ No newline at end of file diff --git a/packages/openapi3/test/versioning.test.ts b/packages/openapi3/test/versioning.test.ts index 336931d76b7..d05e7b3d6a7 100644 --- a/packages/openapi3/test/versioning.test.ts +++ b/packages/openapi3/test/versioning.test.ts @@ -8,7 +8,7 @@ describe("openapi3: versioning", () => { const { v1, v2, v3 } = await openApiFor( ` @versioned(Versions) - @service({title: "My Service", version: "hi"}) + @service({title: "My Service"}) namespace MyService { enum Versions { @useDependency(MyLibrary.Versions.A) diff --git a/packages/versioning/src/lib.ts b/packages/versioning/src/lib.ts index 09de0a43d12..4bda4875ca6 100644 --- a/packages/versioning/src/lib.ts +++ b/packages/versioning/src/lib.ts @@ -51,6 +51,12 @@ const libDef = { default: "@renamedFrom.oldName cannot be empty string.", }, }, + "incompatible-decorator-use": { + severity: "error", + messages: { + default: paramMessage`Namespace '${"name"}' cannot be decorated with @versioned and @service({version: ${"version"}}). Remove the version argument from @service.`, + }, + }, "incompatible-versioned-reference": { severity: "error", messages: { diff --git a/packages/versioning/src/validate.ts b/packages/versioning/src/validate.ts index 1e7d6ad1da4..63812ab115f 100644 --- a/packages/versioning/src/validate.ts +++ b/packages/versioning/src/validate.ts @@ -1,5 +1,6 @@ import { getNamespaceFullName, + getService, getTypeName, isTemplateInstance, Namespace, @@ -93,6 +94,18 @@ export function $onValidate(program: Program) { } }, namespace: (namespace) => { + const [_, versionMap] = getVersions(program, namespace); + const serviceProps = getService(program, namespace); + if (serviceProps?.version !== undefined && versionMap !== undefined) { + reportDiagnostic(program, { + code: "incompatible-decorator-use", + format: { + name: getNamespaceFullName(namespace), + version: serviceProps.version, + }, + target: namespace, + }); + } const versionedNamespace = findVersionedNamespace(program, namespace); const dependencies = getVersionDependencies(program, namespace); if (dependencies === undefined) { diff --git a/packages/versioning/test/incompatible-versioning.test.ts b/packages/versioning/test/incompatible-versioning.test.ts index 09adb03bb15..b652a85177d 100644 --- a/packages/versioning/test/incompatible-versioning.test.ts +++ b/packages/versioning/test/incompatible-versioning.test.ts @@ -7,6 +7,43 @@ import { } from "@typespec/compiler/testing"; import { createVersioningTestHost, createVersioningTestRunner } from "./test-host.js"; +describe("versioning: incompatible use of decorators", () => { + let runner: BasicTestRunner; + let host: TestHost; + const imports: string[] = []; + + beforeEach(async () => { + host = await createVersioningTestHost(); + runner = createTestWrapper(host, { + wrapper: (code) => ` + import "@typespec/versioning"; + ${imports.map((i) => `import "${i}";`).join("\n")} + using TypeSpec.Versioning; + ${code}`, + }); + }); + + it("emit diagnostic when @service({version: 'X'}) is used with @versioned", async () => { + const diagnostics = await runner.diagnose(` + @versioned(Versions) + @service({ + title: "Widget Service", + version: "v3" + }) + namespace DemoService; + + enum Versions { + v1, + v2, + } + `); + expectDiagnostics(diagnostics, { + code: "@typespec/versioning/incompatible-decorator-use", + severity: "error", + }); + }); +}); + describe("versioning: validate incompatible references", () => { let runner: BasicTestRunner; let host: TestHost; From fd23997d0f81fcc7929e9fe479bee29f29d7763f Mon Sep 17 00:00:00 2001 From: Travis Prescott Date: Thu, 27 Apr 2023 09:55:53 -0700 Subject: [PATCH 2/3] Apply suggestions from code review Co-authored-by: Timothee Guerin --- packages/versioning/src/lib.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/versioning/src/lib.ts b/packages/versioning/src/lib.ts index 4bda4875ca6..8396f1f1722 100644 --- a/packages/versioning/src/lib.ts +++ b/packages/versioning/src/lib.ts @@ -51,10 +51,10 @@ const libDef = { default: "@renamedFrom.oldName cannot be empty string.", }, }, - "incompatible-decorator-use": { + "no-service-fixed-version": { severity: "error", messages: { - default: paramMessage`Namespace '${"name"}' cannot be decorated with @versioned and @service({version: ${"version"}}). Remove the version argument from @service.`, + default: paramMessage`Namespace '${"name"}' specify a fixed service version with @service({version: ${"version"}}) while using `@versioned`. Remove the version argument from @service.`, }, }, "incompatible-versioned-reference": { From a517d24aac731c735a26db130f0acd33edacfe2d Mon Sep 17 00:00:00 2001 From: Travis Prescott Date: Thu, 27 Apr 2023 09:58:26 -0700 Subject: [PATCH 3/3] Fix up code suggestions. --- packages/versioning/src/lib.ts | 2 +- packages/versioning/src/validate.ts | 2 +- packages/versioning/test/incompatible-versioning.test.ts | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/versioning/src/lib.ts b/packages/versioning/src/lib.ts index 8396f1f1722..e57a11bd4db 100644 --- a/packages/versioning/src/lib.ts +++ b/packages/versioning/src/lib.ts @@ -54,7 +54,7 @@ const libDef = { "no-service-fixed-version": { severity: "error", messages: { - default: paramMessage`Namespace '${"name"}' specify a fixed service version with @service({version: ${"version"}}) while using `@versioned`. Remove the version argument from @service.`, + default: paramMessage`Namespace '${"name"}' cannot specify a fixed service version with @service({version: ${"version"}}) while using @versioned. Remove the version argument from @service.`, }, }, "incompatible-versioned-reference": { diff --git a/packages/versioning/src/validate.ts b/packages/versioning/src/validate.ts index 63812ab115f..7fe46c70b16 100644 --- a/packages/versioning/src/validate.ts +++ b/packages/versioning/src/validate.ts @@ -98,7 +98,7 @@ export function $onValidate(program: Program) { const serviceProps = getService(program, namespace); if (serviceProps?.version !== undefined && versionMap !== undefined) { reportDiagnostic(program, { - code: "incompatible-decorator-use", + code: "no-service-fixed-version", format: { name: getNamespaceFullName(namespace), version: serviceProps.version, diff --git a/packages/versioning/test/incompatible-versioning.test.ts b/packages/versioning/test/incompatible-versioning.test.ts index b652a85177d..c01debc1225 100644 --- a/packages/versioning/test/incompatible-versioning.test.ts +++ b/packages/versioning/test/incompatible-versioning.test.ts @@ -38,7 +38,7 @@ describe("versioning: incompatible use of decorators", () => { } `); expectDiagnostics(diagnostics, { - code: "@typespec/versioning/incompatible-decorator-use", + code: "@typespec/versioning/no-service-fixed-version", severity: "error", }); });