Implementation of JsonConverter.CanConvert is wrong but can be fixed easily

[Petrusion]

For example when using Template.Long the code-generated CanConvert(type) method returns true if the input type is long, string or the strongly typed id struct (via base class implementation). It should only return true for the strongly typed id struct. CanConvert should return true for types it can convert the json into, not types it can convert from. Right now it is saying it can convert the json into a long as well as string, which is not true.

I'm trying to use StronglyTypedId and System.Text.Json source generators together but sadly for some reason System.Text.Json source generators aren't picking up on the JsonConverter attribute which specifies that the source generated converter should be used, maybe other source generators can't see source generated attributes, or there is a problem with them running in the wrong order. It ends up using some default converter which fails at runtime.

When I try to fix this by specifying the source-generated converted explicitly in json options, the deserializer unfortunately tries to call this converter on any properties of type string and long which it shouldn't, and ends up crashing. In the end I have write my own converter.

To fix this, the CanConvert override should just be removed from all templates, as the JsonConverter base class already returns (type == T).

[andrewlock]

CanConvert should return true for types it can convert the json into, not types it can convert from.

Oops, yeah, reading the docs I definitely got this wrong, thanks 👍 I'll fix this ASAP

maybe other source generators can't see source generated attributes

Unfortunately this is the problem. Currently (.NET 8) source generator code output is completely isolated from other source generators. There's no ordering, they just can't see each other's other code at all. Which means you can't use these with source generation, unless you define the IDs in a separate assembly 🙁

[andrewlock]

The deserializer unfortunately tries to call this converter on any properties of type string and long which it shouldn't, and ends up crashing.

Interestingly, I can't seem to replicate this 🤔

If I create a type like this:

internal record ToSerialize
{
    public LongId Id { get; set; }
    public Guid Guid { get; set; } = Guid.NewGuid();
    public long Long { get; set; } = 123;
    public int Int { get; set; } = 456;
    public string String { get; set; } = "Something";
}

[StronglyTypedId(Template.Long)]
internal partial struct LongId { }

and I have a test like this:

[Fact]
public void CanRoundTripWhenInRecord()
{
    var foo = new ToSerialize()
    {
        Id = new ConvertersLongId(123),
    };

    var serialized = SystemTextJsonSerializer.Serialize(foo);
    var deserialized = SystemTextJsonSerializer.Deserialize<ToSerialize>(serialized);

    Assert.Equal(foo, deserialized);
}

Then it works fine, and serializes as

{
  "Id": 123,
  "Guid": "843946f6-863c-4a2e-afd6-9971e695ddd0",
  "Long": 123,
  "Int": 456,
  "String": "Something"
}

which is what I would expect? 🤔 Are you saying this crashes for you?

[Petrusion]

which is what I would expect? 🤔 Are you saying this crashes for you?

It crashes or produces wrong results when json serializers using System.Text.Json source generation are used. So like this:

string testJson = """
                  {
                    "Id": 123,
                    "Guid": "843946f6-863c-4a2e-afd6-9971e695ddd0",
                    "Long": 123,
                    "Int": 456,
                    "String": "Something"
                  }
                  """;

var deserialized = JsonSerializer.Deserialize(testJson, ToDeserializeJsonContext.Default.ToDeserialize);
testJson = JsonSerializer.Serialize(deserialized, ToDeserializeJsonContext.Default.ToDeserialize);
Console.WriteLine(deserialized);

public record ToDeserialize
{
    public LongId Id { get; set; }
    public Guid Guid { get; set; } = Guid.NewGuid();
    public long Long { get; set; } = 123;
    public int Int { get; set; } = 456;
    public string String { get; set; } = "Something";
}

[JsonSourceGenerationOptions(
    AllowTrailingCommas = true,
    NumberHandling = JsonNumberHandling.Strict,
    IncludeFields = false,
    PropertyNameCaseInsensitive = false,
    WriteIndented = false,
    GenerationMode = JsonSourceGenerationMode.Metadata | JsonSourceGenerationMode.Serialization,
    UseStringEnumConverter = false
)]
[JsonSerializable(typeof(ToDeserialize))]
public partial class ToDeserializeJsonContext : JsonSerializerContext;

[StronglyTypedId(Template.Long)]
public partial struct LongId;

But that is a problem which exists because of the limitations of source generators overall.

The bigger problem (which is how I found out about the error in CanConvert) is that when I try to solve it by specifying converters via a JsonSerializerOptions.Converters or JsonSourceGenerationOptions.Converters (when it even picks up on the converter) it will try to convert every property which is not only LongId, but also string and long because of the way CanConvert is curently written.

Which means you can't use these with source generation, unless you define the IDs in a separate assembly 🙁

That actually solves the problem it seems. I'm going to separate the types into another assembly from now. It still probably is a good idea to remove the CanConvert override from Strongly Typed IDs source generators though, so it works correctly if someone puts one of the generated converters into JsonSerializerOptions.Converters