Repository navigation
fix: reject unsendable query types, and expand query objects - #6
Merged
Merged
Conversation
`[Query] SearchFilter filter` compiled clean, raised no diagnostic, and sent
`?filter=SearchFilter%20%7B%20State%20%3D%20open...` — the record's
ToString(), URL-encoded. A non-record class sent its type name. The server
received garbage and nothing anywhere said so.
RequestValueFormatter's ToString() fallback is right for an unknown
IFormattable, but it should never have been reachable by a type the generator
could see was wrong. A multi-field filter object is the most natural thing a
user tries, and this violated the library's own rules twice over: prefer a
build error over a runtime one, and never guess what goes in a URL.
So the diagnostic rejects what cannot be sent (RXH022, RXH023), and the
expansion makes the natural thing the working thing: one query key per
property, resolved by the generator at compile time. That is what keeps it
reflection-free and trimming-safe, and why only a generator can offer it —
the AOT smoke test now drives it and still publishes with zero trim warnings.
Decisions worth knowing:
- Field names are the property names exactly as written, not camelCased.
Nothing in this library transforms a name — a [Query] parameter already
sends its parameter name verbatim — so transforming properties would make
the two rules disagree. [Query("per_page")] on a property renames it, which
is the same escape hatch parameters have.
- Nested objects are refused, not flattened. ASP.NET Core spells a nested key
"filter.state" and OpenAPI's deepObject spells it "filter[state]"; picking
one silently would be the same guess this change exists to stop. One level
covers the flat filter and pagination objects that motivate the feature,
and refusing leaves room to add nesting later behind an explicit opt-in.
- Inherited properties are expanded, since a filter sharing a pagination base
is the obvious shape and ignoring the base would drop fields silently.
- Null properties are omitted and a null object contributes nothing, matching
how a null [Query] parameter already behaves. Access uses `?.` except on a
non-nullable struct, where it would not compile and null is impossible.
- Dictionaries keyed by string contribute their own keys. They are the one
query shape whose keys are unknown until run time, so they are spread in
from a runtime helper rather than emitted field by field — which is also
why QueryValues is now a collection expression rather than an array.
The generator's sendable-type check mirrors RequestValueFormatter's switch,
including Uri, which is neither a special type nor IFormattable and so has to
be named in both. The two have to agree or the check is a lie.
1461 tests (from 1368); build clean with warnings-as-errors, format clean,
native AOT publishes with no trim warnings and runs.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Katyara11
force-pushed
the
fix/query-object-binding
branch
from
November 7, 2026 11:19
e236a70 to
fbd6dcf
Compare
3 of 6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
[Query]on a complex type compiled clean, raised no diagnostic, and sentToString()to theserver. This rejects what cannot be sent, and expands what can.
Changes
RXH022rejects a[Query]type the runtime cannot turn into text;RXH023names theproperty of an otherwise-expandable object that cannot be sent.
[Query]is now valid on a property, for renaming a field.docs/defining-a-client.md,docs/diagnostics.md,src/Runax.Http/README.md,CHANGELOG.md.Type
Checklist
Notes for review
Why this was worth a fix rather than a doc note.
RequestValueFormatter's_ => ToString()fallback is correct for an unknown
IFormattable, but it should never have been reachable by atype the generator could see was wrong. The library's own rules say prefer a build error over a
runtime one and guessing is how a secret ends up in a URL; this broke both, silently, for the
most natural thing a user tries.
Field naming: property names exactly as written, not camelCased. A
[Query]parameter alreadysends its parameter name verbatim —
[Query] int perPagesendsperPage, and you write[Query("per_page")]to change it. Transforming property names but not parameter names would makethe two rules disagree, and silently transforming is what this PR exists to stop.
[Query("...")]on a property is the same escape hatch, which is why the attribute now targets properties too.
I can see the ergonomic cost: a camelCase API means an attribute per property. I took
predictability over brevity, but this is the decision most worth arguing with.
Nested objects are refused, not flattened. ASP.NET Core spells a nested key
filter.state;OpenAPI's
deepObjectspells itfilter[state]; others differ again. Picking one silently would bethe same guess the diagnostics exist to prevent. One level covers the flat filter/pagination objects
that motivate the feature, and refusing now leaves room to add nesting later behind an explicit
opt-in rather than having to change behaviour someone already depends on.
Smaller calls: inherited properties are expanded (a filter sharing a pagination base is the
obvious shape, and ignoring the base would drop fields silently); null properties are omitted and a
null object contributes nothing, matching a null parameter; access uses
?.except on anon-nullable struct, where it would not compile and null is impossible.
One structural change to shared generator code:
QueryValuesis now emitted as a collectionexpression rather than an array literal, because a dictionary contributes an unknown number of
entries and has to be spread in.
EmitPairsis untouched and still used for paths and headers.The sendable-type check mirrors
RequestValueFormatter's switch,Uriincluded — it is neithera special type nor
IFormattable, so it has to be named in both places. If the two ever disagreethe check becomes a lie, which is worth knowing when either is edited.
Verification
1461 tests (from 1368). Build clean with warnings-as-errors on net8.0/net9.0/net10.0,
dotnet formatclean, and the native-AOT smoke test now drives both expansion and a dictionary query andstill publishes with zero trim warnings — which is the evidence that expansion is genuinely
reflection-free.