C#: Avoid computing full TC in DangerousNonShortCircuitLogic.ql

Author: hvitved

csharp/ql/src/Likely Bugs/DangerousNonShortCircuitLogic.ql

@@ -15,23 +15,6 @@
 
 import csharp
 
-/** An expression containing a qualified member access, a method call, or an array access. */
-class DangerousExpression extends Expr {
-  DangerousExpression() {
-    exists(Expr e | this = e.getParent*() |
-      exists(Expr q | q = e.(MemberAccess).getQualifier() |
-        not q instanceof ThisAccess and
-        not q instanceof BaseAccess
-      )
-      or
-      e instanceof MethodCall
-      or
-      e instanceof ArrayAccess
-    ) and
-    not exists(Expr e | this = e.getParent*() | e.(Call).getTarget().getAParameter().isOutOrRef())
-  }
-}
-
 /** A use of `&` or `|` on operands of type boolean. */
 class NonShortCircuit extends BinaryBitwiseOperation {
   NonShortCircuit() {
@@ -42,10 +25,40 @@ class NonShortCircuit extends BinaryBitwiseOperation {
     ) and
     not exists(AssignBitwiseOperation abo | abo.getExpandedAssignment().getRValue() = this) and
     this.getLeftOperand().getType() instanceof BoolType and
-    this.getRightOperand().getType() instanceof BoolType and
-    this.getRightOperand() instanceof DangerousExpression
+    this.getRightOperand().getType() instanceof BoolType
+  }
+
+  pragma[nomagic]
+  private predicate hasRightOperandDescendant(Expr e) {
+    e = this.getRightOperand()
+    or
+    exists(Expr parent |
+      this.hasRightOperandDescendant(parent) and
+      e.getParent() = parent
+    )
+  }
+
+  /**
+   * Holds if this non-short-circuit expression contains a qualified member access,
+   * a method call, or an array access inside the right operand.
+   */
+  predicate isDangerous() {
+    exists(Expr e | this.hasRightOperandDescendant(e) |
+      exists(Expr q | q = e.(MemberAccess).getQualifier() |
+        not q instanceof ThisAccess and
+        not q instanceof BaseAccess
+      )
+      or
+      e instanceof MethodCall
+      or
+      e instanceof ArrayAccess
+    ) and
+    not exists(Expr e | this.hasRightOperandDescendant(e) |
+      e.(Call).getTarget().getAParameter().isOutOrRef()
+    )
   }
 }
 
 from NonShortCircuit e
+where e.isDangerous()
 select e, "Potentially dangerous use of non-short circuit logic."