Skip to content

Commit ac0438f

Browse files
committed
Ruby: Address review comments.
1 parent 4641bdc commit ac0438f

2 files changed

Lines changed: 17 additions & 16 deletions

File tree

ruby/ql/lib/codeql/ruby/controlflow/ControlFlowGraph.qll

Lines changed: 9 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,7 @@ private module Ast implements AstSig<Location> {
150150
or
151151
findpattern.getElement(index) = result
152152
or
153-
index = 1 + max(int i | exists(findpattern.getElement(i))) and
153+
index = count(findpattern.getElement(_)) and
154154
findpattern.getSuffixVariableAccess() = result
155155
)
156156
or
@@ -198,9 +198,13 @@ private module Ast implements AstSig<Location> {
198198
Callable() { this instanceof R::Ast::Toplevel or this instanceof R::Ast::Callable }
199199
}
200200

201+
additional AstNode toplevelBody(R::Ast::Toplevel t, int index) {
202+
result = t.getBeginBlock(index) or
203+
result = desugar(t.getStmt(index - count(t.getABeginBlock())))
204+
}
205+
201206
AstNode callableGetBody(Callable c) {
202-
result = c.(R::Ast::Toplevel).getABeginBlock() or
203-
result = c.(R::Ast::Toplevel).getAStmt() or
207+
result = toplevelBody(c, _) or
204208
result = c.(R::Ast::Callable).getBody()
205209
}
206210

@@ -482,13 +486,8 @@ private module Input implements InputSig1, InputSig2 {
482486
class CallableContext = Unit;
483487

484488
Ast::AstNode callableGetBodyPart(Ast::Callable c, CallableContext ctx, int index) {
485-
exists(R::Ast::Toplevel t |
486-
c = t and
487-
exists(ctx)
488-
|
489-
result = t.getBeginBlock(index) or
490-
result = desugar(t.getStmt(index - count(t.getABeginBlock())))
491-
)
489+
result = Ast::toplevelBody(c, index) and
490+
exists(ctx)
492491
}
493492

494493
predicate catchAll(Ast::CatchClause catch) {
@@ -546,12 +545,6 @@ private module Input implements InputSig1, InputSig2 {
546545
}
547546

548547
predicate step(PreControlFlowNode n1, PreControlFlowNode n2) {
549-
exists(Ast::ConditionalExpr ce |
550-
n1.isAfterTrue(ce.getCondition()) and not exists(ce.getThen()) and n2.isAfter(ce)
551-
or
552-
n1.isAfterFalse(ce.getCondition()) and not exists(ce.getElse()) and n2.isAfter(ce)
553-
)
554-
or
555548
exists(R::Ast::RescueModifierExpr rescueModifier |
556549
n1.isBefore(rescueModifier) and
557550
n2.isBefore(desugar(rescueModifier.getBody()))

shared/controlflow/codeql/controlflow/ControlFlowGraph.qll

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1549,8 +1549,16 @@ module Make0<LocationSig Location, AstSig<Location> Ast> {
15491549
n1.isAfterTrue(condexpr.getCondition()) and
15501550
n2.isBefore(condexpr.getThen())
15511551
or
1552+
n1.isAfterTrue(condexpr.getCondition()) and
1553+
not exists(condexpr.getThen()) and
1554+
n2.isAfter(condexpr)
1555+
or
15521556
n1.isAfterFalse(condexpr.getCondition()) and
15531557
n2.isBefore(condexpr.getElse())
1558+
or
1559+
n1.isAfterFalse(condexpr.getCondition()) and
1560+
not exists(condexpr.getElse()) and
1561+
n2.isAfter(condexpr)
15541562
)
15551563
or
15561564
exists(PatternMatchExpr pme |

0 commit comments

Comments
 (0)