C#: Prototype breaking pattern assignments into multiple steps.

Author: michaelnebel

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

@@ -273,11 +273,17 @@ module AssignableInternal {
     def = TPatternDefinition(result)
   }
 
-  /** A local variable declaration at the top-level of a pattern. */
-  class TopLevelPatternDecl extends LocalVariableDeclExpr {
+  /** A pattern containing a local variable declaration. */
+  class LocalVariablePatternDecl extends LocalVariableDeclExpr {
     private PatternMatch pm;
 
-    TopLevelPatternDecl() { this = pm.getPattern().(BindingPatternExpr).getVariableDeclExpr() }
+    LocalVariablePatternDecl() {
+      exists(BindingPatternExpr bpe |
+        this = bpe.getVariableDeclExpr() and pm = bpe.getPatternMatch()
+      )
+    }
+
+    predicate isTopLevel() { this = pm.getPattern().(BindingPatternExpr).getVariableDeclExpr() }
 
     PatternMatch getMatch() { result = pm }
   }
@@ -297,7 +303,7 @@ module AssignableInternal {
       TLocalVariableDefinition(LocalVariableDeclExpr lvde) {
         not lvde.hasInitializer() and
         not exists(getTupleSource(TTupleAssignmentDefinition(_, lvde))) and
-        not lvde instanceof TopLevelPatternDecl and
+        not lvde instanceof LocalVariablePatternDecl and
         not lvde.isOutArgument()
       } or
       TImplicitParameterDefinition(Parameter p) {
@@ -309,7 +315,7 @@ module AssignableInternal {
         )
       } or
       TAddressOfDefinition(AddressOfExpr aoe) or
-      TPatternDefinition(TopLevelPatternDecl tlpd)
+      TPatternDefinition(LocalVariablePatternDecl lvpd)
 
     /**
      * Gets the source expression assigned in tuple definition `def`, if any.
@@ -699,22 +705,25 @@ module AssignableDefinitions {
   }
 
   /**
-   * A local variable definition in a pattern, for example `x is int i`.
+   * A local variable definition in a pattern, for example `int i` in `x is int i`.
    */
   class PatternDefinition extends AssignableDefinition, TPatternDefinition {
-    TopLevelPatternDecl tlpd;
+    LocalVariablePatternDecl lvpd;
 
-    PatternDefinition() { this = TPatternDefinition(tlpd) }
+    PatternDefinition() { this = TPatternDefinition(lvpd) }
 
     /** Gets the element matches against this pattern. */
-    PatternMatch getMatch() { result = tlpd.getMatch() }
+    PatternMatch getMatch() { result = lvpd.getMatch() }
 
     /** Gets the underlying local variable declaration. */
-    LocalVariableDeclExpr getDeclaration() { result = tlpd }
+    LocalVariableDeclExpr getDeclaration() { result = lvpd }
 
-    override Expr getSource() { result = this.getMatch().getExpr() }
+    override Expr getSource() { this.isTopLevel() and result = this.getMatch().getExpr() }
 
     override string toString() { result = this.getDeclaration().toString() }
+
+    /** Holds if the local variable definition is at the top level of the pattern. */
+    predicate isTopLevel() { lvpd.isTopLevel() }
   }
 
   /**

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

@@ -104,7 +104,7 @@ private predicate nonNullDef(Ssa::ExplicitDefinition def) {
   def.getADefinition().getSource() instanceof NonNullExpr
   or
   exists(AssignableDefinition ad | ad = def.getADefinition() |
-    ad instanceof AssignableDefinitions::PatternDefinition
+    ad.(AssignableDefinitions::PatternDefinition).isTopLevel()
     or
     ad =
       any(AssignableDefinitions::LocalVariableDefinition d |

csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll

@@ -605,7 +605,7 @@ module LocalFlow {
         isSuccessor = false
         or
         isSuccessor = true and
-        exists(ControlFlowElement cfe | cfe = e2.(TupleExpr).(PatternExpr).getPatternMatch() |
+        exists(ControlFlowElement cfe | cfe = e2.(TuplePatternExpr).getPatternMatch() |
           cfe.(IsExpr).getExpr() = e1 and scope = cfe
           or
           exists(Switch sw | sw.getACase() = cfe and sw.getExpr() = e1 and scope = sw)
@@ -624,25 +624,28 @@ module LocalFlow {
         scope = def.getExpr() and
         isSuccessor = true
         or
-        scope = def.(AssignableDefinitions::PatternDefinition).getMatch().(IsExpr) and
-        isSuccessor = false
-        or
-        exists(Switch s |
-          s.getACase() = def.(AssignableDefinitions::PatternDefinition).getMatch() and
-          isSuccessor = true
-        |
-          scope = s.getExpr()
+        exists(AssignableDefinitions::PatternDefinition def0 | def = def0 and def0.isTopLevel() |
+          scope = def0.getMatch().(IsExpr) and
+          isSuccessor = false
           or
-          scope = s.getACase()
+          exists(Switch s |
+            s.getACase() = def0.getMatch() and
+            isSuccessor = true
+          |
+            scope = s.getExpr()
+            or
+            scope = s.getACase()
+          )
         )
       )
       or
       // Needed for read steps for pattern matching involving properties.
       scope = def.getExpr() and
       e.(VariablePatternExpr).getVariableDeclExpr() =
-        def.(AssignableDefinitions::LocalVariableDefinition).getExpr() and
+        def.(AssignableDefinitions::PatternDefinition).getExpr() and
       exactScope = false and
-      isSuccessor = false
+      isSuccessor = false and
+      not def.(AssignableDefinitions::PatternDefinition).isTopLevel()
     }
   }
 
@@ -2955,8 +2958,10 @@ class CastNode extends Node {
   CastNode() {
     this.asExpr() instanceof Cast
     or
-    this.(AssignableDefinitionNode).getDefinition() instanceof
-      AssignableDefinitions::PatternDefinition
+    this.(AssignableDefinitionNode)
+        .getDefinition()
+        .(AssignableDefinitions::PatternDefinition)
+        .isTopLevel()
   }
 }
 

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

@@ -531,6 +531,13 @@ class LabeledPatternExpr extends PatternExpr {
   override string getAPrimaryQlClass() { result = "LabeledPatternExpr" }
 }
 
+/**
+ * A tuple pattern. For example, `var (x, y)`.
+ */
+class TuplePatternExpr extends TupleExpr, PatternExpr {
+  override string getAPrimaryQlClass() { result = "TuplePatternExpr" }
+}
+
 /** A positional pattern. For example, `(int x, int y)`. */
 class PositionalPatternExpr extends PatternExpr, @positional_pattern_expr {
   override string toString() { result = "( ... )" }