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

Refactor JsonPropertyInfo initialization infrastructure and implement JsonPropertyInfo.AttributeProvider #71514

Merged
merged 7 commits into from
Jul 4, 2022
19 changes: 19 additions & 0 deletions src/libraries/System.Text.Json/src/System/ReflectionExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -43,5 +43,24 @@ public static bool IsAssignableFromInternal(this Type type, Type from)

private static bool HasJsonConstructorAttribute(ConstructorInfo constructorInfo)
=> constructorInfo.GetCustomAttribute<JsonConstructorAttribute>() != null;

public static TAttribute? GetUniqueCustomAttribute<TAttribute>(this MemberInfo memberInfo, bool inherit)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: consider ICustomAttributeProvider rather than MemberInfo

where TAttribute : Attribute
{
object[] attributes = memberInfo.GetCustomAttributes(typeof(TAttribute), inherit);

if (attributes.Length == 0)
{
return null;
}

if (attributes.Length == 1)
{
return (TAttribute)attributes[0];
}

ThrowHelper.ThrowInvalidOperationException_SerializationDuplicateAttribute(typeof(TAttribute), memberInfo);
return null;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ internal virtual void ReadElementAndSetProperty(
throw new InvalidOperationException(SR.NodeJsonObjectCustomConverterNotAllowedOnExtensionProperty);
}

internal abstract JsonPropertyInfo CreateJsonPropertyInfo(JsonTypeInfo parentTypeInfo);
internal abstract JsonPropertyInfo CreateJsonPropertyInfo(JsonTypeInfo declaringTypeInfo, Type propertyType, JsonSerializerOptions options);
eiriktsarpalis marked this conversation as resolved.
Show resolved Hide resolved

internal abstract JsonParameterInfo CreateJsonParameterInfo();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ protected JsonConverterFactory() { }
/// </returns>
public abstract JsonConverter? CreateConverter(Type typeToConvert, JsonSerializerOptions options);

internal override JsonPropertyInfo CreateJsonPropertyInfo(JsonTypeInfo parentTypeInfo)
internal override JsonPropertyInfo CreateJsonPropertyInfo(JsonTypeInfo declaringTypeInfo, Type propertyType, JsonSerializerOptions options)
{
Debug.Fail("We should never get here.");

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,9 @@ public override bool CanConvert(Type typeToConvert)

internal override ConverterStrategy ConverterStrategy => ConverterStrategy.Value;

internal sealed override JsonPropertyInfo CreateJsonPropertyInfo(JsonTypeInfo parentTypeInfo)
internal sealed override JsonPropertyInfo CreateJsonPropertyInfo(JsonTypeInfo declaringTypeInfo, Type propertyType, JsonSerializerOptions options)
{
return new JsonPropertyInfo<T>(parentTypeInfo);
return new JsonPropertyInfo<T>(declaringTypeInfo.Type, propertyType, declaringTypeInfo, options);
eiriktsarpalis marked this conversation as resolved.
Show resolved Hide resolved
}

internal sealed override JsonParameterInfo CreateJsonParameterInfo()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,21 +27,15 @@ public sealed partial class JsonSerializerOptions
public IList<JsonConverter> Converters => _converters;

// This may return factory converter
internal JsonConverter? GetCustomConverterFromMember(Type? parentClassType, Type typeToConvert, MemberInfo? memberInfo)
internal JsonConverter? GetCustomConverterFromMember(Type typeToConvert, MemberInfo memberInfo)
{
Debug.Assert(memberInfo.DeclaringType != null, "Properties and fields always have a declaring type.");
JsonConverter? converter = null;

if (memberInfo != null)
JsonConverterAttribute? converterAttribute = memberInfo.GetUniqueCustomAttribute<JsonConverterAttribute>(inherit: false);
if (converterAttribute != null)
{
Debug.Assert(parentClassType != null);

JsonConverterAttribute? converterAttribute = (JsonConverterAttribute?)
GetAttributeThatCanHaveMultiple(parentClassType!, typeof(JsonConverterAttribute), memberInfo);

if (converterAttribute != null)
{
converter = GetConverterFromAttribute(converterAttribute, typeToConvert, classTypeAttributeIsOn: parentClassType!, memberInfo);
}
converter = GetConverterFromAttribute(converterAttribute, typeToConvert, memberInfo);
}

return converter;
Expand Down Expand Up @@ -180,12 +174,10 @@ private JsonConverter GetConverterFromOptionsOrReflectionConverter(Type typeToCo
// Priority 2: Attempt to get converter from [JsonConverter] on the type being converted.
if (converter == null)
{
JsonConverterAttribute? converterAttribute = (JsonConverterAttribute?)
GetAttributeThatCanHaveMultiple(typeToConvert, typeof(JsonConverterAttribute));

JsonConverterAttribute? converterAttribute = typeToConvert.GetUniqueCustomAttribute<JsonConverterAttribute>(inherit: false);
if (converterAttribute != null)
{
converter = GetConverterFromAttribute(converterAttribute, typeToConvert: typeToConvert, classTypeAttributeIsOn: typeToConvert, memberInfo: null);
converter = GetConverterFromAttribute(converterAttribute, typeToConvert: typeToConvert, memberInfo: null);
}
}

Expand Down Expand Up @@ -215,29 +207,30 @@ private JsonConverter GetConverterFromOptionsOrReflectionConverter(Type typeToCo
// This suppression needs to be removed. https://github.com/dotnet/runtime/issues/68878
[UnconditionalSuppressMessage("AotAnalysis", "IL3050:RequiresDynamicCode", Justification = "The factory constructors are only invoked in the context of reflection serialization code paths " +
"and are marked RequiresDynamicCode")]
private JsonConverter GetConverterFromAttribute(JsonConverterAttribute converterAttribute, Type typeToConvert, Type classTypeAttributeIsOn, MemberInfo? memberInfo)
private JsonConverter GetConverterFromAttribute(JsonConverterAttribute converterAttribute, Type typeToConvert, MemberInfo? memberInfo)
{
JsonConverter? converter;

Type? type = converterAttribute.ConverterType;
if (type == null)
Type declaringType = memberInfo?.DeclaringType ?? typeToConvert;
Type? converterType = converterAttribute.ConverterType;
if (converterType == null)
{
// Allow the attribute to create the converter.
converter = converterAttribute.CreateConverter(typeToConvert);
if (converter == null)
{
ThrowHelper.ThrowInvalidOperationException_SerializationConverterOnAttributeNotCompatible(classTypeAttributeIsOn, memberInfo, typeToConvert);
ThrowHelper.ThrowInvalidOperationException_SerializationConverterOnAttributeNotCompatible(declaringType, memberInfo, typeToConvert);
}
}
else
{
ConstructorInfo? ctor = type.GetConstructor(Type.EmptyTypes);
if (!typeof(JsonConverter).IsAssignableFrom(type) || ctor == null || !ctor.IsPublic)
ConstructorInfo? ctor = converterType.GetConstructor(Type.EmptyTypes);
if (!typeof(JsonConverter).IsAssignableFrom(converterType) || ctor == null || !ctor.IsPublic)
{
ThrowHelper.ThrowInvalidOperationException_SerializationConverterOnAttributeInvalid(classTypeAttributeIsOn, memberInfo);
ThrowHelper.ThrowInvalidOperationException_SerializationConverterOnAttributeInvalid(declaringType, memberInfo);
}

converter = (JsonConverter)Activator.CreateInstance(type)!;
converter = (JsonConverter)Activator.CreateInstance(converterType)!;
}

Debug.Assert(converter != null);
Expand All @@ -255,38 +248,10 @@ private JsonConverter GetConverterFromAttribute(JsonConverterAttribute converter
return NullableConverterFactory.CreateValueConverter(underlyingType, converter);
}

ThrowHelper.ThrowInvalidOperationException_SerializationConverterOnAttributeNotCompatible(classTypeAttributeIsOn, memberInfo, typeToConvert);
ThrowHelper.ThrowInvalidOperationException_SerializationConverterOnAttributeNotCompatible(declaringType, memberInfo, typeToConvert);
}

return converter;
}

private static Attribute? GetAttributeThatCanHaveMultiple(Type classType, Type attributeType, MemberInfo memberInfo)
{
object[] attributes = memberInfo.GetCustomAttributes(attributeType, inherit: false);
return GetAttributeThatCanHaveMultiple(attributeType, classType, memberInfo, attributes);
}

internal static Attribute? GetAttributeThatCanHaveMultiple(Type classType, Type attributeType)
{
object[] attributes = classType.GetCustomAttributes(attributeType, inherit: false);
return GetAttributeThatCanHaveMultiple(attributeType, classType, null, attributes);
}

private static Attribute? GetAttributeThatCanHaveMultiple(Type attributeType, Type classType, MemberInfo? memberInfo, object[] attributes)
{
if (attributes.Length == 0)
{
return null;
}

if (attributes.Length == 1)
{
return (Attribute)attributes[0];
}

ThrowHelper.ThrowInvalidOperationException_SerializationDuplicateAttribute(attributeType, classType, memberInfo);
return default;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,7 @@ internal CustomJsonTypeInfo(JsonSerializerOptions options)
private static JsonConverter GetConverter(JsonSerializerOptions options)
{
DefaultJsonTypeInfoResolver.RootDefaultInstance();
return GetEffectiveConverter(
krwq marked this conversation as resolved.
Show resolved Hide resolved
typeof(T),
parentClassType: null, // A TypeInfo never has a "parent" class.
memberInfo: null, // A TypeInfo never has a "parent" property.
options);
return options.GetConverterForType(typeof(T));
}

internal override JsonParameterInfoValues[] GetParameterInfoValues()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public virtual JsonTypeInfo GetTypeInfo(Type type, JsonSerializerOptions options

_mutable = false;

JsonTypeInfo.ValidateType(type, null, null, options);
JsonTypeInfo.ValidateType(type);
JsonTypeInfo typeInfo = CreateJsonTypeInfo(type, options);

if (_modifiers != null)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,7 @@ public static JsonPropertyInfo CreatePropertyInfo<T>(JsonSerializerOptions optio
throw new InvalidOperationException(SR.Format(SR.FieldCannotBeVirtual, nameof(propertyInfo.IsProperty), nameof(propertyInfo.IsVirtual)));
}

JsonPropertyInfo<T> jsonPropertyInfo = new JsonPropertyInfo<T>(parentTypeInfo: null);
jsonPropertyInfo.InitializeForSourceGen(options, propertyInfo);
return jsonPropertyInfo;
return new JsonPropertyInfo<T>(options, propertyInfo);
}

/// <summary>
Expand Down
Loading