From 112cd9d29c8a8445720b9ec48736d8809192a49b Mon Sep 17 00:00:00 2001
From: Erik Krogh Kristensen <erik-krogh@github.com>
Date: Thu, 18 Nov 2021 12:16:38 +0100
Subject: [PATCH] move use-set-literal implementation to Query.qll file

---
 ql/src/codeql_ql/style/UseSetLiteralQuery.qll | 124 ++++++++++++++++++
 ql/src/queries/style/UseSetLiteral.ql         | 104 +--------------
 2 files changed, 126 insertions(+), 102 deletions(-)
 create mode 100644 ql/src/codeql_ql/style/UseSetLiteralQuery.qll

diff --git a/ql/src/codeql_ql/style/UseSetLiteralQuery.qll b/ql/src/codeql_ql/style/UseSetLiteralQuery.qll
new file mode 100644
index 00000000000..5d08e1834d1
--- /dev/null
+++ b/ql/src/codeql_ql/style/UseSetLiteralQuery.qll
@@ -0,0 +1,124 @@
+import ql
+
+/**
+ * A chain of disjunctions treated as one object. For example the following is
+ * a chain of disjunctions with three operands:
+ * ```
+ * a or b or c
+ * ```
+ */
+class DisjunctionChain extends Disjunction {
+  DisjunctionChain() { not exists(Disjunction parent | parent.getAnOperand() = this) }
+
+  /**
+   * Gets any operand of the chain.
+   */
+  Formula getOperand(int i) {
+    result =
+      rank[i + 1](Formula operand, Location l |
+        operand = getAnOperand*() and
+        not operand instanceof Disjunction and
+        l = operand.getLocation()
+      |
+        operand order by l.getStartLine(), l.getStartColumn()
+      )
+  }
+}
+
+/**
+ * An equality comparison with a `Literal`. For example:
+ * ```
+ * x = 4
+ * ```
+ */
+class EqualsLiteral extends ComparisonFormula {
+  EqualsLiteral() {
+    getOperator() = "=" and
+    getAnOperand() instanceof Literal
+  }
+
+  AstNode getOther() {
+    result = getAnOperand() and
+    not result instanceof Literal
+  }
+
+  Literal getLiteral() { result = getAnOperand() }
+}
+
+/**
+ * A chain of disjunctions where each operand is an equality comparison between
+ * the same thing and various `Literal`s. For example:
+ * ```
+ * x = 4 or
+ * x = 5 or
+ * x = 6
+ * ```
+ */
+class DisjunctionEqualsLiteral extends DisjunctionChain {
+  AstNode firstOperand;
+
+  DisjunctionEqualsLiteral() {
+    // VarAccess on the same variable
+    exists(VarDef v |
+      forex(Formula f | f = getOperand(_) |
+        f.(EqualsLiteral).getAnOperand().(VarAccess).getDeclaration() = v
+      ) and
+      firstOperand = getOperand(0).(EqualsLiteral).getAnOperand() and
+      firstOperand.(VarAccess).getDeclaration() = v
+    )
+    or
+    // FieldAccess on the same variable
+    exists(VarDecl v |
+      forex(Formula f | f = getOperand(_) |
+        f.(EqualsLiteral).getAnOperand().(FieldAccess).getDeclaration() = v
+      ) and
+      firstOperand = getOperand(0).(EqualsLiteral).getAnOperand() and
+      firstOperand.(FieldAccess).getDeclaration() = v
+    )
+    or
+    // ThisAccess
+    forex(Formula f | f = getOperand(_) | f.(EqualsLiteral).getAnOperand() instanceof ThisAccess) and
+    firstOperand = getOperand(0).(EqualsLiteral).getAnOperand().(ThisAccess)
+    or
+    // ResultAccess
+    forex(Formula f | f = getOperand(_) | f.(EqualsLiteral).getAnOperand() instanceof ResultAccess) and
+    firstOperand = getOperand(0).(EqualsLiteral).getAnOperand().(ResultAccess)
+    // (in principle something like GlobalValueNumbering could be used to generalize this)
+  }
+
+  /**
+   * Gets the first "thing" that is the same thing in this chain of equalities.
+   */
+  AstNode getFirstOperand() { result = firstOperand }
+}
+
+/**
+ * A call with a single `Literal` argument. For example:
+ * ```
+ * myPredicate(4)
+ * ```
+ */
+class CallLiteral extends Call {
+  CallLiteral() {
+    getNumberOfArguments() = 1 and
+    getArgument(0) instanceof Literal
+  }
+}
+
+/**
+ * A chain of disjunctions where each operand is a call to the same predicate
+ * using various `Literal`s. For example:
+ * ```
+ * myPredicate(4) or
+ * myPredicate(5) or
+ * myPredicate(6)
+ * ```
+ */
+class DisjunctionPredicateLiteral extends DisjunctionChain {
+  DisjunctionPredicateLiteral() {
+    // Call to the same target
+    exists(PredicateOrBuiltin target |
+      forex(Formula f | f = getOperand(_) | f.(CallLiteral).getTarget() = target)
+    )
+  }
+}
diff --git a/ql/src/queries/style/UseSetLiteral.ql b/ql/src/queries/style/UseSetLiteral.ql
index d0e983a1774..495d13822e3 100644
--- a/ql/src/queries/style/UseSetLiteral.ql
+++ b/ql/src/queries/style/UseSetLiteral.ql
@@ -9,107 +9,7 @@
  */
 
 import ql
-
-/**
- * A chain of disjunctions treated as one object. For example the following is
- * a chain of disjunctions with three operands:
- * ```
- * a or b or c
- * ```
- */
-class DisjunctionChain extends Disjunction {
-  DisjunctionChain() { not exists(Disjunction parent | parent.getAnOperand() = this) }
-
-  /**
-   * Gets any operand of the chain.
-   */
-  Formula getAnOperandRec() {
-    result = getAnOperand*() and
-    not result instanceof Disjunction
-  }
-}
-
-/**
- * An equality comparison with a `Literal`. For example:
- * ```
- * x = 4
- * ```
- */
-class EqualsLiteral extends ComparisonFormula {
-  EqualsLiteral() {
-    getSymbol() = "=" and
-    getAnOperand() instanceof Literal
-  }
-}
-
-/**
- * A chain of disjunctions where each operand is an equality comparison between
- * the same thing and various `Literal`s. For example:
- * ```
- * x = 4 or
- * x = 5 or
- * x = 6
- * ```
- */
-class DisjunctionEqualsLiteral extends DisjunctionChain {
-  DisjunctionEqualsLiteral() {
-    // VarAccess on the same variable
-    exists(VarDef v |
-      forex(Formula f | f = getAnOperandRec() |
-        f.(EqualsLiteral).getAnOperand().(VarAccess).getDeclaration() = v
-      )
-    )
-    or
-    // FieldAccess on the same variable
-    exists(VarDecl v |
-      forex(Formula f | f = getAnOperandRec() |
-        f.(EqualsLiteral).getAnOperand().(FieldAccess).getDeclaration() = v
-      )
-    )
-    or
-    // ThisAccess
-    forex(Formula f | f = getAnOperandRec() |
-      f.(EqualsLiteral).getAnOperand() instanceof ThisAccess
-    )
-    or
-    // ResultAccess
-    forex(Formula f | f = getAnOperandRec() |
-      f.(EqualsLiteral).getAnOperand() instanceof ResultAccess
-    )
-    // (in principle something like GlobalValueNumbering could be used to generalize this)
-  }
-}
-
-/**
- * A call with a single `Literal` argument. For example:
- * ```
- * myPredicate(4)
- * ```
- */
-class CallLiteral extends Call {
-  CallLiteral() {
-    getNumberOfArguments() = 1 and
-    getArgument(0) instanceof Literal
-  }
-}
-
-/**
- * A chain of disjunctions where each operand is a call to the same predicate
- * using various `Literal`s. For example:
- * ```
- * myPredicate(4) or
- * myPredicate(5) or
- * myPredicate(6)
- * ```
- */
-class DisjunctionPredicateLiteral extends DisjunctionChain {
-  DisjunctionPredicateLiteral() {
-    // Call to the same target
-    exists(PredicateOrBuiltin target |
-      forex(Formula f | f = getAnOperandRec() | f.(CallLiteral).getTarget() = target)
-    )
-  }
-}
+import codeql_ql.style.UseSetLiteralQuery
 
 from DisjunctionChain d, string msg, int c
 where
@@ -124,6 +24,6 @@ where
       "This formula of " + c.toString() +
         " predicate calls can be replaced with a single call on a set literal, improving readability."
   ) and
-  c = count(d.getAnOperandRec()) and
+  c = count(d.getOperand(_)) and
   c >= 4
 select d, msg