Fix nullable detection (#12202)

* Fix nullable detection

Fixes: #11828 and #11813
This commit is contained in:
Ryan Nowak 2019-07-16 19:58:30 -07:00 committed by GitHub
parent 892ea7e574
commit 4ba66f05bf
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 124 additions and 33 deletions

View File

@ -2,11 +2,15 @@
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
using System; using System;
using System.Collections;
using System.Collections.Generic; using System.Collections.Generic;
using System.ComponentModel; using System.ComponentModel;
using System.ComponentModel.DataAnnotations; using System.ComponentModel.DataAnnotations;
using System.Diagnostics.Contracts;
using System.Linq; using System.Linq;
using System.Reflection; using System.Reflection;
using System.Runtime.InteropServices;
using System.Runtime.InteropServices.ComTypes;
using Microsoft.AspNetCore.Mvc.ModelBinding; using Microsoft.AspNetCore.Mvc.ModelBinding;
using Microsoft.AspNetCore.Mvc.ModelBinding.Metadata; using Microsoft.AspNetCore.Mvc.ModelBinding.Metadata;
using Microsoft.Extensions.Localization; using Microsoft.Extensions.Localization;
@ -27,6 +31,9 @@ namespace Microsoft.AspNetCore.Mvc.DataAnnotations
private const string NullableAttributeFullTypeName = "System.Runtime.CompilerServices.NullableAttribute"; private const string NullableAttributeFullTypeName = "System.Runtime.CompilerServices.NullableAttribute";
private const string NullableFlagsFieldName = "NullableFlags"; private const string NullableFlagsFieldName = "NullableFlags";
private const string NullableContextAttributeFullName = "System.Runtime.CompilerServices.NullableContextAttribute";
private const string NullableContextFlagsFieldName = "Flag";
private readonly IStringLocalizerFactory _stringLocalizerFactory; private readonly IStringLocalizerFactory _stringLocalizerFactory;
private readonly MvcOptions _options; private readonly MvcOptions _options;
private readonly MvcDataAnnotationsLocalizationOptions _localizationOptions; private readonly MvcDataAnnotationsLocalizationOptions _localizationOptions;
@ -350,20 +357,44 @@ namespace Microsoft.AspNetCore.Mvc.DataAnnotations
if (!_options.SuppressImplicitRequiredAttributeForNonNullableReferenceTypes && if (!_options.SuppressImplicitRequiredAttributeForNonNullableReferenceTypes &&
requiredAttribute == null && requiredAttribute == null &&
!context.Key.ModelType.IsValueType && !context.Key.ModelType.IsValueType &&
context.Key.MetadataKind != ModelMetadataKind.Type)
// Look specifically at attributes on the property/parameter. [Nullable] on
// the type has a different meaning.
IsNonNullable(context.ParameterAttributes ?? context.PropertyAttributes ?? Array.Empty<object>()))
{ {
// Since this behavior specifically relates to non-null-ness, we will use the non-default var addInferredRequiredAttribute = false;
// option to tolerate empty/whitespace strings. empty/whitespace INPUT will still result in if (context.Key.MetadataKind == ModelMetadataKind.Type)
// a validation error by default because we convert empty/whitespace strings to null
// unless you say otherwise.
requiredAttribute = new RequiredAttribute()
{ {
AllowEmptyStrings = true, // Do nothing.
}; }
attributes.Add(requiredAttribute); else if (context.Key.MetadataKind == ModelMetadataKind.Property)
{
addInferredRequiredAttribute = IsNullableReferenceType(
context.Key.ContainerType,
member: null,
context.PropertyAttributes);
}
else if (context.Key.MetadataKind == ModelMetadataKind.Parameter)
{
addInferredRequiredAttribute = IsNullableReferenceType(
context.Key.ParameterInfo?.Member.ReflectedType,
context.Key.ParameterInfo.Member,
context.ParameterAttributes);
}
else
{
throw new InvalidOperationException("Unsupported ModelMetadataKind: " + context.Key.MetadataKind);
}
if (addInferredRequiredAttribute)
{
// Since this behavior specifically relates to non-null-ness, we will use the non-default
// option to tolerate empty/whitespace strings. empty/whitespace INPUT will still result in
// a validation error by default because we convert empty/whitespace strings to null
// unless you say otherwise.
requiredAttribute = new RequiredAttribute()
{
AllowEmptyStrings = true,
};
attributes.Add(requiredAttribute);
}
} }
if (requiredAttribute != null) if (requiredAttribute != null)
@ -419,16 +450,26 @@ namespace Microsoft.AspNetCore.Mvc.DataAnnotations
return string.Empty; 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 for testing
internal static bool IsNonNullable(IEnumerable<object> attributes) internal static bool HasNullableAttribute(IEnumerable<object> attributes, out bool isNullable)
{ {
// [Nullable] is compiler synthesized, comparing by name. // [Nullable] is compiler synthesized, comparing by name.
var nullableAttribute = attributes var nullableAttribute = attributes
.Where(a => string.Equals(a.GetType().FullName, NullableAttributeFullTypeName, StringComparison.Ordinal)) .FirstOrDefault(a => string.Equals(a.GetType().FullName, NullableAttributeFullTypeName, StringComparison.Ordinal));
.FirstOrDefault();
if (nullableAttribute == null) if (nullableAttribute == null)
{ {
return false; isNullable = false;
return false; // [Nullable] not found
} }
// We don't handle cases where generics and NNRT are used. This runs into a // We don't handle cases where generics and NNRT are used. This runs into a
@ -443,10 +484,61 @@ namespace Microsoft.AspNetCore.Mvc.DataAnnotations
flags.Length >= 0 && flags.Length >= 0 &&
flags[0] == 1) // First element is the property/parameter type. flags[0] == 1) // First element is the property/parameter type.
{ {
return true; isNullable = true;
return true; // [Nullable] found and type is an NNRT
} }
return false; isNullable = false;
return true; // [Nullable] found but type is not an NNRT
}
internal static bool IsNullableBasedOnContext(Type containingType, MemberInfo member)
{
// 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;
}
} }
} }
} }

View File

@ -1142,7 +1142,7 @@ namespace Microsoft.AspNetCore.Mvc.DataAnnotations
Assert.Equal(initialValue, context.ValidationMetadata.IsRequired); Assert.Equal(initialValue, context.ValidationMetadata.IsRequired);
} }
[Fact(Skip = "https://github.com/aspnet/AspNetCore/issues/11828")] [Fact]
public void CreateValidationMetadata_InfersRequiredAttribute_NoNonNullableProperty() public void CreateValidationMetadata_InfersRequiredAttribute_NoNonNullableProperty()
{ {
// Arrange // Arrange
@ -1152,7 +1152,7 @@ namespace Microsoft.AspNetCore.Mvc.DataAnnotations
typeof(NullableReferenceTypes), typeof(NullableReferenceTypes),
typeof(NullableReferenceTypes).GetProperty(nameof(NullableReferenceTypes.NonNullableReferenceType))); typeof(NullableReferenceTypes).GetProperty(nameof(NullableReferenceTypes.NonNullableReferenceType)));
var key = ModelMetadataIdentity.ForProperty( var key = ModelMetadataIdentity.ForProperty(
typeof(NullableReferenceTypes), typeof(NullableReferenceTypes),
nameof(NullableReferenceTypes.NonNullableReferenceType), typeof(string)); nameof(NullableReferenceTypes.NonNullableReferenceType), typeof(string));
var context = new ValidationMetadataProviderContext(key, attributes); var context = new ValidationMetadataProviderContext(key, attributes);
@ -1325,7 +1325,7 @@ namespace Microsoft.AspNetCore.Mvc.DataAnnotations
Assert.Same(attribute, validatorMetadata); Assert.Same(attribute, validatorMetadata);
} }
[Fact(Skip = "https://github.com/aspnet/AspNetCore/issues/11828")] [Fact]
public void IsNonNullable_FindsNonNullableProperty() public void IsNonNullable_FindsNonNullableProperty()
{ {
// Arrange // Arrange
@ -1333,7 +1333,7 @@ namespace Microsoft.AspNetCore.Mvc.DataAnnotations
var property = type.GetProperty(nameof(NullableReferenceTypes.NonNullableReferenceType)); var property = type.GetProperty(nameof(NullableReferenceTypes.NonNullableReferenceType));
// Act // Act
var result = DataAnnotationsMetadataProvider.IsNonNullable(property.GetCustomAttributes(inherit: true)); var result = DataAnnotationsMetadataProvider.IsNullableReferenceType(type, member: null, property.GetCustomAttributes(inherit: true));
// Assert // Assert
Assert.True(result); Assert.True(result);
@ -1347,13 +1347,13 @@ namespace Microsoft.AspNetCore.Mvc.DataAnnotations
var property = type.GetProperty(nameof(NullableReferenceTypes.NullableReferenceType)); var property = type.GetProperty(nameof(NullableReferenceTypes.NullableReferenceType));
// Act // Act
var result = DataAnnotationsMetadataProvider.IsNonNullable(property.GetCustomAttributes(inherit: true)); var result = DataAnnotationsMetadataProvider.IsNullableReferenceType(type, member: null, property.GetCustomAttributes(inherit: true));
// Assert // Assert
Assert.False(result); Assert.False(result);
} }
[Fact(Skip = "https://github.com/aspnet/AspNetCore/issues/11828")] [Fact]
public void IsNonNullable_FindsNonNullableParameter() public void IsNonNullable_FindsNonNullableParameter()
{ {
// Arrange // Arrange
@ -1362,7 +1362,7 @@ namespace Microsoft.AspNetCore.Mvc.DataAnnotations
var parameter = method.GetParameters().Where(p => p.Name == "nonNullableParameter").Single(); var parameter = method.GetParameters().Where(p => p.Name == "nonNullableParameter").Single();
// Act // Act
var result = DataAnnotationsMetadataProvider.IsNonNullable(parameter.GetCustomAttributes(inherit: true)); var result = DataAnnotationsMetadataProvider.IsNullableReferenceType(type, method, parameter.GetCustomAttributes(inherit: true));
// Assert // Assert
Assert.True(result); Assert.True(result);
@ -1377,7 +1377,7 @@ namespace Microsoft.AspNetCore.Mvc.DataAnnotations
var parameter = method.GetParameters().Where(p => p.Name == "nullableParameter").Single(); var parameter = method.GetParameters().Where(p => p.Name == "nullableParameter").Single();
// Act // Act
var result = DataAnnotationsMetadataProvider.IsNonNullable(parameter.GetCustomAttributes(inherit: true)); var result = DataAnnotationsMetadataProvider.IsNullableReferenceType(type, method, parameter.GetCustomAttributes(inherit: true));
// Assert // Assert
Assert.False(result); Assert.False(result);
@ -1429,12 +1429,12 @@ namespace Microsoft.AspNetCore.Mvc.DataAnnotations
return CreateProvider(options: null, localizationOptions, useStringLocalizer ? stringLocalizerFactory.Object : null); return CreateProvider(options: null, localizationOptions, useStringLocalizer ? stringLocalizerFactory.Object : null);
} }
private ModelAttributes GetModelAttributes(IEnumerable<object> typeAttributes) private ModelAttributes GetModelAttributes(IEnumerable<object> typeAttributes)
=> new ModelAttributes(typeAttributes, Array.Empty<object>(), Array.Empty<object>()); => new ModelAttributes(typeAttributes, Array.Empty<object>(), Array.Empty<object>());
private ModelAttributes GetModelAttributes( private ModelAttributes GetModelAttributes(
IEnumerable<object> typeAttributes, IEnumerable<object> typeAttributes,
IEnumerable<object> propertyAttributes) IEnumerable<object> propertyAttributes)
=> new ModelAttributes(typeAttributes, propertyAttributes, Array.Empty<object>()); => new ModelAttributes(typeAttributes, propertyAttributes, Array.Empty<object>());
private class KVPEnumGroupAndNameComparer : IEqualityComparer<KeyValuePair<EnumGroupAndName, string>> private class KVPEnumGroupAndNameComparer : IEqualityComparer<KeyValuePair<EnumGroupAndName, string>>

View File

@ -26,7 +26,7 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests
private HttpClient Client { get; set; } private HttpClient Client { get; set; }
[Fact(Skip = "https://github.com/aspnet/AspNetCore/issues/11828")] [Fact]
public async Task CanUseNonNullableReferenceType_WithController_OmitData_ValidationErrors() public async Task CanUseNonNullableReferenceType_WithController_OmitData_ValidationErrors()
{ {
// Arrange // Arrange

View File

@ -25,7 +25,7 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests
} }
#nullable restore #nullable restore
[Fact(Skip = "https://github.com/aspnet/AspNetCore/issues/11828")] [Fact]
public async Task BindProperty_WithNonNullableReferenceType_NoData_ValidationError() public async Task BindProperty_WithNonNullableReferenceType_NoData_ValidationError()
{ {
// Arrange // Arrange
@ -112,7 +112,7 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests
} }
#nullable restore #nullable restore
[Fact(Skip = "https://github.com/aspnet/AspNetCore/issues/11828")] [Fact]
public async Task BindProperty_WithNonNullableReferenceType_NoData_ValidationError_CustomMessage() public async Task BindProperty_WithNonNullableReferenceType_NoData_ValidationError_CustomMessage()
{ {
// Arrange // Arrange
@ -159,7 +159,7 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests
} }
#nullable restore #nullable restore
[Fact(Skip = "https://github.com/aspnet/AspNetCore/issues/11828")] [Fact]
public async Task BindParameter_WithNonNullableReferenceType_NoData_ValidationError() public async Task BindParameter_WithNonNullableReferenceType_NoData_ValidationError()
{ {
// Arrange // Arrange

View File

@ -2,7 +2,6 @@
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
#nullable enable #nullable enable
using BasicWebSite.Models;
using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore.Mvc;
namespace BasicWebSite.Controllers namespace BasicWebSite.Controllers