Ruby: Values in Hash literals and keyword arguments can be omitted

Author: aibaars

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

@@ -293,13 +293,16 @@ class Pair extends Expr, TPair {
   final Expr getKey() { toGenerated(result) = g.getKey() }
 
   /**
-   * Gets the value expression of this pair. For example, the `InteralLiteral`
+   * Gets the value expression of this pair. For example, the `IntegerLiteral`
    * 123 in the following hash pair:
    * ```rb
    * { 'foo' => 123 }
    * ```
    */
-  final Expr getValue() { toGenerated(result) = g.getValue() }
+  final Expr getValue() {
+    toGenerated(result) = g.getValue() or
+    synthChild(this, 0, result)
+  }
 
   final override string toString() { result = "Pair" }
 

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

@@ -975,24 +975,60 @@ private module ForLoopDesugar {
  * ```
  */
 private module ImplicitHashValueSynthesis {
-  private Ruby::AstNode keyWithoutValue(HashPattern parent, int i) {
+  private Ruby::AstNode keyWithoutValue(AstNode parent, int i) {
     exists(Ruby::KeywordPattern pair |
       result = pair.getKey() and
-      result = toGenerated(parent.getKey(i)) and
+      result = toGenerated(parent.(HashPattern).getKey(i)) and
+      not exists(pair.getValue())
+    )
+    or
+    exists(Ruby::Pair pair |
+      i = 0 and
+      result = pair.getKey() and
+      pair = toGenerated(parent) and
       not exists(pair.getValue())
     )
   }
 
+  private string keyName(Ruby::AstNode key) {
+    result = key.(Ruby::String).getChild(0).(Ruby::StringContent).getValue() or
+    result = key.(Ruby::HashKeySymbol).getValue()
+  }
+
   private class ImplicitHashValueSynthesis extends Synthesis {
     final override predicate child(AstNode parent, int i, Child child) {
-      exists(TVariableReal variable |
-        access(keyWithoutValue(parent, i), variable) and
-        child = SynthChild(LocalVariableAccessRealKind(variable))
+      exists(Ruby::AstNode key | key = keyWithoutValue(parent, i) |
+        exists(TVariableReal variable |
+          access(key, variable) and
+          child = SynthChild(LocalVariableAccessRealKind(variable))
+        )
+        or
+        not access(key, _) and
+        exists(string name | name = keyName(key) |
+          child = SynthChild(ConstantReadAccessKind(name)) or
+          child = SynthChild(MethodCallKind(name, false, 0))
+        )
       )
     }
 
+    final override predicate methodCall(string name, boolean setter, int arity) {
+      setter = false and
+      arity = 0 and
+      name = keyName(keyWithoutValue(_, _)) and
+      not name.charAt(0).isUppercase()
+    }
+
+    final override predicate constantReadAccess(string name) {
+      name = keyName(keyWithoutValue(_, _)) and
+      name.charAt(0).isUppercase()
+    }
+
     final override predicate location(AstNode n, Location l) {
-      exists(HashPattern p, int i | n = p.getValue(i) and l = keyWithoutValue(p, i).getLocation())
+      exists(AstNode p, int i | l = keyWithoutValue(p, i).getLocation() |
+        n = p.(HashPattern).getValue(i)
+        or
+        i = 0 and n = p.(Pair).getValue()
+      )
     }
   }
 }

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

@@ -116,6 +116,11 @@ private string variableNameInScope(Ruby::AstNode i, Scope::Range scope) {
       result = i.(Ruby::String).getChild(0).(Ruby::StringContent).getValue() or
       result = i.(Ruby::HashKeySymbol).getValue()
     )
+    or
+    exists(Ruby::Pair p | i = p.getKey() and not exists(p.getValue()) |
+      result = i.(Ruby::String).getChild(0).(Ruby::StringContent).getValue() or
+      result = i.(Ruby::HashKeySymbol).getValue()
+    )
   )
 }
 

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

@@ -676,6 +676,29 @@ calls/calls.rb:
 #  341|         getArgument: [LocalVariableAccess] x
 #  341|         getArgument: [LocalVariableAccess] y
 #  341|         getArgument: [LocalVariableAccess] z
+#  344|   getStmt: [MethodCall] call to foo
+#  344|     getReceiver: [Self, SelfVariableAccess] self
+#  344|     getArgument: [Pair] Pair
+#  344|       getKey: [SymbolLiteral] :x
+#  344|       getValue: [IntegerLiteral] 42
+#  345|   getStmt: [MethodCall] call to foo
+#  345|     getReceiver: [Self, SelfVariableAccess] self
+#  345|     getArgument: [Pair] Pair
+#  345|       getKey: [SymbolLiteral] :x
+#  345|       getValue: [LocalVariableAccess] x
+#  345|     getArgument: [Pair] Pair
+#  345|       getKey: [SymbolLiteral] :novar
+#  345|       getValue: [MethodCall] call to novar
+#  346|   getStmt: [MethodCall] call to foo
+#  346|     getReceiver: [Self, SelfVariableAccess] self
+#  346|     getArgument: [Pair] Pair
+#  346|       getKey: [SymbolLiteral] :X
+#  346|       getValue: [IntegerLiteral] 42
+#  347|   getStmt: [MethodCall] call to foo
+#  347|     getReceiver: [Self, SelfVariableAccess] self
+#  347|     getArgument: [Pair] Pair
+#  347|       getKey: [SymbolLiteral] :X
+#  347|       getValue: [ConstantReadAccess] X
 control/cases.rb:
 #    1| [Toplevel] cases.rb
 #    2|   getStmt: [AssignExpr] ... = ...
@@ -1963,6 +1986,23 @@ literals/literals.rb:
 #  180|       getComponent: [StringTextComponent] 
 #  180|        cat file.txt
 #  180|       
+#  184|   getStmt: [AssignExpr] ... = ...
+#  184|     getAnOperand/getLeftOperand: [LocalVariableAccess] x
+#  184|     getAnOperand/getRightOperand: [IntegerLiteral] 42
+#  185|   getStmt: [HashLiteral] {...}
+#  185|     getElement: [Pair] Pair
+#  185|       getKey: [SymbolLiteral] :x
+#  185|       getValue: [LocalVariableAccess] x
+#  185|     getElement: [Pair] Pair
+#  185|       getKey: [SymbolLiteral] :y
+#  185|       getValue: [IntegerLiteral] 5
+#  186|   getStmt: [HashLiteral] {...}
+#  186|     getElement: [Pair] Pair
+#  186|       getKey: [SymbolLiteral] :y
+#  186|       getValue: [MethodCall] call to y
+#  186|     getElement: [Pair] Pair
+#  186|       getKey: [SymbolLiteral] :Z
+#  186|       getValue: [ConstantReadAccess] Z
 control/loops.rb:
 #    1| [Toplevel] loops.rb
 #    2|   getStmt: [AssignExpr] ... = ...

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

@@ -477,6 +477,24 @@ literals/literals.rb:
 #  118|     getArgument: [HashSplatExpr] ** ...
 #  118|       getAnOperand/getOperand/getReceiver: [MethodCall] call to baz
 #  118|         getReceiver: [Self, SelfVariableAccess] self
+#  185| [HashLiteral] {...}
+#  185|   getDesugared: [MethodCall] call to []
+#  185|     getReceiver: [ConstantReadAccess] Hash
+#  185|     getArgument: [Pair] Pair
+#  185|       getKey: [SymbolLiteral] :x
+#  185|       getValue: [LocalVariableAccess] x
+#  185|     getArgument: [Pair] Pair
+#  185|       getKey: [SymbolLiteral] :y
+#  185|       getValue: [IntegerLiteral] 5
+#  186| [HashLiteral] {...}
+#  186|   getDesugared: [MethodCall] call to []
+#  186|     getReceiver: [ConstantReadAccess] Hash
+#  186|     getArgument: [Pair] Pair
+#  186|       getKey: [SymbolLiteral] :y
+#  186|       getValue: [MethodCall] call to y
+#  186|     getArgument: [Pair] Pair
+#  186|       getKey: [SymbolLiteral] :Z
+#  186|       getValue: [ConstantReadAccess] Z
 control/loops.rb:
 #    9| [ForExpr] for ... in ...
 #    9|   getDesugared: [MethodCall] call to each

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

@@ -65,6 +65,13 @@
 | calls/calls.rb:340:27:340:27 | 4 | 4 |
 | calls/calls.rb:340:29:340:29 | 5 | 5 |
 | calls/calls.rb:340:31:340:31 | 6 | 6 |
+| calls/calls.rb:344:5:344:5 | :x | :x |
+| calls/calls.rb:344:8:344:9 | 42 | 42 |
+| calls/calls.rb:345:5:345:5 | :x | :x |
+| calls/calls.rb:345:9:345:13 | :novar | :novar |
+| calls/calls.rb:346:5:346:5 | :X | :X |
+| calls/calls.rb:346:8:346:9 | 42 | 42 |
+| calls/calls.rb:347:5:347:5 | :X | :X |
 | constants/constants.rb:3:19:3:27 | "const_a" | const_a |
 | constants/constants.rb:6:15:6:23 | "const_b" | const_b |
 | constants/constants.rb:17:12:17:18 | "Hello" | Hello |
@@ -572,6 +579,13 @@
 | literals/literals.rb:162:11:162:16 | <<-BLA | \nsome text\\nand some more\n   |
 | literals/literals.rb:167:9:167:19 | <<~SQUIGGLY | \n   indented stuff\n |
 | literals/literals.rb:180:10:180:19 | <<`SCRIPT` | \n cat file.txt\n |
+| literals/literals.rb:184:5:184:6 | 42 | 42 |
+| literals/literals.rb:185:2:185:2 | :x | :x |
+| literals/literals.rb:185:2:185:2 | x | 42 |
+| literals/literals.rb:185:6:185:6 | :y | :y |
+| literals/literals.rb:185:8:185:8 | 5 | 5 |
+| literals/literals.rb:186:2:186:2 | :y | :y |
+| literals/literals.rb:186:7:186:7 | :Z | :Z |
 | misc/misc.erb:2:15:2:37 | "main_include_admin.js" | main_include_admin.js |
 | misc/misc.rb:1:7:1:11 | "bar" | bar |
 | misc/misc.rb:3:7:3:9 | foo | foo |

ruby/ql/test/library-tests/ast/calls/arguments.expected

@@ -15,6 +15,16 @@ keywordArguments
 | calls.rb:249:15:249:30 | Pair | calls.rb:249:15:249:20 | call to foo | calls.rb:249:25:249:30 | call to bar |
 | calls.rb:278:5:278:13 | Pair | calls.rb:278:5:278:8 | :blah | calls.rb:278:11:278:13 | call to bar |
 | calls.rb:279:5:279:16 | Pair | calls.rb:279:5:279:8 | :blah | calls.rb:279:11:279:16 | call to bar |
+| calls.rb:344:5:344:9 | Pair | calls.rb:344:5:344:5 | :x | calls.rb:344:8:344:9 | 42 |
+| calls.rb:345:5:345:6 | Pair | calls.rb:345:5:345:5 | :x | calls.rb:345:5:345:5 | x |
+| calls.rb:345:9:345:14 | Pair | calls.rb:345:9:345:13 | :novar | calls.rb:345:9:345:13 | call to novar |
+| calls.rb:346:5:346:9 | Pair | calls.rb:346:5:346:5 | :X | calls.rb:346:8:346:9 | 42 |
+| calls.rb:347:5:347:6 | Pair | calls.rb:347:5:347:5 | :X | calls.rb:347:5:347:5 | X |
 keywordArgumentsByKeyword
 | calls.rb:278:1:278:14 | call to foo | blah | calls.rb:278:11:278:13 | call to bar |
 | calls.rb:279:1:279:17 | call to foo | blah | calls.rb:279:11:279:16 | call to bar |
+| calls.rb:344:1:344:10 | call to foo | x | calls.rb:344:8:344:9 | 42 |
+| calls.rb:345:1:345:15 | call to foo | novar | calls.rb:345:9:345:13 | call to novar |
+| calls.rb:345:1:345:15 | call to foo | x | calls.rb:345:5:345:5 | x |
+| calls.rb:346:1:346:10 | call to foo | X | calls.rb:346:8:346:9 | 42 |
+| calls.rb:347:1:347:7 | call to foo | X | calls.rb:347:5:347:5 | X |

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

@@ -3,6 +3,7 @@ callsWithNoReceiverArgumentsOrBlock
 | calls.rb:286:5:286:9 | call to super | super |
 | calls.rb:287:5:287:11 | call to super | super |
 | calls.rb:305:5:305:9 | call to super | super |
+| calls.rb:345:9:345:13 | call to novar | novar |
 callsWithArguments
 | calls.rb:14:1:14:11 | call to foo | foo | 0 | calls.rb:14:5:14:5 | 0 |
 | calls.rb:14:1:14:11 | call to foo | foo | 1 | calls.rb:14:8:14:8 | 1 |
@@ -107,6 +108,11 @@ callsWithArguments
 | calls.rb:341:3:341:13 | call to foo | foo | 0 | calls.rb:341:7:341:7 | x |
 | calls.rb:341:3:341:13 | call to foo | foo | 1 | calls.rb:341:10:341:10 | y |
 | calls.rb:341:3:341:13 | call to foo | foo | 2 | calls.rb:341:13:341:13 | z |
+| calls.rb:344:1:344:10 | call to foo | foo | 0 | calls.rb:344:5:344:9 | Pair |
+| calls.rb:345:1:345:15 | call to foo | foo | 0 | calls.rb:345:5:345:6 | Pair |
+| calls.rb:345:1:345:15 | call to foo | foo | 1 | calls.rb:345:9:345:14 | Pair |
+| calls.rb:346:1:346:10 | call to foo | foo | 0 | calls.rb:346:5:346:9 | Pair |
+| calls.rb:347:1:347:7 | call to foo | foo | 0 | calls.rb:347:5:347:6 | Pair |
 callsWithReceiver
 | calls.rb:2:1:2:5 | call to foo | calls.rb:2:1:2:5 | self |
 | calls.rb:5:1:5:10 | call to bar | calls.rb:5:1:5:3 | Foo |
@@ -358,6 +364,10 @@ callsWithReceiver
 | calls.rb:340:17:340:23 | call to [] | calls.rb:340:17:340:23 | Array |
 | calls.rb:340:26:340:32 | call to [] | calls.rb:340:26:340:32 | Array |
 | calls.rb:341:3:341:13 | call to foo | calls.rb:341:3:341:13 | self |
+| calls.rb:344:1:344:10 | call to foo | calls.rb:344:1:344:10 | self |
+| calls.rb:345:1:345:15 | call to foo | calls.rb:345:1:345:15 | self |
+| calls.rb:346:1:346:10 | call to foo | calls.rb:346:1:346:10 | self |
+| calls.rb:347:1:347:7 | call to foo | calls.rb:347:1:347:7 | self |
 callsWithBlock
 | calls.rb:17:1:17:17 | call to foo | calls.rb:17:5:17:17 | { ... } |
 | calls.rb:20:1:22:3 | call to foo | calls.rb:20:5:22:3 | do ... end |

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

@@ -340,3 +340,8 @@ def foo(a, b, ...)
 for x, y, z in [[1,2,3], [4,5,6]]
   foo x, y, z
 end
+
+foo(x: 42)
+foo(x:, novar:)
+foo(X: 42)
+foo(X:)

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

@@ -217,6 +217,14 @@ allLiterals
 | literals.rb:171:9:171:15 | <<"DOC" | HereDoc | <none> |
 | literals.rb:176:9:176:15 | <<'DOC' | HereDoc | <none> |
 | literals.rb:180:10:180:19 | <<`SCRIPT` | HereDoc | \n cat file.txt\n |
+| literals.rb:184:5:184:6 | 42 | IntegerLiteral | 42 |
+| literals.rb:185:1:185:9 | {...} | HashLiteral | <none> |
+| literals.rb:185:2:185:2 | :x | SymbolLiteral | :x |
+| literals.rb:185:6:185:6 | :y | SymbolLiteral | :y |
+| literals.rb:185:8:185:8 | 5 | IntegerLiteral | 5 |
+| literals.rb:186:1:186:9 | {...} | HashLiteral | <none> |
+| literals.rb:186:2:186:2 | :y | SymbolLiteral | :y |
+| literals.rb:186:7:186:7 | :Z | SymbolLiteral | :Z |
 stringlikeLiterals
 | literals.rb:46:1:46:2 | "" |  |
 | literals.rb:47:1:47:2 | "" |  |
@@ -327,6 +335,10 @@ stringlikeLiterals
 | literals.rb:171:9:171:15 | <<"DOC" | <none> |
 | literals.rb:176:9:176:15 | <<'DOC' | <none> |
 | literals.rb:180:10:180:19 | <<`SCRIPT` | \n cat file.txt\n |
+| literals.rb:185:2:185:2 | :x | :x |
+| literals.rb:185:6:185:6 | :y | :y |
+| literals.rb:186:2:186:2 | :y | :y |
+| literals.rb:186:7:186:7 | :Z | :Z |
 stringLiterals
 | literals.rb:46:1:46:2 | "" |  |
 | literals.rb:47:1:47:2 | "" |  |
@@ -435,6 +447,10 @@ symbolLiterals
 | literals.rb:117:3:117:5 | :foo | :foo |
 | literals.rb:117:11:117:14 | :bar | :bar |
 | literals.rb:118:3:118:5 | :foo | :foo |
+| literals.rb:185:2:185:2 | :x | :x |
+| literals.rb:185:6:185:6 | :y | :y |
+| literals.rb:186:2:186:2 | :y | :y |
+| literals.rb:186:7:186:7 | :Z | :Z |
 subshellLiterals
 | literals.rb:130:1:130:7 | `ls -l` | ls -l |
 | literals.rb:131:1:131:9 | `ls -l` | ls -l |
@@ -701,19 +717,29 @@ hashLiterals
 | literals.rb:116:1:116:2 | {...} | 0 |
 | literals.rb:117:1:117:33 | {...} | 3 |
 | literals.rb:118:1:118:17 | {...} | 2 |
+| literals.rb:185:1:185:9 | {...} | 2 |
+| literals.rb:186:1:186:9 | {...} | 2 |
 hashLiteralElements
 | literals.rb:88:1:88:14 | {...} | 0 | literals.rb:88:3:88:12 | Pair | Pair |
 | literals.rb:117:1:117:33 | {...} | 0 | literals.rb:117:3:117:8 | Pair | Pair |
 | literals.rb:117:1:117:33 | {...} | 1 | literals.rb:117:11:117:19 | Pair | Pair |
 | literals.rb:117:1:117:33 | {...} | 2 | literals.rb:117:22:117:31 | Pair | Pair |
 | literals.rb:118:1:118:17 | {...} | 0 | literals.rb:118:3:118:8 | Pair | Pair |
 | literals.rb:118:1:118:17 | {...} | 1 | literals.rb:118:11:118:15 | ** ... | HashSplatExpr |
+| literals.rb:185:1:185:9 | {...} | 0 | literals.rb:185:2:185:3 | Pair | Pair |
+| literals.rb:185:1:185:9 | {...} | 1 | literals.rb:185:6:185:8 | Pair | Pair |
+| literals.rb:186:1:186:9 | {...} | 0 | literals.rb:186:2:186:3 | Pair | Pair |
+| literals.rb:186:1:186:9 | {...} | 1 | literals.rb:186:7:186:8 | Pair | Pair |
 hashLiteralKeyValuePairs
 | literals.rb:88:1:88:14 | {...} | literals.rb:88:3:88:12 | Pair | literals.rb:88:3:88:5 | :foo | literals.rb:88:8:88:12 | "bar" |
 | literals.rb:117:1:117:33 | {...} | literals.rb:117:3:117:8 | Pair | literals.rb:117:3:117:5 | :foo | literals.rb:117:8:117:8 | 1 |
 | literals.rb:117:1:117:33 | {...} | literals.rb:117:11:117:19 | Pair | literals.rb:117:11:117:14 | :bar | literals.rb:117:19:117:19 | 2 |
 | literals.rb:117:1:117:33 | {...} | literals.rb:117:22:117:31 | Pair | literals.rb:117:22:117:26 | "baz" | literals.rb:117:31:117:31 | 3 |
 | literals.rb:118:1:118:17 | {...} | literals.rb:118:3:118:8 | Pair | literals.rb:118:3:118:5 | :foo | literals.rb:118:8:118:8 | 7 |
+| literals.rb:185:1:185:9 | {...} | literals.rb:185:2:185:3 | Pair | literals.rb:185:2:185:2 | :x | literals.rb:185:2:185:2 | x |
+| literals.rb:185:1:185:9 | {...} | literals.rb:185:6:185:8 | Pair | literals.rb:185:6:185:6 | :y | literals.rb:185:8:185:8 | 5 |
+| literals.rb:186:1:186:9 | {...} | literals.rb:186:2:186:3 | Pair | literals.rb:186:2:186:2 | :y | literals.rb:186:2:186:2 | call to y |
+| literals.rb:186:1:186:9 | {...} | literals.rb:186:7:186:8 | Pair | literals.rb:186:7:186:7 | :Z | literals.rb:186:7:186:7 | Z |
 finiteRangeLiterals
 | literals.rb:121:2:121:6 | _ .. _ | literals.rb:121:2:121:2 | 1 | literals.rb:121:5:121:6 | 10 |
 | literals.rb:122:2:122:7 | _ ... _ | literals.rb:122:2:122:2 | 1 | literals.rb:122:6:122:7 | 10 |
@@ -802,6 +828,8 @@ numericLiterals
 | literals.rb:140:12:140:12 | 1 | IntegerLiteral | 1 |
 | literals.rb:146:10:146:10 | 1 | IntegerLiteral | 1 |
 | literals.rb:146:14:146:14 | 1 | IntegerLiteral | 1 |
+| literals.rb:184:5:184:6 | 42 | IntegerLiteral | 42 |
+| literals.rb:185:8:185:8 | 5 | IntegerLiteral | 5 |
 integerLiterals
 | literals.rb:10:1:10:4 | 1234 | IntegerLiteral | 1234 |
 | literals.rb:11:1:11:5 | 5_678 | IntegerLiteral | 5678 |
@@ -865,6 +893,8 @@ integerLiterals
 | literals.rb:140:12:140:12 | 1 | IntegerLiteral | 1 |
 | literals.rb:146:10:146:10 | 1 | IntegerLiteral | 1 |
 | literals.rb:146:14:146:14 | 1 | IntegerLiteral | 1 |
+| literals.rb:184:5:184:6 | 42 | IntegerLiteral | 42 |
+| literals.rb:185:8:185:8 | 5 | IntegerLiteral | 5 |
 floatLiterals
 | literals.rb:30:1:30:5 | 12.34 | FloatLiteral | 12.34 |
 | literals.rb:31:1:31:7 | 1234e-2 | FloatLiteral | 12.34 |