Code review changes

Author: maca88

src/NHibernate.Test/Async/Linq/ParameterTests.cs

@@ -112,7 +112,7 @@ public async Task UsingNegateValueTypeParameterAsync()
 			var value = 1;
 			await (AssertTotalParametersAsync(
 				db.Orders.Where(o => o.OrderId == value && o.OrderId != -value),
-				2));
+				1));
 		}
 
 		[Test]

src/NHibernate.Test/Linq/ParameterTests.cs

@@ -120,7 +120,7 @@ public void UsingNegateValueTypeParameter()
 			var value = 1;
 			AssertTotalParameters(
 				db.Orders.Where(o => o.OrderId == value && o.OrderId != -value),
-				2);
+				1);
 		}
 
 		[Test]

src/NHibernate/Linq/ExpressionTransformers/RemoveCharToIntConversion.cs

@@ -35,6 +35,19 @@ public Expression Transform(BinaryExpression expression)
 
 			if (!lhsIsConvertExpression && !rhsIsConvertExpression) return expression;
 
+			// Variables are not converted to constants (E.g: o.CharProperty == charVariable)
+			if (lhsIsConvertExpression && rhsIsConvertExpression)
+			{
+				var lhsConvertExpression = (UnaryExpression) lhs;
+				var rhsConvertExpression = (UnaryExpression) rhs;
+				if (!IsConvertCharToInt(lhsConvertExpression) || !IsConvertCharToInt(rhsConvertExpression))
+				{
+					return expression;
+				}
+
+				return Expression.MakeBinary(expression.NodeType, lhsConvertExpression.Operand, rhsConvertExpression.Operand);
+			}
+
 			var lhsIsConstantExpression = IsConstantExpression(lhs);
 			var rhsIsConstantExpression = IsConstantExpression(rhs);
 
@@ -43,7 +56,7 @@ public Expression Transform(BinaryExpression expression)
 			var convertExpression = lhsIsConvertExpression ? (UnaryExpression)lhs : (UnaryExpression)rhs;
 			var constantExpression = lhsIsConstantExpression ? (ConstantExpression)lhs : (ConstantExpression)rhs;
 
-			if (convertExpression.Type == typeof(int) && convertExpression.Operand.Type == typeof(char) && constantExpression.Type == typeof(int))
+			if (IsConvertCharToInt(convertExpression) && constantExpression.Type == typeof(int))
 			{
 				var constant = Expression.Constant(Convert.ToChar((int)constantExpression.Value));
 
@@ -56,6 +69,11 @@ public Expression Transform(BinaryExpression expression)
 			return expression;
 		}
 
+		private static bool IsConvertCharToInt(UnaryExpression expression)
+		{
+			return expression.Type == typeof(int) && expression.Operand.Type == typeof(char);
+		}
+
 		private static bool IsConvertExpression(Expression expression)
 		{
 			return (expression.NodeType == ExpressionType.Convert);
@@ -71,4 +89,4 @@ public ExpressionType[] SupportedExpressionTypes
 			get { return _supportedExpressionTypes; }
 		}
 	}
-}
+}

src/NHibernate/Linq/Visitors/NhPartialEvaluatingExpressionVisitor.cs

@@ -16,11 +16,8 @@
 // 
 
 using System;
-using System.Collections.Generic;
 using System.Linq;
 using System.Linq.Expressions;
-using System.Reflection;
-using System.Runtime.CompilerServices;
 using NHibernate.Collection;
 using NHibernate.Engine;
 using NHibernate.Linq.Functions;
@@ -112,10 +109,10 @@ public override Expression Visit(Expression expression)
 			// Variables in expressions are never a constant, they are encapsulated as fields of a compiler generated class.
 			// Skip detecting variables for DML queries as HQL does not support reusing parameters for them.
 			if (expression.NodeType != ExpressionType.Constant &&
-				_preTransformationParameters.QueryMode == QueryMode.Select &&
+			    _preTransformationParameters.QueryMode == QueryMode.Select &&
 			    evaluatedExpression is ConstantExpression variableConstant &&
 			    !_preTransformationParameters.QueryVariables.ContainsKey(variableConstant) &&
-			    IsVariable(expression, out var path, out var closureContext))
+			    ExpressionsHelper.IsVariable(expression, out var path, out var closureContext))
 			{
 				_preTransformationParameters.QueryVariables.Add(variableConstant, new QueryVariable(path, closureContext));
 			}
@@ -163,46 +160,6 @@ protected override Expression VisitConstant(ConstantExpression expression)
 			}
 			return base.VisitConstant(expression);
 		}
-
-		private bool IsVariable(Expression expression, out string path, out object closureContext)
-		{
-			Expression childExpression;
-			string currentPath;
-			switch (expression)
-			{
-				case MemberExpression memberExpression:
-					childExpression = memberExpression.Expression;
-					currentPath = memberExpression.Member.Name;
-					break;
-				case ConstantExpression constantExpression:
-					path = null;
-					if (constantExpression.Type.Attributes.HasFlag(TypeAttributes.NestedPrivate) &&
-					    Attribute.IsDefined(constantExpression.Type, typeof(CompilerGeneratedAttribute), inherit: true))
-					{
-						closureContext = constantExpression.Value;
-						return true;
-					}
-
-					closureContext = null;
-					return false;
-				case UnaryExpression unaryExpression:
-					childExpression = unaryExpression.Operand;
-					currentPath = $"({unaryExpression.NodeType})";
-					break;
-				default:
-					path = null;
-					closureContext = null;
-					return false;
-			}
-
-			if (!IsVariable(childExpression, out path, out closureContext))
-			{
-				return false;
-			}
-
-			path = path != null ? $"{path}_{currentPath}" : currentPath;
-			return true;
-		}
 	}
 
 	internal struct QueryVariable : IEquatable<QueryVariable>
@@ -255,6 +212,11 @@ public override bool IsEvaluatableConstant(ConstantExpression node)
 			return base.IsEvaluatableConstant(node);
 		}
 
+		public override bool IsEvaluatableUnary(UnaryExpression node)
+		{
+			return !ExpressionsHelper.IsVariable(node.Operand, out _, out _);
+		}
+
 		public override bool IsEvaluatableMember(MemberExpression node)
 		{
 			if (node == null)

src/NHibernate/Util/ExpressionsHelper.cs

@@ -3,6 +3,7 @@
 using System.Linq;
 using System.Linq.Expressions;
 using System.Reflection;
+using System.Runtime.CompilerServices;
 using NHibernate.Engine;
 using NHibernate.Linq;
 using NHibernate.Linq.Expressions;
@@ -28,6 +29,49 @@ public static MemberInfo DecodeMemberAccessExpression<TEntity, TResult>(Expressi
 			return ((MemberExpression)expression.Body).Member;
 		}
 
+		/// <summary>
+		/// Check whether the given expression represent a variable.
+		/// </summary>
+		/// <param name="expression">The expression to check.</param>
+		/// <param name="path">The path of the variable.</param>
+		/// <param name="closureContext">The closure context where the variable is stored.</param>
+		/// <returns></returns>
+		internal static bool IsVariable(Expression expression, out string path, out object closureContext)
+		{
+			Expression childExpression;
+			string currentPath;
+			switch (expression)
+			{
+				case MemberExpression memberExpression:
+					childExpression = memberExpression.Expression;
+					currentPath = memberExpression.Member.Name;
+					break;
+				case ConstantExpression constantExpression:
+					path = null;
+					if (constantExpression.Type.Attributes.HasFlag(TypeAttributes.NestedPrivate) &&
+					    Attribute.IsDefined(constantExpression.Type, typeof(CompilerGeneratedAttribute), inherit: true))
+					{
+						closureContext = constantExpression.Value;
+						return true;
+					}
+
+					closureContext = null;
+					return false;
+				default:
+					path = null;
+					closureContext = null;
+					return false;
+			}
+
+			if (!IsVariable(childExpression, out path, out closureContext))
+			{
+				return false;
+			}
+
+			path = path != null ? $"{currentPath}_{path}" : currentPath;
+			return true;
+		}
+
 		/// <summary>
 		/// Get the mapped type for the given expression.
 		/// </summary>