Skip to content

Commit 0ef4595

Browse files
committed
Ruby: Fix bug in implicitAssignmentNode
1 parent ec9c91c commit 0ef4595

4 files changed

Lines changed: 16 additions & 15 deletions

File tree

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

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,17 @@ private import codeql.ruby.ast.internal.Pattern
1111
private import codeql.ruby.ast.internal.Scope
1212
private import codeql.ruby.ast.internal.Synthesis
1313

14+
private Ruby::AstNode getAssignmentParent(Ruby::AstNode n) {
15+
result = n.getParent() and
16+
(
17+
result instanceof Ruby::DestructuredLeftAssignment
18+
or
19+
result instanceof Ruby::LeftAssignmentList
20+
or
21+
result instanceof Ruby::RestAssignment
22+
)
23+
}
24+
1425
/**
1526
* Holds if `n` is in the left-hand-side of an explicit assignment `assignment`.
1627
*/
@@ -19,16 +30,7 @@ predicate explicitAssignmentNode(Ruby::AstNode n, Ruby::AstNode assignment) {
1930
or
2031
n = assignment.(Ruby::OperatorAssignment).getLeft()
2132
or
22-
exists(Ruby::AstNode parent |
23-
parent = n.getParent() and
24-
explicitAssignmentNode(parent, assignment)
25-
|
26-
parent instanceof Ruby::DestructuredLeftAssignment
27-
or
28-
parent instanceof Ruby::LeftAssignmentList
29-
or
30-
parent instanceof Ruby::RestAssignment
31-
)
33+
explicitAssignmentNode(getAssignmentParent(n), assignment)
3234
}
3335

3436
/** Holds if `n` is inside an implicit assignment. */
@@ -49,7 +51,7 @@ predicate implicitAssignmentNode(Ruby::AstNode n) {
4951
or
5052
n = any(Ruby::For for).getPattern()
5153
or
52-
implicitAssignmentNode(n.getParent())
54+
implicitAssignmentNode(getAssignmentParent(n))
5355
}
5456

5557
/** Holds if `n` is inside a parameter. */

ruby/ql/test/library-tests/variables/ssa.expected

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -321,6 +321,7 @@ read
321321
| scopes.rb:80:3:82:5 | self (name=) | scopes.rb:80:3:82:5 | self | scopes.rb:81:5:81:9 | self |
322322
| scopes.rb:80:13:80:17 | value | scopes.rb:80:13:80:17 | value | scopes.rb:81:13:81:17 | value |
323323
| scopes.rb:84:3:88:5 | self (foo) | scopes.rb:84:3:88:5 | self | scopes.rb:85:5:85:13 | self |
324+
| scopes.rb:84:3:88:5 | self (foo) | scopes.rb:84:3:88:5 | self | scopes.rb:86:13:86:16 | self |
324325
| scopes.rb:84:11:84:13 | msg | scopes.rb:84:11:84:13 | msg | scopes.rb:85:11:85:13 | msg |
325326
| ssa.rb:1:1:16:3 | self (m) | ssa.rb:1:1:16:3 | self | ssa.rb:3:3:3:8 | self |
326327
| ssa.rb:1:1:16:3 | self (m) | ssa.rb:1:1:16:3 | self | ssa.rb:4:3:4:12 | self |
@@ -592,6 +593,7 @@ adjacentReads
592593
| scopes.rb:51:1:64:3 | self (ExceptionVariable) | scopes.rb:51:1:64:3 | self | scopes.rb:59:5:59:21 | self | scopes.rb:61:5:61:10 | self |
593594
| scopes.rb:51:1:64:3 | self (ExceptionVariable) | scopes.rb:51:1:64:3 | self | scopes.rb:61:5:61:10 | self | scopes.rb:63:3:63:8 | self |
594595
| scopes.rb:60:25:60:25 | x | scopes.rb:55:3:55:3 | x | scopes.rb:61:10:61:10 | x | scopes.rb:63:8:63:8 | x |
596+
| scopes.rb:84:3:88:5 | self (foo) | scopes.rb:84:3:88:5 | self | scopes.rb:85:5:85:13 | self | scopes.rb:86:13:86:16 | self |
595597
| ssa.rb:1:1:16:3 | self (m) | ssa.rb:1:1:16:3 | self | ssa.rb:3:3:3:8 | self | ssa.rb:4:3:4:12 | self |
596598
| ssa.rb:1:1:16:3 | self (m) | ssa.rb:1:1:16:3 | self | ssa.rb:4:3:4:12 | self | ssa.rb:7:5:7:10 | self |
597599
| ssa.rb:1:1:16:3 | self (m) | ssa.rb:1:1:16:3 | self | ssa.rb:4:3:4:12 | self | ssa.rb:11:5:11:10 | self |

ruby/ql/test/library-tests/variables/varaccess.expected

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -226,7 +226,6 @@ variableAccess
226226
| scopes.rb:85:5:85:13 | self | scopes.rb:84:3:88:5 | self | scopes.rb:84:3:88:5 | foo |
227227
| scopes.rb:85:11:85:13 | msg | scopes.rb:84:11:84:13 | msg | scopes.rb:84:3:88:5 | foo |
228228
| scopes.rb:86:13:86:16 | self | scopes.rb:84:3:88:5 | self | scopes.rb:84:3:88:5 | foo |
229-
| scopes.rb:86:18:86:21 | name | scopes.rb:86:18:86:21 | name | scopes.rb:84:3:88:5 | foo |
230229
| ssa.rb:1:7:1:7 | b | ssa.rb:1:7:1:7 | b | ssa.rb:1:1:16:3 | m |
231230
| ssa.rb:2:3:2:3 | i | ssa.rb:2:3:2:3 | i | ssa.rb:1:1:16:3 | m |
232231
| ssa.rb:3:3:3:8 | self | ssa.rb:1:1:16:3 | self | ssa.rb:1:1:16:3 | m |
@@ -436,8 +435,6 @@ implicitWrite
436435
| scopes.rb:69:15:69:15 | x |
437436
| scopes.rb:80:13:80:17 | value |
438437
| scopes.rb:84:11:84:13 | msg |
439-
| scopes.rb:86:13:86:16 | self |
440-
| scopes.rb:86:18:86:21 | name |
441438
| ssa.rb:1:7:1:7 | b |
442439
| ssa.rb:18:8:18:8 | x |
443440
| ssa.rb:25:8:25:15 | elements |
@@ -606,6 +603,7 @@ readAccess
606603
| scopes.rb:81:13:81:17 | value |
607604
| scopes.rb:85:5:85:13 | self |
608605
| scopes.rb:85:11:85:13 | msg |
606+
| scopes.rb:86:13:86:16 | self |
609607
| ssa.rb:3:3:3:8 | self |
610608
| ssa.rb:3:8:3:8 | i |
611609
| ssa.rb:4:3:4:12 | self |

ruby/ql/test/library-tests/variables/variable.expected

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,6 @@
138138
| scopes.rb:80:13:80:17 | value |
139139
| scopes.rb:84:3:88:5 | self |
140140
| scopes.rb:84:11:84:13 | msg |
141-
| scopes.rb:86:18:86:21 | name |
142141
| ssa.rb:1:1:16:3 | self |
143142
| ssa.rb:1:1:103:3 | self |
144143
| ssa.rb:1:7:1:7 | b |

0 commit comments

Comments
 (0)