Annotate most of the Cosmos query pipeline for NRTs

src/EFCore.Cosmos/Query/Internal/CosmosRandomTranslator.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.
 
-#nullable disable
-
 namespace Microsoft.EntityFrameworkCore.Cosmos.Query.Internal;
 
 /// <summary>
@@ -14,7 +12,7 @@ namespace Microsoft.EntityFrameworkCore.Cosmos.Query.Internal;
 public class CosmosRandomTranslator : IMethodCallTranslator
 {
     private static readonly MethodInfo MethodInfo = typeof(DbFunctionsExtensions).GetRuntimeMethod(
-        nameof(DbFunctionsExtensions.Random), [typeof(DbFunctions)]);
+        nameof(DbFunctionsExtensions.Random), [typeof(DbFunctions)])!;
 
     private readonly ISqlExpressionFactory _sqlExpressionFactory;
 
@@ -35,8 +33,8 @@ public CosmosRandomTranslator(ISqlExpressionFactory sqlExpressionFactory)
     ///     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 virtual SqlExpression Translate(
-        SqlExpression instance,
+    public virtual SqlExpression? Translate(
+        SqlExpression? instance,
         MethodInfo method,
         IReadOnlyList<SqlExpression> arguments,
         IDiagnosticsLogger<DbLoggerCategory.Query> logger)

src/EFCore.Cosmos/Query/Internal/CosmosValueConverterCompensatingExpressionVisitor.cs

@@ -1,6 +1,8 @@
 // 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.Cosmos.Query.Internal;
 
 /// <summary>
@@ -59,7 +61,7 @@ private Expression VisitSelect(SelectExpression selectExpression)
         var fromExpression = (RootReferenceExpression)Visit(selectExpression.FromExpression);
         changed |= fromExpression != selectExpression.FromExpression;
 
-        var predicate = TryCompensateForBoolWithValueConverter((SqlExpression)Visit(selectExpression.Predicate));
+        var predicate = TryCompensateForBoolWithValueConverter((SqlExpression?)Visit(selectExpression.Predicate));
         changed |= predicate != selectExpression.Predicate;
 
         var orderings = new List<OrderingExpression>();
@@ -70,8 +72,8 @@ private Expression VisitSelect(SelectExpression selectExpression)
             orderings.Add(ordering.Update(orderingExpression));
         }
 
-        var limit = (SqlExpression)Visit(selectExpression.Limit);
-        var offset = (SqlExpression)Visit(selectExpression.Offset);
+        var limit = (SqlExpression?)Visit(selectExpression.Limit);
+        var offset = (SqlExpression?)Visit(selectExpression.Offset);
 
         return changed
             ? selectExpression.Update(projections, fromExpression, predicate, orderings, limit, offset)
@@ -87,30 +89,22 @@ private Expression VisitSqlConditional(SqlConditionalExpression sqlConditionalEx
         return sqlConditionalExpression.Update(test, ifTrue, ifFalse);
     }
 
-    private SqlExpression TryCompensateForBoolWithValueConverter(SqlExpression sqlExpression)
-    {
-        if (sqlExpression is KeyAccessExpression keyAccessExpression
-            && keyAccessExpression.TypeMapping!.ClrType == typeof(bool)
-            && keyAccessExpression.TypeMapping!.Converter != null)
+    [return: NotNullIfNotNull(nameof(sqlExpression))]
+    private SqlExpression? TryCompensateForBoolWithValueConverter(SqlExpression? sqlExpression)
+        => sqlExpression switch
         {
-            return _sqlExpressionFactory.Equal(
-                sqlExpression,
-                _sqlExpressionFactory.Constant(true, sqlExpression.TypeMapping));
-        }
+            KeyAccessExpression keyAccessExpression
+                when keyAccessExpression.TypeMapping!.ClrType == typeof(bool) && keyAccessExpression.TypeMapping!.Converter != null
+                => _sqlExpressionFactory.Equal(sqlExpression, _sqlExpressionFactory.Constant(true, sqlExpression.TypeMapping)),
 
-        if (sqlExpression is SqlUnaryExpression sqlUnaryExpression)
-        {
-            return sqlUnaryExpression.Update(
-                TryCompensateForBoolWithValueConverter(sqlUnaryExpression.Operand));
-        }
+            SqlUnaryExpression sqlUnaryExpression
+                => sqlUnaryExpression.Update(TryCompensateForBoolWithValueConverter(sqlUnaryExpression.Operand)),
 
-        if (sqlExpression is SqlBinaryExpression { OperatorType: ExpressionType.AndAlso or ExpressionType.OrElse } sqlBinaryExpression)
-        {
-            return sqlBinaryExpression.Update(
-                TryCompensateForBoolWithValueConverter(sqlBinaryExpression.Left),
-                TryCompensateForBoolWithValueConverter(sqlBinaryExpression.Right));
-        }
+            SqlBinaryExpression { OperatorType: ExpressionType.AndAlso or ExpressionType.OrElse } sqlBinaryExpression
+                => sqlBinaryExpression.Update(
+                    TryCompensateForBoolWithValueConverter(sqlBinaryExpression.Left),
+                    TryCompensateForBoolWithValueConverter(sqlBinaryExpression.Right)),
 
-        return sqlExpression;
-    }
+            _ => sqlExpression
+        };
 }

src/EFCore.Cosmos/Query/Internal/EntityProjectionExpression.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.
 
-#nullable disable
-
 using Microsoft.EntityFrameworkCore.Cosmos.Internal;
 
 namespace Microsoft.EntityFrameworkCore.Cosmos.Query.Internal;
@@ -71,7 +69,7 @@ public override Type Type
     ///     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 virtual string Name { get; }
+    public virtual string? Name { get; }
 
     /// <summary>
     ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
@@ -117,11 +115,12 @@ public virtual Expression BindProperty(IProperty property, bool clientEval)
         if (!clientEval
             // TODO: Remove once __jObject is translated to the access root in a better fashion and
             // would not otherwise be found to be non-translatable. See issues #17670 and #14121.
+            // TODO: We shouldn't be returning null from here
             && property.Name != StoreKeyConvention.JObjectPropertyName
-            && expression.Name.Length == 0)
+            && expression.Name?.Length is null or 0)
         {
             // Non-persisted property can't be translated
-            return null;
+            return null!;
         }
 
         return (Expression)expression;
@@ -154,10 +153,11 @@ public virtual Expression BindNavigation(INavigation navigation, bool clientEval
         }
 
         if (!clientEval
-            && expression.Name.Length == 0)
+            && expression.Name?.Length is null or 0)
         {
             // Non-persisted navigation can't be translated
-            return null;
+            // TODO: We shouldn't be returning null from here
+            return null!;
         }
 
         return (Expression)expression;
@@ -169,11 +169,11 @@ public virtual Expression BindNavigation(INavigation navigation, bool clientEval
     ///     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 virtual Expression BindMember(
+    public virtual Expression? BindMember(
         string name,
         Type entityType,
         bool clientEval,
-        out IPropertyBase propertyBase)
+        out IPropertyBase? propertyBase)
         => BindMember(MemberIdentity.Create(name), entityType, clientEval, out propertyBase);
 
     /// <summary>
@@ -182,14 +182,14 @@ public virtual Expression BindMember(
     ///     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 virtual Expression BindMember(
+    public virtual Expression? BindMember(
         MemberInfo memberInfo,
         Type entityType,
         bool clientEval,
-        out IPropertyBase propertyBase)
+        out IPropertyBase? propertyBase)
         => BindMember(MemberIdentity.Create(memberInfo), entityType, clientEval, out propertyBase);
 
-    private Expression BindMember(MemberIdentity member, Type entityClrType, bool clientEval, out IPropertyBase propertyBase)
+    private Expression? BindMember(MemberIdentity member, Type? entityClrType, bool clientEval, out IPropertyBase? propertyBase)
     {
         var entityType = EntityType;
         if (entityClrType != null
@@ -199,7 +199,7 @@ private Expression BindMember(MemberIdentity member, Type entityClrType, bool cl
         }
 
         var property = member.MemberInfo == null
-            ? entityType.FindProperty(member.Name)
+            ? entityType.FindProperty(member.Name!)
             : entityType.FindProperty(member.MemberInfo);
         if (property != null)
         {
@@ -208,7 +208,7 @@ private Expression BindMember(MemberIdentity member, Type entityClrType, bool cl
         }
 
         var navigation = member.MemberInfo == null
-            ? entityType.FindNavigation(member.Name)
+            ? entityType.FindNavigation(member.Name!)
             : entityType.FindNavigation(member.MemberInfo);
         if (navigation != null)
         {
@@ -254,7 +254,7 @@ void IPrintableExpression.Print(ExpressionPrinter expressionPrinter)
     ///     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 override bool Equals(object obj)
+    public override bool Equals(object? obj)
         => obj != null
             && (ReferenceEquals(this, obj)
                 || obj is EntityProjectionExpression entityProjectionExpression

src/EFCore.Cosmos/Query/Internal/FromSqlExpression.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.
 
-#nullable disable
-
 namespace Microsoft.EntityFrameworkCore.Cosmos.Query.Internal;
 
 /// <summary>
@@ -70,7 +68,7 @@ void IPrintableExpression.Print(ExpressionPrinter expressionPrinter)
         => expressionPrinter.Append(Sql);
 
     /// <inheritdoc />
-    public override bool Equals(object obj)
+    public override bool Equals(object? obj)
         => obj != null
             && (ReferenceEquals(this, obj)
                 || obj is FromSqlExpression fromSqlExpression

src/EFCore.Cosmos/Query/Internal/IAccessExpression.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.
 
-#nullable disable
-
 namespace Microsoft.EntityFrameworkCore.Cosmos.Query.Internal;
 
 /// <summary>
@@ -19,5 +17,5 @@ public interface IAccessExpression
     ///     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>
-    string Name { get; }
+    string? Name { get; }
 }

src/EFCore.Cosmos/Query/Internal/ISqlExpressionFactory.cs

@@ -19,7 +19,7 @@ public interface ISqlExpressionFactory
     ///     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>
-    [return: NotNullIfNotNull("sqlExpression")]
+    [return: NotNullIfNotNull(nameof(sqlExpression))]
     SqlExpression? ApplyTypeMapping(SqlExpression? sqlExpression, CoreTypeMapping? typeMapping);
 
     /// <summary>
@@ -28,7 +28,7 @@ public interface ISqlExpressionFactory
     ///     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>
-    [return: NotNullIfNotNull("sqlExpression")]
+    [return: NotNullIfNotNull(nameof(sqlExpression))]
     SqlExpression? ApplyDefaultTypeMapping(SqlExpression? sqlExpression);
 
     /// <summary>
@@ -37,7 +37,7 @@ public interface ISqlExpressionFactory
     ///     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>
-    SqlBinaryExpression MakeBinary(
+    SqlBinaryExpression? MakeBinary(
         ExpressionType operatorType,
         SqlExpression left,
         SqlExpression right,

src/EFCore.Cosmos/Query/Internal/KeyAccessExpression.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.
 
-#nullable disable
-
 namespace Microsoft.EntityFrameworkCore.Cosmos.Query.Internal;
 
 /// <summary>
@@ -41,9 +39,7 @@ public KeyAccessExpression(IProperty property, Expression accessExpression)
     ///     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>
-#pragma warning disable 109
     public new virtual IProperty Property { get; }
-#pragma warning restore 109
 
     /// <summary>
     ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
@@ -101,7 +97,7 @@ public override string ToString()
     ///     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 override bool Equals(object obj)
+    public override bool Equals(object? obj)
         => obj != null
             && (ReferenceEquals(this, obj)
                 || obj is KeyAccessExpression keyAccessExpression

src/EFCore.Cosmos/Query/Internal/ObjectAccessExpression.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.
 
-#nullable disable
-
 using Microsoft.EntityFrameworkCore.Cosmos.Internal;
 
 namespace Microsoft.EntityFrameworkCore.Cosmos.Query.Internal;
@@ -23,13 +21,10 @@ public class ObjectAccessExpression : Expression, IPrintableExpression, IAccessE
     /// </summary>
     public ObjectAccessExpression(INavigation navigation, Expression accessExpression)
     {
-        Name = navigation.TargetEntityType.GetContainingPropertyName();
-        if (Name == null)
-        {
-            throw new InvalidOperationException(
+        Name = navigation.TargetEntityType.GetContainingPropertyName()
+            ?? throw new InvalidOperationException(
                 CosmosStrings.NavigationPropertyIsNotAnEmbeddedEntity(
                     navigation.DeclaringEntityType.DisplayName(), navigation.Name));
-        }
 
         Navigation = navigation;
         AccessExpression = accessExpression;
@@ -121,7 +116,7 @@ public override string ToString()
     ///     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 override bool Equals(object obj)
+    public override bool Equals(object? obj)
         => obj != null
             && (ReferenceEquals(this, obj)
                 || obj is ObjectAccessExpression objectAccessExpression

src/EFCore.Cosmos/Query/Internal/ObjectArrayProjectionExpression.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.
 
-#nullable disable warnings
-
 using Microsoft.EntityFrameworkCore.Cosmos.Internal;
 
 namespace Microsoft.EntityFrameworkCore.Cosmos.Query.Internal;
@@ -29,13 +27,10 @@ public ObjectArrayProjectionExpression(
         var targetType = navigation.TargetEntityType;
         Type = typeof(IEnumerable<>).MakeGenericType(targetType.ClrType);
 
-        Name = targetType.GetContainingPropertyName();
-        if (Name == null)
-        {
-            throw new InvalidOperationException(
+        Name = targetType.GetContainingPropertyName()
+            ?? throw new InvalidOperationException(
                 CosmosStrings.NavigationPropertyIsNotAnEmbeddedEntity(
                     navigation.DeclaringEntityType.DisplayName(), navigation.Name));
-        }
 
         Navigation = navigation;
         AccessExpression = accessExpression;
@@ -145,7 +140,7 @@ public override string ToString()
     ///     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 override bool Equals(object obj)
+    public override bool Equals(object? obj)
         => obj != null
             && (ReferenceEquals(this, obj)
                 || obj is ObjectArrayProjectionExpression arrayProjectionExpression

src/EFCore.Cosmos/Query/Internal/ProjectionExpression.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.
 
-#nullable disable
-
 namespace Microsoft.EntityFrameworkCore.Cosmos.Query.Internal;
 
 /// <summary>
@@ -47,7 +45,7 @@ public ProjectionExpression(Expression expression, string alias)
     ///     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 virtual string Name
+    public virtual string? Name
         => (Expression as IAccessExpression)?.Name;
 
     /// <summary>
@@ -109,7 +107,7 @@ void IPrintableExpression.Print(ExpressionPrinter expressionPrinter)
     ///     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 override bool Equals(object obj)
+    public override bool Equals(object? obj)
         => obj != null
             && (ReferenceEquals(this, obj)
                 || obj is ProjectionExpression projectionExpression