Skip to content

Commit 455f540

Browse files
[STJ] Add support for nullable reference annotations on properties (#102499)
* Progress so far on nullability annotations * Complete implementation and make all NullableAnnotations tests pass. * Update annotations for all failing unit tests. * Update src/libraries/System.Text.Json/gen/Helpers/RoslynExtensions.cs * Update src/libraries/System.Text.Json/gen/Helpers/RoslynExtensions.cs * Address feedback * Update to latest approved API and semantics. * Update src/libraries/System.Private.CoreLib/src/System/Reflection/NullabilityInfoContext.cs Co-authored-by: David Cantú <[email protected]> * Update src/libraries/System.Private.CoreLib/src/System/Reflection/NullabilityInfoContext.cs Co-authored-by: David Cantú <[email protected]> * Update src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs Co-authored-by: David Cantú <[email protected]> * Rename more ignoreNullableAnnotations stragglers. * Update src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.cs Co-authored-by: David Cantú <[email protected]> * Remove commented out code and address feedback. * Update src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/System.Text.Json.SourceGeneration.Tests.targets Co-authored-by: David Cantú <[email protected]> * Ensure the original parameter name flows exception messages. * Extract exceptions to a throw helper in the new properties. * Extend test coverage to Nullable<T> properties. * Revert sln changes * Add second-pass review improvements. --------- Co-authored-by: David Cantú <[email protected]>
1 parent 5dc8a1a commit 455f540

File tree

88 files changed

+2374
-889
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

88 files changed

+2374
-889
lines changed

src/libraries/System.Private.CoreLib/src/System/Reflection/NullabilityInfo.cs

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,12 @@ namespace System.Reflection
88
/// <summary>
99
/// A class that represents nullability info
1010
/// </summary>
11-
public sealed class NullabilityInfo
11+
#if NET
12+
public
13+
#else
14+
internal
15+
#endif
16+
sealed class NullabilityInfo
1217
{
1318
internal NullabilityInfo(Type type, NullabilityState readState, NullabilityState writeState,
1419
NullabilityInfo? elementType, NullabilityInfo[] typeArguments)
@@ -46,7 +51,12 @@ internal NullabilityInfo(Type type, NullabilityState readState, NullabilityState
4651
/// <summary>
4752
/// An enum that represents nullability state
4853
/// </summary>
49-
public enum NullabilityState
54+
#if NET
55+
public
56+
#else
57+
internal
58+
#endif
59+
enum NullabilityState
5060
{
5161
/// <summary>
5262
/// Nullability context not enabled (oblivious)

src/libraries/System.Private.CoreLib/src/System/Reflection/NullabilityInfoContext.cs

Lines changed: 85 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,12 @@ namespace System.Reflection
1111
/// Provides APIs for populating nullability information/context from reflection members:
1212
/// <see cref="ParameterInfo"/>, <see cref="FieldInfo"/>, <see cref="PropertyInfo"/> and <see cref="EventInfo"/>.
1313
/// </summary>
14-
public sealed class NullabilityInfoContext
14+
#if NET
15+
public
16+
#else
17+
internal
18+
#endif
19+
sealed class NullabilityInfoContext
1520
{
1621
private const string CompilerServicesNameSpace = "System.Runtime.CompilerServices";
1722
private readonly Dictionary<Module, NotAnnotatedStatus> _publicOnlyModules = new();
@@ -65,7 +70,11 @@ private enum NotAnnotatedStatus
6570
/// <returns><see cref="NullabilityInfo" /></returns>
6671
public NullabilityInfo Create(ParameterInfo parameterInfo)
6772
{
73+
#if NET
6874
ArgumentNullException.ThrowIfNull(parameterInfo);
75+
#else
76+
NetstandardHelpers.ThrowIfNull(parameterInfo, nameof(parameterInfo));
77+
#endif
6978

7079
EnsureIsSupported();
7180

@@ -190,7 +199,11 @@ private static void CheckNullabilityAttributes(NullabilityInfo nullability, ILis
190199
/// <returns><see cref="NullabilityInfo" /></returns>
191200
public NullabilityInfo Create(PropertyInfo propertyInfo)
192201
{
202+
#if NET
193203
ArgumentNullException.ThrowIfNull(propertyInfo);
204+
#else
205+
NetstandardHelpers.ThrowIfNull(propertyInfo, nameof(propertyInfo));
206+
#endif
194207

195208
EnsureIsSupported();
196209

@@ -212,7 +225,9 @@ public NullabilityInfo Create(PropertyInfo propertyInfo)
212225

213226
if (setter != null)
214227
{
215-
CheckNullabilityAttributes(nullability, setter.GetParametersAsSpan()[^1].GetCustomAttributesData());
228+
ReadOnlySpan<ParameterInfo> parameters = setter.GetParametersAsSpan();
229+
ParameterInfo parameter = parameters[parameters.Length - 1];
230+
CheckNullabilityAttributes(nullability, parameter.GetCustomAttributesData());
216231
}
217232
else
218233
{
@@ -243,7 +258,11 @@ private bool IsPrivateOrInternalMethodAndAnnotationDisabled(MethodBase method)
243258
/// <returns><see cref="NullabilityInfo" /></returns>
244259
public NullabilityInfo Create(EventInfo eventInfo)
245260
{
261+
#if NET
246262
ArgumentNullException.ThrowIfNull(eventInfo);
263+
#else
264+
NetstandardHelpers.ThrowIfNull(eventInfo, nameof(eventInfo));
265+
#endif
247266

248267
EnsureIsSupported();
249268

@@ -260,7 +279,11 @@ public NullabilityInfo Create(EventInfo eventInfo)
260279
/// <returns><see cref="NullabilityInfo" /></returns>
261280
public NullabilityInfo Create(FieldInfo fieldInfo)
262281
{
282+
#if NET
263283
ArgumentNullException.ThrowIfNull(fieldInfo);
284+
#else
285+
NetstandardHelpers.ThrowIfNull(fieldInfo, nameof(fieldInfo));
286+
#endif
264287

265288
EnsureIsSupported();
266289

@@ -497,7 +520,11 @@ private bool TryUpdateGenericParameterNullability(NullabilityInfo nullability, T
497520
Debug.Assert(genericParameter.IsGenericParameter);
498521

499522
if (reflectedType is not null
523+
#if NET
500524
&& !genericParameter.IsGenericMethodParameter
525+
#else
526+
&& !genericParameter.IsGenericMethodParameter()
527+
#endif
501528
&& TryUpdateGenericTypeParameterNullabilityFromReflectedType(nullability, genericParameter, reflectedType, reflectedType))
502529
{
503530
return true;
@@ -528,7 +555,12 @@ private bool TryUpdateGenericParameterNullability(NullabilityInfo nullability, T
528555

529556
private bool TryUpdateGenericTypeParameterNullabilityFromReflectedType(NullabilityInfo nullability, Type genericParameter, Type context, Type reflectedType)
530557
{
531-
Debug.Assert(genericParameter.IsGenericParameter && !genericParameter.IsGenericMethodParameter);
558+
Debug.Assert(genericParameter.IsGenericParameter &&
559+
#if NET
560+
!genericParameter.IsGenericMethodParameter);
561+
#else
562+
!genericParameter.IsGenericMethodParameter());
563+
#endif
532564

533565
Type contextTypeDefinition = context.IsGenericType && !context.IsGenericTypeDefinition ? context.GetGenericTypeDefinition() : context;
534566
if (genericParameter.DeclaringType == contextTypeDefinition)
@@ -666,4 +698,54 @@ public bool ParseNullableState(int index, ref NullabilityState state)
666698
}
667699
}
668700
}
701+
702+
#if !NET
703+
internal static class NetstandardHelpers
704+
{
705+
public static void ThrowIfNull(object? argument, string paramName)
706+
{
707+
if (argument is null)
708+
{
709+
Throw(paramName);
710+
static void Throw(string paramName) => throw new ArgumentNullException(paramName);
711+
}
712+
}
713+
714+
[Diagnostics.CodeAnalysis.UnconditionalSuppressMessage("ReflectionAnalysis", "IL2070:UnrecognizedReflectionPattern",
715+
Justification = "This is finding the MemberInfo with the same MetadataToken as specified MemberInfo. If the specified MemberInfo " +
716+
"exists and wasn't trimmed, then the current Type's MemberInfo couldn't have been trimmed.")]
717+
public static MemberInfo GetMemberWithSameMetadataDefinitionAs(this Type type, MemberInfo member)
718+
{
719+
ThrowIfNull(member, nameof(member));
720+
721+
const BindingFlags all = BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Static | BindingFlags.Instance;
722+
foreach (MemberInfo myMemberInfo in type.GetMembers(all))
723+
{
724+
if (myMemberInfo.HasSameMetadataDefinitionAs(member))
725+
{
726+
return myMemberInfo;
727+
}
728+
}
729+
730+
throw new MissingMemberException(type.FullName, member.Name);
731+
}
732+
733+
private static bool HasSameMetadataDefinitionAs(this MemberInfo info, MemberInfo other)
734+
{
735+
if (info.MetadataToken != other.MetadataToken)
736+
return false;
737+
738+
if (!info.Module.Equals(other.Module))
739+
return false;
740+
741+
return true;
742+
}
743+
744+
public static bool IsGenericMethodParameter(this Type type)
745+
=> type.IsGenericParameter && type.DeclaringMethod is not null;
746+
747+
public static ReadOnlySpan<ParameterInfo> GetParametersAsSpan(this MethodBase metaMethod)
748+
=> metaMethod.GetParameters();
749+
}
750+
#endif
669751
}

src/libraries/System.Text.Json/Common/JsonSourceGenerationOptionsAttribute.cs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,11 @@ public JsonSourceGenerationOptionsAttribute(JsonSerializerDefaults defaults)
115115
/// </summary>
116116
public JsonCommentHandling ReadCommentHandling { get; set; }
117117

118+
/// <summary>
119+
/// Specifies the default value of <see cref="JsonSerializerOptions.RespectNullableAnnotations"/> when set.
120+
/// </summary>
121+
public bool RespectNullableAnnotations { get; set; }
122+
118123
/// <summary>
119124
/// Specifies the default value of <see cref="JsonSerializerOptions.UnknownTypeHandling"/> when set.
120125
/// </summary>

src/libraries/System.Text.Json/gen/Helpers/RoslynExtensions.cs

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,12 @@ public static ITypeSymbol EraseCompileTimeMetadata(this Compilation compilation,
5050
type = type.WithNullableAnnotation(NullableAnnotation.None);
5151
}
5252

53+
if (type is IArrayTypeSymbol arrayType)
54+
{
55+
ITypeSymbol elementType = compilation.EraseCompileTimeMetadata(arrayType.ElementType);
56+
return compilation.CreateArrayTypeSymbol(elementType, arrayType.Rank);
57+
}
58+
5359
if (type is INamedTypeSymbol namedType)
5460
{
5561
if (namedType.IsTupleType)
@@ -189,6 +195,9 @@ SpecialType.System_Byte or SpecialType.System_UInt16 or SpecialType.System_UInt3
189195
SpecialType.System_Single or SpecialType.System_Double or SpecialType.System_Decimal;
190196
}
191197

198+
public static bool IsNullableType(this ITypeSymbol type)
199+
=> !type.IsValueType || type.OriginalDefinition.SpecialType is SpecialType.System_Nullable_T;
200+
192201
public static bool IsNullableValueType(this ITypeSymbol type, [NotNullWhen(true)] out ITypeSymbol? elementType)
193202
{
194203
if (type.IsValueType && type is INamedTypeSymbol { OriginalDefinition.SpecialType: SpecialType.System_Nullable_T })
@@ -269,5 +278,107 @@ public static string GetTypeKindKeyword(this TypeDeclarationSyntax typeDeclarati
269278
return null;
270279
}
271280
}
281+
282+
public static void ResolveNullabilityAnnotations(this IFieldSymbol field, out bool isGetterNonNullable, out bool isSetterNonNullable)
283+
{
284+
if (field.Type.IsNullableType())
285+
{
286+
// Because System.Text.Json cannot distinguish between nullable and non-nullable type parameters,
287+
// (e.g. the same metadata is being used for both KeyValuePair<string, string?> and KeyValuePair<string, string>),
288+
// we derive nullability annotations from the original definition of the field and not its instantiation.
289+
// This preserves compatibility with the capabilities of the reflection-based NullabilityInfo reader.
290+
field = field.OriginalDefinition;
291+
292+
isGetterNonNullable = IsOutputTypeNonNullable(field, field.Type);
293+
isSetterNonNullable = IsInputTypeNonNullable(field, field.Type);
294+
}
295+
else
296+
{
297+
isGetterNonNullable = isSetterNonNullable = false;
298+
}
299+
}
300+
301+
public static void ResolveNullabilityAnnotations(this IPropertySymbol property, out bool isGetterNonNullable, out bool isSetterNonNullable)
302+
{
303+
if (property.Type.IsNullableType())
304+
{
305+
// Because System.Text.Json cannot distinguish between nullable and non-nullable type parameters,
306+
// (e.g. the same metadata is being used for both KeyValuePair<string, string?> and KeyValuePair<string, string>),
307+
// we derive nullability annotations from the original definition of the field and not its instantiation.
308+
// This preserves compatibility with the capabilities of the reflection-based NullabilityInfo reader.
309+
property = property.OriginalDefinition;
310+
311+
isGetterNonNullable = property.GetMethod != null && IsOutputTypeNonNullable(property, property.Type);
312+
isSetterNonNullable = property.SetMethod != null && IsInputTypeNonNullable(property, property.Type);
313+
}
314+
else
315+
{
316+
isGetterNonNullable = isSetterNonNullable = false;
317+
}
318+
}
319+
320+
public static bool IsNullable(this IParameterSymbol parameter)
321+
{
322+
if (parameter.Type.IsNullableType())
323+
{
324+
// Because System.Text.Json cannot distinguish between nullable and non-nullable type parameters,
325+
// (e.g. the same metadata is being used for both KeyValuePair<string, string?> and KeyValuePair<string, string>),
326+
// we derive nullability annotations from the original definition of the field and not instation.
327+
// This preserves compatibility with the capabilities of the reflection-based NullabilityInfo reader.
328+
parameter = parameter.OriginalDefinition;
329+
return !IsInputTypeNonNullable(parameter, parameter.Type);
330+
}
331+
332+
return false;
333+
}
334+
335+
private static bool IsOutputTypeNonNullable(this ISymbol symbol, ITypeSymbol returnType)
336+
{
337+
if (symbol.HasCodeAnalysisAttribute("MaybeNullAttribute"))
338+
{
339+
return false;
340+
}
341+
342+
if (symbol.HasCodeAnalysisAttribute("NotNullAttribute"))
343+
{
344+
return true;
345+
}
346+
347+
if (returnType is ITypeParameterSymbol { HasNotNullConstraint: false })
348+
{
349+
return false;
350+
}
351+
352+
return returnType.NullableAnnotation is NullableAnnotation.NotAnnotated;
353+
}
354+
355+
private static bool IsInputTypeNonNullable(this ISymbol symbol, ITypeSymbol inputType)
356+
{
357+
Debug.Assert(inputType.IsNullableType());
358+
359+
if (symbol.HasCodeAnalysisAttribute("AllowNullAttribute"))
360+
{
361+
return false;
362+
}
363+
364+
if (symbol.HasCodeAnalysisAttribute("DisallowNullAttribute"))
365+
{
366+
return true;
367+
}
368+
369+
if (inputType is ITypeParameterSymbol { HasNotNullConstraint: false })
370+
{
371+
return false;
372+
}
373+
374+
return inputType.NullableAnnotation is NullableAnnotation.NotAnnotated;
375+
}
376+
377+
private static bool HasCodeAnalysisAttribute(this ISymbol symbol, string attributeName)
378+
{
379+
return symbol.GetAttributes().Any(attr =>
380+
attr.AttributeClass?.Name == attributeName &&
381+
attr.AttributeClass.ContainingNamespace.ToDisplayString() == "System.Diagnostics.CodeAnalysis");
382+
}
272383
}
273384
}

src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.ExceptionMessages.cs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,9 @@ private static class ExceptionMessages
3030

3131
public const string InvalidSerializablePropertyConfiguration =
3232
"Invalid serializable-property configuration specified for type '{0}'. For more information, see 'JsonSourceGenerationMode.Serialization'.";
33+
34+
public const string PropertyGetterDisallowNull =
35+
"The property or field '{0}' on type '{1}' doesn't allow getting null values. Consider updating its nullability annotation.";
3336
};
3437
}
3538
}

0 commit comments

Comments
 (0)