Obsolete RemoveConvert API as it should not be used

Part of #17529
dotnet · Sep 8, 2019 · 2be14ca · 2be14ca
1 parent c7df045
commit 2be14ca
Show file tree

Hide file tree

Showing 4 changed files with 33 additions and 21 deletions.
diff --git a/src/EFCore/Extensions/Internal/ExpressionExtensions.cs b/src/EFCore/Extensions/Internal/ExpressionExtensions.cs
@@ -31,7 +31,7 @@ public static class ExpressionExtensions
         ///     doing so can result in application failures when updating to a new Entity Framework Core release.
         /// </summary>
         public static bool IsNullConstantExpression([NotNull] this Expression expression)
-            => expression.RemoveConvert() is ConstantExpression constantExpression
+            => RemoveConvert(expression) is ConstantExpression constantExpression
                && constantExpression.Value == null;
 
         /// <summary>
@@ -48,7 +48,7 @@ public static IReadOnlyList<PropertyInfo> MatchPropertyAccessList(
             var parameterExpression
                 = lambdaExpression.Parameters.Single();
 
-            if (lambdaExpression.Body.RemoveConvert() is NewExpression newExpression)
+            if (RemoveConvert(lambdaExpression.Body) is NewExpression newExpression)
             {
                 var propertyInfos
                     = newExpression
@@ -89,7 +89,7 @@ private static IReadOnlyList<PropertyInfo> MatchPropertyAccess(
 
             do
             {
-                memberExpression = RemoveTypeAs(propertyAccessExpression.RemoveConvert()) as MemberExpression;
+                memberExpression = RemoveTypeAs(RemoveConvert(propertyAccessExpression)) as MemberExpression;
 
                 if (!(memberExpression?.Member is PropertyInfo propertyInfo))
                 {
@@ -100,7 +100,7 @@ private static IReadOnlyList<PropertyInfo> MatchPropertyAccess(
 
                 propertyAccessExpression = memberExpression.Expression;
             }
-            while (RemoveTypeAs(memberExpression.Expression.RemoveConvert()) != parameterExpression);
+            while (RemoveTypeAs(RemoveConvert(memberExpression.Expression)) != parameterExpression);
 
             return propertyInfos;
         }
@@ -115,7 +115,7 @@ public static Expression RemoveTypeAs([CanBeNull] this Expression expression)
         {
             while ((expression?.NodeType == ExpressionType.TypeAs))
             {
-                expression = ((UnaryExpression)expression.RemoveConvert()).Operand;
+                expression = ((UnaryExpression)RemoveConvert(expression)).Operand;
             }
 
             return expression;
@@ -168,5 +168,17 @@ public static LambdaExpression UnwrapLambdaFromQuote(this Expression expression)
             => (LambdaExpression)(expression is UnaryExpression unary && expression.NodeType == ExpressionType.Quote
                 ? unary.Operand
                 : expression);
+
+        private static Expression RemoveConvert(Expression expression)
+        {
+            if (expression is UnaryExpression unaryExpression
+                && (expression.NodeType == ExpressionType.Convert
+                    || expression.NodeType == ExpressionType.ConvertChecked))
+            {
+                return RemoveConvert(unaryExpression.Operand);
+            }
+
+            return expression;
+        }
     }
 }
diff --git a/src/EFCore/Infrastructure/ExpressionExtensions.cs b/src/EFCore/Infrastructure/ExpressionExtensions.cs
@@ -210,6 +210,7 @@ var propertyPaths
         /// </summary>
         /// <param name="expression"> The expression. </param>
         /// <returns> A new expression with converts at the head removed. </returns>
+        [Obsolete("Unwrap each convert manually by evaluating how they are used.")]
         public static Expression RemoveConvert([CanBeNull] this Expression expression)
         {
             while (expression != null

diff --git a/src/EFCore/Query/Internal/ParameterExtractingExpressionVisitor.cs b/src/EFCore/Query/Internal/ParameterExtractingExpressionVisitor.cs
@@ -311,6 +311,18 @@ public override Expression Visit(Expression expression)
             }
         }
 
+        private static Expression RemoveConvert(Expression expression)
+        {
+            if (expression is UnaryExpression unaryExpression
+                && (expression.NodeType == ExpressionType.Convert
+                    || expression.NodeType == ExpressionType.ConvertChecked))
+            {
+                return RemoveConvert(unaryExpression.Operand);
+            }
+
+            return expression;
+        }
+
         private object GetValue(Expression expression, out string parameterName)
         {
             parameterName = null;
@@ -332,7 +344,7 @@ private object GetValue(Expression expression, out string parameterName)
                     }
 
                     parameterName = QueryFilterPrefix
-                                    + (expression.RemoveConvert() is MemberExpression memberExpression
+                                    + (RemoveConvert(expression) is MemberExpression memberExpression
                                         ? ("__" + memberExpression.Member.Name)
                                         : "");
 

diff --git a/test/EFCore.Specification.Tests/TestModels/Northwind/NorthwindData.cs b/test/EFCore.Specification.Tests/TestModels/Northwind/NorthwindData.cs
@@ -176,11 +176,11 @@ private AsyncEnumerable(Expression expression)
             }
 
             public IQueryable<TElement> CreateQuery<TElement>(Expression expression)
-                => new AsyncEnumerable<TElement>(RewriteShadowPropertyAccess(expression));
+                => new AsyncEnumerable<TElement>(expression);
 
             public TResult Execute<TResult>(Expression expression)
                 => ((IQueryProvider)_enumerableQuery)
-                    .Execute<TResult>(RewriteShadowPropertyAccess(expression));
+                    .Execute<TResult>(expression);
 
             public IEnumerator<T> GetEnumerator() => ((IQueryable<T>)_enumerableQuery).GetEnumerator();
             IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();
@@ -189,19 +189,6 @@ public TResult Execute<TResult>(Expression expression)
             public Type ElementType => typeof(T);
             public IQueryProvider Provider => this;
 
-            private static Expression RewriteShadowPropertyAccess(Expression expression)
-                => new ShadowStateAccessRewriter().Visit(expression);
-
-            private class ShadowStateAccessRewriter : ExpressionVisitor
-            {
-                protected override Expression VisitMethodCall(MethodCallExpression expression)
-                    => expression.Method.IsEFPropertyMethod()
-                        ? Expression.Property(
-                            expression.Arguments[0].RemoveConvert(),
-                            Expression.Lambda<Func<string>>(expression.Arguments[1]).Compile().Invoke())
-                        : base.VisitMethodCall(expression);
-            }
-
             public IQueryable CreateQuery(Expression expression)
             {
                 throw new NotImplementedException();