STJ fast-path generator does not always deduplicate shadowed members correctly.

tanveerbadar commented 7 months ago

Description

System.Text.Json source generator is generating code which fails to compile if the following conditions are met:

using polymorphic serialization
[JsonSourceGenerationOptions(DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull)]
same property name is present in at least two types in an inheritance hierarchy
their json property names differ, by having different [JsonPropertyName] attributes applied or customized in only some of the types.

Reproduction Steps

Following code demonstrates the problem

using System.Text.Json.Serialization;

[JsonPolymorphic]
[JsonDerivedType(typeof(DerivedType))]
partial class BaseType
{
    [JsonPropertyName("MoreData")]
    public string MoreData { get; set; }
}

partial class DerivedType : BaseType
{
    [JsonPropertyName("MoreData2")]
    public new string MoreData { get; set; }
}

[JsonSerializable(typeof(BaseType))]
[JsonSourceGenerationOptions(DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull)]
partial class MyContext : JsonSerializerContext
{
}

Expected behavior

Generated code should have no compilation errors.

Actual behavior

Generated code has 2 locals with the same name.

    // Intentionally not a static method because we create a delegate to it. Invoking delegates to instance
    // methods is almost as fast as virtual calls. Static methods need to go through a shuffle thunk.
    private void DerivedTypeSerializeHandler(global::System.Text.Json.Utf8JsonWriter writer, global::DerivedType? value)
    {
        if (value == null)
        {
            writer.WriteNullValue();
            return;
        }

        writer.WriteStartObject();

        string __value_MoreData = ((global::DerivedType)value).MoreData;
        if (__value_MoreData != null)
        {
            writer.WriteString(PropName_MoreData2, __value_MoreData);
        }
        string __value_MoreData = ((global::BaseType)value).MoreData; // Error in this line.
        if (__value_MoreData != null)
        {
            writer.WriteString(PropName_MoreData, __value_MoreData);
        }

        writer.WriteEndObject();
    }

Regression?

Not sure

Known Workarounds

No response

Configuration

Using .net 8, Windows 10, x64.

Other information

No response

ghost commented 7 months ago

Tagging subscribers to this area: @dotnet/area-system-text-json, @gregsdennis See info in area-owners.md if you want to be subscribed.

Issue Details

### Description System.Text.Json source generator is generating code which fails to compile if the following conditions are met: - using polymorphic serialization - `[JsonSourceGenerationOptions(DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull)]` - same property name is present in at least two types in an inheritance hierarchy - their json property names differ, by having different [JsonPropertyName] attributes applied or customized in only some of the types. ### Reproduction Steps Following code demonstrates the problem ``` using System.Text.Json.Serialization; [JsonPolymorphic] [JsonDerivedType(typeof(DerivedType))] partial class BaseType { [JsonPropertyName("MoreData")] public string MoreData { get; set; } } partial class DerivedType : BaseType { [JsonPropertyName("MoreData2")] public new string MoreData { get; set; } } [JsonSerializable(typeof(BaseType))] [JsonSourceGenerationOptions(DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull)] partial class MyContext : JsonSerializerContext { } ``` ### Expected behavior Generated code should have no compilation errors. ### Actual behavior Generated code has 2 locals with the same name. ``` // Intentionally not a static method because we create a delegate to it. Invoking delegates to instance // methods is almost as fast as virtual calls. Static methods need to go through a shuffle thunk. private void DerivedTypeSerializeHandler(global::System.Text.Json.Utf8JsonWriter writer, global::DerivedType? value) { if (value == null) { writer.WriteNullValue(); return; } writer.WriteStartObject(); string __value_MoreData = ((global::DerivedType)value).MoreData; if (__value_MoreData != null) { writer.WriteString(PropName_MoreData2, __value_MoreData); } string __value_MoreData = ((global::BaseType)value).MoreData; // Error in this line. if (__value_MoreData != null) { writer.WriteString(PropName_MoreData, __value_MoreData); } writer.WriteEndObject(); } ``` ### Regression? Not sure ### Known Workarounds _No response_ ### Configuration Using .net 8, Windows 10, x64. ### Other information _No response_

Author:	tanveerbadar
Assignees:	-
Labels:	`area-System.Text.Json`, `untriaged`
Milestone:	-

eiriktsarpalis commented 7 months ago

This isn't related to polymorphism per se, here's a minimal reproduction:

partial class BaseType
{
    public string? MoreData { get; set; }
}

partial class DerivedType : BaseType
{
    [JsonPropertyName("MoreData2")]
    public new string? MoreData { get; set; }
}

[JsonSerializable(typeof(DerivedType))]
[JsonSourceGenerationOptions(DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull)]
partial class MyContext : JsonSerializerContext;

TL;DR the fast-path serializer isn't correctly deduplicating shadowed members when the shadowing type uses a custom property name.

dotnet / runtime