C#: Improve dataflow for mutation definition and mutation operator calls.

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

@@ -117,6 +117,63 @@ class AssignableRead extends AssignableAccess {
   AssignableRead getANextRead() { result.getControlFlowNode() = this.getAnAdjacentReadSameVar() }
 }
 
+private newtype TMutationOperationAssignment =
+  TBuiltInMutationOperation(MutatorOperation mo) or
+  TUserMutatorOperatorCall(MutatorOperatorCall moc) {
+    any()
+    // not moc instanceof InstanceMutatorOperatorCall
+  }
+
+/**
+ * A mutation operation that implicitly assigns the result to its operand. For example, `a++` in
+ * line 7 in
+ *
+ * ```csharp
+ * class A {
+ *   public static A operator++(A a) {
+ *     return a;
+ *   }
+ *
+ *   public static A Increment(A a) {
+ *     return a++;
+ *   }
+ * }
+ * ```
+ */
+private class MutationOperationAssignment extends TMutationOperationAssignment {
+  string toString() { none() }
+
+  Expr getOperand() { none() }
+
+  Expr getMutationOperation() { none() }
+}
+
+private class BuiltInMutationOperation extends MutationOperationAssignment,
+  TBuiltInMutationOperation
+{
+  private MutatorOperation mo;
+
+  BuiltInMutationOperation() { this = TBuiltInMutationOperation(mo) }
+
+  override string toString() { result = mo.toString() }
+
+  override Expr getOperand() { result = mo.getOperand() }
+
+  override Expr getMutationOperation() { result = mo }
+}
+
+private class UserMutatorOperatorCall extends MutationOperationAssignment, TUserMutatorOperatorCall {
+  private MutatorOperatorCall moc;
+
+  UserMutatorOperatorCall() { this = TUserMutatorOperatorCall(moc) }
+
+  override string toString() { result = moc.toString() }
+
+  override Expr getOperand() { result = moc.getArgument(0) }
+
+  override Expr getMutationOperation() { result = moc }
+}
+
 /**
  * An access to an assignable that updates the underlying value. Either a
  * variable write (`VariableWrite`), a property write (`PropertyWrite`),
@@ -262,7 +319,8 @@ module AssignableInternal {
     or
     def = TOutRefDefinition(any(AssignableAccess aa | result = aa.getParent()))
     or
-    def = TMutationDefinition(result)
+    def =
+      TMutationDefinition(any(MutationOperationAssignment moa | result = moa.getMutationOperation()))
     or
     def = TLocalVariableDefinition(result)
     or
@@ -299,7 +357,7 @@ module AssignableInternal {
         or
         aa.(RefArg).isPotentialAssignment()
       } or
-      TMutationDefinition(MutatorOperation mo) or
+      TMutationDefinition(MutationOperationAssignment mo) or
       TLocalVariableDefinition(LocalVariableDeclExpr lvde) {
         not lvde.hasInitializer() and
         not exists(getTupleSource(TTupleAssignmentDefinition(_, lvde))) and
@@ -372,7 +430,7 @@ module AssignableInternal {
       or
       def = TOutRefDefinition(result)
       or
-      def = TMutationDefinition(any(MutatorOperation mo | mo.getOperand() = result))
+      def = TMutationDefinition(any(MutationOperationAssignment mo | mo.getOperand() = result))
       or
       def = TAddressOfDefinition(any(AddressOfExpr aoe | aoe.getOperand() = result))
       or
@@ -645,14 +703,26 @@ module AssignableDefinitions {
    * A definition by mutation, for example `x++`.
    */
   class MutationDefinition extends AssignableDefinition, TMutationDefinition {
-    MutatorOperation mo;
+    MutationOperationAssignment moa;
+
+    MutationDefinition() { this = TMutationDefinition(moa) }
+
+    /**
+     * DEPRECATED: Use `getMutationOperation()` instead.
+     *
+     * Gets the underlying mutator operation.
+     */
+    deprecated MutatorOperation getMutatorOperation() { moa = TBuiltInMutationOperation(result) }
 
-    MutationDefinition() { this = TMutationDefinition(mo) }
+    /**
+     * Gets the underlying mutation operation.
+     */
+    Expr getMutationOperation() { result = moa.getMutationOperation() }
 
-    /** Gets the underlying mutator operation. */
-    MutatorOperation getMutatorOperation() { result = mo }
+    // TODO: Removing this below
+    override Expr getSource() { result = this.getMutationOperation() }
 
-    override string toString() { result = mo.toString() }
+    override string toString() { result = moa.toString() }
   }
 
   /**

csharp/ql/lib/semmle/code/csharp/dataflow/internal/rangeanalysis/SignAnalysisSpecific.qll

@@ -125,7 +125,9 @@ private module Impl {
   SsaExplicitWrite getExplicitSsaAssignment(SsaExplicitWrite v) { result = v }
 
   /** Returns the assignment of the variable update `def`. */
-  ExprNode getExprFromSsaAssignment(SsaExplicitWrite def) { result.getExpr() = def.getValue() }
+  ExprNode getExprFromSsaAssignment(SsaExplicitWrite def) {
+    result.getExpr() = def.getValue() and not def.getDefiningExpr() instanceof MutatorOperation
+  }
 
   /** Holds if `def` can have any sign. */
   predicate explicitSsaDefWithAnySign(SsaExplicitWrite def) {

csharp/ql/test/library-tests/dataflow/operators/Operator.cs

@@ -120,3 +120,49 @@ public void M1()
         Sink(x.Field); // $ hasValueFlow=1
     }
 }
+
+public class MutatorOperators
+{
+    static void Sink(object o) { }
+    static T Source<T>(object source) => throw null;
+
+    public class C1
+    {
+        public object Field { get; private set; }
+
+        public C1()
+        {
+            Field = new object();
+        }
+
+        public C1(object o)
+        {
+            Field = o;
+        }
+
+        public void operator ++()
+        {
+            Field = Source<object>(1);
+        }
+
+        public static C1 operator --(C1 x)
+        {
+            var f = Source<object>(2);
+            return new C1(f);
+        }
+
+        public void M1()
+        {
+            var x = new C1();
+            x++;
+            Sink(x.Field); // $ hasValueFlow=1
+        }
+
+        public void M2()
+        {
+            var x = new C1();
+            x--;
+            Sink(x.Field); // $ hasValueFlow=2
+        }
+    }
+}