Skip to content

Commit fb586ad

Browse files
Himenonclaude
Himenon
and
claude
authored
fix: path parameters with same name as query parameters are always required in generated TypeScript interfaces (#150)
## Summary - Added a Kubernetes-style scenario to `test/path-parameter/index.yml` that reproduces the pattern found in `connectCoreV1HeadNodeProxyWithPath` (`/api/v1/nodes/{name}/proxy/{path}`) from the Kubernetes OpenAPI spec - Fixed a bug in `generatePropertySignatures` where a same-named query parameter would overwrite a path parameter when building the TypeScript interface, causing `path?: string` instead of the correct `path: string` - Updated snapshots across all code generators (class, functional, currying-functional) to reflect the corrected output ## Root Cause `generatePropertySignatures` in `Parameter.ts` used the parameter name alone as the deduplication key when building a `typeElementMap`. When a PathItem defined both a path parameter `path` (required) and a query parameter `path` (optional) with the same name — as seen in the Kubernetes spec — the last parameter processed won, and the optional query parameter overwrote the required path parameter. ## Fix Sort parameters before reducing so that path parameters are processed last. Since the reduce uses the parameter name as the key, path parameters now always overwrite same-named non-path parameters, preserving the required status in the generated TypeScript interface. ```diff + const sorted = [...parameters].sort((a, b): number => { + const aIsPath = !Guard.isReference(a) && a.in === "path"; + const bIsPath = !Guard.isReference(b) && b.in === "path"; + if (aIsPath && !bIsPath) return 1; + if (!aIsPath && bIsPath) return -1; + return 0; + }); const typeElementMap = sorted.reduce<Record<string, string>>((all, parameter) => { ``` ## Test plan - [ ] `pnpm test:code:gen:class` — regenerate code passes - [ ] `pnpm test:code:gen:function` — regenerate code passes - [ ] `pnpm test:code:gen:currying-function` — regenerate code passes - [ ] `pnpm test:snapshot` — all 53 snapshot tests pass 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent f980cc7 commit fb586ad

17 files changed

Lines changed: 4725 additions & 4464 deletions

File tree

‎src/__tests__/generateValidRootSchema.test.ts‎

Lines changed: 0 additions & 170 deletions
This file was deleted.

‎src/code-templates/_shared/MethodBody/__tests__/PathParameter-test.ts‎

Lines changed: 27 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -5,85 +5,89 @@ import type { CodeGenerator } from "../../../../types";
55
import * as Utils from "../../utils";
66
import * as PathParameter from "../PathParameter";
77

8+
// テンプレートリテラル式の文字列表現を組み立てるヘルパー。
9+
// "$" を分離して結合することで noTemplateCurlyInString を回避している。
10+
// biome-ignore lint/style/useTemplate: "$" + template is intentional to prevent noTemplateCurlyInString
11+
const p = (name: string): string => "$" + `{encodeURIComponent(params.parameter.${name})}`;
12+
const tpl = (...parts: string[]): string => `\`${parts.join("")}\``;
13+
814
describe("PathParameter Test", () => {
915
const factory = TsGenerator.Factory.create();
1016
const generate = (url: string, pathParameter: CodeGenerator.PickedParameter[]): string => {
1117
const urlTemplates = PathParameter.generateUrlTemplateExpression(factory, url, pathParameter);
1218
return Utils.generateTemplateExpression(factory, urlTemplates);
1319
};
1420
test("generateUrlTemplateExpression", () => {
15-
expect(generate("/{a}", [{ in: "path", name: "a", required: true }])).toEqual("`/${encodeURIComponent(params.parameter.a)}`");
16-
expect(generate("/{a}/", [{ in: "path", name: "a", required: true }])).toEqual("`/${encodeURIComponent(params.parameter.a)}/`");
17-
expect(generate("/a/{b}", [{ in: "path", name: "b", required: true }])).toEqual("`/a/${encodeURIComponent(params.parameter.b)}`");
18-
expect(generate("/a/{b}/", [{ in: "path", name: "b", required: true }])).toEqual("`/a/${encodeURIComponent(params.parameter.b)}/`");
19-
expect(generate("/a/{b}/c", [{ in: "path", name: "b", required: true }])).toEqual("`/a/${encodeURIComponent(params.parameter.b)}/c`");
20-
expect(generate("/a/{b}/c/", [{ in: "path", name: "b", required: true }])).toEqual("`/a/${encodeURIComponent(params.parameter.b)}/c/`");
21-
expect(generate("/a/b/{c}", [{ in: "path", name: "c", required: true }])).toEqual("`/a/b/${encodeURIComponent(params.parameter.c)}`");
22-
expect(generate("/a/b/{c}", [{ in: "path", name: "c", required: true }])).toEqual("`/a/b/${encodeURIComponent(params.parameter.c)}`");
23-
expect(generate("/a/b/{c}/", [{ in: "path", name: "c", required: true }])).toEqual("`/a/b/${encodeURIComponent(params.parameter.c)}/`");
24-
expect(generate("/a/b/{c}.json", [{ in: "path", name: "c", required: true }])).toEqual(
25-
"`/a/b/${encodeURIComponent(params.parameter.c)}.json`",
26-
);
21+
expect(generate("/{a}", [{ in: "path", name: "a", required: true }])).toEqual(tpl("/", p("a")));
22+
expect(generate("/{a}/", [{ in: "path", name: "a", required: true }])).toEqual(tpl("/", p("a"), "/"));
23+
expect(generate("/a/{b}", [{ in: "path", name: "b", required: true }])).toEqual(tpl("/a/", p("b")));
24+
expect(generate("/a/{b}/", [{ in: "path", name: "b", required: true }])).toEqual(tpl("/a/", p("b"), "/"));
25+
expect(generate("/a/{b}/c", [{ in: "path", name: "b", required: true }])).toEqual(tpl("/a/", p("b"), "/c"));
26+
expect(generate("/a/{b}/c/", [{ in: "path", name: "b", required: true }])).toEqual(tpl("/a/", p("b"), "/c/"));
27+
expect(generate("/a/b/{c}", [{ in: "path", name: "c", required: true }])).toEqual(tpl("/a/b/", p("c")));
28+
expect(generate("/a/b/{c}", [{ in: "path", name: "c", required: true }])).toEqual(tpl("/a/b/", p("c")));
29+
expect(generate("/a/b/{c}/", [{ in: "path", name: "c", required: true }])).toEqual(tpl("/a/b/", p("c"), "/"));
30+
expect(generate("/a/b/{c}.json", [{ in: "path", name: "c", required: true }])).toEqual(tpl("/a/b/", p("c"), ".json"));
2731
expect(generate("/{a}.json/{a}.json/{a}.json", [{ in: "path", name: "a", required: true }])).toEqual(
28-
"`/${encodeURIComponent(params.parameter.a)}.json/${encodeURIComponent(params.parameter.a)}.json/${encodeURIComponent(params.parameter.a)}.json`",
32+
tpl("/", p("a"), ".json/", p("a"), ".json/", p("a"), ".json"),
2933
);
3034
expect(generate("/.json.{a}.json/{a}.json.{a}", [{ in: "path", name: "a", required: true }])).toEqual(
31-
"`/.json.${encodeURIComponent(params.parameter.a)}.json/${encodeURIComponent(params.parameter.a)}.json.${encodeURIComponent(params.parameter.a)}`",
35+
tpl("/.json.", p("a"), ".json/", p("a"), ".json.", p("a")),
3236
);
3337

3438
expect(
3539
generate("/{a}/{b}", [
3640
{ in: "path", name: "a", required: true },
3741
{ in: "path", name: "b", required: true },
3842
]),
39-
).toBe("`/${encodeURIComponent(params.parameter.a)}/${encodeURIComponent(params.parameter.b)}`");
43+
).toBe(tpl("/", p("a"), "/", p("b")));
4044
expect(
4145
generate("/{a}/{b}/", [
4246
{ in: "path", name: "a", required: true },
4347
{ in: "path", name: "b", required: true },
4448
]),
45-
).toBe("`/${encodeURIComponent(params.parameter.a)}/${encodeURIComponent(params.parameter.b)}/`");
49+
).toBe(tpl("/", p("a"), "/", p("b"), "/"));
4650
expect(
4751
generate("/{a}/{b}/c", [
4852
{ in: "path", name: "a", required: true },
4953
{ in: "path", name: "b", required: true },
5054
]),
51-
).toBe("`/${encodeURIComponent(params.parameter.a)}/${encodeURIComponent(params.parameter.b)}/c`");
55+
).toBe(tpl("/", p("a"), "/", p("b"), "/c"));
5256
expect(
5357
generate("/{a}/{b}/c/", [
5458
{ in: "path", name: "a", required: true },
5559
{ in: "path", name: "b", required: true },
5660
]),
57-
).toBe("`/${encodeURIComponent(params.parameter.a)}/${encodeURIComponent(params.parameter.b)}/c/`");
61+
).toBe(tpl("/", p("a"), "/", p("b"), "/c/"));
5862
expect(
5963
generate("/{a}/b/{c}", [
6064
{ in: "path", name: "a", required: true },
6165
{ in: "path", name: "c", required: true },
6266
]),
63-
).toBe("`/${encodeURIComponent(params.parameter.a)}/b/${encodeURIComponent(params.parameter.c)}`");
67+
).toBe(tpl("/", p("a"), "/b/", p("c")));
6468
expect(
6569
generate("/{a}/b/{c}/", [
6670
{ in: "path", name: "a", required: true },
6771
{ in: "path", name: "c", required: true },
6872
]),
69-
).toBe("`/${encodeURIComponent(params.parameter.a)}/b/${encodeURIComponent(params.parameter.c)}/`");
73+
).toBe(tpl("/", p("a"), "/b/", p("c"), "/"));
7074
expect(
7175
generate("/a/{b}/{c}", [
7276
{ in: "path", name: "b", required: true },
7377
{ in: "path", name: "c", required: true },
7478
]),
75-
).toBe("`/a/${encodeURIComponent(params.parameter.b)}/${encodeURIComponent(params.parameter.c)}`");
79+
).toBe(tpl("/a/", p("b"), "/", p("c")));
7680
expect(
7781
generate("/a/{b}/{c}/", [
7882
{ in: "path", name: "b", required: true },
7983
{ in: "path", name: "c", required: true },
8084
]),
81-
).toBe("`/a/${encodeURIComponent(params.parameter.b)}/${encodeURIComponent(params.parameter.c)}/`");
85+
).toBe(tpl("/a/", p("b"), "/", p("c"), "/"));
8286
expect(
8387
generate("/a/{b}...{c}/", [
8488
{ in: "path", name: "b", required: true },
8589
{ in: "path", name: "c", required: true },
8690
]),
87-
).toBe("`/a/${encodeURIComponent(params.parameter.b)}...${encodeURIComponent(params.parameter.c)}/`");
91+
).toBe(tpl("/a/", p("b"), "...", p("c"), "/"));
8892
});
8993
});

‎src/generateValidRootSchema.ts‎

Lines changed: 13 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,36 +1,22 @@
11
import type * as Types from "./types";
22

3-
const normalizePathParameters = (parameters: (Types.OpenApi.Parameter | Types.OpenApi.Reference)[] | undefined): void => {
4-
if (!parameters) {
5-
return;
6-
}
7-
for (const parameter of parameters) {
8-
if ("$ref" in parameter) {
9-
continue;
10-
}
11-
// OpenAPI 3.x spec §3.3.2: path パラメータは常に required: true
12-
if (parameter.in === "path") {
13-
parameter.required = true;
14-
}
15-
}
16-
};
17-
183
export const generateValidRootSchema = (input: Types.OpenApi.Document): Types.OpenApi.Document => {
19-
if (input.components?.parameters) {
20-
normalizePathParameters(Object.values(input.components.parameters));
21-
}
22-
234
if (!input.paths) {
245
return input;
256
}
26-
27-
const httpMethods = ["get", "put", "post", "delete", "options", "head", "patch", "trace"] as const;
28-
29-
for (const [path, pathItem] of Object.entries(input.paths)) {
30-
normalizePathParameters(pathItem.parameters);
31-
32-
for (const method of httpMethods) {
33-
const operation = pathItem[method];
7+
/** update undefined operation id */
8+
for (const [path, methods] of Object.entries(input.paths || {})) {
9+
const targets = {
10+
get: methods.get,
11+
put: methods.put,
12+
post: methods.post,
13+
delete: methods.delete,
14+
options: methods.options,
15+
head: methods.head,
16+
patch: methods.patch,
17+
trace: methods.trace,
18+
} satisfies Record<string, Types.OpenApi.Operation | undefined>;
19+
for (const [method, operation] of Object.entries(targets)) {
3420
if (!operation) {
3521
continue;
3622
}
@@ -41,9 +27,7 @@ export const generateValidRootSchema = (input: Types.OpenApi.Document): Types.Op
4127
if (!operation.operationId) {
4228
operation.operationId = `${method.toLowerCase()}${path.charAt(0).toUpperCase() + path.slice(1)}`;
4329
}
44-
normalizePathParameters(operation.parameters);
4530
}
4631
}
47-
4832
return input;
4933
};

‎src/internal/OpenApiTools/components/Parameter.ts‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,16 @@ export const generatePropertySignatures = (
109109
context: ToTypeNode.Context,
110110
converterContext: ConverterContext.Types,
111111
): string[] => {
112-
const typeElementMap = parameters.reduce<Record<string, string>>((all, parameter) => {
112+
// Path parameters must be processed last so they win over same-named query/header
113+
// parameters when building the TypeScript interface (path params are always required).
114+
const sorted = [...parameters].sort((a, b): number => {
115+
const aIsPath = !Guard.isReference(a) && a.in === "path";
116+
const bIsPath = !Guard.isReference(b) && b.in === "path";
117+
if (aIsPath && !bIsPath) return 1;
118+
if (!aIsPath && bIsPath) return -1;
119+
return 0;
120+
});
121+
const typeElementMap = sorted.reduce<Record<string, string>>((all, parameter) => {
113122
const { name, typeElement } = generatePropertySignatureObject(
114123
entryPoint,
115124
currentPoint,

‎src/internal/TsGenerator/__tests__/factory.test.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ describe("TsGenerator Factory Helpers", () => {
88
// バックスラッシュ、バッククォート、${ をエスケープする
99
expect(Factory.escapeTemplateText("path\\to\\file")).toBe("path\\\\to\\\\file");
1010
expect(Factory.escapeTemplateText("`quoted`")).toBe("\\`quoted\\`");
11+
// biome-ignore lint/suspicious/noTemplateCurlyInString: intentional - testing escaping of template literal placeholders
1112
expect(Factory.escapeTemplateText("${variable}")).toBe("\\${variable}");
1213
});
1314
});

0 commit comments

Comments
 (0)