Merge pull request #10692 from aibaars/fix-splats

Ruby: fix CFG and toString for anonymous '*' and '**'

Author: aibaars

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

@@ -183,7 +183,11 @@ class HashSplatParameter extends NamedParameter, THashSplatParameter {
 
   final override LocalVariable getVariable() { result = TLocalVariableReal(_, _, g.getName()) }
 
-  final override string toString() { result = "**" + this.getName() }
+  final override string toString() {
+    result = "**" + this.getName()
+    or
+    not exists(g.getName()) and result = "**"
+  }
 
   final override string getName() { result = g.getName().getValue() }
 }
@@ -305,7 +309,11 @@ class SplatParameter extends NamedParameter, TSplatParameter {
 
   final override LocalVariable getVariable() { result = TLocalVariableReal(_, _, g.getName()) }
 
-  final override string toString() { result = "*" + this.getName() }
+  final override string toString() {
+    result = "*" + this.getName()
+    or
+    not exists(g.getName()) and result = "*"
+  }
 
   final override string getName() { result = g.getName().getValue() }
 }

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

@@ -139,10 +139,16 @@ module Trees {
   abstract private class NonDefaultValueParameterTree extends ControlFlowTree, NamedParameter {
     final override predicate first(AstNode first) {
       this.getDefiningAccess().(ControlFlowTree).first(first)
+      or
+      not exists(this.getDefiningAccess()) and first = this
     }
 
     final override predicate last(AstNode last, Completion c) {
       this.getDefiningAccess().(ControlFlowTree).last(last, c)
+      or
+      not exists(this.getDefiningAccess()) and
+      last = this and
+      c.isValidFor(this)
     }
 
     override predicate propagatesAbnormal(AstNode child) {

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

@@ -3649,7 +3649,7 @@ cfg.rb:
 #-----|  -> call to foo
 
 #  205| ...
-#-----|  -> exit cfg.rb (normal)
+#-----|  -> filter_nil
 
 #  205| call to bar
 #-----|  -> if ...
@@ -3687,6 +3687,52 @@ cfg.rb:
 #  205| x
 #-----|  -> exit { ... } (normal)
 
+#  207| enter filter_nil
+#-----|  -> list
+
+#  207| exit filter_nil
+
+#  207| exit filter_nil (normal)
+#-----|  -> exit filter_nil
+
+#  207| filter_nil
+#-----|  -> exit cfg.rb (normal)
+
+#  207| list
+#-----|  -> list
+
+#  208| list
+#-----|  -> do ... end
+
+#  208| call to reject
+#-----|  -> exit filter_nil (normal)
+
+#  208| do ... end
+#-----|  -> call to reject
+
+#  208| enter do ... end
+#-----|  -> elem
+
+#  208| exit do ... end
+
+#  208| exit do ... end (normal)
+#-----|  -> exit do ... end
+
+#  208| elem
+#-----|  -> *
+
+#  208| *
+#-----|  -> **
+
+#  208| **
+#-----|  -> elem
+
+#  209| elem
+#-----|  -> call to nil?
+
+#  209| call to nil?
+#-----|  -> exit do ... end (normal)
+
 desugar.rb:
 #    1| enter m1
 #-----|  -> x

ruby/ql/test/library-tests/controlflow/graph/Nodes.expected

@@ -42,6 +42,8 @@ callsWithNoArguments
 | cfg.rb:200:1:200:32 | call to times |
 | cfg.rb:202:1:202:35 | call to times |
 | cfg.rb:205:1:205:3 | call to foo |
+| cfg.rb:208:3:210:5 | call to reject |
+| cfg.rb:209:5:209:13 | call to nil? |
 | desugar.rb:6:3:6:7 | call to foo |
 | desugar.rb:10:3:10:7 | call to foo |
 | desugar.rb:14:3:14:7 | call to foo |

ruby/ql/test/library-tests/controlflow/graph/cfg.rb

@@ -204,6 +204,12 @@ def forward_param(a, b, ...)
 # A call with a safe navigation operator
 foo&.bar(1,2) { |x| x }
 
+def filter_nil list
+  list.reject do |elem, *, **|
+    elem.nil?
+  end
+end
+
 __END__
 
 Some ignored nonsense