Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add static Print methods to ExpressionPrinter #29854

Merged
merged 1 commit into from
Dec 14, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions src/EFCore/Diagnostics/CoreLoggerExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -462,7 +462,7 @@ public static (Expression Query, QueryExpressionEventData? EventData) QueryCompi

if (diagnostics.ShouldLog(definition))
{
definition.Log(diagnostics, Environment.NewLine, expressionPrinter.Print(queryExpression));
definition.Log(diagnostics, Environment.NewLine, expressionPrinter.PrintExpression(queryExpression));
}

if (diagnostics.NeedsEventData<IQueryExpressionInterceptor>(
Expand Down Expand Up @@ -490,7 +490,7 @@ private static string QueryCompilationStarting(EventDefinitionBase definition, E
{
var d = (EventDefinition<string, string>)definition;
var p = (QueryExpressionEventData)payload;
return d.GenerateMessage(Environment.NewLine, p.ExpressionPrinter.Print(p.Expression));
return d.GenerateMessage(Environment.NewLine, p.ExpressionPrinter.PrintExpression(p.Expression));
}

/// <summary>
Expand Down Expand Up @@ -668,7 +668,7 @@ public static void QueryExecutionPlanned(

if (diagnostics.ShouldLog(definition))
{
definition.Log(diagnostics, Environment.NewLine, expressionPrinter.Print(queryExecutorExpression));
definition.Log(diagnostics, Environment.NewLine, expressionPrinter.PrintExpression(queryExecutorExpression));
}

if (diagnostics.NeedsEventData(definition, out var diagnosticSourceEnabled, out var simpleLogEnabled))
Expand All @@ -688,7 +688,7 @@ private static string QueryExecutionPlanned(EventDefinitionBase definition, Even
{
var d = (EventDefinition<string, string>)definition;
var p = (QueryExpressionEventData)payload;
return d.GenerateMessage(Environment.NewLine, p.ExpressionPrinter.Print(p.Expression));
return d.GenerateMessage(Environment.NewLine, p.ExpressionPrinter.PrintExpression(p.Expression));
}

/// <summary>
Expand Down
2 changes: 1 addition & 1 deletion src/EFCore/Infrastructure/ExpressionExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ public static class ExpressionExtensions
/// <param name="characterLimit">An optional limit to the number of characters included. Additional output will be truncated.</param>
/// <returns>The printable representation.</returns>
public static string Print(this Expression expression, int? characterLimit = null)
=> new ExpressionPrinter().Print(expression, characterLimit);
=> new ExpressionPrinter().PrintExpression(expression, characterLimit);

/// <summary>
/// Creates a <see cref="MemberExpression"></see> that represents accessing either a field or a property.
Expand Down
158 changes: 82 additions & 76 deletions src/EFCore/Query/ExpressionPrinter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -70,34 +70,6 @@ public ExpressionPrinter()
private int? CharacterLimit { get; set; }
private bool Verbose { get; set; }

/// <summary>
/// Visit given readonly collection of expression for printing.
/// </summary>
/// <param name="items">A collection of items to print.</param>
/// <param name="joinAction">A join action to use when joining printout of individual item in the collection.</param>
public virtual void VisitCollection<T>(
IReadOnlyCollection<T> items,
Action<ExpressionPrinter>? joinAction = null)
where T : Expression
{
joinAction ??= (p => p.Append(", "));

var first = true;
foreach (var item in items)
{
if (!first)
{
joinAction(this);
}
else
{
first = false;
}

Visit(item);
}
}

/// <summary>
/// Appends a new line to current output being built.
/// </summary>
Expand Down Expand Up @@ -149,30 +121,40 @@ public virtual ExpressionPrinter Append(string value)
return this;
}

/// <summary>
/// Creates a printable string representation of the given expression.
/// </summary>
/// <param name="expression">The expression to print.</param>
/// <returns>The printable representation.</returns>
public static string Print(Expression expression)
=> new ExpressionPrinter().PrintCore(expression);

/// <summary>
/// Creates a printable verbose string representation of the given expression.
/// </summary>
/// <param name="expression">The expression to print.</param>
/// <returns>The printable representation.</returns>
public static string PrintDebug(Expression expression)
=> new ExpressionPrinter().PrintCore(expression, verbose: true);

/// <summary>
/// Creates a printable string representation of the given expression.
/// </summary>
/// <param name="expression">The expression to print.</param>
/// <param name="characterLimit">An optional limit to the number of characters included. Additional output will be truncated.</param>
/// <returns>The printable representation.</returns>
public virtual string Print(
Expression expression,
int? characterLimit = null)
=> PrintCore(expression, characterLimit, verbose: false);
public virtual string PrintExpression(Expression expression, int? characterLimit = null)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

C# doesn't allow static/non-static overloading where the signature is otherwise the same. Since the static API is what we're likely to use in debugging (where shorter names are the most important), I'm proposing to have a static Print, and a longer non-static PrintExpression (which we'd use only in code).

=> PrintCore(expression, characterLimit);

/// <summary>
/// Creates a printable verbose string representation of the given expression.
/// </summary>
/// <param name="expression">The expression to print.</param>
/// <returns>The printable representation.</returns>
public virtual string PrintDebug(
Expression expression)
=> PrintCore(expression, characterLimit: null, verbose: true);

private string PrintCore(
Expression expression,
int? characterLimit,
bool verbose)
public virtual string PrintExpressionDebug(Expression expression)
=> PrintCore(expression, verbose: true);

private string PrintCore(Expression expression, int? characterLimit = null, bool verbose = false)
{
_stringBuilder.Clear();
_parametersInScope.Clear();
Expand All @@ -186,8 +168,7 @@ private string PrintCore(

var queryPlan = PostProcess(_stringBuilder.ToString());

if (characterLimit != null
&& characterLimit.Value > 0)
if (characterLimit is > 0)
{
queryPlan = queryPlan.Length > characterLimit
? queryPlan[..characterLimit.Value] + "..."
Expand All @@ -205,6 +186,32 @@ private string PrintCore(
public virtual string GenerateBinaryOperator(ExpressionType expressionType)
=> _binaryOperandMap[expressionType];

/// <summary>
/// Visit given readonly collection of expression for printing.
/// </summary>
/// <param name="items">A collection of items to print.</param>
/// <param name="joinAction">A join action to use when joining printout of individual item in the collection.</param>
public virtual void VisitCollection<T>(IReadOnlyCollection<T> items, Action<ExpressionPrinter>? joinAction = null)
where T : Expression
{
joinAction ??= (p => p.Append(", "));

var first = true;
foreach (var item in items)
{
if (!first)
{
joinAction(this);
}
else
{
first = false;
}

Visit(item);
}
}

/// <inheritdoc />
[return: NotNullIfNotNull("expression")]
public override Expression? Visit(Expression? expression)
Expand Down Expand Up @@ -386,7 +393,7 @@ protected override Expression VisitBlock(BlockExpression blockExpression)
}
}

var expressions = blockExpression.Result != null
var expressions = blockExpression.Expressions.Count > 0
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BlockExpression.Result is non-nullable, but does through for empty blocks (so this was a bug).

? blockExpression.Expressions.Except(new[] { blockExpression.Result })
: blockExpression.Expressions;

Expand All @@ -396,7 +403,7 @@ protected override Expression VisitBlock(BlockExpression blockExpression)
AppendLine(";");
}

if (blockExpression.Result != null)
if (blockExpression.Expressions.Count > 0)
{
if (blockExpression.Result.Type != typeof(void))
{
Expand Down Expand Up @@ -438,50 +445,50 @@ protected override Expression VisitConstant(ConstantExpression constantExpressio
}
else
{
Print(constantExpression.Value);
PrintValue(constantExpression.Value);
}

return constantExpression;
}

private void Print(object? value)
{
if (value is IEnumerable enumerable
&& !(value is string))
void PrintValue(object? value)
{
_stringBuilder.Append(value.GetType().ShortDisplayName() + " { ");

var first = true;
foreach (var item in enumerable)
if (value is IEnumerable enumerable
&& !(value is string))
{
if (first)
{
first = false;
}
else
_stringBuilder.Append(value.GetType().ShortDisplayName() + " { ");

var first = true;
foreach (var item in enumerable)
{
_stringBuilder.Append(", ");
if (first)
{
first = false;
}
else
{
_stringBuilder.Append(", ");
}

PrintValue(item);
}

Print(item);
_stringBuilder.Append(" }");
return;
}

_stringBuilder.Append(" }");
return;
}
var stringValue = value == null
? "null"
: value.ToString() != value.GetType().ToString()
? value.ToString()
: value.GetType().ShortDisplayName();

var stringValue = value == null
? "null"
: value.ToString() != value.GetType().ToString()
? value.ToString()
: value.GetType().ShortDisplayName();
if (value is string)
{
stringValue = $@"""{stringValue}""";
}

if (value is string)
{
stringValue = $@"""{stringValue}""";
_stringBuilder.Append(stringValue ?? "Unknown");
}

_stringBuilder.Append(stringValue ?? "Unknown");
}

/// <inheritdoc />
Expand Down Expand Up @@ -642,8 +649,7 @@ 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))
if (method.Name is nameof(Enumerable.Cast) or nameof(Enumerable.OfType))
{
PrintGenericArguments(method, _stringBuilder);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ private static void VerifyReturnType(Expression expression, ParameterExpression
{
throw new InvalidOperationException(
CoreStrings.QueryInvalidMaterializationType(
new ExpressionPrinter().Print(Expression.Lambda(expression, lambdaParameter)),
new ExpressionPrinter().PrintExpression(Expression.Lambda(expression, lambdaParameter)),
expression.Type.ShortDisplayName()));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -769,7 +769,7 @@ public virtual Task Update_Where_multiple_set(bool async)
[MemberData(nameof(IsAsyncData))]
public virtual Task Update_with_invalid_lambda_in_set_property_throws(bool async)
=> AssertTranslationFailed(
RelationalStrings.InvalidPropertyInSetProperty(new ExpressionPrinter().Print((OrderDetail e) => e.MaybeScalar(e => e.OrderID))),
RelationalStrings.InvalidPropertyInSetProperty(new ExpressionPrinter().PrintExpression((OrderDetail e) => e.MaybeScalar(e => e.OrderID))),
() => AssertUpdate(
async,
ss => ss.Set<OrderDetail>().Where(od => od.OrderID < 10250),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ public virtual Expression QueryCompilationStarting(
{
QueryCompilationStartingCalled = true;
Context = eventData.Context;
QueryExpression = eventData.ExpressionPrinter.Print(queryExpression);
QueryExpression = eventData.ExpressionPrinter.PrintExpression(queryExpression);

return queryExpression;
}
Expand Down
Loading