fix(core): dotted path parameter names generate malformed requests (#3741)

Author: daugvinasr

packages/core/src/getters/keys.test.ts

@@ -1,8 +1,20 @@
 import { describe, expect, it } from 'vitest';
 
-import { getKey } from './keys';
+import { getKey, getPropertyAccessor } from './keys';
 
 describe('getKey', () => {
+  it('leaves a valid identifier name unchanged', () => {
+    expect(getKey('scopeId')).toBe('scopeId');
+  });
+
+  it('quotes a dotted key', () => {
+    expect(getKey('scope.id')).toBe("'scope.id'");
+  });
+
+  it('quotes a dashed key', () => {
+    expect(getKey('user-id')).toBe("'user-id'");
+  });
+
   it('escapes single quote in key', () => {
     const result = getKey("x':[require('fs').execSync('id'),");
     expect(result).toMatch(/^'(.*)'$/);
@@ -15,3 +27,17 @@ describe('getKey', () => {
     expect(result).toBe(String.raw`'a\\b'`);
   });
 });
+
+describe('getPropertyAccessor', () => {
+  it('uses dot access for a valid identifier name', () => {
+    expect(getPropertyAccessor('scopeId')).toBe('.scopeId');
+  });
+
+  it('uses quoted bracket access for a dotted name', () => {
+    expect(getPropertyAccessor('scope.id')).toBe("['scope.id']");
+  });
+
+  it('escapes quotes inside a bracket-access name', () => {
+    expect(getPropertyAccessor("it's")).toBe(String.raw`['it\'s']`);
+  });
+});

packages/core/src/getters/keys.ts

@@ -7,3 +7,12 @@ export function getKey(key: string) {
     ? key
     : `'${jsStringLiteralEscape(key)}'`;
 }
+
+/**
+ * Emits a property access for a possibly non-identifier name: dot access for
+ * valid identifier names (`.petId`), quoted bracket access otherwise
+ * (`['scope.id']`).
+ */
+export function getPropertyAccessor(name: string) {
+  return keyword.isIdentifierNameES5(name) ? `.${name}` : `[${getKey(name)}]`;
+}

packages/core/src/getters/params.test.ts

@@ -0,0 +1,56 @@
+import { describe, expect, it } from 'vitest';
+
+import { createTestContextSpec } from '../test-utils/context';
+import type { GetterParameters } from '../types';
+import { getParams } from './params';
+
+const context = createTestContextSpec();
+
+const pathParam = (name: string): GetterParameters['path'][number] => ({
+  parameter: { name, in: 'path', required: true, schema: { type: 'string' } },
+  imports: [],
+});
+
+describe('getParams getter', () => {
+  it('matches a dotted spec name to its generated identifier in the route', () => {
+    const params = getParams({
+      route: '/api/${scopeId}/items',
+      pathParams: [pathParam('scope.id')],
+      operationId: 'getItems',
+      context,
+      output: context.output,
+    });
+
+    expect(params).toHaveLength(1);
+    expect(params[0].name).toBe('scopeId');
+    expect(params[0].implementation).toBe('scopeId: string');
+  });
+
+  it('throws when a route param has no matching spec parameter', () => {
+    expect(() =>
+      getParams({
+        route: '/api/${scopeId}',
+        pathParams: [pathParam('other')],
+        operationId: 'getItems',
+        context,
+        output: context.output,
+      }),
+    ).toThrow(
+      "The path params scopeId can't be found in parameters (getItems)",
+    );
+  });
+
+  it('throws when two spec names collide on the same generated identifier', () => {
+    expect(() =>
+      getParams({
+        route: '/api/${scopeId}',
+        pathParams: [pathParam('scope.id'), pathParam('scope_id')],
+        operationId: 'getItems',
+        context,
+        output: context.output,
+      }),
+    ).toThrow(
+      "Path parameters 'scope.id', 'scope_id' all map to the same generated identifier 'scopeId' (getItems). Rename them so they don't collide.",
+    );
+  });
+});

packages/core/src/getters/params.ts

@@ -5,7 +5,8 @@ import type {
   GetterParams,
   NormalizedOutputOptions,
 } from '../types';
-import { camel, sanitize, stringify } from '../utils';
+import { stringify } from '../utils';
+import { camelPathParamName } from './route';
 
 /**
  * Return every params in a path
@@ -36,6 +37,41 @@ interface GetParamsOptions {
   output: NormalizedOutputOptions;
 }
 
+/**
+ * Resolves a route placeholder to its single matching spec path parameter.
+ * `identifier` already is the generated JS identifier (it comes from the
+ * processed route), so we re-derive the same identifier from each spec name via
+ * `camelPathParamName` to match. Throws when two spec names collapse onto the
+ * same identifier, or when none match.
+ */
+function resolvePathParam(
+  identifier: string,
+  pathParams: GetterParameters['query'],
+  operationId: string,
+): GetterParameters['query'][number] {
+  const matching = pathParams.filter(
+    ({ parameter }) => camelPathParamName(parameter.name ?? '') === identifier,
+  );
+
+  if (matching.length > 1) {
+    const names = matching
+      .map(({ parameter }) => `'${parameter.name}'`)
+      .join(', ');
+    throw new Error(
+      `Path parameters ${names} all map to the same generated identifier '${identifier}' (${operationId}). Rename them so they don't collide.`,
+    );
+  }
+
+  const pathParam = matching[0];
+  if (!pathParam) {
+    throw new Error(
+      `The path params ${identifier} can't be found in parameters (${operationId})`,
+    );
+  }
+
+  return pathParam;
+}
+
 export function getParams({
   route,
   pathParams = [],
@@ -45,28 +81,15 @@ export function getParams({
 }: GetParamsOptions): GetterParams {
   const params = getParamsInPath(route);
   return params.map((p) => {
-    const pathParam = pathParams.find(
-      ({ parameter }) =>
-        sanitize(camel(parameter.name), {
-          es5keyword: true,
-          underscore: true,
-          dash: true,
-        }) === p,
-    );
-
-    if (!pathParam) {
-      throw new Error(
-        `The path params ${p} can't be found in parameters (${operationId})`,
-      );
-    }
+    const pathParam = resolvePathParam(p, pathParams, operationId);
 
     const {
       name: nameWithoutSanitize,
       required = false,
       schema,
     } = pathParam.parameter;
 
-    const name = sanitize(camel(nameWithoutSanitize), { es5keyword: true });
+    const name = camelPathParamName(nameWithoutSanitize ?? '');
 
     if (!schema) {
       return {

packages/core/src/getters/route.test.ts

@@ -18,8 +18,20 @@ import {
 describe('getRoute getter', () => {
   for (const [input, expected] of [
     ['/api/test/{id}', '/api/test/${id}'],
+    // A malformed spec path without a leading slash is normalized.
+    ['api/test/{id}', '/api/test/${id}'],
     ['/api/test/{path*}', '/api/test/${path}'],
     ['/api/test/{user_id}', '/api/test/${userId}'],
+    // Matches the identifier generated for the function argument
+    // (camelPathParamName), which strips the leading underscore.
+    ['/api/test/{_id}', '/api/test/${id}'],
+    ['/api/test/{scope.id}', '/api/test/${scopeId}'],
+    // A malformed empty `{}` stays literal instead of emitting `${}`.
+    ['/api/test/{}/x', '/api/test/{}/x'],
+    [
+      '/api/v1/{scope.id}/items/{item.name}',
+      '/api/v1/${scopeId}/items/${itemName}',
+    ],
     ['/api/test/{locale}.js', '/api/test/${locale}.js'],
     ['/api/test/i18n-{locale}.js', '/api/test/i18n-${locale}.js'],
     ['/api/test/{param1}-{param2}.js', '/api/test/${param1}-${param2}.js'],

packages/core/src/getters/route.ts

@@ -31,61 +31,70 @@ function runtimeExpressionToUrlPrefix(expression: string): string {
   return '${' + t + '}';
 }
 
-const hasParam = (path: string): boolean => /[^{]*{[\w*_-]*}.*/.test(path);
+// Matches a `{name}` path-parameter template and captures the name
+// (`{petId}`, `{user_id}`, `{scope.id}`, `{kebab-case}`, `{path*}`). The
+// (?<!\$) guard is shared policy for every consumer (template-literal,
+// Hono and MSW routes): a `${...}` block in a spec path is never treated
+// as an OpenAPI param — it stays literal text in the emitted route. The name
+// must be non-empty so a malformed `{}` also stays literal instead of
+// emitting an invalid `${}` interpolation.
+const PATH_PARAM_REGEX = /(?<!\$)\{([\w.*-]+)\}/g;
+
+// Spec paths are required to start with `/`, but malformed specs without it
+// are tolerated by normalizing here.
+const ensureLeadingSlash = (path: string): string =>
+  path && !path.startsWith('/') ? `/${path}` : path;
 
-const esc = (str: string) => jsesc(str, { quotes: 'backtick', wrap: false });
+/**
+ * Sanitizes an OpenAPI path-parameter name while preserving the spec's
+ * spelling: keeps word characters, underscores, dashes and dots, strips
+ * everything else, and prefixes ES5 keywords with an underscore. Use this
+ * when the emitted name must match the spec (e.g. Hono routes).
+ */
+export const sanitizePathParamName = (name: string): string =>
+  sanitize(name, { es5keyword: true, underscore: true, dash: true, dot: true });
 
-const getRoutePath = (path: string): string => {
-  // Don't treat ${...} as an OpenAPI path param — the $ makes it literal text,
-  // not a {param} template. Escape the ${...} block and continue processing
-  // any legitimate {param} segments after it.
-  const braceIdx = path.indexOf('{');
-  if (braceIdx > 0 && path[braceIdx - 1] === '$') {
-    const closeIdx = path.indexOf('}', braceIdx);
-    if (closeIdx === -1) return esc(path);
-    const before = esc(path.slice(0, closeIdx + 1));
-    const rest = path.slice(closeIdx + 1);
-    return hasParam(rest)
-      ? `${before}${getRoutePath(rest)}`
-      : `${before}${esc(rest)}`;
-  }
+/**
+ * Derives the generated JS identifier for an OpenAPI path-parameter name
+ * (`scope.id` → `scopeId`, `_id` → `id`, `class` → `_class`). This is the
+ * single source of truth for param variable names: the emitted route
+ * interpolations, the generated function arguments and the spec-parameter
+ * matching must all agree on it.
+ */
+export const camelPathParamName = (name: string): string =>
+  sanitize(camel(name), { es5keyword: true });
 
-  const matches = /([^{]*){?([\w*_-]*)}?(.*)/.exec(path);
-  if (!matches?.length) return esc(path);
-
-  const prev = matches[1];
-  const rawParam = matches[2];
-  const rest = matches[3];
-  const param = sanitize(camel(rawParam), {
-    es5keyword: true,
-    underscore: true,
-    dash: true,
-    dot: true,
-  });
-  const next = hasParam(rest) ? getRoutePath(rest) : esc(rest);
-
-  return hasParam(path)
-    ? `${esc(prev)}\${${param}}${next}`
-    : `${esc(prev)}${param}${next}`;
-};
+/**
+ * Converts every `{param}` in an OpenAPI path to `:param` (Hono/MSW style
+ * routes). `formatParamName` maps the raw OpenAPI parameter name to the
+ * emitted one (`sanitizePathParamName` or `camelPathParamName`).
+ */
+export const toColonRoutePath = (
+  path: string,
+  formatParamName: (rawName: string) => string,
+): string =>
+  ensureLeadingSlash(path).replaceAll(
+    PATH_PARAM_REGEX,
+    (_, name: string) => `:${formatParamName(name)}`,
+  );
+
+const esc = (str: string) => jsesc(str, { quotes: 'backtick', wrap: false });
 
 /**
  * Converts an OpenAPI path (`{param}`) to a template-literal route (`${param}`),
- * escaping static segments with jsesc for safe embedding in backtick strings.
- * The `route` arg must be a raw OpenAPI path.
+ * escaping static text with jsesc for safe embedding in backtick strings.
+ * The `route` arg must be a raw OpenAPI path; a non-empty route always emits
+ * with a leading `/`.
  */
 export function getRoute(route: string) {
-  const splittedRoute = route.split('/');
-
-  let result = '';
-  for (const [i, path] of splittedRoute.entries()) {
-    if (!path && !i) {
-      continue;
-    }
-
-    result += path.includes('{') ? `/${getRoutePath(path)}` : `/${esc(path)}`;
-  }
-  return result;
+  // Splitting on the capture group leaves param names at odd indices and
+  // literal text at even indices. `${...}` blocks in the spec path fall into
+  // the literal parts (via the lookbehind) so they are escaped, not
+  // interpolated.
+  return ensureLeadingSlash(route)
+    .split(PATH_PARAM_REGEX)
+    .map((part, i) => (i % 2 ? `\${${camelPathParamName(part)}}` : esc(part)))
+    .join('');
 }
 
 /**

packages/hono/src/index.ts

@@ -1,7 +1,7 @@
 import nodePath from 'node:path';
 
 import {
-  camel,
+  camelPathParamName,
   type ClientBuilder,
   type ClientExtraFilesBuilder,
   type ClientFooterBuilder,
@@ -24,8 +24,9 @@ import {
   type NormalizedMutator,
   type NormalizedOutputOptions,
   type OpenApiInfoObject,
+  getKey,
   pascal,
-  sanitize,
+  sanitizePathParamName,
   getImportExtension,
   type Tsconfig,
   upath,
@@ -730,12 +731,14 @@ const getContext = (verbOption: GeneratorVerbOptions) => {
   if (verbOption.params.length > 0) {
     const params = getParamsInPath(verbOption.pathRoute).map((name) => {
       const param = verbOption.params.find(
-        (p) => p.name === sanitize(camel(name), { es5keyword: true }),
+        (p) => p.name === camelPathParamName(name),
       );
       const definition = param?.definition.split(':')[1];
       const required = param?.required ?? false;
+      // The key must be the same name the emitted route uses (`:name`), which
+      // is the sanitized spec name — that is the key Hono's runtime exposes.
       return {
-        definition: `${name}${required ? '' : '?'}:${definition}`,
+        definition: `${getKey(sanitizePathParamName(name))}${required ? '' : '?'}:${definition}`,
       };
     });
     paramType = `param: {\n ${params