Moved nullable check code into a separated class

Author: maca88

src/NHibernate/Linq/Visitors/HqlGeneratorExpressionVisitor.cs

@@ -1,20 +1,15 @@
 using System;
-using System.Collections.Generic;
 using System.Dynamic;
 using System.Linq;
 using System.Linq.Expressions;
 using System.Runtime.CompilerServices;
 using NHibernate.Engine.Query;
 using NHibernate.Hql.Ast;
-using NHibernate.Linq.Clauses;
 using NHibernate.Linq.Expressions;
 using NHibernate.Linq.Functions;
-using NHibernate.Mapping.ByCode;
 using NHibernate.Param;
 using NHibernate.Util;
-using Remotion.Linq.Clauses;
 using Remotion.Linq.Clauses.Expressions;
-using Remotion.Linq.Clauses.ResultOperators;
 
 namespace NHibernate.Linq.Visitors
 {
@@ -23,17 +18,7 @@ public class HqlGeneratorExpressionVisitor : IHqlExpressionVisitor
 		private readonly HqlTreeBuilder _hqlTreeBuilder = new HqlTreeBuilder();
 		private readonly VisitorParameters _parameters;
 		private readonly ILinqToHqlGeneratorsRegistry _functionRegistry;
-		private readonly Dictionary<BinaryExpression, List<MemberExpression>> _equalityNotNullMembers =
-			new Dictionary<BinaryExpression, List<MemberExpression>>();
-
-		private static readonly HashSet<System.Type> NotNullOperators = new HashSet<System.Type>()
-		{
-			typeof(AllResultOperator),
-			typeof(AnyResultOperator),
-			typeof(ContainsResultOperator),
-			typeof(CountResultOperator),
-			typeof(LongCountResultOperator)
-		};
+		private readonly NullableExpressionDetector _nullableExpressionDetector;
 
 		public static HqlTreeNode Visit(Expression expression, VisitorParameters parameters)
 		{
@@ -44,6 +29,7 @@ public HqlGeneratorExpressionVisitor(VisitorParameters parameters)
 		{
 			_functionRegistry = parameters.SessionFactory.Settings.LinqToHqlGeneratorsRegistry;
 			_parameters = parameters;
+			_nullableExpressionDetector = new NullableExpressionDetector(_parameters.SessionFactory, _functionRegistry);
 		}
 
 
@@ -299,94 +285,6 @@ private HqlTreeNode VisitVBStringComparisonExpression(VBStringComparisonExpressi
 			return VisitExpression(expression.Comparison);
 		}
 
-		private void SearchForNotNullMembersCheck(BinaryExpression expression)
-		{
-			// Check for a member not null check that has a not equals expression
-			// Example: o.Status != null && o.Status != "New"
-			// Example: (o.Status != null && o.OldStatus != null) && (o.Status != o.OldStatus)
-			// Example: (o.Status != null && o.OldStatus != null) && (o.Status == o.OldStatus)
-			if (expression.NodeType != ExpressionType.AndAlso || 
-			    expression.Right.NodeType != ExpressionType.NotEqual && 
-			    expression.Right.NodeType != ExpressionType.Equal ||
-			    expression.Left.NodeType != ExpressionType.AndAlso &&
-			    expression.Left.NodeType != ExpressionType.NotEqual)
-			{
-				return;
-			}
-
-			// Skip if there are no member access expressions on the right side
-			var notEqualExpression = (BinaryExpression) expression.Right;
-			if (!IsMemberAccess(notEqualExpression.Left) && !IsMemberAccess(notEqualExpression.Right))
-			{
-				return;
-			}
-
-			var notNullMembers = new List<MemberExpression>();
-			// We may have multiple conditions
-			// Example: o.Status != null && o.OldStatus != null
-			if (expression.Left.NodeType == ExpressionType.AndAlso)
-			{
-				FindAllNotNullMembers((BinaryExpression) expression.Left, notNullMembers);
-			}
-			else
-			{
-				FindNotNullMember((BinaryExpression) expression.Left, notNullMembers);
-			}
-
-			if (notNullMembers.Count > 0)
-			{
-				_equalityNotNullMembers[notEqualExpression] = notNullMembers;
-			}
-		}
-
-		private static bool IsMemberAccess(Expression expression)
-		{
-			if (expression.NodeType == ExpressionType.MemberAccess)
-			{
-				return true;
-			}
-
-			// Nullable members can be wrapped in a convert expression
-			return expression is UnaryExpression unaryExpression && unaryExpression.Operand.NodeType == ExpressionType.MemberAccess;
-		}
-
-		private static void FindAllNotNullMembers(BinaryExpression andAlsoExpression, List<MemberExpression> notNullMembers)
-		{
-			if (andAlsoExpression.Right.NodeType == ExpressionType.NotEqual)
-			{
-				FindNotNullMember((BinaryExpression) andAlsoExpression.Right, notNullMembers);
-			}
-			else if (andAlsoExpression.Right.NodeType == ExpressionType.AndAlso)
-			{
-				FindAllNotNullMembers((BinaryExpression) andAlsoExpression.Right, notNullMembers);
-			}
-			else
-			{
-				return;
-			}
-
-			if (andAlsoExpression.Left.NodeType == ExpressionType.NotEqual)
-			{
-				FindNotNullMember((BinaryExpression) andAlsoExpression.Left, notNullMembers);
-			}
-			else if (andAlsoExpression.Left.NodeType == ExpressionType.AndAlso)
-			{
-				FindAllNotNullMembers((BinaryExpression) andAlsoExpression.Left, notNullMembers);
-			}
-		}
-
-		private static void FindNotNullMember(BinaryExpression notEqualExpression, List<MemberExpression> notNullMembers)
-		{
-			if (notEqualExpression.Left.NodeType == ExpressionType.MemberAccess && VisitorUtil.IsNullConstant(notEqualExpression.Right))
-			{
-				notNullMembers.Add((MemberExpression) notEqualExpression.Left);
-			}
-			else if (VisitorUtil.IsNullConstant(notEqualExpression.Left) && notEqualExpression.Right.NodeType == ExpressionType.MemberAccess)
-			{
-				notNullMembers.Add((MemberExpression) notEqualExpression.Right);
-			}
-		}
-
 		protected HqlTreeNode VisitBinaryExpression(BinaryExpression expression)
 		{
 			if (expression.NodeType == ExpressionType.Equal)
@@ -398,7 +296,7 @@ protected HqlTreeNode VisitBinaryExpression(BinaryExpression expression)
 				return TranslateInequalityComparison(expression);
 			}
 
-			SearchForNotNullMembersCheck(expression);
+			_nullableExpressionDetector.SearchForNotNullMemberChecks(expression);
 
 			var lhs = VisitExpression(expression.Left).AsExpression();
 			var rhs = VisitExpression(expression.Right).AsExpression();
@@ -481,8 +379,8 @@ private HqlTreeNode TranslateInequalityComparison(BinaryExpression expression)
 				return _hqlTreeBuilder.IsNotNull(lhs);
 			}
 
-			var lhsNullable = IsNullable(expression.Left, expression);
-			var rhsNullable = IsNullable(expression.Right, expression);
+			var lhsNullable = _nullableExpressionDetector.IsNullable(expression.Left, expression);
+			var rhsNullable = _nullableExpressionDetector.IsNullable(expression.Right, expression);
 
 			var inequality = _hqlTreeBuilder.Inequality(lhs, rhs);
 
@@ -544,8 +442,8 @@ private HqlTreeNode TranslateEqualityComparison(BinaryExpression expression)
 				return _hqlTreeBuilder.IsNull((lhs));
 			}
 
-			var lhsNullable = IsNullable(expression.Left, expression);
-			var rhsNullable = IsNullable(expression.Right, expression);
+			var lhsNullable = _nullableExpressionDetector.IsNullable(expression.Left, expression);
+			var rhsNullable = _nullableExpressionDetector.IsNullable(expression.Right, expression);
 
 			var equality = _hqlTreeBuilder.Equality(lhs, rhs);
 
@@ -564,190 +462,6 @@ private HqlTreeNode TranslateEqualityComparison(BinaryExpression expression)
 					_hqlTreeBuilder.IsNull(rhs2)));
 		}
 
-		private bool IsNullable(Expression expression, BinaryExpression equalityExpression)
-		{
-			var currentExpression = expression;
-			while (true)
-			{
-				switch (currentExpression.NodeType)
-				{
-					case ExpressionType.Convert:
-					case ExpressionType.ConvertChecked:
-					case ExpressionType.TypeAs:
-						var unaryExpression = (UnaryExpression) currentExpression;
-						return IsNullable(unaryExpression.Operand, equalityExpression); // a cast will not return null if the operand is not null
-					case ExpressionType.Not:
-					case ExpressionType.And:
-					case ExpressionType.Or:
-					case ExpressionType.ExclusiveOr:
-					case ExpressionType.LeftShift:
-					case ExpressionType.RightShift:
-					case ExpressionType.AndAlso:
-					case ExpressionType.OrElse:
-					case ExpressionType.Equal:
-					case ExpressionType.NotEqual:
-					case ExpressionType.GreaterThanOrEqual:
-					case ExpressionType.GreaterThan:
-					case ExpressionType.LessThan:
-					case ExpressionType.LessThanOrEqual:
-						return false;
-					case ExpressionType.Add:
-					case ExpressionType.AddChecked:
-					case ExpressionType.Divide:
-					case ExpressionType.Modulo:
-					case ExpressionType.Multiply:
-					case ExpressionType.MultiplyChecked:
-					case ExpressionType.Power:
-					case ExpressionType.Subtract:
-					case ExpressionType.SubtractChecked:
-						var binaryExpression = (BinaryExpression) currentExpression;
-						return IsNullable(binaryExpression.Left, equalityExpression) || IsNullable(binaryExpression.Right, equalityExpression);
-					case ExpressionType.ArrayIndex:
-						return true; // for indexed lists we cannot determine whether the item will be null or not
-					case ExpressionType.Coalesce:
-						return IsNullable(((BinaryExpression) currentExpression).Right, equalityExpression);
-					case ExpressionType.Conditional:
-						var conditionalExpression = (ConditionalExpression) currentExpression;
-						return IsNullable(conditionalExpression.IfTrue, equalityExpression) || 
-						       IsNullable(conditionalExpression.IfFalse, equalityExpression);
-					case ExpressionType.Call:
-						var methodInfo = ((MethodCallExpression) currentExpression).Method;
-						return !_functionRegistry.TryGetGenerator(methodInfo, out var method) || method.AllowsNullableReturnType(methodInfo);
-					case ExpressionType.MemberAccess:
-						var memberExpression = (MemberExpression) currentExpression;
-
-						if (_functionRegistry.TryGetGenerator(memberExpression.Member, out _))
-						{
-							// We have to skip the property as it will be converted to a function that can return null
-							// if the argument is null
-							currentExpression = memberExpression.Expression;
-							continue;
-						}
-
-						var memberType = ReflectHelper.GetPropertyOrFieldType(memberExpression.Member);
-						if (memberType?.IsValueType == true && !memberType.IsNullable())
-						{
-							currentExpression = memberExpression.Expression;
-							continue;
-						}
-
-						// Check if there was a not null check prior the equality expression
-						if ((
-							    equalityExpression.NodeType == ExpressionType.NotEqual ||
-							    equalityExpression.NodeType == ExpressionType.Equal
-							) &&
-							_equalityNotNullMembers.TryGetValue(equalityExpression, out var notNullMembers) &&
-						    notNullMembers.Any(o => AreEqual(o, memberExpression)))
-						{
-							return false;
-						}
-
-						// We have to check the member mapping to determine if is nullable
-						var entityName = TryGetEntityName(memberExpression);
-						if (entityName == null)
-						{
-							return true; // not mapped
-						}
-
-						var persister = _parameters.SessionFactory.GetEntityPersister(entityName);
-						var index = persister.EntityMetamodel.GetPropertyIndexOrNull(memberExpression.Member.Name);
-						if (!index.HasValue || persister.EntityMetamodel.PropertyNullability[index.Value])
-						{
-							return true; // not mapped or nullable
-						}
-
-						currentExpression = memberExpression.Expression;
-						continue;
-					case ExpressionType.Extension:
-						switch (currentExpression)
-						{
-							case QuerySourceReferenceExpression querySourceReferenceExpression:
-								switch (querySourceReferenceExpression.ReferencedQuerySource)
-								{
-									case MainFromClause _:
-										return false; // we reached to the root expression, there were no nullable expressions
-									case NhJoinClause joinClause:
-										return IsNullable(joinClause.FromExpression, equalityExpression);
-									default:
-										return true; // unknown query source
-								}
-							case SubQueryExpression subQuery:
-								if (subQuery.QueryModel.SelectClause.Selector is NhAggregatedExpression subQueryAggregatedExpression)
-								{
-									return subQueryAggregatedExpression.AllowsNullableReturnType;
-								}
-								else if (subQuery.QueryModel.ResultOperators.Any(o => NotNullOperators.Contains(o.GetType())))
-								{
-									return false;
-								}
-
-								return true;
-							case NhAggregatedExpression aggregatedExpression:
-								return aggregatedExpression.AllowsNullableReturnType;
-							default:
-								return true; // a query can return null and we cannot calculate it as it is not yet executed
-						}
-					case ExpressionType.TypeIs: // an equal or in operator will be generated and those cannot return null
-					case ExpressionType.NewArrayInit:
-						return false;
-					case ExpressionType.Constant:
-						return VisitorUtil.IsNullConstant(currentExpression);
-					case ExpressionType.Parameter:
-						return !currentExpression.Type.IsValueType;
-					default:
-						return true;
-				}
-			}
-		}
-
-		private bool AreEqual(MemberExpression memberExpression, MemberExpression otherMemberExpression)
-		{
-			if (memberExpression.Member != otherMemberExpression.Member ||
-			    memberExpression.Expression.NodeType != otherMemberExpression.Expression.NodeType)
-			{
-				return false;
-			}
-
-			switch (memberExpression.Expression)
-			{
-				case QuerySourceReferenceExpression querySourceReferenceExpression:
-					if (otherMemberExpression.Expression is QuerySourceReferenceExpression otherQuerySourceReferenceExpression)
-					{
-						return querySourceReferenceExpression.ReferencedQuerySource ==
-						       otherQuerySourceReferenceExpression.ReferencedQuerySource;
-					}
-
-					return false;
-				// Components have a nested member expression
-				case MemberExpression nestedMemberExpression:
-					if (otherMemberExpression.Expression is MemberExpression otherNestedMemberExpression)
-					{
-						return AreEqual(nestedMemberExpression, otherNestedMemberExpression);
-					}
-
-					return false;
-				default:
-					return memberExpression.Expression == otherMemberExpression.Expression;
-			}
-		}
-
-		private string TryGetEntityName(MemberExpression memberExpression)
-		{
-			System.Type entityType;
-			// Try to get the actual entity type from the query source if possbile as member can be declared
-			// in a base type
-			if (memberExpression.Expression is QuerySourceReferenceExpression querySourceReferenceExpression)
-			{
-				entityType = querySourceReferenceExpression.Type;
-			}
-			else
-			{
-				entityType = memberExpression.Member.ReflectedType;
-			}
-
-			return _parameters.SessionFactory.TryGetGuessEntityName(entityType);
-		}
-
 		protected HqlTreeNode VisitUnaryExpression(UnaryExpression expression)
 		{
 			switch (expression.NodeType)