Merge pull request #21158 from michaelnebel/csharp/cfgforaccess

C# 14: Null conditional assignments.

Author: michaelnebel

csharp/ql/lib/change-notes/2026-01-14-null-conditional-assignments.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* C# 14: Support for null-conditional assignments (such as `c?.Prop = p`). Furthermore, the `MaybeNullExpr` class now takes null-conditional access (such as `?.`) into account when modeling potential null values.

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

@@ -429,7 +429,7 @@ module Expressions {
       not this instanceof ObjectCreation and
       not this instanceof ArrayCreation and
       not this instanceof QualifiedWriteAccess and
-      not this instanceof AccessorWrite and
+      not this instanceof QualifiedAccessorWrite and
       not this instanceof NoNodeExpr and
       not this instanceof SwitchExpr and
       not this instanceof SwitchCaseExpr and
@@ -446,21 +446,29 @@ module Expressions {
   }
 
   /**
-   * A qualified write access. In a qualified write access, the access itself is
-   * not evaluated, only the qualifier and the indexer arguments (if any).
+   * A qualified write access.
+   *
+   * The successor declaration in `QualifiedAccessorWrite` ensures that the access itself
+   * is evaluated after the qualifier and the indexer arguments (if any)
+   * and the right hand side of the assignment.
+   *
+   * When a qualified write access is used as an `out/ref` argument, the access itself is evaluated immediately.
    */
   private class QualifiedWriteAccess extends ControlFlowTree instanceof WriteAccess, QualifiableExpr
   {
     QualifiedWriteAccess() {
-      this.hasQualifier()
-      or
-      // Member initializers like
-      // ```csharp
-      // new Dictionary<int, string>() { [0] = "Zero", [1] = "One", [2] = "Two" }
-      // ```
-      // need special treatment, because the accesses `[0]`, `[1]`, and `[2]`
-      // have no qualifier.
-      this = any(MemberInitializer mi).getLValue()
+      (
+        this.hasQualifier()
+        or
+        // Member initializers like
+        // ```csharp
+        // new Dictionary<int, string>() { [0] = "Zero", [1] = "One", [2] = "Two" }
+        // ```
+        // need special treatment, because the accesses `[0]`, `[1]`, and `[2]`
+        // have no qualifier.
+        this = any(MemberInitializer mi).getLValue()
+      ) and
+      not exists(AssignableDefinitions::OutRefDefinition def | def.getTargetAccess() = this)
     }
 
     final override predicate propagatesAbnormal(AstNode child) { child = getExprChild(this, _) }
@@ -470,25 +478,25 @@ module Expressions {
     final override predicate last(AstNode last, Completion c) {
       // Skip the access in a qualified write access
       last(getLastExprChild(this), last, c)
+      or
+      // Qualifier exits with a null completion
+      super.isConditional() and
+      last(super.getQualifier(), last, c) and
+      c.(NullnessCompletion).isNull()
     }
 
     final override predicate succ(AstNode pred, AstNode succ, Completion c) {
       exists(int i |
         last(getExprChild(this, i), pred, c) and
         c instanceof NormalCompletion and
+        (if i = 0 then not c.(NullnessCompletion).isNull() else any()) and
         first(getExprChild(this, i + 1), succ)
       )
     }
   }
 
-  private class StatOrDynAccessorCall_ =
-    @dynamic_member_access_expr or @dynamic_element_access_expr or @call_access_expr;
-
-  /** A normal or a (potential) dynamic call to an accessor. */
-  private class StatOrDynAccessorCall extends Expr, StatOrDynAccessorCall_ { }
-
   /**
-   * An expression that writes via an accessor call, for example `x.Prop = 0`,
+   * An expression that writes via a qualifiable expression, for example `x.Prop = 0`,
    * where `Prop` is a property.
    *
    * Accessor writes need special attention, because we need to model the fact
@@ -498,24 +506,33 @@ module Expressions {
    * ```csharp
    * x -> 0 -> set_Prop -> x.Prop = 0
    * ```
+   *
+   * For consistency, control flow is implemented the same way for other qualified writes.
+   * For example, `x.Field = 0`, where `Field` is a field, we want a CFG that looks like
+   *
+   * ```csharp
+   * x -> 0 -> x.Field -> x.Field = 0
+   * ```
    */
-  class AccessorWrite extends PostOrderTree instanceof Expr {
+  private class QualifiedAccessorWrite extends PostOrderTree instanceof Expr {
     AssignableDefinition def;
 
-    AccessorWrite() {
+    QualifiedAccessorWrite() {
       def.getExpr() = this and
-      def.getTargetAccess().(WriteAccess) instanceof StatOrDynAccessorCall and
+      def.getTargetAccess().(WriteAccess) instanceof QualifiableExpr and
+      not def instanceof AssignableDefinitions::OutRefDefinition and
       not this instanceof AssignOperationWithExpandedAssignment
     }
 
     /**
      * Gets the `i`th accessor being called in this write. More than one call
      * can happen in tuple assignments.
      */
-    StatOrDynAccessorCall getCall(int i) {
+    QualifiableExpr getAccess(int i) {
       result =
         rank[i + 1](AssignableDefinitions::TupleAssignmentDefinition tdef |
-          tdef.getExpr() = this and tdef.getTargetAccess() instanceof StatOrDynAccessorCall
+          tdef.getExpr() = this and
+          tdef.getTargetAccess() instanceof QualifiableExpr
         |
           tdef order by tdef.getEvaluationOrder()
         ).getTargetAccess()
@@ -528,7 +545,13 @@ module Expressions {
     final override predicate propagatesAbnormal(AstNode child) {
       child = getExprChild(this, _)
       or
-      child = this.getCall(_)
+      child = this.getAccess(_)
+    }
+
+    final override predicate last(AstNode last, Completion c) {
+      PostOrderTree.super.last(last, c)
+      or
+      last(getExprChild(this, 0), last, c) and c.(NullnessCompletion).isNull()
     }
 
     final override predicate first(AstNode first) { first(getExprChild(this, 0), first) }
@@ -538,24 +561,25 @@ module Expressions {
       exists(int i |
         last(getExprChild(this, i), pred, c) and
         c instanceof NormalCompletion and
+        (if i = 0 then not c.(NullnessCompletion).isNull() else any()) and
         first(getExprChild(this, i + 1), succ)
       )
       or
       // Flow from last element of last child to first accessor call
       last(getLastExprChild(this), pred, c) and
-      succ = this.getCall(0) and
+      succ = this.getAccess(0) and
       c instanceof NormalCompletion
       or
       // Flow from one call to the next
-      exists(int i | pred = this.getCall(i) |
-        succ = this.getCall(i + 1) and
+      exists(int i | pred = this.getAccess(i) |
+        succ = this.getAccess(i + 1) and
         c.isValidFor(pred) and
         c instanceof NormalCompletion
       )
       or
       // Post-order: flow from last call to element itself
-      exists(int last | last = max(int i | exists(this.getCall(i))) |
-        pred = this.getCall(last) and
+      exists(int last | last = max(int i | exists(this.getAccess(i))) |
+        pred = this.getAccess(last) and
         succ = this and
         c.isValidFor(pred) and
         c instanceof NormalCompletion
@@ -704,7 +728,9 @@ module Expressions {
   private class ConditionallyQualifiedExpr extends PostOrderTree instanceof QualifiableExpr {
     private Expr qualifier;
 
-    ConditionallyQualifiedExpr() { this.isConditional() and qualifier = getExprChild(this, 0) }
+    ConditionallyQualifiedExpr() {
+      this.isConditional() and qualifier = getExprChild(this, 0) and not this instanceof WriteAccess
+    }
 
     final override predicate propagatesAbnormal(AstNode child) { child = qualifier }
 

csharp/ql/lib/semmle/code/csharp/dataflow/Nullness.qll

@@ -43,6 +43,13 @@ private Expr maybeNullExpr(Expr reason) {
     )
   or
   result.(NullCoalescingExpr).getRightOperand() = maybeNullExpr(reason)
+  or
+  result =
+    any(QualifiableExpr qe |
+      qe.isConditional() and
+      reason = qe.getQualifier() and
+      not qe instanceof AssignableWrite
+    )
 }
 
 /** An expression that may be `null`. */

csharp/ql/test/library-tests/controlflow/graph/Assignments.cs

@@ -21,4 +21,26 @@ void M()
 
     delegate void EventHandler(object sender, object e);
     event EventHandler Event;
+    int IntField;
+    string StringField;
+
+    void SetParamSingle(out int x)
+    {
+        x = 42;
+    }
+
+    void SetParamMulti(out int x, object o, out string y)
+    {
+        x = 42;
+        y = "Hello";
+    }
+
+    void M2()
+    {
+        int x1;
+        SetParamSingle(out x1);
+        SetParamSingle(out IntField);
+        SetParamMulti(out var y, null, out StringField);
+        SetParamMulti(out IntField, null, out StringField);
+    }
 }