Skip to content

Commit f6cb090

Browse files
committed
ruby: more tweaking
I opened up `ActiveRecordModelFinderCall` to be public
1 parent a300eed commit f6cb090

File tree

3 files changed

+96
-51
lines changed

3 files changed

+96
-51
lines changed

ruby/ql/lib/codeql/ruby/frameworks/ActiveRecord.qll

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -352,8 +352,7 @@ private Expr getUltimateReceiver(MethodCall call) {
352352
}
353353

354354
// A call to `find`, `where`, etc. that may return active record model object(s)
355-
private class ActiveRecordModelFinderCall extends ActiveRecordModelInstantiation, DataFlow::CallNode
356-
{
355+
class ActiveRecordModelFinderCall extends ActiveRecordModelInstantiation, DataFlow::CallNode {
357356
private ActiveRecordModelClass cls;
358357

359358
ActiveRecordModelFinderCall() {
Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
/**
2+
* @id ruby/could-be-async
3+
* @kind problem
4+
* @problem.severity info
5+
* @problem.description Use `ActiveRecord::Relation#load_async` to load records asynchronously.
6+
*/
7+
8+
import ruby
9+
// private import codeql.ruby.CFG
10+
import codeql.ruby.Concepts
11+
import codeql.ruby.frameworks.ActiveRecord
12+
13+
// collection.each { |item| expensiveCall(item) }
14+
predicate expensiveCall(DataFlow::CallNode c) { c instanceof SqlExecution }
15+
16+
string loopMethodName() {
17+
// TODO: check these
18+
result in [
19+
"each", "reverse_each", "map", "map!", "foreach", "flat_map", "in_batches", "one?", "all?",
20+
"collect", "collect!", "select", "select!", "reject", "reject!"
21+
// , "collect", "select", "reject", "find_all", "find",
22+
// "detect", "any?", "all?", "one?", "none?", "one", "none", "min", "max", "minmax", "min_by",
23+
// "max_by", "minmax_by", "sort", "sort_by", "sort_by!", "sort"
24+
]
25+
}
26+
27+
class LoopingCall extends DataFlow::CallNode {
28+
DataFlow::CallableNode loopBlock;
29+
30+
LoopingCall() {
31+
this.getMethodName() = loopMethodName() and loopBlock = this.getBlock().asCallable()
32+
}
33+
34+
DataFlow::CallableNode getLoopBlock() { result = loopBlock }
35+
}
36+
37+
predicate happensInLoop(LoopingCall loop, DataFlow::CallNode e) {
38+
loop.getLoopBlock().asCallableAstNode() = e.asExpr().getScope()
39+
}
40+
41+
// predicate directLoop(@ruby_while)
42+
predicate happensInOuterLoop(LoopingCall outerLoop, DataFlow::CallNode e) {
43+
exists(LoopingCall innerLoop |
44+
happensInLoop(outerLoop, innerLoop) and
45+
happensInLoop(innerLoop, e)
46+
)
47+
}
48+
49+
predicate happensInInnermostLoop(LoopingCall loop, DataFlow::CallNode e) {
50+
happensInLoop(loop, e) and
51+
not happensInOuterLoop(loop, e)
52+
}
53+
54+
// Active Record
55+
private class LoadElementsCall extends ActiveRecordInstanceMethodCall {
56+
LoadElementsCall() {
57+
this.getMethodName() in ["latest", "find_by", "where", "count", "find"] and
58+
not any(PluckCall p).chaines() = this
59+
}
60+
}
61+
62+
private class PluckCall extends ActiveRecordInstanceMethodCall {
63+
PluckCall() { this.getMethodName() in ["pluck"] }
64+
65+
ActiveRecordInstanceMethodCall chaines() { result = getChain(this.getInstance()) }
66+
}
67+
68+
private ActiveRecordInstanceMethodCall getChain(ActiveRecordInstanceMethodCall c) {
69+
result = c
70+
or
71+
result = getChain(c.getInstance())
72+
}
73+
74+
ActiveRecordInstanceMethodCall longChain(ActiveRecordInstanceMethodCall c) {
75+
2 < strictcount(ActiveRecordInstanceMethodCall ch | ch = getChain(c)) and
76+
result = getChain(c) and
77+
result != c
78+
}
79+
80+
ActiveRecordInstanceMethodCall pluckChain(PluckCall c) { result = longChain(c) }
81+
82+
// from LoopingCall loopCall
83+
// select loopCall, loopCall.getLoopBlock()
84+
from LoopingCall loop, DataFlow::CallNode call, string message
85+
where
86+
happensInInnermostLoop(loop, call) and
87+
(
88+
call instanceof ActiveRecordModelFinderCall and
89+
not call.getMethodName() in ["new", "create"] and
90+
message = "could be chained with load_async"
91+
or
92+
call instanceof PluckCall and
93+
message = "could be async_pluck"
94+
)
95+
select call, "This call happens inside $@, and " + message, loop, "this loop"

ruby/ql/src/queries/performance/CouldBebatched.ql

Lines changed: 0 additions & 49 deletions
This file was deleted.

0 commit comments

Comments
 (0)