ExpressionPrinter fixes/cleanup

Fix to #18709 - Query: expression printer should still print type argument for Cast and OfType Fix to #18413 - Query: clean-up expression printer when we can do breaking changes Resolves #18709 Resolves #18413
dotnet · Nov 5, 2019 · 37f0135 · 37f0135
1 parent 83b5ad6
commit 37f0135
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 30 deletions.
diff --git a/src/EFCore/Query/ExpressionPrinter.cs b/src/EFCore/Query/ExpressionPrinter.cs
@@ -53,8 +53,7 @@ public ExpressionPrinter()
         }
 
         private int? CharacterLimit { get; set; }
-
-        private bool GenerateUniqueParameterIds { get; set; }
+        private bool Verbose { get; set; }
 
         public virtual void VisitList<T>(
             IReadOnlyList<T> items,
@@ -110,26 +109,26 @@ private void AppendLine([NotNull] string message)
         public virtual string Print(
             Expression expression,
             int? characterLimit = null)
-            => PrintCore(expression, characterLimit, generateUniqueParameterIds: false);
+            => PrintCore(expression, characterLimit, verbose: false);
 
         public virtual string PrintDebug(
             Expression expression,
             int? characterLimit = null,
-            bool generateUniqueParameterIds = true)
-            => PrintCore(expression, characterLimit, generateUniqueParameterIds);
+            bool verbose = true)
+            => PrintCore(expression, characterLimit, verbose);
 
         protected virtual string PrintCore(
             Expression expression,
             int? characterLimit,
-            bool generateUniqueParameterIds)
+            bool verbose)
         {
             _stringBuilder.Clear();
             _parametersInScope.Clear();
             _namelessParameters.Clear();
             _encounteredParameters.Clear();
 
             CharacterLimit = characterLimit;
-            GenerateUniqueParameterIds = generateUniqueParameterIds;
+            Verbose = verbose;
 
             Visit(expression);
 
@@ -552,8 +551,7 @@ protected override Expression VisitMethodCall(MethodCallExpression methodCallExp
             var methodArguments = methodCallExpression.Arguments.ToList();
             var method = methodCallExpression.Method;
 
-            // TODO: issue #18413
-            var extensionMethod = !GenerateUniqueParameterIds
+            var extensionMethod = !Verbose
                 && methodCallExpression.Arguments.Count > 0
                 && method.IsDefined(typeof(ExtensionAttribute), inherit: false);
 
@@ -564,6 +562,11 @@ protected override Expression VisitMethodCall(MethodCallExpression methodCallExp
                 _stringBuilder.AppendLine();
                 _stringBuilder.Append($".{method.Name}");
                 methodArguments = methodArguments.Skip(1).ToList();
+                if (method.Name == nameof(Enumerable.Cast)
+                    || method.Name == nameof(Enumerable.OfType))
+                {
+                    PrintGenericArguments(method, _stringBuilder);
+                }
             }
             else
             {
@@ -573,23 +576,7 @@ protected override Expression VisitMethodCall(MethodCallExpression methodCallExp
                 }
 
                 _stringBuilder.Append(method.Name);
-                if (method.IsGenericMethod)
-                {
-                    _stringBuilder.Append("<");
-                    var first = true;
-                    foreach (var genericArgument in method.GetGenericArguments())
-                    {
-                        if (!first)
-                        {
-                            _stringBuilder.Append(", ");
-                        }
-
-                        _stringBuilder.Append(genericArgument.ShortDisplayName());
-                        first = false;
-                    }
-
-                    _stringBuilder.Append(">");
-                }
+                PrintGenericArguments(method, _stringBuilder);
             }
 
             _stringBuilder.Append("(");
@@ -647,6 +634,27 @@ var argumentNames
             }
 
             return methodCallExpression;
+
+            static void PrintGenericArguments(MethodInfo method, IndentedStringBuilder stringBuilder)
+            {
+                if (method.IsGenericMethod)
+                {
+                    stringBuilder.Append("<");
+                    var first = true;
+                    foreach (var genericArgument in method.GetGenericArguments())
+                    {
+                        if (!first)
+                        {
+                            stringBuilder.Append(", ");
+                        }
+
+                        stringBuilder.Append(genericArgument.ShortDisplayName());
+                        first = false;
+                    }
+
+                    stringBuilder.Append(">");
+                }
+            }
         }
 
         protected override Expression VisitNew(NewExpression newExpression)
@@ -756,8 +764,7 @@ protected override Expression VisitParameter(ParameterExpression parameterExpres
             }
             else
             {
-                // TODO: issue #18413
-                if (GenerateUniqueParameterIds)
+                if (Verbose)
                 {
                     Append("(Unhandled parameter: ");
                     Append(parameterExpression.Name);
@@ -769,7 +776,7 @@ protected override Expression VisitParameter(ParameterExpression parameterExpres
                 }
             }
 
-            if (GenerateUniqueParameterIds)
+            if (Verbose)
             {
                 var parameterIndex = _encounteredParameters.Count;
                 if (_encounteredParameters.Contains(parameterExpression))

diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/QueryBugsTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/QueryBugsTest.cs
@@ -7301,7 +7301,7 @@ public void Cast_to_non_implemented_interface_is_not_removed_from_expression_tre
                 () => queryBase.Cast<IDummyEntity>().FirstOrDefault(x => x.Id == id)).Message;
 
             Assert.Equal(
-                CoreStrings.TranslationFailed(@"DbSet<MockEntity>    .Cast()    .Where(e => e.Id == __id_0)"),
+                CoreStrings.TranslationFailed(@"DbSet<MockEntity>    .Cast<IDummyEntity>()    .Where(e => e.Id == __id_0)"),
                 message.Replace("\r", "").Replace("\n", ""));
         }