Ruby: Refactor isArrayConstant

Author: hmac

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

@@ -36,7 +36,7 @@ private import ExprNodes
  * constant value in some cases.
  */
 private module Propagation {
-  private ExprCfgNode getSource(VariableReadAccessCfgNode read) {
+  ExprCfgNode getSource(VariableReadAccessCfgNode read) {
     exists(Ssa::WriteDefinition def |
       def.assigns(result) and
       read = def.getARead()
@@ -511,7 +511,8 @@ private module Cached {
 import Cached
 
 /**
- * Holds if `e` is an array constructed from an array literal.
+ * Holds if the control flow node `e` refers to an array constructed from the
+ * array literal `arr`.
  * Example:
  * ```rb
  * [1, 2, 3]
@@ -523,9 +524,33 @@ predicate isArrayConstant(ExprCfgNode e, ArrayLiteralCfgNode arr) {
   // [...]
   e = arr
   or
-  // C = [...]; C
-  e.(ExprNodes::ConstantReadAccessCfgNode).getExpr().getValue().getDesugared() = arr.getExpr()
+  // e = [...]; e
+  isArrayConstant(getSource(e), arr)
   or
-  // x = [...]; x
-  exists(Ssa::WriteDefinition def | def.getARead() = e and def.assigns(arr))
+  isArrayExpr(e.getExpr(), arr)
+}
+
+/**
+ * Holds if the expression `e` refers to an array constructed from the array literal `arr`.
+ */
+predicate isArrayExpr(Expr e, ArrayLiteralCfgNode arr) {
+  // e = [...]
+  e = arr.getExpr()
+  or
+  // Like above, but handles the desugaring of array literals to Array.[] calls.
+  e.getDesugared() = arr.getExpr()
+  or
+  // A = [...]; A
+  // A = a; A
+  isArrayExpr(e.(ConstantReadAccess).getValue(), arr)
+  or
+  // Recurse via CFG nodes. Necessary for example in:
+  // a = [...]
+  // A = a
+  // A
+  //
+  // We map from A to a via ConstantReadAccess::getValue, yielding the Expr a.
+  // To get to [...] we need to go via getSource(ExprCfgNode e), so we find a
+  // CFG node for a and call `isArrayConstant`.
+  isArrayConstant(e.getAControlFlowNode(), arr)
 }

ruby/ql/test/library-tests/ast/constants/constants.expected

@@ -61,6 +61,13 @@ constantAccess
 | constants.rb:71:5:71:14 | FOURTY_SIX | write | FOURTY_SIX | ConstantAssignment |
 | constants.rb:73:18:73:21 | Mod3 | read | Mod3 | ConstantReadAccess |
 | constants.rb:73:18:73:33 | FOURTY_SIX | read | FOURTY_SIX | ConstantReadAccess |
+| constants.rb:78:5:78:13 | Array | read | Array | ConstantReadAccess |
+| constants.rb:79:1:79:1 | A | write | A | ConstantAssignment |
+| constants.rb:79:5:79:13 | Array | read | Array | ConstantReadAccess |
+| constants.rb:80:1:80:1 | B | write | B | ConstantAssignment |
+| constants.rb:81:1:81:1 | C | write | C | ConstantAssignment |
+| constants.rb:81:5:81:5 | A | read | A | ConstantReadAccess |
+| constants.rb:82:5:82:5 | B | read | B | ConstantReadAccess |
 getConst
 | constants.rb:1:1:15:3 | ModuleA | CONST_B | constants.rb:6:15:6:23 | "const_b" |
 | constants.rb:1:1:15:3 | ModuleA | FOURTY_FOUR | constants.rb:53:17:53:29 | "fourty-four" |
@@ -133,3 +140,12 @@ constantWriteAccessQualifiedName
 | constants.rb:70:3:72:5 | Mod5 | Mod3::Mod5 |
 | constants.rb:71:5:71:14 | FOURTY_SIX | Mod1::Mod3::Mod5::FOURTY_SIX |
 | constants.rb:71:5:71:14 | FOURTY_SIX | Mod3::Mod5::FOURTY_SIX |
+| constants.rb:79:1:79:1 | A | A |
+| constants.rb:80:1:80:1 | B | B |
+| constants.rb:81:1:81:1 | C | C |
+arrayConstant
+| constants.rb:20:13:20:37 | call to [] | constants.rb:20:13:20:37 | call to [] |
+| constants.rb:78:5:78:13 | call to [] | constants.rb:78:5:78:13 | call to [] |
+| constants.rb:79:5:79:13 | call to [] | constants.rb:79:5:79:13 | call to [] |
+| constants.rb:80:5:80:5 | a | constants.rb:78:5:78:13 | call to [] |
+| constants.rb:81:5:81:5 | A | constants.rb:79:5:79:13 | call to [] |

ruby/ql/test/library-tests/ast/constants/constants.ql

@@ -1,5 +1,6 @@
 import ruby
 import codeql.ruby.ast.internal.Module as M
+import codeql.ruby.ast.internal.Constant
 
 query predicate constantAccess(ConstantAccess a, string kind, string name, string cls) {
   (
@@ -20,3 +21,5 @@ query predicate constantValue(ConstantReadAccess a, Expr e) { e = a.getValue() }
 query predicate constantWriteAccessQualifiedName(ConstantWriteAccess w, string qualifiedName) {
   w.getAQualifiedName() = qualifiedName
 }
+
+query predicate arrayConstant = isArrayConstant/2;

ruby/ql/test/library-tests/ast/constants/constants.rb

@@ -72,3 +72,11 @@ module Mod3::Mod5
   end
   @@fourty_six = Mod3::FOURTY_SIX
 end
+
+# Array constants
+
+a = [1, 2, 3]
+A = [1, 2, 3]
+B = a
+C = A
+b = B