Skip to content

Commit a7aff11

Browse files
aibaarsaibaars
authored
Merge pull request #7394 from aibaars/ruby-cfg-expr-post
Ruby: CFG: make all expressions "post-order" nodes
2 parents c138a27 + a86ba3b commit a7aff11

File tree

15 files changed

+325
-260
lines changed

15 files changed

+325
-260
lines changed

‎ruby/ql/consistency-queries/CfgConsistency.ql

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1 +1,20 @@
11
import codeql.ruby.controlflow.internal.ControlFlowGraphImplShared::Consistency
2+
import codeql.ruby.AST
3+
import codeql.ruby.controlflow.internal.Completion
4+
import codeql.ruby.controlflow.internal.ControlFlowGraphImpl
5+
6+
/**
7+
* All `Expr` nodes are `PostOrderTree`s
8+
*/
9+
query predicate nonPostOrderExpr(Expr e, string cls) {
10+
cls = e.getPrimaryQlClasses() and
11+
not exists(e.getDesugared()) and
12+
not e instanceof BeginExpr and
13+
not e instanceof Namespace and
14+
not e instanceof Toplevel and
15+
exists(AstNode last, Completion c |
16+
last(e, last, c) and
17+
last != e and
18+
c instanceof NormalCompletion
19+
)
20+
}

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

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -361,22 +361,22 @@ class CaseExpr extends ControlExpr instanceof CaseExprImpl {
361361
final Expr getValue() { result = super.getValue() }
362362

363363
/**
364-
* Gets the `n`th branch of this case expression, either a `WhenExpr`, an
364+
* Gets the `n`th branch of this case expression, either a `WhenClause`, an
365365
* `InClause`, or a `StmtSequence`.
366366
*/
367-
final Expr getBranch(int n) { result = super.getBranch(n) }
367+
final AstNode getBranch(int n) { result = super.getBranch(n) }
368368

369369
/**
370-
* Gets a branch of this case expression, either a `WhenExpr`, an
370+
* Gets a branch of this case expression, either a `WhenClause`, an
371371
* `InClause`, or a `StmtSequence`.
372372
*/
373-
final Expr getABranch() { result = this.getBranch(_) }
373+
final AstNode getABranch() { result = this.getBranch(_) }
374374

375375
/** Gets the `n`th `when` branch of this case expression. */
376-
deprecated final WhenExpr getWhenBranch(int n) { result = this.getBranch(n) }
376+
deprecated final WhenClause getWhenBranch(int n) { result = this.getBranch(n) }
377377

378378
/** Gets a `when` branch of this case expression. */
379-
deprecated final WhenExpr getAWhenBranch() { result = this.getABranch() }
379+
deprecated final WhenClause getAWhenBranch() { result = this.getABranch() }
380380

381381
/** Gets the `else` branch of this case expression, if any. */
382382
final StmtSequence getElseBranch() { result = this.getABranch() }
@@ -401,6 +401,11 @@ class CaseExpr extends ControlExpr instanceof CaseExprImpl {
401401
}
402402
}
403403

404+
/**
405+
* DEPRECATED: Use `WhenClause` instead.
406+
*/
407+
deprecated class WhenExpr = WhenClause;
408+
404409
/**
405410
* A `when` branch of a `case` expression.
406411
* ```rb
@@ -409,12 +414,12 @@ class CaseExpr extends ControlExpr instanceof CaseExprImpl {
409414
* end
410415
* ```
411416
*/
412-
class WhenExpr extends Expr, TWhenExpr {
417+
class WhenClause extends AstNode, TWhenClause {
413418
private Ruby::When g;
414419

415-
WhenExpr() { this = TWhenExpr(g) }
420+
WhenClause() { this = TWhenClause(g) }
416421

417-
final override string getAPrimaryQlClass() { result = "WhenExpr" }
422+
final override string getAPrimaryQlClass() { result = "WhenClause" }
418423

419424
/** Gets the body of this case-when expression. */
420425
final Stmt getBody() { toGenerated(result) = g.getBody() }
@@ -461,7 +466,7 @@ class WhenExpr extends Expr, TWhenExpr {
461466
* end
462467
* ```
463468
*/
464-
class InClause extends Expr, TInClause {
469+
class InClause extends AstNode, TInClause {
465470
private Ruby::InClause g;
466471

467472
InClause() { this = TInClause(g) }

‎ruby/ql/lib/codeql/ruby/ast/internal/AST.qll

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -305,7 +305,7 @@ private module Cached {
305305
TUntilExpr(Ruby::Until g) or
306306
TUntilModifierExpr(Ruby::UntilModifier g) or
307307
TVariableReferencePattern(Ruby::VariableReferencePattern g) or
308-
TWhenExpr(Ruby::When g) or
308+
TWhenClause(Ruby::When g) or
309309
TWhileExpr(Ruby::While g) or
310310
TWhileModifierExpr(Ruby::WhileModifier g) or
311311
TYieldCall(Ruby::Yield g)
@@ -344,7 +344,7 @@ private module Cached {
344344
TTernaryIfExpr or TThen or TTokenConstantAccess or TTokenMethodName or TTokenSuperCall or
345345
TToplevel or TTrueLiteral or TUnaryMinusExpr or TUnaryPlusExpr or TUndefStmt or
346346
TUnlessExpr or TUnlessModifierExpr or TUntilExpr or TUntilModifierExpr or
347-
TVariableReferencePattern or TWhenExpr or TWhileExpr or TWhileModifierExpr or TYieldCall;
347+
TVariableReferencePattern or TWhenClause or TWhileExpr or TWhileModifierExpr or TYieldCall;
348348

349349
class TAstNodeSynth =
350350
TAddExprSynth or TAssignExprSynth or TBitwiseAndExprSynth or TBitwiseOrExprSynth or
@@ -511,7 +511,7 @@ private module Cached {
511511
n = TUntilExpr(result) or
512512
n = TUntilModifierExpr(result) or
513513
n = TVariableReferencePattern(result) or
514-
n = TWhenExpr(result) or
514+
n = TWhenClause(result) or
515515
n = TWhileExpr(result) or
516516
n = TWhileModifierExpr(result) or
517517
n = TYieldCall(result)
@@ -657,10 +657,9 @@ class TSelf = TSelfReal or TSelfSynth;
657657
class TDestructuredLhsExpr = TDestructuredLeftAssignment or TLeftAssignmentList;
658658

659659
class TExpr =
660-
TSelf or TArgumentList or TInClause or TRescueClause or TRescueModifierExpr or TPair or
661-
TStringConcatenation or TCall or TBlockArgument or TConstantAccess or TControlExpr or
662-
TWhenExpr or TLiteral or TCallable or TVariableAccess or TStmtSequence or TOperation or
663-
TForwardArgument or TDestructuredLhsExpr;
660+
TSelf or TArgumentList or TRescueClause or TRescueModifierExpr or TPair or TStringConcatenation or
661+
TCall or TBlockArgument or TConstantAccess or TControlExpr or TLiteral or TCallable or
662+
TVariableAccess or TStmtSequence or TOperation or TForwardArgument or TDestructuredLhsExpr;
664663

665664
class TSplatExpr = TSplatExprReal or TSplatExprSynth;
666665

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,17 +5,17 @@ private import codeql.ruby.ast.internal.AST
55
abstract class CaseExprImpl extends ControlExpr, TCase {
66
abstract Expr getValue();
77

8-
abstract Expr getBranch(int n);
8+
abstract AstNode getBranch(int n);
99
}
1010

11-
class CaseWhenExpr extends CaseExprImpl, TCaseExpr {
11+
class CaseWhenClause extends CaseExprImpl, TCaseExpr {
1212
private Ruby::Case g;
1313

14-
CaseWhenExpr() { this = TCaseExpr(g) }
14+
CaseWhenClause() { this = TCaseExpr(g) }
1515

1616
final override Expr getValue() { toGenerated(result) = g.getValue() }
1717

18-
final override Expr getBranch(int n) {
18+
final override AstNode getBranch(int n) {
1919
toGenerated(result) = g.getChild(n) or
2020
toGenerated(result) = g.getChild(n)
2121
}
@@ -28,7 +28,7 @@ class CaseMatch extends CaseExprImpl, TCaseMatch {
2828

2929
final override Expr getValue() { toGenerated(result) = g.getValue() }
3030

31-
final override Expr getBranch(int n) {
31+
final override AstNode getBranch(int n) {
3232
toGenerated(result) = g.getClauses(n)
3333
or
3434
n = count(g.getClauses(_)) and toGenerated(result) = g.getElse()

‎ruby/ql/lib/codeql/ruby/controlflow/CfgNodes.qll

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -338,7 +338,7 @@ module ExprNodes {
338338
}
339339

340340
private class CaseExprChildMapping extends ExprChildMapping, CaseExpr {
341-
override predicate relevantChild(Expr e) { e = this.getValue() or e = this.getBranch(_) }
341+
override predicate relevantChild(Expr e) { e = this.getValue() }
342342
}
343343

344344
/** A control-flow node that wraps a `MethodCall` AST expression. */
@@ -356,11 +356,6 @@ module ExprNodes {
356356

357357
/** Gets the expression being compared, if any. */
358358
final ExprCfgNode getValue() { e.hasCfgChild(e.getValue(), this, result) }
359-
360-
/**
361-
* Gets the `n`th branch of this case expression.
362-
*/
363-
final ExprCfgNode getBranch(int n) { e.hasCfgChild(e.getBranch(n), this, result) }
364359
}
365360

366361
private class ConditionalExprChildMapping extends ExprChildMapping, ConditionalExpr {

‎ruby/ql/lib/codeql/ruby/controlflow/internal/Completion.qll

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -107,11 +107,7 @@ abstract class Completion extends TCompletion {
107107
n = any(RescueModifierExpr parent).getBody() and
108108
this = [TSimpleCompletion().(TCompletion), TRaiseCompletion()]
109109
or
110-
(
111-
mayRaise(n)
112-
or
113-
n instanceof CaseMatch and not exists(n.(CaseExpr).getElseBranch())
114-
) and
110+
mayRaise(n) and
115111
(
116112
this = TRaiseCompletion()
117113
or
@@ -209,7 +205,7 @@ private predicate inBooleanContext(AstNode n) {
209205
or
210206
n = any(StmtSequence parent | inBooleanContext(parent)).getLastStmt()
211207
or
212-
exists(CaseExpr c, WhenExpr w |
208+
exists(CaseExpr c, WhenClause w |
213209
not exists(c.getValue()) and
214210
c.getABranch() = w and
215211
w.getPattern(_) = n
@@ -231,7 +227,7 @@ private predicate mustHaveMatchingCompletion(AstNode n) {
231227
private predicate inMatchingContext(AstNode n) {
232228
n = any(RescueClause r).getException(_)
233229
or
234-
exists(CaseExpr c, WhenExpr w |
230+
exists(CaseExpr c, WhenClause w |
235231
exists(c.getValue()) and
236232
c.getABranch() = w and
237233
w.getPattern(_) = n

0 commit comments

Comments
 (0)