Repository navigation
fix(kotlin-client): avoid name shadowing in ktor/multiplatform form bodies and responses - #25624
khayashi4337 wants to merge 4 commits into
Conversation
…multiplatform form bodies
jvm-ktor and multiplatform built urlencoded form bodies inside
`ParametersBuilder().also { it.append(...) }`; a form field named `it`
shadows the implicit builder parameter and fails to compile, and the
urlencoded array branch called a receiver-less `append(...)` that cannot
resolve inside the lambda. The builder is now bound to an explicit
`localVariableBuilder` name.
Multipart file parts referenced the wire name as a bare expression
(`append(my-file)`, a syntax error) or the undefined implicit `it` in
jvm-ktor's file-array loop; they now append the parameter/loop variable.
multiplatform's `.map { value }` is a receiver lambda whose `value` can
be shadowed by a spec parameter named `value`; it now reads `this.value`.
Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Form bodies now use the explicitly named localVariableBuilder instead of the implicit `it`, and multiplatform wrap maps read this.value. Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… tests
Address review findings on the form-body implicit-it fix:
- revert append(it)->append(x) in the jvm-ktor isFile loop nested under
{{^isFile}}: that branch is unreachable, so the change had no effect
(master's reachable file-array branch already emits append(x)); the
third generation pass and append(x) assertion in the multipart test
are removed accordingly
- fix the Javadoc's shadowing direction: the also{} implicit `it` (the
builder) shadows the function parameter derived from the spec field,
not the other way around; multiplatform appends .toString(), so it
sends the builder's string representation rather than failing to
compile
- describe append(my-file) precisely: it parses as a subtraction and
fails because `my`/`file` are unresolved identifiers, not a syntax
error
- restrict the urlencoded `tags` assertion to jvm-ktor (multiplatform
serializes form arrays via toString() — a separate pre-existing bug)
- run both libraries through a @dataProvider per class convention and
fix the double blank line
Verified: regenerating the 5 kotlin jvm-ktor/multiplatform samples
produces zero diff, confirming the reverted branch was dead code.
Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
2 issues found across 14 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="modules/openapi-generator/src/main/resources/kotlin-client/libraries/jvm-ktor/api.mustache">
<violation number="1" location="modules/openapi-generator/src/main/resources/kotlin-client/libraries/jvm-ktor/api.mustache:99">
P1: This lambda parameter can shadow a valid generated form parameter named `localVariableBuilder`. Bind the builder through a label/receiver or reserve this identifier and update all builder references so generated form methods remain valid for that field name.</violation>
</file>
<file name="modules/openapi-generator/src/main/resources/kotlin-client/libraries/multiplatform/api.mustache">
<violation number="1" location="modules/openapi-generator/src/main/resources/kotlin-client/libraries/multiplatform/api.mustache:106">
P2: This fixed lambda name can shadow a legitimate generated form parameter named `localVariableBuilder`, causing the request to send the builder’s `toString()` instead of the caller’s value. Generate a collision-free local name or reserve this identifier when generating parameter names.</violation>
</file>
Reply with feedback, questions, or to request a fix.
View guided diff | Re-trigger cubic
| {{/isMultipart}} | ||
| {{^isMultipart}} | ||
| ParametersBuilder().also { | ||
| ParametersBuilder().also { localVariableBuilder -> |
There was a problem hiding this comment.
P1: This lambda parameter can shadow a valid generated form parameter named localVariableBuilder. Bind the builder through a label/receiver or reserve this identifier and update all builder references so generated form methods remain valid for that field name.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At modules/openapi-generator/src/main/resources/kotlin-client/libraries/jvm-ktor/api.mustache, line 99:
<comment>This lambda parameter can shadow a valid generated form parameter named `localVariableBuilder`. Bind the builder through a label/receiver or reserve this identifier and update all builder references so generated form methods remain valid for that field name.</comment>
<file context>
@@ -96,28 +96,28 @@ import com.fasterxml.jackson.databind.ObjectMapper
{{/isMultipart}}
{{^isMultipart}}
- ParametersBuilder().also {
+ ParametersBuilder().also { localVariableBuilder ->
{{#formParams}}
{{#isFile}}
</file context>
There was a problem hiding this comment.
Thanks. The localVariable… prefix is the one these templates already use for their generated locals (localVariableConfig, localVariableBody, …), and a form field with exactly one of those names collides in the same way on master today. I've noted this limitation in the PR description. Reserving those generated names (renaming spec parameters that collide with them across the kotlin-client templates) is a separate change; this PR stays with the shadowing that breaks specs using common names such as it and value.
| {{/isMultipart}} | ||
| {{^isMultipart}} | ||
| ParametersBuilder().also { | ||
| ParametersBuilder().also { localVariableBuilder -> |
There was a problem hiding this comment.
P2: This fixed lambda name can shadow a legitimate generated form parameter named localVariableBuilder, causing the request to send the builder’s toString() instead of the caller’s value. Generate a collision-free local name or reserve this identifier when generating parameter names.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At modules/openapi-generator/src/main/resources/kotlin-client/libraries/multiplatform/api.mustache, line 106:
<comment>This fixed lambda name can shadow a legitimate generated form parameter named `localVariableBuilder`, causing the request to send the builder’s `toString()` instead of the caller’s value. Generate a collision-free local name or reserve this identifier when generating parameter names.</comment>
<file context>
@@ -103,9 +103,9 @@ import kotlinx.serialization.encoding.*
{{/isMultipart}}
{{^isMultipart}}
- ParametersBuilder().also {
+ ParametersBuilder().also { localVariableBuilder ->
{{#formParams}}
- {{{paramName}}}?.apply { it.append("{{{baseName}}}", {{{paramName}}}.toString()) }
</file context>
There was a problem hiding this comment.
Same as the jvm-ktor thread above: a form field literally named localVariableBuilder is the same class of collision as one named localVariableConfig or localVariableBody, which already exists on master. It's noted in the PR description and left to a separate change that reserves the generated localVariable… names.
…ame-shadowing Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Fixes name-resolution bugs in code generated by the
kotlinclient: in the urlencoded and multipart form bodies oflibrary=jvm-ktorandlibrary=multiplatform, and in howlibrary=multiplatformunwraps array and map responses. Depending on the spec, the generated client fails to compile, or sends or returns the wrong value.Bugs and reproducers
All three specs are added by this PR under
modules/openapi-generator/src/test/resources/3_0/kotlin/.1. A form field named
it(kotlin-form-it-param.yaml). The urlencoded body is built insideParametersBuilder().also { … }, whose implicit lambda parameteritis the builder. The form fielditbecomes a function parameter, generated as`it`. Inside the lambda, the implicitit(the builder) shadows that function parameter, so the field's value is never read:`it`?.apply { it.append("it", `it`) }. The value argument is the builder, a type mismatch, so the client does not compile.`it`?.apply { it.append("it", `it`.toString()) }. This compiles, but sends the builder's string representation instead of the caller's value.2. Urlencoded array fields call a receiver-less
append(jvm-ktor). Inside.also { }the builder is the lambda parameterit, not the receiver, soappend("tags", x.toString())in the array loop does not call the builder.3. A multipart file field named
my-file(kotlin-multipart-file.yaml, multiplatform). multiplatform emitsmyFile?.apply { append(my-file) }. The wire name is emitted as an expression, which Kotlin reads asmy - file, two undefined identifiers, so the client does not compile.4. A parameter named
value(kotlin-receiver-value.yaml, multiplatform). Operations that return an array or a map end withwrap<…Response>().map { value }.maptakes a receiver lambda (T.() -> V), andvalueis meant to be the response wrapper's field. When the operation also has a parameter namedvalue, Kotlin resolvesvalueto the parameter first, so the function returns the caller's argument instead of the decoded response, or fails to compile when the types differ.Fix
ParametersBuilder().also { localVariableBuilder -> … }, and everyappendinside it, including the array loop, callslocalVariableBuilder.append(…)(jvm-ktor, multiplatform).append({{{paramName}}}).this.value(.map { this.value }and.map { this.value.toTypedArray() }).The new name follows the
localVariable…prefix the templates already use for generated locals (localVariableConfig,localVariableBody, …). As with those, a form field literally namedlocalVariableBuilderwould still collide. That is the same existing class of collision and is not addressed here.Not changed here: multiplatform sends a urlencoded array field as
tags.toString()([a, b]). That is a separate issue; the test below checks thetagsline only for jvm-ktor.Tests
testKtorMultiplatformFormBuilderDoesNotUseImplicitItlocalVariableBuilder.append("it"is emitted, and noit.append("it"/it.append("name"; for jvm-ktor alsolocalVariableBuilder.append("tags"testKtorMultiplatformMultipartFileAppendUsesParameterappend(myFile)is emitted andappend(my-file)is not (jvm-ktor already emits this on master; the row guards it)testMultiplatformWrapMapReceiverIsThisValuemap { this.value }is emitted andmap { value }is not./mvnw -nsu -Denforcer.skip=true -pl modules/openapi-generator -am -Dtest=KotlinClientCodegenApiTest -Dsurefire.failIfNoSpecifiedTests=false testapi.mustachechanges reverted (tests kept; only the new tests run)testKtorMultiplatformFormBuilderDoesNotUseImplicitIt, the multiplatform row oftestKtorMultiplatformMultipartFileAppendUsesParameter, andtestMultiplatformWrapMapReceiverIsThisValueSamples
8 files changed in
kotlin-jvm-ktor-gson,kotlin-jvm-ktor-jackson,kotlin-jvm-ktor-kotlinx_serialization,kotlin-multiplatformandkotlin-multiplatform-kotlinx-datetime. The petstore specs have none of the names above, so the only changes are the builder's name andthis.value. All five directories are compiled bysamples-kotlin-client.yaml.This PR and its sibling PRs #25622, #25623 and #25625 (
$escaping in wire names) all touchKotlinClientCodegenApiTestand neighboring kotlin-client templates; whichever merges first, I will rebase the others.cc @wing328 @karismann @Zomzog @andrewemery @4brunu @yutaka0m @stefankoppier @e5l (Kotlin technical committee)
PR checklist
./mvnw -pl modules/openapi-generator-cli -am package -DskipTests) and regenerated the jvm-ktor and multiplatform configs with./bin/generate-samples.sh; the resulting diff is committed.export_docs_generators.shnot run: no generator option or metadata changes.Commit all changed files.
This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
These must match the expectations made by your contribution.
You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example
./bin/generate-samples.sh bin/configs/java*.IMPORTANT: Do NOT purge/delete any folders/files (e.g. tests) when regenerating the samples as manually written tests may be removed.
🤖 Generated with Claude Code