Fix issue with @removed decorator if model was not added from begin…

…ning (#3409) Fixes #3210 While the issue was for model properties, it also applies to interfaces, so tests are included for both. --------- Co-authored-by: Timothee Guerin <[email protected]>
microsoft · May 21, 2024 · a6bcade · a6bcade
1 parent eba81f0
commit a6bcade
Show file tree

Hide file tree

Showing 4 changed files with 81 additions and 1 deletion.
diff --git a/.chronus/changes/versioning-FixVersionBug1-2024-4-21-16-34-18.md b/.chronus/changes/versioning-FixVersionBug1-2024-4-21-16-34-18.md
@@ -0,0 +1,8 @@
+---
+# Change versionKind to one of: internal, fix, dependencies, feature, deprecation, breaking
+changeKind: fix
+packages:
+  - "@typespec/versioning"
+---
+
+Using `@removed` on member types and `@added` on containing type could result in errors
diff --git a/packages/versioning/src/validate.ts b/packages/versioning/src/validate.ts
@@ -786,6 +786,7 @@ function validateAvailabilityForContains(
   for (const key of keySet) {
     const sourceVal = sourceAvail.get(key)!;
     const targetVal = targetAvail.get(key)!;
+    if (sourceVal === targetVal) continue;
     if (
       [Availability.Added].includes(targetVal) &&
       [Availability.Removed, Availability.Unavailable].includes(sourceVal)

diff --git a/packages/versioning/src/versioning.ts b/packages/versioning/src/versioning.ts
@@ -15,6 +15,7 @@ import {
   Type,
   Union,
   UnionVariant,
+  compilerAssert,
   getNamespaceFullName,
 } from "@typespec/compiler";
 import {
@@ -806,10 +807,27 @@ export function getAvailabilityMap(
   )
     return undefined;
 
+  let parentMap: Map<string, Availability> | undefined = undefined;
+  if (type.kind === "ModelProperty" && type.model !== undefined) {
+    parentMap = getAvailabilityMap(program, type.model);
+  } else if (type.kind === "Operation" && type.interface !== undefined) {
+    parentMap = getAvailabilityMap(program, type.interface);
+  }
+
   // implicitly, all versioned things are assumed to have been added at
   // v1 if not specified
   if (!added.length) {
-    added.push(allVersions[0]);
+    if (parentMap !== undefined) {
+      parentMap.forEach((key, value) => {
+        if (key === Availability.Added.valueOf()) {
+          const match = allVersions.find((x) => x.name === value);
+          compilerAssert(match !== undefined, "Version not found");
+          added.push(match);
+        }
+      });
+    } else {
+      added.push(allVersions[0]);
+    }
   }
 
   // something isn't available by default

diff --git a/packages/versioning/test/versioning.test.ts b/packages/versioning/test/versioning.test.ts
@@ -280,6 +280,34 @@ describe("versioning: logic", () => {
       );
     });
 
+    it("can be removed respecting model versioning", async () => {
+      const {
+        source,
+        projections: [v2, v3, v4],
+      } = await versionedModel(
+        ["v2", "v3", "v4"],
+        `@added(Versions.v2)
+        model Test {
+          a: int32;
+          @removed(Versions.v3) b: int32;
+        }
+        `
+      );
+
+      assertHasProperties(v2, ["a", "b"]);
+      assertHasProperties(v3, ["a"]);
+      assertHasProperties(v4, ["a"]);
+
+      assertModelProjectsTo(
+        [
+          [v2, "v2"],
+          [v3, "v3"],
+          [v3, "v4"],
+        ],
+        source
+      );
+    });
+
     it("can be renamed", async () => {
       const {
         source,
@@ -1365,6 +1393,31 @@ describe("versioning: logic", () => {
       );
     });
 
+    it("can be removed respecting interface versioning", async () => {
+      const {
+        source,
+        projections: [v2, v3, v4],
+      } = await versionedInterface(
+        ["v2", "v3", "v4"],
+        `@added(Versions.v2)
+        interface Test {
+          allVersions(): void;
+          @removed(Versions.v3) version2Only(): void;
+        }
+        `
+      );
+      assertHasOperations(v2, ["allVersions", "version2Only"]);
+      assertHasOperations(v3, ["allVersions"]);
+      assertInterfaceProjectsTo(
+        [
+          [v2, "v2"],
+          [v3, "v3"],
+          [v4, "v4"],
+        ],
+        source
+      );
+    });
+
     it("can be renamed", async () => {
       const {
         source,