C#: Remove expanded assignments.

csharp/extractor/Semmle.Extraction.CSharp/Entities/Expressions/Assignment.cs

@@ -22,26 +22,12 @@ public static Assignment Create(ExpressionNodeInfo info)
 
         protected override void PopulateExpression(TextWriter trapFile)
         {
-            var operatorKind = OperatorKind;
-            if (operatorKind.HasValue)
-            {
-                // Convert assignment such as `a += b` into `a = a + b`.
-                var simpleAssignExpr = new Expression(new ExpressionInfo(Context, Type, Location, ExprKind.SIMPLE_ASSIGN, this, 2, isCompilerGenerated: true, null));
-                Create(Context, Syntax.Left, simpleAssignExpr, 1);
-                var opexpr = new Expression(new ExpressionInfo(Context, Type, Location, operatorKind.Value, simpleAssignExpr, 0, isCompilerGenerated: true, null));
-                Create(Context, Syntax.Left, opexpr, 0, isCompilerGenerated: true);
-                Create(Context, Syntax.Right, opexpr, 1);
-                opexpr.OperatorCall(trapFile, Syntax);
-            }
-            else
-            {
-                Create(Context, Syntax.Left, this, 1);
-                Create(Context, Syntax.Right, this, 0);
+            Create(Context, Syntax.Left, this, 0);
+            Create(Context, Syntax.Right, this, 1);
 
-                if (Kind == ExprKind.ADD_EVENT || Kind == ExprKind.REMOVE_EVENT)
-                {
-                    OperatorCall(trapFile, Syntax);
-                }
+            if (Kind != ExprKind.SIMPLE_ASSIGN && Kind != ExprKind.ASSIGN_COALESCE)
+            {
+                OperatorCall(trapFile, Syntax);
             }
         }
 
@@ -108,56 +94,5 @@ private static ExprKind GetKind(Context cx, AssignmentExpressionSyntax syntax)
 
             return kind;
         }
-
-        /// <summary>
-        /// Gets the kind of this assignment operator (<code>null</code> if the
-        /// assignment is not an assignment operator). For example, the operator
-        /// kind of `*=` is `*`.
-        /// </summary>
-        private ExprKind? OperatorKind
-        {
-            get
-            {
-                var kind = Kind;
-                if (kind == ExprKind.REMOVE_EVENT || kind == ExprKind.ADD_EVENT || kind == ExprKind.SIMPLE_ASSIGN)
-                    return null;
-
-                if (CallType.AdjustKind(kind) == ExprKind.OPERATOR_INVOCATION)
-                    return ExprKind.OPERATOR_INVOCATION;
-
-                switch (kind)
-                {
-                    case ExprKind.ASSIGN_ADD:
-                        return ExprKind.ADD;
-                    case ExprKind.ASSIGN_AND:
-                        return ExprKind.BIT_AND;
-                    case ExprKind.ASSIGN_DIV:
-                        return ExprKind.DIV;
-                    case ExprKind.ASSIGN_LSHIFT:
-                        return ExprKind.LSHIFT;
-                    case ExprKind.ASSIGN_MUL:
-                        return ExprKind.MUL;
-                    case ExprKind.ASSIGN_OR:
-                        return ExprKind.BIT_OR;
-                    case ExprKind.ASSIGN_REM:
-                        return ExprKind.REM;
-                    case ExprKind.ASSIGN_RSHIFT:
-                        return ExprKind.RSHIFT;
-                    case ExprKind.ASSIGN_URSHIFT:
-                        return ExprKind.URSHIFT;
-                    case ExprKind.ASSIGN_SUB:
-                        return ExprKind.SUB;
-                    case ExprKind.ASSIGN_XOR:
-                        return ExprKind.BIT_XOR;
-                    case ExprKind.ASSIGN_COALESCE:
-                        return ExprKind.NULL_COALESCING;
-                    default:
-                        Context.ModelError(Syntax, $"Couldn't unfold assignment of type {kind}");
-                        return ExprKind.UNKNOWN;
-                }
-            }
-        }
-
-        public new CallType CallType => GetCallType(Context, Syntax);
     }
 }

csharp/extractor/Semmle.Extraction.CSharp/Entities/Expressions/Initializer.cs

@@ -83,30 +83,31 @@ protected override void PopulateExpression(TextWriter trapFile)
                 {
                     var assignmentInfo = new ExpressionNodeInfo(Context, init, this, child++).SetKind(ExprKind.SIMPLE_ASSIGN);
                     var assignmentEntity = new Expression(assignmentInfo);
-                    var typeInfoRight = Context.GetTypeInfo(assignment.Right);
-                    if (typeInfoRight.Type is null)
-                        // The type may be null for nested initializers such as
-                        // ```csharp
-                        // new ClassWithArrayField() { As = { [0] = a } }
-                        // ```
-                        // In this case we take the type from the assignment
-                        // `As = { [0] = a }` instead
-                        typeInfoRight = assignmentInfo.TypeInfo;
-                    CreateFromNode(new ExpressionNodeInfo(Context, assignment.Right, assignmentEntity, 0, typeInfoRight));
-
                     var target = Context.GetSymbolInfo(assignment.Left);
 
                     // If the target is null, then assume that this is an array initializer (of the form `[...] = ...`)
-
                     var access = target.Symbol is null ?
-                        new Expression(new ExpressionNodeInfo(Context, assignment.Left, assignmentEntity, 1).SetKind(ExprKind.ARRAY_ACCESS)) :
-                        Access.Create(new ExpressionNodeInfo(Context, assignment.Left, assignmentEntity, 1), target.Symbol, false, Context.CreateEntity(target.Symbol));
+                        new Expression(new ExpressionNodeInfo(Context, assignment.Left, assignmentEntity, 0).SetKind(ExprKind.ARRAY_ACCESS)) :
+                        Access.Create(new ExpressionNodeInfo(Context, assignment.Left, assignmentEntity, 0), target.Symbol, false, Context.CreateEntity(target.Symbol));
 
                     if (assignment.Left is ImplicitElementAccessSyntax iea)
                     {
                         // An array/indexer initializer of the form `[...] = ...`
                         access.PopulateArguments(trapFile, iea.ArgumentList.Arguments, 0);
                     }
+
+                    var typeInfoRight = Context.GetTypeInfo(assignment.Right);
+                    if (typeInfoRight.Type is null)
+                    {
+                        // The type may be null for nested initializers such as
+                        // ```csharp
+                        // new ClassWithArrayField() { As = { [0] = a } }
+                        // ```
+                        // In this case we take the type from the assignment
+                        // `As = { [0] = a }` instead
+                        typeInfoRight = assignmentInfo.TypeInfo;
+                    }
+                    CreateFromNode(new ExpressionNodeInfo(Context, assignment.Right, assignmentEntity, 1, typeInfoRight));
                 }
                 else
                 {

csharp/extractor/Semmle.Extraction.CSharp/Entities/Expressions/ObjectCreation/AnonymousObjectCreation.cs

@@ -41,11 +41,11 @@ protected override void PopulateExpression(TextWriter trapFile)
                 var loc = Context.CreateLocation(init.GetLocation());
 
                 var assignment = new Expression(new ExpressionInfo(Context, type, loc, ExprKind.SIMPLE_ASSIGN, objectInitializer, child++, isCompilerGenerated: false, null));
-                Create(Context, init.Expression, assignment, 0);
                 Property.Create(Context, property);
-
-                var access = new Expression(new ExpressionInfo(Context, type, loc, ExprKind.PROPERTY_ACCESS, assignment, 1, isCompilerGenerated: false, null));
+                var access = new Expression(new ExpressionInfo(Context, type, loc, ExprKind.PROPERTY_ACCESS, assignment, 0, isCompilerGenerated: false, null));
                 trapFile.expr_access(access, propEntity);
+
+                Create(Context, init.Expression, assignment, 1);
             }
         }
     }

csharp/extractor/Semmle.Extraction.CSharp/Entities/Expressions/Query.cs

@@ -94,12 +94,12 @@ protected Expression DeclareRangeVariable(Context cx, IExpressionParentEntity pa
                     child
                     );
 
-                Expression.Create(cx, Expr, decl, 0);
-
                 var nameLoc = cx.CreateLocation(name.GetLocation());
-                var access = new Expression(new ExpressionInfo(cx, type, nameLoc, ExprKind.LOCAL_VARIABLE_ACCESS, decl, 1, isCompilerGenerated: false, null));
+                var access = new Expression(new ExpressionInfo(cx, type, nameLoc, ExprKind.LOCAL_VARIABLE_ACCESS, decl, 0, isCompilerGenerated: false, null));
                 cx.TrapWriter.Writer.expr_access(access, LocalVariable.Create(cx, variableSymbol));
 
+                Expression.Create(cx, Expr, decl, 1);
+
                 return decl;
             }
 

csharp/extractor/Semmle.Extraction.CSharp/Entities/Expressions/VariableDeclaration.cs

@@ -176,11 +176,11 @@ public static VariableDeclaration CreateDeclarator(Context cx, VariableDeclarato
 
                 if (d.Initializer is not null)
                 {
-                    Create(cx, d.Initializer.Value, ret, 0);
-
                     // Create an access
-                    var access = new Expression(new ExpressionInfo(cx, type, localVar.Location, ExprKind.LOCAL_VARIABLE_ACCESS, ret, 1, isCompilerGenerated: false, null));
+                    var access = new Expression(new ExpressionInfo(cx, type, localVar.Location, ExprKind.LOCAL_VARIABLE_ACCESS, ret, 0, isCompilerGenerated: false, null));
                     cx.TrapWriter.Writer.expr_access(access, localVar);
+
+                    Create(cx, d.Initializer.Value, ret, 1);
                 }
 
                 if (d.Parent is VariableDeclarationSyntax decl)

csharp/extractor/Semmle.Extraction.CSharp/Entities/Field.cs

@@ -116,9 +116,9 @@ private Expression AddInitializerAssignment(TextWriter trapFile, ExpressionSynta
         {
             var type = Symbol.GetAnnotatedType();
             var simpleAssignExpr = new Expression(new ExpressionInfo(Context, type, loc, ExprKind.SIMPLE_ASSIGN, this, child++, isCompilerGenerated: true, constValue));
-            Expression.CreateFromNode(new ExpressionNodeInfo(Context, initializer, simpleAssignExpr, 0));
-            var access = new Expression(new ExpressionInfo(Context, type, Location, ExprKind.FIELD_ACCESS, simpleAssignExpr, 1, isCompilerGenerated: true, constValue));
+            var access = new Expression(new ExpressionInfo(Context, type, Location, ExprKind.FIELD_ACCESS, simpleAssignExpr, 0, isCompilerGenerated: true, constValue));
             trapFile.expr_access(access, this);
+            Expression.CreateFromNode(new ExpressionNodeInfo(Context, initializer, simpleAssignExpr, 1));
             return access;
         }
 

csharp/extractor/Semmle.Extraction.CSharp/Entities/Property.cs

@@ -94,9 +94,9 @@ public override void Populate(TextWriter trapFile)
                         var loc = Context.CreateLocation(initializer!.GetLocation());
                         var annotatedType = AnnotatedTypeSymbol.CreateNotAnnotated(Symbol.Type);
                         var simpleAssignExpr = new Expression(new ExpressionInfo(Context, annotatedType, loc, ExprKind.SIMPLE_ASSIGN, this, child++, isCompilerGenerated: true, null));
-                        Expression.CreateFromNode(new ExpressionNodeInfo(Context, initializer.Value, simpleAssignExpr, 0));
-                        var access = new Expression(new ExpressionInfo(Context, annotatedType, Location, ExprKind.PROPERTY_ACCESS, simpleAssignExpr, 1, isCompilerGenerated: true, null));
+                        var access = new Expression(new ExpressionInfo(Context, annotatedType, Location, ExprKind.PROPERTY_ACCESS, simpleAssignExpr, 0, isCompilerGenerated: true, null));
                         trapFile.expr_access(access, this);
+                        Expression.CreateFromNode(new ExpressionNodeInfo(Context, initializer.Value, simpleAssignExpr, 1));
                         if (!Symbol.IsStatic)
                         {
                             This.CreateImplicit(Context, Symbol.ContainingType, Location, access, -1);

csharp/ql/campaigns/Solorigate/src/ModifiedFnvFunctionDetection.ql

@@ -16,16 +16,9 @@ import experimental.code.csharp.Cryptography.NonCryptographicHashes
 from Variable v, Literal l, LoopStmt loop, Expr additional_xor
 where
   maybeUsedInFnvFunction(v, _, _, loop) and
-  (
-    exists(BitwiseXorExpr xor2 | xor2.getAnOperand() = l and additional_xor = xor2 |
-      loop.getAControlFlowExitNode().getASuccessor*() = xor2.getAControlFlowNode() and
-      xor2.getAnOperand() = v.getAnAccess()
-    )
-    or
-    exists(AssignXorExpr xor2 | xor2.getAnOperand() = l and additional_xor = xor2 |
-      loop.getAControlFlowExitNode().getASuccessor*() = xor2.getAControlFlowNode() and
-      xor2.getAnOperand() = v.getAnAccess()
-    )
+  exists(BitwiseXorOperation xor2 | xor2.getAnOperand() = l and additional_xor = xor2 |
+    loop.getAControlFlowExitNode().getASuccessor*() = xor2.getAControlFlowNode() and
+    xor2.getAnOperand() = v.getAnAccess()
   )
 select l, "This literal is used in an $@ after an FNV-like hash calculation with variable $@.",
   additional_xor, "additional xor", v, v.toString()

csharp/ql/lib/experimental/code/csharp/Cryptography/NonCryptographicHashes.qll

@@ -48,7 +48,7 @@ private predicate maybeUsedInElfHashFunction(Variable v, Operation xor, Operatio
     Expr e1, Expr e2, AssignExpr addAssign, AssignExpr xorAssign, Operation notOp,
     AssignExpr notAssign
   |
-    (add instanceof AddExpr or add instanceof AssignAddExpr) and
+    add instanceof AddOperation and
     e1.getAChild*() = add.getAnOperand() and
     e1 instanceof BinaryBitwiseOperation and
     e2 = e1.(BinaryBitwiseOperation).getLeftOperand() and

csharp/ql/lib/semmle/code/csharp/Assignable.qll

@@ -78,6 +78,8 @@
       this.isRefArgument()
       or
       this = any(AssignableDefinitions::AddressOfDefinition def).getTargetAccess()
+      or
+      this = any(AssignableDefinitions::AssignOperationDefinition def).getTargetAccess()
     ) and
     not nameOfChild(_, this)
   }
@@ -271,6 +273,8 @@
     def = TAddressOfDefinition(result)
     or
     def = TPatternDefinition(result)
+    or
+    def = TAssignOperationDefinition(result)
   }
 
   /** A local variable declaration at the top-level of a pattern. */
@@ -286,7 +290,11 @@
   private module Cached {
     cached
     newtype TAssignableDefinition =
-      TAssignmentDefinition(Assignment a) { not a.getLValue() instanceof TupleExpr } or
+      TAssignmentDefinition(Assignment a) {
+        not a.getLValue() instanceof TupleExpr and
+        not a instanceof AssignCallOperation and
+        not a instanceof AssignCoalesceExpr
+      } or
       TTupleAssignmentDefinition(AssignExpr ae, Expr leaf) { tupleAssignmentDefinition(ae, leaf) } or
       TOutRefDefinition(AssignableAccess aa) {
         aa.isOutArgument()
@@ -309,7 +317,11 @@
         )
       } or
       TAddressOfDefinition(AddressOfExpr aoe) or
-      TPatternDefinition(TopLevelPatternDecl tlpd)
+      TPatternDefinition(TopLevelPatternDecl tlpd) or
+      TAssignOperationDefinition(AssignOperation ao) {
+        ao instanceof AssignCallOperation or
+        ao instanceof AssignCoalesceExpr
+      }
 
     /**
      * Gets the source expression assigned in tuple definition `def`, if any.
@@ -355,6 +367,8 @@
       def = TMutationDefinition(any(MutatorOperation mo | mo.getOperand() = result))
       or
       def = TAddressOfDefinition(any(AddressOfExpr aoe | aoe.getOperand() = result))
+      or
+      def = TAssignOperationDefinition(any(AssignOperation ao | ao.getLeftOperand() = result))
     }
 
     /**
@@ -369,8 +383,10 @@
       or
       exists(Assignment ass | ac = ass.getLValue() |
         result = ass.getRValue() and
-        not ass.(AssignOperation).hasExpandedAssignment()
+        not ass instanceof AssignOperation
       )
+      or
+      exists(AssignOperation ao | ac = ao.getLeftOperand() | result = ao)
     }
   }
 
@@ -388,8 +404,9 @@
  * a mutation update (`AssignableDefinitions::MutationDefinition`), a local variable
  * declaration without an initializer (`AssignableDefinitions::LocalVariableDefinition`),
  * an implicit parameter definition (`AssignableDefinitions::ImplicitParameterDefinition`),
- * an address-of definition (`AssignableDefinitions::AddressOfDefinition`), or a pattern
- * definition (`AssignableDefinitions::PatternDefinition`).
+ * an address-of definition (`AssignableDefinitions::AddressOfDefinition`), a pattern
+ * definition (`AssignableDefinitions::PatternDefinition`), or a compound assignment
+ * operation definition (`AssignableDefinitions::AssignOperationDefinition`)
  */
 class AssignableDefinition extends TAssignableDefinition {
   /**
@@ -511,7 +528,7 @@
 
     override Expr getSource() {
       result = a.getRValue() and
-      not a instanceof AssignOperation
+      not a instanceof AddOrRemoveEventExpr
     }
 
     override string toString() { result = a.toString() }
@@ -735,4 +752,17 @@
     /** Gets the assignable (field or property) being initialized. */
     Assignable getAssignable() { result = fieldOrProp }
   }
+
+  /**
+   * A definition by a compound assignment operation, for example `x += y`.
+   */
+  class AssignOperationDefinition extends AssignableDefinition, TAssignOperationDefinition {
+    AssignOperation ao;
+
+    AssignOperationDefinition() { this = TAssignOperationDefinition(ao) }
+
+    override Expr getSource() { result = ao }
+
+    override string toString() { result = ao.toString() }
+  }
 }