Skip to content

Use NullabilityInfoContext in DataAnnotationsMetadataProvider #39219

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jan 4, 2022
Merged

Use NullabilityInfoContext in DataAnnotationsMetadataProvider #39219

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2063212
Use NullabilityInfoContext in DataAnnotationsMetadataProvider
captainsafia Dec 26, 2021
182abe3
Remove HasNullableAttribute method
captainsafia Jan 3, 2022
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
145 changes: 14 additions & 131 deletions src/Mvc/Mvc.DataAnnotations/src/DataAnnotationsMetadataProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)
Expand All @@ -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
{
Expand Down Expand Up @@ -467,108 +442,16 @@ private static string GetDisplayGroup(FieldInfo field)
return string.Empty;
}

internal static bool IsNullableReferenceType(Type? containingType, MemberInfo? member, IEnumerable<object> attributes)
{
if (HasNullableAttribute(attributes, out var result))
{
return result;
}

return IsNullableBasedOnContext(containingType, member);
}

// Internal for testing
internal static bool HasNullableAttribute(IEnumerable<object> 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<object>();
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;
}
}
29 changes: 18 additions & 11 deletions src/Mvc/Mvc.DataAnnotations/test/DataAnnotationsMetadataProviderTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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);
Expand All @@ -1606,9 +1603,11 @@ public void IsNullableReferenceType_ReturnsFalse_ForKeyValuePairWithoutNullableC
// Arrange
var type = typeof(KeyValuePair<string, object>);
var property = type.GetProperty(nameof(KeyValuePair<string, object>.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);
Expand All @@ -1621,9 +1620,11 @@ public void IsNullableReferenceType_ReturnsTrue_ForKeyValuePairWithNullableConst
// Arrange
var type = typeof(KeyValuePair<string, object>);
var property = type.GetProperty(nameof(KeyValuePair<string, object>.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.
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand Down