Skip to content

Commit 58ecd77

Browse files
hvitvedhvitved
committed
AST: Exclude empty then/else/do statements
1 parent ca7c058 commit 58ecd77

10 files changed

Lines changed: 130 additions & 9 deletions

File tree

‎ql/src/codeql_ruby/ast/internal/AST.qll‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -94,10 +94,10 @@ private module Cached {
9494
not strictcount(int i | exists(g.getParent().(Generated::LeftAssignmentList).getChild(i))) = 1
9595
} or
9696
TDivExpr(Generated::Binary g) { g instanceof @binary_slash } or
97-
TDo(Generated::Do g) or
97+
TDo(Generated::Do g) { exists(g.getChild(_)) } or
9898
TDoBlock(Generated::DoBlock g) { not g.getParent() instanceof Generated::Lambda } or
9999
TElementReference(Generated::ElementReference g) or
100-
TElse(Generated::Else g) or
100+
TElse(Generated::Else g) { exists(g.getChild(_)) } or
101101
TElsif(Generated::Elsif g) or
102102
TEmptyStmt(Generated::EmptyStatement g) or
103103
TEndBlock(Generated::EndBlock g) or
@@ -198,7 +198,7 @@ private module Cached {
198198
TSubshellLiteral(Generated::Subshell g) or
199199
TSymbolArrayLiteral(Generated::SymbolArray g) or
200200
TTernaryIfExpr(Generated::Conditional g) or
201-
TThen(Generated::Then g) or
201+
TThen(Generated::Then g) { exists(g.getChild(_)) } or
202202
TTokenConstantAccess(Generated::Constant g) {
203203
// A tree-sitter `constant` token is a read of that constant in any context
204204
// where an identifier would be a vcall.

‎ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -643,10 +643,17 @@ module Trees {
643643

644644
final override predicate first(AstNode first) { first(this.getCondition(), first) }
645645

646+
private AstNode getConditionSucc() {
647+
result = this.getBody()
648+
or
649+
not exists(this.getBody()) and
650+
result = this.getCondition()
651+
}
652+
646653
final override predicate succ(AstNode pred, AstNode succ, Completion c) {
647654
last(this.getCondition(), pred, c) and
648655
this.entersLoopWhenConditionIs(c.(BooleanCompletion).getValue()) and
649-
first(this.getBody(), succ)
656+
first(this.getConditionSucc(), succ)
650657
or
651658
last(this.getBody(), pred, c) and
652659
first(this.getCondition(), succ) and

‎ql/test/library-tests/ast/Ast.expected‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -671,6 +671,18 @@ control/conditionals.rb:
671671
# 58| getBranch/getElse: [SubExpr] ... - ...
672672
# 58| getAnOperand/getLeftOperand: [LocalVariableAccess] e
673673
# 58| getAnOperand/getRightOperand: [IntegerLiteral] 2
674+
# 61| getStmt: [IfExpr] if ...
675+
# 61| getCondition: [GTExpr] ... > ...
676+
# 61| getAnOperand/getGreaterOperand/getLeftOperand: [LocalVariableAccess] a
677+
# 61| getAnOperand/getLesserOperand/getRightOperand: [LocalVariableAccess] b
678+
# 61| getBranch/getThen: [StmtSequence] c
679+
# 62| getStmt: [LocalVariableAccess] c
680+
# 67| getStmt: [IfExpr] if ...
681+
# 67| getCondition: [GTExpr] ... > ...
682+
# 67| getAnOperand/getGreaterOperand/getLeftOperand: [LocalVariableAccess] a
683+
# 67| getAnOperand/getLesserOperand/getRightOperand: [LocalVariableAccess] b
684+
# 68| getBranch/getElse: [StmtSequence] c
685+
# 69| getStmt: [LocalVariableAccess] c
674686
constants/constants.rb:
675687
# 1| [Toplevel] constants.rb
676688
# 1| getStmt: [Module] ModuleA
@@ -1262,6 +1274,10 @@ control/loops.rb:
12621274
# 63| getCondition: [EqExpr] ... == ...
12631275
# 63| getAnOperand/getLeftOperand: [LocalVariableAccess] x
12641276
# 63| getAnOperand/getRightOperand: [IntegerLiteral] 0
1277+
# 66| getStmt: [WhileExpr] while ...
1278+
# 66| getCondition: [LTExpr] ... < ...
1279+
# 66| getAnOperand/getLeftOperand/getLesserOperand: [LocalVariableAccess] x
1280+
# 66| getAnOperand/getGreaterOperand/getRightOperand: [LocalVariableAccess] y
12651281
misc/misc.rb:
12661282
# 1| [Toplevel] misc.rb
12671283
# 1| getStmt: [AssignExpr] ... = ...

‎ql/test/library-tests/ast/control/ConditionalExpr.expected‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ conditionalExprs
1818
| conditionals.rb:55:1:55:18 | ... unless ... | UnlessModifierExpr | conditionals.rb:55:14:55:18 | ... < ... | conditionals.rb:55:1:55:5 | ... = ... | false |
1919
| conditionals.rb:58:5:58:25 | ... ? ... : ... | TernaryIfExpr | conditionals.rb:58:5:58:9 | ... > ... | conditionals.rb:58:13:58:17 | ... + ... | true |
2020
| conditionals.rb:58:5:58:25 | ... ? ... : ... | TernaryIfExpr | conditionals.rb:58:5:58:9 | ... > ... | conditionals.rb:58:21:58:25 | ... - ... | false |
21+
| conditionals.rb:61:1:64:3 | if ... | IfExpr | conditionals.rb:61:4:61:8 | ... > ... | conditionals.rb:61:10:62:5 | c | true |
22+
| conditionals.rb:67:1:70:3 | if ... | IfExpr | conditionals.rb:67:4:67:8 | ... > ... | conditionals.rb:68:1:69:5 | c | false |
2123
ifExprs
2224
| conditionals.rb:10:1:12:3 | if ... | IfExpr | conditionals.rb:10:4:10:8 | ... > ... | conditionals.rb:10:10:11:5 | c | (none) | false |
2325
| conditionals.rb:15:1:19:3 | if ... | IfExpr | conditionals.rb:15:4:15:9 | ... == ... | conditionals.rb:15:10:16:5 | c | d | false |
@@ -26,6 +28,7 @@ ifExprs
2628
| conditionals.rb:26:1:29:5 | elsif ... | IfExpr | conditionals.rb:26:7:26:12 | ... == ... | conditionals.rb:26:14:27:5 | e | f | true |
2729
| conditionals.rb:33:1:37:3 | if ... | IfExpr | conditionals.rb:33:4:33:9 | ... == ... | conditionals.rb:33:10:34:5 | b | elsif ... | false |
2830
| conditionals.rb:35:1:36:5 | elsif ... | IfExpr | conditionals.rb:35:7:35:12 | ... == ... | conditionals.rb:35:13:36:5 | c | (none) | true |
31+
| conditionals.rb:61:1:64:3 | if ... | IfExpr | conditionals.rb:61:4:61:8 | ... > ... | conditionals.rb:61:10:62:5 | c | (none) | false |
2932
unlessExprs
3033
| conditionals.rb:40:1:42:3 | unless ... | UnlessExpr | conditionals.rb:40:8:40:12 | ... > ... | conditionals.rb:40:14:41:5 | c | (none) |
3134
| conditionals.rb:45:1:49:3 | unless ... | UnlessExpr | conditionals.rb:45:8:45:13 | ... == ... | conditionals.rb:45:14:46:5 | c | d |

‎ql/test/library-tests/ast/control/ControlExpr.expected‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@
1212
| conditionals.rb:52:1:52:14 | ... if ... | IfModifierExpr |
1313
| conditionals.rb:55:1:55:18 | ... unless ... | UnlessModifierExpr |
1414
| conditionals.rb:58:5:58:25 | ... ? ... : ... | TernaryIfExpr |
15+
| conditionals.rb:61:1:64:3 | if ... | IfExpr |
16+
| conditionals.rb:67:1:70:3 | if ... | IfExpr |
1517
| loops.rb:9:1:12:3 | for ... in ... | ForExpr |
1618
| loops.rb:16:1:19:3 | for ... in ... | ForExpr |
1719
| loops.rb:22:1:25:3 | for ... in ... | ForExpr |
@@ -22,3 +24,4 @@
2224
| loops.rb:51:1:54:3 | until ... | UntilExpr |
2325
| loops.rb:57:1:60:3 | until ... | UntilExpr |
2426
| loops.rb:63:1:63:19 | ... until ... | UntilModifierExpr |
27+
| loops.rb:66:1:67:3 | while ... | WhileExpr |

‎ql/test/library-tests/ast/control/conditionals.rb‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,4 +55,16 @@
5555
a = b unless c < d
5656

5757
# Ternary if expr
58-
a = b > c ? d + 1 : e - 2
58+
a = b > c ? d + 1 : e - 2
59+
60+
# If expr with empty else (treated as no else)
61+
if a > b then
62+
c
63+
else
64+
end
65+
66+
# If expr with empty then (treated as no then)
67+
if a > b then
68+
else
69+
c
70+
end

‎ql/test/library-tests/ast/control/loops.rb‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,4 +60,8 @@
6060
end
6161

6262
# Until-modified expression
63-
x -= 1 until x == 0
63+
x -= 1 until x == 0
64+
65+
# While loop with empty `do` block
66+
while x < y do
67+
end

‎ql/test/library-tests/controlflow/graph/Cfg.expected‎

Lines changed: 65 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2330,7 +2330,7 @@ ifs.rb:
23302330
#-----| -> true
23312331

23322332
# 40| constant_condition
2333-
#-----| -> exit ifs.rb (normal)
2333+
#-----| -> empty_else
23342334

23352335
# 40| exit constant_condition
23362336

@@ -2346,6 +2346,39 @@ ifs.rb:
23462346
# 41| true
23472347
#-----| true -> [false] ! ...
23482348

2349+
# 46| enter empty_else
2350+
#-----| -> b
2351+
2352+
# 46| empty_else
2353+
#-----| -> exit ifs.rb (normal)
2354+
2355+
# 46| exit empty_else
2356+
2357+
# 46| exit empty_else (normal)
2358+
#-----| -> exit empty_else
2359+
2360+
# 46| b
2361+
#-----| -> b
2362+
2363+
# 47| if ...
2364+
#-----| -> "done"
2365+
2366+
# 47| b
2367+
#-----| true -> "true"
2368+
#-----| false -> if ...
2369+
2370+
# 48| call to puts
2371+
#-----| -> if ...
2372+
2373+
# 48| "true"
2374+
#-----| -> call to puts
2375+
2376+
# 51| call to puts
2377+
#-----| -> exit empty_else (normal)
2378+
2379+
# 51| "done"
2380+
#-----| -> call to puts
2381+
23492382
loops.rb:
23502383
# 1| enter m1
23512384
#-----| -> x
@@ -2503,7 +2536,7 @@ loops.rb:
25032536
#-----| -> 1
25042537

25052538
# 24| m3
2506-
#-----| -> exit loops.rb (normal)
2539+
#-----| -> m4
25072540

25082541
# 24| exit m3
25092542

@@ -2545,6 +2578,36 @@ loops.rb:
25452578
# 26| x
25462579
#-----| -> call to puts
25472580

2581+
# 30| enter m4
2582+
#-----| -> x
2583+
2584+
# 30| m4
2585+
#-----| -> exit loops.rb (normal)
2586+
2587+
# 30| exit m4
2588+
2589+
# 30| exit m4 (normal)
2590+
#-----| -> exit m4
2591+
2592+
# 30| x
2593+
#-----| -> y
2594+
2595+
# 30| y
2596+
#-----| -> x
2597+
2598+
# 31| while ...
2599+
#-----| -> exit m4 (normal)
2600+
2601+
# 31| ... < ...
2602+
#-----| true -> x
2603+
#-----| false -> while ...
2604+
2605+
# 31| x
2606+
#-----| -> y
2607+
2608+
# 31| y
2609+
#-----| -> ... < ...
2610+
25482611
raise.rb:
25492612
# 1| enter raise.rb
25502613
#-----| -> ExceptionA

‎ql/test/library-tests/controlflow/graph/ifs.rb‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,3 +42,11 @@ def constant_condition()
4242
puts "Impossible"
4343
end
4444
end
45+
46+
def empty_else b
47+
if b then
48+
puts "true"
49+
else
50+
end
51+
puts "done"
52+
end

‎ql/test/library-tests/controlflow/graph/loops.rb‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,4 +25,9 @@ def m3
2525
[1,2,3].each do |x|
2626
puts x
2727
end
28-
end
28+
end
29+
30+
def m4(x, y)
31+
while x < y do
32+
end
33+
end

0 commit comments

Comments
 (0)