Skip to content

Extensions: Native AOT JsonSerializerContext + formatting extension methods - #440

Merged
twcclegg merged 2 commits into
mainfrom
feat/extensions-aot-json-and-formatting-helpers
Aug 31, 2026
Merged

Extensions: Native AOT JsonSerializerContext + formatting extension methods#440
twcclegg merged 2 commits into
mainfrom
feat/extensions-aot-json-and-formatting-helpers

Conversation

@twcclegg

@twcclegg twcclegg commented Aug 29, 2026

Copy link
Copy Markdown
Owner

Changes

  • Adds PhoneNumberJsonContext (a source-generated JsonSerializerContext) and PhoneNumberJsonOptions (ready-made JsonSerializerOptions, plus Serialize/Deserialize helpers) to PhoneNumbers.Extensions, so consumers building trimmed/Native AOT apps get JSON support for PhoneNumber without hand-writing a context.
  • Adds PhoneNumberExtensions with ToE164() / ToNationalFormat() / ToInternationalFormat() / IsValid() extension methods on PhoneNumber, for the one-liners callers currently have to route through PhoneNumberUtil.GetInstance() by hand.
  • Sets IsAotCompatible on PhoneNumbers.Extensions's modern TFMs (net8.0/net10.0), matching PhoneNumbers.csproj, so the trim/AOT analyzers keep this enforced in CI going forward.
  • xUnit tests for all of the above in PhoneNumbers.Extensions.Test.

On PR #405 / PhoneNumberAttribute

Per the task brief, I audited whether a ValidationAttribute/IValidatableObject already existed before adding anything. It doesPhoneNumberAttribute : ValidationAttribute (backed by PhoneNumberUtil.IsValidNumber, with an optional Region for national-format strings) was added by PR #405 (commits e559f548 / 257b975e, merged as 9466f2b0) and is present and unchanged on main today, with its own tests in TestPhoneNumberAttributeValidation.cs. Nothing was reverted; no changes were needed here. This PR does not touch it.

The JSON gotcha (found empirically, not just theorized)

PhoneNumber.DefaultInstanceForType is a public get-only property that returns the instance itself. That means the raw member-based JsonTypeInfo<PhoneNumber> the source generator produces for PhoneNumberJsonContext — if you ever call JsonSerializer.Serialize(number, PhoneNumberJsonContext.Default.PhoneNumber) directly instead of going through options — walks straight into infinite recursion instead of ever using PhoneNumberConverter (reproduced locally: InvalidOperationException at the writer's max depth, or a real stack overflow at a higher depth limit). PhoneNumberJsonOptions.Default/Create wire PhoneNumberJsonContext (TypeInfoResolver) and PhoneNumberConverter (Converters) onto the same options instance so this can't happen by accident, and the doc comments on PhoneNumberJsonContext spell out why.

Separately, the options-based JsonSerializer.Serialize(value, options)/Deserialize(...) overloads are always flagged IL2026/IL3050 by a consumer's own trim/AOT analyzer, since the analyzer can't statically prove options.TypeInfoResolver never falls back to reflection. PhoneNumberJsonOptions.Serialize/Deserialize carry UnconditionalSuppressMessage internally (the library can make that guarantee; the analyzer can't) so consumers get a genuinely warning-free path.

Verified end-to-end with a throwaway dotnet publish -r linux-x64 --self-contained -p:PublishAot=true console app (not part of this repo) referencing the built PhoneNumbers/PhoneNumbers.Extensions projects: the published native binary serializes and round-trips a PhoneNumber correctly through PhoneNumberJsonOptions, and confirms the raw-context bypass throws rather than hangs/crashes.

Testing

  • dotnet build csharp --no-restore — full solution, all TFMs — clean, 0 warnings.
  • dotnet test csharp/PhoneNumbers.slnx -p:TargetFrameworks=net10.0 — 464 passed (414 PhoneNumbers.Test + 50 PhoneNumbers.Extensions.Test), 0 failed.
  • dotnet pack -c Release csharp/PhoneNumbers.Extensions -p:VersionPrefix=9.0.38 (above the pinned package-validation baseline) — packs clean, no API-compat breaks; purely additive surface.
  • Native AOT round-trip verified as described above (net8.0 test run was skipped locally — this sandbox only has the net10.0 runtime installed — but net8.0 and netstandard2.0 both build clean).

…sion methods

PhoneNumberJsonContext + PhoneNumberJsonOptions ship a source-generated
JsonSerializerContext for PhoneNumber plus ready-made JsonSerializerOptions,
so consumers building trimmed/Native AOT apps don't have to hand-write one.

The two pieces have to be combined carefully: PhoneNumber.DefaultInstanceForType
is a public get-only property returning the instance itself, so the raw
member-based JsonTypeInfo<PhoneNumber> the source generator produces recurses
into itself without end if ever invoked directly (JsonSerializer.Serialize(v,
Context.Default.PhoneNumber) throws once the writer hits its max depth, or
overflows the stack at a larger depth limit) instead of using
PhoneNumberConverter. PhoneNumberJsonOptions.Default/Create wire the context
and the converter onto the same JsonSerializerOptions so callers can't get
this wrong; PhoneNumberJsonOptions.Serialize/Deserialize additionally avoid
the IL2026/IL3050 trim/AOT warnings a consumer's own analyzer would otherwise
raise on the options-based JsonSerializer overloads. Verified with a
throwaway `dotnet publish -p:PublishAot=true` app (not part of this repo).

IsAotCompatible is now set for Extensions' modern TFMs too, matching
PhoneNumbers.csproj, so this stays enforced in CI.

PhoneNumberExtensions adds ToE164/ToNationalFormat/ToInternationalFormat/
IsValid as extension methods on PhoneNumber, for the common formatting
one-liners callers currently have to route through PhoneNumberUtil.GetInstance()
by hand.

Note: PhoneNumberAttribute (a ValidationAttribute backed by
PhoneNumberUtil.IsValidNumber) already exists on main via PR #405 and needed
no changes here.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WVGRJyQwtvBGXAmWswxyqW
@codecov

codecov Bot commented Aug 29, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 85.71429% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.28%. Comparing base (0bf7478) to head (bcb664b).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
.../PhoneNumbers.Extensions/PhoneNumberJsonOptions.cs 77.77% 2 Missing ⚠️

❌ Your patch check has failed because the patch coverage (85.71%) is below the target coverage (90.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #440      +/-   ##
==========================================
+ Coverage   87.26%   87.28%   +0.02%     
==========================================
  Files          41       43       +2     
  Lines        3831     3845      +14     
  Branches      978      980       +2     
==========================================
+ Hits         3343     3356      +13     
- Misses        284      285       +1     
  Partials      204      204              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

…umberJsonOptions.Create

Create() previously replaced baseOptions.TypeInfoResolver outright with
PhoneNumberJsonContext.Default, which broke the exact "combine with your own
JsonSerializerContext for a DTO with a PhoneNumber property" workflow the
method's own doc comment described: a consumer's own context alone has no
metadata for PhoneNumber, and PhoneNumberJsonContext.Default alone has none
for their DTO, so passing either as baseOptions threw NotSupportedException.
Combine the two resolvers instead when baseOptions already carries one.

Also corrects a doc comment that said PhoneNumber.DefaultInstanceForType
"returns the instance itself" (it returns the static PhoneNumber.DefaultInstance,
not `this`) and drops a dead net6-only #else branch in a test that only ever
targets net8.0/net10.0.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WVGRJyQwtvBGXAmWswxyqW
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants