feat!: reflection-free source-generated request building#2210
Open
glennawatson wants to merge 84 commits into
Open
feat!: reflection-free source-generated request building#2210glennawatson wants to merge 84 commits into
glennawatson wants to merge 84 commits into
Conversation
- A multipart part whose declared type is a bool, enum, or sealed DTO now serializes inline through settings.ContentSerializer.ToHttpContent(value), matching the reflection builder's AddSerializedMultipartItem serializer fallback; the declared type drives ToHttpContent<T>, so a sealed/value part's serialized form is byte-identical. - An open, interface, or object-typed part keeps falling back (runtime type decides the serialized shape); a non-sealed DTO part is a declared-type divergence handled separately. - Live parity test serializes a bool and a sealed DTO part byte-for-byte.
- A path parameter of any concrete class or struct (not only sealed/value) now binds inline. The reflection builder renders a route value with a virtual ToString call that dispatches to the runtime type; the generated ToString does the same, so a runtime subtype renders identically - the only divergence is the vanishing case of a subtype whose IFormattable.ToString(null, invariant) differs from its parameterless ToString(). - object, interface, and open generic path parameters keep falling back: they have no usable declared shape and must be inspected at runtime.
- A collection or array path parameter (List<int>, byte[], int[]) now binds inline: the reflection builder renders it with the URL parameter formatter, which for a non-IFormattable value is value.ToString() (the System.Collections text), and the generated ToString reproduces that identically.
…rt parts - A dictionary value or multipart part of any concrete class or struct (not only sealed/value) now flattens/serializes inline by its declared type, the same accepted declared-type divergence a non-sealed query object already carries (matching the System.Text.Json source generator). - A value that is not a runtime subtype renders identically to the reflection builder; a polymorphic subtype renders by its declared type. object, interface, and open generic types keep falling back - they have no usable declared shape. - Live parity tests flatten a non-sealed dict value and serialize a non-sealed multipart part, both byte-for-byte vs reflection.
- Remove the .claude/worktrees gitlink accidentally staged into the previous commit; the directory is a transient local agent worktree, not project content. - Add .claude/ to .gitignore.
- Generate IObservable<T> methods inline through a new GeneratedRequestRunner.SendObservable helper instead of the reflection request builder, closing the last non-Task return-shape fallback where the shape is statically known. - The helper returns a ReactiveUI.Primitives cold FromAsyncSignal that rebuilds and re-sends the request per subscription, linking the method and subscription cancellation tokens. - Wrap request construction in a per-subscription local function so a second subscription never reuses a disposed request. - Mirror the async send's isApiResponse/dispose/buffer flags, so IObservable<ApiResponse<T>> and IObservable<IApiResponse<T>> reach parity with the async path. - Move the return-shape inline/fallback switch tests into their own file and add a live cold-observable parity test.
- Stop forcing a reflection fallback for method paths without a leading slash; they now build inline like any other path. - Under RFC 3986 resolution the HttpClient merges the base address with the relative path; under legacy resolution the generated request rejects it with the same ArgumentException the reflection builder raises, so behaviour matches in both modes. - Add a live test covering inline generation, RFC 3986 parity, and the legacy rejection.
- Bind nested {a.b.c} route placeholders through a property chain in both
the source-generated and reflection request builders, where only a
single {a.b} level bound before.
- Walk the chain with null-conditional access so a null intermediate
renders an empty segment instead of throwing, matching the reflection
builder; the top-level property is marked consumed so residual object
properties still flatten into the query correctly.
- Extend the public RestMethodParameterProperty with the navigation chain
(single-level bindings remain a one-element chain).
…utes - Wrap a generated multipart part's serialization failure in the same descriptive ArgumentException the reflection request builder raises, via a new GeneratedRequestRunner.SerializeMultipartPart helper. - Update the leading-slash-less route tests: generated request building validates the slash when the request is built, so the throw now surfaces on the first call, not from RestService.For. Document the timing in the README. - Switch the generated-factory test interface off the now-inline IObservable shape onto a dynamic query-map parameter so it still exercises the reflection factory registration path.
- A custom HTTP verb previously allocated a new HttpMethod on every call through inline request building. Emit it once into a static field and reference it, matching the reflection builder, which reads the verb from the attribute once per method. - A known verb still resolves to the framework-cached singleton, so only custom-verb methods gain the field.
- A scalar query parameter's key is a compile-time constant, so escape it once during generation and append it verbatim at request time instead of escaping it on every call. - Add GeneratedQueryStringBuilder.AddPreEscapedKey / AddFormattedPreEscapedKey overloads that take an already-escaped key; the escaping-key overloads remain for runtime keys (dictionary entries, [QueryName] values). - Uri.EscapeDataString follows RFC 3986 consistently across target frameworks, so the generation-time escape matches the reflection builder's runtime escape (parity tests confirm).
- Escape a span-formatted URL value directly into the builder with an in-place RFC 3986 encoder (StringHelpers.AppendUriDataEscaped) shared by path and query building, instead of allocating an escaped string via Uri.EscapeDataString(span). - Enable the span-escape tier wherever ISpanFormattable exists (net8+), not just where Uri.EscapeDataString(ReadOnlySpan) does (net9+), so net8 formats-and-escapes escapable values without the intermediate string. - The formatted spans are invariant ASCII; a non-ASCII character defers to Uri.EscapeDataString for exact UTF-8 parity.
- Add TimestampQuery / TimestampPath rows (a DateTimeOffset whose invariant form needs escaping) so the span-escape encoder is measured against the reflection builder for both query and path values. - Add a custom-verb row so the cached HttpMethod path is exercised.
- IObservable methods now build inline, so RF006 no longer fires for them; move the shape from the reflection-fallback analyzer cases to the inline-supported cases, matching the generator's fallback contract.
…uest-building # Conflicts: # src/Refit/GeneratedParameterAttributeProvider.cs
- Resolved in favour of the request-building rework: the branch predates it and its path-parameter code is superseded by the inline rework. - Dropped PathFragmentModel (the rework has its own path models). - Kept the updated analyzer rule configuration. Co-Authored-By: Timothy Makkison <timmakkison@gmail.com>
- Move the nested attribute-array flatten out of the AllAttributesCache getter into a static FlattenAttributes helper that copies each array with Array.Copy, removing the nested loop that needed the suppression.
- Replace the StyleSharp SST2229/2230/2233 and stylesharp.* hot-path LINQ configuration with the PerformanceSharp PSH1100/1101/1102 and performancesharp.* equivalents in the root and tests .editorconfig. - Point the S103/S3880 duplicate-rule notes at their new owners and keep the 200-char line limit the repo enforces.
- Add runtime, generator, and reflection tests bringing every file touched by the reflection-free request-building feature to 100% line coverage. - Remove dead code surfaced by the coverage push: inline FindHttpMethodProperty so its unreachable return null folds into the covered fallback, drop the dead PropertyDeclarationSyntax getter arm, and delete the redundant adapter bounds guard (the source-generated binder indexes the same slots without one). - Bump StyleSharp/PerformanceSharp to 3.21.2, SonarAnalyzer.CSharp to 10.29.0.143774, TUnit to 1.59.0, Verify.TUnit to 31.24.1, and Serilog to 4.4.0; the analyzer update clears the SonarCloud SST2015/SST1462 build errors.
…dies - Implemented `Parser.Request.Query.Objects` to flatten complex query values into query pairs for Refit stubs. - Introduced `GeneratedRequestRunner.BodyContent` for serializing request bodies in various formats (JSON, JSON Lines, URL-encoded). - Added `GeneratedRequestRunner.Sending` to handle sending requests and processing responses, including support for observables and streaming responses.
- Split oversized files into behavior-named partial classes and extract long methods (SST1522/SST1523) across Refit.Reflection, Refit.Testing, and the test and benchmark projects. - Fix mechanical findings: buried ++/-- (SST2015), numeric-suffix casing (SST2244), redundant interface public (SST1491), replaceable collections (SST2305), per-call serializer options (PSH1416), and assorted others. - Suppress the few genuinely-unfixable findings with justifications: CS0501- mandated abstract on an explicit-interface re-abstraction, options capturing a per-call resolver, the interface-dispatch and sync-stream test surfaces, and the BCL-mirror [Flags] enum polyfill. - Behavior unchanged; the full solution builds analyzer-clean and all tests pass.
ChrisPulman
approved these changes
Jul 13, 2026
…mbdas in various files
- Add branch-covering tests across the generator, runtime, reflection, testing, analyzer and code-fix files, closing 74 of the 125 partial/uncovered branches flagged on the touched files. - Drive the real generated/parsed and reflection request-building surface for each untested condition outcome (attribute combos, param/return shapes, dotted paths, query objects, dictionaries, multipart parts, custom verbs, adapters, RFC-3986 escaping, cancellation linking, and error/fallback paths). - The remaining 51 branches are provably unreachable (compiler-generated string jump tables, dead null-conditionals on never-null operands, exhaustive switch defaults, async-iterator dispose-mode IL edges, contract-guaranteed non-null).
- Remove provably-dead branches surfaced by the coverage push: drop `?.`/`??` guards on operands the surrounding invariant proves non-null (Roslyn symbol namespaces/containers, JsonSerializerOptions.GetTypeInfo, a String JSON element's GetString, a CanRead property's getter, the resolved adapter type), and dead ternary/switch arms that cannot be reached. - Isolate the structurally-uncoverable branches into tiny private helpers marked [ExcludeFromCodeCoverage] (compiler string-switch jump tables, async-iterator dispose-mode IL edges, BCL-nullable defensive guards, analyzer-blocked arms), keeping the excluded surface minimal and the surrounding coverable logic intact. - Consolidate the duplicated known-verb string switch into one shared Parser.MapKnownHttpVerb, and the linked-cancellation-token pattern into one GeneratedRequestRunner.ResolveRequestCancellationToken. - Behavior unchanged: generator snapshots byte-identical, all tests pass, and every touched file now reports 100% branch coverage.
|
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
3 participants
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.
What kind of change does this PR introduce?
Feature, performance, and a breaking release (V14). It completes the move to source-generated, reflection-free request building.
What is the new behavior?
Refit builds requests directly, so the common method shapes work in generated-only and Native AOT clients with no runtime reflection:
[AliasAs],[Query](Format,CollectionFormat), scalar collections, and query-object flattening.[QueryConverter]for shapes only known at runtime.[QueryName]valueless flags and[Encoded]per-parameter encoding opt-out.[Body]on POST/PUT/PATCH, and an all-scalar form body serialized straight-line with no descriptor array and no boxing.IReturnTypeAdapter.Generated code targets C# 7.3 and lights up newer syntax (nullable annotations and so on) when the consumer's language version allows.
What is the current behavior?
Most of these shapes went through the reflection request builder, which is not trim- or AOT-safe. The RF006 analyzer now flags any method that still needs reflection, so a generated-only build fails at compile time instead of throwing at runtime.
Closes #2185
Closes #2190
Closes #2191
Closes #2192
Closes #2168
Closes #2169
Closes #2002
Closes #2165
Closes #1101
Closes #2200
What might this PR break?
ValueTaskinstead ofTask(ApiResponse<T>.EnsureSuccess*Async, theIApiResponse<T>extensions,DefaultApiExceptionFactory.CreateAsync, and theRefitSettingsasync delegates).Refit.Reflectionpackage. Interfaces that all generate inline do not need it; RF006 names any method that does.RestService.For.Full detail is in
docs/breaking-changes.md.Checklist
mainbranchAdditional information
A lot of the path-parameter work here grew out of @TimothyMakkison's ideas. He was already exploring AOT path-parameter support in #2209 and said he planned to work on it. The
ISpanFormattablefast-write for path values is his idea, and that commit credits him as co-author. Thanks, @TimothyMakkison.Verified across net8 to net11: the full generator and runtime suites pass, the solution builds with no analyzer warnings, and generated code compiles down to the C# 7.3 baseline.