Merge pull request #147 from github/calumgrant/use-detect

Ruby: New query UseDetect

ql/src/queries/performance/UseDetect.ql

@@ -0,0 +1,65 @@
+/**
+ * @name Use detect
+ * @description Use 'detect' instead of 'select' followed by 'first' or 'last'.
+ * @kind problem
+ * @problem.severity warning
+ * @id rb/use-detect
+ * @tags performance rubocop
+ * @precision high
+ *
+ * This is an implementation of Rubocop rule
+ * https://github.com/rubocop/rubocop-performance/blob/master/lib/rubocop/cop/performance/detect.rb
+ */
+
+import ruby
+import codeql_ruby.dataflow.SSA
+
+/** A call that extracts the first or last element of a list. */
+class EndCall extends MethodCall {
+  string detect;
+
+  EndCall() {
+    detect = "detect" and
+    (
+      this.getMethodName() = "first" and
+      this.getNumberOfArguments() = 0
+      or
+      this.getNumberOfArguments() = 1 and
+      this.getArgument(0).(IntegerLiteral).getValueText() = "0"
+    )
+    or
+    detect = "reverse_detect" and
+    (
+      this.getMethodName() = "last" and
+      this.getNumberOfArguments() = 0
+      or
+      this.getNumberOfArguments() = 1 and
+      this.getArgument(0).(UnaryMinusExpr).getOperand().(IntegerLiteral).getValueText() = "1"
+    )
+  }
+
+  string detectCall() { result = detect }
+}
+
+Expr getUniqueRead(Expr e) {
+  exists(AssignExpr ae |
+    e = ae.getRightOperand() and
+    forex(Ssa::WriteDefinition def | def.getWriteAccess() = ae.getLeftOperand() |
+      strictcount(def.getARead()) = 1 and
+      not def = any(Ssa::PhiNode phi).getAnInput() and
+      def.getARead() = result.getAControlFlowNode()
+    )
+  )
+}
+
+class SelectBlock extends MethodCall {
+  SelectBlock() {
+    this.getMethodName() in ["select", "filter", "find_all"] and
+    exists(this.getBlock())
+  }
+}
+
+from EndCall call, SelectBlock selectBlock
+where getUniqueRead*(selectBlock) = call.getReceiver()
+select call, "Replace this call and $@ with '" + call.detectCall() + "'.", selectBlock,
+  "'select' call"

ql/test/query-tests/performance/UseDetect/UseDetect.expected

@@ -0,0 +1,7 @@
+| UseDetect.rb:5:9:5:36 | call to first | Replace this call and $@ with 'detect'. | UseDetect.rb:5:9:5:30 | call to select | 'select' call |
+| UseDetect.rb:6:9:6:35 | call to last | Replace this call and $@ with 'reverse_detect'. | UseDetect.rb:6:9:6:30 | call to select | 'select' call |
+| UseDetect.rb:7:9:7:33 | ...[...] | Replace this call and $@ with 'detect'. | UseDetect.rb:7:9:7:30 | call to select | 'select' call |
+| UseDetect.rb:8:9:8:34 | ...[...] | Replace this call and $@ with 'reverse_detect'. | UseDetect.rb:8:9:8:30 | call to select | 'select' call |
+| UseDetect.rb:9:9:9:36 | call to first | Replace this call and $@ with 'detect'. | UseDetect.rb:9:9:9:30 | call to filter | 'select' call |
+| UseDetect.rb:10:9:10:37 | call to last | Replace this call and $@ with 'reverse_detect'. | UseDetect.rb:10:9:10:32 | call to find_all | 'select' call |
+| UseDetect.rb:12:9:12:24 | call to first | Replace this call and $@ with 'detect'. | UseDetect.rb:11:22:11:43 | call to select | 'select' call |

ql/test/query-tests/performance/UseDetect/UseDetect.qlref

@@ -0,0 +1 @@
+queries/performance/UseDetect.ql

ql/test/query-tests/performance/UseDetect/UseDetect.rb

@@ -0,0 +1,21 @@
+
+class DetectTest
+    def test
+        # These are bad
+        [].select { |i| true }.first
+        [].select { |i| true }.last
+        [].select { |i| true }[0]
+        [].select { |i| true }[-1]
+        [].filter { |i| true }.first
+        [].find_all { |i| true }.last
+        selection1 = [].select { |i| true }
+        selection1.first
+
+        # These are good
+        [].select("").first # Selecting a string
+        [].select { |i| true }[1] # Selecting the second element
+        selection2 = [].select { |i| true }
+        selection2.first # Selection used elsewhere
+        selection3 = selection2
+    end
+end