Skip to content
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

Jump to bottom

More nullability work #16985

Merged
merged 2 commits into from
Aug 7, 2019
Merged

More nullability work #16985

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
f1a177a
NullableContextAttribute can no longer appear at module level
roji Jul 31, 2019
7cc88dc
Recognize [MaybeNull] for nullability
roji Aug 1, 2019
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
83 changes: 37 additions & 46 deletions src/EFCore/Metadata/Conventions/NonNullableConventionBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,12 @@

using System;
using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;
using System.Linq;
using System.Reflection;
using JetBrains.Annotations;
using Microsoft.EntityFrameworkCore.Metadata.Builders;
using Microsoft.EntityFrameworkCore.Metadata.Conventions.Infrastructure;
using JetbrainsNotNull = JetBrains.Annotations.NotNullAttribute;

namespace Microsoft.EntityFrameworkCore.Metadata.Conventions
{
Expand All @@ -28,7 +29,7 @@ public abstract class NonNullableConventionBase : IModelFinalizedConvention
/// Creates a new instance of <see cref="NonNullableConventionBase" />.
/// </summary>
/// <param name="dependencies"> Parameter object containing dependencies for this convention. </param>
protected NonNullableConventionBase([NotNull] ProviderConventionSetBuilderDependencies dependencies)
protected NonNullableConventionBase([JetbrainsNotNull] ProviderConventionSetBuilderDependencies dependencies)
{
Dependencies = dependencies;
}
Expand All @@ -38,36 +39,15 @@ protected NonNullableConventionBase([NotNull] ProviderConventionSetBuilderDepend
/// </summary>
protected virtual ProviderConventionSetBuilderDependencies Dependencies { get; }

private byte? GetNullabilityContextFlag(NonNullabilityConventionState state, Attribute[] attributes)
{
if (attributes.FirstOrDefault(a => a.GetType().FullName == NullableContextAttributeFullName) is Attribute attribute)
{
var attributeType = attribute.GetType();

if (attributeType != state.NullableContextAttrType)
{
state.NullableContextFlagFieldInfo = attributeType.GetField("Flag");
state.NullableContextAttrType = attributeType;
}

if (state.NullableContextFlagFieldInfo?.GetValue(attribute) is byte flag)
{
return flag;
}
}

return null;
}

/// <summary>
/// Returns a value indicating whether the member type is a non-nullable reference type.
/// </summary>
/// <param name="modelBuilder"> The model builder used to build the model. </param>
/// <param name="memberInfo"> The member info. </param>
/// <returns> <c>true</c> if the member type is a non-nullable reference type. </returns>
protected virtual bool IsNonNullableRefType(
[NotNull] IConventionModelBuilder modelBuilder,
[NotNull] MemberInfo memberInfo)
[JetbrainsNotNull] IConventionModelBuilder modelBuilder,
[JetbrainsNotNull] MemberInfo memberInfo)
{
if (memberInfo.GetMemberType().IsValueType)
{
Expand All @@ -76,14 +56,27 @@ protected virtual bool IsNonNullableRefType(

var state = GetOrInitializeState(modelBuilder);

// First check for [MaybeNull] on the return value. If it exists, the member is nullable.
var isMaybeNull = memberInfo switch
{
FieldInfo f => f.GetCustomAttribute<MaybeNullAttribute>() != null,
PropertyInfo p => p.GetMethod?.ReturnParameter?.GetCustomAttribute<MaybeNullAttribute>() != null,
_ => false
};

if (isMaybeNull)
{
return false;
}

// For C# 8.0 nullable types, the C# currently synthesizes a NullableAttribute that expresses nullability into assemblies
// it produces. If the model is spread across more than one assembly, there will be multiple versions of this attribute,
// so look for it by name, caching to avoid reflection on every check.
// Note that this may change - if https://github.com/dotnet/corefx/issues/36222 is done we can remove all of this.

// First look for NullableAttribute on the member itself
if (Attribute.GetCustomAttributes(memberInfo)
.FirstOrDefault(a => a.GetType().FullName == NullableAttributeFullName) is Attribute attribute)
.FirstOrDefault(a => a.GetType().FullName == NullableAttributeFullName) is Attribute attribute)
{
var attributeType = attribute.GetType();

Expand All @@ -103,33 +96,32 @@ protected virtual bool IsNonNullableRefType(
var type = memberInfo.DeclaringType;
if (type != null)
{
if (state.TypeNonNullabilityContextCache.TryGetValue(type, out var cachedTypeNonNullable))
if (state.TypeCache.TryGetValue(type, out var cachedTypeNonNullable))
{
return cachedTypeNonNullable;
}

var typeContextFlag = GetNullabilityContextFlag(state, Attribute.GetCustomAttributes(type));
if (typeContextFlag.HasValue)
if (Attribute.GetCustomAttributes(type)
.FirstOrDefault(a => a.GetType().FullName == NullableContextAttributeFullName) is Attribute contextAttr)
{
return state.TypeNonNullabilityContextCache[type] = typeContextFlag.Value == 1;
var attributeType = contextAttr.GetType();

if (attributeType != state.NullableContextAttrType)
{
state.NullableContextFlagFieldInfo = attributeType.GetField("Flag");
state.NullableContextAttrType = attributeType;
}

if (state.NullableContextFlagFieldInfo?.GetValue(contextAttr) is byte flag)
{
return state.TypeCache[type] = flag == 1;
}
}
}

// Not found at the type level, try at the module level
var module = memberInfo.Module;
if (!state.ModuleNonNullabilityContextCache.TryGetValue(module, out var moduleNonNullable))
{
var moduleContextFlag = GetNullabilityContextFlag(state, Attribute.GetCustomAttributes(memberInfo.Module));
moduleNonNullable = state.ModuleNonNullabilityContextCache[module] =
moduleContextFlag.HasValue && moduleContextFlag == 1;
}

if (type != null)
{
state.TypeNonNullabilityContextCache[type] = moduleNonNullable;
return state.TypeCache[type] = false;
}

return moduleNonNullable;
return false;
}

private NonNullabilityConventionState GetOrInitializeState(IConventionModelBuilder modelBuilder)
Expand All @@ -152,8 +144,7 @@ private class NonNullabilityConventionState
public Type NullableContextAttrType;
public FieldInfo NullableFlagsFieldInfo;
public FieldInfo NullableContextFlagFieldInfo;
public Dictionary<Type, bool> TypeNonNullabilityContextCache { get; } = new Dictionary<Type, bool>();
public Dictionary<Module, bool> ModuleNonNullabilityContextCache { get; } = new Dictionary<Module, bool>();
public Dictionary<Type, bool> TypeCache { get; } = new Dictionary<Type, bool>();
}
}
}
70 changes: 45 additions & 25 deletions test/EFCore.Tests/Metadata/Conventions/NonNullableReferencePropertyConventionTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System;
using System.ComponentModel.DataAnnotations;
using System.Diagnostics.CodeAnalysis;
using Microsoft.EntityFrameworkCore.Metadata.Builders;
using Microsoft.EntityFrameworkCore.Metadata.Conventions.Infrastructure;
using Microsoft.EntityFrameworkCore.Metadata.Conventions.Internal;
Expand Down Expand Up @@ -49,32 +50,35 @@ public void Non_nullability_sets_is_nullable_with_conventional_builder()
var modelBuilder = CreateModelBuilder();
var entityTypeBuilder = modelBuilder.Entity<A>();

Assert.False(entityTypeBuilder.Property(e => e.Name).Metadata.IsNullable);
Assert.False(entityTypeBuilder.Property(e => e.NonNullable).Metadata.IsNullable);
}

[ConditionalTheory]
[InlineData(nameof(A.NullAwareNonNullable), false)]
[InlineData(nameof(A.NullAwareNullable), true)]
[InlineData(nameof(A.NullObliviousNonNullable), true)]
[InlineData(nameof(A.NullObliviousNullable), true)]
[InlineData(nameof(A.RequiredAndNullable), false)]
public void Reference_nullability_sets_is_nullable_correctly1(string propertyName, bool expectedNullable)
[InlineData(typeof(A), nameof(A.NonNullable), false)]
[InlineData(typeof(A), nameof(A.Nullable), true)]

[InlineData(typeof(A), nameof(A.NonNullablePropertyMaybeNull), true)]
[InlineData(typeof(A), nameof(A.NonNullablePropertyAllowNull), false)]
[InlineData(typeof(A), nameof(A.NullablePropertyNotNull), true)]
[InlineData(typeof(A), nameof(A.NullablePropertyDisallowNull), true)]

[InlineData(typeof(A), nameof(A.NonNullableFieldMaybeNull), true)]
[InlineData(typeof(A), nameof(A.NonNullableFieldAllowNull), false)]
[InlineData(typeof(A), nameof(A.NullableFieldNotNull), true)]
[InlineData(typeof(A), nameof(A.NullableFieldDisallowNull), true)]

[InlineData(typeof(A), nameof(A.RequiredAndNullable), false)]
[InlineData(typeof(A), nameof(A.NullObliviousNonNullable), true)]
[InlineData(typeof(A), nameof(A.NullObliviousNullable), true)]

[InlineData(typeof(B), nameof(B.NonNullableValueType), false)]
[InlineData(typeof(B), nameof(B.NullableValueType), true)]
[InlineData(typeof(B), nameof(B.NonNullableRefType), false)]
[InlineData(typeof(B), nameof(B.NullableRefType), true)]
public void Reference_nullability_sets_is_nullable_correctly(Type type, string propertyName, bool expectedNullable)
{
var modelBuilder = CreateModelBuilder();
var entityTypeBuilder = modelBuilder.Entity<A>();

Assert.Equal(expectedNullable, entityTypeBuilder.Property(propertyName).Metadata.IsNullable);
}

[ConditionalTheory]
[InlineData(nameof(B.NonNullableValueType), false)]
[InlineData(nameof(B.NullableValueType), true)]
[InlineData(nameof(B.NonNullableRefType), false)]
[InlineData(nameof(B.NullableRefType), true)]
public void Reference_nullability_sets_is_nullable_correctly2(string propertyName, bool expectedNullable)
{
var modelBuilder = CreateModelBuilder();
var entityTypeBuilder = modelBuilder.Entity<B>();
var entityTypeBuilder = modelBuilder.Entity(type);

Assert.Equal(expectedNullable, entityTypeBuilder.Property(propertyName).Metadata.IsNullable);
}
Expand Down Expand Up @@ -107,10 +111,26 @@ private class A
public int Id { get; set; }

#nullable enable
public string Name { get; set; } = "";

public string NullAwareNonNullable { get; set; } = "";
public string? NullAwareNullable { get; set; }
public string NonNullable { get; set; } = "";
public string? Nullable { get; set; }

[MaybeNull]
public string NonNullablePropertyMaybeNull { get; set; } = "";
[AllowNull]
public string NonNullablePropertyAllowNull { get; set; } = "";
[NotNull]
public string? NullablePropertyNotNull { get; set; } = "";
[DisallowNull]
public string? NullablePropertyDisallowNull { get; set; } = "";

[MaybeNull]
public string NonNullableFieldMaybeNull = "";
[AllowNull]
public string NonNullableFieldAllowNull = "";
[NotNull]
public string? NullableFieldNotNull = "";
[DisallowNull]
public string? NullableFieldDisallowNull = "";

[Required]
public string? RequiredAndNullable { get; set; }
Expand Down