Merge pull request github#5018 from tamasvajk/feature/csharp9-binary-pattern-cfg

C#: Extract 'and' and 'or' patterns

Author: tamasvajk

csharp/change-notes/2021-01-27-Add-binary-pattern.md

@@ -0,0 +1,3 @@
+lgtm,codescanning
+* The `BinaryPatternExpr` class has been added to support C# 9 `and` and `or`
+patterns.

csharp/extractor/Semmle.Extraction.CSharp/Entities/Expressions/Patterns/BinaryPattern.cs

@@ -0,0 +1,28 @@
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.CSharp;
+using Microsoft.CodeAnalysis.CSharp.Syntax;
+using Semmle.Extraction.Entities;
+using Semmle.Extraction.Kinds;
+
+namespace Semmle.Extraction.CSharp.Entities.Expressions
+{
+    internal class BinaryPattern : Expression
+    {
+        public BinaryPattern(Context cx, BinaryPatternSyntax syntax, IExpressionParentEntity parent, int child) :
+            base(new ExpressionInfo(cx, null, cx.Create(syntax.GetLocation()), GetKind(syntax.OperatorToken, syntax), parent, child, false, null))
+        {
+            Pattern.Create(cx, syntax.Left, this, 0);
+            Pattern.Create(cx, syntax.Right, this, 1);
+        }
+
+        private static ExprKind GetKind(SyntaxToken operatorToken, BinaryPatternSyntax syntax)
+        {
+            return operatorToken.Kind() switch
+            {
+                SyntaxKind.AndKeyword => ExprKind.AND_PATTERN,
+                SyntaxKind.OrKeyword => ExprKind.OR_PATTERN,
+                _ => throw new InternalError(syntax, $"Operator '{operatorToken.Kind()}' is not supported in binary patterns.")
+            };
+        }
+    }
+}

csharp/extractor/Semmle.Extraction.CSharp/Entities/Expressions/Patterns/Pattern.cs

@@ -23,6 +23,9 @@ internal static Expression Create(Context cx, PatternSyntax syntax, IExpressionP
                 case UnaryPatternSyntax unaryPattern:
                     return new UnaryPattern(cx, unaryPattern, parent, child);
 
+                case BinaryPatternSyntax binaryPattern:
+                    return new BinaryPattern(cx, binaryPattern, parent, child);
+
                 case DeclarationPatternSyntax declPattern:
                     // Creates a single local variable declaration.
                     {

csharp/extractor/Semmle.Extraction.CSharp/Kinds/ExprKind.cs

@@ -121,6 +121,8 @@ public enum ExprKind
         LE_PATTERN = 124,
         GE_PATTERN = 125,
         NOT_PATTERN = 126,
+        AND_PATTERN = 127,
+        OR_PATTERN = 128,
         FUNCTION_POINTER_INVOCATION = 129,
     }
 }

csharp/ql/src/semmle/code/csharp/controlflow/internal/Completion.qll

@@ -551,7 +551,17 @@ private predicate mustHaveMatchingCompletion(Expr e, PatternExpr pe) {
   or
   pe = any(IsExpr ie | inBooleanContext(ie) and e = ie.getExpr()).getPattern()
   or
+  pe = any(RecursivePatternExpr rpe | mustHaveMatchingCompletion(e, rpe)).getPositionalPatterns()
+  or
+  pe = any(RecursivePatternExpr rpe | mustHaveMatchingCompletion(e, rpe)).getPropertyPatterns()
+  or
+  pe = any(PositionalPatternExpr ppe | mustHaveMatchingCompletion(e, ppe)).getPattern(_)
+  or
+  pe = any(PropertyPatternExpr ppe | mustHaveMatchingCompletion(e, ppe)).getPattern(_)
+  or
   pe = any(UnaryPatternExpr upe | mustHaveMatchingCompletion(e, upe)).getPattern()
+  or
+  pe = any(BinaryPatternExpr bpe | mustHaveMatchingCompletion(e, bpe)).getAnOperand()
 }
 
 /**

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

@@ -51,12 +51,21 @@ private import Splitting
 private import semmle.code.csharp.ExprOrStmtParent
 
 /** An element that defines a new CFG scope. */
-class CfgScope extends Element, @top_level_exprorstmt_parent { }
+class CfgScope extends Element, @top_level_exprorstmt_parent {
+  CfgScope() { not this instanceof Attribute }
+}
 
 module ControlFlowTree {
   private class Range_ = @callable or @control_flow_element;
 
-  class Range extends Element, Range_ { }
+  class Range extends Element, Range_ {
+    Range() { this = getAChild*(any(CfgScope scope)) }
+  }
+
+  Element getAChild(Element p) {
+    result = p.getAChild() or
+    result = p.(AssignOperation).getExpandedAssignment()
+  }
 
   private predicate id(Range x, Range y) { x = y }
 
@@ -360,7 +369,9 @@ module Expressions {
       not this instanceof SwitchExpr and
       not this instanceof SwitchCaseExpr and
       not this instanceof ConstructorInitializer and
-      not this instanceof NotPatternExpr
+      not this instanceof NotPatternExpr and
+      not this instanceof OrPatternExpr and
+      not this instanceof AndPatternExpr
     }
 
     final override ControlFlowElement getChildElement(int i) { result = getExprChild(this, i) }
@@ -902,6 +913,56 @@ module Expressions {
       c instanceof NormalCompletion
     }
   }
+
+  private class AndPatternExprTree extends PostOrderTree, AndPatternExpr {
+    final override predicate propagatesAbnormal(ControlFlowElement child) {
+      child = this.getAnOperand()
+    }
+
+    final override predicate first(ControlFlowElement first) { first(this.getLeftOperand(), first) }
+
+    final override predicate succ(ControlFlowElement pred, ControlFlowElement succ, Completion c) {
+      // Flow from last element of left operand to first element of right operand
+      last(this.getLeftOperand(), pred, c) and
+      c.(MatchingCompletion).getValue() = true and
+      first(this.getRightOperand(), succ)
+      or
+      // Post-order: flow from last element of left operand to element itself
+      last(this.getLeftOperand(), pred, c) and
+      c.(MatchingCompletion).getValue() = false and
+      succ = this
+      or
+      // Post-order: flow from last element of right operand to element itself
+      last(this.getRightOperand(), pred, c) and
+      c instanceof MatchingCompletion and
+      succ = this
+    }
+  }
+
+  private class OrPatternExprTree extends PostOrderTree, OrPatternExpr {
+    final override predicate propagatesAbnormal(ControlFlowElement child) {
+      child = this.getAnOperand()
+    }
+
+    final override predicate first(ControlFlowElement first) { first(this.getLeftOperand(), first) }
+
+    final override predicate succ(ControlFlowElement pred, ControlFlowElement succ, Completion c) {
+      // Flow from last element of left operand to first element of right operand
+      last(this.getLeftOperand(), pred, c) and
+      c.(MatchingCompletion).getValue() = false and
+      first(this.getRightOperand(), succ)
+      or
+      // Post-order: flow from last element of left operand to element itself
+      last(this.getLeftOperand(), pred, c) and
+      c.(MatchingCompletion).getValue() = true and
+      succ = this
+      or
+      // Post-order: flow from last element of right operand to element itself
+      last(this.getRightOperand(), pred, c) and
+      c instanceof MatchingCompletion and
+      succ = this
+    }
+  }
 }
 
 module Statements {
@@ -1273,7 +1334,7 @@ module Statements {
   /** Gets a child of `cfe` that is in CFG scope `scope`. */
   pragma[noinline]
   private ControlFlowElement getAChildInScope(ControlFlowElement cfe, Callable scope) {
-    result = [cfe.getAChild(), cfe.(AssignOperation).getExpandedAssignment()] and
+    result = ControlFlowTree::getAChild(cfe) and
     scope = result.getEnclosingCallable()
   }
 

csharp/ql/src/semmle/code/csharp/controlflow/internal/Splitting.qll

@@ -471,6 +471,12 @@ module ConditionalCompletionSplitting {
         or
         last(succ.(IsExpr).getPattern(), pred, c) and
         completion.(BooleanCompletion).getValue() = c.(MatchingCompletion).getValue()
+        or
+        last(succ.(AndPatternExpr).getAnOperand(), pred, c) and
+        completion = c
+        or
+        last(succ.(OrPatternExpr).getAnOperand(), pred, c) and
+        completion = c
       )
     }
 
@@ -1535,11 +1541,7 @@ predicate succEntrySplits(CfgScope pred, ControlFlowElement succ, Splits succSpl
   exists(int rnk |
     scopeFirst(pred, succ) and
     t instanceof NormalSuccessor and
-    succEntrySplitsFromRank(pred, succ, succSplits, rnk) and
-    // Attribute arguments in assemblies are represented as expressions, even though
-    // they are not from source. We are not interested in constructing a CFG for such
-    // expressions.
-    succ.fromSource()
+    succEntrySplitsFromRank(pred, succ, succSplits, rnk)
   |
     rnk = 0 and
     not any(SplitImpl split).hasEntryScope(pred, succ)

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

@@ -290,17 +290,32 @@ private predicate hasChildPattern(ControlFlowElement pm, Expr child) {
   or
   exists(Expr mid |
     hasChildPattern(pm, mid) and
-    mid instanceof @recursive_pattern_expr
-  |
-    child = mid.getChild(2).getAChildExpr() or
-    child = mid.getChild(3).getAChildExpr()
+    mid instanceof @property_pattern_expr and
+    child = mid.getAChildExpr()
+  )
+  or
+  exists(Expr mid |
+    hasChildPattern(pm, mid) and
+    mid instanceof @positional_pattern_expr and
+    child = mid.getAChildExpr()
+  )
+  or
+  exists(Expr mid |
+    hasChildPattern(pm, mid) and
+    mid instanceof @recursive_pattern_expr and
+    child = mid.getChild([2, 3])
   )
   or
   exists(Expr mid |
     hasChildPattern(pm, mid) and
     mid instanceof @unary_pattern_expr and
     child = mid.getChild(0)
   )
+  or
+  exists(Expr mid | hasChildPattern(pm, mid) and mid instanceof @binary_pattern_expr |
+    child = mid.getChild(0) or
+    child = mid.getChild(1)
+  )
 }
 
 /**
@@ -457,7 +472,7 @@ class RecursivePatternExpr extends BindingPatternExpr, @recursive_pattern_expr {
 }
 
 /** A property pattern. For example, `{ Length: 5 }`. */
-class PropertyPatternExpr extends Expr, @property_pattern_expr {
+class PropertyPatternExpr extends PatternExpr, @property_pattern_expr {
   override string toString() { result = "{ ... }" }
 
   /** Gets the `n`th pattern. */
@@ -480,7 +495,7 @@ class LabeledPatternExpr extends PatternExpr {
 }
 
 /** A positional pattern. For example, `(int x, int y)`. */
-class PositionalPatternExpr extends Expr, @positional_pattern_expr {
+class PositionalPatternExpr extends PatternExpr, @positional_pattern_expr {
   override string toString() { result = "( ... )" }
 
   /** Gets the `n`th pattern. */
@@ -502,6 +517,32 @@ class NotPatternExpr extends UnaryPatternExpr, @not_pattern_expr {
   override string getAPrimaryQlClass() { result = "NotPatternExpr" }
 }
 
+/** A binary pattern. For example, `1 or 2`. */
+class BinaryPatternExpr extends PatternExpr, @binary_pattern_expr {
+  /** Gets a pattern. */
+  PatternExpr getAnOperand() { result = getLeftOperand() or result = getRightOperand() }
+
+  /** Gets the left pattern. */
+  PatternExpr getLeftOperand() { result = this.getChild(0) }
+
+  /** Gets the right pattern. */
+  PatternExpr getRightOperand() { result = this.getChild(1) }
+}
+
+/** A binary `or` pattern. For example, `1 or 2`. */
+class OrPatternExpr extends BinaryPatternExpr, @or_pattern_expr {
+  override string toString() { result = "... or ..." }
+
+  override string getAPrimaryQlClass() { result = "OrPatternExpr" }
+}
+
+/** A binary `and` pattern. For example, `< 1 and > 2`. */
+class AndPatternExpr extends BinaryPatternExpr, @and_pattern_expr {
+  override string toString() { result = "... and ..." }
+
+  override string getAPrimaryQlClass() { result = "AndPatternExpr" }
+}
+
 /**
  * An expression or statement that matches the value of an expression against
  * a pattern. Either an `is` expression or a `case` expression/statement.

csharp/ql/src/semmlecode.csharp.dbscheme

@@ -1032,6 +1032,8 @@ case @expr.kind of
 | 124 = @le_pattern_expr
 | 125 = @ge_pattern_expr
 | 126 = @not_pattern_expr
+| 127 = @and_pattern_expr
+| 128 = @or_pattern_expr
 | 129 = @function_pointer_invocation_expr
 ;
 
@@ -1040,6 +1042,7 @@ case @expr.kind of
 @pattern_match = @case | @is_expr;
 @unary_pattern_expr = @not_pattern_expr;
 @relational_pattern_expr = @gt_pattern_expr | @lt_pattern_expr | @ge_pattern_expr | @le_pattern_expr;
+@binary_pattern_expr = @and_pattern_expr | @or_pattern_expr;
 
 @integer_literal_expr = @int_literal_expr | @long_literal_expr | @uint_literal_expr | @ulong_literal_expr;
 @real_literal_expr = @float_literal_expr | @double_literal_expr | @decimal_literal_expr;