Annotate metadata for nullability, leaving only builders and conventions

src/EFCore.Cosmos/Metadata/Internal/CosmosPropertyExtensions.cs

@@ -29,11 +29,12 @@ public static bool IsOrdinalKeyProperty([NotNull] this IProperty property)
                 property.DeclaringEntityType.IsOwned(), $"Expected {property.DeclaringEntityType.DisplayName()} to be owned.");
             Check.DebugAssert(property.GetJsonPropertyName().Length == 0, $"Expected {property.Name} to be non-persisted.");
 
-            return property.IsPrimaryKey()
+            return property.FindContainingPrimaryKey() is IKey key
+                && key.Properties.Count > 1
                 && !property.IsForeignKey()
                 && property.ClrType == typeof(int)
                 && property.ValueGenerated == ValueGenerated.OnAdd
-                && property.DeclaringEntityType.FindPrimaryKey().Properties.Count > 1;
+                && property.FindContainingPrimaryKey()!.Properties.Count > 1;
         }
     }
 }

src/EFCore.Relational/Extensions/RelationalModelExtensions.cs

@@ -235,7 +235,7 @@ public static void SetMaxIdentifierLength([NotNull] this IMutableModel model, in
         /// <param name="name"> The sequence name. </param>
         /// <param name="schema"> The schema name, or <see langword="null" /> to use the default schema. </param>
         /// <returns> The sequence. </returns>
-        public static IMutableSequence AddSequence(
+        public static IMutableSequence? AddSequence(
             [NotNull] this IMutableModel model,
             [NotNull] string name,
             [CanBeNull] string? schema = null)
@@ -250,7 +250,7 @@ public static IMutableSequence AddSequence(
         /// <param name="schema"> The schema name, or <see langword="null" /> to use the default schema. </param>
         /// <param name="fromDataAnnotation"> Indicates whether the configuration was specified using a data annotation. </param>
         /// <returns> The sequence. </returns>
-        public static IConventionSequence AddSequence(
+        public static IConventionSequence? AddSequence(
             [NotNull] this IConventionModel model,
             [NotNull] string name,
             [CanBeNull] string? schema = null,
@@ -376,7 +376,7 @@ public static IEnumerable<IConventionSequence> GetSequences([NotNull] this IConv
         /// <param name="model"> The model to add the function to. </param>
         /// <param name="methodInfo"> The <see cref="MethodInfo" /> for the method that is mapped to the function. </param>
         /// <returns> The new <see cref="IMutableDbFunction" />. </returns>
-        public static IMutableDbFunction AddDbFunction([NotNull] this IMutableModel model, [NotNull] MethodInfo methodInfo)
+        public static IMutableDbFunction? AddDbFunction([NotNull] this IMutableModel model, [NotNull] MethodInfo methodInfo)
             => DbFunction.AddDbFunction(
                 model, Check.NotNull(methodInfo, nameof(methodInfo)), ConfigurationSource.Explicit);
 
@@ -387,7 +387,7 @@ public static IMutableDbFunction AddDbFunction([NotNull] this IMutableModel mode
         /// <param name="methodInfo"> The <see cref="MethodInfo" /> for the method that is mapped to the function. </param>
         /// <param name="fromDataAnnotation"> Indicates whether the configuration was specified using a data annotation. </param>
         /// <returns> The new <see cref="IConventionDbFunction" />. </returns>
-        public static IConventionDbFunction AddDbFunction(
+        public static IConventionDbFunction? AddDbFunction(
             [NotNull] this IConventionModel model,
             [NotNull] MethodInfo methodInfo,
             bool fromDataAnnotation = false)
@@ -402,7 +402,7 @@ public static IConventionDbFunction AddDbFunction(
         /// <param name="name"> The model name of the function. </param>
         /// <param name="returnType"> The function return type. </param>
         /// <returns> The new <see cref="IMutableDbFunction" />. </returns>
-        public static IMutableDbFunction AddDbFunction(
+        public static IMutableDbFunction? AddDbFunction(
             [NotNull] this IMutableModel model,
             [NotNull] string name,
             [NotNull] Type returnType)
@@ -417,7 +417,7 @@ public static IMutableDbFunction AddDbFunction(
         /// <param name="returnType"> The function return type. </param>
         /// <param name="fromDataAnnotation"> Indicates whether the configuration was specified using a data annotation. </param>
         /// <returns> The new <see cref="IConventionDbFunction" />. </returns>
-        public static IConventionDbFunction AddDbFunction(
+        public static IConventionDbFunction? AddDbFunction(
             [NotNull] this IConventionModel model,
             [NotNull] string name,
             [NotNull] Type returnType,

src/EFCore.Relational/Metadata/Internal/Column.cs

@@ -25,10 +25,9 @@ public class Column : ColumnBase, IColumn
         ///     any release. You should only use it directly in your code with extreme caution and knowing that
         ///     doing so can result in application failures when updating to a new Entity Framework Core release.
         /// </summary>
-        public Column([NotNull] string name, [CanBeNull] string? type, [NotNull] Table table)
+        public Column([NotNull] string name, [NotNull] string type, [NotNull] Table table)
             : base(name, type, table)
         {
-            // TODO-NULLABLE: Seems like an annotation mismatch. Is it really OK for type to be null, in which cases?
         }
 
         /// <inheritdoc />

src/EFCore.Relational/Metadata/Internal/ColumnMapping.cs

@@ -27,11 +27,10 @@ public class ColumnMapping : ColumnMappingBase, IColumnMapping
         public ColumnMapping(
             [NotNull] IProperty property,
             [NotNull] Column column,
-            [CanBeNull] RelationalTypeMapping? typeMapping,
+            [NotNull] RelationalTypeMapping typeMapping,
             [NotNull] TableMapping tableMapping)
             : base(property, column, typeMapping, tableMapping)
         {
-            // TODO-NULLABLE: As with type, nullability mismatch for typeMapping - what's the right annotation?
         }
 
         /// <inheritdoc />

src/EFCore.Relational/Metadata/Internal/DbFunction.cs

@@ -6,6 +6,7 @@
 using System.Diagnostics;
 using System.Linq;
 using System.Reflection;
+using System.Text;
 using JetBrains.Annotations;
 using Microsoft.EntityFrameworkCore.Diagnostics;
 using Microsoft.EntityFrameworkCore.Infrastructure;
@@ -127,12 +128,24 @@ public DbFunction(
         }
 
         private static string GetFunctionName(MethodInfo methodInfo, ParameterInfo[] parameters)
-            => methodInfo.DeclaringType!.FullName
-                + "."
-                + methodInfo.Name
-                + "("
-                + string.Join(",", parameters.Select(p => p.ParameterType.FullName))
-                + ")";
+        {
+            var builder = new StringBuilder();
+
+            if (methodInfo.DeclaringType != null)
+            {
+                builder
+                    .Append(methodInfo.DeclaringType.FullName)
+                    .Append(".");
+            }
+
+            builder
+                .Append(methodInfo.Name)
+                .Append('(')
+                .AppendJoin(',', parameters.Select(p => p.ParameterType.FullName))
+                .Append(')');
+
+            return builder.ToString();
+        }
 
         /// <inheritdoc />
         public virtual IMutableModel Model { get; }
@@ -186,7 +199,7 @@ public static IEnumerable<DbFunction> GetDbFunctions([NotNull] IModel model)
         ///     any release. You should only use it directly in your code with extreme caution and knowing that
         ///     doing so can result in application failures when updating to a new Entity Framework Core release.
         /// </summary>
-        public static DbFunction AddDbFunction(
+        public static DbFunction? AddDbFunction(
             [NotNull] IMutableModel model,
             [NotNull] MethodInfo methodInfo,
             ConfigurationSource configurationSource)
@@ -203,7 +216,7 @@ public static DbFunction AddDbFunction(
         ///     any release. You should only use it directly in your code with extreme caution and knowing that
         ///     doing so can result in application failures when updating to a new Entity Framework Core release.
         /// </summary>
-        public static DbFunction AddDbFunction(
+        public static DbFunction? AddDbFunction(
             [NotNull] IMutableModel model,
             [NotNull] string name,
             [NotNull] Type returnType,
@@ -579,10 +592,6 @@ public virtual Func<IReadOnlyCollection<SqlExpression>, SqlExpression>? Translat
         /// </summary>
         public virtual IStoreFunction? StoreFunction { get; [param: NotNull] set; }
 
-        // Relational model creation ensures StoreFunction is populated
-        IStoreFunction IDbFunction.StoreFunction
-            => StoreFunction!;
-
         /// <summary>
         ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
         ///     the same compatibility standards as public APIs. It may be changed or removed without notice in
@@ -691,5 +700,9 @@ bool IConventionDbFunction.SetIsNullable(bool nullable, bool fromDataAnnotation)
             Func<IReadOnlyCollection<SqlExpression>, SqlExpression>? translation,
             bool fromDataAnnotation)
             => SetTranslation(translation, fromDataAnnotation ? ConfigurationSource.DataAnnotation : ConfigurationSource.Convention);
+
+        /// <inheritdoc />
+        IStoreFunction IDbFunction.StoreFunction
+            => StoreFunction!; // Relational model creation ensures StoreFunction is populated
     }
 }

src/EFCore.Relational/Metadata/Internal/DbFunctionParameter.cs

@@ -117,10 +117,6 @@ public virtual string? StoreType
             set => SetStoreType(value, ConfigurationSource.Explicit);
         }
 
-        // Model validation ensures all parameters have a type mapping
-        string IDbFunctionParameter.StoreType
-            => StoreType!;
-
         /// <summary>
         ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
         ///     the same compatibility standards as public APIs. It may be changed or removed without notice in
@@ -233,12 +229,7 @@ public virtual bool SetPropagatesNullability(bool propagatesNullability, Configu
             => _typeMappingConfigurationSource;
 
         /// <inheritdoc />
-        [DebuggerStepThrough]
-        RelationalTypeMapping? IConventionDbFunctionParameter.SetTypeMapping(RelationalTypeMapping? typeMapping, bool fromDataAnnotation)
-            => SetTypeMapping(typeMapping, fromDataAnnotation ? ConfigurationSource.DataAnnotation : ConfigurationSource.Convention);
-
-        /// <inheritdoc />
-        public virtual IStoreFunctionParameter? StoreFunctionParameter { get; [param: NotNull] set; }
+        public virtual IStoreFunctionParameter StoreFunctionParameter { get; [param: NotNull] set; } = default!;
 
         /// <summary>
         ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
@@ -248,5 +239,14 @@ public virtual bool SetPropagatesNullability(bool propagatesNullability, Configu
         /// </summary>
         public override string ToString()
             => this.ToDebugString(MetadataDebugStringOptions.SingleLineDefault);
+
+        /// <inheritdoc />
+        [DebuggerStepThrough]
+        RelationalTypeMapping? IConventionDbFunctionParameter.SetTypeMapping(RelationalTypeMapping? typeMapping, bool fromDataAnnotation)
+            => SetTypeMapping(typeMapping, fromDataAnnotation ? ConfigurationSource.DataAnnotation : ConfigurationSource.Convention);
+
+        /// <inheritdoc />
+        string IDbFunctionParameter.StoreType
+            => StoreType!; // Model validation ensures all parameters have a type mapping
     }
 }

src/EFCore.Relational/Metadata/Internal/RelationalModel.cs

@@ -844,10 +844,10 @@ private static void PopulateConstraints(Table table)
                             break;
                         }
 
-                        // TODO-NULLABLE: Can there be a keyless entity here?
                         if (entityTypeMapping.IncludesDerivedTypes
                             && foreignKey.DeclaringEntityType != entityType
-                            && foreignKey.Properties.SequenceEqual(entityType.FindPrimaryKey().Properties))
+                            && entityType.FindPrimaryKey() is IConventionKey primaryKey
+                            && foreignKey.Properties.SequenceEqual(primaryKey.Properties))
                         {
                             // The identifying FK constraint is needed to be created only on the table that corresponds
                             // to the declaring entity type

src/EFCore.Relational/Metadata/Internal/Sequence.cs

@@ -108,7 +108,7 @@ public Sequence(
             Name = name;
             _schema = schema;
             _configurationSource = configurationSource;
-            Builder = new InternalSequenceBuilder(this, ((IConventionModel)model).Builder);
+            Builder = new InternalSequenceBuilder(this, ((IConventionModel)model).Builder!);
         }
 
         /// <summary>
@@ -173,7 +173,7 @@ public static IEnumerable<Sequence> GetSequences([NotNull] IModel model)
         ///     any release. You should only use it directly in your code with extreme caution and knowing that
         ///     doing so can result in application failures when updating to a new Entity Framework Core release.
         /// </summary>
-        public static Sequence AddSequence(
+        public static Sequence? AddSequence(
             [NotNull] IMutableModel model,
             [NotNull] string name,
             [CanBeNull] string? schema,

src/EFCore.Relational/Metadata/Internal/TableIndex.cs

@@ -27,7 +27,7 @@ public TableIndex(
             [NotNull] string name,
             [NotNull] Table table,
             [NotNull] IReadOnlyList<Column> columns,
-            [CanBeNull] string filter,
+            [CanBeNull] string? filter,
             bool unique)
         {
             Name = name;

src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs

@@ -1333,9 +1333,7 @@ protected override Expression VisitExtension(Expression extensionExpression)
 
                 var navigation = member.MemberInfo != null
                     ? entityType.FindNavigation(member.MemberInfo)
-                    : member.Name is not null
-                        ? entityType.FindNavigation(member.Name)
-                        : null;
+                    : entityType.FindNavigation(member.Name!);
 
                 if (navigation == null)
                 {

src/EFCore.Relational/Query/RelationalSqlTranslatingExpressionVisitor.cs

@@ -1077,9 +1077,7 @@ protected override Expression VisitUnary(UnaryExpression unaryExpression)
             var entityType = entityReferenceExpression.EntityType;
             var property = member.MemberInfo != null
                 ? entityType.FindProperty(member.MemberInfo)
-                : member.Name is not null
-                    ? entityType.FindProperty(member.Name)
-                    : null;
+                : entityType.FindProperty(member.Name!);
 
             if (property != null)
             {
@@ -1414,10 +1412,12 @@ private Expression CreatePropertyAccessExpression(Expression target, IProperty p
         {
             switch (target)
             {
-                // TODO-NULLABLE: Smit, take a look
                 case SqlConstantExpression sqlConstantExpression:
                     return Expression.Constant(
-                        property.GetGetter().GetClrValue(sqlConstantExpression.Value), property.ClrType.MakeNullable());
+                        sqlConstantExpression.Value is null
+                            ? null
+                            : property.GetGetter().GetClrValue(sqlConstantExpression.Value),
+                        property.ClrType.MakeNullable());
 
                 case SqlParameterExpression sqlParameterExpression
                     when sqlParameterExpression.Name.StartsWith(QueryCompilationContext.QueryParameterPrefix, StringComparison.Ordinal):

src/EFCore/Metadata/EntityTypeFullNameComparer.cs

@@ -110,15 +110,13 @@ public int GetHashCode(IEntityType obj)
             while (true)
             {
                 hash.Add(obj.Name, StringComparer.Ordinal);
-                var definingNavigationName = obj.DefiningNavigationName;
-                if (definingNavigationName == null)
+                if (!obj.HasDefiningNavigation())
                 {
                     return hash.ToHashCode();
                 }
 
-                hash.Add(definingNavigationName, StringComparer.Ordinal);
-                // TODO-NULLABLE: Put MemberNotNull on HasDefiningNavigation when we target net5.0
-                obj = obj.DefiningEntityType!;
+                hash.Add(obj.DefiningNavigationName, StringComparer.Ordinal);
+                obj = obj.DefiningEntityType;
             }
         }
     }

src/EFCore/Metadata/MemberIdentity.cs

@@ -100,8 +100,7 @@ public override bool Equals(object? obj)
 
         /// <inheritdoc />
         public bool Equals(MemberIdentity other)
-        // TODO-NULLABLE: Bangs can be removed when targeting net5.0
-            => EqualityComparer<object>.Default.Equals(_nameOrMember!, other._nameOrMember!);
+            => EqualityComparer<object>.Default.Equals(_nameOrMember, other._nameOrMember);
 
         /// <inheritdoc />
         public override int GetHashCode()

src/EFCore/Metadata/ParameterBindingInfo.cs

@@ -22,10 +22,10 @@ public readonly struct ParameterBindingInfo
         /// <param name="materializationContextExpression"> The expression tree from which the parameter value will come. </param>
         public ParameterBindingInfo(
             [NotNull] IEntityType entityType,
-            // TODO-NULLABLE: Shouldn't this be [NotNull]?
-            [CanBeNull] Expression materializationContextExpression)
+            [NotNull] Expression materializationContextExpression)
         {
             Check.NotNull(entityType, nameof(entityType));
+            Check.NotNull(entityType, nameof(materializationContextExpression));
 
             EntityType = entityType;
             MaterializationContextExpression = materializationContextExpression;

src/EFCore/Query/Internal/NavigationExpandingExpressionVisitor.ExpressionVisitors.cs

@@ -124,9 +124,7 @@ protected override Expression VisitMethodCall(MethodCallExpression methodCallExp
 
                     var navigation = memberIdentity.MemberInfo != null
                         ? entityType.FindNavigation(memberIdentity.MemberInfo)
-                        : memberIdentity.Name is not null
-                            ? entityType.FindNavigation(memberIdentity.Name)
-                            : null;
+                        : entityType.FindNavigation(memberIdentity.Name!);
                     if (navigation != null)
                     {
                         return ExpandNavigation(root, entityReference, navigation, convertedType != null);