Fix complex assignment operator handling in LinqToCSharpSyntaxTransla…

…tor (#32715)

Part of #32659
Fixes #32716

src/EFCore.Design/Properties/DesignStrings.resx

@@ -380,6 +380,9 @@ Change your target project to the migrations project by using the Package Manage
   <data name="RevertMigration" xml:space="preserve">
     <value>The migration '{name}' has already been applied to the database. Revert it and try again. If the migration has been applied to other databases, consider reverting its changes using a new migration instead.</value>
   </data>
+  <data name="SameParameterExpressionDeclaredAsVariableInNestedBlocks" xml:space="preserve">
+    <value>The same ParameterExpression instance with name '{parameter}' was used as a variable declaration in a block and a nested block inside it. This is not allowed - use different ParameterExpression instances.</value>
+  </data>
   <data name="SensitiveInformationWarning" xml:space="preserve">
     <value>To protect potentially sensitive information in your connection string, you should move it out of source code. You can avoid scaffolding the connection string by using the Name= syntax to read it from configuration - see https://go.microsoft.com/fwlink/?linkid=2131148. For more guidance on storing connection strings, see https://go.microsoft.com/fwlink/?LinkId=723263.</value>
     <comment>Localize the URL if we have localized docs.</comment>

src/EFCore.Design/Query/Internal/LinqToCSharpSyntaxTranslator.cs

@@ -10,6 +10,7 @@
 using Microsoft.CodeAnalysis.CSharp;
 using Microsoft.CodeAnalysis.CSharp.Syntax;
 using Microsoft.CodeAnalysis.Editing;
+using Microsoft.EntityFrameworkCore.Internal;
 using static Microsoft.CodeAnalysis.CSharp.SyntaxFactory;
 using ConditionalExpression = System.Linq.Expressions.ConditionalExpression;
 using E = System.Linq.Expressions.Expression;
@@ -223,8 +224,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(
@@ -288,14 +320,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,
@@ -313,9 +337,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,
@@ -328,7 +349,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);
 
@@ -349,8 +370,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,
@@ -376,7 +420,7 @@ Expression VisitAssignment(BinaryExpression assignment)
                 }
                 else
                 {
-                    Result = AssignmentExpression(SyntaxKind.SimpleAssignmentExpression, translatedLeft, translatedRight);
+                    Result = AssignmentExpression(kind, translatedLeft, translatedRight);
                 }
             }
 
@@ -422,7 +466,12 @@ protected override Expression VisitBlock(BlockExpression block)
                 }
                 else
                 {
-                    variables.Add(parameter, uniquifiedName);
+                    if (!variables.TryAdd(parameter, uniquifiedName))
+                    {
+                        throw new InvalidOperationException(
+                            DesignStrings.SameParameterExpressionDeclaredAsVariableInNestedBlocks(parameter.Name ?? "<null>"));
+                    }
+
                     variableNames.Add(uniquifiedName);
                 }
             }

test/EFCore.Design.Tests/Query/LinqToCSharpTranslatorTest.cs

@@ -64,11 +64,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()
@@ -88,6 +101,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")]