diff --git a/src/EFCore/Metadata/Conventions/NonNullableConventionBase.cs b/src/EFCore/Metadata/Conventions/NonNullableConventionBase.cs index b45f0f1d0ee..80055aa0fdf 100644 --- a/src/EFCore/Metadata/Conventions/NonNullableConventionBase.cs +++ b/src/EFCore/Metadata/Conventions/NonNullableConventionBase.cs @@ -1,8 +1,6 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System.Diagnostics.CodeAnalysis; - namespace Microsoft.EntityFrameworkCore.Metadata.Conventions; /// @@ -14,12 +12,7 @@ namespace Microsoft.EntityFrameworkCore.Metadata.Conventions; /// public abstract class NonNullableConventionBase : IModelFinalizingConvention { - // For the interpretation of nullability metadata, see - // https://github.com/dotnet/roslyn/blob/master/docs/features/nullable-metadata.md - private const string StateAnnotationName = "NonNullableConventionState"; - private const string NullableAttributeFullName = "System.Runtime.CompilerServices.NullableAttribute"; - private const string NullableContextAttributeFullName = "System.Runtime.CompilerServices.NullableContextAttribute"; /// /// Creates a new instance of . @@ -50,118 +43,25 @@ protected virtual bool IsNonNullableReferenceType( return false; } - var state = GetOrInitializeState(modelBuilder); - - // First check for [MaybeNull] on the return value. If it exists, the member is nullable. - // Note: avoid using GetCustomAttribute<> below because of https://github.com/mono/mono/issues/17477 - var isMaybeNull = memberInfo switch - { - FieldInfo f - => f.CustomAttributes.Any(a => a.AttributeType == typeof(MaybeNullAttribute)), - PropertyInfo p - => p.GetMethod?.ReturnParameter?.CustomAttributes?.Any(a => a.AttributeType == typeof(MaybeNullAttribute)) == true, - _ => false - }; - - if (isMaybeNull) - { - return false; - } - - // For C# 8.0 nullable types, the C# compiler 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) - { - var attributeType = attribute.GetType(); - - if (attributeType != state.NullableAttrType) - { - state.NullableFlagsFieldInfo = attributeType.GetField("NullableFlags"); - state.NullableAttrType = attributeType; - } - - if (state.NullableFlagsFieldInfo?.GetValue(attribute) is byte[] flags) - { - return flags.FirstOrDefault() == 1; - } - } - - // No attribute on the member, try to find a NullableContextAttribute on the declaring type - var type = memberInfo.DeclaringType; - if (type is not null) - { - // We currently don't calculate support nullability for generic properties, since calculating that is complex - // (depends on the nullability of generic type argument). - // However, we special case Dictionary as it's used for property bags, and specifically don't identify its indexer - // as non-nullable. - if (memberInfo is PropertyInfo property - && property.IsIndexerProperty() - && type.IsGenericType - && type.GetGenericTypeDefinition() == typeof(Dictionary<,>)) - { - return false; - } - - return DoesTypeHaveNonNullableContext(type, state); - } - - return false; - } - - private static bool DoesTypeHaveNonNullableContext(Type type, NonNullabilityConventionState state) - { - if (state.TypeCache.TryGetValue(type, out var cachedTypeNonNullable)) - { - return cachedTypeNonNullable; - } - - if (Attribute.GetCustomAttributes(type) - .FirstOrDefault(a => a.GetType().FullName == NullableContextAttributeFullName) is Attribute contextAttr) - { - var attributeType = contextAttr.GetType(); + var annotation = + modelBuilder.Metadata.FindAnnotation(StateAnnotationName) + ?? modelBuilder.Metadata.AddAnnotation(StateAnnotationName, new NullabilityInfoContext()); - if (attributeType != state.NullableContextAttrType) - { - state.NullableContextFlagFieldInfo = attributeType.GetField("Flag"); - state.NullableContextAttrType = attributeType; - } + var nullabilityInfoContext = (NullabilityInfoContext)annotation.Value!; - if (state.NullableContextFlagFieldInfo?.GetValue(contextAttr) is byte flag) - { - return state.TypeCache[type] = flag == 1; - } - } - else if (type.IsNested) + var nullabilityInfo = memberInfo switch { - return state.TypeCache[type] = DoesTypeHaveNonNullableContext(type.DeclaringType!, state); - } + PropertyInfo propertyInfo => nullabilityInfoContext.Create(propertyInfo), + FieldInfo fieldInfo => nullabilityInfoContext.Create(fieldInfo), + _ => null + }; - return state.TypeCache[type] = false; + return nullabilityInfo?.ReadState == NullabilityState.NotNull; } - private static NonNullabilityConventionState GetOrInitializeState(IConventionModelBuilder modelBuilder) - => (NonNullabilityConventionState)( - modelBuilder.Metadata.FindAnnotation(StateAnnotationName) - ?? modelBuilder.Metadata.AddAnnotation(StateAnnotationName, new NonNullabilityConventionState()) - ).Value!; - /// public virtual void ProcessModelFinalizing( IConventionModelBuilder modelBuilder, IConventionContext context) => modelBuilder.Metadata.RemoveAnnotation(StateAnnotationName); - - private sealed class NonNullabilityConventionState - { - public Type? NullableAttrType; - public Type? NullableContextAttrType; - public FieldInfo? NullableFlagsFieldInfo; - public FieldInfo? NullableContextFlagFieldInfo; - public Dictionary TypeCache { get; } = new(); - } } diff --git a/src/EFCore/Metadata/Conventions/NonNullableReferencePropertyConvention.cs b/src/EFCore/Metadata/Conventions/NonNullableReferencePropertyConvention.cs index 0574f9a61e3..77e7d51b36c 100644 --- a/src/EFCore/Metadata/Conventions/NonNullableReferencePropertyConvention.cs +++ b/src/EFCore/Metadata/Conventions/NonNullableReferencePropertyConvention.cs @@ -26,8 +26,6 @@ public NonNullableReferencePropertyConvention(ProviderConventionSetBuilderDepend private void Process(IConventionPropertyBuilder propertyBuilder) { - // If the model is spread across multiple assemblies, it may contain different NullableAttribute types as - // the compiler synthesizes them for each assembly. if (propertyBuilder.Metadata.GetIdentifyingMemberInfo() is MemberInfo memberInfo && IsNonNullableReferenceType(propertyBuilder.ModelBuilder, memberInfo)) { diff --git a/test/EFCore.Tests/Metadata/Conventions/NonNullableNavigationConventionTest.cs b/test/EFCore.Tests/Metadata/Conventions/NonNullableNavigationConventionTest.cs index bd450fbbc90..7cf261b98a4 100644 --- a/test/EFCore.Tests/Metadata/Conventions/NonNullableNavigationConventionTest.cs +++ b/test/EFCore.Tests/Metadata/Conventions/NonNullableNavigationConventionTest.cs @@ -8,21 +8,23 @@ namespace Microsoft.EntityFrameworkCore.Metadata.Conventions; +#nullable enable + public class NonNullableNavigationConventionTest { [ConditionalFact] public void Non_nullability_does_not_override_configuration_from_explicit_source() { var dependentEntityTypeBuilder = CreateInternalEntityTypeBuilder(); - var principalEntityTypeBuilder = dependentEntityTypeBuilder.ModelBuilder.Entity(typeof(Blog), ConfigurationSource.Convention); + var principalEntityTypeBuilder = dependentEntityTypeBuilder.ModelBuilder.Entity(typeof(Blog), ConfigurationSource.Convention)!; var relationshipBuilder = dependentEntityTypeBuilder.HasRelationship( principalEntityTypeBuilder.Metadata, nameof(Post.Blog), nameof(Blog.Posts), - ConfigurationSource.Convention); + ConfigurationSource.Convention)!; - var navigation = dependentEntityTypeBuilder.Metadata.FindNavigation(nameof(Post.Blog)); + var navigation = dependentEntityTypeBuilder.Metadata.FindNavigation(nameof(Post.Blog))!; relationshipBuilder.IsRequired(false, ConfigurationSource.Explicit); @@ -39,15 +41,15 @@ public void Non_nullability_does_not_override_configuration_from_explicit_source public void Non_nullability_does_not_override_configuration_from_data_annotation() { var dependentEntityTypeBuilder = CreateInternalEntityTypeBuilder(); - var principalEntityTypeBuilder = dependentEntityTypeBuilder.ModelBuilder.Entity(typeof(Blog), ConfigurationSource.Convention); + var principalEntityTypeBuilder = dependentEntityTypeBuilder.ModelBuilder.Entity(typeof(Blog), ConfigurationSource.Convention)!; var relationshipBuilder = dependentEntityTypeBuilder.HasRelationship( principalEntityTypeBuilder.Metadata, nameof(Post.Blog), nameof(Blog.Posts), - ConfigurationSource.Convention); + ConfigurationSource.Convention)!; - var navigation = dependentEntityTypeBuilder.Metadata.FindNavigation(nameof(Post.Blog)); + var navigation = dependentEntityTypeBuilder.Metadata.FindNavigation(nameof(Post.Blog))!; relationshipBuilder.IsRequired(false, ConfigurationSource.DataAnnotation); @@ -65,15 +67,15 @@ public void Non_nullability_does_not_set_is_required_for_collection_navigation() { var dependentEntityTypeBuilder = CreateInternalEntityTypeBuilder(); var principalEntityTypeBuilder = - dependentEntityTypeBuilder.ModelBuilder.Entity(typeof(Principal), ConfigurationSource.Convention); + dependentEntityTypeBuilder.ModelBuilder.Entity(typeof(Principal), ConfigurationSource.Convention)!; var relationshipBuilder = principalEntityTypeBuilder.HasRelationship( dependentEntityTypeBuilder.Metadata, nameof(Principal.Dependents), nameof(Dependent.Principal), - ConfigurationSource.Convention); + ConfigurationSource.Convention)!; - var navigation = principalEntityTypeBuilder.Metadata.FindNavigation(nameof(Principal.Dependents)); + var navigation = principalEntityTypeBuilder.Metadata.FindNavigation(nameof(Principal.Dependents))!; Assert.False(relationshipBuilder.Metadata.IsRequired); @@ -89,17 +91,17 @@ public void Non_nullability_does_not_set_is_required_for_navigation_to_dependent { var dependentEntityTypeBuilder = CreateInternalEntityTypeBuilder(); var principalEntityTypeBuilder = - dependentEntityTypeBuilder.ModelBuilder.Entity(typeof(Principal), ConfigurationSource.Convention); + dependentEntityTypeBuilder.ModelBuilder.Entity(typeof(Principal), ConfigurationSource.Convention)!; var relationshipBuilder = dependentEntityTypeBuilder.HasRelationship( principalEntityTypeBuilder.Metadata, nameof(Dependent.Principal), nameof(Principal.Dependent), - ConfigurationSource.Convention) + ConfigurationSource.Convention)! .HasEntityTypes - (principalEntityTypeBuilder.Metadata, dependentEntityTypeBuilder.Metadata, ConfigurationSource.Explicit); + (principalEntityTypeBuilder.Metadata, dependentEntityTypeBuilder.Metadata, ConfigurationSource.Explicit)!; - var navigation = principalEntityTypeBuilder.Metadata.FindNavigation(nameof(Principal.Dependent)); + var navigation = principalEntityTypeBuilder.Metadata.FindNavigation(nameof(Principal.Dependent))!; Assert.False(relationshipBuilder.Metadata.IsRequired); @@ -116,7 +118,7 @@ public void Non_nullability_sets_is_required_with_conventional_builder() modelBuilder.Entity(); Assert.True( - model.FindEntityType(typeof(BlogDetails)).GetForeignKeys().Single(fk => fk.PrincipalEntityType?.ClrType == typeof(Blog)) + model.FindEntityType(typeof(BlogDetails))!.GetForeignKeys().Single(fk => fk.PrincipalEntityType?.ClrType == typeof(Blog)) .IsRequired); } @@ -125,7 +127,7 @@ private Navigation RunConvention(InternalForeignKeyBuilder relationshipBuilder, var context = new ConventionContext( relationshipBuilder.Metadata.DeclaringEntityType.Model.ConventionDispatcher); CreateNotNullNavigationConvention().ProcessNavigationAdded(navigation.Builder, context); - return context.ShouldStopProcessing() ? (Navigation)context.Result?.Metadata : navigation; + return context.ShouldStopProcessing() ? (Navigation)context.Result?.Metadata! : navigation; } private NonNullableNavigationConvention CreateNotNullNavigationConvention() @@ -146,15 +148,15 @@ private InternalEntityTypeBuilder CreateInternalEntityTypeBuilder() var modelBuilder = new Model(conventionSet).Builder; - return modelBuilder.Entity(typeof(T), ConfigurationSource.Explicit); + return modelBuilder.Entity(typeof(T), ConfigurationSource.Explicit)!; } private ModelBuilder CreateModelBuilder() { var serviceProvider = CreateServiceProvider(); return new ModelBuilder( - serviceProvider.GetService().CreateConventionSet(), - serviceProvider.GetService()); + serviceProvider.GetRequiredService().CreateConventionSet(), + serviceProvider.GetRequiredService()); } private ProviderConventionSetBuilderDependencies CreateDependencies() @@ -179,9 +181,9 @@ protected IServiceProvider CreateServiceProvider() return modelLogger; } -#nullable enable #pragma warning disable CS8618 - + // ReSharper disable UnusedMember.Local + // ReSharper disable ClassNeverInstantiated.Local private class Blog { public int Id { get; set; } @@ -245,6 +247,7 @@ private class Dependent [ForeignKey("PrincipalId, PrincipalFk")] public Principal? CompositePrincipal { get; set; } } + // ReSharper restore ClassNeverInstantiated.Local + // ReSharper restore UnusedMember.Local #pragma warning restore CS8618 -#nullable disable } diff --git a/test/EFCore.Tests/Metadata/Conventions/NonNullableReferencePropertyConventionTest.cs b/test/EFCore.Tests/Metadata/Conventions/NonNullableReferencePropertyConventionTest.cs index df1d7bc8d39..a8ae62337a0 100644 --- a/test/EFCore.Tests/Metadata/Conventions/NonNullableReferencePropertyConventionTest.cs +++ b/test/EFCore.Tests/Metadata/Conventions/NonNullableReferencePropertyConventionTest.cs @@ -8,6 +8,8 @@ namespace Microsoft.EntityFrameworkCore.Metadata.Conventions; +#nullable enable + public class NonNullableReferencePropertyConventionTest { [ConditionalFact] @@ -15,7 +17,7 @@ public void Non_nullability_does_not_override_configuration_from_explicit_source { var entityTypeBuilder = CreateInternalEntityTypeBuilder(); - var propertyBuilder = entityTypeBuilder.Property(typeof(string), "Name", ConfigurationSource.Explicit); + var propertyBuilder = entityTypeBuilder.Property(typeof(string), "Name", ConfigurationSource.Explicit)!; propertyBuilder.IsRequired(false, ConfigurationSource.Explicit); @@ -29,7 +31,7 @@ public void Non_nullability_does_not_override_configuration_from_data_annotation { var entityTypeBuilder = CreateInternalEntityTypeBuilder(); - var propertyBuilder = entityTypeBuilder.Property(typeof(string), "Name", ConfigurationSource.Explicit); + var propertyBuilder = entityTypeBuilder.Property(typeof(string), "Name", ConfigurationSource.Explicit)!; propertyBuilder.IsRequired(false, ConfigurationSource.DataAnnotation); @@ -77,7 +79,7 @@ public void Reference_nullability_sets_is_nullable_correctly(Type type, string p } [ConditionalFact] - public void Non_nullability_ignores_context_on_generic_types() + public void Dictionary_indexer_is_not_configured_as_non_nullable() { var modelBuilder = CreateModelBuilder(); var entityTypeBuilder = modelBuilder.SharedTypeEntity>("SomeBag"); @@ -102,30 +104,31 @@ private InternalEntityTypeBuilder CreateInternalEntityTypeBuilder() var modelBuilder = new InternalModelBuilder(new Model(conventionSet)); - return modelBuilder.Entity(typeof(T), ConfigurationSource.Explicit); + return modelBuilder.Entity(typeof(T), ConfigurationSource.Explicit)!; } private ProviderConventionSetBuilderDependencies CreateDependencies() => InMemoryTestHelpers.Instance.CreateContextServices().GetRequiredService(); + // ReSharper disable PropertyCanBeMadeInitOnly.Local private class A { + // ReSharper disable once UnusedMember.Local public int Id { get; set; } -#nullable enable - public string NonNullable { get; } = ""; + public string NonNullable => ""; public string? Nullable { get; set; } [MaybeNull] - public string NonNullablePropertyMaybeNull { get; } = ""; + public string NonNullablePropertyMaybeNull { get; set; } = ""; [AllowNull] - public string NonNullablePropertyAllowNull { get; } = ""; + public string NonNullablePropertyAllowNull { get; set; } = ""; - public string? NullablePropertyNotNull { get; } = ""; + public string? NullablePropertyNotNull { get; set; } = ""; [DisallowNull] - public string? NullablePropertyDisallowNull { get; } = ""; + public string? NullablePropertyDisallowNull { get; set; } = ""; [MaybeNull] public string NonNullableFieldMaybeNull = ""; @@ -140,15 +143,15 @@ private class A [Required] public string? RequiredAndNullable { get; set; } -#nullable disable +#nullable disable #pragma warning disable CS8632 // The annotation for nullable reference types should only be used in code within a '#nullable' context. public string NullObliviousNonNullable { get; set; } public string? NullObliviousNullable { get; set; } #pragma warning restore CS8632 // The annotation for nullable reference types should only be used in code within a '#nullable' context. +#nullable enable } -#nullable enable public class B { [Key] @@ -168,7 +171,7 @@ public class BaseClass { public string? Nullable { get; set; } } -#nullable disable + // ReSharper restore PropertyCanBeMadeInitOnly.Local private static ModelBuilder CreateModelBuilder() => InMemoryTestHelpers.Instance.CreateConventionBuilder();