Address comments

Author: aibaars

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

@@ -329,7 +329,7 @@ private module Cached {
 
   private class Access extends Ruby::Token {
     Access() {
-      access(this, _) or
+      access(this.(Ruby::Identifier), _) or
       this instanceof Ruby::GlobalVariable or
       this instanceof Ruby::InstanceVariable or
       this instanceof Ruby::ClassVariable or

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

@@ -712,7 +712,8 @@ module Trees {
       c.(MatchingCompletion).getValue() = false
       or
       last(this.getVariableAccess(), last, any(SimpleCompletion x)) and
-      c.(MatchingCompletion).getValue() = true
+      c.(MatchingCompletion).getValue() = true and
+      not c instanceof NestedCompletion
     }
 
     final override predicate succ(AstNode pred, AstNode succ, Completion c) {

ruby/ql/test/library-tests/dataflow/local/DataflowStep.expected

@@ -70,16 +70,17 @@
 | local_dataflow.rb:50:18:50:18 | [post] x | local_dataflow.rb:51:20:51:20 | x |
 | local_dataflow.rb:50:18:50:18 | x | local_dataflow.rb:51:20:51:20 | x |
 | local_dataflow.rb:51:9:51:15 | "break" | local_dataflow.rb:51:3:51:15 | break |
-| local_dataflow.rb:60:1:89:3 | self (test_case) | local_dataflow.rb:78:12:78:20 | self |
-| local_dataflow.rb:60:1:89:3 | self in test_case | local_dataflow.rb:78:12:78:20 | self |
-| local_dataflow.rb:60:1:89:3 | self in test_case | local_dataflow.rb:79:18:79:24 | self |
-| local_dataflow.rb:60:1:89:3 | self in test_case | local_dataflow.rb:80:22:80:28 | self |
-| local_dataflow.rb:60:1:89:3 | self in test_case | local_dataflow.rb:82:6:82:12 | self |
-| local_dataflow.rb:60:1:89:3 | self in test_case | local_dataflow.rb:83:6:83:12 | self |
-| local_dataflow.rb:60:1:89:3 | self in test_case | local_dataflow.rb:84:6:84:12 | self |
-| local_dataflow.rb:60:1:89:3 | self in test_case | local_dataflow.rb:85:20:85:26 | self |
-| local_dataflow.rb:60:1:89:3 | self in test_case | local_dataflow.rb:86:26:86:32 | self |
-| local_dataflow.rb:60:1:89:3 | self in test_case | local_dataflow.rb:87:18:87:24 | self |
+| local_dataflow.rb:60:1:90:3 | self (test_case) | local_dataflow.rb:78:12:78:20 | self |
+| local_dataflow.rb:60:1:90:3 | self in test_case | local_dataflow.rb:78:12:78:20 | self |
+| local_dataflow.rb:60:1:90:3 | self in test_case | local_dataflow.rb:79:18:79:24 | self |
+| local_dataflow.rb:60:1:90:3 | self in test_case | local_dataflow.rb:80:22:80:28 | self |
+| local_dataflow.rb:60:1:90:3 | self in test_case | local_dataflow.rb:82:6:82:12 | self |
+| local_dataflow.rb:60:1:90:3 | self in test_case | local_dataflow.rb:83:6:83:12 | self |
+| local_dataflow.rb:60:1:90:3 | self in test_case | local_dataflow.rb:84:6:84:12 | self |
+| local_dataflow.rb:60:1:90:3 | self in test_case | local_dataflow.rb:85:20:85:26 | self |
+| local_dataflow.rb:60:1:90:3 | self in test_case | local_dataflow.rb:86:26:86:32 | self |
+| local_dataflow.rb:60:1:90:3 | self in test_case | local_dataflow.rb:87:18:87:24 | self |
+| local_dataflow.rb:60:1:90:3 | self in test_case | local_dataflow.rb:88:3:88:9 | self |
 | local_dataflow.rb:60:15:60:15 | x | local_dataflow.rb:60:15:60:15 | x |
 | local_dataflow.rb:60:15:60:15 | x | local_dataflow.rb:61:12:61:12 | x |
 | local_dataflow.rb:61:7:68:5 | case ... | local_dataflow.rb:61:3:68:5 | ... = ... |
@@ -98,6 +99,8 @@
 | local_dataflow.rb:66:3:67:5 | else ... | local_dataflow.rb:61:7:68:5 | case ... |
 | local_dataflow.rb:67:5:67:5 | x | local_dataflow.rb:66:3:67:5 | else ... |
 | local_dataflow.rb:67:5:67:5 | x | local_dataflow.rb:69:12:69:12 | x |
+| local_dataflow.rb:69:3:76:5 | ... = ... | local_dataflow.rb:88:8:88:8 | z |
+| local_dataflow.rb:69:7:76:5 | case ... | local_dataflow.rb:69:3:76:5 | ... = ... |
 | local_dataflow.rb:69:7:76:5 | case ... | local_dataflow.rb:69:3:76:5 | ... = ... |
 | local_dataflow.rb:69:12:69:12 | x | local_dataflow.rb:71:13:71:13 | x |
 | local_dataflow.rb:69:12:69:12 | x | local_dataflow.rb:73:7:73:7 | x |
@@ -110,7 +113,7 @@
 | local_dataflow.rb:73:7:73:7 | x | local_dataflow.rb:72:7:73:7 | then ... |
 | local_dataflow.rb:74:3:75:6 | else ... | local_dataflow.rb:69:7:76:5 | case ... |
 | local_dataflow.rb:75:6:75:6 | x | local_dataflow.rb:74:3:75:6 | else ... |
-| local_dataflow.rb:78:7:88:3 | case ... | local_dataflow.rb:78:3:88:3 | ... = ... |
+| local_dataflow.rb:78:7:89:3 | case ... | local_dataflow.rb:78:3:89:3 | ... = ... |
 | local_dataflow.rb:78:12:78:20 | [post] self | local_dataflow.rb:79:18:79:24 | self |
 | local_dataflow.rb:78:12:78:20 | [post] self | local_dataflow.rb:80:22:80:28 | self |
 | local_dataflow.rb:78:12:78:20 | [post] self | local_dataflow.rb:82:6:82:12 | self |
@@ -124,28 +127,30 @@
 | local_dataflow.rb:78:12:78:20 | self | local_dataflow.rb:86:26:86:32 | self |
 | local_dataflow.rb:78:12:78:20 | self | local_dataflow.rb:87:18:87:24 | self |
 | local_dataflow.rb:79:11:79:11 | b | local_dataflow.rb:79:23:79:23 | b |
-| local_dataflow.rb:79:13:79:43 | then ... | local_dataflow.rb:78:7:88:3 | case ... |
+| local_dataflow.rb:79:13:79:43 | then ... | local_dataflow.rb:78:7:89:3 | case ... |
 | local_dataflow.rb:79:18:79:24 | call to sink | local_dataflow.rb:79:13:79:43 | then ... |
 | local_dataflow.rb:80:6:80:6 | a | local_dataflow.rb:80:11:80:11 | a |
 | local_dataflow.rb:80:11:80:11 | [post] a | local_dataflow.rb:80:27:80:27 | a |
 | local_dataflow.rb:80:11:80:11 | a | local_dataflow.rb:80:27:80:27 | a |
-| local_dataflow.rb:80:17:80:47 | then ... | local_dataflow.rb:78:7:88:3 | case ... |
+| local_dataflow.rb:80:17:80:47 | then ... | local_dataflow.rb:78:7:89:3 | case ... |
 | local_dataflow.rb:80:22:80:28 | call to sink | local_dataflow.rb:80:17:80:47 | then ... |
 | local_dataflow.rb:81:7:81:7 | c | local_dataflow.rb:82:11:82:11 | c |
 | local_dataflow.rb:81:11:81:11 | d | local_dataflow.rb:83:11:83:11 | d |
 | local_dataflow.rb:81:14:81:14 | e | local_dataflow.rb:84:11:84:11 | e |
-| local_dataflow.rb:81:18:84:32 | then ... | local_dataflow.rb:78:7:88:3 | case ... |
+| local_dataflow.rb:81:18:84:32 | then ... | local_dataflow.rb:78:7:89:3 | case ... |
 | local_dataflow.rb:81:23:84:13 | call to [] | local_dataflow.rb:81:18:84:32 | then ... |
 | local_dataflow.rb:82:6:82:12 | [post] self | local_dataflow.rb:83:6:83:12 | self |
 | local_dataflow.rb:82:6:82:12 | self | local_dataflow.rb:83:6:83:12 | self |
 | local_dataflow.rb:83:6:83:12 | [post] self | local_dataflow.rb:84:6:84:12 | self |
 | local_dataflow.rb:83:6:83:12 | self | local_dataflow.rb:84:6:84:12 | self |
 | local_dataflow.rb:85:11:85:11 | f | local_dataflow.rb:85:25:85:25 | f |
-| local_dataflow.rb:85:15:85:45 | then ... | local_dataflow.rb:78:7:88:3 | case ... |
+| local_dataflow.rb:85:15:85:45 | then ... | local_dataflow.rb:78:7:89:3 | case ... |
 | local_dataflow.rb:85:20:85:26 | call to sink | local_dataflow.rb:85:15:85:45 | then ... |
 | local_dataflow.rb:86:16:86:16 | g | local_dataflow.rb:86:31:86:31 | g |
-| local_dataflow.rb:86:21:86:51 | then ... | local_dataflow.rb:78:7:88:3 | case ... |
+| local_dataflow.rb:86:21:86:51 | then ... | local_dataflow.rb:78:7:89:3 | case ... |
 | local_dataflow.rb:86:26:86:32 | call to sink | local_dataflow.rb:86:21:86:51 | then ... |
 | local_dataflow.rb:87:8:87:8 | x | local_dataflow.rb:87:23:87:23 | x |
-| local_dataflow.rb:87:13:87:43 | then ... | local_dataflow.rb:78:7:88:3 | case ... |
-| local_dataflow.rb:87:18:87:24 | call to sink | local_dataflow.rb:87:13:87:43 | then ... |
+| local_dataflow.rb:87:13:88:28 | then ... | local_dataflow.rb:78:7:89:3 | case ... |
+| local_dataflow.rb:87:18:87:24 | [post] self | local_dataflow.rb:88:3:88:9 | self |
+| local_dataflow.rb:87:18:87:24 | self | local_dataflow.rb:88:3:88:9 | self |
+| local_dataflow.rb:88:3:88:9 | call to sink | local_dataflow.rb:87:13:88:28 | then ... |

ruby/ql/test/library-tests/dataflow/local/Nodes.expected

@@ -12,7 +12,7 @@ ret
 | local_dataflow.rb:50:3:50:13 | next |
 | local_dataflow.rb:51:3:51:15 | break |
 | local_dataflow.rb:52:3:52:10 | "normal" |
-| local_dataflow.rb:78:3:88:3 | ... = ... |
+| local_dataflow.rb:78:3:89:3 | ... = ... |
 arg
 | local_dataflow.rb:3:8:3:10 | self | local_dataflow.rb:3:8:3:10 | call to p | self |
 | local_dataflow.rb:3:10:3:10 | a | local_dataflow.rb:3:8:3:10 | call to p | position 0 |
@@ -73,3 +73,5 @@ arg
 | local_dataflow.rb:86:31:86:31 | g | local_dataflow.rb:86:26:86:32 | call to sink | position 0 |
 | local_dataflow.rb:87:18:87:24 | self | local_dataflow.rb:87:18:87:24 | call to sink | self |
 | local_dataflow.rb:87:23:87:23 | x | local_dataflow.rb:87:18:87:24 | call to sink | position 0 |
+| local_dataflow.rb:88:3:88:9 | self | local_dataflow.rb:88:3:88:9 | call to sink | self |
+| local_dataflow.rb:88:8:88:8 | z | local_dataflow.rb:88:3:88:9 | call to sink | position 0 |

ruby/ql/test/library-tests/dataflow/local/TaintflowStep.expected

@@ -1,4 +1,5 @@
 failures
+| local_dataflow.rb:88:11:88:28 | # $ hasTaintFlow=1 | Missing result:hasTaintFlow=1 |
 edges
 | local_dataflow.rb:78:12:78:20 | call to source :  | local_dataflow.rb:79:23:79:23 | b |
 | local_dataflow.rb:78:12:78:20 | call to source :  | local_dataflow.rb:80:27:80:27 | a |

ruby/ql/test/library-tests/dataflow/local/local_dataflow.rb

@@ -85,6 +85,7 @@ def test_case x
   in { a: f } then sink(f) # $ hasTaintFlow=1
   in { foo: 1, g: } then sink(g) # $ hasTaintFlow=1
   in { x: } then sink(x) # $ hasTaintFlow=1
+  sink(z) # $ hasTaintFlow=1
 end
 end