[System.Text.Json] Utf8JsonReader/Writer should support TimeSpan

[layomia]

from @blankensteiner:

As explained in the related issue #32429, I'm currently writing my own (de)serializer because I need to support creating the object without calling a constructor (using System.Runtime.Serialization.FormatterServices.GetUninitializedObject) and getting/setting private and read-only fields (using System.Reflection.FieldInfo.SetValue).

Just like JsonSerializer and JsonDocument, I am building on top of Utf8JsonReader and Utf8JsonWriter.
That decision was made because it's encouraged:

The Utf8JsonReader is a low-level type that can be used to build custom parsers and deserializers

and because I want to generate the exact same JSON and if possible also respect all options (JsonWriterOptions, JsonReaderOptions, JsonSerializerOptions).

However, I find that I'm not able to just focus on mapping from C# fields to JSON properties (and vise versa), because I have to duplicate functionality/behavior. I think these methods are missing from the current API:

public ref partial struct Utf8JsonReader
{
    public TimeSpan GetTimeSpan();
    public bool TryGetTimeSpan(out TimeSpan value);
}

public sealed partial class Utf8JsonWriter : System.IAsyncDisposable, System.IDisposable
{
    public void WriteString(string propertyName, TimeSpan value)
}

public readonly partial struct JsonElement
{
    public TimeSpan GetTimeSpan();
    public bool TryGetTimeSpan(out TimeSpan value);
}

In my opinion, it's the domain of Utf8JsonReader and Utf8JsonWriter to know how to (de)serialize the primitive types to whatever data types JSON happens to support. They already know how to do this for most types, but inconsistently TimeSpan is left out. By that logic (the arguments for not adding it), I don't see why DateTime and DateTimeOffset are handled in the reader and writer, but not TimeSpan? The challenge is the same: We have a C#-type and a JSON string, do some magic.

In .NET Core 3.1 a TimeSpan is mapped to an object, but in .NET 5 that will change as TimeSpans are mapped to strings. That is an implementation I have to duplicate in my serializer. If that logic was instead pushed down into Utf8JsonReader and Utf8JsonWriter, all users (and people like me creating serializers) could be free'ed from dealing with that lower-level logic.
We should just pass through the wish to get a TimeSpan and not care what JSON data type the authors of Utf8JsonReader/Writer chose for it and what mapping logic there currently is.

[layomia]

@blankensteiner can you please update the description to address TimeSpan only and include the new API proposals in a format ready for review? See #29936 for an example specific to Utf8JsonReader/Writer.

[blankensteiner]

Hi @layomia
I would love to, but I can't edit your post. Should I just create a new comment and copy/past + the alteration needed?

[layomia]

@blankensteiner - ah, I see. Yes, a new comment is fine.

[blankensteiner]

As explained in the related issue #32429, I'm currently writing my own (de)serializer because I need to support creating the object without calling a constructor (using System.Runtime.Serialization.FormatterServices.GetUninitializedObject) and getting/setting private and read-only fields (using System.Reflection.FieldInfo.SetValue).

Just like JsonSerializer and JsonDocument, I am building on top of Utf8JsonReader and Utf8JsonWriter.
That decision was made because it's encouraged:

The Utf8JsonReader is a low-level type that can be used to build custom parsers and deserializers

and because I want to generate the exact same JSON and if possible also respect all options (JsonWriterOptions, JsonReaderOptions, JsonSerializerOptions).

However, I find that I'm not able to just focus on mapping from C# fields to JSON properties (and vise versa), because I have to duplicate functionality/behavior. I think these methods are missing from the current API:

public ref partial struct Utf8JsonReader
{
    public TimeSpan GetTimeSpan();
    public bool TryGetTimeSpan(out TimeSpan value);
}

public sealed partial class Utf8JsonWriter : System.IAsyncDisposable, System.IDisposable
{
    public void WriteString(string propertyName, TimeSpan value)
}

public readonly partial struct JsonElement
{
    public TimeSpan GetTimeSpan();
    public bool TryGetTimeSpan(out TimeSpan value);
}

In my opinion, it's the domain of Utf8JsonReader and Utf8JsonWriter to know how to (de)serialize the primitive types to whatever data types JSON happens to support. They already know how to do this for most types, but inconsistently TimeSpan is left out. By that logic (the arguments for not adding it), I don't see why DateTime and DateTimeOffset are handled in the reader and writer, but not TimeSpan? The challenge is the same: We have a C#-type and a JSON string, do some magic.

In .NET Core 3.1 a TimeSpan is mapped to an object, but in .NET 5 that will change as TimeSpans are mapped to strings. That is an implementation I have to duplicate in my serializer. If that logic was instead pushed down into Utf8JsonReader and Utf8JsonWriter, all users (and people like me creating serializers) could be free'ed from dealing with that lower-level logic.
We should just pass through the wish to get a TimeSpan and not care what JSON data type the authors of Utf8JsonReader/Writer chose for it and what mapping logic there currently is.

[CodeBlanch]

I went through System.Text.Json and added TimeSpan support everywhere DateTime \ DateTimeOffset were supported. Below are the API additions necessary to do that. PR is #53908. Happy to remove some stuff if not everything is approved but I thought it better to be consistent.

System.Text.Json full TimeSpan support public API changes

    public readonly partial struct JsonElement
    {
       public System.TimeSpan GetTimeSpan() { throw null; }
       public bool TryGetTimeSpan(out System.TimeSpan value) { throw null; }
    }

    public ref partial struct Utf8JsonReader
    {
       public System.TimeSpan GetTimeSpan() { throw null; }
       public bool TryGetTimeSpan(out System.TimeSpan value) { throw null; }
    }

    public sealed partial class Utf8JsonWriter
    {
       public void WriteString(System.ReadOnlySpan<byte> utf8PropertyName, System.TimeSpan value) { }
       public void WriteString(System.ReadOnlySpan<char> propertyName, System.TimeSpan value) { }
       public void WriteString(string propertyName, System.TimeSpan value) { }
       public void WriteString(System.Text.Json.JsonEncodedText propertyName, System.TimeSpan value) { }
       public void WriteStringValue(System.TimeSpan value) { }
    }

    public abstract partial class JsonNode
    {
       public static explicit operator System.TimeSpan (System.Text.Json.Nodes.JsonNode value) { throw null; }
       public static explicit operator System.TimeSpan? (System.Text.Json.Nodes.JsonNode? value) { throw null; }
       public static implicit operator System.Text.Json.Nodes.JsonNode (System.TimeSpan value) { throw null; }
       public static implicit operator System.Text.Json.Nodes.JsonNode? (System.TimeSpan? value) { throw null; }
    }

    public abstract partial class JsonValue
    {
       public static System.Text.Json.Nodes.JsonValue Create(System.TimeSpan value, System.Text.Json.Nodes.JsonNodeOptions? options = default(System.Text.Json.Nodes.JsonNodeOptions?)) { throw null; }
       public static System.Text.Json.Nodes.JsonValue? Create(System.TimeSpan? value, System.Text.Json.Nodes.JsonNodeOptions? options = default(System.Text.Json.Nodes.JsonNodeOptions?)) { throw null; }
    }

    public static partial class JsonMetadataServices
    {
       public static System.Text.Json.Serialization.JsonConverter<System.TimeSpan> TimeSpanConverter { get { throw null; } }
    }

[eiriktsarpalis]

FWIW I'm not convinced that we should be adding TimeSpan support at the Utf8JsonReader/Utf8JsonWriter layer. It is not a serialization API so arguably it should be reserved for primitives and not support every single type under the BCL.

[CodeBlanch]

Good point @eiriktsarpalis. I think in this case since DateTime & DateTimeOffset are there, and TimeSpan is a closely related type, it might be worth doing. System.Buffers.Text.Utf8Parser/Utf8Formatter has all three, not sure the reasoning behind that.

PS: How did Guid get in there? 🤣