Revert validation of operation signature versioning (microsoft#2692)

Fix microsoft#2688. Azure Core PR: https://github.com/Azure/typespec-azure/pull/3953
jrcribb · Nov 27, 2023 · a90dd28 · a90dd28
1 parent b3075f0
commit a90dd28
Show file tree

Hide file tree

Showing 4 changed files with 26 additions and 108 deletions.
diff --git a/common/changes/@typespec/versioning/revertOpParamValidation_2023-11-22-23-40.json b/common/changes/@typespec/versioning/revertOpParamValidation_2023-11-22-23-40.json
@@ -0,0 +1,10 @@
+{
+  "changes": [
+    {
+      "packageName": "@typespec/versioning",
+      "comment": "",
+      "type": "none"
+    }
+  ],
+  "packageName": "@typespec/versioning"
+}
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
@@ -2,9 +2,9 @@
   "changes": [
     {
       "packageName": "@typespec/versioning",
-      "comment": "Fix issue where the version compatability of parameters in operation signatures was not checked.",
+      "comment": "Fix crash in versioning library.",
       "type": "none"
     }
   ],
   "packageName": "@typespec/versioning"
-}
+}
diff --git a/packages/versioning/src/validate.ts b/packages/versioning/src/validate.ts
@@ -3,11 +3,9 @@ import {
   getService,
   getTypeName,
   isTemplateInstance,
-  ModelProperty,
   Namespace,
   navigateProgram,
   NoTarget,
-  Operation,
   Program,
   Type,
   TypeNameOptions,
@@ -98,21 +96,6 @@ export function $onValidate(program: Program) {
         if (op.interface) {
           validateTargetVersionCompatible(program, op.interface, op, { isTargetADependent: true });
         }
-
-        for (const prop of op.parameters.properties.values()) {
-          addNamespaceDependency(namespace, prop.type);
-
-          const typeChangedFrom = getTypeChangedFrom(program, prop);
-          if (typeChangedFrom !== undefined) {
-            validateMultiTypeReference(program, prop);
-          } else {
-            validateOperationParameter(program, op, prop);
-          }
-
-          // Validate model property type is correct when madeOptional
-          validateMadeOptional(program, prop);
-        }
-        validateVersionedPropertyNames(program, op.parameters);
         validateReference(program, op, op.returnType);
       },
       interface: (iface) => {
@@ -476,64 +459,6 @@ interface IncompatibleVersionValidateOptions {
   isTargetADependent?: boolean;
 }
 
-/**
- * Validate the target reference versioning is compatible with the source versioning.
- * @param operation The operation being containing the parameter
- * @param parameter The parameter used in the operation
- */
-function validateOperationParameter(
-  program: Program,
-  operation: Operation,
-  parameter: ModelProperty
-) {
-  const allVersions = getAllVersions(program, operation);
-  if (allVersions === undefined) return;
-  const alwaysAvailMap = new Map<string, Availability>();
-  allVersions.forEach((ver) => alwaysAvailMap.set(ver.name, Availability.Available));
-
-  const operationAvailability = getAvailabilityMapWithParentInfo(program, operation);
-  const paramAvailability = getAvailabilityMapWithParentInfo(program, parameter);
-  const paramTypeAvailability = getAvailabilityMapWithParentInfo(program, parameter.type);
-  const [paramTypeNamespace] = getVersions(program, parameter.type);
-  // everything is available in all versions
-  if (
-    operationAvailability === undefined &&
-    paramAvailability === undefined &&
-    paramTypeAvailability === undefined
-  ) {
-    return;
-  }
-  // intrinstic types are always available
-  if (paramTypeNamespace === undefined) return;
-
-  // check if a parameter or parameter type is versioned but the operation is not
-  if (operationAvailability === undefined) {
-    if (paramAvailability !== undefined) {
-      reportDiagnostic(program, {
-        code: "incompatible-versioned-reference",
-        messageId: "default",
-        format: {
-          sourceName: getTypeName(operation),
-          targetName: getTypeName(parameter),
-        },
-        target: operation,
-      });
-      return;
-    } else if (paramTypeAvailability !== undefined) {
-      reportDiagnostic(program, {
-        code: "incompatible-versioned-reference",
-        messageId: "default",
-        format: {
-          sourceName: getTypeName(operation),
-          targetName: getTypeName(parameter.type),
-        },
-        target: operation,
-      });
-      return;
-    }
-  }
-}
-
 /**
  * Validate the target reference versioning is compatible with the source versioning.
  * This will also validate any template arguments used in the reference.
@@ -792,24 +717,6 @@ function validateAvailabilityForRef(
         target: source,
       });
     }
-    if (
-      [Availability.Unavailable].includes(sourceVal) &&
-      [Availability.Added, Availability.Available].includes(targetVal)
-    ) {
-      const targetAddedOn = findAvailabilityAfterVersion(key, Availability.Added, sourceAvail);
-
-      reportDiagnostic(program, {
-        code: "incompatible-versioned-reference",
-        messageId: "addedAfter",
-        format: {
-          sourceName: getTypeName(target),
-          targetName: getTypeName(source),
-          sourceAddedOn: key,
-          targetAddedOn: targetAddedOn!,
-        },
-        target: target,
-      });
-    }
     if (
       [Availability.Removed].includes(sourceVal) &&
       [Availability.Unavailable].includes(targetVal)

diff --git a/packages/versioning/test/incompatible-versioning.test.ts b/packages/versioning/test/incompatible-versioning.test.ts
@@ -5,6 +5,7 @@ import {
   expectDiagnostics,
   TestHost,
 } from "@typespec/compiler/testing";
+import { ok } from "assert";
 import { createVersioningTestHost, createVersioningTestRunner } from "./test-host.js";
 
 describe("versioning: incompatible use of decorators", () => {
@@ -103,7 +104,8 @@ describe("versioning: validate incompatible references", () => {
   });
 
   describe("operation", () => {
-    it("emit diagnostic when unversioned op has a versioned model as a parameter", async () => {
+    // TODO See: https://github.com/microsoft/typespec/issues/2695
+    it.skip("emit diagnostic when unversioned op has a versioned model as a parameter", async () => {
       const diagnostics = await runner.diagnose(`
         @added(Versions.v2)
         model Foo {}
@@ -117,20 +119,18 @@ describe("versioning: validate incompatible references", () => {
       });
     });
 
-    it("emit diagnostic when unversioned op has a versioned parameter", async () => {
-      const diagnostics = await runner.diagnose(`
+    it("allow unversioned op to have a versioned parameter", async () => {
+      ok(
+        await runner.compile(`
         model Foo {}
 
         op test(param: string, @added(Versions.v2) newParam: Foo): void;
-      `);
-      expectDiagnostics(diagnostics, {
-        code: "@typespec/versioning/incompatible-versioned-reference",
-        message:
-          "'TestService.test' is referencing versioned type 'TestService.(anonymous model).newParam' but is not versioned itself.",
-      });
+      `)
+      );
     });
 
-    it("emit diagnostic when unversioned op based on a template has a versioned model as a parameter", async () => {
+    // TODO See: https://github.com/microsoft/typespec/issues/2695
+    it.skip("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 {}
@@ -146,16 +146,17 @@ describe("versioning: validate incompatible references", () => {
       });
     });
 
-    it("emit diagnostic when type changed to types that don't exist", async () => {
+    // TODO See: https://github.com/microsoft/typespec/issues/2695
+    it.skip("emit diagnostic when type changed to types that don't exist", async () => {
       const diagnostics = await runner.diagnose(`
       @added(Versions.v3)  
       model Foo {}
 
       @removed(Versions.v1)
       model Doo {}
 
-        @added(Versions.v3)
-        op test(@typeChangedFrom(Versions.v2, Doo) param: Foo): void;
+      @added(Versions.v3)
+      op test(@typeChangedFrom(Versions.v2, Doo) param: Foo): void;
       `);
       expectDiagnostics(diagnostics, [
         {