Skip to content

Commit

Permalink
Add support for serializing properties in interface hierarchies (#78788)
Browse files Browse the repository at this point in the history
* Add support for serializing properties in interface hierarchies.

* Attempt to fix trim analysis error.

* Work around trimmer warning issue and address misc feedback.
  • Loading branch information
eiriktsarpalis authored Nov 25, 2022
1 parent 3d3c23d commit 58f59db
Show file tree
Hide file tree
Showing 8 changed files with 453 additions and 102 deletions.
106 changes: 106 additions & 0 deletions src/libraries/System.Text.Json/Common/ReflectionExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -324,5 +324,111 @@ public static bool TryGetDeserializationConstructor(

return defaultValue;
}

/// <summary>
/// Returns the type hierarchy for the given type, starting from the current type up to the base type(s) in the hierarchy.
/// Interface hierarchies with multiple inheritance will return results using topological sorting.
/// </summary>
public static Type[] GetSortedTypeHierarchy(
#if !BUILDING_SOURCE_GENERATOR
[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.Interfaces)]
#endif
this Type type)
{
if (!type.IsInterface)
{
// Non-interface hierarchies are linear, just walk up to the earliest ancestor.

var results = new List<Type>();
for (Type? current = type; current != null; current = current.BaseType)
{
results.Add(current);
}

return results.ToArray();
}
else
{
// Interface hierarchies support multiple inheritance,
// query the entire list and sort them topologically.
Type[] interfaces = type.GetInterfaces();
{
// include the current type into the list of interfaces
Type[] newArray = new Type[interfaces.Length + 1];
newArray[0] = type;
interfaces.CopyTo(newArray, 1);
interfaces = newArray;
}

TopologicalSort(interfaces, static (t1, t2) => t1.IsAssignableFrom(t2));
return interfaces;
}
}

private static void TopologicalSort<T>(T[] inputs, Func<T, T, bool> isLessThan)
where T : notnull
{
// Standard implementation of in-place topological sorting using Kahn's algorithm.

if (inputs.Length < 2)
{
return;
}

var graph = new Dictionary<T, HashSet<T>>();
var next = new Queue<T>();

// Step 1: construct the dependency graph.
for (int i = 0; i < inputs.Length; i++)
{
T current = inputs[i];
HashSet<T>? dependencies = null;

for (int j = 0; j < inputs.Length; j++)
{
if (i != j && isLessThan(current, inputs[j]))
{
(dependencies ??= new()).Add(inputs[j]);
}
}

if (dependencies is null)
{
next.Enqueue(current);
}
else
{
graph.Add(current, dependencies);
}
}

Debug.Assert(next.Count > 0, "Input graph must be a DAG.");
int index = 0;

// Step 2: Walk the dependency graph starting with nodes that have no dependencies.
do
{
T nextTopLevelDependency = next.Dequeue();

foreach (KeyValuePair<T, HashSet<T>> kvp in graph)
{
HashSet<T> dependencies = kvp.Value;
if (dependencies.Count > 0)
{
dependencies.Remove(nextTopLevelDependency);

if (dependencies.Count == 0)
{
next.Enqueue(kvp.Key);
}
}
}

inputs[index++] = nextTopLevelDependency;
}
while (next.Count > 0);

Debug.Assert(index == inputs.Length);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -1026,7 +1026,8 @@ private TypeGenerationSpec GetOrAddTypeGenerationSpec(Type type, JsonSourceGener

bool propertyOrderSpecified = false;

for (Type? currentType = type; currentType != null; currentType = currentType.BaseType)
// Walk the type hierarchy starting from the current type up to the base type(s)
foreach (Type currentType in type.GetSortedTypeHierarchy())
{
PropertyGenerationSpec spec;

Expand Down Expand Up @@ -1260,6 +1261,7 @@ private PropertyGenerationSpec GetPropertyGenerationSpec(
IsExtensionData = isExtensionData,
TypeGenerationSpec = GetOrAddTypeGenerationSpec(memberCLRType, generationMode),
DeclaringTypeRef = memberInfo.DeclaringType.GetCompilableName(),
DeclaringType = memberInfo.DeclaringType,
ConverterInstantiationLogic = converterInstantiationLogic,
HasFactoryConverter = hasFactoryConverter
};
Expand Down
2 changes: 2 additions & 0 deletions src/libraries/System.Text.Json/gen/PropertyGenerationSpec.cs
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,8 @@ internal sealed class PropertyGenerationSpec
/// </summary>
public string DeclaringTypeRef { get; init; }

public Type DeclaringType { get; init; }

/// <summary>
/// Source code to instantiate design-time specified custom converter.
/// </summary>
Expand Down
57 changes: 41 additions & 16 deletions src/libraries/System.Text.Json/gen/TypeGenerationSpec.cs
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,7 @@ public bool TryFilterSerializableProps(
[NotNullWhen(true)] out Dictionary<string, PropertyGenerationSpec>? serializableProperties,
out bool castingRequiredForProps)
{
castingRequiredForProps = false;
serializableProperties = new Dictionary<string, PropertyGenerationSpec>();
Dictionary<string, PropertyGenerationSpec>? ignoredMembers = null;

Expand Down Expand Up @@ -198,6 +199,10 @@ public bool TryFilterSerializableProps(
continue;
}

// Using properties from an interface hierarchy -- require explicit casting when
// getting properties in the fast path to account for possible diamond ambiguities.
castingRequiredForProps |= Type.IsInterface && propGenSpec.DeclaringType != Type;

string memberName = propGenSpec.ClrName!;

// The JsonPropertyNameAttribute or naming policy resulted in a collision.
Expand All @@ -210,32 +215,52 @@ public bool TryFilterSerializableProps(
// Overwrite previously cached property since it has [JsonIgnore].
serializableProperties[propGenSpec.RuntimePropertyName] = propGenSpec;
}
else if (
// Does the current property have `JsonIgnoreAttribute`?
propGenSpec.DefaultIgnoreCondition != JsonIgnoreCondition.Always &&
// Is the current property hidden by the previously cached property
// (with `new` keyword, or by overriding)?
other.ClrName != memberName &&
// Was a property with the same CLR name was ignored? That property hid the current property,
// thus, if it was ignored, the current property should be ignored too.
ignoredMembers?.ContainsKey(memberName) != true)
else
{
// We throw if we have two public properties that have the same JSON property name, and neither have been ignored.
serializableProperties = null;
castingRequiredForProps = false;
return false;
bool ignoreCurrentProperty;

if (!Type.IsInterface)
{
ignoreCurrentProperty =
// Does the current property have `JsonIgnoreAttribute`?
propGenSpec.DefaultIgnoreCondition == JsonIgnoreCondition.Always ||
// Is the current property hidden by the previously cached property
// (with `new` keyword, or by overriding)?
other.ClrName == memberName ||
// Was a property with the same CLR name ignored? That property hid the current property,
// thus, if it was ignored, the current property should be ignored too.
ignoredMembers?.ContainsKey(memberName) == true;
}
else
{
// Unlike classes, interface hierarchies reject all naming conflicts for non-ignored properties.
// Conflicts like this are possible in two cases:
// 1. Diamond ambiguity in property names, or
// 2. Linear interface hierarchies that use properties with DIMs.
//
// Diamond ambiguities are not supported. Assuming there is demand, we might consider
// adding support for DIMs in the future, however that would require adding more APIs
// for the case of source gen.

ignoreCurrentProperty = propGenSpec.DefaultIgnoreCondition == JsonIgnoreCondition.Always;
}

if (!ignoreCurrentProperty)
{
// We have a conflict, emit a stub method that throws.
goto ReturnFalse;
}
}
// Ignore the current property.
}

if (propGenSpec.DefaultIgnoreCondition == JsonIgnoreCondition.Always)
{
(ignoredMembers ??= new Dictionary<string, PropertyGenerationSpec>()).Add(memberName, propGenSpec);
(ignoredMembers ??= new()).Add(memberName, propGenSpec);
}
}

Debug.Assert(PropertyGenSpecList.Count >= serializableProperties.Count);
castingRequiredForProps = PropertyGenSpecList.Count > serializableProperties.Count;
castingRequiredForProps |= PropertyGenSpecList.Count > serializableProperties.Count;
return true;

ReturnFalse:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -791,25 +791,46 @@ internal void CacheMember(JsonPropertyInfo jsonPropertyInfo, JsonPropertyDiction
// Overwrite previously cached property since it has [JsonIgnore].
propertyCache[jsonPropertyInfo.Name] = jsonPropertyInfo;
}
else if (
// Does the current property have `JsonIgnoreAttribute`?
!jsonPropertyInfo.IsIgnored &&
// Is the current property hidden by the previously cached property
// (with `new` keyword, or by overriding)?
other.MemberName != memberName &&
// Was a property with the same CLR name was ignored? That property hid the current property,
// thus, if it was ignored, the current property should be ignored too.
ignoredMembers?.ContainsKey(memberName) != true)
else
{
// We throw if we have two public properties that have the same JSON property name, and neither have been ignored.
ThrowHelper.ThrowInvalidOperationException_SerializerPropertyNameConflict(Type, jsonPropertyInfo.Name);
bool ignoreCurrentProperty;

if (!Type.IsInterface)
{
ignoreCurrentProperty =
// Does the current property have `JsonIgnoreAttribute`?
jsonPropertyInfo.IsIgnored ||
// Is the current property hidden by the previously cached property
// (with `new` keyword, or by overriding)?
other.MemberName == memberName ||
// Was a property with the same CLR name ignored? That property hid the current property,
// thus, if it was ignored, the current property should be ignored too.
ignoredMembers?.ContainsKey(memberName) == true;
}
else
{
// Unlike classes, interface hierarchies reject all naming conflicts for non-ignored properties.
// Conflicts like this are possible in two cases:
// 1. Diamond ambiguity in property names, or
// 2. Linear interface hierarchies that use properties with DIMs.
//
// Diamond ambiguities are not supported. Assuming there is demand, we might consider
// adding support for DIMs in the future, however that would require adding more APIs
// for the case of source gen.

ignoreCurrentProperty = jsonPropertyInfo.IsIgnored;
}

if (!ignoreCurrentProperty)
{
ThrowHelper.ThrowInvalidOperationException_SerializerPropertyNameConflict(Type, jsonPropertyInfo.Name);
}
}
// Ignore the current property.
}

if (jsonPropertyInfo.IsIgnored)
{
(ignoredMembers ??= new Dictionary<string, JsonPropertyInfo>()).Add(memberName, jsonPropertyInfo);
(ignoredMembers ??= new()).Add(memberName, jsonPropertyInfo);
}
}

Expand Down
Loading

0 comments on commit 58f59db

Please sign in to comment.