Skip to content

Commit 4133759

Browse files
committed
Ruby: Flatten nested statements inside desugared for loops
1 parent 9125b85 commit 4133759

File tree

9 files changed

+171
-161
lines changed

9 files changed

+171
-161
lines changed

ruby/ql/lib/codeql/ruby/ast/Control.qll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -583,7 +583,7 @@ class ForExpr extends Loop, TForExpr {
583583
final override string getAPrimaryQlClass() { result = "ForExpr" }
584584

585585
/** Gets the body of this `for` loop. */
586-
final override Stmt getBody() { toGenerated(result) = g.getBody() }
586+
final override StmtSequence getBody() { toGenerated(result) = g.getBody() }
587587

588588
/** Gets the pattern representing the iteration argument. */
589589
final Pattern getPattern() { toGenerated(result) = g.getPattern() }

ruby/ql/lib/codeql/ruby/ast/Expr.qll

Lines changed: 18 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
private import codeql.ruby.AST
22
private import codeql.ruby.CFG
33
private import internal.AST
4+
private import internal.Expr
45
private import internal.TreeSitter
56

67
/**
@@ -91,90 +92,19 @@ class StmtSequence extends Expr, TStmtSequence {
9192
}
9293
}
9394

94-
private class StmtSequenceSynth extends StmtSequence, TStmtSequenceSynth {
95-
final override Stmt getStmt(int n) { synthChild(this, n, result) }
96-
97-
final override string toString() { result = "..." }
98-
}
99-
100-
private class Then extends StmtSequence, TThen {
101-
private Ruby::Then g;
102-
103-
Then() { this = TThen(g) }
104-
105-
override Stmt getStmt(int n) { toGenerated(result) = g.getChild(n) }
106-
107-
final override string toString() { result = "then ..." }
108-
}
109-
110-
private class Else extends StmtSequence, TElse {
111-
private Ruby::Else g;
112-
113-
Else() { this = TElse(g) }
114-
115-
override Stmt getStmt(int n) { toGenerated(result) = g.getChild(n) }
116-
117-
final override string toString() { result = "else ..." }
118-
}
119-
120-
private class Do extends StmtSequence, TDo {
121-
private Ruby::Do g;
122-
123-
Do() { this = TDo(g) }
124-
125-
override Stmt getStmt(int n) { toGenerated(result) = g.getChild(n) }
126-
127-
final override string toString() { result = "do ..." }
128-
}
129-
130-
private class Ensure extends StmtSequence, TEnsure {
131-
private Ruby::Ensure g;
132-
133-
Ensure() { this = TEnsure(g) }
134-
135-
override Stmt getStmt(int n) { toGenerated(result) = g.getChild(n) }
136-
137-
final override string toString() { result = "ensure ..." }
138-
}
139-
14095
/**
14196
* A sequence of statements representing the body of a method, class, module,
14297
* or do-block. That is, any body that may also include rescue/ensure/else
14398
* statements.
14499
*/
145100
class BodyStmt extends StmtSequence, TBodyStmt {
146-
// Not defined by dispatch, as it should not be exposed
147-
private Ruby::AstNode getChild(int i) {
148-
result = any(Ruby::Method g | this = TMethod(g)).getChild(i)
149-
or
150-
result = any(Ruby::SingletonMethod g | this = TSingletonMethod(g)).getChild(i)
151-
or
152-
exists(Ruby::Lambda g | this = TLambda(g) |
153-
result = g.getBody().(Ruby::DoBlock).getChild(i) or
154-
result = g.getBody().(Ruby::Block).getChild(i)
155-
)
156-
or
157-
result = any(Ruby::DoBlock g | this = TDoBlock(g)).getChild(i)
158-
or
159-
result = any(Ruby::Program g | this = TToplevel(g)).getChild(i) and
160-
not result instanceof Ruby::BeginBlock
161-
or
162-
result = any(Ruby::Class g | this = TClassDeclaration(g)).getChild(i)
163-
or
164-
result = any(Ruby::SingletonClass g | this = TSingletonClass(g)).getChild(i)
165-
or
166-
result = any(Ruby::Module g | this = TModuleDeclaration(g)).getChild(i)
167-
or
168-
result = any(Ruby::Begin g | this = TBeginExpr(g)).getChild(i)
169-
}
170-
171101
final override Stmt getStmt(int n) {
172-
result =
173-
rank[n + 1](AstNode node, int i |
174-
toGenerated(node) = this.getChild(i) and
175-
not node instanceof Else and
176-
not node instanceof RescueClause and
177-
not node instanceof Ensure
102+
toGenerated(result) =
103+
rank[n + 1](Ruby::AstNode node, int i |
104+
node = getBodyStmtChild(this, i) and
105+
not node instanceof Ruby::Else and
106+
not node instanceof Ruby::Rescue and
107+
not node instanceof Ruby::Ensure
178108
|
179109
node order by i
180110
)
@@ -183,17 +113,25 @@ class BodyStmt extends StmtSequence, TBodyStmt {
183113
/** Gets the `n`th rescue clause in this block. */
184114
final RescueClause getRescue(int n) {
185115
result =
186-
rank[n + 1](RescueClause node, int i | toGenerated(node) = this.getChild(i) | node order by i)
116+
rank[n + 1](RescueClause node, int i |
117+
toGenerated(node) = getBodyStmtChild(this, i)
118+
|
119+
node order by i
120+
)
187121
}
188122

189123
/** Gets a rescue clause in this block. */
190124
final RescueClause getARescue() { result = this.getRescue(_) }
191125

192126
/** Gets the `else` clause in this block, if any. */
193-
final StmtSequence getElse() { result = unique(Else s | toGenerated(s) = getChild(_)) }
127+
final StmtSequence getElse() {
128+
result = unique(Else s | toGenerated(s) = getBodyStmtChild(this, _))
129+
}
194130

195131
/** Gets the `ensure` clause in this block, if any. */
196-
final StmtSequence getEnsure() { result = unique(Ensure s | toGenerated(s) = getChild(_)) }
132+
final StmtSequence getEnsure() {
133+
result = unique(Ensure s | toGenerated(s) = getBodyStmtChild(this, _))
134+
}
197135

198136
final predicate hasEnsure() { exists(this.getEnsure()) }
199137

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
private import codeql.ruby.AST
2+
private import codeql.ruby.CFG
3+
private import AST
4+
private import TreeSitter
5+
6+
class StmtSequenceSynth extends StmtSequence, TStmtSequenceSynth {
7+
final override Stmt getStmt(int n) { synthChild(this, n, result) }
8+
9+
final override string toString() { result = "..." }
10+
}
11+
12+
class Then extends StmtSequence, TThen {
13+
private Ruby::Then g;
14+
15+
Then() { this = TThen(g) }
16+
17+
override Stmt getStmt(int n) { toGenerated(result) = g.getChild(n) }
18+
19+
final override string toString() { result = "then ..." }
20+
}
21+
22+
class Else extends StmtSequence, TElse {
23+
private Ruby::Else g;
24+
25+
Else() { this = TElse(g) }
26+
27+
override Stmt getStmt(int n) { toGenerated(result) = g.getChild(n) }
28+
29+
final override string toString() { result = "else ..." }
30+
}
31+
32+
class Do extends StmtSequence, TDo {
33+
private Ruby::Do g;
34+
35+
Do() { this = TDo(g) }
36+
37+
override Stmt getStmt(int n) { toGenerated(result) = g.getChild(n) }
38+
39+
final override string toString() { result = "do ..." }
40+
}
41+
42+
class Ensure extends StmtSequence, TEnsure {
43+
private Ruby::Ensure g;
44+
45+
Ensure() { this = TEnsure(g) }
46+
47+
override Stmt getStmt(int n) { toGenerated(result) = g.getChild(n) }
48+
49+
final override string toString() { result = "ensure ..." }
50+
}
51+
52+
// Not defined by dispatch, as it should not be exposed
53+
Ruby::AstNode getBodyStmtChild(TBodyStmt b, int i) {
54+
result = any(Ruby::Method g | b = TMethod(g)).getChild(i)
55+
or
56+
result = any(Ruby::SingletonMethod g | b = TSingletonMethod(g)).getChild(i)
57+
or
58+
exists(Ruby::Lambda g | b = TLambda(g) |
59+
result = g.getBody().(Ruby::DoBlock).getChild(i) or
60+
result = g.getBody().(Ruby::Block).getChild(i)
61+
)
62+
or
63+
result = any(Ruby::DoBlock g | b = TDoBlock(g)).getChild(i)
64+
or
65+
result = any(Ruby::Program g | b = TToplevel(g)).getChild(i) and
66+
not result instanceof Ruby::BeginBlock
67+
or
68+
result = any(Ruby::Class g | b = TClassDeclaration(g)).getChild(i)
69+
or
70+
result = any(Ruby::SingletonClass g | b = TSingletonClass(g)).getChild(i)
71+
or
72+
result = any(Ruby::Module g | b = TModuleDeclaration(g)).getChild(i)
73+
or
74+
result = any(Ruby::Begin g | b = TBeginExpr(g)).getChild(i)
75+
}

ruby/ql/lib/codeql/ruby/ast/internal/Synthesis.qll

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
private import AST
44
private import TreeSitter
55
private import codeql.ruby.ast.internal.Call
6+
private import codeql.ruby.ast.internal.Expr
67
private import codeql.ruby.ast.internal.Variable
78
private import codeql.ruby.ast.internal.Pattern
89
private import codeql.ruby.ast.internal.Scope
@@ -880,8 +881,7 @@ private module ForLoopDesugar {
880881
or
881882
// rest of block body
882883
parent = block and
883-
i = 2 and
884-
child = RealChild(for.getBody())
884+
child = RealChild(for.getBody().(Do).getStmt(i - 2))
885885
)
886886
)
887887
)
@@ -902,5 +902,9 @@ private module ForLoopDesugar {
902902
n instanceof TSimpleParameterSynth and
903903
i = 0
904904
}
905+
906+
final override predicate excludeFromControlFlowTree(AstNode n) {
907+
n = any(ForExpr for).getBody()
908+
}
905909
}
906910
}

ruby/ql/lib/codeql/ruby/controlflow/internal/ControlFlowGraphImpl.qll

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,9 @@ module CfgScope {
6161

6262
final override predicate exit(AstNode last, Completion c) {
6363
last(this.(Trees::EndBlockTree).getLastBodyChild(), last, c)
64+
or
65+
last(this.(Trees::EndBlockTree).getBodyChild(_, _), last, c) and
66+
not c instanceof NormalCompletion
6467
}
6568
}
6669

@@ -79,6 +82,9 @@ module CfgScope {
7982

8083
final override predicate exit(AstNode last, Completion c) {
8184
last(this.(Trees::BraceBlockTree).getLastBodyChild(), last, c)
85+
or
86+
last(this.(Trees::BraceBlockTree).getBodyChild(_, _), last, c) and
87+
not c instanceof NormalCompletion
8288
}
8389
}
8490
}

0 commit comments

Comments
 (0)