dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.25k stars 4.73k forks source link

JSON source generator emitting source for types unexpectedly #108682

Open stephentoub opened 3 weeks ago

stephentoub commented 3 weeks ago

Description

In the repo below, Exception should not be serialized as it's defined, but rather only via the ExceptionConverter that treats it as a string. However, the source generator is emitting code to handle all of the properties on Exception. While this is unnecessary code, it also results in trimmer warnings about members of Exception that aren't safe to serialized.

Reproduction Steps

using System.Text.Json;
using System.Text.Json.Serialization;

Console.WriteLine(JsonSerializer.Serialize(new MyClass { Error = new Exception("error") } ));

sealed class MyClass
{
    [JsonConverter(typeof(ExceptionConverter))]
    public Exception? Error { get; set; }
}

sealed class ExceptionConverter : JsonConverter<Exception>
{
    public override Exception Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) =>
        new Exception(reader.GetString());

    public override void Write(Utf8JsonWriter writer, Exception? value, JsonSerializerOptions options) =>
        writer.WriteStringValue(value?.Message ?? string.Empty);
}

[JsonSerializable(typeof(MyClass))]
partial class MyContext : JsonSerializerContext;

Expected behavior

Code emitted for Exception related to its converter.

Actual behavior

Emits this:

// <auto-generated/>

#nullable enable annotations
#nullable disable warnings

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

partial class MyContext
{
    private global::System.Text.Json.Serialization.Metadata.JsonTypeInfo<global::System.Exception>? _Exception;

    /// <summary>
    /// Defines the source generated JSON serialization contract metadata for a given type.
    /// </summary>
    #nullable disable annotations // Marking the property type as nullable-oblivious.
    public global::System.Text.Json.Serialization.Metadata.JsonTypeInfo<global::System.Exception> Exception
    #nullable enable annotations
    {
        get => _Exception ??= (global::System.Text.Json.Serialization.Metadata.JsonTypeInfo<global::System.Exception>)Options.GetTypeInfo(typeof(global::System.Exception));
    }

    private global::System.Text.Json.Serialization.Metadata.JsonTypeInfo<global::System.Exception> Create_Exception(global::System.Text.Json.JsonSerializerOptions options)
    {
        if (!TryGetTypeInfoForRuntimeCustomConverter<global::System.Exception>(options, out global::System.Text.Json.Serialization.Metadata.JsonTypeInfo<global::System.Exception> jsonTypeInfo))
        {
            var objectInfo = new global::System.Text.Json.Serialization.Metadata.JsonObjectInfoValues<global::System.Exception>
            {
                ObjectCreator = () => new global::System.Exception(),
                ObjectWithParameterizedConstructorCreator = null,
                PropertyMetadataInitializer = _ => ExceptionPropInit(options),
                ConstructorParameterMetadataInitializer = null,
                ConstructorAttributeProviderFactory = static () => typeof(global::System.Exception).GetConstructor(InstanceMemberBindingFlags, binder: null, global::System.Array.Empty<global::System.Type>(), modifiers: null),
                SerializeHandler = ExceptionSerializeHandler,
            };

            jsonTypeInfo = global::System.Text.Json.Serialization.Metadata.JsonMetadataServices.CreateObjectInfo<global::System.Exception>(options, objectInfo);
            jsonTypeInfo.NumberHandling = null;
        }

        jsonTypeInfo.OriginatingResolver = this;
        return jsonTypeInfo;
    }

    private static global::System.Text.Json.Serialization.Metadata.JsonPropertyInfo[] ExceptionPropInit(global::System.Text.Json.JsonSerializerOptions options)
    {
        var properties = new global::System.Text.Json.Serialization.Metadata.JsonPropertyInfo[8];

        var info0 = new global::System.Text.Json.Serialization.Metadata.JsonPropertyInfoValues<global::System.Collections.IDictionary>
        {
            IsProperty = true,
            IsPublic = true,
            IsVirtual = true,
            DeclaringType = typeof(global::System.Exception),
            Converter = null,
            Getter = static obj => ((global::System.Exception)obj).Data,
            Setter = null,
            IgnoreCondition = null,
            HasJsonInclude = false,
            IsExtensionData = false,
            NumberHandling = null,
            PropertyName = "Data",
            JsonPropertyName = null,
            AttributeProviderFactory = static () => typeof(global::System.Exception).GetProperty("Data", InstanceMemberBindingFlags, null, typeof(global::System.Collections.IDictionary), global::System.Array.Empty<global::System.Type>(), null),
        };

        properties[0] = global::System.Text.Json.Serialization.Metadata.JsonMetadataServices.CreatePropertyInfo<global::System.Collections.IDictionary>(options, info0);
        properties[0].IsGetNullable = false;

        var info1 = new global::System.Text.Json.Serialization.Metadata.JsonPropertyInfoValues<string>
        {
            IsProperty = true,
            IsPublic = true,
            IsVirtual = true,
            DeclaringType = typeof(global::System.Exception),
            Converter = null,
            Getter = static obj => ((global::System.Exception)obj).HelpLink,
            Setter = static (obj, value) => ((global::System.Exception)obj).HelpLink = value!,
            IgnoreCondition = null,
            HasJsonInclude = false,
            IsExtensionData = false,
            NumberHandling = null,
            PropertyName = "HelpLink",
            JsonPropertyName = null,
            AttributeProviderFactory = static () => typeof(global::System.Exception).GetProperty("HelpLink", InstanceMemberBindingFlags, null, typeof(string), global::System.Array.Empty<global::System.Type>(), null),
        };

        properties[1] = global::System.Text.Json.Serialization.Metadata.JsonMetadataServices.CreatePropertyInfo<string>(options, info1);

        var info2 = new global::System.Text.Json.Serialization.Metadata.JsonPropertyInfoValues<int>
        {
            IsProperty = true,
            IsPublic = true,
            IsVirtual = false,
            DeclaringType = typeof(global::System.Exception),
            Converter = null,
            Getter = static obj => ((global::System.Exception)obj).HResult,
            Setter = static (obj, value) => ((global::System.Exception)obj).HResult = value!,
            IgnoreCondition = null,
            HasJsonInclude = false,
            IsExtensionData = false,
            NumberHandling = null,
            PropertyName = "HResult",
            JsonPropertyName = null,
            AttributeProviderFactory = static () => typeof(global::System.Exception).GetProperty("HResult", InstanceMemberBindingFlags, null, typeof(int), global::System.Array.Empty<global::System.Type>(), null),
        };

        properties[2] = global::System.Text.Json.Serialization.Metadata.JsonMetadataServices.CreatePropertyInfo<int>(options, info2);

        var info3 = new global::System.Text.Json.Serialization.Metadata.JsonPropertyInfoValues<global::System.Exception>
        {
            IsProperty = true,
            IsPublic = true,
            IsVirtual = false,
            DeclaringType = typeof(global::System.Exception),
            Converter = null,
            Getter = static obj => ((global::System.Exception)obj).InnerException,
            Setter = null,
            IgnoreCondition = null,
            HasJsonInclude = false,
            IsExtensionData = false,
            NumberHandling = null,
            PropertyName = "InnerException",
            JsonPropertyName = null,
            AttributeProviderFactory = static () => typeof(global::System.Exception).GetProperty("InnerException", InstanceMemberBindingFlags, null, typeof(global::System.Exception), global::System.Array.Empty<global::System.Type>(), null),
        };

        properties[3] = global::System.Text.Json.Serialization.Metadata.JsonMetadataServices.CreatePropertyInfo<global::System.Exception>(options, info3);

        var info4 = new global::System.Text.Json.Serialization.Metadata.JsonPropertyInfoValues<string>
        {
            IsProperty = true,
            IsPublic = true,
            IsVirtual = true,
            DeclaringType = typeof(global::System.Exception),
            Converter = null,
            Getter = static obj => ((global::System.Exception)obj).Message,
            Setter = null,
            IgnoreCondition = null,
            HasJsonInclude = false,
            IsExtensionData = false,
            NumberHandling = null,
            PropertyName = "Message",
            JsonPropertyName = null,
            AttributeProviderFactory = static () => typeof(global::System.Exception).GetProperty("Message", InstanceMemberBindingFlags, null, typeof(string), global::System.Array.Empty<global::System.Type>(), null),
        };

        properties[4] = global::System.Text.Json.Serialization.Metadata.JsonMetadataServices.CreatePropertyInfo<string>(options, info4);
        properties[4].IsGetNullable = false;

        var info5 = new global::System.Text.Json.Serialization.Metadata.JsonPropertyInfoValues<string>
        {
            IsProperty = true,
            IsPublic = true,
            IsVirtual = true,
            DeclaringType = typeof(global::System.Exception),
            Converter = null,
            Getter = static obj => ((global::System.Exception)obj).Source,
            Setter = static (obj, value) => ((global::System.Exception)obj).Source = value!,
            IgnoreCondition = null,
            HasJsonInclude = false,
            IsExtensionData = false,
            NumberHandling = null,
            PropertyName = "Source",
            JsonPropertyName = null,
            AttributeProviderFactory = static () => typeof(global::System.Exception).GetProperty("Source", InstanceMemberBindingFlags, null, typeof(string), global::System.Array.Empty<global::System.Type>(), null),
        };

        properties[5] = global::System.Text.Json.Serialization.Metadata.JsonMetadataServices.CreatePropertyInfo<string>(options, info5);

        var info6 = new global::System.Text.Json.Serialization.Metadata.JsonPropertyInfoValues<string>
        {
            IsProperty = true,
            IsPublic = true,
            IsVirtual = true,
            DeclaringType = typeof(global::System.Exception),
            Converter = null,
            Getter = static obj => ((global::System.Exception)obj).StackTrace,
            Setter = null,
            IgnoreCondition = null,
            HasJsonInclude = false,
            IsExtensionData = false,
            NumberHandling = null,
            PropertyName = "StackTrace",
            JsonPropertyName = null,
            AttributeProviderFactory = static () => typeof(global::System.Exception).GetProperty("StackTrace", InstanceMemberBindingFlags, null, typeof(string), global::System.Array.Empty<global::System.Type>(), null),
        };

        properties[6] = global::System.Text.Json.Serialization.Metadata.JsonMetadataServices.CreatePropertyInfo<string>(options, info6);

        var info7 = new global::System.Text.Json.Serialization.Metadata.JsonPropertyInfoValues<global::System.Reflection.MethodBase>
        {
            IsProperty = true,
            IsPublic = true,
            IsVirtual = false,
            DeclaringType = typeof(global::System.Exception),
            Converter = null,
            Getter = static obj => ((global::System.Exception)obj).TargetSite,
            Setter = null,
            IgnoreCondition = null,
            HasJsonInclude = false,
            IsExtensionData = false,
            NumberHandling = null,
            PropertyName = "TargetSite",
            JsonPropertyName = null,
            AttributeProviderFactory = static () => typeof(global::System.Exception).GetProperty("TargetSite", InstanceMemberBindingFlags, null, typeof(global::System.Reflection.MethodBase), global::System.Array.Empty<global::System.Type>(), null),
        };

        properties[7] = global::System.Text.Json.Serialization.Metadata.JsonMetadataServices.CreatePropertyInfo<global::System.Reflection.MethodBase>(options, info7);

        return properties;
    }

    // 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 ExceptionSerializeHandler(global::System.Text.Json.Utf8JsonWriter writer, global::System.Exception? value)
    {
        if (value is null)
        {
            writer.WriteNullValue();
            return;
        }

        writer.WriteStartObject();

        writer.WritePropertyName(PropName_Data);
        global::System.Text.Json.JsonSerializer.Serialize(writer, ((global::System.Exception)value).Data, IDictionary);
        writer.WriteString(PropName_HelpLink, ((global::System.Exception)value).HelpLink);
        writer.WriteNumber(PropName_HResult, ((global::System.Exception)value).HResult);
        writer.WritePropertyName(PropName_InnerException);
        ExceptionSerializeHandler(writer, ((global::System.Exception)value).InnerException);
        writer.WriteString(PropName_Message, ((global::System.Exception)value).Message);
        writer.WriteString(PropName_Source, ((global::System.Exception)value).Source);
        writer.WriteString(PropName_StackTrace, ((global::System.Exception)value).StackTrace);
        writer.WritePropertyName(PropName_TargetSite);
        global::System.Text.Json.JsonSerializer.Serialize(writer, ((global::System.Exception)value).TargetSite, MethodBase);

        writer.WriteEndObject();
    }
}

Regression?

n/a

Known Workarounds

n/a

Configuration

.NET 9

Other information

No response

dotnet-policy-service[bot] commented 3 weeks ago

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

eiriktsarpalis commented 3 weeks ago

This happens because the source generator will attempt to source generate for every type that is found within the transitive closure of JsonSerializable types, even if those types are only defined on properties that define custom converters:

// A JsonTypeInfo<Bar> has been generated even if it isn't being used by Foo
Console.WriteLine(JsonSerializer.Serialize(new Bar("Value"), MyContext.Default.Bar)); // {"Value":"Value"}

class Foo
{
    [JsonConverter(typeof(CustomBarConverter))]
    public Bar? PropertyWithCustomConverter { get; set; }

    // Serializes Bar as a plain old string
    public class CustomBarConverter : JsonConverter<Bar>
    {
        public override Bar Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
            => new Bar(reader.GetString()!);

        public override void Write(Utf8JsonWriter writer, Bar value, JsonSerializerOptions options) =>
            writer.WriteStringValue(value.Value);
    }
}

record Bar(string Value);

[JsonSerializable(typeof(Foo))]
partial class MyContext : JsonSerializerContext;

I don't think we could change this today without incurring breaking changes.

stephentoub commented 3 weeks ago

How can trimmer warnings be suppressed just in the generated files? Should the source generator be suppressing them with pragmas? Otherwise, it seems you need to suppress them project-wide.

eiriktsarpalis commented 3 weeks ago

Pragma suppressions won't prevent the linker from emitting warnings when it encounters unsafe APIs. The generated code must be annotated with attributes.

stephentoub commented 3 weeks ago

Then what do you recommend? You moved this to future, but that's effectively saying such warnings need to be suppressed at the project level, which then defeats the purpose of such warnings and makes it near impossible to guarantee linker safety.

eiriktsarpalis commented 3 weeks ago

but that's effectively saying such warnings need to be suppressed at the project level

I don't believe either pragma suppressions or project level suppressions at the library will prevent the same warnings from being surfaced when running dotnet publish. Suppressions must be made using attributes. @vitek-karas should be able to confirm this.

Then what do you recommend?

I can't think of anything that doesn't involve introducing new API. An ad hoc fix would involve just disabling generation for exception -- the code that it does generate today by default won't work since it has a number of already unsupported property types.

vitek-karas commented 3 weeks ago

Pragma suppressions only work on analyzer. That said, if the generated code is never used, then trimmer or AOT will not produce warnings on it since they'll get rid of it first. So if the discussion is only around the case where the source generator produces code which is never used, pragma suppressions should be enough.

There's nothing really like "project level suppressions" - even if you suppress them via NoWarn, if it's in the library, the final app will show them again (because trimmer runs on all IL globally). What is in theory possible is injecting suppression attributes via XML trimmer files, but that's not something we want to do if at all possible.

You can suppress the warnings on the app level via NoWarn, just like any other warning, and that will work for all sources (analyzer, trimmer, AOT).

@sbomer