Prevent source-gen from generating code for private fields and those marked with JsonIgnore

[krwq]

Originally posted: https://twitter.com/dotMorten/status/1580085523846074369

Following code:

using System.Text.Json.Serialization;

[JsonSerializable(typeof(MyObject))]
[JsonSourceGenerationOptions(GenerationMode = JsonSourceGenerationMode.Metadata, IgnoreReadOnlyFields = true)]
internal partial class MyObjectSerializatioNContext : JsonSerializerContext
{
}

public class MyObject
{
    [JsonPropertyName("test")]
    public string? Test { get; set; }

    [JsonIgnore]
    private readonly ComplexObject? _data;
}
public class ComplexObject
{
    private protected object _thisLock = new object();
}

produces following build errors:

Error CS5001: Program does not contain a static 'Main' method suitable for an entry point
Error CS0122: 'ComplexObject._thisLock' is inaccessible due to its protection level
Error CS0122: 'ComplexObject._thisLock' is inaccessible due to its protection level
Warning CS0169: The field 'MyObject._data' is never used

Here is the generated code - it produces way more than it should:

// <auto-generated/>

#nullable enable annotations
#nullable disable warnings

// Suppress warnings about [Obsolete] member usage in generated code.
#pragma warning disable CS0618
    internal partial class MyObjectSerializatioNContext
    {
        private global::System.Text.Json.Serialization.Metadata.JsonTypeInfo<global::ComplexObject>? _ComplexObject;
        /// <summary>
        /// Defines the source generated JSON serialization contract metadata for a given type.
        /// </summary>
        public global::System.Text.Json.Serialization.Metadata.JsonTypeInfo<global::ComplexObject> ComplexObject
        {
            get => _ComplexObject ??= Create_ComplexObject(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::ComplexObject> Create_ComplexObject(global::System.Text.Json.JsonSerializerOptions options)
        {
            global::System.Text.Json.Serialization.Metadata.JsonTypeInfo<global::ComplexObject>? jsonTypeInfo = null;
            global::System.Text.Json.Serialization.JsonConverter? customConverter;
            if (options.Converters.Count > 0 && (customConverter = GetRuntimeProvidedCustomConverter(options, typeof(global::ComplexObject))) != null)
            {
                jsonTypeInfo = global::System.Text.Json.Serialization.Metadata.JsonMetadataServices.CreateValueInfo<global::ComplexObject>(options, customConverter);
            }
            else
            {
                global::System.Text.Json.Serialization.Metadata.JsonObjectInfoValues<global::ComplexObject> objectInfo = new global::System.Text.Json.Serialization.Metadata.JsonObjectInfoValues<global::ComplexObject>()
                {
                    ObjectCreator = static () => new global::ComplexObject(),
                    ObjectWithParameterizedConstructorCreator = null,
                    PropertyMetadataInitializer = _ => ComplexObjectPropInit(options),
                    ConstructorParameterMetadataInitializer = null,
                    NumberHandling = default,
                    SerializeHandler = null
                };
        
                jsonTypeInfo = global::System.Text.Json.Serialization.Metadata.JsonMetadataServices.CreateObjectInfo<global::ComplexObject>(options, objectInfo);
            }
        
            return jsonTypeInfo;
        }
        
        private static global::System.Text.Json.Serialization.Metadata.JsonPropertyInfo[] ComplexObjectPropInit(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.Object> info0 = new global::System.Text.Json.Serialization.Metadata.JsonPropertyInfoValues<global::System.Object>()
            {
                IsProperty = false,
                IsPublic = false,
                IsVirtual = false,
                DeclaringType = typeof(global::ComplexObject),
                Converter = null,
                Getter = static (obj) => ((global::ComplexObject)obj)._thisLock!,
                Setter = static (obj, value) => ((global::ComplexObject)obj)._thisLock = value!,
                IgnoreCondition = null,
                HasJsonInclude = false,
                IsExtensionData = false,
                NumberHandling = default,
                PropertyName = "_thisLock",
                JsonPropertyName = null
            };
        
            global::System.Text.Json.Serialization.Metadata.JsonPropertyInfo propertyInfo0 = global::System.Text.Json.Serialization.Metadata.JsonMetadataServices.CreatePropertyInfo<global::System.Object>(options, info0);
            properties[0] = propertyInfo0;
        
            return properties;
        }
    }

cc: @dotMorten

[ghost]

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

---

Originally posted: https://twitter.com/dotMorten/status/1580085523846074369

Following code:

using System.Text.Json.Serialization;

[JsonSerializable(typeof(MyObject))]
[JsonSourceGenerationOptions(GenerationMode = JsonSourceGenerationMode.Metadata, IgnoreReadOnlyFields = true)]
internal partial class MyObjectSerializatioNContext : JsonSerializerContext
{
}

public class MyObject
{
    [JsonPropertyName("test")]
    public string? Test { get; set; }

    [JsonIgnore]
    private readonly ComplexObject? _data;
}
public class ComplexObject
{
    private protected object _thisLock = new object();
}

produces following build errors:

Error CS5001: Program does not contain a static 'Main' method suitable for an entry point
Error CS0122: 'ComplexObject._thisLock' is inaccessible due to its protection level
Error CS0122: 'ComplexObject._thisLock' is inaccessible due to its protection level
Warning CS0169: The field 'MyObject._data' is never used

Here is the generated code - it produces way more than it should:

// <auto-generated/>

#nullable enable annotations
#nullable disable warnings

// Suppress warnings about [Obsolete] member usage in generated code.
#pragma warning disable CS0618
    internal partial class MyObjectSerializatioNContext
    {
        private global::System.Text.Json.Serialization.Metadata.JsonTypeInfo<global::ComplexObject>? _ComplexObject;
        /// <summary>
        /// Defines the source generated JSON serialization contract metadata for a given type.
        /// </summary>
        public global::System.Text.Json.Serialization.Metadata.JsonTypeInfo<global::ComplexObject> ComplexObject
        {
            get => _ComplexObject ??= Create_ComplexObject(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::ComplexObject> Create_ComplexObject(global::System.Text.Json.JsonSerializerOptions options)
        {
            global::System.Text.Json.Serialization.Metadata.JsonTypeInfo<global::ComplexObject>? jsonTypeInfo = null;
            global::System.Text.Json.Serialization.JsonConverter? customConverter;
            if (options.Converters.Count > 0 && (customConverter = GetRuntimeProvidedCustomConverter(options, typeof(global::ComplexObject))) != null)
            {
                jsonTypeInfo = global::System.Text.Json.Serialization.Metadata.JsonMetadataServices.CreateValueInfo<global::ComplexObject>(options, customConverter);
            }
            else
            {
                global::System.Text.Json.Serialization.Metadata.JsonObjectInfoValues<global::ComplexObject> objectInfo = new global::System.Text.Json.Serialization.Metadata.JsonObjectInfoValues<global::ComplexObject>()
                {
                    ObjectCreator = static () => new global::ComplexObject(),
                    ObjectWithParameterizedConstructorCreator = null,
                    PropertyMetadataInitializer = _ => ComplexObjectPropInit(options),
                    ConstructorParameterMetadataInitializer = null,
                    NumberHandling = default,
                    SerializeHandler = null
                };
        
                jsonTypeInfo = global::System.Text.Json.Serialization.Metadata.JsonMetadataServices.CreateObjectInfo<global::ComplexObject>(options, objectInfo);
            }
        
            return jsonTypeInfo;
        }
        
        private static global::System.Text.Json.Serialization.Metadata.JsonPropertyInfo[] ComplexObjectPropInit(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.Object> info0 = new global::System.Text.Json.Serialization.Metadata.JsonPropertyInfoValues<global::System.Object>()
            {
                IsProperty = false,
                IsPublic = false,
                IsVirtual = false,
                DeclaringType = typeof(global::ComplexObject),
                Converter = null,
                Getter = static (obj) => ((global::ComplexObject)obj)._thisLock!,
                Setter = static (obj, value) => ((global::ComplexObject)obj)._thisLock = value!,
                IgnoreCondition = null,
                HasJsonInclude = false,
                IsExtensionData = false,
                NumberHandling = default,
                PropertyName = "_thisLock",
                JsonPropertyName = null
            };
        
            global::System.Text.Json.Serialization.Metadata.JsonPropertyInfo propertyInfo0 = global::System.Text.Json.Serialization.Metadata.JsonMetadataServices.CreatePropertyInfo<global::System.Object>(options, info0);
            properties[0] = propertyInfo0;
        
            return properties;
        }
    }

cc: @dotMorten

Author:	krwq
Assignees:	-
Labels:	area-System.Text.Json
Milestone:	8.0.0

[eiriktsarpalis]

This repro is surfacing a couple of issues:

1. The source generator appears to not be honoring accessibility modifiers when accessing private protected properties. Minimal repro:

   [JsonSerializable(typeof(MyPoco))]
   internal partial class MyContext : JsonSerializerContext
   {
   }
   
   public class MyPoco
   {
       private protected int _field;
   }

   This falls in the same family of bugs as JsonSourceGenerationMode.Metadata emits references to internal members from another assembly #66679 and a fix should be addressing both in one go.

2. The same bug as the one reported here, namely the source generator is emitting property metadata even though it is marked JsonIgnore. Note that this is in line with what the reflection serializer is doing -- we should be emitting property metadata for ignored properties, however this should not (in and of itself) trigger generation of metadata for the property type.

I think we should keep this issue open to track the second issue. The first one should be tracked by #66679.

[dotMorten]

LOL you beat me to it - didn't notice you logged it before I logged it this morning. Thanks :-)

[dotMorten]

krwq added this to the 8.0.0 milestone

My issue is I'm trying to get our .NET 6 library compatible with trimming now that 7 is making it default, by moving to the new codegenerator. However this bug is blocking me from doing that, and I'd rather not wait until .net8 before we can officially support trimming in our SDK. What are the odds that this can be backported to the .net6 nuget package?

[eiriktsarpalis]

Development for .NET 6 and .NET 7 has been concluded, and the bar for servicing these releases only includes high-impact issues like regressions, security issues or functional bugs without a known workaround. I'm not saying that this bug doesn't meet the bar, but generally speaking we need more evidence before we can get the gears rolling on a servicing fix.

In your case, one simple workaround would be change your private protected member into a property:

public class ComplexObject
{
    private protected object ThisLock { get; } = new object();
}

I acknowledge that your actual models might be more complicated than this example, but we can always work with you to extend that workaround to your actual codebase.

[dotMorten]

I acknowledge that your actual models might be more complicated than this example

Yes I simplified this example a lot. In the actual case there are 100s of classes that have metadata generated for it, bloating my assembly size.

[krwq]

Unfortunatelly servicing bar is high at this point :-( System.Text.Json ships as a NuGet package though so once we have a fix you should be able to consume it first preview after that

[dotMorten]

Would I be able to use the newer 7.0/8.0 nuget package to just generate the code and still be compatible with .net6? (and in a way where it isn't getting declared as a dependency in the generated nuspec?)

[krwq]

You can use 7.0/8.0 System.Text.Json package on 6.0. You can see all supported target frameworks on nuget.org, i.e.:
https://www.nuget.org/packages/System.Text.Json/7.0.0-rc.2.22472.3#supportedframeworks-body-tab

[dotMorten]

That's awesome. That would make the need for a backport less important.

[dotMorten]

Just want to add here that this issue prevents me from moving from DataContractJsonSerializer to S.T.J and fully support trimming.

[eiriktsarpalis]

Related to #66679.

[andrewdbond]

In your case, one simple workaround would be change your private protected member into a property:

public class ComplexObject
{
    private protected object ThisLock { get; } = new object();
}

I acknowledge that your actual models might be more complicated than this example, but we can always work with you to extend that workaround to your actual codebase.

I guess this workaround would not work in cases where one of the serialized class's properties or fields is of a type we can't modify, such as one of the types in System.Text.RegularExpressions—is there a recommended workaround for similar cases?

public class SerializedClass
{
  [JsonIgnore]
  private Regex MyProperty { get; set; }
}

Thank you.

[eiriktsarpalis]

Moving to 8.0.0 since this seems to be impacting partner projects.

cc @stephentoub

[dotMorten]

Thank you @eiriktsarpalis. MUCH appreciated.