From 9ac1bb92beb38c7d62b4a3d3101f6d1987ccd212 Mon Sep 17 00:00:00 2001 From: Shay Rojansky Date: Thu, 4 Jan 2024 01:33:42 +0100 Subject: [PATCH] Fix complex assignment operator handling in LinqToCSharpSyntaxTranslator Part of #32659 --- .../Internal/LinqToCSharpSyntaxTranslator.cs | 71 +++++++++++++++---- .../Query/LinqToCSharpTranslatorTest.cs | 46 +++++++++++- 2 files changed, 100 insertions(+), 17 deletions(-) diff --git a/src/EFCore.Design/Query/Internal/LinqToCSharpSyntaxTranslator.cs b/src/EFCore.Design/Query/Internal/LinqToCSharpSyntaxTranslator.cs index a5c501be876..5e5336537e9 100644 --- a/src/EFCore.Design/Query/Internal/LinqToCSharpSyntaxTranslator.cs +++ b/src/EFCore.Design/Query/Internal/LinqToCSharpSyntaxTranslator.cs @@ -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( @@ -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, @@ -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, @@ -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(assignment.Left); @@ -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(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, @@ -378,7 +421,7 @@ Expression VisitAssignment(BinaryExpression assignment) } else { - Result = AssignmentExpression(SyntaxKind.SimpleAssignmentExpression, translatedLeft, translatedRight); + Result = AssignmentExpression(kind, translatedLeft, translatedRight); } } diff --git a/test/EFCore.Design.Tests/Query/LinqToCSharpTranslatorTest.cs b/test/EFCore.Design.Tests/Query/LinqToCSharpTranslatorTest.cs index 0cb89d6e707..59664bb7712 100644 --- a/test/EFCore.Design.Tests/Query/LinqToCSharpTranslatorTest.cs +++ b/test/EFCore.Design.Tests/Query/LinqToCSharpTranslatorTest.cs @@ -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() @@ -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")]