ruby: simplify and document

Author: yoff

ruby/ql/src/queries/performance/DatabaseQueryInLoop.ql

@@ -23,28 +23,26 @@ string getALoopMethodName() {
     ]
 }
 
-/** A call to a loop operation. */
+/** A loop, represented by a call to a loop operation. */
 class LoopingCall extends DataFlow::CallNode {
-  DataFlow::CallableNode loopBlock;
+  Callable loopScope;
 
   LoopingCall() {
-    this.getMethodName() = getALoopMethodName() and loopBlock = this.getBlock().asCallable()
+    this.getMethodName() = getALoopMethodName() and
+    loopScope = this.getBlock().asCallable().asCallableAstNode()
   }
 
-  DataFlow::CallableNode getLoopBlock() { result = loopBlock }
+  /** Holds if `c` is executed as part of the body of this loop. */
+  predicate executesCall(DataFlow::CallNode c) { c.asExpr().getScope() = loopScope }
 }
 
-predicate happensInLoop(LoopingCall loop, DataFlow::CallNode e) {
-  loop.getLoopBlock().asCallableAstNode() = e.asExpr().getScope()
-}
-
-// The ActiveRecord instance is used to potentially control the loop
+/** Holds if `ar` influences `guard`, which may control the execution of a loop. */
 predicate usedInLoopControlGuard(ActiveRecordInstance ar, DataFlow::Node guard) {
   TaintTracking::localTaint(ar, guard) and
   guard = guardForLoopControl(_, _)
 }
 
-// A guard for controlling the loop
+/** Gets a dataflow node that is used to decide whether to break a loop. */
 DataFlow::Node guardForLoopControl(ConditionalExpr cond, Stmt control) {
   result.asExpr().getAstNode() = cond.getCondition().getAChild*() and
   (
@@ -55,15 +53,14 @@ DataFlow::Node guardForLoopControl(ConditionalExpr cond, Stmt control) {
   control = cond.getBranch(_).getAChild()
 }
 
-from LoopingCall loop, DataFlow::CallNode call
+from LoopingCall loop, ActiveRecordModelFinderCall call
 where
+  loop.executesCall(call) and
   // Disregard loops over constants
   not isArrayConstant(loop.getReceiver().asExpr(), _) and
   // Disregard cases where the looping is influenced by the query result
   not usedInLoopControlGuard(call, _) and
-  happensInLoop(loop, call) and
   // Only report calls that are likely to be expensive
-  call instanceof ActiveRecordModelFinderCall and
   not call.getMethodName() in ["new", "create"]
 select call,
   "This call to a database query operation happens inside $@, and could be hoisted to a single call outside the loop.",