BinaryData + JSON source gen produces compiler warnings

[krwq]

EDIT by @eiriktsarpalis to reflect more recent error modes

In #73448 we've added internal converter for BinaryData but that's not working with JSON source-gen.

Repro

using System.Text.Json.Serialization;

[JsonSerializable(typeof(BinaryData))]
partial class MyContext : JsonSerializerContext { }

Expected behavior:

Compiles and works when you serialize/deserialize

Actual

warning SYSLIB1041: The 'JsonConverterAttribute' type 'System.BinaryDataConverter' specified on member 'System.BinaryData' is not a converter type or does not contain an accessible parameterless constructor.

API Proposal

We should make the constructor of the already included custom converter public:

namespace System;

[JsonConverter(typeof(BinaryDataConverter))]
public partial class BinaryData { }

-internal sealed class BinaryDataConverter : JsonConverter<BinaryData> { }
+namespace System.Text.Json.Serialization;
+
+public sealed class BinaryDataConverter : JsonConverter<BinaryData> { }

[ghost]

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

Issue Details

---

In #73448 we've added internal converter for BinaryData but that's not working with JSON source-gen.

Repro

using System.Text.Json.Serialization;

[JsonSerializable(typeof(BinaryData))]
partial class MyContext : JsonSerializerContext { }

Expected behavior:

Works

Actual

Error    CS0122    'BinaryDataConverter' is inaccessible due to its protection level

Code produced by source-gen (7.0):

// <auto-generated/>

#nullable enable annotations
#nullable disable warnings

// Suppress warnings about [Obsolete] member usage in generated code.
#pragma warning disable CS0618
    partial class MyContext
    {
        private global::System.Text.Json.Serialization.Metadata.JsonTypeInfo<global::System.BinaryData>? _BinaryData;
        /// <summary>
        /// Defines the source generated JSON serialization contract metadata for a given type.
        /// </summary>
        public global::System.Text.Json.Serialization.Metadata.JsonTypeInfo<global::System.BinaryData> BinaryData
        {
            get => _BinaryData ??= Create_BinaryData(Options, makeReadOnly: true);
        }
        
        private global::System.Text.Json.Serialization.Metadata.JsonTypeInfo<global::System.BinaryData> Create_BinaryData(global::System.Text.Json.JsonSerializerOptions options, bool makeReadOnly)
        {
            global::System.Text.Json.Serialization.Metadata.JsonTypeInfo<global::System.BinaryData>? jsonTypeInfo = null;
            global::System.Text.Json.Serialization.JsonConverter? customConverter;
            if (options.Converters.Count > 0 && (customConverter = GetRuntimeProvidedCustomConverter(options, typeof(global::System.BinaryData))) != null)
            {
                jsonTypeInfo = global::System.Text.Json.Serialization.Metadata.JsonMetadataServices.CreateValueInfo<global::System.BinaryData>(options, customConverter);
            }
            else
            {
                global::System.Text.Json.Serialization.JsonConverter converter = new global::System.BinaryDataConverter();
                        global::System.Type typeToConvert = typeof(global::System.BinaryData);
                        if (!converter.CanConvert(typeToConvert))
                        {
                            throw new global::System.InvalidOperationException(string.Format("The converter '{0}' is not compatible with the type '{1}'.", converter.GetType(), typeToConvert));
                        }
                        jsonTypeInfo = global::System.Text.Json.Serialization.Metadata.JsonMetadataServices.CreateValueInfo<global::System.BinaryData> (options, converter); 
            }
        
            if (makeReadOnly)
            {
                jsonTypeInfo.MakeReadOnly();
            }
        
            return jsonTypeInfo;
        }
        
    }

Author:	krwq
Assignees:	-
Labels:	area-System.Memory
Milestone:	8.0.0

[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

---

In #73448 we've added internal converter for BinaryData but that's not working with JSON source-gen.

Repro

using System.Text.Json.Serialization;

[JsonSerializable(typeof(BinaryData))]
partial class MyContext : JsonSerializerContext { }

Expected behavior:

Compiles and works when you serialize/deserialize

Actual

Error    CS0122    'BinaryDataConverter' is inaccessible due to its protection level

Code produced by source-gen (7.0):

// <auto-generated/>

#nullable enable annotations
#nullable disable warnings

// Suppress warnings about [Obsolete] member usage in generated code.
#pragma warning disable CS0618
    partial class MyContext
    {
        private global::System.Text.Json.Serialization.Metadata.JsonTypeInfo<global::System.BinaryData>? _BinaryData;
        /// <summary>
        /// Defines the source generated JSON serialization contract metadata for a given type.
        /// </summary>
        public global::System.Text.Json.Serialization.Metadata.JsonTypeInfo<global::System.BinaryData> BinaryData
        {
            get => _BinaryData ??= Create_BinaryData(Options, makeReadOnly: true);
        }
        
        private global::System.Text.Json.Serialization.Metadata.JsonTypeInfo<global::System.BinaryData> Create_BinaryData(global::System.Text.Json.JsonSerializerOptions options, bool makeReadOnly)
        {
            global::System.Text.Json.Serialization.Metadata.JsonTypeInfo<global::System.BinaryData>? jsonTypeInfo = null;
            global::System.Text.Json.Serialization.JsonConverter? customConverter;
            if (options.Converters.Count > 0 && (customConverter = GetRuntimeProvidedCustomConverter(options, typeof(global::System.BinaryData))) != null)
            {
                jsonTypeInfo = global::System.Text.Json.Serialization.Metadata.JsonMetadataServices.CreateValueInfo<global::System.BinaryData>(options, customConverter);
            }
            else
            {
                global::System.Text.Json.Serialization.JsonConverter converter = new global::System.BinaryDataConverter();
                        global::System.Type typeToConvert = typeof(global::System.BinaryData);
                        if (!converter.CanConvert(typeToConvert))
                        {
                            throw new global::System.InvalidOperationException(string.Format("The converter '{0}' is not compatible with the type '{1}'.", converter.GetType(), typeToConvert));
                        }
                        jsonTypeInfo = global::System.Text.Json.Serialization.Metadata.JsonMetadataServices.CreateValueInfo<global::System.BinaryData> (options, converter); 
            }
        
            if (makeReadOnly)
            {
                jsonTypeInfo.MakeReadOnly();
            }
        
            return jsonTypeInfo;
        }
        
    }

Author:	krwq
Assignees:	eiriktsarpalis
Labels:	area-System.Text.Json, source-generator
Milestone:	8.0.0

[eiriktsarpalis]

In order to have the type supported in source gen the custom converter needs to be made public.

[layomia]

@eiriktsarpalis with #87136 do we stop generating the converter logic?

[eiriktsarpalis]

That's right. It will no longer fail to compile but the custom converter won't be picked up.

[eiriktsarpalis]

#87140 tracks adding a SG diagnostic for this particular failure.

[ghost]

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

Issue Details

---

In #73448 we've added internal converter for BinaryData but that's not working with JSON source-gen.

Repro

using System.Text.Json.Serialization;

[JsonSerializable(typeof(BinaryData))]
partial class MyContext : JsonSerializerContext { }

Expected behavior:

Compiles and works when you serialize/deserialize

Actual

Error    CS0122    'BinaryDataConverter' is inaccessible due to its protection level

Code produced by source-gen (7.0):

// <auto-generated/>

#nullable enable annotations
#nullable disable warnings

// Suppress warnings about [Obsolete] member usage in generated code.
#pragma warning disable CS0618
    partial class MyContext
    {
        private global::System.Text.Json.Serialization.Metadata.JsonTypeInfo<global::System.BinaryData>? _BinaryData;
        /// <summary>
        /// Defines the source generated JSON serialization contract metadata for a given type.
        /// </summary>
        public global::System.Text.Json.Serialization.Metadata.JsonTypeInfo<global::System.BinaryData> BinaryData
        {
            get => _BinaryData ??= Create_BinaryData(Options, makeReadOnly: true);
        }
        
        private global::System.Text.Json.Serialization.Metadata.JsonTypeInfo<global::System.BinaryData> Create_BinaryData(global::System.Text.Json.JsonSerializerOptions options, bool makeReadOnly)
        {
            global::System.Text.Json.Serialization.Metadata.JsonTypeInfo<global::System.BinaryData>? jsonTypeInfo = null;
            global::System.Text.Json.Serialization.JsonConverter? customConverter;
            if (options.Converters.Count > 0 && (customConverter = GetRuntimeProvidedCustomConverter(options, typeof(global::System.BinaryData))) != null)
            {
                jsonTypeInfo = global::System.Text.Json.Serialization.Metadata.JsonMetadataServices.CreateValueInfo<global::System.BinaryData>(options, customConverter);
            }
            else
            {
                global::System.Text.Json.Serialization.JsonConverter converter = new global::System.BinaryDataConverter();
                        global::System.Type typeToConvert = typeof(global::System.BinaryData);
                        if (!converter.CanConvert(typeToConvert))
                        {
                            throw new global::System.InvalidOperationException(string.Format("The converter '{0}' is not compatible with the type '{1}'.", converter.GetType(), typeToConvert));
                        }
                        jsonTypeInfo = global::System.Text.Json.Serialization.Metadata.JsonMetadataServices.CreateValueInfo<global::System.BinaryData> (options, converter); 
            }
        
            if (makeReadOnly)
            {
                jsonTypeInfo.MakeReadOnly();
            }
        
            return jsonTypeInfo;
        }
        
    }

Author:	krwq
Assignees:	-
Labels:	api-suggestion, area-System.Memory
Milestone:	Future

[terrajobst]

Video

• We should add Json somewhere in the name. It felt right to put before the converter.

namespace System.Text.Json.Serialization;

public sealed class BinaryDataJsonConverter : JsonConverter<BinaryData>
{
    // Overrides omitted
}