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); + } +}