Skip to content
Open
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
12 changes: 12 additions & 0 deletions src/libraries/System.Text.Json/gen/Helpers/KnownTypeSymbols.cs
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,18 @@ public KnownTypeSymbols(Compilation compilation)
public INamedTypeSymbol? JsonElementType => GetOrResolveType("System.Text.Json.JsonElement", ref _JsonElementType);
private Option<INamedTypeSymbol?> _JsonElementType;

public INamedTypeSymbol? StringObjectDictionaryType => _StringObjectDictionaryType.HasValue
? _StringObjectDictionaryType.Value
: (_StringObjectDictionaryType = new(DictionaryOfTKeyTValueType?.Construct(StringType, ObjectType))).Value;
private Option<INamedTypeSymbol?> _StringObjectDictionaryType;

public INamedTypeSymbol? StringJsonElementDictionaryType => _StringJsonElementDictionaryType.HasValue
? _StringJsonElementDictionaryType.Value
: (_StringJsonElementDictionaryType = new(DictionaryOfTKeyTValueType is { } dictType && JsonElementType is { } jsonElemType
? dictType.Construct(StringType, jsonElemType)
: null)).Value;
private Option<INamedTypeSymbol?> _StringJsonElementDictionaryType;

public INamedTypeSymbol? JsonNodeType => GetOrResolveType("System.Text.Json.Nodes.JsonNode", ref _JsonNodeType);
private Option<INamedTypeSymbol?> _JsonNodeType;

Expand Down
35 changes: 30 additions & 5 deletions src/libraries/System.Text.Json/gen/JsonSourceGenerator.Parser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1105,14 +1105,39 @@ private bool IsValidDataExtensionPropertyType(ITypeSymbol type)
}

INamedTypeSymbol? actualDictionaryType = type.GetCompatibleGenericBaseType(_knownSymbols.IDictionaryOfTKeyTValueType);
if (actualDictionaryType == null)
if (actualDictionaryType != null)
{
return false;
if (SymbolEqualityComparer.Default.Equals(actualDictionaryType.TypeArguments[0], _knownSymbols.StringType) &&
(SymbolEqualityComparer.Default.Equals(actualDictionaryType.TypeArguments[1], _knownSymbols.ObjectType) ||
SymbolEqualityComparer.Default.Equals(actualDictionaryType.TypeArguments[1], _knownSymbols.JsonElementType)))
{
return true;
}
}

return SymbolEqualityComparer.Default.Equals(actualDictionaryType.TypeArguments[0], _knownSymbols.StringType) &&
(SymbolEqualityComparer.Default.Equals(actualDictionaryType.TypeArguments[1], _knownSymbols.ObjectType) ||
SymbolEqualityComparer.Default.Equals(actualDictionaryType.TypeArguments[1], _knownSymbols.JsonElementType));
// Also check for IReadOnlyDictionary<string, object> or IReadOnlyDictionary<string, JsonElement>
// but only if Dictionary can be assigned to it (to exclude ImmutableDictionary and similar types)
Comment on lines +1118 to +1119
Copy link
Member

Choose a reason for hiding this comment

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

Practically, this means we're extending support for only two types: IReadOnlyDictionary<> and Dictionary<> (which we already support). So we only need to check for an exact match of the former.

Copy link
Member

@eiriktsarpalis eiriktsarpalis Oct 13, 2025

Choose a reason for hiding this comment

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

More importantly though, the premise of the JsonExtensionData feature is appending unbound data to mutable dictionary like types. Assuming we wanted to extend this to read-only types, we want to additionally check if the property has a setter. Today we're only checking on the basis of the property type, so this would need to be refactored quite a bit.

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

@stephentoub stephentoub Oct 15, 2025

Choose a reason for hiding this comment

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

More importantly though, the premise of the JsonExtensionData feature is appending unbound data to mutable dictionary like types.

Why? JsonSerializer already deserializes read-only collection interfaces by instantiating a mutable concrete instance, populating it, and storing that as the read-only collection... why would JsonExtensionData be any different?

Even with a mutable dictionary, it doesn't appear today to work to have a [JsonExtensionsData] property not have a setter, even if Populate is used. The data doesn't get added.

INamedTypeSymbol? actualReadOnlyDictionaryType = type.GetCompatibleGenericBaseType(_knownSymbols.IReadonlyDictionaryOfTKeyTValueType);
if (actualReadOnlyDictionaryType != null)
{
if (SymbolEqualityComparer.Default.Equals(actualReadOnlyDictionaryType.TypeArguments[0], _knownSymbols.StringType) &&
(SymbolEqualityComparer.Default.Equals(actualReadOnlyDictionaryType.TypeArguments[1], _knownSymbols.ObjectType) ||
SymbolEqualityComparer.Default.Equals(actualReadOnlyDictionaryType.TypeArguments[1], _knownSymbols.JsonElementType)))
{
// Check if Dictionary can be assigned to this type
INamedTypeSymbol? dictionaryType = SymbolEqualityComparer.Default.Equals(actualReadOnlyDictionaryType.TypeArguments[1], _knownSymbols.ObjectType)
? _knownSymbols.StringObjectDictionaryType
: _knownSymbols.StringJsonElementDictionaryType;

if (dictionaryType != null)
{
Conversion conversion = _knownSymbols.Compilation.ClassifyConversion(dictionaryType, type);
return conversion.IsImplicit || conversion.IsIdentity;
}
}
}

return false;
}

private PropertyGenerationSpec? ParsePropertyGenerationSpec(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,14 @@
namespace System.Text.Json.Serialization
{
/// <summary>
/// When placed on a property or field of type <see cref="System.Text.Json.Nodes.JsonObject"/> or
/// <see cref="System.Collections.Generic.IDictionary{TKey, TValue}"/>, any properties that do not have a
/// When placed on a property or field of type <see cref="System.Text.Json.Nodes.JsonObject"/>,
/// <see cref="System.Collections.Generic.IDictionary{TKey, TValue}"/>, or
/// <see cref="System.Collections.Generic.IReadOnlyDictionary{TKey, TValue}"/>, any properties that do not have a
/// matching property or field are added during deserialization and written during serialization.
/// </summary>
/// <remarks>
/// When using <see cref="System.Collections.Generic.IDictionary{TKey, TValue}"/>, the TKey value must be <see cref="string"/>
/// When using <see cref="System.Collections.Generic.IDictionary{TKey, TValue}"/> or
/// <see cref="System.Collections.Generic.IReadOnlyDictionary{TKey, TValue}"/>, the TKey value must be <see cref="string"/>
/// and TValue must be <see cref="JsonElement"/> or <see cref="object"/>.
///
/// During deserializing with a <see cref="System.Collections.Generic.IDictionary{TKey, TValue}"/> extension property with TValue as
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,9 @@ internal static void CreateExtensionDataProperty(
{
// Create the appropriate dictionary type. We already verified the types.
#if DEBUG
Type underlyingIDictionaryType = jsonPropertyInfo.PropertyType.GetCompatibleGenericInterface(typeof(IDictionary<,>))!;
Type? underlyingIDictionaryType = jsonPropertyInfo.PropertyType.GetCompatibleGenericInterface(typeof(IDictionary<,>))
?? jsonPropertyInfo.PropertyType.GetCompatibleGenericInterface(typeof(IReadOnlyDictionary<,>));
Debug.Assert(underlyingIDictionaryType is not null);
Type[] genericArgs = underlyingIDictionaryType.GetGenericArguments();

Debug.Assert(underlyingIDictionaryType.IsGenericType);
Expand All @@ -136,6 +138,25 @@ internal static void CreateExtensionDataProperty(
{
ThrowHelper.ThrowInvalidOperationException_NodeJsonObjectCustomConverterNotAllowedOnExtensionProperty();
}
// For IReadOnlyDictionary<string, object> or IReadOnlyDictionary<string, JsonElement> interface types,
// create a Dictionary<TKey, TValue> instance. We only do this if Dictionary can be assigned back
// to the property (i.e., the property is the interface type itself, not a concrete implementation).
else if (typeof(IReadOnlyDictionary<string, object>).IsAssignableFrom(jsonPropertyInfo.PropertyType) &&
jsonPropertyInfo.PropertyType.IsAssignableFrom(typeof(Dictionary<string, object>)))
{
extensionData = new Dictionary<string, object>();
Debug.Assert(jsonPropertyInfo.Set != null);
jsonPropertyInfo.Set(obj, extensionData);
return;
}
else if (typeof(IReadOnlyDictionary<string, JsonElement>).IsAssignableFrom(jsonPropertyInfo.PropertyType) &&
jsonPropertyInfo.PropertyType.IsAssignableFrom(typeof(Dictionary<string, JsonElement>)))
{
extensionData = new Dictionary<string, JsonElement>();
Debug.Assert(jsonPropertyInfo.Set != null);
jsonPropertyInfo.Set(obj, extensionData);
return;
}
else
{
ThrowHelper.ThrowNotSupportedException_SerializationNotSupported(jsonPropertyInfo.PropertyType);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1326,6 +1326,8 @@ internal static bool IsValidExtensionDataProperty(Type propertyType)
{
return typeof(IDictionary<string, object>).IsAssignableFrom(propertyType) ||
typeof(IDictionary<string, JsonElement>).IsAssignableFrom(propertyType) ||
propertyType == typeof(IReadOnlyDictionary<string, object>) ||
propertyType == typeof(IReadOnlyDictionary<string, JsonElement>) ||
// Avoid a reference to typeof(JsonNode) to support trimming.
(propertyType.FullName == JsonObjectTypeName && ReferenceEquals(propertyType.Assembly, typeof(JsonTypeInfo).Assembly));
}
Expand Down
69 changes: 69 additions & 0 deletions src/libraries/System.Text.Json/tests/Common/ExtensionDataTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1483,5 +1483,74 @@ public class ClassWithEmptyPropertyNameAndExtensionProperty
[JsonExtensionData]
public IDictionary<string, JsonElement> MyOverflow { get; set; }
}

[Fact]
public async Task IReadOnlyDictionary_ObjectExtensionPropertyRoundTrip()
{
string json = @"{""MyIntMissing"":2, ""MyInt"":1}";
ClassWithIReadOnlyDictionaryExtensionPropertyAsObjectWithProperty obj = await Serializer.DeserializeWrapper<ClassWithIReadOnlyDictionaryExtensionPropertyAsObjectWithProperty>(json);

Assert.NotNull(obj.MyOverflow);
Assert.Equal(1, obj.MyInt);
Assert.IsType<JsonElement>(obj.MyOverflow["MyIntMissing"]);
Assert.Equal(2, ((JsonElement)obj.MyOverflow["MyIntMissing"]).GetInt32());

string jsonSerialized = await Serializer.SerializeWrapper(obj);
Assert.Contains("\"MyIntMissing\"", jsonSerialized);
Assert.Contains("\"MyInt\"", jsonSerialized);
Assert.DoesNotContain(nameof(ClassWithIReadOnlyDictionaryExtensionPropertyAsObjectWithProperty.MyOverflow), jsonSerialized);
}

[Fact]
public async Task IReadOnlyDictionary_JsonElementExtensionPropertyRoundTrip()
{
string json = @"{""MyIntMissing"":2, ""MyInt"":1}";
ClassWithIReadOnlyDictionaryExtensionPropertyAsJsonElementWithProperty obj = await Serializer.DeserializeWrapper<ClassWithIReadOnlyDictionaryExtensionPropertyAsJsonElementWithProperty>(json);

Assert.NotNull(obj.MyOverflow);
Assert.Equal(1, obj.MyInt);
Assert.Equal(2, obj.MyOverflow["MyIntMissing"].GetInt32());

string jsonSerialized = await Serializer.SerializeWrapper(obj);
Assert.Contains("\"MyIntMissing\"", jsonSerialized);
Assert.Contains("\"MyInt\"", jsonSerialized);
Assert.DoesNotContain(nameof(ClassWithIReadOnlyDictionaryExtensionPropertyAsJsonElementWithProperty.MyOverflow), jsonSerialized);
}

[Fact]
public async Task IReadOnlyDictionary_ExtensionPropertyIgnoredWhenWritingDefault()
{
string expected = @"{}";
string actual = await Serializer.SerializeWrapper(new ClassWithIReadOnlyDictionaryExtensionPropertyAsObject());
Assert.Equal(expected, actual);
}

public class ClassWithIReadOnlyDictionaryExtensionPropertyAsObject
{
[JsonExtensionData]
public IReadOnlyDictionary<string, object> MyOverflow { get; set; }
}

public class ClassWithIReadOnlyDictionaryExtensionPropertyAsJsonElement
{
[JsonExtensionData]
public IReadOnlyDictionary<string, JsonElement> MyOverflow { get; set; }
}

public class ClassWithIReadOnlyDictionaryExtensionPropertyAsObjectWithProperty
{
public int MyInt { get; set; }

[JsonExtensionData]
public IReadOnlyDictionary<string, object> MyOverflow { get; set; }
}

public class ClassWithIReadOnlyDictionaryExtensionPropertyAsJsonElementWithProperty
{
public int MyInt { get; set; }

[JsonExtensionData]
public IReadOnlyDictionary<string, JsonElement> MyOverflow { get; set; }
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,10 @@ public ExtensionDataTests_Metadata()
[JsonSerializable(typeof(int))]
[JsonSerializable(typeof(DummyObj))]
[JsonSerializable(typeof(DummyStruct))]
[JsonSerializable(typeof(ClassWithIReadOnlyDictionaryExtensionPropertyAsObject))]
[JsonSerializable(typeof(ClassWithIReadOnlyDictionaryExtensionPropertyAsJsonElement))]
[JsonSerializable(typeof(ClassWithIReadOnlyDictionaryExtensionPropertyAsObjectWithProperty))]
[JsonSerializable(typeof(ClassWithIReadOnlyDictionaryExtensionPropertyAsJsonElementWithProperty))]
internal sealed partial class ExtensionDataTestsContext_Metadata : JsonSerializerContext
{
}
Expand Down Expand Up @@ -132,6 +136,10 @@ public ExtensionDataTests_Default()
[JsonSerializable(typeof(int))]
[JsonSerializable(typeof(DummyObj))]
[JsonSerializable(typeof(DummyStruct))]
[JsonSerializable(typeof(ClassWithIReadOnlyDictionaryExtensionPropertyAsObject))]
[JsonSerializable(typeof(ClassWithIReadOnlyDictionaryExtensionPropertyAsJsonElement))]
[JsonSerializable(typeof(ClassWithIReadOnlyDictionaryExtensionPropertyAsObjectWithProperty))]
[JsonSerializable(typeof(ClassWithIReadOnlyDictionaryExtensionPropertyAsJsonElementWithProperty))]
internal sealed partial class ExtensionDataTestsContext_Default : JsonSerializerContext
{
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1285,6 +1285,8 @@ public static void ClassWithExtensionDataAttribute_RemovingExtensionDataProperty
[Theory]
[InlineData(typeof(IDictionary<string, object>))]
[InlineData(typeof(IDictionary<string, JsonElement>))]
[InlineData(typeof(IReadOnlyDictionary<string, object>))]
[InlineData(typeof(IReadOnlyDictionary<string, JsonElement>))]
[InlineData(typeof(Dictionary<string, object>))]
[InlineData(typeof(Dictionary<string, JsonElement>))]
[InlineData(typeof(ConcurrentDictionary<string, JsonElement>))]
Expand Down
Loading