Merge pull request #86 from github/use-set-literal

New query: Use set literal

ql/src/queries/style/UseSetLiteral.ql

@@ -0,0 +1,129 @@
+/**
+ * @name Use a set literal in place of `or`
+ * @description A chain of `or`s can be replaced with a set literal, improving readability.
+ * @kind problem
+ * @problem.severity recommendation
+ * @id ql/use-set-literal
+ * @tags maintainability
+ * @precision high
+ */
+
+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)
+    )
+  }
+}
+
+from DisjunctionChain d, string msg, int c
+where
+  (
+    d instanceof DisjunctionEqualsLiteral and
+    msg =
+      "This formula of " + c.toString() +
+        " comparisons can be replaced with a single equality on a set literal, improving readability."
+    or
+    d instanceof DisjunctionPredicateLiteral and
+    msg =
+      "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 >= 4
+select d, msg

ql/test/queries/style/UseSetLiteral/UseSetLiteral.expected

@@ -0,0 +1,8 @@
+| test.qll:4:3:7:7 | Disjunction | This formula of 4 comparisons can be replaced with a single equality on a set literal, improving readability. |
+| test.qll:30:3:33:10 | Disjunction | This formula of 4 predicate calls can be replaced with a single call on a set literal, improving readability. |
+| test.qll:44:3:47:12 | Disjunction | This formula of 4 comparisons can be replaced with a single equality on a set literal, improving readability. |
+| test.qll:62:7:65:14 | Disjunction | This formula of 4 comparisons can be replaced with a single equality on a set literal, improving readability. |
+| test.qll:68:7:71:13 | Disjunction | This formula of 4 comparisons can be replaced with a single equality on a set literal, improving readability. |
+| test.qll:74:7:77:13 | Disjunction | This formula of 4 comparisons can be replaced with a single equality on a set literal, improving readability. |
+| test.qll:87:3:90:9 | Disjunction | This formula of 4 predicate calls can be replaced with a single call on a set literal, improving readability. |
+| test.qll:128:3:134:3 | Disjunction | This formula of 4 comparisons can be replaced with a single equality on a set literal, improving readability. |

ql/test/queries/style/UseSetLiteral/UseSetLiteral.qlref

@@ -0,0 +1 @@
+queries/style/UseSetLiteral.ql

ql/test/queries/style/UseSetLiteral/test.qll

@@ -0,0 +1,135 @@
+import ql
+
+predicate test1(int a) {
+  a = 1 or // BAD
+  a = 2 or
+  a = 3 or
+  a = 4
+}
+
+predicate test2(int a) {
+  a = [1, 2, 3, 4] // GOOD
+}
+
+predicate test3(int a) {
+  a = 1 and // GOOD (for the purposes of this query)
+  a = 2 and
+  a = 3 and
+  a = 4
+}
+
+bindingset[a]
+predicate test4(int a) {
+  a < 1 or // GOOD (for the purposes of this query)
+  a = 2 or
+  a >= 3 or
+  a > 4
+}
+
+predicate test5() {
+  test1(1) or // BAD
+  test1(2) or
+  test1(3) or
+  test1(4)
+}
+
+predicate test6() {
+  test1(1) or // GOOD
+  test2(2) or
+  test3(3) or
+  test4(4)
+}
+
+int test7() {
+  1 = result or // BAD
+  2 = result or
+  3 = result or
+  4 = result
+}
+
+predicate test8() {
+  test7() = 1 or // BAD [NOT DETECTED]
+  test7() = 2 or
+  test7() = 3 or
+  test7() = 4
+}
+
+class MyTest8Class extends int {
+  string s;
+
+  MyTest8Class() {
+    (
+      this = 1 or // BAD
+      this = 2 or
+      this = 3 or
+      this = 4
+    ) and
+    (
+      s = "1" or // BAD
+      s = "2" or
+      s = "3" or
+      s = "4"
+    ) and
+    exists(float f |
+      f = 1.0 or // BAD
+      f = 1.5 or
+      f = 2.0 or
+      f = 2.5
+    )
+  }
+
+  predicate is(int x) { x = this }
+
+  int get() { result = this }
+}
+
+predicate test9(MyTest8Class c) {
+  c.is(1) or // BAD
+  c.is(2) or
+  c.is(3) or
+  c.is(4)
+}
+
+predicate test10(MyTest8Class c) {
+  c.get() = 1 or // BAD [NOT DETECTED]
+  c.get() = 2 or
+  c.get() = 3 or
+  c.get() = 4
+}
+
+bindingset[a, b, c, d]
+predicate test11(int a, int b, int c, int d) {
+  a = 1 or // GOOD
+  b = 2 or
+  c = 3 or
+  d = 4
+}
+
+bindingset[a, b]
+predicate test12(int a, int b) {
+  a = 1 or // BAD [NOT DETECTED]
+  a = 2 or
+  a = 3 or
+  a = 4 or
+  b = 0
+}
+
+predicate test13(int a, int b) {
+  a = 1 and b = 1 // GOOD
+  or
+  a = 2 and b = 4
+  or
+  a = 3 and b = 9
+  or
+  a = 4 and b = 16
+}
+
+predicate test14(int a) {
+  a = 1 // BAD
+  or
+  (
+    (a = 2 or a = 3)
+    or
+    a = 4
+  )
+}