diff --git a/src/Mvc/Mvc.DataAnnotations/src/DataAnnotationsMetadataProvider.cs b/src/Mvc/Mvc.DataAnnotations/src/DataAnnotationsMetadataProvider.cs index d232d669feef..a16239a55238 100644 --- a/src/Mvc/Mvc.DataAnnotations/src/DataAnnotationsMetadataProvider.cs +++ b/src/Mvc/Mvc.DataAnnotations/src/DataAnnotationsMetadataProvider.cs @@ -23,13 +23,6 @@ internal class DataAnnotationsMetadataProvider : IDisplayMetadataProvider, IValidationMetadataProvider { - // The [Nullable] attribute is synthesized by the compiler. It's best to just compare the type name. - private const string NullableAttributeFullTypeName = "System.Runtime.CompilerServices.NullableAttribute"; - private const string NullableFlagsFieldName = "NullableFlags"; - - private const string NullableContextAttributeFullName = "System.Runtime.CompilerServices.NullableContextAttribute"; - private const string NullableContextFlagsFieldName = "Flag"; - private readonly IStringLocalizerFactory? _stringLocalizerFactory; private readonly MvcOptions _options; private readonly MvcDataAnnotationsLocalizationOptions _localizationOptions; @@ -362,25 +355,9 @@ public void CreateValidationMetadata(ValidationMetadataProviderContext context) else if (context.Key.MetadataKind == ModelMetadataKind.Property) { var property = context.Key.PropertyInfo; - if (property is null) - { - // PropertyInfo was unavailable on ModelIdentity prior to 3.1. - // Making a cogent argument about the nullability of the property requires inspecting the declared type, - // since looking at the runtime type may result in false positives: https://github.com/dotnet/aspnetcore/issues/14812 - // The only way we could arrive here is if the ModelMetadata was constructed using the non-default provider. - // We'll cursorily examine the attributes on the property, but not the ContainerType to make a decision about it's nullability. - - if (HasNullableAttribute(context.PropertyAttributes!, out var propertyHasNullableAttribute)) - { - addInferredRequiredAttribute = propertyHasNullableAttribute; - } - } - else + if (property is not null) { - addInferredRequiredAttribute = IsNullableReferenceType( - property.DeclaringType!, - member: null, - context.PropertyAttributes!); + addInferredRequiredAttribute = IsRequired(context); } } else if (context.Key.MetadataKind == ModelMetadataKind.Parameter) @@ -389,11 +366,9 @@ public void CreateValidationMetadata(ValidationMetadataProviderContext context) // since the parameter will be optional. if (!context.Key.ParameterInfo!.HasDefaultValue) { - addInferredRequiredAttribute = IsNullableReferenceType( - context.Key.ParameterInfo!.Member.ReflectedType, - context.Key.ParameterInfo.Member, - context.ParameterAttributes!); + addInferredRequiredAttribute = IsRequired(context); } + } else { @@ -467,108 +442,16 @@ private static string GetDisplayGroup(FieldInfo field) return string.Empty; } - internal static bool IsNullableReferenceType(Type? containingType, MemberInfo? member, IEnumerable attributes) - { - if (HasNullableAttribute(attributes, out var result)) - { - return result; - } - - return IsNullableBasedOnContext(containingType, member); - } - - // Internal for testing - internal static bool HasNullableAttribute(IEnumerable attributes, out bool isNullable) + internal static bool IsRequired(ValidationMetadataProviderContext context) { - // [Nullable] is compiler synthesized, comparing by name. - var nullableAttribute = attributes - .FirstOrDefault(a => string.Equals(a.GetType().FullName, NullableAttributeFullTypeName, StringComparison.Ordinal)); - if (nullableAttribute == null) - { - isNullable = false; - return false; // [Nullable] not found - } - - // We don't handle cases where generics and NNRT are used. This runs into a - // fundamental limitation of ModelMetadata - we use a single Type and Property/Parameter - // to look up the metadata. However when generics are involved and NNRT is in use - // the distance between the [Nullable] and member we're looking at is potentially - // unbounded. - // - // See: https://github.com/dotnet/roslyn/blob/master/docs/features/nullable-reference-types.md#annotations - if (nullableAttribute.GetType().GetField(NullableFlagsFieldName) is FieldInfo field && - field.GetValue(nullableAttribute) is byte[] flags && - flags.Length > 0 && - flags[0] == 1) // First element is the property/parameter type. - { - isNullable = true; - return true; // [Nullable] found and type is an NNRT - } - - isNullable = false; - return true; // [Nullable] found but type is not an NNRT - } - - internal static bool IsNullableBasedOnContext(Type? containingType, MemberInfo? member) - { - if (containingType is null) - { - return false; - } - - // For generic types, inspecting the nullability requirement additionally requires - // inspecting the nullability constraint on generic type parameters. This is fairly non-triviial - // so we'll just avoid calculating it. Users should still be able to apply an explicit [Required] - // attribute on these members. - if (containingType.IsGenericType) - { - return false; - } - - // The [Nullable] and [NullableContext] attributes are not inherited. - // - // The [NullableContext] attribute can appear on a method or on the module. - var attributes = member?.GetCustomAttributes(inherit: false) ?? Array.Empty(); - var isNullable = AttributesHasNullableContext(attributes); - if (isNullable != null) - { - return isNullable.Value; - } - - // Check on the containing type - var type = containingType; - do - { - attributes = type.GetCustomAttributes(inherit: false); - isNullable = AttributesHasNullableContext(attributes); - if (isNullable != null) - { - return isNullable.Value; - } - - type = type.DeclaringType; - } - while (type != null); - - // If we don't find the attribute on the declaring type then repeat at the module level - attributes = containingType.Module.GetCustomAttributes(inherit: false); - isNullable = AttributesHasNullableContext(attributes); - return isNullable ?? false; - - bool? AttributesHasNullableContext(object[] attributes) - { - var nullableContextAttribute = attributes - .FirstOrDefault(a => string.Equals(a.GetType().FullName, NullableContextAttributeFullName, StringComparison.Ordinal)); - if (nullableContextAttribute != null) - { - if (nullableContextAttribute.GetType().GetField(NullableContextFlagsFieldName) is FieldInfo field && - field.GetValue(nullableContextAttribute) is byte @byte) - { - return @byte == 1; // [NullableContext] found - } - } - - return null; - } + var nullabilityContext = new NullabilityInfoContext(); + var nullability = context.Key.MetadataKind switch + { + ModelMetadataKind.Parameter => nullabilityContext.Create(context.Key.ParameterInfo!), + ModelMetadataKind.Property => nullabilityContext.Create(context.Key.PropertyInfo!), + _ => null + }; + var isOptional = nullability != null && nullability.ReadState != NullabilityState.NotNull; + return !isOptional; } } diff --git a/src/Mvc/Mvc.DataAnnotations/test/DataAnnotationsMetadataProviderTest.cs b/src/Mvc/Mvc.DataAnnotations/test/DataAnnotationsMetadataProviderTest.cs index 8f0b685468f7..120f34fac945 100644 --- a/src/Mvc/Mvc.DataAnnotations/test/DataAnnotationsMetadataProviderTest.cs +++ b/src/Mvc/Mvc.DataAnnotations/test/DataAnnotationsMetadataProviderTest.cs @@ -1237,11 +1237,6 @@ public void CreateValidationMetadata_InfersRequiredAttributeOnDerivedType_BaseAn var key = ModelMetadataIdentity.ForProperty(property, property.PropertyType, modelType); var context = new ValidationMetadataProviderContext(key, ModelAttributes.GetAttributesForProperty(modelType, property)); - // This test verifies how MVC reads the NullableContextOptions. We expect the property to not have a Nullable attribute on, and for - // the types to have NullableContext. We'll encode our expectations as assertions so that we can catch if or when the compiler changes - // this behavior and the test needs to be tweaked. - Assert.False(DataAnnotationsMetadataProvider.HasNullableAttribute(context.PropertyAttributes, out _), "We do not expect NullableAttribute to be defined on the property"); - // Act provider.CreateValidationMetadata(context); @@ -1592,9 +1587,11 @@ public void IsNonNullable_FindsNonNullableProperty() // Arrange var type = typeof(NullableReferenceTypes); var property = type.GetProperty(nameof(NullableReferenceTypes.NonNullableReferenceType)); + var key = ModelMetadataIdentity.ForProperty(property, type, type); + var context = new ValidationMetadataProviderContext(key, GetModelAttributes(property.GetCustomAttributes(inherit: true))); // Act - var result = DataAnnotationsMetadataProvider.IsNullableReferenceType(type, member: null, property.GetCustomAttributes(inherit: true)); + var result = DataAnnotationsMetadataProvider.IsRequired(context); // Assert Assert.True(result); @@ -1606,9 +1603,11 @@ public void IsNullableReferenceType_ReturnsFalse_ForKeyValuePairWithoutNullableC // Arrange var type = typeof(KeyValuePair); var property = type.GetProperty(nameof(KeyValuePair.Key)); + var key = ModelMetadataIdentity.ForProperty(property, type, type); + var context = new ValidationMetadataProviderContext(key, GetModelAttributes(property.GetCustomAttributes(inherit: true))); // Act - var result = DataAnnotationsMetadataProvider.IsNullableReferenceType(type, member: null, property.GetCustomAttributes(inherit: true)); + var result = DataAnnotationsMetadataProvider.IsRequired(context); // Assert Assert.False(result); @@ -1621,9 +1620,11 @@ public void IsNullableReferenceType_ReturnsTrue_ForKeyValuePairWithNullableConst // Arrange var type = typeof(KeyValuePair); var property = type.GetProperty(nameof(KeyValuePair.Key))!; + var key = ModelMetadataIdentity.ForProperty(property, type, type); + var context = new ValidationMetadataProviderContext(key, GetModelAttributes(property.GetCustomAttributes(inherit: true))); // Act - var result = DataAnnotationsMetadataProvider.IsNullableReferenceType(type, member: null, property.GetCustomAttributes(inherit: true)); + var result = DataAnnotationsMetadataProvider.IsRequired(context); // Assert // While we'd like for result to be 'true', we don't have a very good way of actually calculating it correctly. @@ -1638,9 +1639,11 @@ public void IsNonNullable_FindsNullableProperty() // Arrange var type = typeof(NullableReferenceTypes); var property = type.GetProperty(nameof(NullableReferenceTypes.NullableReferenceType)); + var key = ModelMetadataIdentity.ForProperty(property, type, type); + var context = new ValidationMetadataProviderContext(key, GetModelAttributes(property.GetCustomAttributes(inherit: true))); // Act - var result = DataAnnotationsMetadataProvider.IsNullableReferenceType(type, member: null, property.GetCustomAttributes(inherit: true)); + var result = DataAnnotationsMetadataProvider.IsRequired(context); // Assert Assert.False(result); @@ -1653,9 +1656,11 @@ public void IsNonNullable_FindsNonNullableParameter() var type = typeof(NullableReferenceTypes); var method = type.GetMethod(nameof(NullableReferenceTypes.Method)); var parameter = method.GetParameters().Where(p => p.Name == "nonNullableParameter").Single(); + var key = ModelMetadataIdentity.ForParameter(parameter); + var context = new ValidationMetadataProviderContext(key, GetModelAttributes(parameter.GetCustomAttributes(inherit: true))); // Act - var result = DataAnnotationsMetadataProvider.IsNullableReferenceType(type, method, parameter.GetCustomAttributes(inherit: true)); + var result = DataAnnotationsMetadataProvider.IsRequired(context); // Assert Assert.True(result); @@ -1668,9 +1673,11 @@ public void IsNonNullable_FindsNullableParameter() var type = typeof(NullableReferenceTypes); var method = type.GetMethod(nameof(NullableReferenceTypes.Method)); var parameter = method.GetParameters().Where(p => p.Name == "nullableParameter").Single(); + var key = ModelMetadataIdentity.ForParameter(parameter); + var context = new ValidationMetadataProviderContext(key, GetModelAttributes(parameter.GetCustomAttributes(inherit: true))); // Act - var result = DataAnnotationsMetadataProvider.IsNullableReferenceType(type, method, parameter.GetCustomAttributes(inherit: true)); + var result = DataAnnotationsMetadataProvider.IsRequired(context); // Assert Assert.False(result);