From c439da64ab26ec239057a442f4c5f000576029ed Mon Sep 17 00:00:00 2001 From: Maksym Kacharaba Date: Sat, 7 Nov 2026 13:20:42 +0200 Subject: [PATCH] feat: catch a type missing from the JsonSerializerContext (RXH024) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes the last sharp edge in the AOT story. The generator already knows every type a contract sends and receives, so the user should not be the one tracking whether their JsonSerializerContext kept up. A missing registration is now a build warning naming the type, the method, the exact [JsonSerializable] line and the context to add it to — instead of a resolver error on the first call that needed it, in a deployed trimmed application, a long way from the registration nobody updated. The context itself cannot be generated, and I verified why rather than assuming. Roslyn output registered with RegisterPostInitializationOutput joins the compilation every generator sees: a JsonSerializerContext emitted that way IS picked up and populated by System.Text.Json's generator, and serializes at run time. Output registered with RegisterSourceOutput is not — the other generator never sees it and the emitted partial fails CS0534 for the abstract members nobody implemented. The type set can only come from analysing the contracts, which means RegisterSourceOutput, so emission is impossible rather than merely awkward. Recorded in docs/aot.md and on JsonContextCoverage so the next person does not retry it. Covers [Body] parameters that reach the serializer, Task, Task> and every [Response] type — including the collection type for Task>, since a probe confirmed registering T does not cover IReadOnlyList. That is the easiest gap to miss by hand and the one most worth catching. Two blind spots, both chosen to avoid warning when nothing is wrong, because a diagnostic that cries wolf gets suppressed wholesale and then protects nobody. Bare primitives and string are never reported: a probe showed a context registering one record resolved its int and string property types for free, while bool and every collection came back null — so nothing is unconditionally free, but those are overwhelmingly likely to be. And a type registered by any context counts, since nothing at compile time can tell which context is wired to which client. A warning rather than an error: a mixed setup, one client on a source-generated context and another deliberately on the reflection-based serializer, is legitimate and an error would fail that build with no recourse but suppression. Under warnings-as-errors — this repository included — it fails the build anyway, which is how the AOT smoke test now guards its own context. Reads the type set off the existing model rather than re-walking symbols, so the generator keeps exactly one understanding of what each return shape serializes; a second walk would drift the first time a shape is added. 1416 tests; build clean with warnings-as-errors, format clean, AOT publishes with zero trim warnings and runs. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 27 ++ docs/aot.md | 28 +- docs/diagnostics.md | 40 +++ docs/serialization.md | 5 + .../AnalyzerReleases.Unshipped.md | 1 + .../ClientGenerator.cs | 48 ++- src/Runax.Http.SourceGenerator/Diagnostics.cs | 7 + .../JsonContextCoverage.cs | 256 ++++++++++++++++ src/Runax.Http/README.md | 17 +- .../JsonContextCoverageTests.cs | 282 ++++++++++++++++++ 10 files changed, 702 insertions(+), 9 deletions(-) create mode 100644 src/Runax.Http.SourceGenerator/JsonContextCoverage.cs create mode 100644 tests/Runax.Http.SourceGenerator.Tests/JsonContextCoverageTests.cs diff --git a/CHANGELOG.md b/CHANGELOG.md index 39de390..70095b7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,6 +41,33 @@ All packages in the repository are versioned together. — the one query shape whose keys are not known until run time, and so the one resolved there. - Two new diagnostics. `RXH022` rejects a `[Query]` type the runtime cannot turn into text; `RXH023` names the property of an otherwise-expandable object that cannot be sent. +- **`RXH024` catches a type missing from your `JsonSerializerContext`**, closing the last sharp edge + in the AOT story. The generator already knows every type your contracts send and receive, so you + no longer have to keep the context in step by hand: a missing registration is a build warning + naming the type, the method, the exact `[JsonSerializable]` line and the context to add it to. + Previously it surfaced as a resolver error on the first call that needed it, in a deployed + trimmed application, a long way from the registration nobody updated. + - Covers `[Body]` parameters that reach the serializer, `Task`, `Task>` and + every `[Response]` type — including the *collection* type for `Task>`, since + registering `T` does not cover `IReadOnlyList`. That is the easiest gap to miss by hand. + - Stays quiet when the compilation has no context at all, because then you are on the + reflection-based serializer, which is a supported choice that already carries its own warning. + - Two deliberate blind spots, both to avoid warning when nothing is wrong: bare primitives and + `string` are never reported, since they resolve for free whenever any registered type has a + property of that type; and a type registered by *any* context counts, since nothing at compile + time can tell which context is wired to which client. + - A warning rather than an error, because a mixed setup — one client on a source-generated + context, another deliberately on the reflection-based serializer — is legitimate and would + otherwise fail a build with no recourse but suppression. Under warnings-as-errors it fails the + build anyway. + - **The context itself cannot be generated**, and the reason is recorded in `docs/aot.md` so + nobody tries again. Roslyn output registered with `RegisterPostInitializationOutput` joins the + compilation every generator sees, so a `JsonSerializerContext` emitted that way *is* populated + by System.Text.Json's generator — verified. Output registered with `RegisterSourceOutput` is + not: the other generator never sees it and the partial fails with `CS0534`. The type set can + only come from analysing the contracts, which means `RegisterSourceOutput`, so emission is + impossible rather than merely awkward. + - **`[Retry]` on a method**, for the one flaky endpoint on an otherwise healthy API. Surfaces an attempt count, `DelayMilliseconds` (milliseconds because a `TimeSpan` cannot be an attribute argument) and `AllowNonIdempotent`. It refines the client's registered settings rather than diff --git a/docs/aot.md b/docs/aot.md index 60c3f3a..7402f7a 100644 --- a/docs/aot.md +++ b/docs/aot.md @@ -71,8 +71,32 @@ services.AddRunaxClient(); ``` Every type a client sends or receives needs a `[JsonSerializable]` entry — including the types -behind each `[Response]`. A missing one fails with a message naming the type and saying the context -needs extending, rather than a generic resolver error. +behind each `[Response]`, and the *collection* type for a method returning +`Task>`, since registering `T` does not cover `IReadOnlyList`. + +**You will not be the one tracking that.** The generator knows the complete set, so a type missing +from your context is [`RXH024`](diagnostics.md#rxh024) at build time — naming the type, the method, +the exact attribute line, and the context to add it to. Previously a forgotten entry surfaced as a +resolver error on the first call that needed it, in a deployed application, a long way from the +registration nobody updated. + +It only fires when the compilation has a `JsonSerializerContext` at all, so staying on the +reflection-based serializer does not produce a stream of advice you did not ask for. + +### Why the context is not generated for you + +The obvious next step would be for the generator to *write* the context, since it knows the types. +It cannot, and the reason is worth recording so nobody tries again. + +Output registered with Roslyn's `RegisterPostInitializationOutput` joins the compilation every +generator sees, so a `JsonSerializerContext` emitted that way **is** picked up and populated by +System.Text.Json's generator. Output registered with `RegisterSourceOutput` is not — the other +generator never sees it, and the emitted partial fails with `CS0534` for the abstract members +nobody implemented. + +The type set can only come from analysing your contracts, which means `RegisterSourceOutput`. So +emission is not awkward, it is impossible; the diagnostic is the reachable 80%, and it closes the +part that actually hurt. ## How it is verified diff --git a/docs/diagnostics.md b/docs/diagnostics.md index 26abd52..8cf85e1 100644 --- a/docs/diagnostics.md +++ b/docs/diagnostics.md @@ -40,6 +40,7 @@ removal added `RXH019` rather than repurposing the number. | [RXH021](#rxh021) | Error | A `[Retry]` with fewer than one attempt | | [RXH022](#rxh022) | Error | A `[Query]` type that cannot be sent | | [RXH023](#rxh023) | Error | A query object property that cannot be sent | +| [RXH024](#rxh024) | **Warning** | A serialized type missing from every `JsonSerializerContext` | --- @@ -348,6 +349,45 @@ flattening would have to guess, which is the thing [`RXH022`](#rxh022) exists to **Fix:** flatten the type, pass the inner object as its own `[Query]` parameter, or drop the property. +### RXH024 + +**This type is not in any `JsonSerializerContext`.** *(warning)* + +```csharp +[JsonSerializable(typeof(Repository))] +internal sealed partial class ApiJsonContext : JsonSerializerContext; + +[Get("/repos/{owner}/{name}/issues")] +Task> ListIssuesAsync(string owner, string name); // ⚠ IReadOnlyList +``` + +Under trimming or native AOT a type the context does not register fails at run time, on the first +call that needs it, with a resolver error — in a deployed application, a long way from the +registration someone forgot to update. The generator already knows every type your contracts send +and receive, so it can tell you now instead. + +The message names the type, the method, the exact attribute line to add, and which context to add +it to. + +Note the example: registering `Repository` does **not** cover `IReadOnlyList`. +Collections need their own registration, and that is the easiest coverage gap to miss by hand. + +**Only fires when the compilation has at least one `JsonSerializerContext`.** With none, you are on +the reflection-based serializer — a supported choice that carries its own trimming warning — and +this stays quiet. + +Two deliberate blind spots, both to avoid warning when nothing is wrong: + +- **Bare primitives and `string`** are never reported. They resolve for free whenever any + registered type has a property of that type, which in a real context is close enough to always. +- **Any context counts.** Nothing at compile time can tell which context is wired to which client, + and several may be, so a type registered anywhere is treated as covered. + +A warning rather than an error, because a mixed setup — one client on a source-generated context, +another deliberately on the reflection-based serializer — is legitimate and would otherwise fail a +build with no recourse but suppression. In this repository, and anywhere else with +warnings-as-errors, it fails the build anyway. + ## Suppressing The normal mechanisms apply — `#pragma warning disable RXH010`, a `` entry, or an diff --git a/docs/serialization.md b/docs/serialization.md index 485471e..df10a72 100644 --- a/docs/serialization.md +++ b/docs/serialization.md @@ -35,6 +35,11 @@ AOT-correct path is the one without it. A client with no serializer registered fails when it is first resolved, with a message naming both options. +If you take the source-generated path, you do not have to keep the context in step by hand: a type +your contracts serialize that the context does not register is [`RXH024`](diagnostics.md#rxh024) at +build time. See [Native AOT & trimming](aot.md) for what it covers and the two cases it stays quiet +about. + ## System.Text.Json `SystemTextJsonContentSerializer` serializes through `JsonTypeInfo` rather than the reflection-based diff --git a/src/Runax.Http.SourceGenerator/AnalyzerReleases.Unshipped.md b/src/Runax.Http.SourceGenerator/AnalyzerReleases.Unshipped.md index 4f8c8e6..c39d0b4 100644 --- a/src/Runax.Http.SourceGenerator/AnalyzerReleases.Unshipped.md +++ b/src/Runax.Http.SourceGenerator/AnalyzerReleases.Unshipped.md @@ -22,3 +22,4 @@ RXH020 | Runax.Http | Warning | This [Retry] will not retry anything RXH021 | Runax.Http | Error | [Retry] needs at least one attempt RXH022 | Runax.Http | Error | This parameter type cannot be sent as a query value RXH023 | Runax.Http | Error | A query object property cannot be sent +RXH024 | Runax.Http | Warning | This type is not in any JsonSerializerContext diff --git a/src/Runax.Http.SourceGenerator/ClientGenerator.cs b/src/Runax.Http.SourceGenerator/ClientGenerator.cs index 7598a5a..e8d9205 100644 --- a/src/Runax.Http.SourceGenerator/ClientGenerator.cs +++ b/src/Runax.Http.SourceGenerator/ClientGenerator.cs @@ -51,6 +51,32 @@ public void Initialize(IncrementalGeneratorInitializationContext context) if (result.Model is not null) ctx.AddSource(result.HintName!, Emitter.Emit(result.Model)); }); + + // JSON context coverage is a second, compilation-wide pass: it needs every contract's + // serialized types joined against every JsonSerializerContext, which no per-interface + // analysis can see. See JsonContextCoverage for why this is a diagnostic and not an + // emitted context. + var contexts = context.SyntaxProvider + .CreateSyntaxProvider( + static (node, _) => JsonContextCoverage.CouldBeJsonContext(node), + static (ctx, token) => JsonContextCoverage.ReadContext(ctx, token)) + .Where(static found => found is not null); + + context.RegisterSourceOutput( + candidates.Collect().Combine(contexts.Collect()), + static (ctx, pair) => + { + var serialized = pair.Left + .SelectMany(static result => result!.SerializedTypes) + .ToImmutableArray(); + + var declared = pair.Right + .Select(static found => found!) + .ToImmutableArray(); + + foreach (var diagnostic in JsonContextCoverage.Check(serialized, declared)) + ctx.ReportDiagnostic(diagnostic); + }); } private static AnalysisResult? Analyze(GeneratorSyntaxContext context, CancellationToken token) @@ -151,7 +177,11 @@ public void Initialize(IncrementalGeneratorInitializationContext context) declaredTransports.ToImmutable(), methods.ToImmutable()); - return new AnalysisResult(model, $"{contract.Name}.RunaxClient.g.cs", diagnostics.ToImmutable()); + return new AnalysisResult( + model, + $"{contract.Name}.RunaxClient.g.cs", + diagnostics.ToImmutable(), + JsonContextCoverage.Collect(model, Location(contract))); } private static MethodModel? AnalyzeMethod( @@ -1201,11 +1231,18 @@ private static Location Location(ISymbol symbol) /// What one interface produced: a model to emit, diagnostics, or both. private sealed class AnalysisResult { - internal AnalysisResult(ClientModel? model, string? hintName, ImmutableArray diagnostics) + internal AnalysisResult( + ClientModel? model, + string? hintName, + ImmutableArray diagnostics, + ImmutableArray serializedTypes = default) { Model = model; HintName = hintName; Diagnostics = diagnostics; + SerializedTypes = serializedTypes.IsDefault + ? ImmutableArray.Empty + : serializedTypes; } internal ClientModel? Model { get; } @@ -1213,5 +1250,12 @@ internal AnalysisResult(ClientModel? model, string? hintName, ImmutableArray Diagnostics { get; } + + /// + /// What this contract hands to the serializer, for the compilation-wide JSON context + /// check. Empty for a result that produced no model, so the early returns above need no + /// change. + /// + internal ImmutableArray SerializedTypes { get; } } } diff --git a/src/Runax.Http.SourceGenerator/Diagnostics.cs b/src/Runax.Http.SourceGenerator/Diagnostics.cs index 37e667b..a95c98b 100644 --- a/src/Runax.Http.SourceGenerator/Diagnostics.cs +++ b/src/Runax.Http.SourceGenerator/Diagnostics.cs @@ -151,6 +151,13 @@ internal static class Diagnostics "'{0}' on '{1}' is a query object, but its property '{2}' is typed '{3}', which cannot be " + "sent in a query string. {4}"); + internal static readonly DiagnosticDescriptor UncoveredSerializedType = Warning( + "RXH024", + "This type is not in any JsonSerializerContext", + "'{0}' sends or receives '{1}', which no JsonSerializerContext in this project registers. " + + "Under trimming or native AOT that fails at run time on the first call that needs it. Add " + + "{2} to {3}."); + private static DiagnosticDescriptor Error(string id, string title, string format) => new(id, title, format, Category, DiagnosticSeverity.Error, isEnabledByDefault: true); diff --git a/src/Runax.Http.SourceGenerator/JsonContextCoverage.cs b/src/Runax.Http.SourceGenerator/JsonContextCoverage.cs new file mode 100644 index 0000000..d3b2afa --- /dev/null +++ b/src/Runax.Http.SourceGenerator/JsonContextCoverage.cs @@ -0,0 +1,256 @@ +using System.Collections.Immutable; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp.Syntax; + +namespace Runax.Http.SourceGenerator; + +/// One type a contract hands to the serializer, and where it was declared. +internal sealed class SerializedTypeRef +{ + internal SerializedTypeRef(string typeName, string memberDisplay, Location location) + { + TypeName = typeName; + MemberDisplay = memberDisplay; + Location = location; + } + + /// + /// The fully qualified name, normalized so it can be compared with a typeof() argument. + /// + internal string TypeName { get; } + + /// The method that sends or receives it, for the diagnostic message. + internal string MemberDisplay { get; } + + /// Where to report, so the warning lands on the method rather than the project. + internal Location Location { get; } +} + +/// A JsonSerializerContext in the compilation and what it registers. +internal sealed class JsonContextRef +{ + internal JsonContextRef(string contextName, ImmutableArray registeredTypes) + { + ContextName = contextName; + RegisteredTypes = registeredTypes; + } + + internal string ContextName { get; } + + internal ImmutableArray RegisteredTypes { get; } +} + +/// +/// Checks that every type a client sends or receives is registered with a +/// JsonSerializerContext. +/// +/// +/// +/// This is a diagnostic rather than the context itself, and the reason is a hard Roslyn +/// constraint that is worth recording. Output registered with +/// RegisterPostInitializationOutput joins the compilation that every generator sees, so a +/// JsonSerializerContext emitted that way is picked up and populated by +/// System.Text.Json's own generator — verified. Output registered with +/// RegisterSourceOutput is not: the other generator never sees it, and the emitted partial +/// fails with CS0534 for the abstract members nobody implemented. +/// +/// +/// The type set here can only come from analysing the contracts, which means +/// RegisterSourceOutput. So emitting the context is not merely awkward, it cannot work — +/// and the achievable thing is to stop the user forgetting a type, which was the actual pain. +/// +/// +internal static class JsonContextCoverage +{ + /// + /// Types that ride along for free when any registered type has a property of that type, which + /// in a real context is close enough to always that reporting them would be noise. + /// + /// + /// Established by probe rather than assumed: a context registering only a record with an + /// int and a string property resolved int and string but returned + /// null for bool and for every collection. So nothing is unconditionally free — these + /// are merely overwhelmingly likely to be, and a method whose body or response is a bare + /// primitive is rare enough that silence beats a false positive. + /// + private static readonly ImmutableHashSet RidesAlong = ImmutableHashSet.Create( + "string", "object", "bool", "byte", "sbyte", "short", "ushort", + "int", "uint", "long", "ulong", "float", "double", "decimal", "char"); + + /// + /// Everything a contract hands to the serializer. + /// + /// The analysed contract. + /// Where to report, which is the interface declaration. + /// One entry per type per method. + /// + /// Read off the model's own type names rather than re-walking symbols, so there is exactly one + /// understanding in the generator of what each return shape serializes. A second walk here + /// would drift the first time a shape is added. + /// + internal static ImmutableArray Collect(ClientModel model, Location location) + { + var found = ImmutableArray.CreateBuilder(); + + foreach (var method in model.Methods) + { + var member = $"{model.ContractName}.{method.Name}"; + + // Task and Task> both deserialize T. Task, Task and + // Task carry no single type — the ApiResult types come from [Response]. + if (method.Shape is ResultShape.Content or ResultShape.ApiResponseOfContent + && method.ContentTypeName is { } contentType) + { + found.Add(new SerializedTypeRef(Normalize(contentType), member, location)); + } + + foreach (var response in method.Responses) + found.Add(new SerializedTypeRef(Normalize(response.TypeName), member, location)); + + // Only a JSON body reaches the serializer. Raw is already bytes, and form and + // multipart bodies are built from the argument's shape. + if (method.BodyFormat != "Json") + continue; + + foreach (var parameter in method.Parameters) + { + if (parameter.Binding == ParameterBinding.Body) + found.Add(new SerializedTypeRef(Normalize(parameter.TypeName), member, location)); + } + } + + return found.ToImmutable(); + } + + /// + /// Finds the JsonSerializerContext classes in the compilation and what each registers. + /// + internal static JsonContextRef? ReadContext(GeneratorSyntaxContext context, CancellationToken token) + { + if (context.SemanticModel.GetDeclaredSymbol(context.Node, token) is not INamedTypeSymbol type) + return null; + + if (!InheritsJsonSerializerContext(type)) + return null; + + var registered = ImmutableArray.CreateBuilder(); + + foreach (var attribute in type.GetAttributes()) + { + if (attribute.AttributeClass?.ToDisplayString() + != "System.Text.Json.Serialization.JsonSerializableAttribute") + { + continue; + } + + if (attribute.ConstructorArguments.Length > 0 + && attribute.ConstructorArguments[0].Value is ITypeSymbol registeredType) + { + registered.Add(Normalize(registeredType)); + } + } + + // A partial context declared in several files shows up once per part. Returning every part + // is fine: the union across parts is what matters and duplicates are harmless. + return new JsonContextRef(type.ToDisplayString(), registered.ToImmutable()); + } + + /// + /// Reports every serialized type no context registers. + /// + /// What the contracts send and receive. + /// The contexts found in the compilation. + /// The diagnostics to report. + internal static ImmutableArray Check( + ImmutableArray serialized, + ImmutableArray contexts) + { + // No context at all means the application is on the reflection-based serializer, which is + // a supported choice that already carries its own trimming warning. Nagging about coverage + // there would be advice nobody asked for. + if (contexts.IsDefaultOrEmpty || serialized.IsDefaultOrEmpty) + return ImmutableArray.Empty; + + var covered = new HashSet(StringComparer.Ordinal); + + foreach (var context in contexts) + foreach (var type in context.RegisteredTypes) + covered.Add(type); + + // Any context counts, because nothing here can tell which context is wired to which + // client — several may be, and guessing would invent false positives. + var names = string.Join(", ", contexts.Select(c => c.ContextName).Distinct().OrderBy(n => n)); + var diagnostics = ImmutableArray.CreateBuilder(); + var reported = new HashSet(StringComparer.Ordinal); + + foreach (var type in serialized) + { + if (RidesAlong.Contains(type.TypeName) || covered.Contains(type.TypeName)) + continue; + + // One report per type per method, so a type used by five methods does not produce five + // identical warnings at the same location. + if (!reported.Add($"{type.MemberDisplay}|{type.TypeName}")) + continue; + + diagnostics.Add(Diagnostic.Create( + Diagnostics.UncoveredSerializedType, + type.Location, + type.MemberDisplay, + type.TypeName, + $"[JsonSerializable(typeof({type.TypeName}))]", + names)); + } + + return diagnostics.ToImmutable(); + } + + /// + /// Renders a type the way a typeof() argument renders, so the two sides compare. + /// + /// + /// Nullable annotations are dropped: a method returning Task<Repository?> needs + /// the same registration as one returning Task<Repository>, and + /// typeof(Repository) is how it is written either way. A nullable value type normalizes + /// to its underlying type for the same reason — registering the struct covers both. + /// + internal static string Normalize(ITypeSymbol type) + { + var unwrapped = type is INamedTypeSymbol { IsGenericType: true, Arity: 1 } nullable + && nullable.ConstructedFrom.SpecialType == SpecialType.System_Nullable_T + ? nullable.TypeArguments[0] + : type; + + return unwrapped.WithNullableAnnotation(NullableAnnotation.None) + .ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat); + } + + /// + /// Normalizes a name the generator already rendered, which carries nullable annotations. + /// + /// + /// The model renders types with the nullable modifier included, and a typeof() argument + /// never has one, so the trailing marker comes off before comparing. On a nullable value type + /// that also lands on the underlying type, which is the right answer — registering the struct + /// covers both. + /// + internal static string Normalize(string renderedTypeName) + => renderedTypeName.EndsWith("?", StringComparison.Ordinal) + ? renderedTypeName.Substring(0, renderedTypeName.Length - 1) + : renderedTypeName; + + /// A cheap syntactic filter: only a class with a base list can be a context. + internal static bool CouldBeJsonContext(SyntaxNode node) + => node is ClassDeclarationSyntax { BaseList: not null }; + + private static bool InheritsJsonSerializerContext(INamedTypeSymbol type) + { + for (var current = type.BaseType; current is not null; current = current.BaseType) + { + if (current.ToDisplayString() == "System.Text.Json.Serialization.JsonSerializerContext") + return true; + } + + return false; + } +} diff --git a/src/Runax.Http/README.md b/src/Runax.Http/README.md index fabd40b..dcc6f6a 100644 --- a/src/Runax.Http/README.md +++ b/src/Runax.Http/README.md @@ -278,11 +278,12 @@ eventually be used. Full reasoning in ## What the generator catches -Twenty diagnostics: a `{placeholder}` with no parameter, two `[Body]` parameters, an unbindable -parameter, a synchronous return type, a generic interface or method, a `[BaseAddress]` that isn't -absolute, two parameters bound to one placeholder, a body parameter typed wrong for its format, a -`[Retry]` that cannot retry anything. Every one is a build error — or a warning where the code is -legal but almost certainly not what you meant — with a location and a sentence saying what to do. +Twenty-three diagnostics: a `{placeholder}` with no parameter, two `[Body]` parameters, an +unbindable parameter, a synchronous return type, a generic interface or method, a `[BaseAddress]` +that isn't absolute, two parameters bound to one placeholder, a body or query parameter typed wrong +for how it is sent, a `[Retry]` that cannot retry anything, a serialized type missing from your +`JsonSerializerContext`. Every one is a build error — or a warning where the code is legal but +almost certainly not what you meant — with a location and a sentence saying what to do. That list is the reason to prefer a generator over a runtime proxy. A reflection-based client finds these on the first call that reaches them, in whatever environment gets there first. @@ -307,6 +308,12 @@ services.AddRunaxHttp().UseSystemTextJson(); `AddRunaxHttp()` deliberately registers **no** serializer by default. Silently picking the reflection-based one would opt every application out of AOT without saying so. +If you use a source-generated context, you do not have to keep it in step by hand. A type your +contracts serialize that the context does not register is a build warning naming the type, the +method, the exact `[JsonSerializable]` line and the context to add it to — including the collection +type for `Task>`, which registering `T` does not cover. It stays quiet when there +is no context at all, so the reflection path is not nagged. + ## Two invariants worth knowing **The request URI is composed once, above the transport.** A request path appends to the base diff --git a/tests/Runax.Http.SourceGenerator.Tests/JsonContextCoverageTests.cs b/tests/Runax.Http.SourceGenerator.Tests/JsonContextCoverageTests.cs new file mode 100644 index 0000000..055f2f4 --- /dev/null +++ b/tests/Runax.Http.SourceGenerator.Tests/JsonContextCoverageTests.cs @@ -0,0 +1,282 @@ +namespace Runax.Http.SourceGenerator.Tests; + +/// +/// RXH024: every type a contract serializes has to be registered with a +/// JsonSerializerContext, or a trimmed deployment fails on the first call that needs it. +/// +/// +/// +/// These tests assert on the generator's diagnostics only, not on the resulting compilation. +/// The harness drives our generator alone, so a JsonSerializerContext declared in a snippet +/// is never populated by System.Text.Json's generator and the compilation carries the expected +/// CS0534. That is an artefact of the harness, not of the feature. +/// +/// +/// The deliberate exclusions get as much coverage as the positive cases, because a diagnostic that +/// fires when nothing is wrong gets suppressed wholesale and then protects nobody. +/// +/// +public class JsonContextCoverageTests +{ + private const string Context = """ + + [System.Text.Json.Serialization.JsonSerializable(typeof(Thing))] + internal sealed partial class ProbeContext : System.Text.Json.Serialization.JsonSerializerContext; + """; + + [Fact] + public void A_compilation_with_no_context_is_left_alone() + { + // The reflection-based serializer is a supported choice that already carries its own + // trimming warning. Nagging about coverage on top of it would be advice nobody asked for. + var run = GeneratorHarness.RunContract(""" + [BaseAddress("https://api.example.com")] + public interface IClient + { + [Get("/things")] + Task GetAsync(); + } + """); + + run.Ids.ShouldNotContain("RXH024"); + } + + [Fact] + public void A_type_no_context_registers_is_reported() + { + var run = GeneratorHarness.RunContract($$""" + [BaseAddress("https://api.example.com")] + public interface IClient + { + [Get("/others")] + Task GetAsync(); + } + {{Context}} + """); + + run.Ids.ShouldContain("RXH024"); + + var message = run.Message("RXH024"); + + // The message has to carry the fix, not just the complaint: the type, the exact attribute + // line, and which context to put it on. + message.ShouldContain("Other"); + message.ShouldContain("[JsonSerializable(typeof("); + message.ShouldContain("ProbeContext"); + } + + [Fact] + public void A_registered_type_is_not_reported() + { + var run = GeneratorHarness.RunContract($$""" + [BaseAddress("https://api.example.com")] + public interface IClient + { + [Get("/things")] + Task GetAsync(); + } + {{Context}} + """); + + run.Ids.ShouldNotContain("RXH024"); + } + + [Fact] + public void A_body_is_checked_as_well_as_a_response() + { + var run = GeneratorHarness.RunContract($$""" + [BaseAddress("https://api.example.com")] + public interface IClient + { + [Post("/things")] + Task CreateAsync([Body] Other body); + } + {{Context}} + """); + + run.Ids.ShouldContain("RXH024"); + run.Message("RXH024").ShouldContain("Other"); + } + + [Fact] + public void A_collection_return_type_needs_the_collection_registered() + { + // Registering Thing does not give you IReadOnlyList — verified against a real + // context, where every collection resolved to null. This is the easiest coverage gap to + // miss by hand, which is most of why the check is worth having. + var run = GeneratorHarness.RunContract($$""" + [BaseAddress("https://api.example.com")] + public interface IClient + { + [Get("/things")] + Task> ListAsync(); + } + {{Context}} + """); + + run.Ids.ShouldContain("RXH024"); + run.Message("RXH024").ShouldContain("IReadOnlyList"); + } + + [Fact] + public void Response_attribute_types_are_checked() + { + var run = GeneratorHarness.RunContract($$""" + [BaseAddress("https://api.example.com")] + public interface IClient + { + [Get("/things/{id}")] + [Response(200, typeof(Thing))] + [Response(404, typeof(Other))] + Task GetAsync(int id); + } + {{Context}} + """); + + run.Ids.ShouldContain("RXH024"); + run.Message("RXH024").ShouldContain("Other"); + } + + [Fact] + public void An_ApiResponse_content_type_is_checked() + { + var run = GeneratorHarness.RunContract($$""" + [BaseAddress("https://api.example.com")] + public interface IClient + { + [Get("/things")] + Task> TryGetAsync(); + } + {{Context}} + """); + + run.Ids.ShouldContain("RXH024"); + } + + [Theory] + [InlineData("BodyFormat.Raw", "string")] + [InlineData("BodyFormat.UrlEncoded", "Dictionary")] + public void A_body_that_never_reaches_the_serializer_is_not_checked(string format, string type) + { + // Raw is already bytes; a form body is built from the argument's own shape. Neither goes + // anywhere near IContentSerializer, so registering them would be cargo cult. + var run = GeneratorHarness.RunContract($$""" + [BaseAddress("https://api.example.com")] + public interface IClient + { + [Post("/things")] + Task SendAsync([Body(Format = {{format}})] {{type}} body); + } + {{Context}} + """); + + run.Ids.ShouldNotContain("RXH024"); + } + + [Fact] + public void A_method_with_no_response_body_is_not_checked() + { + var run = GeneratorHarness.RunContract($$""" + [BaseAddress("https://api.example.com")] + public interface IClient + { + [Delete("/things/{id}")] + Task DeleteAsync(int id); + + [Head("/things/{id}")] + Task ExistsAsync(int id); + } + {{Context}} + """); + + run.Ids.ShouldNotContain("RXH024"); + } + + [Theory] + [InlineData("string")] + [InlineData("int")] + [InlineData("bool")] + public void A_bare_primitive_response_is_not_reported(string type) + { + // These ride along whenever any registered type has a property of that type, which in a + // real context is close enough to always. A probe confirmed a context registering one + // record resolved its property types for free. Reporting them would be noise, and noise + // gets a diagnostic suppressed wholesale. + var run = GeneratorHarness.RunContract($$""" + [BaseAddress("https://api.example.com")] + public interface IClient + { + [Get("/value")] + Task<{{type}}> GetAsync(); + } + {{Context}} + """); + + run.Ids.ShouldNotContain("RXH024"); + } + + [Fact] + public void A_nullable_response_matches_a_non_nullable_registration() + { + // typeof(Thing) is how the registration is written either way, so the annotation must not + // make a covered type look uncovered. + var run = GeneratorHarness.RunContract($$""" + [BaseAddress("https://api.example.com")] + public interface IClient + { + [Get("/things")] + Task GetAsync(); + } + {{Context}} + """); + + run.Ids.ShouldNotContain("RXH024"); + } + + [Fact] + public void Any_context_in_the_compilation_counts() + { + // Nothing here can tell which context is wired to which client, and several may be. Taking + // the union is the only choice that cannot invent a false positive. + var run = GeneratorHarness.RunContract(""" + [BaseAddress("https://api.example.com")] + public interface IClient + { + [Get("/things")] + Task GetThingAsync(); + + [Get("/others")] + Task GetOtherAsync(); + } + + [System.Text.Json.Serialization.JsonSerializable(typeof(Thing))] + internal sealed partial class FirstContext : System.Text.Json.Serialization.JsonSerializerContext; + + [System.Text.Json.Serialization.JsonSerializable(typeof(Other))] + internal sealed partial class SecondContext : System.Text.Json.Serialization.JsonSerializerContext; + """); + + run.Ids.ShouldNotContain("RXH024"); + } + + [Fact] + public void One_type_used_by_many_methods_is_reported_once_per_method() + { + var run = GeneratorHarness.RunContract($$""" + [BaseAddress("https://api.example.com")] + public interface IClient + { + [Get("/a")] + Task GetAAsync(); + + [Get("/b")] + Task GetBAsync(); + } + {{Context}} + """); + + // Two methods, two reports — not four, and not one. A single report would hide the second + // call site; duplicates per method would be noise. + run.Diagnostics.Count(d => d.Id == "RXH024").ShouldBe(2); + } +}