[rust] fix: map-typed query parameters do not compile (.to_string() on a HashMap) - #24896
[rust] fix: map-typed query parameters do not compile (.to_string() on a HashMap)#24896wiebren wants to merge 1 commit into
Conversation
HashMap implements neither Display nor ToString, so the generated client did not compile (E0599) wherever a map-typed query parameter fell through to the scalar path: a required non-nullable map in both reqwest and reqwest-trait, and the nullable/optional non-deepObject fallthroughs in reqwest-trait. Serialize the map as one json-encoded parameter with serde_json::to_string, exactly how reqwest's optional branch already handles non-primitive parameters. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GcwZ1arjLZNpetHz2a3TJz
There was a problem hiding this comment.
1 issue found across 4 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="modules/openapi-generator/src/main/resources/rust/reqwest/api.mustache">
<violation number="1" location="modules/openapi-generator/src/main/resources/rust/reqwest/api.mustache:170">
P3: A required, non-nullable map declared with `style: deepObject` falls into this `{{#isMap}}` branch and is serialized as a single JSON-encoded parameter (`labels={...}`) instead of the deep-object `labels[key]=value` form. The template only implements deepObject spreading inside the `{{#isNullable}}` branch, so required non-nullable deep-object maps bypass it. Previously this path emitted `.to_string()` and did not compile, so this is an incomplete fix, not a regression, but the wire format is now silently wrong for that style.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| {{^isArray}} | ||
| {{^isNullable}} | ||
| {{#isMap}} | ||
| req_builder = req_builder.query(&[("{{{baseName}}}", &serde_json::to_string(&{{{vendorExtensions.x-rust-param-identifier}}})?)]); |
There was a problem hiding this comment.
P3: A required, non-nullable map declared with style: deepObject falls into this {{#isMap}} branch and is serialized as a single JSON-encoded parameter (labels={...}) instead of the deep-object labels[key]=value form. The template only implements deepObject spreading inside the {{#isNullable}} branch, so required non-nullable deep-object maps bypass it. Previously this path emitted .to_string() and did not compile, so this is an incomplete fix, not a regression, but the wire format is now silently wrong for that style.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At modules/openapi-generator/src/main/resources/rust/reqwest/api.mustache, line 170:
<comment>A required, non-nullable map declared with `style: deepObject` falls into this `{{#isMap}}` branch and is serialized as a single JSON-encoded parameter (`labels={...}`) instead of the deep-object `labels[key]=value` form. The template only implements deepObject spreading inside the `{{#isNullable}}` branch, so required non-nullable deep-object maps bypass it. Previously this path emitted `.to_string()` and did not compile, so this is an incomplete fix, not a regression, but the wire format is now silently wrong for that style.</comment>
<file context>
@@ -166,7 +166,12 @@ pub {{#supportAsync}}async {{/supportAsync}}fn {{{operationId}}}(configuration:
{{^isArray}}
{{^isNullable}}
+ {{#isMap}}
+ req_builder = req_builder.query(&[("{{{baseName}}}", &serde_json::to_string(&{{{vendorExtensions.x-rust-param-identifier}}})?)]);
+ {{/isMap}}
+ {{^isMap}}
</file context>
There was a problem hiding this comment.
Accurate, and worth separating from this PR's scope. That branch - required, non-nullable - has never had any isDeepObject handling at all: unlike the nullable and optional paths just below it, it goes straight to the scalar form for every type. So a required non-nullable style: deepObject map was not previously exploded either; it did not compile at all, which is what this PR fixes. The json encoding it now produces is the same shape the other libraries' non-primitive parameters use, so nothing regresses - but you are right that it is not the declared style.
The deepObject format work lives in #24899, which routes exploded deepObject maps through parse_deep_object (both the free-form and the typed-map shape). Extending that to the required non-nullable branch is a small addition and I am happy to push it - I would suggest doing it there rather than here, so all deepObject encoding stays in one PR and these two do not both edit the same template lines. Say which you prefer and I will add it.
A map-typed query parameter (
type: objectwith typedadditionalProperties, generated asHashMap<String, T>) falls through to the scalar query path in several template branches,which call
.to_string()on it.HashMapimplements neitherDisplaynorToString, sothe generated crate does not compile:
The holes, from a spec with one required and one optional map parameter:
.to_string()— broken.to_string()— broken.to_string()fallthrough — broken.to_string()fallthrough — brokenparse_deep_object/ per-entry)This was noted as a pre-existing, out-of-scope breakage in #24866's PR body; this is the
follow-up. Found generating a client from a production registry API whose listing filters
are map-typed.
The fix
Add an
{{#isMap}}split at each hole, serializing the map as one json-encoded parameterwith
serde_json::to_string— exactly how reqwest's optional branch already handlesnon-primitive parameters, so the optional-vs-required and reqwest-vs-reqwest-trait wire
behaviors now agree. No behavior change for any parameter that compiled before: a required
free-form map (
HashMap<String, serde_json::Value>) moves from a broken.to_string()tothe same JSON text, and
serde_json::Value'sDisplaywas that JSON already.Tests
RustClientCodegenTest#testMapQueryParamsSerializeAsJsongenerates the new3_0/rust/map-query-params.yamlfixture with both libraries and asserts the json-encodedform with no
.to_string()left on either parameter. Fails without the template changes(verified by stashing only the templates).
Verified by
cargo buildof clients generated from the fixture: both libraries compileclean with the fix and fail with E0599 without it.
PR checklist
./bin/generate-samples.sh ./bin/configs/rust-*):zero diffs across all 39 rust configs - no sample spec has a plain map-typed query
parameter, which is how the breakage stayed unnoticed.
Generated with Claude Code
Summary by cubic
Fixes Rust client codegen for map-typed query parameters, which previously emitted
.to_string()on aHashMap(which doesn't implementDisplayorToString), causing compilation errors in bothreqwestandreqwest-traitlibraries for required non-nullable maps, and optional/nullable maps inreqwest-trait. Now these serialize as a single JSON-encoded query parameter viaserde_json::to_string, matching how non-primitive parameters are already handled. No behavior change for parameters that compiled before.Written for commit 58b99b6. Summary will update on new commits.