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

Add support for serializing properties in interface hierarchies #78788

Merged
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
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