runtime: System.Text.Json source generator fails to deserialize init-only & required properties

@jeffhandley Just to chime in on the importance of this: Right now, Records are the recommended way of doing DTOs in .NET. If you use the recommended way of doing DTOs in .NET then you can’t use the source generator. That’s a major issue because your recommendations are in direct conflict and makes Json source generators largely useless unless you open yourself up to mutable DTOs which is exactly what DTOs are not.

I’d strongly suggest that this gets prioritized for .NET 7 to be able to deserialize records properly otherwise this feature is really just a dustbin feature that doesn’t get used and wastes dev’s time and makes us annoyed that a good feature that significantly improves perf can’t be used in the obvious use case. I think the comments makes that obvious.

At the very least this needs to be right at the top of the documentation for .NET 6+ until it’s fixed “If you’re using records or init only properties, don’t bother with this functionality it doesn’t work.” Of course that will generate a ton of complaints from people that get annoyed with the obviousness of the only real use case for these not working, but at least you’re being honest and direct and preventing people from wasting their time.

As a long-time .NET dev I’d like to provide another perspective, or rather, my expectations.

When I tripped over this yesterday, we had started a new project at work and written a bunch of types we want to deserialize. As I remembered the relatively recent blog post regarding the System.Text.Json source generator, I wanted to give it a try and started with the metadata approach. Init-only properties are a perfect match for our data structure, so I quickly checked the docs and saw they are supported, thus I used them. Then, when deserialization didn’t yield the expected result, I thought that I had done something wrong and it took me quite a while to pinpoint the problem and build the repro.

Thanks to your analysis and explanations, I do now see the underlying technical reasons for the current implementation.

I think there is another option to be discussed, though. Basically, I’m thinking of the System.Text.Json source generator as a performance optimization. Therefore, I do expect it to exhibit the exact same semantics as the standard (non-generated) deserializer. Of course, there are a lot of cases where optimizations achieve better performance exactly because they only support a subset of features, and that’s fine. However, in these cases the optimized code only has two options from my pov:

1. Fall back to a less optimized solution like @eiriktsarpalis suggested. It might not yield the same speedups, but at least it works. At the minimum, it must achieve the same performance that the non-generated approach has.
2. Fail with an exception, telling the developer that and in the best case why the optimization is unavailable in the respective situation. Such as:

            properties[1] = global::System.Text.Json.Serialization.Metadata.JsonMetadataServices.CreatePropertyInfo<global::System.String>(
                getter: static (obj) => ((global::JsonSerialization.TagLine)obj).Settable,
                setter: static (obj, value) => throw new NotSupportedException("You can only have init-only properties OR source generators, but not both. Choose wisely."),
...);

The currently implemented option (different behavior) would be ok if it would be a totally revamped API (e.g. version 2 with lots of other breaking changes), but in the case of a performance optimization it’s a violation of the principle of least surprise and I’d say subpar.

@jeffhandley @JohnGalt1717 I just want to second the sense of urgency here. My team is actively trying to do two things simultaneously: follow guidance for best practices on newest frameworks, and optimize performance in a setting where microseconds matter. Not having this feature means that we are having to do some uncomfortable half-measures throughout our code. In particular, we’re trying to use:

• Records for better code brevity in code contracts
• Null check enabling
• Generated serializers

With generated serializers unable to cope with init members, our use of records is as noted earlier just using standard classes with the class keyword replaced by record. It loses most of the benefit of using records, and means all properties have public setters, which in turn makes null checking incredibly grumpy. The resulting code is awkward and unnatural at best, confusing and kind of ugly at worst. It’s enough that it makes members of my team uncomfortable, PR reviews painful, and denies us the best parts of each of the features we are trying to use.

The feature of generated serializers in its current form being unable to play well with records, which are arguably the biggest tentpole feature of contract coding alongside generated serializers, is a substantial gap and makes the story about why to use them really, really hard to sell. Records and generated serializers together are a consistent story about code simplification and speed around JSON contracts. Having them essentially pitted against each other for adoption seems a situation we want to remedy as soon as possible.

I’m in the same spot as zejji - I have an asp.net web api that makes use of immutable record types, so I’m getting SYSLIB1037 warnings telling me my types can’t be deserialized.

My initial reaction to this warning was: huh? shouldn’t deserialization be calling the constructor? And if I ignore the warning and give it a go, sure enough, it works just fine. Seems like the analyzer is off here, and should only be emitting that warning for init-only properties that aren’t parameters of the deserialization constructor.

As such, I’d have to disable this warning altogether (eg, <NoWarn>SYSLIB1037</NoWarn>), which will also silence legitimate warnings about non-constructor init-only properties. That feels dangerous, so unfortunately I think I will also have to shelve the source generator for the time being.

I’d also like to comment on layomia’s suggestion:

Folks can use the source generator for improved serialization performance, and use the pre-existing deserialization methods that work using reflection.

This seems like a non-starter for asp.net use cases. We don’t get separate JsonSerializerOptions for serialization and deserialization. We either call options.SerializerOptions.AddContext or we don’t, and the context we pass either includes the type or it doesn’t.

I’d imagine a generated deserialize handler would have no problem with init-only properties, as it would use a perfectly normal new Foo(...) { Prop = ... } expression that could set those init-only properties during construction. For metadata-only mode, I’d be in favor of a reflection fallback for init-only properties.

• I disagree that it’s a hack. If it’s such a terrible awful idea unto itself, then why was it deemed acceptable behavior for the reflection-based deserializer? Not implying any expectation here about feature parity between the two serializers, just pointing out that there’s precedent for using reflection to set an init-only property.
• I disagree that it defeats the purpose of the source generator. That would be true for deserialization of a type that consisted only of init-only properties that aren’t constructor parameters. For a type that uses mostly constructor parameters or mostly mutable properties, and just has one init-only property, you’re back to reflection for the entire type. You miss out on all the benefits of the source generator for the constructor (deserialization), the non-init-only property setters (deserialization), and the property getters (serialization) - which you would have gotten if there were a reflection fallback for that one init-only property. That actually defeats the purpose of the source generator, since it just can’t be used at all.

I could live with the reflection fallback requiring an explicit opt-in. But I’m not of the opinion that it’s necessary. If I’m a novice, I don’t really understand what all this means anyway, so that opt-in flag is just one more arcane confusing thing for me to trip over. If I’m an expert, then I think by giving my type an init-only property, I’ve already opted in - how else do I think a deserializer in metadata mode is supposed to set that property?

https://github.com/dotnet/runtime/pull/68064 only concerns improvements for positional record parameters. The problem hasn’t been addressed fully yet, which is why this issue is still open.

@eiriktsarpalis The PR is opened: https://github.com/dotnet/runtime/pull/68064

I don’t think that’s right.

When a property happens to have an init-only setter, but is also included in the constructor chosen for deserialization, then deserialization will not use the init-only setter - it will pass the value in via the constructor. The fast path isn’t disrupted, and the values DO round trip.

This is exactly the pattern that arises from desugaring record types into classes. The positional parameters become constructor parameters, and also happen to get init-only property setters.

This is why the source generator actually does work just fine with record types, if you ignore the SYSLIB1037 warning and your record types don’t have any non-positional init-only properties. But disabling the warning at the project level is a bad idea, because then you also won’t get the legitimate warnings for those non-positional init-only properties that aren’t constructor parameters and do break the fast path.

To give an example, this class will work just fine with the source generator and shouldn’t produce the SYSLIB1037 warning:

public class Foo
{
  public string Bar { get; init; } // <-- analyzer produces SYSLIB1037 because it found an init-only property setter

  public Foo(string Bar) // <-- but it's a constructor parameter! deserialization isn't going to use the property setter
  {
    this.Bar = Bar;
  }
}

And that class is of course the desugaring of: public record Foo(string Bar)

Just to echo @dallmair’s comment above, we were also investigating using the System.Text.Json source generator in a new minimal API, together with the following configuration to specify that the generated code should be used to serialize/deserialize all requests and responses:

builder.Services.Configure<JsonOptions>(options =>
{
    options.SerializerOptions.AddContext<MyJsonContext>();
});

Since we are making use of record types for all requests and responses, we immediately ran into error “SYSLIB1037 The type ‘<typename>’ defines init-only properties, deserialization of which is currently not supported in source generation mode.”

Obviously we’ve had to shelve using the source generator and go back to standard reflection-based (de)serialization, but I just wanted to reinforce the point that it is reasonable to expect that the source generator would exhibit the exact same semantics as the standard (non-generated) deserializer.

This warning should only occur using a 6.0 version of the SDK. If you force a 7.0 version of the sdk (e.g. by setting the sdk version in your global.json file) the warning should go away, even if your project is targeting net6.0.

Thank you for the valuable input, @JohnGalt1717. I appreciate each aspect of your recommendations here.

I’m updating this feature’s milestone to Future. We understand that handling init-only properties will be very useful, but we want to set expectations appropriately that it is not likely to make it into .NET 7.

Please refer to #63762 to see our revised set of System.Text.Json work planned for .NET 7. We are also doing a lot of incremental refactoring during .NET 7 that makes it easier for us to add features like this one and ensure developers have a more consistent experience when using System.Text.Json #63918. The list of PRs in System.Text.Json illustrates the progress we’ve been making on that effort.

I just did a deep dive on C# 10/.Net 6 features for my team this week and ran into this problem.

Specifically, I was trying to demonstrate this feature but also show off the new record and record struct features, and in their shorthand forms these two features are incompatible. It seems a real shame, sadly, because it would be really, REALLY nice in our codebase to be able to do the following:

public record MyClass(...);

And then be able to use source generation to enable fast serialization/deserialization. Right now, because we have a great deal of code for JSON contracts that look like:

public class MyClass { public string? Field1 { get; set; } public int Field1 { get; set; } }

It would be wonderful to be able to move these to just using the shorthand record notation, which would also allow us to remove the nullable notation on some fields. In the meantime, I’m recommending to them to simply swap the class keyword for record, which gives us some of the benefit, but this really feels like a missed opportunity.

@eerhardt has suggested that this can be solved without reflection by extending the JsonConstructor mechanism so that init-only properties are incorporated as constructor parameters.

I was totally expecting this to be the mechanism for getting around this, and found myself very surprised when JsonConstructor didn’t get rid of this warning.

I do not think we would ever add any enforcement around it to the runtime. It would be a hard to justify breaking change.

The lack of runtime enforcement is actually an intended feature here. We specifically designed init properties to be friendly for reflection-based JSON deserialization here.

it is reasonable to expect that the source generator would exhibit the exact same semantics as the standard (non-generated) deserializer.

This is our goal as well, though please keep in mind that it is not always possible to achieve 100% convergence. For instance, JsonIncludeAttribute will likely never be supported in sourcegen since it requires private reflection to work.

We are currently working on a document meant to highlight existing semantic differences between sourcegen and the reflection-based serializer: https://github.com/dotnet/docs/issues/26040

@eerhardt has suggested that this can be solved without reflection by extending the JsonConstructor mechanism so that init-only properties are incorporated as constructor parameters.

We’re addressing the issue in 6.0 by emitting warnings and runtime exceptions. We will be working on a proper fix for 7.0.0.

@jkotas @jcouv - do you have any thoughts here? Is relying on reflection to set an init property a good long-term decision?

Reading https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/proposals/csharp-9.0/init, it says:

The init accessor makes immutable objects more flexible by allowing the caller to mutate the members during the act of construction. That means the object’s immutable properties can participate in object initializers

Should we consider “deserialization” a “construction phase” of the object? Is the runtime guaranteed to allow reflection to set init properties long-term?

I agree that the current behavior of failing silently is not good, however the reflection-based approach feels like too much of a hack. For 6.0 I propose we throw NotSupportedException with a clear message in the setter code, and see if we can land on a better solution in the future. Folks can use the source generator for improved serialization performance, and use the pre-existing deserialization methods that work using reflection.

do you think it might make sense to include a reflection-based workaround? For example:

So far we have avoided the use of runtime reflection in the source-gen programming model, as a first-class goal of the feature. Having a reflection workaround seems like something users should explicitly opt-into via a feature/new API. I don’t think the use of init-only setters should implicitly opt users into runtime reflection.

This makes sense in principle, however the end result is we’re blocking users with DTOs using init-only properties. The reflection suggested here should be linker-safe and have no issue other than being slightly slower compared to equivalent properties with setters. I don’t think think this should be opt-in behavior, getting the correct serialization behavior should not require additional configuration.

given that fast-path deserialization has not been implemented yet

I believe the same issues would apply in the generated code for fast-path deserialization, unless I’m missing something.

A fast-path deserializer should be capable of initializing properties at construction path.

cc @steveharter @ericstj @eerhardt for more thoughts.