System.Text.Json-attribute support

[xecrets]

Hi!

Just started migrating to ReactiveUI.SourceGenerators from ReactiveUI.Fody. While Fody was very elegant, I do support the move towards source generators for a variety of reasons, although it's still a bit awkward and working with the source is not quite so convenient.

I can live with it all, but I would like to suggest support for attaching System.Text.Json attributes to the generated property.

If you're using the System.Text.Json source generator, which is reasonable given that this is about ReactiveUI.SourceGenerators, it can't serialize private fields. So the explicitly declared private field for example _myProperty, annoted with ReactiveUI.SourceGenerators [Reactive] attribute generates a public property MyProperty.

The problem is that you can't tell the source generator to add a System.Text.Json JsonPropertyName or any other such attributes. It would be really nice if it could support at least JsonPropertyName, JsonInclude and JsonIgnore.

I'm not sure if I like the idea of having ReactiveUI.SourceGenerators.JsonPropertyNameAttribute etc. as the risk for mistakes is too large, I think. Or maybe not, it would look nice... The alternative would perhaps be arguments to the attribute, i.e. something like:

[Reactive(JsonInclude = true, JsonPropertyName = "my_property")]

Or am I completely missing something? Is System.Text.Json supported somehow, and I'm just not reading the documentation properly?

Comments, suggestions, thoughts appreciated.

Never did any work with source generators, but it can't be that hard... If I get some kind of provisional go ahead from the maintainers I could give it a go try to make a pull request.

[glennawatson]

Source generators can't trigger other source generators is a problem. Eg if you put the attributes of the system text json generator in our generator it won't cause it to generate in the same assembly

If you do your json generation in a separate project it can work

That's my concern anyway

[xecrets]

Hi, thanks for the response. Maybe I don't understand this quite well, but it almost already does what I want...

You wrote:

Eg if you put the attributes of the system text json generator in our generator it won't cause it to generate in the same assembly

As far as I can tell the generated soure, which is partial to the annotated class, is indeed in the same assembly. Or am I misunderstanding something? As mentioned, my experience with source generators is not that great, but it seems fairly straightforward and there's nothing magic as far as I know about the generated source, it's just source that gets compiled, right?

Currently, the following:

    [Reactive, JsonInclude, JsonPropertyName("my_property")]
    public double _myProperty;

generates the following (slightly abbreviated):

        [global::System.Text.Json.Serialization.JsonIncludeAttribute()]
        [global::System.Text.Json.Serialization.JsonPropertyNameAttribute("my_property")]
        public double MyProperty { get => _myProperty; set => this.RaiseAndSetIfChanged(ref _myProperty, value); }

which actually seems to work, but A) it requires the backing field to be public, B) it actually annotates both the field and the generated property. Not sure really how that works out, but functionally it seems to work, but it may actually write the Json twice? Haven't checked, but it looks weird.

If instead I try:

    [Reactive, JsonInclude, JsonPropertyName("globalScaleFactor")]
    private double _myProperty;

I get a compilation error, which makes sense, since the System.Text.Json source generator can't access private fields.

Error SYSLIB1038 : The member 'MyClass._myProperty' has been annotated with the JsonIncludeAttribute but is not visible to the source generator. (https://learn.microsoft.com/dotnet/fundamentals/syslib-diagnostics/syslib1038)

The source generator can certainly emit attributes in the generated source, as it seems to 'mirror' the attributes set on the backing field. But, as mentioned, it requires the field to be public and we get the weird situation that both a field and a property both has the same JsonInclude, JsonPropertyName annotation.

If the ReactiveUI.SourceGenerators generator instead accepted arguments to it's ReactiveAttribute class, as for example I suggested:

[Reactive(JsonInclude = true, JsonPropertyName = "my_property")]

and this caused the generator to emit source code with the JsonInclude and JsonPropertyName attributes on the generated property, this would both solve the duplicate annotations, and remove the need for the backing field to be public.

Does this make sense?

[glennawatson]

dotnet/roslyn#57912
https://discussions.unity.com/t/roslyn-code-generator-and-ecs-s-code-gen-pipeline/1596913

Thanks! To clarify the constraint: Roslyn source generators don’t have a defined run order and each generator sees the same initial compilation. Code/attributes that one generator emits aren’t visible to other generators in the same compilation round, so adding JsonPropertyName/JsonInclude from ReactiveUI.SourceGenerators won’t cause the System.Text.Json source generator to pick them up in that project. That’s by design of the generator pipeline (no inter-generator dependencies / ordering).

[xecrets]

Code/attributes that one generator emits aren’t visible to other generators in the same compilation round, so adding JsonPropertyName/JsonInclude from ReactiveUI.SourceGenerators won’t cause the System.Text.Json source generator to pick them up in that project

Oh... Bummer... I see. Thanks for the explanation.

[glennawatson]

The work around:
Practical options:

• Split projects: put the types that need STJ source-gen into a project that references the project where ReactiveUI.SourceGenerators ran. Then STJ sees the generated properties via metadata, rather than relying on attributes emitted by another generator in the same compilation. Note that your solution of adding to the Reactive attribute probably can assist with the idea, but we'd likely get people commenting since they aren't getting the predicted output since 1 source generator can't interact with another.
  GitHub

So my point is likely we'd be potentially adding to our support workload where people aren't understanding the output from our ReactiveUI source generator isn't having the desired output on the STJ output.

[xecrets]

put the types that need STJ source-gen into a project that references the project where ReactiveUI.SourceGenerators ran.

Not sure I completely understand this, but will try and come back if I still don't get it after some experimentation. If I get it working, maybe I could write something up that can be included in the documentation, since I'm guessing I won't be the only only one wanting to use [Reactive] and System.Text.Json on the same type.

Thanks for taking the time!

[glennawatson]

If you make a new csproj, it's a new generation unit, so all your JsonProperty tags etc will be picked up in the new csproj, but since you can't run multiple dependent source generators on the one csproj it won't pick up the injected Json tags.

In terms of doing a PR btw I'm happy for you to do a PR as long as you document the hidden trap in the readme.md file btw.

[xecrets]

I've verified that it works fine to move the types that needs both [Reactive] and System.Text.Json attributes for subsequent source generation for Json serialization to a separate assembly. There's no need it seems to add any Json-specific attributes or arguments, the source generator will happily take for example:

    [Reactive, JsonInclude, JsonPropertyName("myProperty")]
    private double _myProperty;

and generate

        [global::System.Text.Json.Serialization.JsonIncludeAttribute()]
        [global::System.Text.Json.Serialization.JsonPropertyNameAttribute("myProperty")]
        public double MyProperty { get => _myProperty; set => this.RaiseAndSetIfChanged(ref _myProperty, value); 

(Not even sure if I need the JsonInclude now, but it doesn't hurt.)

I've also submitted a very small PR with just a summary of how this works in the Readme.

Thanks for the help and explanation.