Skip to content

Commit

Permalink
Ensure operations validate versioning of their parameters (#2662)
Browse files Browse the repository at this point in the history 
Fixes https://github.com/Azure/typespec-azure/issues/3732 by ensuring
that parameters in operation signatures are evaluated for version
compatibility.

Also updates versioning error message slightly.
  • Loading branch information
@tjprescott
tjprescott authored Nov 14, 2023
1 parent bc62493 commit b6fb119
Show file tree
Hide file tree
Showing 4 changed files with 94 additions and 19 deletions.
10 changes: 10 additions & 0 deletions common/changes/@typespec/versioning/versioning-OpParams_2023-11-10-16-19.json
View file
Original file line number Diff line number Diff line change
@@ -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"
}
8 changes: 4 additions & 4 deletions packages/versioning/src/lib.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"}'.`,
Expand Down
5 changes: 3 additions & 2 deletions packages/versioning/src/validate.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down
90 changes: 77 additions & 13 deletions packages/versioning/test/incompatible-versioning.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<T>(param: T): void;
op test is Template<Foo>;
`);
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<T>(param: T): void;
@added(Versions.v1)
op test is Template<Foo>;
`);
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(`
Expand All @@ -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'.",
});
});

Expand All @@ -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'.",
});
});

Expand Down Expand Up @@ -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'.",
});
});

Expand All @@ -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'.",
});
});

Expand Down Expand Up @@ -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'.",
});
});

Expand All @@ -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'.",
});
});

Expand Down Expand Up @@ -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 {}
Expand All @@ -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 {}
Expand All @@ -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 {}
Expand Down Expand Up @@ -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'.",
});
});

Expand All @@ -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'.",
});
});

Expand Down Expand Up @@ -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'.",
});
});

Expand Down Expand Up @@ -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'.",
});
});

Expand Down

0 comments on commit b6fb119

Please sign in to comment.