diff --git a/docs/architecture/protocol-and-generation.md b/docs/architecture/protocol-and-generation.md index 98801000..74260a3a 100644 --- a/docs/architecture/protocol-and-generation.md +++ b/docs/architecture/protocol-and-generation.md @@ -209,7 +209,9 @@ reported upstream rather than repaired from private implementation knowledge (AD ADR-0014). - A tri-state request property emits `[JsonIgnore(Condition = WhenWritingDefault)]` beside a property-level `[JsonConverter]` naming the closed converter emitted for its instantiation: the - wrapper is generic, so no type-level converter can name it. A member-level converter costs the + wrapper is generic, so no type-level converter can name it; a special-number instantiation's + converter reads and writes the named literals `NaN`, `Infinity` and `-Infinity` itself, because + `[JsonNumberHandling]` is invalid on the wrapper member. A member-level converter costs the containing type its serialization fast path, which makes `JsonSourceGenerationMode.Metadata` on the emitted registry load-bearing rather than a performance preference (ADR-0004, ADR-0014). - An object schema that declares named properties beside an unrestricted additional-properties diff --git a/tests/OpenCode.Sdk.Tools.Tests/Generator/Emission/ModelEmitterTests.cs b/tests/OpenCode.Sdk.Tools.Tests/Generator/Emission/ModelEmitterTests.cs index 6a93636f..3f5bcfde 100644 --- a/tests/OpenCode.Sdk.Tools.Tests/Generator/Emission/ModelEmitterTests.cs +++ b/tests/OpenCode.Sdk.Tools.Tests/Generator/Emission/ModelEmitterTests.cs @@ -155,4 +155,22 @@ public async Task Emit_Should_Render_An_Extension_Data_Member_For_An_Open_Model( await Assert.That(model).DoesNotContain("public IDictionary"); await Assert.That(model).DoesNotContain("[JsonPropertyName(\"additionalProperties\")]"); } + + [Test] + public async Task Emit_Should_Apply_Number_Handling_Only_To_Unwrapped_Special_Numbers() + { + var model = UnionHoistPlanData.Record("WidgetPatchRequest", + UnionHoistPlanData.Property("required", UnionHoistPlanData.SpecialNumber(), isRequired: true), + UnionHoistPlanData.Property("optional", UnionHoistPlanData.SpecialNumber(isNullable: true), isRequired: false), + UnionHoistPlanData.Property("wrapped", UnionHoistPlanData.SpecialNumber(isNullable: true), isRequired: false, emitsOptionalWrapper: true)); + var plan = (EmitterPlanFixture.CreateModelSnapshot() with { Models = [model], Unions = [] }).WithSingleSlice(); + var root = await CSharpSyntaxTree.ParseText(EmitterSnapshot.Create(ModelEmitter.Emit(plan))).GetRootAsync(); + var properties = root.DescendantNodes().OfType() + .ToDictionary(property => property.Identifier.ValueText, StringComparer.Ordinal); + + await Assert.That(properties["Required"].ToString()).Contains("[JsonNumberHandling(JsonNumberHandling.AllowNamedFloatingPointLiterals)]"); + await Assert.That(properties["Optional"].ToString()).Contains("[JsonNumberHandling(JsonNumberHandling.AllowNamedFloatingPointLiterals)]"); + await Assert.That(properties["Wrapped"].ToString()).DoesNotContain("JsonNumberHandling"); + await Assert.That(properties["Wrapped"].ToString()).Contains("[JsonConverter(typeof(OptionalOfSpecialNumberJsonConverter))]"); + } } diff --git a/tests/OpenCode.Sdk.Tools.Tests/Generator/Emission/ModelMaterializationMatrixTests.cs b/tests/OpenCode.Sdk.Tools.Tests/Generator/Emission/ModelMaterializationMatrixTests.cs index 57e83b59..55ceb89c 100644 --- a/tests/OpenCode.Sdk.Tools.Tests/Generator/Emission/ModelMaterializationMatrixTests.cs +++ b/tests/OpenCode.Sdk.Tools.Tests/Generator/Emission/ModelMaterializationMatrixTests.cs @@ -132,6 +132,9 @@ private static async Task AssertTristateRequestMembersAsync(Assembly assembly, E .Single(static model => model.Properties.Any(static property => property.WireName == "patchScalar")); await Assert.That(PatchMember(patch, "patchScalar").EmitsOptionalWrapper).IsTrue(); await Assert.That(PatchMember(patch, "patchNumber").EmitsOptionalWrapper).IsTrue(); + await Assert.That(PatchMember(patch, "patchSpecialNumber").EmitsOptionalWrapper).IsTrue(); + await Assert.That(PatchMember(patch, "patchSpecialNumber").Type).IsTypeOf(); + await Assert.That(PatchMember(patch, "patchSpecialNumber").Type.IsNullable).IsTrue(); await Assert.That(PatchMember(patch, "patchList").EmitsOptionalWrapper).IsTrue(); await Assert.That(PatchMember(patch, "patchDictionary").EmitsOptionalWrapper).IsTrue(); await Assert.That(PatchMember(patch, "patchPlain").EmitsOptionalWrapper).IsFalse(); @@ -146,6 +149,7 @@ private static async Task AssertTristateRequestMembersAsync(Assembly assembly, E { ["patchScalar"] = null, ["patchNumber"] = null, + ["patchSpecialNumber"] = null, ["patchList"] = null, ["patchDictionary"] = null, ["patchPlain"] = null, @@ -154,7 +158,7 @@ private static async Task AssertTristateRequestMembersAsync(Assembly assembly, E typeInfo); using (var document = JsonDocument.Parse(await SerializeAsync(cleared, typeInfo))) { - foreach (var name in new[] { "patchScalar", "patchNumber", "patchList", "patchDictionary", }) + foreach (var name in new[] { "patchScalar", "patchNumber", "patchList", "patchDictionary", "patchSpecialNumber", }) { await Assert.That(document.RootElement.GetProperty(name).ValueKind).IsEqualTo(JsonValueKind.Null); } @@ -182,6 +186,26 @@ private static async Task AssertTristateRequestMembersAsync(Assembly assembly, E await Assert.That(document.RootElement.GetProperty("patchPlain").GetString()).IsEqualTo("plain"); await Assert.That(document.RootElement.GetProperty("patchRequired").GetString()).IsEqualTo("kept"); } + + foreach (var (input, expected, json) in new[] + { + (JsonValue.Create(4.5), 4.5, "4.5"), + (JsonValue.Create("NaN"), double.NaN, "\"NaN\""), + (JsonValue.Create("Infinity"), double.PositiveInfinity, "\"Infinity\""), + (JsonValue.Create("-Infinity"), double.NegativeInfinity, "\"-Infinity\""), + }) + { + var special = Deserialize(new JsonObject { ["patchRequired"] = null, ["patchSpecialNumber"] = input, }, typeInfo); + await Assert.That(GetProperty(GetProperty(special, "PatchSpecialNumber")!, "Value")).IsEqualTo(expected); + using var document = JsonDocument.Parse(await SerializeAsync(special, typeInfo)); + await Assert.That(document.RootElement.GetProperty("patchSpecialNumber").GetRawText()).IsEqualTo(json); + } + + foreach (var invalid in new[] { "nan", "+Infinity", "1.5", }) + { + var payload = new JsonObject { ["patchRequired"] = null, ["patchSpecialNumber"] = invalid, }; + _ = await Assert.That(() => Deserialize(payload, typeInfo)).Throws(); + } } private static ModelPropertyPlan PatchMember(ObjectModelPlan model, string wireName) => diff --git a/tests/OpenCode.Sdk.Tools.Tests/Generator/Emission/OptionalConverterEmitterTests.cs b/tests/OpenCode.Sdk.Tools.Tests/Generator/Emission/OptionalConverterEmitterTests.cs index 2ef24b66..b1377773 100644 --- a/tests/OpenCode.Sdk.Tools.Tests/Generator/Emission/OptionalConverterEmitterTests.cs +++ b/tests/OpenCode.Sdk.Tools.Tests/Generator/Emission/OptionalConverterEmitterTests.cs @@ -17,6 +17,39 @@ namespace OpenCode.Sdk.Tools.Tests.Generator.Emission; [ClassDataSource(Shared = SharedType.PerTestSession)] public sealed class OptionalConverterEmitterTests(PinnedBinding pinned) { + [Test] + public async Task Emit_Should_Keep_Plain_And_Special_Number_Converters_Distinct_And_Ordered() + { + var model = Record("WidgetPatchRequest", + Property("special", SpecialNumber(isNullable: true), isRequired: false, emitsOptionalWrapper: true), + Property("plain", Named("double", isNullable: true), isRequired: false, emitsOptionalWrapper: true)); + + var sources = OptionalConverterEmitter.Emit([model]); + + await Assert.That(sources.Select(static source => source.RelativePath)).IsEquivalentTo( + [ + "Internal/Serialization/OptionalOfDoubleJsonConverter.cs", + "Internal/Serialization/OptionalOfSpecialNumberJsonConverter.cs", + ], CollectionOrdering.Matching); + } + + [Test] + [Arguments(false)] + [Arguments(true)] + public async Task Emit_Should_Refuse_A_Wrapped_Container_Of_Special_Numbers(bool dictionary) + { + var special = SpecialNumber(); + TypeReferencePlan container = dictionary + ? new DictionaryTypeReferencePlan { ValueType = special, IsNullable = true, JsonNullRepresentation = JsonNullRepresentation.ClrNull, } + : new ListTypeReferencePlan { ElementType = special, IsNullable = true, JsonNullRepresentation = JsonNullRepresentation.ClrNull, }; + var model = Record("WidgetPatchRequest", Property("values", container, isRequired: false, emitsOptionalWrapper: true)); + + var exception = Assert.Throws(() => OptionalConverterEmitter.Emit([model])); + + await Assert.That(exception.Message).IsEqualTo( + "No optional converter name exists for plan 'SpecialNumberTypeReferencePlan'; admit the representation first."); + } + [Test] public async Task Emit_Should_Produce_One_Converter_Per_Distinct_Instantiation() { diff --git a/tests/OpenCode.Sdk.Tools.Tests/Generator/Emission/SourceEmitterTests.cs b/tests/OpenCode.Sdk.Tools.Tests/Generator/Emission/SourceEmitterTests.cs index 585071ae..1c0c9f24 100644 --- a/tests/OpenCode.Sdk.Tools.Tests/Generator/Emission/SourceEmitterTests.cs +++ b/tests/OpenCode.Sdk.Tools.Tests/Generator/Emission/SourceEmitterTests.cs @@ -9,6 +9,7 @@ using OpenCode.Sdk.Tools.Tests.Generator.Emission.Support; using OpenCode.Sdk.Tools.Tests.Support; using TUnit.Assertions.Enums; +using static OpenCode.Sdk.Tools.Tests.Support.UnionHoistPlanData; namespace OpenCode.Sdk.Tools.Tests.Generator.Emission; @@ -235,6 +236,19 @@ .. root ]; } + [Test] + public async Task Emit_Should_Refuse_Two_Optional_Instantiations_That_Share_A_Converter_Name() + { + var model = Record("WidgetPatchRequest", + Property("primitive", Named("double", isNullable: true), isRequired: false, emitsOptionalWrapper: true), + Property("nominal", Named("Double", isNullable: true), isRequired: false, emitsOptionalWrapper: true)); + var plan = (EmitterPlanFixture.CreateModelSnapshot() with { Models = [model], Unions = [], Clients = [] }).WithSingleSlice(); + + var exception = Assert.Throws(() => SourceEmitter.Emit(plan)); + + await Assert.That(exception.Message).IsEqualTo("Multiple emitters produced 'Internal/Serialization/OptionalOfDoubleJsonConverter.cs'."); + } + private static IReadOnlyList> ReadStructuralConverterMappings(GeneratedSource source) { var root = Parse(source); diff --git a/tests/OpenCode.Sdk.Tools.Tests/Generator/Emission/Support/MaterializationMatrixScenario.cs b/tests/OpenCode.Sdk.Tools.Tests/Generator/Emission/Support/MaterializationMatrixScenario.cs index 7b473022..bfe389e3 100644 --- a/tests/OpenCode.Sdk.Tools.Tests/Generator/Emission/Support/MaterializationMatrixScenario.cs +++ b/tests/OpenCode.Sdk.Tools.Tests/Generator/Emission/Support/MaterializationMatrixScenario.cs @@ -8,6 +8,8 @@ internal sealed class MaterializationMatrixScenario : SpecScenario public const string ChoiceOperationId = "matrix.choice"; public const string PatchOperationId = "matrix.patch"; + private static readonly string[] SpecialNumberLiterals = ["NaN", "Infinity", "-Infinity"]; + protected override void Arrange(SpecDocumentBuilder spec) => spec .WithSchema("MatrixChoiceAlpha", schema => Choice(schema, "alpha")) .WithSchema("MatrixChoiceBeta", schema => Choice(schema, "beta")) @@ -40,6 +42,14 @@ private static void Patch(SchemaBuilder schema) => schema.Type("object") .AdditionalPropertiesFalse() .Property("patchScalar", NullableString) .Property("patchNumber", NullableNumber) + .Property("patchSpecialNumber", property => property.AnyOf( + branch => branch.AnyOf( + [ + value => value.Type("number"), + .. SpecialNumberLiterals.Select(static literal => + (Action)(value => value.Type("string").Enum(literal))), + ]), + branch => branch.Type("null"))) .Property("patchList", property => property.AnyOf( branch => branch.Type("array").Items(item => item.Type("string")), branch => branch.Type("null"))) diff --git a/tests/OpenCode.Sdk.Tools.Tests/Support/UnionHoistPlanData.cs b/tests/OpenCode.Sdk.Tools.Tests/Support/UnionHoistPlanData.cs index a8660328..a3c05b8a 100644 --- a/tests/OpenCode.Sdk.Tools.Tests/Support/UnionHoistPlanData.cs +++ b/tests/OpenCode.Sdk.Tools.Tests/Support/UnionHoistPlanData.cs @@ -133,6 +133,13 @@ public static NamedTypeReferencePlan Named(string name, bool isNullable = false) JsonNullRepresentation = JsonNullRepresentation.ClrNull, }; + public static SpecialNumberTypeReferencePlan SpecialNumber(bool isNullable = false) => + new() + { + IsNullable = isNullable, + JsonNullRepresentation = JsonNullRepresentation.ClrNull, + }; + /// /// Runs the hoist binder over a scenario and wraps the result as the emit plan the source /// emitters read, for the cases whose proof is the rendered member rather than the bound plan. diff --git a/tools/OpenCode.Sdk.Tools/Generator/Emission/ModelEmitter.cs b/tools/OpenCode.Sdk.Tools/Generator/Emission/ModelEmitter.cs index 7c0dc96d..284ea145 100644 --- a/tools/OpenCode.Sdk.Tools/Generator/Emission/ModelEmitter.cs +++ b/tools/OpenCode.Sdk.Tools/Generator/Emission/ModelEmitter.cs @@ -153,7 +153,7 @@ private static PropertyDeclarationSyntax EmitProperty(ModelPropertyPlan property .PropertyDeclaration(propertyType, memberName) .AddAttributeLists(EmissionSyntax.Attribute("JsonPropertyName", EmissionSyntax.StringArgument(property.WireName))) .WithLeadingTrivia(EmissionSyntax.Documentation(property.Description ?? GeneratedDisplayName.MemberSummary(memberName))); - if (ContainsSpecialNumber(property.Type)) + if (!property.EmitsOptionalWrapper && ContainsSpecialNumber(property.Type)) { declaration = declaration.AddAttributeLists(EmissionSyntax.Attribute( "JsonNumberHandling", diff --git a/tools/OpenCode.Sdk.Tools/Generator/Emission/OptionalConverterEmitter.cs b/tools/OpenCode.Sdk.Tools/Generator/Emission/OptionalConverterEmitter.cs index fda7f289..582c33a2 100644 --- a/tools/OpenCode.Sdk.Tools/Generator/Emission/OptionalConverterEmitter.cs +++ b/tools/OpenCode.Sdk.Tools/Generator/Emission/OptionalConverterEmitter.cs @@ -18,6 +18,14 @@ internal static class OptionalConverterEmitter /// The named types the emitters render without reaching into the generated models namespace. private static readonly string[] NonModelNames = ["bool", "double", "long", "string", "int", "JsonElement", "Uri"]; + /// The named floating-point literals AllowNamedFloatingPointLiterals admits, beside the double member each one names. + private static readonly (string Literal, string Member)[] SpecialNumbers = + [ + ("NaN", "NaN"), + ("Infinity", "PositiveInfinity"), + ("-Infinity", "NegativeInfinity"), + ]; + public static IReadOnlyList Emit(IReadOnlyList models) { ArgumentNullException.ThrowIfNull(models); @@ -27,7 +35,7 @@ public static IReadOnlyList Emit(IReadOnlyList model .SelectMany(static model => model.Properties) .Where(static property => property.EmitsOptionalWrapper) .Select(static property => property.Type) - .DistinctBy(static type => TypeReferenceNamePolicy.Format(type), StringComparer.Ordinal) + .DistinctBy(static type => (TypeReferenceNamePolicy.Format(type), OptionalConverterNamePolicy.ConverterTypeName(type))) .OrderBy(static type => OptionalConverterNamePolicy.ConverterTypeName(type), StringComparer.Ordinal); return Array.AsReadOnly([.. instantiations.Select(EmitConverter)]); } @@ -77,6 +85,25 @@ private static MethodDeclarationSyntax EmitRead(TypeReferencePlan inner, TypeSyn EmissionSyntax.MemberAccess(SyntaxFactory.IdentifierName("reader"), "TokenType"), EmissionSyntax.MemberAccess(SyntaxFactory.IdentifierName("JsonTokenType"), "Null")), SyntaxFactory.Block(SyntaxFactory.ReturnStatement(EmissionSyntax.MemberAccess(wrapper, "Null"))))); + if (inner is SpecialNumberTypeReferencePlan) + { + var literal = EmissionSyntax.Invocation(EmissionSyntax.MemberAccess(SyntaxFactory.IdentifierName("reader"), "GetString")); + var number = SyntaxFactory.SwitchExpression(literal).WithArms(SyntaxFactory.SeparatedList( + SpecialNumbers.Select(static number => SyntaxFactory.SwitchExpressionArm( + SyntaxFactory.ConstantPattern(SyntaxFactory.LiteralExpression(SyntaxKind.StringLiteralExpression, SyntaxFactory.Literal(number.Literal))), + EmissionSyntax.MemberAccess(SyntaxFactory.PredefinedType(SyntaxFactory.Token(SyntaxKind.DoubleKeyword)), number.Member))) + .Append(SyntaxFactory.SwitchExpressionArm(SyntaxFactory.DiscardPattern(), SyntaxFactory.ThrowExpression( + SyntaxFactory.ObjectCreationExpression(SyntaxFactory.IdentifierName("JsonException")) + .WithArgumentList(SyntaxFactory.ArgumentList(SyntaxFactory.SingletonSeparatedList( + SyntaxFactory.Argument(SyntaxFactory.LiteralExpression( + SyntaxKind.StringLiteralExpression, + SyntaxFactory.Literal("The JSON string is not a named floating-point literal."))))))))))); + body.Add(SyntaxFactory.IfStatement(SyntaxFactory.BinaryExpression(SyntaxKind.EqualsExpression, + EmissionSyntax.MemberAccess(SyntaxFactory.IdentifierName("reader"), "TokenType"), + EmissionSyntax.MemberAccess(SyntaxFactory.IdentifierName("JsonTokenType"), "String")), + SyntaxFactory.Block(SyntaxFactory.ReturnStatement(Construct(wrapper, number))))); + } + body.AddRange(EmitReadValue(inner, wrapper)); return SyntaxFactory .MethodDeclaration(wrapper, "Read") @@ -134,6 +161,20 @@ private static MethodDeclarationSyntax EmitWrite(TypeReferencePlan inner, TypeSy SyntaxFactory.ExpressionStatement(EmissionSyntax.Invocation( EmissionSyntax.MemberAccess(SyntaxFactory.IdentifierName("writer"), "WriteNullValue"))), SyntaxFactory.ReturnStatement()))); + if (inner is SpecialNumberTypeReferencePlan) + { + foreach (var number in SpecialNumbers) + { + body.Add(SyntaxFactory.IfStatement(EmissionSyntax.Invocation(EmissionSyntax.MemberAccess( + SyntaxFactory.PredefinedType(SyntaxFactory.Token(SyntaxKind.DoubleKeyword)), "Is" + number.Member), + SyntaxFactory.Argument(EmissionSyntax.MemberAccess(value, "Value"))), SyntaxFactory.Block( + SyntaxFactory.ExpressionStatement(EmissionSyntax.Invocation( + EmissionSyntax.MemberAccess(SyntaxFactory.IdentifierName("writer"), "WriteStringValue"), + SyntaxFactory.Argument(SyntaxFactory.LiteralExpression(SyntaxKind.StringLiteralExpression, SyntaxFactory.Literal(number.Literal))))), + SyntaxFactory.ReturnStatement()))); + } + } + body.AddRange(EmitWriteValue(inner, value)); return SyntaxFactory .MethodDeclaration(SyntaxFactory.PredefinedType(SyntaxFactory.Token(SyntaxKind.VoidKeyword)), "Write") @@ -212,6 +253,7 @@ private static ObjectCreationExpressionSyntax Construct(TypeSyntax wrapper, Expr NamedTypeReferencePlan { Name: "bool" } => "GetBoolean", NamedTypeReferencePlan { Name: "long" } => "GetInt64", NamedTypeReferencePlan { Name: "double" } => "GetDouble", + SpecialNumberTypeReferencePlan => "GetDouble", _ => null, }; @@ -220,6 +262,7 @@ private static ObjectCreationExpressionSyntax Construct(TypeSyntax wrapper, Expr NamedTypeReferencePlan { Name: "string" } => "WriteStringValue", NamedTypeReferencePlan { Name: "bool" } => "WriteBooleanValue", NamedTypeReferencePlan { Name: "long" or "double" } => "WriteNumberValue", + SpecialNumberTypeReferencePlan => "WriteNumberValue", _ => null, }; diff --git a/tools/OpenCode.Sdk.Tools/Generator/Emission/OptionalConverterNamePolicy.cs b/tools/OpenCode.Sdk.Tools/Generator/Emission/OptionalConverterNamePolicy.cs index 7fffb6a8..e25cd1b5 100644 --- a/tools/OpenCode.Sdk.Tools/Generator/Emission/OptionalConverterNamePolicy.cs +++ b/tools/OpenCode.Sdk.Tools/Generator/Emission/OptionalConverterNamePolicy.cs @@ -10,7 +10,9 @@ namespace OpenCode.Sdk.Tools.Generator.Emission; internal static class OptionalConverterNamePolicy { public static string ConverterTypeName(TypeReferencePlan inner) => - $"OptionalOf{Suffix(inner ?? throw new ArgumentNullException(nameof(inner)))}JsonConverter"; + inner is SpecialNumberTypeReferencePlan + ? "OptionalOfSpecialNumberJsonConverter" + : $"OptionalOf{Suffix(inner ?? throw new ArgumentNullException(nameof(inner)))}JsonConverter"; /// /// The instantiation's name in identifier form: the CLR spelling of a primitive, the model