Skip to content

Commit 9b63001

Browse files
krwqkrwq
authored
Make JSON support required properties (#72937)
* Throw exception when 'required' keyword used but ctor does not have 'SetsRequiredMembers' * Support required keyword (and internal IsRequired) * remove local variable (forgot to press Save All) * revert added namespaces in two files * Apply feedback * apply second round of feedback (minus HashSet optimization) * Use BitArray rather than HashSet * simplify validation code * Truncate the message if too long, flip meaning of bit array bits * remove default value, add description for BitArray * for compiler attributes check for full type name rather than using typeof on net7.0
1 parent b6115b0 commit 9b63001

17 files changed

+807
-25
lines changed

‎src/libraries/System.Text.Json/src/Resources/Strings.resx

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -659,4 +659,13 @@
659659
<data name="JsonPolymorphismOptionsAssociatedWithDifferentJsonTypeInfo" xml:space="preserve">
660660
<value>Parameter already associated with a different JsonTypeInfo instance.</value>
661661
</data>
662+
<data name="JsonPropertyRequiredAndNotDeserializable" xml:space="preserve">
663+
<value>JsonPropertyInfo '{0}' defined in type '{1}' is marked required but does not specify a setter.</value>
664+
</data>
665+
<data name="JsonPropertyRequiredAndExtensionData" xml:space="preserve">
666+
<value>JsonPropertyInfo '{0}' defined in type '{1}' is marked both as required and as an extension data property. This combination is not supported.</value>
667+
</data>
668+
<data name="JsonRequiredPropertiesMissing" xml:space="preserve">
669+
<value>JSON deserialization for type '{0}' was missing required properties, including the following: {1}</value>
670+
</data>
662671
</root>

‎src/libraries/System.Text.Json/src/System/ReflectionExtensions.cs

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// Licensed to the .NET Foundation under one or more agreements.
22
// The .NET Foundation licenses this file to you under the MIT license.
33

4+
using System.Diagnostics.CodeAnalysis;
45
using System.Reflection;
56
using System.Runtime.CompilerServices;
67
using System.Runtime.ExceptionServices;
@@ -42,6 +43,32 @@ public static bool IsInSubtypeRelationshipWith(this Type type, Type other) =>
4243
private static bool HasJsonConstructorAttribute(ConstructorInfo constructorInfo)
4344
=> constructorInfo.GetCustomAttribute<JsonConstructorAttribute>() != null;
4445

46+
public static bool HasRequiredMemberAttribute(this ICustomAttributeProvider memberInfo)
47+
{
48+
// For compiler related attributes we should only look at full type name rather than trying to do something different for version when attribute was introduced.
49+
// I.e. library is targetting netstandard2.0 with polyfilled attributes and is being consumed by app targetting net7.0.
50+
return memberInfo.HasCustomAttributeWithName("System.Runtime.CompilerServices.RequiredMemberAttribute", inherit: true);
51+
}
52+
53+
public static bool HasSetsRequiredMembersAttribute(this ICustomAttributeProvider memberInfo)
54+
{
55+
// See comment for HasRequiredMemberAttribute for why we need to always only look at full name
56+
return memberInfo.HasCustomAttributeWithName("System.Diagnostics.CodeAnalysis.SetsRequiredMembersAttribute", inherit: true);
57+
}
58+
59+
private static bool HasCustomAttributeWithName(this ICustomAttributeProvider memberInfo, string fullName, bool inherit)
60+
{
61+
foreach (object attribute in memberInfo.GetCustomAttributes(inherit))
62+
{
63+
if (attribute.GetType().FullName == fullName)
64+
{
65+
return true;
66+
}
67+
}
68+
69+
return false;
70+
}
71+
4572
public static TAttribute? GetUniqueCustomAttribute<TAttribute>(this MemberInfo memberInfo, bool inherit)
4673
where TAttribute : Attribute
4774
{

‎src/libraries/System.Text.Json/src/System/Text/Json/JsonHelpers.cs

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// The .NET Foundation licenses this file to you under the MIT license.
33

44
using System.Buffers;
5+
using System.Collections;
56
using System.Collections.Generic;
67
using System.Diagnostics;
78
using System.Runtime.CompilerServices;
@@ -147,5 +148,19 @@ public static void ValidateInt32MaxArrayLength(uint length)
147148
ThrowHelper.ThrowOutOfMemoryException(length);
148149
}
149150
}
151+
152+
public static bool AllBitsEqual(this BitArray bitArray, bool value)
153+
{
154+
// Optimize this when https://github.com/dotnet/runtime/issues/72999 is fixed
155+
for (int i = 0; i < bitArray.Count; i++)
156+
{
157+
if (bitArray[i] != value)
158+
{
159+
return false;
160+
}
161+
}
162+
163+
return true;
164+
}
150165
}
151166
}

‎src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectDefaultConverter.cs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ internal override bool OnTryRead(ref Utf8JsonReader reader, Type typeToConvert,
3939
obj = jsonTypeInfo.CreateObject()!;
4040

4141
jsonTypeInfo.OnDeserializing?.Invoke(obj);
42+
state.Current.InitializeRequiredPropertiesValidationState(jsonTypeInfo);
4243

4344
// Process all properties.
4445
while (true)
@@ -143,6 +144,7 @@ internal override bool OnTryRead(ref Utf8JsonReader reader, Type typeToConvert,
143144

144145
state.Current.ReturnValue = obj;
145146
state.Current.ObjectState = StackFrameObjectState.CreatedObject;
147+
state.Current.InitializeRequiredPropertiesValidationState(jsonTypeInfo);
146148
}
147149
else
148150
{
@@ -250,6 +252,7 @@ internal override bool OnTryRead(ref Utf8JsonReader reader, Type typeToConvert,
250252
}
251253

252254
jsonTypeInfo.OnDeserialized?.Invoke(obj);
255+
state.Current.ValidateAllRequiredPropertiesAreRead(jsonTypeInfo);
253256

254257
// Unbox
255258
Debug.Assert(obj != null);

‎src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectWithParameterizedConstructorConverter.Large.cs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,9 @@ protected sealed override bool ReadAndCacheConstructorArgument(ref ReadStack sta
2424
if (success && !(arg == null && jsonParameterInfo.IgnoreNullTokensOnRead))
2525
{
2626
((object[])state.Current.CtorArgumentState!.Arguments)[jsonParameterInfo.ClrInfo.Position] = arg!;
27+
28+
// if this is required property IgnoreNullTokensOnRead will always be false because we don't allow for both to be true
29+
state.Current.MarkRequiredPropertyAsRead(jsonParameterInfo.MatchingProperty);
2730
}
2831

2932
return success;

‎src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectWithParameterizedConstructorConverter.Small.cs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,11 @@ private static bool TryRead<TArg>(
7171
? (TArg?)info.DefaultValue! // Use default value specified on parameter, if any.
7272
: value!;
7373

74+
if (success)
75+
{
76+
state.Current.MarkRequiredPropertyAsRead(jsonParameterInfo.MatchingProperty);
77+
}
78+
7479
return success;
7580
}
7681

‎src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectWithParameterizedConstructorConverter.cs

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,15 @@ internal sealed override bool OnTryRead(ref Utf8JsonReader reader, Type typeToCo
184184

185185
if (dataExtKey == null)
186186
{
187-
jsonPropertyInfo.SetExtensionDictionaryAsObject(obj, propValue);
187+
Debug.Assert(jsonPropertyInfo.Set != null);
188+
189+
if (propValue is not null || !jsonPropertyInfo.IgnoreNullTokensOnRead || default(T) is not null)
190+
{
191+
jsonPropertyInfo.Set(obj, propValue);
192+
193+
// if this is required property IgnoreNullTokensOnRead will always be false because we don't allow for both to be true
194+
state.Current.MarkRequiredPropertyAsRead(jsonPropertyInfo);
195+
}
188196
}
189197
else
190198
{
@@ -211,6 +219,7 @@ internal sealed override bool OnTryRead(ref Utf8JsonReader reader, Type typeToCo
211219
}
212220

213221
jsonTypeInfo.OnDeserialized?.Invoke(obj);
222+
state.Current.ValidateAllRequiredPropertiesAreRead(jsonTypeInfo);
214223

215224
// Unbox
216225
Debug.Assert(obj != null);
@@ -272,6 +281,7 @@ private void ReadConstructorArguments(ref ReadStack state, ref Utf8JsonReader re
272281
continue;
273282
}
274283

284+
Debug.Assert(jsonParameterInfo.MatchingProperty != null);
275285
ReadAndCacheConstructorArgument(ref state, ref reader, jsonParameterInfo);
276286

277287
state.Current.EndConstructorParameter();
@@ -532,6 +542,8 @@ private void BeginRead(ref ReadStack state, ref Utf8JsonReader reader, JsonSeria
532542
ThrowHelper.ThrowInvalidOperationException_ConstructorParameterIncompleteBinding(TypeToConvert);
533543
}
534544

545+
state.Current.InitializeRequiredPropertiesValidationState(jsonTypeInfo);
546+
535547
// Set current JsonPropertyInfo to null to avoid conflicts on push.
536548
state.Current.JsonPropertyInfo = null;
537549

‎src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.HandlePropertyName.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,8 @@ internal static void CreateExtensionDataProperty(
139139
}
140140

141141
extensionData = createObjectForExtensionDataProp();
142-
jsonPropertyInfo.SetExtensionDictionaryAsObject(obj, extensionData);
142+
Debug.Assert(jsonPropertyInfo.Set != null);
143+
jsonPropertyInfo.Set(obj, extensionData);
143144
}
144145

145146
// We don't add the value to the dictionary here because we need to support the read-ahead functionality for Streams.

‎src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonParameterInfo.cs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,11 @@ public JsonTypeInfo JsonTypeInfo
5353

5454
public bool ShouldDeserialize { get; private set; }
5555

56+
public JsonPropertyInfo MatchingProperty { get; private set; } = null!;
57+
5658
public virtual void Initialize(JsonParameterInfoValues parameterInfo, JsonPropertyInfo matchingProperty, JsonSerializerOptions options)
5759
{
60+
MatchingProperty = matchingProperty;
5861
ClrInfo = parameterInfo;
5962
Options = options;
6063
ShouldDeserialize = true;

‎src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPropertyInfo.cs

Lines changed: 61 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,18 @@ public bool IsExtensionData
203203

204204
private bool _isExtensionDataProperty;
205205

206+
internal bool IsRequired
207+
{
208+
get => _isRequired;
209+
set
210+
{
211+
VerifyMutable();
212+
_isRequired = value;
213+
}
214+
}
215+
216+
private bool _isRequired;
217+
206218
internal JsonPropertyInfo(Type declaringType, Type propertyType, JsonTypeInfo? declaringTypeInfo, JsonSerializerOptions options)
207219
{
208220
Debug.Assert(declaringTypeInfo is null || declaringTypeInfo.Type == declaringType);
@@ -279,6 +291,21 @@ internal void Configure()
279291
DetermineIgnoreCondition();
280292
DetermineSerializationCapabilities();
281293
}
294+
295+
if (IsRequired)
296+
{
297+
if (!CanDeserialize)
298+
{
299+
ThrowHelper.ThrowInvalidOperationException_JsonPropertyRequiredAndNotDeserializable(this);
300+
}
301+
302+
if (IsExtensionData)
303+
{
304+
ThrowHelper.ThrowInvalidOperationException_JsonPropertyRequiredAndExtensionData(this);
305+
}
306+
307+
Debug.Assert(!IgnoreNullTokensOnRead);
308+
}
282309
}
283310

284311
private protected abstract void DetermineEffectiveConverter(JsonTypeInfo jsonTypeInfo);
@@ -341,7 +368,7 @@ private void DetermineIgnoreCondition()
341368
Debug.Assert(Options.DefaultIgnoreCondition == JsonIgnoreCondition.Never);
342369
if (PropertyTypeCanBeNull)
343370
{
344-
IgnoreNullTokensOnRead = !_isUserSpecifiedSetter;
371+
IgnoreNullTokensOnRead = !_isUserSpecifiedSetter && !IsRequired;
345372
IgnoreDefaultValuesOnWrite = ShouldSerialize is null;
346373
}
347374
}
@@ -477,6 +504,14 @@ private bool NumberHandingIsApplicable()
477504
potentialNumberType == JsonTypeInfo.ObjectType;
478505
}
479506

507+
private void DetermineIsRequired(MemberInfo memberInfo, bool shouldCheckForRequiredKeyword)
508+
{
509+
if (shouldCheckForRequiredKeyword && memberInfo.HasRequiredMemberAttribute())
510+
{
511+
IsRequired = true;
512+
}
513+
}
514+
480515
internal abstract bool GetMemberAndWriteJson(object obj, ref WriteStack state, Utf8JsonWriter writer);
481516
internal abstract bool GetMemberAndWriteJsonExtensionData(object obj, ref WriteStack state, Utf8JsonWriter writer);
482517

@@ -504,7 +539,7 @@ internal string GetDebugInfo(int indent = 0)
504539
internal bool HasGetter => _untypedGet is not null;
505540
internal bool HasSetter => _untypedSet is not null;
506541

507-
internal void InitializeUsingMemberReflection(MemberInfo memberInfo, JsonConverter? customConverter, JsonIgnoreCondition? ignoreCondition)
542+
internal void InitializeUsingMemberReflection(MemberInfo memberInfo, JsonConverter? customConverter, JsonIgnoreCondition? ignoreCondition, bool shouldCheckForRequiredKeyword)
508543
{
509544
Debug.Assert(AttributeProvider == null);
510545

@@ -531,6 +566,7 @@ internal void InitializeUsingMemberReflection(MemberInfo memberInfo, JsonConvert
531566
CustomConverter = customConverter;
532567
DeterminePoliciesFromMember(memberInfo);
533568
DeterminePropertyNameFromMember(memberInfo);
569+
DetermineIsRequired(memberInfo, shouldCheckForRequiredKeyword);
534570

535571
if (ignoreCondition != JsonIgnoreCondition.Always)
536572
{
@@ -760,8 +796,6 @@ internal JsonTypeInfo JsonTypeInfo
760796
}
761797
}
762798

763-
internal abstract void SetExtensionDictionaryAsObject(object obj, object? extensionDict);
764-
765799
internal bool IsIgnored => _ignoreCondition == JsonIgnoreCondition.Always;
766800

767801
/// <summary>
@@ -823,6 +857,29 @@ public JsonNumberHandling? NumberHandling
823857
/// </summary>
824858
internal abstract object? DefaultValue { get; }
825859

860+
/// <summary>
861+
/// Required property index on the list of JsonTypeInfo properties.
862+
/// It is used as a unique identifier for required properties.
863+
/// It is set just before property is configured and does not change afterward.
864+
/// It is not equivalent to index on the properties list
865+
/// </summary>
866+
internal int RequiredPropertyIndex
867+
{
868+
get
869+
{
870+
Debug.Assert(_isConfigured);
871+
Debug.Assert(IsRequired);
872+
return _index;
873+
}
874+
set
875+
{
876+
Debug.Assert(!_isConfigured);
877+
_index = value;
878+
}
879+
}
880+
881+
private int _index;
882+
826883
[DebuggerBrowsable(DebuggerBrowsableState.Never)]
827884
private string DebuggerDisplay => $"PropertyType = {PropertyType}, Name = {Name}, DeclaringType = {DeclaringType}";
828885
}

‎src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPropertyInfoOfT.cs

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -342,6 +342,7 @@ internal override bool ReadJsonAndSetMember(object obj, ref ReadStack state, ref
342342
}
343343

344344
success = true;
345+
state.Current.MarkRequiredPropertyAsRead(this);
345346
}
346347
else if (TypedEffectiveConverter.CanUseDirectReadOrWrite && state.Current.NumberHandling == null)
347348
{
@@ -356,6 +357,7 @@ internal override bool ReadJsonAndSetMember(object obj, ref ReadStack state, ref
356357
}
357358

358359
success = true;
360+
state.Current.MarkRequiredPropertyAsRead(this);
359361
}
360362
else
361363
{
@@ -366,6 +368,7 @@ internal override bool ReadJsonAndSetMember(object obj, ref ReadStack state, ref
366368
if (success)
367369
{
368370
Set!(obj, value!);
371+
state.Current.MarkRequiredPropertyAsRead(this);
369372
}
370373
}
371374
}
@@ -408,13 +411,6 @@ internal override bool ReadJsonAsObject(ref ReadStack state, ref Utf8JsonReader
408411
return success;
409412
}
410413

411-
internal override void SetExtensionDictionaryAsObject(object obj, object? extensionDict)
412-
{
413-
Debug.Assert(HasSetter);
414-
T typedValue = (T)extensionDict!;
415-
Set!(obj, typedValue);
416-
}
417-
418414
private protected override void ConfigureIgnoreCondition(JsonIgnoreCondition? ignoreCondition)
419415
{
420416
switch (ignoreCondition)

‎src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.cs

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,11 @@ public abstract partial class JsonTypeInfo
2626

2727
private JsonPropertyInfoList? _properties;
2828

29+
/// <summary>
30+
/// Indices of required properties.
31+
/// </summary>
32+
internal int NumberOfRequiredProperties { get; private set; }
33+
2934
private Action<object>? _onSerializing;
3035
private Action<object>? _onSerialized;
3136
private Action<object>? _onDeserializing;
@@ -878,13 +883,21 @@ internal void InitializePropertyCache()
878883
ExtensionDataProperty.EnsureConfigured();
879884
}
880885

886+
int numberOfRequiredProperties = 0;
881887
foreach (KeyValuePair<string, JsonPropertyInfo> jsonPropertyInfoKv in PropertyCache.List)
882888
{
883889
JsonPropertyInfo jsonPropertyInfo = jsonPropertyInfoKv.Value;
884890

891+
if (jsonPropertyInfo.IsRequired)
892+
{
893+
jsonPropertyInfo.RequiredPropertyIndex = numberOfRequiredProperties++;
894+
}
895+
885896
jsonPropertyInfo.EnsureChildOf(this);
886897
jsonPropertyInfo.EnsureConfigured();
887898
}
899+
900+
NumberOfRequiredProperties = numberOfRequiredProperties;
888901
}
889902

890903
internal void InitializeConstructorParameters(JsonParameterInfoValues[] jsonParameters, bool sourceGenMode = false)

0 commit comments

Comments
 (0)