Skip to content

Commit

Permalink
Fix complex assignment operator handling in LinqToCSharpSyntaxTranslator
Browse files Browse the repository at this point in the history 
Part of dotnet#32659
  • Loading branch information
@roji
roji committed Jan 4, 2024
1 parent 414d9c3 commit 9ac1bb9
Show file tree
Hide file tree
Showing 2 changed files with 100 additions and 17 deletions.
71 changes: 57 additions & 14 deletions src/EFCore.Design/Query/Internal/LinqToCSharpSyntaxTranslator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -225,8 +225,39 @@ protected override Expression VisitBinary(BinaryExpression binary)
// Handle some special cases
switch (binary.NodeType)
{
// TODO: Confirm what to do with the checked expression types

case ExpressionType.Assign:
return VisitAssignment(binary);
return VisitAssignment(binary, SyntaxKind.SimpleAssignmentExpression);

case ExpressionType.AddAssign:
return VisitAssignment(binary, SyntaxKind.AddAssignmentExpression);
case ExpressionType.AddAssignChecked:
return VisitAssignment(binary, SyntaxKind.AddAssignmentExpression);
case ExpressionType.MultiplyAssign:
return VisitAssignment(binary, SyntaxKind.MultiplyAssignmentExpression);
case ExpressionType.MultiplyAssignChecked:
return VisitAssignment(binary, SyntaxKind.MultiplyAssignmentExpression);
case ExpressionType.DivideAssign:
return VisitAssignment(binary, SyntaxKind.DivideAssignmentExpression);
case ExpressionType.ModuloAssign:
return VisitAssignment(binary, SyntaxKind.ModuloAssignmentExpression);
case ExpressionType.SubtractAssign:
return VisitAssignment(binary, SyntaxKind.SubtractAssignmentExpression);
case ExpressionType.SubtractAssignChecked:
return VisitAssignment(binary, SyntaxKind.SubtractAssignmentExpression);

// Bitwise assignment operators
case ExpressionType.AndAssign:
return VisitAssignment(binary, SyntaxKind.AndAssignmentExpression);
case ExpressionType.OrAssign:
return VisitAssignment(binary, SyntaxKind.OrAssignmentExpression);
case ExpressionType.LeftShiftAssign:
return VisitAssignment(binary, SyntaxKind.LeftShiftAssignmentExpression);
case ExpressionType.RightShiftAssign:
return VisitAssignment(binary, SyntaxKind.RightShiftAssignmentExpression);
case ExpressionType.ExclusiveOrAssign:
return VisitAssignment(binary, SyntaxKind.ExclusiveOrAssignmentExpression);

case ExpressionType.Power when binary.Left.Type == typeof(double) && binary.Right.Type == typeof(double):
return Visit(
Expand Down Expand Up @@ -290,14 +321,6 @@ protected override Expression VisitBinary(BinaryExpression binary)
ExpressionType.MultiplyChecked => SyntaxKind.MultiplyExpression,
ExpressionType.Divide => SyntaxKind.DivideExpression,
ExpressionType.Modulo => SyntaxKind.ModuloExpression,
ExpressionType.AddAssign => SyntaxKind.AddAssignmentExpression,
ExpressionType.AddAssignChecked => SyntaxKind.AddAssignmentExpression,
ExpressionType.SubtractAssign => SyntaxKind.SubtractAssignmentExpression,
ExpressionType.SubtractAssignChecked => SyntaxKind.SubtractAssignmentExpression,
ExpressionType.MultiplyAssign => SyntaxKind.MultiplyAssignmentExpression,
ExpressionType.MultiplyAssignChecked => SyntaxKind.MultiplyAssignmentExpression,
ExpressionType.DivideAssign => SyntaxKind.DivideAssignmentExpression,
ExpressionType.ModuloAssign => SyntaxKind.ModuloAssignmentExpression,

ExpressionType.GreaterThan => SyntaxKind.GreaterThanExpression,
ExpressionType.GreaterThanOrEqual => SyntaxKind.GreaterThanOrEqualExpression,
Expand All @@ -315,9 +338,6 @@ protected override Expression VisitBinary(BinaryExpression binary)
ExpressionType.LeftShift => SyntaxKind.LeftShiftExpression,
ExpressionType.RightShift => SyntaxKind.RightShiftExpression,
// TODO UnsignedRightShiftExpression
ExpressionType.ExclusiveOrAssign => SyntaxKind.ExclusiveOrAssignmentExpression,
ExpressionType.LeftShiftAssign => SyntaxKind.LeftShiftAssignmentExpression,
ExpressionType.RightShiftAssign => SyntaxKind.RightShiftAssignmentExpression,

ExpressionType.TypeIs => SyntaxKind.IsExpression,
ExpressionType.TypeAs => SyntaxKind.AsExpression,
Expand All @@ -330,7 +350,7 @@ protected override Expression VisitBinary(BinaryExpression binary)

return binary;

Expression VisitAssignment(BinaryExpression assignment)
Expression VisitAssignment(BinaryExpression assignment, SyntaxKind kind)
{
var translatedLeft = Translate<ExpressionSyntax>(assignment.Left);

Expand All @@ -351,8 +371,31 @@ Expression VisitAssignment(BinaryExpression assignment)
ArgumentList.Arguments: [var lValue]
})
{
// If we have a simple assignment, use the RHS directly (fieldInfo.SetValue(lValue, rValue)).
// For compound assignment operators, apply the appropriate operator (fieldInfo.setValue(lValue, rValue + lValue)
translatedRight = Translate<ExpressionSyntax>(assignment.Right);

if (kind != SyntaxKind.SimpleAssignmentExpression)
{
var nonAssignmentOperator = kind switch
{
SyntaxKind.AddAssignmentExpression => SyntaxKind.AddExpression,
SyntaxKind.MultiplyAssignmentExpression => SyntaxKind.MultiplyExpression,
SyntaxKind.DivideAssignmentExpression => SyntaxKind.DivideExpression,
SyntaxKind.ModuloAssignmentExpression => SyntaxKind.ModuloExpression,
SyntaxKind.SubtractAssignmentExpression => SyntaxKind.SubtractExpression,
SyntaxKind.AndAssignmentExpression => SyntaxKind.BitwiseAndExpression,
SyntaxKind.OrAssignmentExpression => SyntaxKind.BitwiseOrExpression,
SyntaxKind.LeftShiftAssignmentExpression => SyntaxKind.LeftShiftExpression,
SyntaxKind.RightShiftAssignmentExpression => SyntaxKind.RightShiftExpression,
SyntaxKind.ExclusiveOrAssignmentExpression => SyntaxKind.ExclusiveOrExpression,

_ => throw new UnreachableException()
};

translatedRight = BinaryExpression(nonAssignmentOperator, translatedLeft, translatedRight);
}

Result = InvocationExpression(
MemberAccessExpression(
SyntaxKind.SimpleMemberAccessExpression,
Expand All @@ -378,7 +421,7 @@ Expression VisitAssignment(BinaryExpression assignment)
}
else
{
Result = AssignmentExpression(SyntaxKind.SimpleAssignmentExpression, translatedLeft, translatedRight);
Result = AssignmentExpression(kind, translatedLeft, translatedRight);
}
}

Expand Down
46 changes: 43 additions & 3 deletions test/EFCore.Design.Tests/Query/LinqToCSharpTranslatorTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,11 +70,24 @@ public void Enum_with_unknown_value()
[Theory]
[InlineData(ExpressionType.Add, "+")]
[InlineData(ExpressionType.Subtract, "-")]
// TODO: Complete
[InlineData(ExpressionType.Assign, "=")]
[InlineData(ExpressionType.AddAssign, "+=")]
[InlineData(ExpressionType.AddAssignChecked, "+=")]
[InlineData(ExpressionType.MultiplyAssign, "*=")]
[InlineData(ExpressionType.MultiplyAssignChecked, "*=")]
[InlineData(ExpressionType.DivideAssign, "/=")]
[InlineData(ExpressionType.ModuloAssign, "%=")]
[InlineData(ExpressionType.SubtractAssign, "-=")]
[InlineData(ExpressionType.SubtractAssignChecked, "-=")]
[InlineData(ExpressionType.AndAssign, "&=")]
[InlineData(ExpressionType.OrAssign, "|=")]
[InlineData(ExpressionType.LeftShiftAssign, "<<=")]
[InlineData(ExpressionType.RightShiftAssign, ">>=")]
[InlineData(ExpressionType.ExclusiveOrAssign, "^=")]
public void Binary_numeric(ExpressionType expressionType, string op)
=> AssertExpression(
MakeBinary(expressionType, Constant(2), Constant(3)),
$"2 {op} 3");
MakeBinary(expressionType, Parameter(typeof(int), "i"), Constant(3)),
$"i {op} 3");

[Fact]
public void Binary_ArrayIndex()
Expand All @@ -94,6 +107,33 @@ public void Binary_PowerAssign()
PowerAssign(Parameter(typeof(double), "d"), Constant(3.0)),
"d = Math.Pow(d, 3D)");

[Fact]
public void Private_instance_field_SimpleAssign()
=> AssertExpression(
Assign(
Field(Parameter(typeof(Blog), "blog"), "_privateField"),
Constant(3)),
"""typeof(LinqToCSharpTranslatorTest.Blog).GetField("_privateField", BindingFlags.Instance | BindingFlags.NonPublic).SetValue(blog, 3)""");

[Theory]
[InlineData(ExpressionType.AddAssign, "+")]
[InlineData(ExpressionType.MultiplyAssign, "*")]
[InlineData(ExpressionType.DivideAssign, "/")]
[InlineData(ExpressionType.ModuloAssign, "%")]
[InlineData(ExpressionType.SubtractAssign, "-")]
[InlineData(ExpressionType.AndAssign, "&")]
[InlineData(ExpressionType.OrAssign, "|")]
[InlineData(ExpressionType.LeftShiftAssign, "<<")]
[InlineData(ExpressionType.RightShiftAssign, ">>")]
[InlineData(ExpressionType.ExclusiveOrAssign, "^")]
public void Private_instance_field_AssignOperators(ExpressionType expressionType, string op)
=> AssertExpression(
MakeBinary(
expressionType,
Field(Parameter(typeof(Blog), "blog"), "_privateField"),
Constant(3)),
$"""typeof(LinqToCSharpTranslatorTest.Blog).GetField("_privateField", BindingFlags.Instance | BindingFlags.NonPublic).SetValue(blog, typeof(LinqToCSharpTranslatorTest.Blog).GetField("_privateField", BindingFlags.Instance | BindingFlags.NonPublic).GetValue(blog) {op} 3)""");

[Theory]
[InlineData(ExpressionType.Negate, "-i")]
[InlineData(ExpressionType.NegateChecked, "-i")]
Expand Down

0 comments on commit 9ac1bb9

Please sign in to comment.