feat(extensions): TryParse overloads, TypeConverter, and a real phone-number ValidationAttribute - #405
Merged
Merged
Conversation
The bare catch in PhoneNumbers.Extensions.PhoneNumber swallowed every exception, not just parse failures, silently masking bugs.
…-number ValidationAttribute - PhoneNumber.TryParse/TryParseValid gain a 2-arg overload (region defaults to null) matching the conventional .NET TryParse shape. - PhoneNumberTypeConverter converts PhoneNumber to/from its E.164 string, for consumers that register TypeConverters (config binding, PropertyGrid, etc.) — not applied automatically. - PhoneNumberAttribute is a DataAnnotations ValidationAttribute backed by PhoneNumberUtil.IsValidNumber, unlike the framework's built-in [Phone] which is a loose regex. Accepts a string (parsed via the optional Region property) or an already-parsed PhoneNumber. netstandard2.0 needs System.ComponentModel.Annotations for ValidationAttribute; net8.0/net10.0 already have it.
Codecov Report❌ Patch coverage is
❌ Your patch check has failed because the patch coverage (83.33%) 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 #405 +/- ##
==========================================
- Coverage 87.47% 87.43% -0.04%
==========================================
Files 39 41 +2
Lines 3839 3861 +22
Branches 986 991 +5
==========================================
+ Hits 3358 3376 +18
Misses 283 283
- Partials 198 202 +4 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
📊 Benchmark Results
PR branch
PR base
|
4 tasks
…neNumberAttribute - PhoneNumberTypeConverter.ConvertFrom now uses Util.Parse instead of ParseAndKeepRawInput. RawInput feeds Equals/GetHashCode on the generated PhoneNumber type, so a number converted through this TypeConverter previously compared unequal/hashed differently from the same number parsed via PhoneNumber.TryParse, even though nothing in ConvertTo ever reads RawInput back. - PhoneNumberAttribute.IsValid now caches PhoneNumberUtil.GetInstance() in a static field, matching the pattern PhoneNumber.cs and PhoneNumberTypeConverter.cs already use.
Owner
Author
|
Updated after code review (8-angle pass): fixed two findings on |
2 tasks
…ng-ergonomics # Conflicts: # csharp/Directory.Packages.props
This was referenced Aug 30, 2026
Closed
pull Bot
pushed a commit
to LoadsAForks/libphonenumber-csharp
that referenced
this pull request
Aug 31, 2026
…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 twcclegg#405 and needed no changes here. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WVGRJyQwtvBGXAmWswxyqW
This was referenced Sep 1, 2026
Merged
Closed
Open
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
PhoneNumber.TryParse/TryParseValidgain a 2-arg overload (region defaults tonull), matching the conventional .NETTryParse(string, out T)shape.PhoneNumberTypeConverterconvertsPhoneNumberto/from its E.164 string — for consumers that registerTypeConverters (config binding,PropertyGrid, MVC model binding, etc.). Not applied automatically; register it explicitly, same pattern as the existingPhoneNumberConverterforSystem.Text.Json.PhoneNumberAttributeis aDataAnnotationsValidationAttributebacked byPhoneNumberUtil.IsValidNumber, unlike the framework's built-in[Phone]which is a loose regex. Accepts astring(parsed via the optionalRegionproperty) or an already-parsedPhoneNumber.Note on scope: an earlier version of this considered a
TryParse(string, IFormatProvider, out PhoneNumber)shape aimed at ASP.NET Core minimal-API automatic parameter binding, but that only works if the method lives onPhoneNumbers.PhoneNumberitself (the framework reflects on the parameter's own type) — decided to keep this Extensions-only rather than touch the core faithful-port model class, so this PR is a plain convenience overload rather than automatic binding support.netstandard2.0needsSystem.ComponentModel.AnnotationsforValidationAttribute;net8.0/net10.0already have it in the shared framework — added centrally inDirectory.Packages.props, referenced only on thenetstandard2.0leg like the existingSystem.Text.Jsonreference.Test plan
dotnet build csharp --no-restore— 0 warnings/errorsdotnet test csharp/PhoneNumbers.slnx— 888 tests passed (net8.0 + net10.0), including new tests for all three additionsdotnet pack -c Release csharp/PhoneNumbers.Extensions -p:VersionPrefix=9.0.38— packs and passesEnablePackageValidationagainst the 9.0.37 baseline (confirms the additions are purely additive, no API-compat breaks)