Fix typeChangedFrom of template/union/tuple to catch invalid versioning (microsoft#5262)

chrisradek · Christopher Radek · web-flow · commit e9003b1a60c7 · 2024-12-05T04:53:53.000Z
Fixes microsoft#4752 and replaces microsoft#5191 microsoft#5191 was the original attempt to fix this issue. However the implementation only worked if the current type of a `@typeChangedFrom` decorated property wasn't itself a template/union. This PR gets away from using `validateReference` like the previous PR did, and instead will recursively crawl the target type such that it visits any template args/union variants/tuple members, checking if each sub-type available for the version being validated. Edit: Here is an example illustrating the issue introduced in microsoft#5191: [playground](https://cadlplayground.z22.web.core.windows.net/prs/5191/?c=aW1wb3J0ICJAdHlwZXNwZWMvdmVyc2lvbmluZyI7DQoNCnVzaW5nIFR5cGVTcGVjLlbJH8UeQMcvZWQoxxpzKQ0KQHNlcnZpY2UNCm5hbWVzcGFjZSBEZW1vU8YXxTplbnVtIMg0IHsNCiAgdjEsxQcyLA0KfcVeYWRky1oudjEpDQptb2RlbCBPcmlnaW5hbCB71ioyySpVcGRhdGVkxynGFEJhcsZz5QD4Q2hhbmdlZEZyb23MQizJZVtdKcQtZmFpbHM6yFFbXeUAyN9E0ERwYXNzZcpFO%2BcA6g%3D%3D&e=%40typespec%2Fopenapi3&options=%7B%7D) --------- Co-authored-by: Christopher Radek <Christopher.Radek@microsoft.com>
diff --git a/.chronus/changes/fix-versioning-typechangeof-2024-11-3-22-56-22.md b/.chronus/changes/fix-versioning-typechangeof-2024-11-3-22-56-22.md
@@ -0,0 +1,6 @@
+---
+changeKind: internal
+packages:
+  - "@typespec/versioning"
+---
+
diff --git a/.chronus/changes/fix-versioning-typechangeof-2024-11-3-22-56-4.md b/.chronus/changes/fix-versioning-typechangeof-2024-11-3-22-56-4.md
@@ -0,0 +1,7 @@
+---
+changeKind: fix
+packages:
+  - "@typespec/versioning"
+---
+
+Fixes diagnostics for @typeChangedFrom to properly detect when an incompatible version is referenced inside of a template, union, or tuple.
diff --git a/packages/versioning/src/validate.ts b/packages/versioning/src/validate.ts
@@ -223,21 +223,64 @@ function validateMultiTypeReference(program: Program, source: Type, options?: Ty
   if (versionTypeMap === undefined) return;
   for (const [version, type] of versionTypeMap!) {
     if (type === undefined) continue;
+    validateTypeAvailability(program, version, type, source, options);
+  }
+}
+
+/**
+ * Ensures that a type is available in a given version.
+ * For types that may wrap other types, e.g. unions, tuples, or template instances,
+ * this function will recursively check the wrapped types.
+ */
+function validateTypeAvailability(
+  program: Program,
+  version: Version,
+  targetType: Type,
+  source: Type,
+  options?: TypeNameOptions,
+) {
+  const typesToCheck: Type[] = [targetType];
+  while (typesToCheck.length) {
+    const type = typesToCheck.pop()!;
     const availMap = getAvailabilityMap(program, type);
-    const availability = availMap?.get(version.name) ?? Availability.Available;
-    if ([Availability.Added, Availability.Available].includes(availability)) {
-      continue;
+    const availability = availMap?.get(version?.name) ?? Availability.Available;
+    if (![Availability.Added, Availability.Available].includes(availability)) {
+      reportDiagnostic(program, {
+        code: "incompatible-versioned-reference",
+        messageId: "doesNotExist",
+        format: {
+          sourceName: getTypeName(source, options),
+          targetName: getTypeName(type, options),
+          version: prettyVersion(version),
+        },
+        target: source,
+      });
+    }
+
+    if (isTemplateInstance(type)) {
+      for (const arg of type.templateMapper.args) {
+        if (isType(arg)) {
+          typesToCheck.push(arg);
+        }
+      }
+    } else if (type.kind === "Union") {
+      for (const variant of type.variants.values()) {
+        if (type.expression) {
+          // Union expressions don't have decorators applied,
+          // so we need to check the type directly.
+          typesToCheck.push(variant.type);
+        } else {
+          // Named unions can have decorators applied,
+          // so we need to check that the variant type is valid
+          // for whatever decoration the variant has.
+          validateTargetVersionCompatible(program, variant, variant.type);
+        }
+      }
+    } else if (type.kind === "Tuple") {
+      for (const value of type.values) {
+        typesToCheck.push(value);
+      }
     }
-    reportDiagnostic(program, {
-      code: "incompatible-versioned-reference",
-      messageId: "doesNotExist",
-      format: {
-        sourceName: getTypeName(source, options),
-        targetName: getTypeName(type, options),
-        version: prettyVersion(version),
-      },
-      target: source,
-    });
   }
 }
 
diff --git a/packages/versioning/test/incompatible-versioning.test.ts b/packages/versioning/test/incompatible-versioning.test.ts
@@ -387,6 +387,160 @@ describe("versioning: validate incompatible references", () => {
       });
     });
 
+    it("emit diagnostic when using @typeChangedFrom with a type parameter that does not yet exist in arrays", async () => {
+      const diagnostics = await runner.diagnose(`        
+        @test
+        @added(Versions.v2)
+        model Original {}
+
+        @test
+        @added(Versions.v2)
+        model Updated {}
+
+        @test
+        model Test {
+          @typeChangedFrom(Versions.v2, Original[])
+          prop: Updated[];
+        }
+        `);
+      expectDiagnostics(diagnostics, {
+        code: "@typespec/versioning/incompatible-versioned-reference",
+        severity: "error",
+        message:
+          "'TestService.Test.prop' is referencing type 'TestService.Original' which does not exist in version 'v1'.",
+      });
+    });
+
+    it("emit diagnostic when using @typeChangedFrom with a type parameter that does not yet exist in records", async () => {
+      const diagnostics = await runner.diagnose(`        
+        @test
+        @added(Versions.v2)
+        model Original {}
+
+        @test
+        @added(Versions.v2)
+        model Updated {}
+
+        @test
+        model Test {
+          @typeChangedFrom(Versions.v2, Record<Original>)
+          prop: Record<Updated>;
+        }
+        `);
+      expectDiagnostics(diagnostics, {
+        code: "@typespec/versioning/incompatible-versioned-reference",
+        severity: "error",
+        message:
+          "'TestService.Test.prop' is referencing type 'TestService.Original' which does not exist in version 'v1'.",
+      });
+    });
+
+    it("emit diagnostic when using @typeChangedFrom with a type parameter that does not yet exist in unions", async () => {
+      const diagnostics = await runner.diagnose(`        
+        @test
+        @added(Versions.v2)
+        model Original {}
+
+        @test
+        @added(Versions.v2)
+        model Updated {}
+
+        @test
+        model Test {
+          @typeChangedFrom(Versions.v2, Original | string)
+          prop: Updated;
+        }
+        `);
+      expectDiagnostics(diagnostics, {
+        code: "@typespec/versioning/incompatible-versioned-reference",
+        severity: "error",
+        message:
+          "'TestService.Test.prop' is referencing type 'TestService.Original' which does not exist in version 'v1'.",
+      });
+    });
+
+    it("emit diagnostic when using @typeChangedFrom with a type parameter that does not yet exist in named unions", async () => {
+      const diagnostics = await runner.diagnose(`        
+        @test
+        @added(Versions.v2)
+        model Original {}
+
+        @test
+        @added(Versions.v2)
+        model Updated {}
+
+        @test
+        union InvalidUnion {
+          string,
+          Updated,
+        }
+
+        @test
+        model Test {
+          @typeChangedFrom(Versions.v2, InvalidUnion)
+          prop: string;
+        }
+        `);
+      expectDiagnostics(diagnostics, {
+        code: "@typespec/versioning/incompatible-versioned-reference",
+        severity: "error",
+        message:
+          "'TestService.Updated' is referencing versioned type 'TestService.Updated' but is not versioned itself.",
+      });
+    });
+
+    it("emit diagnostic when using @typeChangedFrom with a type parameter that does not yet exist in tuples", async () => {
+      const diagnostics = await runner.diagnose(`        
+        @test
+        @added(Versions.v2)
+        model Original {}
+
+        @test
+        @added(Versions.v2)
+        model Updated {}
+
+        @test
+        model Test {
+          @typeChangedFrom(Versions.v2, [Original, string])
+          prop: [Updated, string];
+        }
+        `);
+      expectDiagnostics(diagnostics, {
+        code: "@typespec/versioning/incompatible-versioned-reference",
+        severity: "error",
+        message:
+          "'TestService.Test.prop' is referencing type 'TestService.Original' which does not exist in version 'v1'.",
+      });
+    });
+
+    it("emit diagnostic when using @typeChangedFrom with a type parameter that does not yet exist in template", async () => {
+      const diagnostics = await runner.diagnose(`        
+        @test
+        @added(Versions.v2)
+        model Original {}
+
+        @test
+        @added(Versions.v2)
+        model Updated {}
+
+        model Template<T> {
+          prop: T;
+        }
+
+        @test
+        model Test {
+          @typeChangedFrom(Versions.v2, Template<Original>)
+          prop: Template<Updated>;
+        }
+        `);
+      expectDiagnostics(diagnostics, {
+        code: "@typespec/versioning/incompatible-versioned-reference",
+        severity: "error",
+        message:
+          "'TestService.Test.prop' is referencing type 'TestService.Original' which does not exist in version 'v1'.",
+      });
+    });
+
     it("succeed if version are compatible in model", async () => {
       const diagnostics = await runner.diagnose(`
         @added(Versions.v2)
diff --git a/packages/versioning/test/versioning.test.ts b/packages/versioning/test/versioning.test.ts
@@ -9,6 +9,7 @@ import {
   type Program,
   type ProjectionApplication,
   type Scalar,
+  type Tuple,
   type Type,
   type Union,
 } from "@typespec/compiler";
@@ -553,6 +554,132 @@ describe("versioning: logic", () => {
       ok((v3.properties.get("prop")!.type as Model).name === "Updated");
     });
 
+    it("can change template arg types to versioned models", async () => {
+      const {
+        projections: [v1, v2, v3],
+      } = await versionedModel(
+        ["v1", "v2", "v3"],
+        `
+        @test
+        model Original {}
+
+        @test
+        @added(Versions.v2)
+        model Updated {}
+
+        @test
+        model Test {
+          @added(Versions.v2)
+          @typeChangedFrom(Versions.v3, Original[])
+          prop: Updated[];
+        }
+        `,
+      );
+
+      ok(v1.properties.get("prop") === undefined);
+
+      const propV2 = v2.properties.get("prop")!.type as Model;
+      const propV3 = v3.properties.get("prop")!.type as Model;
+      ok(propV2.name === "Array");
+      ok((propV2.indexer!.value as Model).name === "Original");
+      ok(propV3.name === "Array");
+      ok((propV3.indexer!.value as Model).name === "Updated");
+    });
+
+    it("can change types to versioned unions", async () => {
+      const {
+        projections: [v1, v2, v3],
+      } = await versionedModel(
+        ["v1", "v2", "v3"],
+        `
+        @test
+        model Original {}
+
+        @test
+        @added(Versions.v2)
+        model Updated {}
+
+        @test
+        union TemporaryUnion {
+          string,
+
+          @added(Versions.v2)
+          Updated,
+        }
+
+        @test
+        model Test {
+          @typeChangedFrom(Versions.v3, TemporaryUnion)
+          prop: string | Updated;
+        }
+        `,
+      );
+
+      const propV1 = v1.properties.get("prop")!.type as Union;
+      const propV2 = v2.properties.get("prop")!.type as Union;
+      const propV3 = v3.properties.get("prop")!.type as Union;
+      ok(propV1.name === "TemporaryUnion");
+      ok(propV2.name === "TemporaryUnion");
+      ok(propV3.expression);
+      ok([...propV3.variants.values()].find((v) => (v.type as Model)?.name === "Updated"));
+    });
+
+    it("can change types to versioned tuples", async () => {
+      const {
+        projections: [v1, v2],
+      } = await versionedModel(
+        ["v1", "v2"],
+        `
+        @test
+        model Original {}
+
+        @test
+        @added(Versions.v2)
+        model Updated {}
+
+        @test
+        model Test {
+          @typeChangedFrom(Versions.v2, [Original])
+          prop: [Updated];
+        }
+        `,
+      );
+
+      const propV1 = v1.properties.get("prop")!.type as Tuple;
+      const propV2 = v2.properties.get("prop")!.type as Tuple;
+      ok((propV1.values[0] as Model).name === "Original");
+      ok((propV2.values[0] as Model).name === "Updated");
+    });
+
+    it("can change types to versioned union expressions", async () => {
+      const {
+        projections: [v1, v2],
+      } = await versionedModel(
+        ["v1", "v2"],
+        `
+        @test
+        model Original {}
+
+        @test
+        @added(Versions.v2)
+        model Updated {}
+
+        @test
+        model Test {
+          @typeChangedFrom(Versions.v2, string | Original)
+          prop: string | Updated;
+        }
+        `,
+      );
+
+      const propV1 = v1.properties.get("prop")!.type as Union;
+      const propV2 = v2.properties.get("prop")!.type as Union;
+      ok(propV1.expression);
+      ok(propV2.expression);
+      ok([...propV1.variants.values()].find((v) => (v.type as Model)?.name === "Original"));
+      ok([...propV2.variants.values()].find((v) => (v.type as Model)?.name === "Updated"));
+    });
+
     it("can change type over multiple versions", async () => {
       const {
         projections: [v1, v2, v3],
