C#: Remove some unnecessary TCs

Author: hvitved

csharp/ql/lib/semmle/code/csharp/controlflow/Guards.qll

@@ -142,6 +142,7 @@ private module GuardsInput implements
     }
   }
 
+  pragma[nomagic]
   predicate equalityTest(Expr eqtest, Expr left, Expr right, boolean polarity) {
     exists(ComparisonTest ct |
       ct.getExpr() = eqtest and
@@ -410,6 +411,22 @@ private predicate typePattern(PatternMatch pm, TypePatternExpr tpe, Type t) {
   t = pm.getExpr().getType()
 }
 
+pragma[nomagic]
+private predicate dereferenceableExpr(Expr e, boolean isNullableType) {
+  exists(Type t | t = e.getType() |
+    t instanceof NullableType and
+    isNullableType = true
+    or
+    t instanceof RefType and
+    isNullableType = false
+  )
+  or
+  exists(Expr parent |
+    dereferenceableExpr(parent, isNullableType) and
+    e = getNullEquivParent(parent)
+  )
+}
+
 /**
  * An expression that evaluates to a value that can be dereferenced. That is,
  * an expression that may evaluate to `null`.
@@ -418,21 +435,12 @@ class DereferenceableExpr extends Expr {
   private boolean isNullableType;
 
   DereferenceableExpr() {
-    exists(Expr e, Type t |
-      // There is currently a bug in the extractor: the type of `x?.Length` is
-      // incorrectly `int`, while it should have been `int?`. We apply
-      // `getNullEquivParent()` as a workaround
-      this = getNullEquivParent*(e) and
-      t = e.getType() and
-      not this instanceof SwitchCaseExpr and
-      not this instanceof PatternExpr
-    |
-      t instanceof NullableType and
-      isNullableType = true
-      or
-      t instanceof RefType and
-      isNullableType = false
-    )
+    // There is currently a bug in the extractor: the type of `x?.Length` is
+    // incorrectly `int`, while it should have been `int?`. We apply
+    // `getNullEquivParent()` as a workaround
+    dereferenceableExpr(this, isNullableType) and
+    not this instanceof SwitchCaseExpr and
+    not this instanceof PatternExpr
   }
 
   /** Holds if this expression has a nullable type `T?`. */

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

@@ -94,9 +94,23 @@ private Element getAChild(Element p) {
   result = p.(AssignOperation).getExpandedAssignment()
 }
 
+pragma[nomagic]
+private predicate astNode(Element e) {
+  exists(@top_level_exprorstmt_parent p | not p instanceof Attribute |
+    e = p
+    or
+    e = getAChild(p)
+  )
+  or
+  exists(Element parent |
+    astNode(parent) and
+    e = getAChild(parent)
+  )
+}
+
 /** An AST node. */
 class AstNode extends Element, TAstNode {
-  AstNode() { this = getAChild*(any(@top_level_exprorstmt_parent p | not p instanceof Attribute)) }
+  AstNode() { astNode(this) }
 
   int getId() { idOf(this, result) }
 }

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

@@ -7,6 +7,7 @@ private import FlowSummaryImpl as FlowSummaryImpl
 private import semmle.code.csharp.dataflow.FlowSummary as FlowSummary
 private import semmle.code.csharp.dataflow.internal.ExternalFlow
 private import semmle.code.csharp.Conversion
+private import semmle.code.csharp.exprs.internal.Expr
 private import semmle.code.csharp.dataflow.internal.SsaImpl as SsaImpl
 private import semmle.code.csharp.ExprOrStmtParent
 private import semmle.code.csharp.Unification
@@ -2377,6 +2378,16 @@ predicate storeStep(Node node1, ContentSet c, Node node2) {
   storeStepDelegateCall(node1, c, node2)
 }
 
+pragma[nomagic]
+private predicate isAssignExprLValue(Expr e) {
+  e = any(AssignExpr ae).getLValue()
+  or
+  exists(Expr parent |
+    isAssignExprLValue(parent) and
+    e = parent.getAChildExpr()
+  )
+}
+
 private class ReadStepConfiguration extends ControlFlowReachabilityConfiguration {
   ReadStepConfiguration() { this = "ReadStepConfiguration" }
 
@@ -2432,7 +2443,7 @@ private class ReadStepConfiguration extends ControlFlowReachabilityConfiguration
     scope =
       any(AssignExpr ae |
         ae = defTo.(AssignableDefinitions::TupleAssignmentDefinition).getAssignment() and
-        e = ae.getLValue().getAChildExpr*().(TupleExpr) and
+        isAssignExprLValue(e.(TupleExpr)) and
         exactScope = false and
         isSuccessor = true
       )
@@ -2488,7 +2499,7 @@ private predicate readContentStep(Node node1, Content c, Node node2) {
       )
       or
       // item = variable in node1 = (..., variable, ...) in a case/is var (..., ...)
-      te = any(PatternExpr pe).getAChildExpr*() and
+      isPatternExprAncestor(te) and
       exists(AssignableDefinitions::LocalVariableDefinition lvd |
         node2.(AssignableDefinitionNode).getDefinition() = lvd and
         lvd.getDeclaration() = item and

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

@@ -21,6 +21,7 @@ import semmle.code.csharp.Type
 private import semmle.code.csharp.ExprOrStmtParent
 private import semmle.code.csharp.frameworks.System
 private import semmle.code.csharp.TypeRef
+private import internal.Expr
 
 /**
  * An expression. Either an access (`Access`), a call (`Call`), an object or
@@ -64,14 +65,24 @@ class Expr extends ControlFlowElement, @expr {
   /** Gets the enclosing callable of this expression, if any. */
   override Callable getEnclosingCallable() { enclosingCallable(this, result) }
 
+  pragma[nomagic]
+  private predicate isExpandedAssignmentRValue() {
+    this =
+      any(AssignOperation op).getExpandedAssignment().getRValue().getChildExpr(0).getAChildExpr()
+    or
+    exists(Expr parent |
+      parent.isExpandedAssignmentRValue() 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 =
-      any(AssignOperation op).getExpandedAssignment().getRValue().getChildExpr(0).getAChildExpr+()
+    this.isExpandedAssignmentRValue()
   }
 
   /**
@@ -1133,7 +1144,7 @@ class TupleExpr extends Expr, @tuple_expr {
   /** Holds if this expression is a tuple construction. */
   predicate isConstruction() {
     not this = getAnAssignOrForeachChild() and
-    not this = any(PatternExpr pe).getAChildExpr*()
+    not isPatternExprAncestor(this)
   }
 
   override string getAPrimaryQlClass() { result = "TupleExpr" }

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

@@ -0,0 +1,11 @@
+private import csharp
+
+pragma[nomagic]
+predicate isPatternExprAncestor(Expr e) {
+  e instanceof PatternExpr
+  or
+  exists(Expr parent |
+    isPatternExprAncestor(parent) and
+    e = parent.getAChildExpr()
+  )
+}