diff --git a/common/changes/@typespec/versioning/versioning-OpParams_2023-11-10-16-19.json b/common/changes/@typespec/versioning/versioning-OpParams_2023-11-10-16-19.json new file mode 100644 index 0000000000..885c527c79 --- /dev/null +++ b/common/changes/@typespec/versioning/versioning-OpParams_2023-11-10-16-19.json @@ -0,0 +1,10 @@ +{ + "changes": [ + { + "packageName": "@typespec/versioning", + "comment": "Fix issue where the version compatability of parameters in operation signatures was not checked.", + "type": "none" + } + ], + "packageName": "@typespec/versioning" +} \ No newline at end of file diff --git a/packages/versioning/src/lib.ts b/packages/versioning/src/lib.ts index 8d2a6628d1..9a4d74df89 100644 --- a/packages/versioning/src/lib.ts +++ b/packages/versioning/src/lib.ts @@ -67,10 +67,10 @@ const libDef = { severity: "error", messages: { default: paramMessage`'${"sourceName"}' is referencing versioned type '${"targetName"}' but is not versioned itself.`, - addedAfter: paramMessage`'${"sourceName"}' was added on version '${"sourceAddedOn"}' but referencing type '${"targetName"}' added in version '${"targetAddedOn"}'.`, - dependentAddedAfter: paramMessage`'${"sourceName"}' was added on version '${"sourceAddedOn"}' but contains type '${"targetName"}' added in version '${"targetAddedOn"}'.`, - removedBefore: paramMessage`'${"sourceName"}' was removed on version '${"sourceRemovedOn"}' but referencing type '${"targetName"}' removed in version '${"targetRemovedOn"}'.`, - dependentRemovedBefore: paramMessage`'${"sourceName"}' was removed on version '${"sourceRemovedOn"}' but contains type '${"targetName"}' removed in version '${"targetRemovedOn"}'.`, + addedAfter: paramMessage`'${"sourceName"}' was added in version '${"sourceAddedOn"}' but referencing type '${"targetName"}' added in version '${"targetAddedOn"}'.`, + dependentAddedAfter: paramMessage`'${"sourceName"}' was added in version '${"sourceAddedOn"}' but contains type '${"targetName"}' added in version '${"targetAddedOn"}'.`, + removedBefore: paramMessage`'${"sourceName"}' was removed in version '${"sourceRemovedOn"}' but referencing type '${"targetName"}' removed in version '${"targetRemovedOn"}'.`, + dependentRemovedBefore: paramMessage`'${"sourceName"}' was removed in version '${"sourceRemovedOn"}' but contains type '${"targetName"}' removed in version '${"targetRemovedOn"}'.`, versionedDependencyAddedAfter: paramMessage`'${"sourceName"}' is referencing type '${"targetName"}' added in version '${"targetAddedOn"}' but version used is ${"dependencyVersion"}.`, versionedDependencyRemovedBefore: paramMessage`'${"sourceName"}' is referencing type '${"targetName"}' removed in version '${"targetAddedOn"}' but version used is ${"dependencyVersion"}.`, doesNotExist: paramMessage`'${"sourceName"}' is referencing type '${"targetName"}' which does not exist in version '${"version"}'.`, diff --git a/packages/versioning/src/validate.ts b/packages/versioning/src/validate.ts index eb79e45db0..5748c727c1 100644 --- a/packages/versioning/src/validate.ts +++ b/packages/versioning/src/validate.ts @@ -93,10 +93,11 @@ export function $onValidate(program: Program) { addDependency(namespace, op.returnType); if (op.interface) { - // Validate model -> property have correct versioning validateTargetVersionCompatible(program, op.interface, op, { isTargetADependent: true }); } - + for (const prop of op.parameters.properties.values()) { + validateReference(program, op, prop.type); + } validateReference(program, op, op.returnType); }, interface: (iface) => { diff --git a/packages/versioning/test/incompatible-versioning.test.ts b/packages/versioning/test/incompatible-versioning.test.ts index 578a16b0c7..84a5172c7a 100644 --- a/packages/versioning/test/incompatible-versioning.test.ts +++ b/packages/versioning/test/incompatible-versioning.test.ts @@ -102,6 +102,70 @@ describe("versioning: validate incompatible references", () => { }); }); + describe("operation", () => { + it("emit diagnostic when unversioned op has a versioned model as a parameter", async () => { + const diagnostics = await runner.diagnose(` + @added(Versions.v2) + model Foo {} + + op test(param: Foo): void; + `); + expectDiagnostics(diagnostics, { + code: "@typespec/versioning/incompatible-versioned-reference", + message: + "'TestService.test' is referencing versioned type 'TestService.Foo' but is not versioned itself.", + }); + }); + + it("emit diagnostic when unversioned op based on a template has a versioned model as a parameter", async () => { + const diagnostics = await runner.diagnose(` + @added(Versions.v2) + model Foo {} + + op Template(param: T): void; + + op test is Template; + `); + expectDiagnostics(diagnostics, { + code: "@typespec/versioning/incompatible-versioned-reference", + message: + "'TestService.test' is referencing versioned type 'TestService.Foo' but is not versioned itself.", + }); + }); + + it("emit diagnostic when when op was added before parameter", async () => { + const diagnostics = await runner.diagnose(` + @added(Versions.v2) + model Foo {} + + @added(Versions.v1) + op test(param: Foo): void; + `); + expectDiagnostics(diagnostics, { + code: "@typespec/versioning/incompatible-versioned-reference", + message: + "'TestService.test' was added in version 'v1' but referencing type 'TestService.Foo' added in version 'v2'.", + }); + }); + + it("emit diagnostic when op based on a template was added before parameter", async () => { + const diagnostics = await runner.diagnose(` + @added(Versions.v2) + model Foo {} + + op Template(param: T): void; + + @added(Versions.v1) + op test is Template; + `); + expectDiagnostics(diagnostics, { + code: "@typespec/versioning/incompatible-versioned-reference", + message: + "'TestService.test' was added in version 'v1' but referencing type 'TestService.Foo' added in version 'v2'.", + }); + }); + }); + describe("operation return type", () => { it("emit diagnostic when unversioned op is returning versioned model", async () => { const diagnostics = await runner.diagnose(` @@ -128,7 +192,7 @@ describe("versioning: validate incompatible references", () => { expectDiagnostics(diagnostics, { code: "@typespec/versioning/incompatible-versioned-reference", message: - "'TestService.test' was added on version 'v2' but referencing type 'TestService.Foo' added in version 'v3'.", + "'TestService.test' was added in version 'v2' but referencing type 'TestService.Foo' added in version 'v3'.", }); }); @@ -143,7 +207,7 @@ describe("versioning: validate incompatible references", () => { expectDiagnostics(diagnostics, { code: "@typespec/versioning/incompatible-versioned-reference", message: - "'TestService.test' was removed on version 'v3' but referencing type 'TestService.Foo' removed in version 'v2'.", + "'TestService.test' was removed in version 'v3' but referencing type 'TestService.Foo' removed in version 'v2'.", }); }); @@ -206,7 +270,7 @@ describe("versioning: validate incompatible references", () => { expectDiagnostics(diagnostics, { code: "@typespec/versioning/incompatible-versioned-reference", message: - "'TestService.Bar.foo' was added on version 'v2' but referencing type 'TestService.Foo' added in version 'v3'.", + "'TestService.Bar.foo' was added in version 'v2' but referencing type 'TestService.Foo' added in version 'v3'.", }); }); @@ -223,7 +287,7 @@ describe("versioning: validate incompatible references", () => { expectDiagnostics(diagnostics, { code: "@typespec/versioning/incompatible-versioned-reference", message: - "'TestService.Bar.foo' was removed on version 'v3' but referencing type 'TestService.Foo' removed in version 'v2'.", + "'TestService.Bar.foo' was removed in version 'v3' but referencing type 'TestService.Foo' removed in version 'v2'.", }); }); @@ -328,7 +392,7 @@ describe("versioning: validate incompatible references", () => { expectDiagnostics(diagnostics, { code: "@typespec/versioning/incompatible-versioned-reference", message: - "'TestService.Bar' was added on version 'v3' but contains type 'TestService.Bar.foo' added in version 'v2'.", + "'TestService.Bar' was added in version 'v3' but contains type 'TestService.Bar.foo' added in version 'v2'.", }); }); @@ -343,7 +407,7 @@ describe("versioning: validate incompatible references", () => { expectDiagnostics(diagnostics, { code: "@typespec/versioning/incompatible-versioned-reference", message: - "'TestService.Bar' was removed on version 'v2' but contains type 'TestService.Bar.foo' removed in version 'v3'.", + "'TestService.Bar' was removed in version 'v2' but contains type 'TestService.Bar.foo' removed in version 'v3'.", }); }); @@ -404,7 +468,7 @@ describe("versioning: validate incompatible references", () => { "'TestService.Bar.foo' is referencing versioned type 'TestService.Versioned' but is not versioned itself.", }); }); - it("emit diagnostic when using versioned union variant in non versioned operation return type", async () => { + it("emit diagnostic when using versioned union variant in nin versioned operation return type", async () => { const diagnostics = await runner.diagnose(` @added(Versions.v2) model Versioned {} @@ -417,7 +481,7 @@ describe("versioning: validate incompatible references", () => { }); }); - it("emit diagnostic when using versioned array element in non versioned operation return type", async () => { + it("emit diagnostic when using versioned array element in nin versioned operation return type", async () => { const diagnostics = await runner.diagnose(` @added(Versions.v2) model Versioned {} @@ -430,7 +494,7 @@ describe("versioning: validate incompatible references", () => { }); }); - it("emit diagnostic when using versioned tuple element in non versioned operation return type", async () => { + it("emit diagnostic when using versioned tuple element in nin versioned operation return type", async () => { const diagnostics = await runner.diagnose(` @added(Versions.v2) model Versioned {} @@ -466,7 +530,7 @@ describe("versioning: validate incompatible references", () => { expectDiagnostics(diagnostics, { code: "@typespec/versioning/incompatible-versioned-reference", message: - "'TestService.Bar' was added on version 'v3' but contains type 'TestService.foo' added in version 'v2'.", + "'TestService.Bar' was added in version 'v3' but contains type 'TestService.foo' added in version 'v2'.", }); }); @@ -481,7 +545,7 @@ describe("versioning: validate incompatible references", () => { expectDiagnostics(diagnostics, { code: "@typespec/versioning/incompatible-versioned-reference", message: - "'TestService.Bar' was removed on version 'v2' but contains type 'TestService.foo' removed in version 'v3'.", + "'TestService.Bar' was removed in version 'v2' but contains type 'TestService.foo' removed in version 'v3'.", }); }); @@ -544,7 +608,7 @@ describe("versioning: validate incompatible references", () => { expectDiagnostics(diagnostics, { code: "@typespec/versioning/incompatible-versioned-reference", message: - "'TestService.WidgetService' was added on version 'v1' but referencing type 'TestService.Widget' added in version 'v2'.", + "'TestService.WidgetService' was added in version 'v1' but referencing type 'TestService.Widget' added in version 'v2'.", }); }); @@ -601,7 +665,7 @@ describe("versioning: validate incompatible references", () => { expectDiagnostics(diagnostics, { code: "@typespec/versioning/incompatible-versioned-reference", message: - "'TestService.test' was added on version 'v1' but referencing type 'VersionedLib.Foo' added in version 'v3'.", + "'TestService.test' was added in version 'v1' but referencing type 'VersionedLib.Foo' added in version 'v3'.", }); });