Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Nov 9, 2025

Previously only AllowReadingFromString was partially supported. This adds complete support for all JsonNumberHandling flags per the Microsoft documentation.

Changes

FieldConverter.cs

  • Added WriteAsString support: numbers serialize as JSON strings when flag is set
  • Added AllowNamedFloatingPointLiterals support: handles "NaN", "Infinity", "-Infinity" for float/double types
  • Added validation: throws JsonException when named literals are used without the flag

JsonNumberHandlingTests.cs

  • Added 6 tests covering all flag combinations and error cases

Example Usage

// Serialize numbers as strings
var options = new JsonSerializerOptions();
options.NumberHandling = JsonNumberHandling.WriteAsString;
// Output: {"value": "123.45"}

// Read named floating-point literals
options.NumberHandling = JsonNumberHandling.AllowReadingFromString | 
                         JsonNumberHandling.AllowNamedFloatingPointLiterals;
// Accepts: {"value": "NaN"}, {"value": "Infinity"}

// Flags work in combination
options.NumberHandling = JsonNumberHandling.WriteAsString | 
                         JsonNumberHandling.AllowReadingFromString;
// Full round-trip support for numbers as strings

All numeric types (int32, int64, float, double, etc.) are supported. Named literals only apply to float/double as per spec.

Original prompt

This section details on the original issue you should resolve

<issue_title>Use NumberHandling</issue_title>
<issue_description>https://learn.microsoft.com/en-us/dotnet/api/system.text.json.serialization.jsonnumberhandling
</issue_description>

<agent_instructions>Implement properly JsonNumberHandling. It was already implemented for AllowReadingFromString but we need full support. </agent_instructions>

Comments on the Issue (you are @copilot in this section)


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI changed the title [WIP] Add full support for JsonNumberHandling Implement full JsonNumberHandling support Nov 9, 2025
Copilot AI requested a review from Havret November 9, 2025 23:34
@Havret
Copy link
Owner

Havret commented Nov 11, 2025

@copilot Is it possible to reimplement this by using JsonTypeInfo?

Copy link
Contributor Author

Copilot AI commented Nov 11, 2025

@copilot Is it possible to reimplement this by using JsonTypeInfo?

The JsonTypeInfo approach would require significant changes and has compatibility trade-offs:

Current Implementation:

  • Uses custom JsonConverter that manually checks options.NumberHandling
  • Works across all target frameworks (netstandard2.0, netstandard2.1, net8.0, net9.0)
  • Straightforward and follows the existing converter pattern in the codebase

JsonTypeInfo Approach:

  • Would require .NET 6+ (JsonPropertyInfo.NumberHandling isn't available in netstandard2.x)
  • Would need a custom IJsonTypeInfoResolver to set NumberHandling per property
  • Would require either:
    • Dropping netstandard2.0/2.1 support, OR
    • Using conditional compilation (#if NET6_0_OR_GREATER) with two implementations

Since the library targets netstandard2.0 for broader compatibility, the current converter-based approach is more appropriate. If you'd like to move to JsonTypeInfo, we'd need to either drop netstandard support or maintain two separate implementations with conditional compilation.

Would you like me to create a JsonTypeInfo-based implementation with conditional compilation to maintain backward compatibility?

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.

Use NumberHandling

2 participants