Skip to content

Commit b2c2cb0

Browse files
captainsafiacaptainsafia
authored
Use NullabilityInfoContext in DataAnnotationsMetadataProvider (#39219)
* Use NullabilityInfoContext in DataAnnotationsMetadataProvider * Remove HasNullableAttribute method
1 parent 95d5aab commit b2c2cb0

File tree

2 files changed

+32
-142
lines changed

2 files changed

+32
-142
lines changed

‎src/Mvc/Mvc.DataAnnotations/src/DataAnnotationsMetadataProvider.cs‎

Lines changed: 14 additions & 131 deletions
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,6 @@ internal class DataAnnotationsMetadataProvider :
2121
IDisplayMetadataProvider,
2222
IValidationMetadataProvider
2323
{
24-
// The [Nullable] attribute is synthesized by the compiler. It's best to just compare the type name.
25-
private const string NullableAttributeFullTypeName = "System.Runtime.CompilerServices.NullableAttribute";
26-
private const string NullableFlagsFieldName = "NullableFlags";
27-
28-
private const string NullableContextAttributeFullName = "System.Runtime.CompilerServices.NullableContextAttribute";
29-
private const string NullableContextFlagsFieldName = "Flag";
30-
3124
private readonly IStringLocalizerFactory? _stringLocalizerFactory;
3225
private readonly MvcOptions _options;
3326
private readonly MvcDataAnnotationsLocalizationOptions _localizationOptions;
@@ -360,25 +353,9 @@ public void CreateValidationMetadata(ValidationMetadataProviderContext context)
360353
else if (context.Key.MetadataKind == ModelMetadataKind.Property)
361354
{
362355
var property = context.Key.PropertyInfo;
363-
if (property is null)
364-
{
365-
// PropertyInfo was unavailable on ModelIdentity prior to 3.1.
366-
// Making a cogent argument about the nullability of the property requires inspecting the declared type,
367-
// since looking at the runtime type may result in false positives: https://github.com/dotnet/aspnetcore/issues/14812
368-
// The only way we could arrive here is if the ModelMetadata was constructed using the non-default provider.
369-
// We'll cursorily examine the attributes on the property, but not the ContainerType to make a decision about it's nullability.
370-
371-
if (HasNullableAttribute(context.PropertyAttributes!, out var propertyHasNullableAttribute))
372-
{
373-
addInferredRequiredAttribute = propertyHasNullableAttribute;
374-
}
375-
}
376-
else
356+
if (property is not null)
377357
{
378-
addInferredRequiredAttribute = IsNullableReferenceType(
379-
property.DeclaringType!,
380-
member: null,
381-
context.PropertyAttributes!);
358+
addInferredRequiredAttribute = IsRequired(context);
382359
}
383360
}
384361
else if (context.Key.MetadataKind == ModelMetadataKind.Parameter)
@@ -387,11 +364,9 @@ public void CreateValidationMetadata(ValidationMetadataProviderContext context)
387364
// since the parameter will be optional.
388365
if (!context.Key.ParameterInfo!.HasDefaultValue)
389366
{
390-
addInferredRequiredAttribute = IsNullableReferenceType(
391-
context.Key.ParameterInfo!.Member.ReflectedType,
392-
context.Key.ParameterInfo.Member,
393-
context.ParameterAttributes!);
367+
addInferredRequiredAttribute = IsRequired(context);
394368
}
369+
395370
}
396371
else
397372
{
@@ -465,108 +440,16 @@ private static string GetDisplayGroup(FieldInfo field)
465440
return string.Empty;
466441
}
467442

468-
internal static bool IsNullableReferenceType(Type? containingType, MemberInfo? member, IEnumerable<object> attributes)
469-
{
470-
if (HasNullableAttribute(attributes, out var result))
471-
{
472-
return result;
473-
}
474-
475-
return IsNullableBasedOnContext(containingType, member);
476-
}
477-
478-
// Internal for testing
479-
internal static bool HasNullableAttribute(IEnumerable<object> attributes, out bool isNullable)
443+
internal static bool IsRequired(ValidationMetadataProviderContext context)
480444
{
481-
// [Nullable] is compiler synthesized, comparing by name.
482-
var nullableAttribute = attributes
483-
.FirstOrDefault(a => string.Equals(a.GetType().FullName, NullableAttributeFullTypeName, StringComparison.Ordinal));
484-
if (nullableAttribute == null)
485-
{
486-
isNullable = false;
487-
return false; // [Nullable] not found
488-
}
489-
490-
// We don't handle cases where generics and NNRT are used. This runs into a
491-
// fundamental limitation of ModelMetadata - we use a single Type and Property/Parameter
492-
// to look up the metadata. However when generics are involved and NNRT is in use
493-
// the distance between the [Nullable] and member we're looking at is potentially
494-
// unbounded.
495-
//
496-
// See: https://github.com/dotnet/roslyn/blob/master/docs/features/nullable-reference-types.md#annotations
497-
if (nullableAttribute.GetType().GetField(NullableFlagsFieldName) is FieldInfo field &&
498-
field.GetValue(nullableAttribute) is byte[] flags &&
499-
flags.Length > 0 &&
500-
flags[0] == 1) // First element is the property/parameter type.
501-
{
502-
isNullable = true;
503-
return true; // [Nullable] found and type is an NNRT
504-
}
505-
506-
isNullable = false;
507-
return true; // [Nullable] found but type is not an NNRT
508-
}
509-
510-
internal static bool IsNullableBasedOnContext(Type? containingType, MemberInfo? member)
511-
{
512-
if (containingType is null)
513-
{
514-
return false;
515-
}
516-
517-
// For generic types, inspecting the nullability requirement additionally requires
518-
// inspecting the nullability constraint on generic type parameters. This is fairly non-triviial
519-
// so we'll just avoid calculating it. Users should still be able to apply an explicit [Required]
520-
// attribute on these members.
521-
if (containingType.IsGenericType)
522-
{
523-
return false;
524-
}
525-
526-
// The [Nullable] and [NullableContext] attributes are not inherited.
527-
//
528-
// The [NullableContext] attribute can appear on a method or on the module.
529-
var attributes = member?.GetCustomAttributes(inherit: false) ?? Array.Empty<object>();
530-
var isNullable = AttributesHasNullableContext(attributes);
531-
if (isNullable != null)
532-
{
533-
return isNullable.Value;
534-
}
535-
536-
// Check on the containing type
537-
var type = containingType;
538-
do
539-
{
540-
attributes = type.GetCustomAttributes(inherit: false);
541-
isNullable = AttributesHasNullableContext(attributes);
542-
if (isNullable != null)
543-
{
544-
return isNullable.Value;
545-
}
546-
547-
type = type.DeclaringType;
548-
}
549-
while (type != null);
550-
551-
// If we don't find the attribute on the declaring type then repeat at the module level
552-
attributes = containingType.Module.GetCustomAttributes(inherit: false);
553-
isNullable = AttributesHasNullableContext(attributes);
554-
return isNullable ?? false;
555-
556-
bool? AttributesHasNullableContext(object[] attributes)
557-
{
558-
var nullableContextAttribute = attributes
559-
.FirstOrDefault(a => string.Equals(a.GetType().FullName, NullableContextAttributeFullName, StringComparison.Ordinal));
560-
if (nullableContextAttribute != null)
561-
{
562-
if (nullableContextAttribute.GetType().GetField(NullableContextFlagsFieldName) is FieldInfo field &&
563-
field.GetValue(nullableContextAttribute) is byte @byte)
564-
{
565-
return @byte == 1; // [NullableContext] found
566-
}
567-
}
568-
569-
return null;
570-
}
445+
var nullabilityContext = new NullabilityInfoContext();
446+
var nullability = context.Key.MetadataKind switch
447+
{
448+
ModelMetadataKind.Parameter => nullabilityContext.Create(context.Key.ParameterInfo!),
449+
ModelMetadataKind.Property => nullabilityContext.Create(context.Key.PropertyInfo!),
450+
_ => null
451+
};
452+
var isOptional = nullability != null && nullability.ReadState != NullabilityState.NotNull;
453+
return !isOptional;
571454
}
572455
}

‎src/Mvc/Mvc.DataAnnotations/test/DataAnnotationsMetadataProviderTest.cs‎

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1232,11 +1232,6 @@ public void CreateValidationMetadata_InfersRequiredAttributeOnDerivedType_BaseAn
12321232
var key = ModelMetadataIdentity.ForProperty(property, property.PropertyType, modelType);
12331233
var context = new ValidationMetadataProviderContext(key, ModelAttributes.GetAttributesForProperty(modelType, property));
12341234

1235-
// This test verifies how MVC reads the NullableContextOptions. We expect the property to not have a Nullable attribute on, and for
1236-
// the types to have NullableContext. We'll encode our expectations as assertions so that we can catch if or when the compiler changes
1237-
// this behavior and the test needs to be tweaked.
1238-
Assert.False(DataAnnotationsMetadataProvider.HasNullableAttribute(context.PropertyAttributes, out _), "We do not expect NullableAttribute to be defined on the property");
1239-
12401235
// Act
12411236
provider.CreateValidationMetadata(context);
12421237

@@ -1587,9 +1582,11 @@ public void IsNonNullable_FindsNonNullableProperty()
15871582
// Arrange
15881583
var type = typeof(NullableReferenceTypes);
15891584
var property = type.GetProperty(nameof(NullableReferenceTypes.NonNullableReferenceType));
1585+
var key = ModelMetadataIdentity.ForProperty(property, type, type);
1586+
var context = new ValidationMetadataProviderContext(key, GetModelAttributes(property.GetCustomAttributes(inherit: true)));
15901587

15911588
// Act
1592-
var result = DataAnnotationsMetadataProvider.IsNullableReferenceType(type, member: null, property.GetCustomAttributes(inherit: true));
1589+
var result = DataAnnotationsMetadataProvider.IsRequired(context);
15931590

15941591
// Assert
15951592
Assert.True(result);
@@ -1601,9 +1598,11 @@ public void IsNullableReferenceType_ReturnsFalse_ForKeyValuePairWithoutNullableC
16011598
// Arrange
16021599
var type = typeof(KeyValuePair<string, object>);
16031600
var property = type.GetProperty(nameof(KeyValuePair<string, object>.Key));
1601+
var key = ModelMetadataIdentity.ForProperty(property, type, type);
1602+
var context = new ValidationMetadataProviderContext(key, GetModelAttributes(property.GetCustomAttributes(inherit: true)));
16041603

16051604
// Act
1606-
var result = DataAnnotationsMetadataProvider.IsNullableReferenceType(type, member: null, property.GetCustomAttributes(inherit: true));
1605+
var result = DataAnnotationsMetadataProvider.IsRequired(context);
16071606

16081607
// Assert
16091608
Assert.False(result);
@@ -1616,9 +1615,11 @@ public void IsNullableReferenceType_ReturnsTrue_ForKeyValuePairWithNullableConst
16161615
// Arrange
16171616
var type = typeof(KeyValuePair<string, object>);
16181617
var property = type.GetProperty(nameof(KeyValuePair<string, object>.Key))!;
1618+
var key = ModelMetadataIdentity.ForProperty(property, type, type);
1619+
var context = new ValidationMetadataProviderContext(key, GetModelAttributes(property.GetCustomAttributes(inherit: true)));
16191620

16201621
// Act
1621-
var result = DataAnnotationsMetadataProvider.IsNullableReferenceType(type, member: null, property.GetCustomAttributes(inherit: true));
1622+
var result = DataAnnotationsMetadataProvider.IsRequired(context);
16221623

16231624
// Assert
16241625
// While we'd like for result to be 'true', we don't have a very good way of actually calculating it correctly.
@@ -1633,9 +1634,11 @@ public void IsNonNullable_FindsNullableProperty()
16331634
// Arrange
16341635
var type = typeof(NullableReferenceTypes);
16351636
var property = type.GetProperty(nameof(NullableReferenceTypes.NullableReferenceType));
1637+
var key = ModelMetadataIdentity.ForProperty(property, type, type);
1638+
var context = new ValidationMetadataProviderContext(key, GetModelAttributes(property.GetCustomAttributes(inherit: true)));
16361639

16371640
// Act
1638-
var result = DataAnnotationsMetadataProvider.IsNullableReferenceType(type, member: null, property.GetCustomAttributes(inherit: true));
1641+
var result = DataAnnotationsMetadataProvider.IsRequired(context);
16391642

16401643
// Assert
16411644
Assert.False(result);
@@ -1648,9 +1651,11 @@ public void IsNonNullable_FindsNonNullableParameter()
16481651
var type = typeof(NullableReferenceTypes);
16491652
var method = type.GetMethod(nameof(NullableReferenceTypes.Method));
16501653
var parameter = method.GetParameters().Where(p => p.Name == "nonNullableParameter").Single();
1654+
var key = ModelMetadataIdentity.ForParameter(parameter);
1655+
var context = new ValidationMetadataProviderContext(key, GetModelAttributes(parameter.GetCustomAttributes(inherit: true)));
16511656

16521657
// Act
1653-
var result = DataAnnotationsMetadataProvider.IsNullableReferenceType(type, method, parameter.GetCustomAttributes(inherit: true));
1658+
var result = DataAnnotationsMetadataProvider.IsRequired(context);
16541659

16551660
// Assert
16561661
Assert.True(result);
@@ -1663,9 +1668,11 @@ public void IsNonNullable_FindsNullableParameter()
16631668
var type = typeof(NullableReferenceTypes);
16641669
var method = type.GetMethod(nameof(NullableReferenceTypes.Method));
16651670
var parameter = method.GetParameters().Where(p => p.Name == "nullableParameter").Single();
1671+
var key = ModelMetadataIdentity.ForParameter(parameter);
1672+
var context = new ValidationMetadataProviderContext(key, GetModelAttributes(parameter.GetCustomAttributes(inherit: true)));
16661673

16671674
// Act
1668-
var result = DataAnnotationsMetadataProvider.IsNullableReferenceType(type, method, parameter.GetCustomAttributes(inherit: true));
1675+
var result = DataAnnotationsMetadataProvider.IsRequired(context);
16691676

16701677
// Assert
16711678
Assert.False(result);

0 commit comments

Comments
 (0)