Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

System.Text.Json is adding EqualityContract properties metadata for records #77675

Closed
Tracked by #79161
Sergio0694 opened this issue Oct 31, 2022 · 5 comments · Fixed by #87136
Closed
Tracked by #79161

System.Text.Json is adding EqualityContract properties metadata for records #77675

Sergio0694 opened this issue Oct 31, 2022 · 5 comments · Fixed by #87136
Assignees
Labels
area-System.Text.Json enhancement Product code improvement that does NOT require public API changes/additions partner-impact This issue impacts a partner who needs to be kept updated
Milestone

Comments

@Sergio0694
Copy link
Contributor

Sergio0694 commented Oct 31, 2022

Description

Possible regression from #58136, cc. @eiriktsarpalis.

"Records contains a property "EqualityContract" of type Type. This leads to the generator generating serializers for many of the reflection related types as for example Type, ConstructorInfo, MethodInfo, Assembly, Module, etc."

Reproduction Steps

Paste the following code:

public sealed record Test;

[JsonSerializable(typeof(Test))]
public sealed partial class MyTestContext : JsonSerializerContext
{
}

Expected behavior

The generated code for Test should not include the "EqualityContract" property.

Actual behavior

// <auto-generated/>

#nullable enable annotations
#nullable disable warnings

// Suppress warnings about [Obsolete] member usage in generated code.
#pragma warning disable CS0618

namespace MyTestProject
{
    public sealed partial class MyTestContext
    {
        private global::System.Text.Json.Serialization.Metadata.JsonTypeInfo<global::MyTestProject.Test>? _Test;
        /// <summary>
        /// Defines the source generated JSON serialization contract metadata for a given type.
        /// </summary>
        public global::System.Text.Json.Serialization.Metadata.JsonTypeInfo<global::MyTestProject.Test> Test
        {
            get => _Test ??= Create_Test(Options);
        }
        
        // 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 global::System.Text.Json.Serialization.Metadata.JsonTypeInfo<global::MyTestProject.Test> Create_Test(global::System.Text.Json.JsonSerializerOptions options)
        {
            global::System.Text.Json.Serialization.Metadata.JsonTypeInfo<global::MyTestProject.Test>? jsonTypeInfo = null;
            global::System.Text.Json.Serialization.JsonConverter? customConverter;
            if (options.Converters.Count > 0 && (customConverter = GetRuntimeProvidedCustomConverter(options, typeof(global::MyTestProject.Test))) != null)
            {
                jsonTypeInfo = global::System.Text.Json.Serialization.Metadata.JsonMetadataServices.CreateValueInfo<global::MyTestProject.Test>(options, customConverter);
            }
            else
            {
                global::System.Text.Json.Serialization.Metadata.JsonObjectInfoValues<global::MyTestProject.Test> objectInfo = new global::System.Text.Json.Serialization.Metadata.JsonObjectInfoValues<global::MyTestProject.Test>()
                {
                    ObjectCreator = static () => new global::MyTestProject.Test(),
                    ObjectWithParameterizedConstructorCreator = null,
                    PropertyMetadataInitializer = _ => TestPropInit(options),
                    ConstructorParameterMetadataInitializer = null,
                    NumberHandling = default,
                    SerializeHandler = TestSerializeHandler
                };
        
                jsonTypeInfo = global::System.Text.Json.Serialization.Metadata.JsonMetadataServices.CreateObjectInfo<global::MyTestProject.Test>(options, objectInfo);
            }
        
            return jsonTypeInfo;
        }
        
        private static global::System.Text.Json.Serialization.Metadata.JsonPropertyInfo[] TestPropInit(global::System.Text.Json.JsonSerializerOptions options)
        {
            global::System.Text.Json.Serialization.Metadata.JsonPropertyInfo[] properties = new global::System.Text.Json.Serialization.Metadata.JsonPropertyInfo[1];
        
            global::System.Text.Json.Serialization.Metadata.JsonPropertyInfoValues<global::System.Type> info0 = new global::System.Text.Json.Serialization.Metadata.JsonPropertyInfoValues<global::System.Type>()
            {
                IsProperty = true,
                IsPublic = false,
                IsVirtual = false,
                DeclaringType = typeof(global::MyTestProject.Test),
                Converter = null,
                Getter = null,
                Setter = null,
                IgnoreCondition = null,
                HasJsonInclude = false,
                IsExtensionData = false,
                NumberHandling = default,
                PropertyName = "EqualityContract",
                JsonPropertyName = null
            };
        
            global::System.Text.Json.Serialization.Metadata.JsonPropertyInfo propertyInfo0 = global::System.Text.Json.Serialization.Metadata.JsonMetadataServices.CreatePropertyInfo<global::System.Type>(options, info0);
            properties[0] = propertyInfo0;
        
            return properties;
        }
        
        private void TestSerializeHandler(global::System.Text.Json.Utf8JsonWriter writer, global::MyTestProject.Test? value)
        {
            if (value == null)
            {
                writer.WriteNullValue();
                return;
            }
        
            writer.WriteStartObject();
        
            writer.WriteEndObject();
        }
    }
}

Regression?

Seems like it might be the case, yes. I'm using 7.0.0-rc.2.22472.3.

Known Workarounds

None.

Configuration

.NET Standard 2.0 class library, using System.Text.Json 7.0.0-rc.2.22472.3.

@ghost ghost added the untriaged New issue has not been triaged by the area owner label Oct 31, 2022
@ghost
Copy link

ghost commented Oct 31, 2022

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

Possible regression from #58136, cc. @eiriktsarpalis.

"Records contains a property "EqualityContract" of type Type. This leads to the generator generating serializers for many of the reflection related types as for example Type, ConstructorInfo, MethodInfo, Assembly, Module, etc."

Reproduction Steps

Paste the following code:

public sealed record Test;

[JsonSerializable(typeof(Test))]
public sealed partial class DAJsonSerializerContext : JsonSerializerContext
{
}

### Expected behavior

The generated code for `Test` should not include the "EqualityContract" property.

### Actual behavior

```csharp
// <auto-generated/>

#nullable enable annotations
#nullable disable warnings

// Suppress warnings about [Obsolete] member usage in generated code.
#pragma warning disable CS0618

namespace MyTestProject
{
    public sealed partial class DAJsonSerializerContext
    {
        private global::System.Text.Json.Serialization.Metadata.JsonTypeInfo<global::MyTestProject.Test>? _Test;
        /// <summary>
        /// Defines the source generated JSON serialization contract metadata for a given type.
        /// </summary>
        public global::System.Text.Json.Serialization.Metadata.JsonTypeInfo<global::MyTestProject.Test> Test
        {
            get => _Test ??= Create_Test(Options);
        }
        
        // 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 global::System.Text.Json.Serialization.Metadata.JsonTypeInfo<global::MyTestProject.Test> Create_Test(global::System.Text.Json.JsonSerializerOptions options)
        {
            global::System.Text.Json.Serialization.Metadata.JsonTypeInfo<global::MyTestProject.Test>? jsonTypeInfo = null;
            global::System.Text.Json.Serialization.JsonConverter? customConverter;
            if (options.Converters.Count > 0 && (customConverter = GetRuntimeProvidedCustomConverter(options, typeof(global::MyTestProject.Test))) != null)
            {
                jsonTypeInfo = global::System.Text.Json.Serialization.Metadata.JsonMetadataServices.CreateValueInfo<global::MyTestProject.Test>(options, customConverter);
            }
            else
            {
                global::System.Text.Json.Serialization.Metadata.JsonObjectInfoValues<global::MyTestProject.Test> objectInfo = new global::System.Text.Json.Serialization.Metadata.JsonObjectInfoValues<global::MyTestProject.Test>()
                {
                    ObjectCreator = static () => new global::MyTestProject.Test(),
                    ObjectWithParameterizedConstructorCreator = null,
                    PropertyMetadataInitializer = _ => TestPropInit(options),
                    ConstructorParameterMetadataInitializer = null,
                    NumberHandling = default,
                    SerializeHandler = TestSerializeHandler
                };
        
                jsonTypeInfo = global::System.Text.Json.Serialization.Metadata.JsonMetadataServices.CreateObjectInfo<global::MyTestProject.Test>(options, objectInfo);
            }
        
            return jsonTypeInfo;
        }
        
        private static global::System.Text.Json.Serialization.Metadata.JsonPropertyInfo[] TestPropInit(global::System.Text.Json.JsonSerializerOptions options)
        {
            global::System.Text.Json.Serialization.Metadata.JsonPropertyInfo[] properties = new global::System.Text.Json.Serialization.Metadata.JsonPropertyInfo[1];
        
            global::System.Text.Json.Serialization.Metadata.JsonPropertyInfoValues<global::System.Type> info0 = new global::System.Text.Json.Serialization.Metadata.JsonPropertyInfoValues<global::System.Type>()
            {
                IsProperty = true,
                IsPublic = false,
                IsVirtual = false,
                DeclaringType = typeof(global::MyTestProject.Test),
                Converter = null,
                Getter = null,
                Setter = null,
                IgnoreCondition = null,
                HasJsonInclude = false,
                IsExtensionData = false,
                NumberHandling = default,
                PropertyName = "EqualityContract",
                JsonPropertyName = null
            };
        
            global::System.Text.Json.Serialization.Metadata.JsonPropertyInfo propertyInfo0 = global::System.Text.Json.Serialization.Metadata.JsonMetadataServices.CreatePropertyInfo<global::System.Type>(options, info0);
            properties[0] = propertyInfo0;
        
            return properties;
        }
        
        private void TestSerializeHandler(global::System.Text.Json.Utf8JsonWriter writer, global::MyTestProject.Test? value)
        {
            if (value == null)
            {
                writer.WriteNullValue();
                return;
            }
        
            writer.WriteStartObject();
        
            writer.WriteEndObject();
        }
    }
}

Regression?

Seems like it might be the case, yes. I'm using 7.0.0-rc.2.22472.3.

Known Workarounds

None.

Configuration

.NET Standard 2.0 class library, using System.Text.Json 7.0.0-rc.2.22472.3.

Other information

No response

Author: Sergio0694
Assignees: -
Labels:

area-System.Text.Json

Milestone: -

@layomia
Copy link
Contributor

layomia commented Nov 1, 2022

I was able to repro. Just to explicitly state the distinction here vs #58136:

We do have a change where the "EqualityContract" property is included in the list of JsonPropertyInfoValues when creating JsonTypeInfo instances for record types, but unlike the other issue we are not generating metadata for System.Type's members. The change in generated output should have minimal impact to assembly size, and has no impact to actual JSON (de)serialization AFAICT. Thus I don't think it needs to be addressed for .NET 7.

cc @krwq @eiriktsarpalis do you know why this property is being added (say a side effect of contract generation changes in 7.0)?

@layomia layomia removed the untriaged New issue has not been triaged by the area owner label Nov 1, 2022
@layomia layomia added this to the 8.0.0 milestone Nov 1, 2022
@eiriktsarpalis
Copy link
Member

I checked and it seems this property is also being added in .NET 6, so based on the feedback in #58136 there might have been a change/regression in .NET 6 GA. Further investigation is needed to determine if and why such a change was made.

@eiriktsarpalis eiriktsarpalis added the enhancement Product code improvement that does NOT require public API changes/additions label Nov 1, 2022
@eiriktsarpalis
Copy link
Member

Related to #66679, as they both seem to stem from us not honoring protected accessibility modifiers (EqualityContract is a compiler-generated protected property).

@Sergio0694
Copy link
Contributor Author

@eiriktsarpalis Just for additional context, this is also one of the group of issues we're hitting in the Microsoft Store, should we add partner-impact here too so that they're all better classified with respect this? Thank you! 😄

@eiriktsarpalis eiriktsarpalis added the partner-impact This issue impacts a partner who needs to be kept updated label Dec 5, 2022
@krwq krwq modified the milestones: 8.0.0, Future Jan 25, 2023
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Jun 5, 2023
@eiriktsarpalis eiriktsarpalis self-assigned this Jun 5, 2023
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Jun 5, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Jul 6, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Text.Json enhancement Product code improvement that does NOT require public API changes/additions partner-impact This issue impacts a partner who needs to be kept updated
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants