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

[release/8.0-rc1] Fix support for non-public default constructors using JsonIncludeAttribute #90615

Merged
merged 3 commits into from
Aug 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ private static JsonTypeInfo CreateTypeInfoCore(Type type, JsonConverter converte
typeInfo.PopulatePolymorphismMetadata();
typeInfo.MapInterfaceTypesToCallbacks();

Func<object>? createObject = MemberAccessor.CreateConstructor(typeInfo.Type);
Func<object>? createObject = DetermineCreateObjectDelegate(type, converter);
typeInfo.SetCreateObjectIfCompatible(createObject);
typeInfo.CreateObjectForExtensionDataProperty = createObject;

Expand Down Expand Up @@ -411,5 +411,24 @@ internal static void DeterminePropertyAccessors<T>(JsonPropertyInfo<T> jsonPrope
break;
}
}

[RequiresUnreferencedCode(JsonSerializer.SerializationUnreferencedCodeMessage)]
[RequiresDynamicCode(JsonSerializer.SerializationRequiresDynamicCodeMessage)]
private static Func<object>? DetermineCreateObjectDelegate(Type type, JsonConverter converter)
{
ConstructorInfo? defaultCtor = null;

if (converter.ConstructorInfo != null && !converter.ConstructorIsParameterized)
{
// A parameterless constructor has been resolved by the converter
// (e.g. it might be a non-public ctor with JsonConverterAttribute).
defaultCtor = converter.ConstructorInfo;
}

// Fall back to resolving any public constructors on the type.
defaultCtor ??= type.GetConstructor(BindingFlags.Public | BindingFlags.Instance, binder: null, Type.EmptyTypes, modifiers: null);

return MemberAccessor.CreateParameterlessConstructor(type, defaultCtor);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,9 @@ namespace System.Text.Json.Serialization.Metadata
{
internal abstract class MemberAccessor
{
public abstract Func<object>? CreateConstructor(
[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] Type classType);
public abstract Func<object>? CreateParameterlessConstructor(
[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] Type type,
ConstructorInfo? constructorInfo);

public abstract Func<object[], T> CreateParameterizedConstructor<T>(ConstructorInfo constructor);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,12 @@ internal sealed partial class ReflectionEmitCachingMemberAccessor : MemberAccess
=> s_cache.GetOrAdd((nameof(CreateAddMethodDelegate), typeof(TCollection), null),
static (_) => s_sourceAccessor.CreateAddMethodDelegate<TCollection>());

public override Func<object>? CreateConstructor([DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] Type classType)
=> s_cache.GetOrAdd((nameof(CreateConstructor), classType, null),
public override Func<object>? CreateParameterlessConstructor([DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] Type type, ConstructorInfo? ctorInfo)
=> s_cache.GetOrAdd((nameof(CreateParameterlessConstructor), type, ctorInfo),
[UnconditionalSuppressMessage("ReflectionAnalysis", "IL2077:UnrecognizedReflectionPattern",
Justification = "Cannot apply DynamicallyAccessedMembersAttribute to tuple properties.")]
#pragma warning disable IL2077 // The suppression doesn't work for the trim analyzer: https://github.com/dotnet/roslyn/issues/59746
static (key) => s_sourceAccessor.CreateConstructor(key.declaringType));
static (key) => s_sourceAccessor.CreateParameterlessConstructor(key.declaringType, (ConstructorInfo?)key.member));
#pragma warning restore IL2077

public override Func<object, TProperty> CreateFieldGetter<TProperty>(FieldInfo fieldInfo)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,18 +13,19 @@ namespace System.Text.Json.Serialization.Metadata
[RequiresDynamicCode(JsonSerializer.SerializationRequiresDynamicCodeMessage)]
internal sealed class ReflectionEmitMemberAccessor : MemberAccessor
{
public override Func<object>? CreateConstructor(
[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] Type type)
public override Func<object>? CreateParameterlessConstructor(
[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] Type type,
ConstructorInfo? constructorInfo)
{
Debug.Assert(type != null);
ConstructorInfo? realMethod = type.GetConstructor(BindingFlags.Public | BindingFlags.Instance, binder: null, Type.EmptyTypes, modifiers: null);
Debug.Assert(constructorInfo is null || constructorInfo.GetParameters().Length == 0);

if (type.IsAbstract)
{
return null;
}

if (realMethod == null && !type.IsValueType)
if (constructorInfo is null && !type.IsValueType)
{
return null;
}
Expand All @@ -38,8 +39,10 @@ internal sealed class ReflectionEmitMemberAccessor : MemberAccessor

ILGenerator generator = dynamicMethod.GetILGenerator();

if (realMethod == null)
if (constructorInfo is null)
{
Debug.Assert(type.IsValueType);

LocalBuilder local = generator.DeclareLocal(type);

generator.Emit(OpCodes.Ldloca_S, local);
Expand All @@ -49,7 +52,7 @@ internal sealed class ReflectionEmitMemberAccessor : MemberAccessor
}
else
{
generator.Emit(OpCodes.Newobj, realMethod);
generator.Emit(OpCodes.Newobj, constructorInfo);
if (type.IsValueType)
{
// Since C# 10 it's now possible to have parameterless constructors in structs
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,35 +10,26 @@ namespace System.Text.Json.Serialization.Metadata
{
internal sealed class ReflectionMemberAccessor : MemberAccessor
{
private sealed class ConstructorContext
{
[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)]
private readonly Type _type;

public ConstructorContext([DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] Type type)
=> _type = type;

public object? CreateInstance()
=> Activator.CreateInstance(_type, nonPublic: false);
}

public override Func<object>? CreateConstructor(
[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] Type type)
public override Func<object>? CreateParameterlessConstructor(
[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] Type type,
ConstructorInfo? ctorInfo)
{
Debug.Assert(type != null);
ConstructorInfo? realMethod = type.GetConstructor(BindingFlags.Public | BindingFlags.Instance, binder: null, Type.EmptyTypes, modifiers: null);
Debug.Assert(ctorInfo is null || ctorInfo.GetParameters().Length == 0);

if (type.IsAbstract)
{
return null;
}

if (realMethod == null && !type.IsValueType)
if (ctorInfo is null)
{
return null;
return type.IsValueType
? () => Activator.CreateInstance(type, nonPublic: false)!
: null;
}

return new ConstructorContext(type).CreateInstance!;
return () => ctorInfo.Invoke(null);
}

public override Func<object[], T> CreateParameterizedConstructor<T>(ConstructorInfo constructor)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,24 @@ public async Task NonPublicCtors_WithJsonConstructorAttribute_WorksAsExpected(Ty
}
}

[Theory]
[InlineData(typeof(PrivateParameterlessCtor_WithAttribute), false)]
[InlineData(typeof(InternalParameterlessCtor_WithAttribute), true)]
[InlineData(typeof(ProtectedParameterlessCtor_WithAttribute), false)]
public async Task NonPublicParameterlessCtors_WithJsonConstructorAttribute_WorksAsExpected(Type type, bool isAccessibleBySourceGen)
{
if (!Serializer.IsSourceGeneratedSerializer || isAccessibleBySourceGen)
{
object? result = await Serializer.DeserializeWrapper("{}", type);
Assert.IsType(type, result);
}
else
{
NotSupportedException ex = await Assert.ThrowsAsync<NotSupportedException>(() => Serializer.DeserializeWrapper("{}", type));
Assert.Contains("JsonConstructorAttribute", ex.ToString());
}
}

[Fact]
public async Task SinglePublicParameterizedCtor_SingleParameterlessCtor_NoAttribute_Supported_UseParameterlessCtor()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,33 @@ public class ProtectedParameterizedCtor_WithAttribute
protected ProtectedParameterizedCtor_WithAttribute(int x) => X = x;
}

public class PrivateParameterlessCtor_WithAttribute
{
public int X { get; }

[JsonConstructor]
private PrivateParameterlessCtor_WithAttribute()
=> X = 42;
}

public class ProtectedParameterlessCtor_WithAttribute
{
public int X { get; }

[JsonConstructor]
protected ProtectedParameterlessCtor_WithAttribute()
=> X = 42;
}

public class InternalParameterlessCtor_WithAttribute
{
public int X { get; }

[JsonConstructor]
internal InternalParameterlessCtor_WithAttribute()
=> X = 42;
}

public class PrivateParameterlessCtor_InternalParameterizedCtor_WithMultipleAttributes
{
[JsonConstructor]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,9 @@ protected ConstructorTests_Metadata(JsonSerializerWrapper stringWrapper)
[JsonSerializable(typeof(PrivateParameterizedCtor_WithAttribute))]
[JsonSerializable(typeof(InternalParameterizedCtor_WithAttribute))]
[JsonSerializable(typeof(ProtectedParameterizedCtor_WithAttribute))]
[JsonSerializable(typeof(PrivateParameterlessCtor_WithAttribute))]
[JsonSerializable(typeof(InternalParameterlessCtor_WithAttribute))]
[JsonSerializable(typeof(ProtectedParameterlessCtor_WithAttribute))]
[JsonSerializable(typeof(SinglePublicParameterizedCtor))]
[JsonSerializable(typeof(SingleParameterlessCtor_MultiplePublicParameterizedCtor))]
[JsonSerializable(typeof(SingleParameterlessCtor_MultiplePublicParameterizedCtor_Struct))]
Expand Down Expand Up @@ -186,6 +189,9 @@ public ConstructorTests_Default(JsonSerializerWrapper jsonSerializer) : base(jso
[JsonSerializable(typeof(PrivateParameterizedCtor_WithAttribute))]
[JsonSerializable(typeof(InternalParameterizedCtor_WithAttribute))]
[JsonSerializable(typeof(ProtectedParameterizedCtor_WithAttribute))]
[JsonSerializable(typeof(PrivateParameterlessCtor_WithAttribute))]
[JsonSerializable(typeof(InternalParameterlessCtor_WithAttribute))]
[JsonSerializable(typeof(ProtectedParameterlessCtor_WithAttribute))]
[JsonSerializable(typeof(SinglePublicParameterizedCtor))]
[JsonSerializable(typeof(SingleParameterlessCtor_MultiplePublicParameterizedCtor))]
[JsonSerializable(typeof(SingleParameterlessCtor_MultiplePublicParameterizedCtor_Struct))]
Expand Down
Loading