[TCGC] Change the responses and exceptions to be arrays (#1540)

Fixes #1453 This is a breaking change in TCGC, but there are some reasons of making this change: 1. The `HttpOperation.responses` from typespec core library also defines as an array. 2. The type `Map` is not persistable in typescript, which causes issues in some emitter implementation such as dotnet, because it needs to serialize everything in TCGC output and sends it to the generator written in other language (details [here](Azure/autorest.csharp#5028)) 3. In most cases, we have no more than 4 responses corresponding different status codes. In the majority of cases, we have 2 or 1. In such a small collections, maps do not have distinguishable performance improvement comparing with arrays with `find`, and arrays have the advantages above.
Azure · Sep 19, 2024 · 8adc5b1 · 8adc5b1
1 parent 354be86
commit 8adc5b1
Show file tree

Hide file tree

Showing 11 changed files with 325 additions and 249 deletions.
diff --git a/.chronus/changes/change-response-to-be-array-2024-8-14-11-27-13.md b/.chronus/changes/change-response-to-be-array-2024-8-14-11-27-13.md
@@ -0,0 +1,22 @@
+---
+changeKind: breaking
+packages:
+  - "@azure-tools/typespec-client-generator-core"
+---
+
+1. The type of `responses` and `exceptions` in `SdkHttpOperation` changed from `Map<number | HttpStatusCodeRange | "*", SdkHttpResponse>` to `SdkHttpResponse[]`.
+2. The type of `responses` in `SdkHttpOperationExample` changed from `Map<number, SdkHttpResponseExampleValue>` to `SdkHttpResponseExampleValue[]`.
+3. `SdkHttpResponse` adds a new property `statusCodes` to store its corresponding status code or status code range.
+
+Migration hints:
+The type changed from map to array, and the key of the map is moved as a new property of the value type. For example, for code like this:
+```
+for (const [statusCodes, response] of operation.responses)
+```
+you could do the same in this way:
+```
+for (const response of operation.responses)
+{
+  const statusCodes = response.statusCodes;
+}
+```
diff --git a/docs/howtos/Client Generation/07tcgcTypes.mdx b/docs/howtos/Client Generation/07tcgcTypes.mdx
@@ -911,8 +911,8 @@ export interface SdkHttpOperation {
   bodyParam: SdkBodyParameter;
   // mapping of status codes to SdkHttpResponse for valid responses
   // HttpStatusCodeRange can represent either a single status code or a range.
-  responses: Map<HttpStatusCodeRange, SdkHttpResponse>;
-  exceptions: Map<HttpStatusCodeRange, SdkHttpResponse>;
+  responses: SdkHttpResponse[];
+  exceptions: SdkHttpResponse[];
   examples?: SdkHttpOperationExample[];
 }
 ```
@@ -1277,6 +1277,7 @@ interface HttpStatusCodeRange {
 
 export interface SdkHttpResponse {
   kind: "http";
+  statusCodes: number | HttpStatusCodeRange | "*";
   headers: SdkServiceResponseHeader[];
   apiVersions: string[];
   type?: SdkType;
@@ -1527,7 +1528,7 @@ interface SdkExampleBase {
 export interface SdkHttpOperationExample extends SdkExampleBase {
   kind: "http";
   parameters: SdkHttpParameterExampleValue[];
-  responses: Map<number, SdkHttpResponseExampleValue>;
+  responses: SdkHttpResponseExampleValue[];
 }
 
 export interface SdkHttpParameterExampleValue {
@@ -1537,6 +1538,7 @@ export interface SdkHttpParameterExampleValue {
 
 export interface SdkHttpResponseExampleValue {
   response: SdkHttpResponse;
+  statusCode: number;
   headers: SdkHttpResponseHeaderExampleValue[];
   bodyValue?: SdkExampleValue;
 }

diff --git a/packages/typespec-client-generator-core/src/example.ts b/packages/typespec-client-generator-core/src/example.ts
@@ -9,7 +9,6 @@ import {
   isService,
   resolvePath,
 } from "@typespec/compiler";
-import { HttpStatusCodeRange } from "@typespec/http";
 import { getOperationId } from "@typespec/openapi";
 import {
   SdkArrayExampleValue,
@@ -291,12 +290,12 @@ function handleHttpParameters(
 }
 
 function handleHttpResponses(
-  responses: Map<number | HttpStatusCodeRange, SdkHttpResponse>,
+  responses: SdkHttpResponse[],
   example: any,
   relativePath: string
-): [Map<number, SdkHttpResponseExampleValue>, readonly Diagnostic[]] {
+): [SdkHttpResponseExampleValue[], readonly Diagnostic[]] {
   const diagnostics = createDiagnosticCollector();
-  const responseExamples = new Map<number, SdkHttpResponseExampleValue>();
+  const responseExamples: SdkHttpResponseExampleValue[] = [];
   if (
     "responses" in example &&
     typeof example.responses === "object" &&
@@ -305,11 +304,13 @@ function handleHttpResponses(
     for (const code of Object.keys(example.responses)) {
       const statusCode = parseInt(code, 10);
       let found = false;
-      for (const [responseCode, response] of responses.entries()) {
+      for (const response of responses) {
+        const responseCode = response.statusCodes;
         if (responseCode === statusCode) {
-          responseExamples.set(
-            statusCode,
-            diagnostics.pipe(handleHttpResponse(response, example.responses[code], relativePath))
+          responseExamples.push(
+            diagnostics.pipe(
+              handleHttpResponse(response, statusCode, example.responses[code], relativePath)
+            )
           );
           found = true;
           break;
@@ -319,9 +320,10 @@ function handleHttpResponses(
           responseCode.start <= statusCode &&
           responseCode.end >= statusCode
         ) {
-          responseExamples.set(
-            statusCode,
-            diagnostics.pipe(handleHttpResponse(response, example.responses[code], relativePath))
+          responseExamples.push(
+            diagnostics.pipe(
+              handleHttpResponse(response, statusCode, example.responses[code], relativePath)
+            )
           );
           found = true;
           break;
@@ -341,12 +343,14 @@ function handleHttpResponses(
 
 function handleHttpResponse(
   response: SdkHttpResponse,
+  statusCode: number,
   example: any,
   relativePath: string
 ): [SdkHttpResponseExampleValue, readonly Diagnostic[]] {
   const diagnostics = createDiagnosticCollector();
   const responseExample: SdkHttpResponseExampleValue = {
     response,
+    statusCode,
     headers: [],
   };
   if (typeof example === "object" && example !== null) {

diff --git a/packages/typespec-client-generator-core/src/http.ts b/packages/typespec-client-generator-core/src/http.ts
@@ -15,7 +15,6 @@ import {
   HttpOperationParameter,
   HttpOperationPathParameter,
   HttpOperationQueryParameter,
-  HttpStatusCodeRange,
   getHeaderFieldName,
   getHeaderFieldOptions,
   getPathParamName,
@@ -412,14 +411,14 @@ function getSdkHttpResponseAndExceptions(
   httpOperation: HttpOperation
 ): [
   {
-    responses: Map<HttpStatusCodeRange | number, SdkHttpResponse>;
-    exceptions: Map<HttpStatusCodeRange | number | "*", SdkHttpResponse>;
+    responses: SdkHttpResponse[];
+    exceptions: SdkHttpResponse[];
   },
   readonly Diagnostic[],
 ] {
   const diagnostics = createDiagnosticCollector();
-  const responses: Map<HttpStatusCodeRange | number, SdkHttpResponse> = new Map();
-  const exceptions: Map<HttpStatusCodeRange | number | "*", SdkHttpResponse> = new Map();
+  const responses: SdkHttpResponse[] = [];
+  const exceptions: SdkHttpResponse[] = [];
   for (const response of httpOperation.responses) {
     const headers: SdkServiceResponseHeader[] = [];
     let body: Type | undefined;
@@ -471,6 +470,7 @@ function getSdkHttpResponseAndExceptions(
       kind: "http",
       type: body ? diagnostics.pipe(getClientTypeWithDiagnostics(context, body)) : undefined,
       headers,
+      statusCodes: response.statusCodes,
       contentTypes: contentTypes.length > 0 ? contentTypes : undefined,
       defaultContentType: contentTypes.includes("application/json")
         ? "application/json"
@@ -482,10 +482,10 @@ function getSdkHttpResponseAndExceptions(
       ),
       description: response.description,
     };
-    if (response.statusCodes === "*" || (body && isErrorModel(context.program, body))) {
-      exceptions.set(response.statusCodes, sdkResponse);
+    if (sdkResponse.statusCodes === "*" || (body && isErrorModel(context.program, body))) {
+      exceptions.push(sdkResponse);
     } else {
-      responses.set(response.statusCodes, sdkResponse);
+      responses.push(sdkResponse);
     }
   }
   return diagnostics.wrap({ responses, exceptions });

diff --git a/packages/typespec-client-generator-core/src/interfaces.ts b/packages/typespec-client-generator-core/src/interfaces.ts
@@ -550,6 +550,7 @@ export interface SdkHttpResponse extends SdkServiceResponse {
   contentTypes?: string[];
   defaultContentType?: string;
   description?: string;
+  statusCodes: number | HttpStatusCodeRange | "*";
 }
 
 interface SdkServiceOperationBase {}
@@ -564,8 +565,8 @@ export interface SdkHttpOperation extends SdkServiceOperationBase {
   verb: HttpVerb;
   parameters: (SdkPathParameter | SdkQueryParameter | SdkHeaderParameter)[];
   bodyParam?: SdkBodyParameter;
-  responses: Map<HttpStatusCodeRange | number, SdkHttpResponse>;
-  exceptions: Map<HttpStatusCodeRange | number | "*", SdkHttpResponse>;
+  responses: SdkHttpResponse[];
+  exceptions: SdkHttpResponse[];
   examples?: SdkHttpOperationExample[];
 }
 
@@ -701,7 +702,7 @@ interface SdkExampleBase {
 export interface SdkHttpOperationExample extends SdkExampleBase {
   kind: "http";
   parameters: SdkHttpParameterExampleValue[];
-  responses: Map<number, SdkHttpResponseExampleValue>;
+  responses: SdkHttpResponseExampleValue[];
 }
 
 export interface SdkHttpParameterExampleValue {
@@ -711,6 +712,7 @@ export interface SdkHttpParameterExampleValue {
 
 export interface SdkHttpResponseExampleValue {
   response: SdkHttpResponse;
+  statusCode: number;
   headers: SdkHttpResponseHeaderExampleValue[];
   bodyValue?: SdkExampleValue;
 }

diff --git a/packages/typespec-client-generator-core/src/internal-utils.ts b/packages/typespec-client-generator-core/src/internal-utils.ts
@@ -28,7 +28,6 @@ import {
   HttpOperationBody,
   HttpOperationMultipartBody,
   HttpOperationResponseContent,
-  HttpStatusCodeRange,
 } from "@typespec/http";
 import { getAddedOnVersions, getRemovedOnVersions, getVersions } from "@typespec/versioning";
 import { getParamAlias } from "./decorators.js";
@@ -397,15 +396,13 @@ export function getNullOption(type: Union): Type | undefined {
   return [...type.variants.values()].map((x) => x.type).filter((t) => isNullType(t))[0];
 }
 
-export function getAllResponseBodiesAndNonBodyExists(
-  responses: Map<HttpStatusCodeRange | number | "*", SdkHttpResponse>
-): {
+export function getAllResponseBodiesAndNonBodyExists(responses: SdkHttpResponse[]): {
   allResponseBodies: SdkType[];
   nonBodyExists: boolean;
 } {
   const allResponseBodies: SdkType[] = [];
   let nonBodyExists = false;
-  for (const response of responses.values()) {
+  for (const response of responses) {
     if (response.type) {
       if (response.type.kind === "nullable") {
         nonBodyExists = true;
@@ -418,9 +415,7 @@ export function getAllResponseBodiesAndNonBodyExists(
   return { allResponseBodies, nonBodyExists };
 }
 
-export function getAllResponseBodies(
-  responses: Map<HttpStatusCodeRange | number | "*", SdkHttpResponse>
-): SdkType[] {
+export function getAllResponseBodies(responses: SdkHttpResponse[]): SdkType[] {
   return getAllResponseBodiesAndNonBodyExists(responses).allResponseBodies;
 }