[HTTP] Don't allow multiple properties in a response to be annotated …

…with `@statusCode` (#2454) Fix #2347. The fix was surprisingly less involved than I expected. **BREAKING CHANGE** since this will now fail instead of outputting nonsense.
microsoft · Sep 27, 2023 · a096823 · a096823
1 parent b9452d0
commit a096823
Show file tree

Hide file tree

Showing 6 changed files with 79 additions and 31 deletions.
diff --git a/common/changes/@typespec/http/openapi3-DuplicateRefs_2023-09-27-19-57.json b/common/changes/@typespec/http/openapi3-DuplicateRefs_2023-09-27-19-57.json
@@ -0,0 +1,10 @@
+{
+  "changes": [
+    {
+      "packageName": "@typespec/http",
+      "comment": "Emit error when multiple properties on a response model have the `@statusCode` decorator.",
+      "type": "none"
+    }
+  ],
+  "packageName": "@typespec/http"
+}
diff --git a/common/changes/@typespec/openapi3/openapi3-DuplicateRefs_2023-09-27-19-57.json b/common/changes/@typespec/openapi3/openapi3-DuplicateRefs_2023-09-27-19-57.json
@@ -0,0 +1,10 @@
+{
+  "changes": [
+    {
+      "packageName": "@typespec/openapi3",
+      "comment": "",
+      "type": "none"
+    }
+  ],
+  "packageName": "@typespec/openapi3"
+}
diff --git a/packages/http/src/lib.ts b/packages/http/src/lib.ts
@@ -62,6 +62,12 @@ export const $lib = createTypeSpecLibrary({
         default: paramMessage`Duplicate operation "${"operationName"}" routed at "${"verb"} ${"path"}".`,
       },
     },
+    "multiple-status-codes": {
+      severity: "error",
+      messages: {
+        default: "Multiple `@statusCode` decorators defined for this operation response.",
+      },
+    },
     "status-code-invalid": {
       severity: "error",
       messages: {

diff --git a/packages/http/src/responses.ts b/packages/http/src/responses.ts
@@ -25,7 +25,7 @@ import {
   isHeader,
   isStatusCode,
 } from "./decorators.js";
-import { createDiagnostic } from "./lib.js";
+import { createDiagnostic, reportDiagnostic } from "./lib.js";
 import { gatherMetadata, isApplicableMetadata, Visibility } from "./metadata.js";
 import { HttpOperationResponse } from "./types.js";
 
@@ -131,9 +131,18 @@ function getResponseStatusCodes(
 ): string[] {
   const codes: string[] = [];
 
+  let statusFound = false;
   for (const prop of metadata) {
     if (isStatusCode(program, prop)) {
-      codes.push(...getStatusCodes(program, prop));
+      if (statusFound) {
+        reportDiagnostic(program, {
+          code: "multiple-status-codes",
+          target: responseType,
+        });
+      }
+      statusFound = true;
+      const propCodes = getStatusCodes(program, prop);
+      codes.push(...propCodes);
     }
   }
 

diff --git a/packages/http/test/http-decorators.test.ts b/packages/http/test/http-decorators.test.ts
@@ -424,6 +424,48 @@ describe("http: decorators", () => {
       ]);
     });
 
+    it("emits error if multiple properties are decorated with `@statusCode` in return type", async () => {
+      const diagnostics = await runner.diagnose(
+        `
+        model CreatedOrUpdatedResponse {
+          @statusCode ok: "200";
+          @statusCode created: "201";
+        }
+        model DateHeader {
+          @header date: utcDateTime;
+        }
+        model Key {
+          key: string;
+        }
+        @put op create(): CreatedOrUpdatedResponse & DateHeader & Key;
+        `
+      );
+      expectDiagnostics(diagnostics, [{ code: "@typespec/http/multiple-status-codes" }]);
+    });
+
+    it("emits error if multiple `@statusCode` decorators are composed together", async () => {
+      const diagnostics = await runner.diagnose(
+        `      
+        model CustomUnauthorizedResponse {
+          @statusCode _: 401;
+          @body body: UnauthorizedResponse;
+        }
+  
+        model Pet {
+          name: string;
+        }
+        
+        model PetList {
+          @statusCode _: 200;
+          @body body: Pet[];
+        }
+        
+        op list(): PetList | CustomUnauthorizedResponse;
+        `
+      );
+      expectDiagnostics(diagnostics, [{ code: "@typespec/http/multiple-status-codes" }]);
+    });
+
     it("set the statusCode with @statusCode", async () => {
       const { code } = await runner.compile(`
           op test(): {

diff --git a/packages/openapi3/test/return-types.test.ts b/packages/openapi3/test/return-types.test.ts
@@ -159,35 +159,6 @@ describe("openapi3: return types", () => {
     );
   });
 
-  it("defines separate responses for each status code property in return type", async () => {
-    const res = await openApiFor(
-      `
-      model CreatedOrUpdatedResponse {
-        @statusCode ok: "200";
-        @statusCode created: "201";
-      }
-      model DateHeader {
-        @header date: utcDateTime;
-      }
-      model Key {
-        key: string;
-      }
-      @put op create(): CreatedOrUpdatedResponse & DateHeader & Key;
-      `
-    );
-    ok(res.paths["/"].put.responses["200"]);
-    ok(res.paths["/"].put.responses["201"]);
-    // Note: 200 and 201 response should be equal except for description
-    deepStrictEqual(
-      res.paths["/"].put.responses["200"].headers,
-      res.paths["/"].put.responses["201"].headers
-    );
-    deepStrictEqual(
-      res.paths["/"].put.responses["200"].content,
-      res.paths["/"].put.responses["201"].content
-    );
-  });
-
   it("defines separate responses for each variant of a union return type", async () => {
     const res = await openApiFor(
       `