Fix missing parameters in operation object (#1090)

Author: drwpow

CONTRIBUTING.md

@@ -40,15 +40,17 @@ This will compile the code as you change automatically.
 
 **Please fill out the template!** It’s a very lightweight template 🙂.
 
-### TDD
+### Use Test-driven Development!
 
-This library is a great usecase for [test-driven development (TDD)](https://en.wikipedia.org/wiki/Test-driven_development). If you’re new to this, the basic workflow is:
+Contributing to this library is hard-bordering-on-impossible without a [test-driven development (TDD)](https://en.wikipedia.org/wiki/Test-driven_development) strategy. If you’re new to this, the basic workflow is:
 
 1. First, write a [test](#testing) that fully outlines what you’d _like_ the output to be.
-2. Make sure this test **fails** when you run `npm test` (yes, _fails!_)
+2. Next, make sure this test **fails** when you run `npm test` (yes, _fails!_)
 3. Then, make changes to `src/` until the tests pass.
 
-_Code generation is hard!_ And for that reason, starting with a very clear expectation of your end-goal can make working easier.
+Reasoning about code generation can be quite difficult until you “invert your thinking” and approach it output-first. Adopting TDD can turn very unclear/abstract problems into concrete ones with clear steps to resolution.
+
+✨ When starting any task, **write a failing test first!** ✨
 
 #### Updating snapshot tests
 

src/transform/path-item-object.ts

@@ -24,25 +24,32 @@ export default function transformPathItemObject(
     if (!operationObject) continue;
     const c = getSchemaObjectComment(operationObject, indentLv);
     if (c) output.push(indent(c, indentLv));
+
+    // fold top-level PathItem parameters into method-level, with the latter overriding the former
+    const keyedParameters: Record<string, ParameterObject | ReferenceObject> = {};
+    if (!("$ref" in operationObject)) {
+      // important: OperationObject parameters come last, and will override any conflicts with PathItem parameters
+      for (const parameter of [...(pathItem.parameters ?? []), ...(operationObject.parameters ?? [])]) {
+        // note: the actual key doesn’t matter here, as long as it can match between PathItem and OperationObject
+        keyedParameters["$ref" in parameter ? parameter.$ref : parameter.name] = parameter;
+      }
+    }
+
     if ("$ref" in operationObject) {
       output.push(indent(`${method}: ${operationObject.$ref}`, indentLv));
     }
     // if operationId exists, move into an `operations` export and pass the reference in here
     else if (operationObject.operationId) {
-      const operationType = transformOperationObject(operationObject, { path, ctx: { ...ctx, indentLv: 1 } });
+      const operationType = transformOperationObject(
+        { ...operationObject, parameters: Object.values(keyedParameters) },
+        { path, ctx: { ...ctx, indentLv: 1 } }
+      );
       ctx.operations[operationObject.operationId] = {
         operationType,
         comment: getSchemaObjectComment(operationObject, 1),
       };
       output.push(indent(`${method}: operations[${escStr(operationObject.operationId)}];`, indentLv));
     } else {
-      // fold top-level PathItem parameters into method-level, with the latter overriding the former
-      const keyedParameters: Record<string, ParameterObject | ReferenceObject> = {};
-      // important: OperationObject parameters come last, and will override any conflicts with PathItem parameters
-      for (const parameter of [...(pathItem.parameters ?? []), ...(operationObject.parameters ?? [])]) {
-        // note: the actual key doesn’t matter here, as long as it can match between PathItem and OperationObject
-        keyedParameters["$ref" in parameter ? parameter.$ref : parameter.name] = parameter;
-      }
       const operationType = transformOperationObject(
         { ...operationObject, parameters: Object.values(keyedParameters) },
         { path, ctx: { ...ctx, indentLv } }

src/types.ts

@@ -276,7 +276,7 @@ export interface RequestBodyObject extends Extensable {
  */
 export interface MediaTypeObject extends Extensable {
   /** The schema defining the content of the request, response, or parameter. */
-  schema?: SchemaObject;
+  schema?: SchemaObject | ReferenceObject;
   /** Example of the media type. The example object SHOULD be in the correct format as specified by the media type. The example field is mutually exclusive of the examples field. Furthermore, if referencing a schema which contains an example, the example value SHALL override the example provided by the schema. */
   example?: any;
   /** Examples of the media type. Each example object SHOULD match the media type and specified schema if present. The examples field is mutually exclusive of the example field. Furthermore, if referencing a schema which contains an example, the examples value SHALL override the example provided by the schema. */

test/index.test.ts

@@ -33,6 +33,7 @@ describe("openapiTS", () => {
   });
 
   describe("3.0", () => {
+    /** test that custom $refs are ignored rather than throw errors */
     test("custom properties", async () => {
       const generated = await openapiTS({
         openapi: "3.0",
@@ -74,6 +75,7 @@ export type operations = Record<string, never>;
 `);
     });
 
+    /** test that component examples aren’t parsed (they may be invalid / pseudocode) */
     test("components.examples are skipped", async () => {
       const generated = await openapiTS({
         openapi: "3.0",
@@ -124,6 +126,7 @@ export type operations = Record<string, never>;
 `);
     });
 
+    /** test that $ref’d parameters make it into the paths object correctly */
     test("parameter $refs", async () => {
       const generated = await openapiTS(new URL("./fixtures/parameters-test.yaml", import.meta.url));
       expect(generated).toBe(`${BOILERPLATE}
@@ -177,6 +180,115 @@ export type operations = Record<string, never>;
 `);
     });
 
+    /** test that $ref’d parameters, operation parameters, and method parameters all make it into the operation object correctly */
+    test("operations keep all parameters", async () => {
+      const schema: OpenAPI3 = {
+        openapi: "3.0",
+        info: { title: "Test", version: "1.0" },
+        paths: {
+          "/post/{id}": {
+            get: {
+              operationId: "getPost",
+              parameters: [
+                { name: "format", in: "query", schema: { type: "string" } },
+                { $ref: "#/components/parameters/post_id" },
+              ],
+              responses: {
+                200: {
+                  description: "OK",
+                  content: {
+                    "application/json": { schema: { $ref: "#/components/schemas/Post" } },
+                  },
+                },
+              },
+            },
+            parameters: [{ name: "revision", in: "query", schema: { type: "number" } }],
+          },
+        },
+        components: {
+          schemas: {
+            Post: {
+              type: "object",
+              properties: {
+                id: { type: "number" },
+                title: { type: "string" },
+                body: { type: "string" },
+                published_at: { type: "number" },
+              },
+              required: ["id", "title", "body"],
+            },
+          },
+          parameters: {
+            post_id: {
+              name: "post_id",
+              in: "path",
+              schema: { type: "string" },
+              required: true,
+            },
+          },
+        },
+      };
+      const generated = await openapiTS(schema);
+      expect(generated).toBe(`${BOILERPLATE}
+export interface paths {
+  "/post/{id}": {
+    get: operations["getPost"];
+    parameters: {
+      query: {
+        revision?: number;
+      };
+    };
+  };
+}
+
+export type webhooks = Record<string, never>;
+
+export interface components {
+  schemas: {
+    Post: {
+      id: number;
+      title: string;
+      body: string;
+      published_at?: number;
+    };
+  };
+  responses: never;
+  parameters: {
+    post_id: string;
+  };
+  requestBodies: never;
+  headers: never;
+  pathItems: never;
+}
+
+export type external = Record<string, never>;
+
+export interface operations {
+
+  getPost: {
+    parameters: {
+      query: {
+        revision?: number;
+        format?: string;
+      };
+      path: {
+        post_id: components["parameters"]["post_id"];
+      };
+    };
+    responses: {
+      /** @description OK */
+      200: {
+        content: {
+          "application/json": components["schemas"]["Post"];
+        };
+      };
+    };
+  };
+}
+`);
+    });
+
+    /** test that remote $refs are loaded correctly */
     test("remote $refs", async () => {
       const generated = await openapiTS(new URL("./fixtures/remote-ref-test.yaml", import.meta.url));
       expect(generated).toBe(`${BOILERPLATE}