Codegen: better handling of duplicate param names (#3780)

Co-authored-by: Lenz Weber-Tronic <mail@lenzw.de>

Author: kahirokunn

packages/rtk-query-codegen-openapi/package.json

@@ -17,10 +17,10 @@
     "rtk-query-codegen-openapi": "lib/bin/cli.js"
   },
   "scripts": {
-    "build": "tsc",
+    "build": "tsc && chmod +x lib/bin/cli.js",
     "prepare": "npm run build && chmod +x ./lib/bin/cli.js",
     "format": "prettier --write \"src/**/*.ts\"",
-    "test:update": "lib/bin/cli.js test/fixtures/petstore.json --file test/fixtures/generated.ts -h",
+    "test:update": "jest --runInBand --updateSnapshot",
     "test:update:enum": "lib/bin/cli.js test/config.example.enum.ts",
     "test": "jest --runInBand",
     "cli": "esr src/bin/cli.ts"

packages/rtk-query-codegen-openapi/src/generate.ts

@@ -258,12 +258,27 @@ export async function generateApi(
 
     const allNames = parameters.map((p) => p.name);
     const queryArg: QueryArgDefinitions = {};
-    for (const param of parameters) {
-      const isPureSnakeCase = /^[a-zA-Z][\w]*$/.test(param.name);
-      const camelCaseName = camelCase(param.name);
-
-      const name = isPureSnakeCase && !allNames.includes(camelCaseName) ? camelCaseName : param.name;
+    function generateName(name: string, potentialPrefix: string) {
+      const isPureSnakeCase = /^[a-zA-Z][a-zA-Z0-9_]*$/.test(name);
+      // prefix with `query`, `path` or `body` if there are multiple paramters with the same name
+      const hasNamingConflict = allNames.filter((n) => n === name).length > 1;
+      if (hasNamingConflict) {
+        name = `${potentialPrefix}_${name}`;
+      }
+      // convert to camelCase if the name is pure snake_case and there are no naming conflicts
+      const camelCaseName = camelCase(name);
+      if (isPureSnakeCase && !allNames.includes(camelCaseName)) {
+        name = camelCaseName;
+      }
+      // if there are still any naming conflicts, prepend with underscore
+      while (name in queryArg) {
+        name = '_' + name;
+      }
+      return name;
+    }
 
+    for (const param of parameters) {
+      const name = generateName(param.name, param.in);
       queryArg[name] = {
         origin: 'param',
         name,
@@ -279,11 +294,7 @@ export async function generateApi(
       const schema = apiGen.getSchemaFromContent(body.content);
       const type = apiGen.getTypeFromSchema(schema);
       const schemaName = camelCase((type as any).name || getReferenceName(schema) || 'body');
-      let name = schemaName in queryArg ? 'body' : schemaName;
-
-      while (name in queryArg) {
-        name = '_' + name;
-      }
+      const name = generateName(schemaName in queryArg ? 'body' : schemaName, 'body');
 
       queryArg[name] = {
         origin: 'body',
@@ -479,6 +490,7 @@ type QueryArgDefinition = {
   originalName: string;
   type: ts.TypeNode;
   required?: boolean;
+  param?: OpenAPIV3.ParameterObject;
 } & (
   | {
       origin: 'param';

packages/rtk-query-codegen-openapi/test/__snapshots__/generateEndpoints.test.ts.snap

@@ -263,6 +263,39 @@ export type User = {
 
 `;
 
+exports[`duplicate parameter names must be prefixed with a path or query prefix 1`] = `
+import { api } from './fixtures/emptyApi';
+const injectedRtkApi = api.injectEndpoints({
+  endpoints: (build) => ({
+    patchApiV1ListByItemId: build.mutation<PatchApiV1ListByItemIdApiResponse, PatchApiV1ListByItemIdApiArg>({
+      query: (queryArg) => ({
+        url: \`/api/v1/list/\${queryArg['item.id']}\`,
+        method: 'PATCH',
+      }),
+    }),
+    patchApiV2BySomeName: build.mutation<PatchApiV2BySomeNameApiResponse, PatchApiV2BySomeNameApiArg>({
+      query: (queryArg) => ({
+        url: \`/api/v2/\${queryArg.pathSomeName}\`,
+        method: 'PATCH',
+        params: { some_name: queryArg.querySomeName },
+      }),
+    }),
+  }),
+  overrideExisting: false,
+});
+export { injectedRtkApi as enhancedApi };
+export type PatchApiV1ListByItemIdApiResponse = /** status 200 A successful response. */ string;
+export type PatchApiV1ListByItemIdApiArg = {
+  'item.id': string;
+};
+export type PatchApiV2BySomeNameApiResponse = /** status 200 A successful response. */ string;
+export type PatchApiV2BySomeNameApiArg = {
+  pathSomeName: string;
+  querySomeName: string;
+};
+
+`;
+
 exports[`endpoint filtering: should only have endpoints loginUser, placeOrder, getOrderById, deleteOrder 1`] = `
 import { api } from './fixtures/emptyApi';
 const injectedRtkApi = api.injectEndpoints({
@@ -434,6 +467,13 @@ const injectedRtkApi = api.injectEndpoints({
         method: 'PATCH',
       }),
     }),
+    patchApiV2BySomeName: build.mutation<PatchApiV2BySomeNameApiResponse, PatchApiV2BySomeNameApiArg>({
+      query: (queryArg) => ({
+        url: \`/api/v2/\${queryArg.pathSomeName}\`,
+        method: 'PATCH',
+        params: { some_name: queryArg.querySomeName },
+      }),
+    }),
   }),
   overrideExisting: false,
 });
@@ -442,6 +482,11 @@ export type PatchApiV1ListByItemIdApiResponse = /** status 200 A successful resp
 export type PatchApiV1ListByItemIdApiArg = {
   'item.id': string;
 };
+export type PatchApiV2BySomeNameApiResponse = /** status 200 A successful response. */ string;
+export type PatchApiV2BySomeNameApiArg = {
+  pathSomeName: string;
+  querySomeName: string;
+};
 
 `;
 

packages/rtk-query-codegen-openapi/test/fixtures/params.json

@@ -27,6 +27,32 @@
           }
         ]
       }
+    },
+    "/api/v2/{some_name}": {
+      "patch": {
+        "responses": {
+          "200": {
+            "description": "A successful response.",
+            "schema": {
+              "type": "string"
+            }
+          }
+        },
+        "parameters": [
+          {
+            "name": "some_name",
+            "in": "path",
+            "required": true,
+            "type": "string"
+          },
+          {
+            "name": "some_name",
+            "in": "query",
+            "required": true,
+            "type": "string"
+          }
+        ]
+      }
     }
   }
 }

packages/rtk-query-codegen-openapi/test/generateEndpoints.test.ts

@@ -229,6 +229,18 @@ test('should use brackets in a querystring urls arg, when the arg contains full
   expect(api).toMatchSnapshot();
 });
 
+test('duplicate parameter names must be prefixed with a path or query prefix', async () => {
+  const api = await generateEndpoints({
+    unionUndefined: true,
+    apiFile: './fixtures/emptyApi.ts',
+    schemaFile: resolve(__dirname, 'fixtures/params.json'),
+  });
+  // eslint-disable-next-line no-template-curly-in-string
+  expect(api).toContain('pathSomeName: string');
+  expect(api).toContain('querySomeName: string');
+  expect(api).toMatchSnapshot();
+});
+
 test('apiImport builds correct `import` statement', async () => {
   const api = await generateEndpoints({
     unionUndefined: true,