C#: Remove uses of expanded assignment.

Author: michaelnebel

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

@@ -379,7 +379,8 @@ module AssignableInternal {
       or
       exists(Assignment ass | ac = ass.getLValue() |
         result = ass.getRValue() and
-        not ass.(AssignOperation).hasExpandedAssignment()
+        // TODO: Maybe be an issue for event accessor calls.
+        not ass instanceof AssignOperation
       )
     }
   }

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

@@ -20,7 +20,7 @@ class ExprOrStmtParent extends Element, @exprorstmt_parent {
 
   /** Gets the `i`th child expression of this element (zero-based). */
   final Expr getChildExpr(int i) {
-    expr_parent_adjusted(result, i, this) or
+    expr_parent(result, i, this) or
     expr_parent_top_level_adjusted(result, i, this)
   }
 
@@ -119,66 +119,14 @@ private module Cached {
   }
 
   /**
-   * The `expr_parent()` relation adjusted for expandable assignments. For example,
-   * the assignment `x += y` is extracted as
-   *
-   * ```
-   *          +=
-   *           |
-   *           2
-   *           |
-   *           =
-   *          / \
-   *         1   0
-   *        /     \
-   *       x       +
-   *              / \
-   *             1   0
-   *            /     \
-   *           x       y
-   * ```
-   *
-   * in order to be able to retrieve the expanded assignment `x = x + y` as the 2nd
-   * child. This predicate changes the diagram above into
-   *
-   * ```
-   *          +=
-   *         /  \
-   *        1    0
-   *       /      \
-   *      x        y
-   * ```
+   * Use `expr_parent` instead.
    */
   cached
-  predicate expr_parent_adjusted(Expr child, int i, ControlFlowElement parent) {
-    if parent instanceof AssignOperation
-    then
-      parent =
-        any(AssignOperation ao |
-          exists(AssignExpr ae | ae = ao.getExpandedAssignment() |
-            i = 0 and
-            exists(Expr right |
-              // right = `x + y`
-              expr_parent(right, 0, ae)
-            |
-              expr_parent(child, 1, right)
-            )
-            or
-            i = 1 and
-            expr_parent(child, 1, ae)
-          )
-          or
-          not ao.hasExpandedAssignment() and
-          expr_parent(child, i, parent)
-        )
-    else expr_parent(child, i, parent)
-  }
+  deprecated predicate expr_parent_adjusted(Expr child, int i, ControlFlowElement parent) { none() }
 
   private Expr getAChildExpr(ExprOrStmtParent parent) {
     result = parent.getAChildExpr() and
     not result = parent.(DeclarationWithGetSetAccessors).getExpressionBody()
-    or
-    result = parent.(AssignOperation).getExpandedAssignment()
   }
 
   private ControlFlowElement getAChild(ExprOrStmtParent parent) {

csharp/ql/lib/semmle/code/csharp/controlflow/internal/ControlFlowGraphImpl.qll

@@ -89,18 +89,13 @@ class CfgScope extends Element, @top_level_exprorstmt_parent {
 
 private class TAstNode = @callable or @control_flow_element;
 
-private Element getAChild(Element p) {
-  result = p.getAChild() or
-  result = p.(AssignOperation).getExpandedAssignment()
-}
-
 pragma[nomagic]
 private predicate astNode(Element e) {
   e = any(@top_level_exprorstmt_parent p | not p instanceof Attribute)
   or
   exists(Element parent |
     astNode(parent) and
-    e = getAChild(parent)
+    e = parent.getAChild()
   )
 }
 
@@ -521,7 +516,6 @@ module Expressions {
       not this instanceof LogicalOrExpr and
       not this instanceof NullCoalescingExpr and
       not this instanceof ConditionalExpr and
-      not this instanceof AssignOperationWithExpandedAssignment and
       not this instanceof ConditionallyQualifiedExpr and
       not this instanceof ThrowExpr and
       not this instanceof ObjectCreation and
@@ -619,7 +613,7 @@ module Expressions {
       def.getExpr() = this and
       def.getTargetAccess().(WriteAccess) instanceof QualifiableExpr and
       not def instanceof AssignableDefinitions::OutRefDefinition and
-      not this instanceof AssignOperationWithExpandedAssignment
+      not def instanceof AssignableDefinitions::AssignOperationDefinition
     }
 
     /**
@@ -802,26 +796,6 @@ module Expressions {
     }
   }
 
-  /**
-   * An assignment operation that has an expanded version. We use the expanded
-   * version in the control flow graph in order to get better data flow / taint
-   * tracking.
-   */
-  private class AssignOperationWithExpandedAssignment extends ControlFlowTree instanceof AssignOperation
-  {
-    private Expr expanded;
-
-    AssignOperationWithExpandedAssignment() { expanded = this.getExpandedAssignment() }
-
-    final override predicate first(AstNode first) { first(expanded, first) }
-
-    final override predicate last(AstNode last, Completion c) { last(expanded, last, c) }
-
-    final override predicate propagatesAbnormal(AstNode child) { none() }
-
-    final override predicate succ(AstNode pred, AstNode succ, Completion c) { none() }
-  }
-
   /** A conditionally qualified expression. */
   private class ConditionallyQualifiedExpr extends PostOrderTree instanceof QualifiableExpr {
     private Expr qualifier;
@@ -1579,7 +1553,7 @@ module Statements {
   /** Gets a child of `cfe` that is in CFG scope `scope`. */
   pragma[noinline]
   private ControlFlowElement getAChildInScope(AstNode cfe, Callable scope) {
-    result = getAChild(cfe) and
+    result = cfe.getAChild() and
     scope = result.getEnclosingCallable()
   }
 

csharp/ql/lib/semmle/code/csharp/dispatch/Dispatch.qll

@@ -202,8 +202,7 @@ private module Internal {
   private predicate isPotentialEventCall(
     AssignArithmeticOperation aao, DynamicMemberAccess dma, string name
   ) {
-    exists(DynamicOperatorCall doc, AssignExpr ae |
-      ae = aao.getExpandedAssignment() and
+    exists(DynamicOperatorCall doc, AssignOperation ae |
       dma = ae.getLValue() and
       doc = ae.getRValue()
     |

csharp/ql/lib/semmle/code/csharp/exprs/Assignment.qll

@@ -71,26 +71,14 @@ class AssignOperation extends Assignment, OperatorCall, @assign_op_expr {
   override string getOperator() { none() }
 
   /**
-   * Gets the expanded version of this assignment operation, if any.
-   *
-   * For example, if this assignment operation is `x += y` then
-   * the expanded assignment is `x = x + y`.
-   *
-   * If an expanded version exists, then it is used in the control
-   * flow graph.
+   * Expanded versions of compound assignments are no longer extracted.
    */
-  AssignExpr getExpandedAssignment() { none() }
+  deprecated AssignExpr getExpandedAssignment() { none() }
 
   /**
-   * Holds if this assignment operation has an expanded version.
-   *
-   * For example, if this assignment operation is `x += y` then
-   * it has the expanded version `x = x + y`.
-   *
-   * If an expanded version exists, then it is used in the control
-   * flow graph.
+   * Expanded versions of compound assignments are no longer extracted.
    */
-  predicate hasExpandedAssignment() { exists(this.getExpandedAssignment()) }
+  deprecated predicate hasExpandedAssignment() { none() }
 
   override Expr getLeftOperand() { result = this.getChild(0) }
 

csharp/ql/lib/semmle/code/csharp/exprs/Expr.qll

@@ -65,25 +65,11 @@ class Expr extends ControlFlowElement, @expr {
   /** Gets the enclosing callable of this expression, if any. */
   override Callable getEnclosingCallable() { enclosingCallable(this, result) }
 
-  pragma[nomagic]
-  private predicate isExpandedAssignmentRValueDescendant() {
-    this =
-      any(AssignOperation op).getExpandedAssignment().getRValue().getChildExpr(0).getAChildExpr()
-    or
-    exists(Expr parent |
-      parent.isExpandedAssignmentRValueDescendant() and
-      this = parent.getAChildExpr()
-    )
-  }
-
   /**
    * Holds if this expression is generated by the compiler and does not appear
    * explicitly in the source code.
    */
-  final predicate isImplicit() {
-    compiler_generated(this) or
-    this.isExpandedAssignmentRValueDescendant()
-  }
+  final predicate isImplicit() { compiler_generated(this) }
 
   /**
    * Gets an expression that is the result of stripping (recursively) all

csharp/ql/src/Likely Bugs/DangerousNonShortCircuitLogic.ql

@@ -18,14 +18,9 @@ import csharp
 /** A use of `&` or `|` on operands of type boolean. */
 class NonShortCircuit extends BinaryBitwiseOperation {
   NonShortCircuit() {
-    (
-      this instanceof BitwiseAndExpr
-      or
-      this instanceof BitwiseOrExpr
-    ) and
-    not exists(AssignBitwiseOperation abo | abo.getExpandedAssignment().getRValue() = this) and
-    this.getLeftOperand().getType() instanceof BoolType and
-    this.getRightOperand().getType() instanceof BoolType
+    this instanceof BitwiseAndExpr
+    or
+    this instanceof BitwiseOrExpr
   }
 
   pragma[nomagic]

csharp/ql/src/Performance/StringConcatenationInLoop.ql

@@ -23,7 +23,6 @@ class StringCat extends AddExpr {
  * where `v` is a simple variable (and not, for example, a property).
  */
 predicate isSelfConcatAssignExpr(AssignExpr e, Variable v) {
-  not e = any(AssignAddExpr a).getExpandedAssignment() and
   exists(VariableAccess use |
     stringCatContains(e.getRValue(), use) and
     use.getTarget() = e.getTargetVariable() and

csharp/ql/src/Security Features/InsecureRandomness.ql

@@ -89,11 +89,7 @@ module Random {
         e = any(SensitiveLibraryParameter v).getAnAssignedArgument()
         or
         // Assignment operation, e.g. += or similar
-        exists(AssignOperation ao |
-          ao.getRValue() = e and
-          // "expanded" assignments will be covered by simple assignment
-          not ao.hasExpandedAssignment()
-        |
+        exists(AssignOperation ao | ao.getRValue() = e |
           ao.getLValue() = any(SensitiveVariable v).getAnAccess() or
           ao.getLValue() = any(SensitiveProperty v).getAnAccess() or
           ao.getLValue() = any(SensitiveLibraryParameter v).getAnAccess()