Merge remote-tracking branch 'origin/main' into nickrolfe/extractor-performance

C++: Upgrade cpp/cleartext-storage-buffer

.github/workflows/ruby-qltest.yml

@@ -32,14 +32,14 @@ jobs:
       - uses: ./ruby/actions/create-extractor-pack
       - name: Run QL tests
         run: |
-          codeql test run --search-path "${{ github.workspace }}/ruby/extractor-pack" --check-databases --check-unused-labels --check-repeated-labels --check-redefined-labels --check-use-before-definition --consistency-queries ql/consistency-queries ql/test
+          codeql test run --threads=0 --ram 5000 --search-path "${{ github.workspace }}/ruby/extractor-pack" --check-databases --check-unused-labels --check-repeated-labels --check-redefined-labels --check-use-before-definition --consistency-queries ql/consistency-queries ql/test
         env:
           GITHUB_TOKEN: ${{ github.token }}
       - name: Check QL formatting
         run: find ql "(" -name "*.ql" -or -name "*.qll" ")" -print0 | xargs -0 codeql query format --check-only
       - name: Check QL compilation
         run: |
-          codeql query compile --check-only --threads=4 --warnings=error "ql/src" "ql/examples"
+          codeql query compile --check-only --threads=0 --ram 5000 --warnings=error "ql/src" "ql/examples"
         env:
           GITHUB_TOKEN: ${{ github.token }}
       - name: Check DB upgrade scripts

cpp/ql/lib/semmle/code/cpp/ir/dataflow/DefaultTaintTracking.qll

@@ -1,3 +1,8 @@
+/**
+ * An IR taint tracking library that uses an IR DataFlow configuration to track
+ * taint from user inputs as defined by `semmle.code.cpp.security.Security`.
+ */
+
 import cpp
 import semmle.code.cpp.security.Security
 private import semmle.code.cpp.ir.dataflow.DataFlow

cpp/ql/lib/semmle/code/cpp/security/TaintTracking.qll

@@ -1,7 +1,10 @@
 /*
- * Support for tracking tainted data through the program.
+ * Support for tracking tainted data through the program. This is an alias for
+ * `semmle.code.cpp.ir.dataflow.DefaultTaintTracking` provided for backwards
+ * compatibility.
  *
- * Prefer to use `semmle.code.cpp.dataflow.TaintTracking` when designing new queries.
+ * Prefer to use `semmle.code.cpp.dataflow.TaintTracking` or
+ * `semmle.code.cpp.ir.dataflow.TaintTracking` when designing new queries.
  */
 
 import semmle.code.cpp.ir.dataflow.DefaultTaintTracking

cpp/ql/src/Security/CWE/CWE-311/CleartextBufferWrite.ql

@@ -12,23 +12,33 @@
  */
 
 import cpp
-import semmle.code.cpp.security.BufferWrite
-import semmle.code.cpp.security.TaintTracking
+import semmle.code.cpp.security.BufferWrite as BufferWrite
 import semmle.code.cpp.security.SensitiveExprs
-import TaintedWithPath
+import semmle.code.cpp.security.FlowSources
+import semmle.code.cpp.ir.dataflow.TaintTracking
+import DataFlow::PathGraph
 
-class Configuration extends TaintTrackingConfiguration {
-  override predicate isSink(Element tainted) { exists(BufferWrite w | w.getASource() = tainted) }
+/**
+ * Taint flow from user input to a buffer write.
+ */
+class ToBufferConfiguration extends TaintTracking::Configuration {
+  ToBufferConfiguration() { this = "ToBufferConfiguration" }
+
+  override predicate isSource(DataFlow::Node source) { source instanceof FlowSource }
+
+  override predicate isSink(DataFlow::Node sink) {
+    exists(BufferWrite::BufferWrite w | w.getASource() = sink.asExpr())
+  }
 }
 
 from
-  BufferWrite w, Expr taintedArg, Expr taintSource, PathNode sourceNode, PathNode sinkNode,
-  string taintCause, SensitiveExpr dest
+  ToBufferConfiguration config, BufferWrite::BufferWrite w, DataFlow::PathNode sourceNode,
+  DataFlow::PathNode sinkNode, FlowSource source, SensitiveExpr dest
 where
-  taintedWithPath(taintSource, taintedArg, sourceNode, sinkNode) and
-  isUserInput(taintSource, taintCause) and
-  w.getASource() = taintedArg and
+  config.hasFlowPath(sourceNode, sinkNode) and
+  sourceNode.getNode() = source and
+  w.getASource() = sinkNode.getNode().asExpr() and
   dest = w.getDest()
 select w, sourceNode, sinkNode,
-  "This write into buffer '" + dest.toString() + "' may contain unencrypted data from $@",
-  taintSource, "user input (" + taintCause + ")"
+  "This write into buffer '" + dest.toString() + "' may contain unencrypted data from $@", source,
+  "user input (" + source.getSourceType() + ")"

cpp/ql/src/change-notes/2022-01-25-cleartext-storage-buffer.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* The `cpp/cleartext-storage-buffer` query has been updated to use the `semmle.code.cpp.dataflow.TaintTracking` library.

cpp/ql/test/query-tests/Security/CWE/CWE-311/semmle/tests/CleartextBufferWrite.expected

@@ -1,18 +1,8 @@
 edges
 | test.cpp:54:17:54:20 | argv | test.cpp:58:25:58:29 | input |
-| test.cpp:54:17:54:20 | argv | test.cpp:58:25:58:29 | input |
-| test.cpp:54:17:54:20 | argv | test.cpp:58:25:58:29 | input |
-| test.cpp:54:17:54:20 | argv | test.cpp:58:25:58:29 | input |
-| test.cpp:54:17:54:20 | argv | test.cpp:58:25:58:29 | input indirection |
-| test.cpp:54:17:54:20 | argv | test.cpp:58:25:58:29 | input indirection |
-subpaths
 nodes
 | test.cpp:54:17:54:20 | argv | semmle.label | argv |
-| test.cpp:54:17:54:20 | argv | semmle.label | argv |
 | test.cpp:58:25:58:29 | input | semmle.label | input |
-| test.cpp:58:25:58:29 | input | semmle.label | input |
-| test.cpp:58:25:58:29 | input | semmle.label | input |
-| test.cpp:58:25:58:29 | input indirection | semmle.label | input indirection |
-| test.cpp:58:25:58:29 | input indirection | semmle.label | input indirection |
+subpaths
 #select
-| test.cpp:58:3:58:9 | call to sprintf | test.cpp:54:17:54:20 | argv | test.cpp:58:25:58:29 | input | This write into buffer 'passwd' may contain unencrypted data from $@ | test.cpp:54:17:54:20 | argv | user input (argv) |
+| test.cpp:58:3:58:9 | call to sprintf | test.cpp:54:17:54:20 | argv | test.cpp:58:25:58:29 | input | This write into buffer 'passwd' may contain unencrypted data from $@ | test.cpp:54:17:54:20 | argv | user input (a command-line argument) |

ruby/extractor/Cargo.toml

@@ -18,3 +18,4 @@ tracing-subscriber = { version = "0.3.3", features = ["env-filter"] }
 rayon = "1.5.0"
 num_cpus = "1.13.0"
 regex = "1.4.3"
+indexmap = "1.7.0"

ruby/extractor/src/extractor.rs

@@ -1,161 +1,111 @@
+use crate::trap;
+use indexmap::IndexMap;
 use node_types::{EntryKind, Field, NodeTypeMap, Storage, TypeName};
-use std::borrow::Cow;
-use std::collections::BTreeMap as Map;
-use std::collections::BTreeSet as Set;
 use std::fmt;
-use std::io::Write;
 use std::path::Path;
 
 use tracing::{error, info, span, Level};
 use tree_sitter::{Language, Node, Parser, Range, Tree};
 
-pub struct TrapWriter {
-    /// The accumulated trap entries
-    trap_output: Vec<TrapEntry>,
-    /// A counter for generating fresh labels
-    counter: u32,
-    /// cache of global keys
-    global_keys: std::collections::HashMap<String, Label>,
+pub fn populate_file(writer: &mut trap::Writer, absolute_path: &Path) -> trap::Label {
+    let (file_label, fresh) =
+        writer.global_id(trap::full_id_for_file(&normalize_path(absolute_path)));
+    if fresh {
+        writer.add_tuple(
+            "files",
+            vec![
+                trap::Arg::Label(file_label),
+                trap::Arg::String(normalize_path(absolute_path)),
+            ],
+        );
+        populate_parent_folders(writer, file_label, absolute_path.parent());
+    }
+    file_label
 }
 
-pub fn new_trap_writer() -> TrapWriter {
-    TrapWriter {
-        counter: 0,
-        trap_output: Vec::new(),
-        global_keys: std::collections::HashMap::new(),
+fn populate_empty_file(writer: &mut trap::Writer) -> trap::Label {
+    let (file_label, fresh) = writer.global_id("empty;sourcefile".to_owned());
+    if fresh {
+        writer.add_tuple(
+            "files",
+            vec![
+                trap::Arg::Label(file_label),
+                trap::Arg::String("".to_string()),
+            ],
+        );
     }
+    file_label
 }
 
-impl TrapWriter {
-    ///  Gets a label that will hold the unique ID of the passed string at import time.
-    ///  This can be used for incrementally importable TRAP files -- use globally unique
-    ///  strings to compute a unique ID for table tuples.
-    ///
-    ///  Note: You probably want to make sure that the key strings that you use are disjoint
-    ///  for disjoint column types; the standard way of doing this is to prefix (or append)
-    ///  the column type name to the ID. Thus, you might identify methods in Java by the
-    ///  full ID "methods_com.method.package.DeclaringClass.method(argumentList)".
+pub fn populate_empty_location(writer: &mut trap::Writer) {
+    let file_label = populate_empty_file(writer);
+    location(writer, file_label, 0, 0, 0, 0);
+}
 
-    fn fresh_id(&mut self) -> Label {
-        let label = Label(self.counter);
-        self.counter += 1;
-        self.trap_output.push(TrapEntry::FreshId(label));
-        label
-    }
-
-    fn global_id(&mut self, key: &str) -> (Label, bool) {
-        if let Some(label) = self.global_keys.get(key) {
-            return (*label, false);
-        }
-        let label = Label(self.counter);
-        self.counter += 1;
-        self.global_keys.insert(key.to_owned(), label);
-        self.trap_output
-            .push(TrapEntry::MapLabelToKey(label, key.to_owned()));
-        (label, true)
-    }
-
-    fn add_tuple(&mut self, table_name: &str, args: Vec<Arg>) {
-        self.trap_output
-            .push(TrapEntry::GenericTuple(table_name.to_owned(), args))
-    }
-
-    fn populate_file(&mut self, absolute_path: &Path) -> Label {
-        let (file_label, fresh) = self.global_id(&full_id_for_file(absolute_path));
-        if fresh {
-            self.add_tuple(
-                "files",
-                vec![
-                    Arg::Label(file_label),
-                    Arg::String(normalize_path(absolute_path)),
-                ],
-            );
-            self.populate_parent_folders(file_label, absolute_path.parent());
-        }
-        file_label
-    }
-
-    fn populate_empty_file(&mut self) -> Label {
-        let (file_label, fresh) = self.global_id("empty;sourcefile");
-        if fresh {
-            self.add_tuple(
-                "files",
-                vec![Arg::Label(file_label), Arg::String("".to_string())],
-            );
-        }
-        file_label
-    }
-
-    pub fn populate_empty_location(&mut self) {
-        let file_label = self.populate_empty_file();
-        self.location(file_label, 0, 0, 0, 0);
-    }
-
-    fn populate_parent_folders(&mut self, child_label: Label, path: Option<&Path>) {
-        let mut path = path;
-        let mut child_label = child_label;
-        loop {
-            match path {
-                None => break,
-                Some(folder) => {
-                    let (folder_label, fresh) = self.global_id(&full_id_for_folder(folder));
-                    self.add_tuple(
-                        "containerparent",
-                        vec![Arg::Label(folder_label), Arg::Label(child_label)],
+pub fn populate_parent_folders(
+    writer: &mut trap::Writer,
+    child_label: trap::Label,
+    path: Option<&Path>,
+) {
+    let mut path = path;
+    let mut child_label = child_label;
+    loop {
+        match path {
+            None => break,
+            Some(folder) => {
+                let (folder_label, fresh) =
+                    writer.global_id(trap::full_id_for_folder(&normalize_path(folder)));
+                writer.add_tuple(
+                    "containerparent",
+                    vec![
+                        trap::Arg::Label(folder_label),
+                        trap::Arg::Label(child_label),
+                    ],
+                );
+                if fresh {
+                    writer.add_tuple(
+                        "folders",
+                        vec![
+                            trap::Arg::Label(folder_label),
+                            trap::Arg::String(normalize_path(folder)),
+                        ],
                     );
-                    if fresh {
-                        self.add_tuple(
-                            "folders",
-                            vec![
-                                Arg::Label(folder_label),
-                                Arg::String(normalize_path(folder)),
-                            ],
-                        );
-                        path = folder.parent();
-                        child_label = folder_label;
-                    } else {
-                        break;
-                    }
+                    path = folder.parent();
+                    child_label = folder_label;
+                } else {
+                    break;
                 }
             }
         }
     }
+}
 
-    fn location(
-        &mut self,
-        file_label: Label,
-        start_line: usize,
-        start_column: usize,
-        end_line: usize,
-        end_column: usize,
-    ) -> Label {
-        let (loc_label, fresh) = self.global_id(&format!(
-            "loc,{{{}}},{},{},{},{}",
-            file_label, start_line, start_column, end_line, end_column
-        ));
-        if fresh {
-            self.add_tuple(
-                "locations_default",
-                vec![
-                    Arg::Label(loc_label),
-                    Arg::Label(file_label),
-                    Arg::Int(start_line),
-                    Arg::Int(start_column),
-                    Arg::Int(end_line),
-                    Arg::Int(end_column),
-                ],
-            );
-        }
-        loc_label
-    }
-
-    fn comment(&mut self, text: String) {
-        self.trap_output.push(TrapEntry::Comment(text));
-    }
-
-    pub fn output(self, writer: &mut dyn Write) -> std::io::Result<()> {
-        write!(writer, "{}", Program(self.trap_output))
+fn location(
+    writer: &mut trap::Writer,
+    file_label: trap::Label,
+    start_line: usize,
+    start_column: usize,
+    end_line: usize,
+    end_column: usize,
+) -> trap::Label {
+    let (loc_label, fresh) = writer.global_id(format!(
+        "loc,{{{}}},{},{},{},{}",
+        file_label, start_line, start_column, end_line, end_column
+    ));
+    if fresh {
+        writer.add_tuple(
+            "locations_default",
+            vec![
+                trap::Arg::Label(loc_label),
+                trap::Arg::Label(file_label),
+                trap::Arg::Int(start_line),
+                trap::Arg::Int(start_column),
+                trap::Arg::Int(end_line),
+                trap::Arg::Int(end_column),
+            ],
+        );
     }
+    loc_label
 }
 
 /// Extracts the source file at `path`, which is assumed to be canonicalized.
@@ -163,71 +113,42 @@ pub fn extract(
     language: Language,
     language_prefix: &str,
     schema: &NodeTypeMap,
-    trap_writer: &mut TrapWriter,
+    trap_writer: &mut trap::Writer,
     path: &Path,
     source: &[u8],
     ranges: &[Range],
 ) -> std::io::Result<()> {
+    let path_str = format!("{}", path.display());
     let span = span!(
         Level::TRACE,
         "extract",
-        file = %path.display()
+        file = %path_str
     );
 
     let _enter = span.enter();
 
-    info!("extracting: {}", path.display());
+    info!("extracting: {}", path_str);
 
     let mut parser = Parser::new();
     parser.set_language(language).unwrap();
     parser.set_included_ranges(ranges).unwrap();
     let tree = parser.parse(&source, None).expect("Failed to parse file");
-    trap_writer.comment(format!("Auto-generated TRAP file for {}", path.display()));
-    let file_label = &trap_writer.populate_file(path);
-    let mut visitor = Visitor {
+    let file_label = populate_file(trap_writer, path);
+    let mut visitor = Visitor::new(
         source,
         trap_writer,
         // TODO: should we handle path strings that are not valid UTF8 better?
-        path: format!("{}", path.display()),
-        file_label: *file_label,
-        toplevel_child_counter: 0,
-        stack: Vec::new(),
+        &path_str,
+        file_label,
         language_prefix,
         schema,
-    };
+    );
     traverse(&tree, &mut visitor);
 
     parser.reset();
     Ok(())
 }
 
-/// Escapes a string for use in a TRAP key, by replacing special characters with
-/// HTML entities.
-fn escape_key<'a, S: Into<Cow<'a, str>>>(key: S) -> Cow<'a, str> {
-    fn needs_escaping(c: char) -> bool {
-        matches!(c, '&' | '{' | '}' | '"' | '@' | '#')
-    }
-
-    let key = key.into();
-    if key.contains(needs_escaping) {
-        let mut escaped = String::with_capacity(2 * key.len());
-        for c in key.chars() {
-            match c {
-                '&' => escaped.push_str("&amp;"),
-                '{' => escaped.push_str("&lbrace;"),
-                '}' => escaped.push_str("&rbrace;"),
-                '"' => escaped.push_str("&quot;"),
-                '@' => escaped.push_str("&commat;"),
-                '#' => escaped.push_str("&num;"),
-                _ => escaped.push(c),
-            }
-        }
-        Cow::Owned(escaped)
-    } else {
-        key
-    }
-}
-
 /// Normalizes the path according the common CodeQL specification. Assumes that
 /// `path` has already been canonicalized using `std::fs::canonicalize`.
 fn normalize_path(path: &Path) -> String {
@@ -267,34 +188,28 @@ fn normalize_path(path: &Path) -> String {
     }
 }
 
-fn full_id_for_file(path: &Path) -> String {
-    format!("{};sourcefile", escape_key(&normalize_path(path)))
-}
-
-fn full_id_for_folder(path: &Path) -> String {
-    format!("{};folder", escape_key(&normalize_path(path)))
-}
-
 struct ChildNode {
     field_name: Option<&'static str>,
-    label: Label,
+    label: trap::Label,
     type_name: TypeName,
 }
 
 struct Visitor<'a> {
     /// The file path of the source code (as string)
-    path: String,
+    path: &'a str,
     /// The label to use whenever we need to refer to the `@file` entity of this
     /// source file.
-    file_label: Label,
+    file_label: trap::Label,
     /// The source code as a UTF-8 byte array
     source: &'a [u8],
-    /// A TrapWriter to accumulate trap entries
-    trap_writer: &'a mut TrapWriter,
+    /// A trap::Writer to accumulate trap entries
+    trap_writer: &'a mut trap::Writer,
     /// A counter for top-level child nodes
     toplevel_child_counter: usize,
-    /// Language prefix
-    language_prefix: &'a str,
+    /// Language-specific name of the AST parent table
+    ast_node_parent_table_name: String,
+    /// Language-specific name of the tokeninfo table
+    tokeninfo_table_name: String,
     /// A lookup table from type name to node types
     schema: &'a NodeTypeMap,
     /// A stack for gathering information from child nodes. Whenever a node is
@@ -303,39 +218,62 @@ struct Visitor<'a> {
     /// node the list containing the child data is popped from the stack and
     /// matched against the dbscheme for the node. If the expectations are met
     /// the corresponding row definitions are added to the trap_output.
-    stack: Vec<(Label, usize, Vec<ChildNode>)>,
+    stack: Vec<(trap::Label, usize, Vec<ChildNode>)>,
 }
 
-impl Visitor<'_> {
+impl<'a> Visitor<'a> {
+    fn new(
+        source: &'a [u8],
+        trap_writer: &'a mut trap::Writer,
+        path: &'a str,
+        file_label: trap::Label,
+        language_prefix: &str,
+        schema: &'a NodeTypeMap,
+    ) -> Visitor<'a> {
+        Visitor {
+            path,
+            file_label,
+            source,
+            trap_writer,
+            toplevel_child_counter: 0,
+            ast_node_parent_table_name: format!("{}_ast_node_parent", language_prefix),
+            tokeninfo_table_name: format!("{}_tokeninfo", language_prefix),
+            schema,
+            stack: Vec::new(),
+        }
+    }
+
     fn record_parse_error(
         &mut self,
         error_message: String,
         full_error_message: String,
-        loc: Label,
+        loc: trap::Label,
     ) {
         error!("{}", full_error_message);
         let id = self.trap_writer.fresh_id();
         self.trap_writer.add_tuple(
             "diagnostics",
             vec![
-                Arg::Label(id),
-                Arg::Int(40), // severity 40 = error
-                Arg::String("parse_error".to_string()),
-                Arg::String(error_message),
-                Arg::String(full_error_message),
-                Arg::Label(loc),
+                trap::Arg::Label(id),
+                trap::Arg::Int(40), // severity 40 = error
+                trap::Arg::String("parse_error".to_string()),
+                trap::Arg::String(error_message),
+                trap::Arg::String(full_error_message),
+                trap::Arg::Label(loc),
             ],
         );
     }
 
-    fn record_parse_error_for_node(
-        &mut self,
-        error_message: String,
-        full_error_message: String,
-        node: Node,
-    ) {
+    fn record_parse_error_for_node(&mut self, error_message: String, node: &Node) {
+        let full_error_message = format!(
+            "{}:{}: {}",
+            &self.path,
+            node.start_position().row + 1,
+            &error_message
+        );
         let (start_line, start_column, end_line, end_column) = location_for(self.source, node);
-        let loc = self.trap_writer.location(
+        let loc = location(
+            self.trap_writer,
             self.file_label,
             start_line,
             start_column,
@@ -345,20 +283,14 @@ impl Visitor<'_> {
         self.record_parse_error(error_message, full_error_message, loc);
     }
 
-    fn enter_node(&mut self, node: Node) -> bool {
+    fn enter_node(&mut self, node: &Node) -> bool {
         if node.is_error() || node.is_missing() {
             let error_message = if node.is_missing() {
                 format!("parse error: expecting '{}'", node.kind())
             } else {
                 "parse error".to_string()
             };
-            let full_error_message = format!(
-                "{}:{}: {}",
-                &self.path,
-                node.start_position().row + 1,
-                error_message
-            );
-            self.record_parse_error_for_node(error_message, full_error_message, node);
+            self.record_parse_error_for_node(error_message, node);
             return false;
         }
 
@@ -368,13 +300,14 @@ impl Visitor<'_> {
         true
     }
 
-    fn leave_node(&mut self, field_name: Option<&'static str>, node: Node) {
+    fn leave_node(&mut self, field_name: Option<&'static str>, node: &Node) {
         if node.is_error() || node.is_missing() {
             return;
         }
         let (id, _, child_nodes) = self.stack.pop().expect("Vistor: empty stack");
         let (start_line, start_column, end_line, end_column) = location_for(self.source, node);
-        let loc = self.trap_writer.location(
+        let loc = location(
+            self.trap_writer,
             self.file_label,
             start_line,
             start_column,
@@ -402,20 +335,20 @@ impl Visitor<'_> {
         match &table.kind {
             EntryKind::Token { kind_id, .. } => {
                 self.trap_writer.add_tuple(
-                    &format!("{}_ast_node_parent", self.language_prefix),
+                    &self.ast_node_parent_table_name,
                     vec![
-                        Arg::Label(id),
-                        Arg::Label(parent_id),
-                        Arg::Int(parent_index),
+                        trap::Arg::Label(id),
+                        trap::Arg::Label(parent_id),
+                        trap::Arg::Int(parent_index),
                     ],
                 );
                 self.trap_writer.add_tuple(
-                    &format!("{}_tokeninfo", self.language_prefix),
+                    &self.tokeninfo_table_name,
                     vec![
-                        Arg::Label(id),
-                        Arg::Int(*kind_id),
+                        trap::Arg::Label(id),
+                        trap::Arg::Int(*kind_id),
                         sliced_source_arg(self.source, node),
-                        Arg::Label(loc),
+                        trap::Arg::Label(loc),
                     ],
                 );
             }
@@ -423,18 +356,18 @@ impl Visitor<'_> {
                 fields,
                 name: table_name,
             } => {
-                if let Some(args) = self.complex_node(&node, fields, &child_nodes, id) {
+                if let Some(args) = self.complex_node(node, fields, &child_nodes, id) {
                     self.trap_writer.add_tuple(
-                        &format!("{}_ast_node_parent", self.language_prefix),
+                        &self.ast_node_parent_table_name,
                         vec![
-                            Arg::Label(id),
-                            Arg::Label(parent_id),
-                            Arg::Int(parent_index),
+                            trap::Arg::Label(id),
+                            trap::Arg::Label(parent_id),
+                            trap::Arg::Int(parent_index),
                         ],
                     );
-                    let mut all_args = vec![Arg::Label(id)];
+                    let mut all_args = vec![trap::Arg::Label(id)];
                     all_args.extend(args);
-                    all_args.push(Arg::Label(loc));
+                    all_args.push(trap::Arg::Label(loc));
                     self.trap_writer.add_tuple(table_name, all_args);
                 }
             }
@@ -472,9 +405,9 @@ impl Visitor<'_> {
         node: &Node,
         fields: &[Field],
         child_nodes: &[ChildNode],
-        parent_id: Label,
-    ) -> Option<Vec<Arg>> {
-        let mut map: Map<&Option<String>, (&Field, Vec<Arg>)> = Map::new();
+        parent_id: trap::Label,
+    ) -> Option<Vec<trap::Arg>> {
+        let mut map: IndexMap<&Option<String>, (&Field, Vec<trap::Arg>)> = IndexMap::new();
         for field in fields {
             map.insert(&field.name, (field, Vec::new()));
         }
@@ -482,46 +415,47 @@ impl Visitor<'_> {
             if let Some((field, values)) = map.get_mut(&child_node.field_name.map(|x| x.to_owned()))
             {
                 //TODO: handle error and missing nodes
-                if self.type_matches(&child_node.type_name, &field.type_info) {
-                    if let node_types::FieldTypeInfo::ReservedWordInt(int_mapping) =
-                        &field.type_info
+                if field.type_info.valid_types.contains(&child_node.type_name) {
+                    if let node_types::FieldTypeKind::ReservedWordInt(int_mapping) =
+                        &field.type_info.kind
                     {
-                        // We can safely unwrap because type_matches checks the key is in the map.
-                        let (int_value, _) = int_mapping.get(&child_node.type_name.kind).unwrap();
-                        values.push(Arg::Int(*int_value));
+                        match int_mapping.get(&child_node.type_name.kind) {
+                            Some((int_value, _)) => values.push(trap::Arg::Int(*int_value)),
+                            None => self.record_parse_error_for_node(
+                                format!(
+                                    "could not map field {}::{} with type {:?} to an integer value",
+                                    node.kind(),
+                                    child_node.field_name.unwrap_or("child"),
+                                    child_node.type_name
+                                ),
+                                node,
+                            ),
+                        };
                     } else {
-                        values.push(Arg::Label(child_node.label));
+                        values.push(trap::Arg::Label(child_node.label));
                     }
                 } else if field.name.is_some() {
-                    let error_message = format!(
-                        "type mismatch for field {}::{} with type {:?} != {:?}",
-                        node.kind(),
-                        child_node.field_name.unwrap_or("child"),
-                        child_node.type_name,
-                        field.type_info
+                    self.record_parse_error_for_node(
+                        format!(
+                            "type mismatch for field {}::{} with type {:?} != {:?}",
+                            node.kind(),
+                            child_node.field_name.unwrap_or("child"),
+                            child_node.type_name,
+                            field.type_info
+                        ),
+                        node,
                     );
-                    let full_error_message = format!(
-                        "{}:{}: {}",
-                        &self.path,
-                        node.start_position().row + 1,
-                        error_message
-                    );
-                    self.record_parse_error_for_node(error_message, full_error_message, *node);
                 }
             } else if child_node.field_name.is_some() || child_node.type_name.named {
-                let error_message = format!(
-                    "value for unknown field: {}::{} and type {:?}",
-                    node.kind(),
-                    &child_node.field_name.unwrap_or("child"),
-                    &child_node.type_name
+                self.record_parse_error_for_node(
+                    format!(
+                        "value for unknown field: {}::{} and type {:?}",
+                        node.kind(),
+                        &child_node.field_name.unwrap_or("child"),
+                        &child_node.type_name
+                    ),
+                    node,
                 );
-                let full_error_message = format!(
-                    "{}:{}: {}",
-                    &self.path,
-                    node.start_position().row + 1,
-                    error_message
-                );
-                self.record_parse_error_for_node(error_message, full_error_message, *node);
             }
         }
         let mut args = Vec::new();
@@ -534,23 +468,19 @@ impl Visitor<'_> {
                         args.push(child_values.first().unwrap().clone());
                     } else {
                         is_valid = false;
-                        let error_message = format!(
-                            "{} for field: {}::{}",
-                            if child_values.is_empty() {
-                                "missing value"
-                            } else {
-                                "too many values"
-                            },
-                            node.kind(),
-                            column_name
+                        self.record_parse_error_for_node(
+                            format!(
+                                "{} for field: {}::{}",
+                                if child_values.is_empty() {
+                                    "missing value"
+                                } else {
+                                    "too many values"
+                                },
+                                node.kind(),
+                                column_name
+                            ),
+                            node,
                         );
-                        let full_error_message = format!(
-                            "{}:{}: {}",
-                            &self.path,
-                            node.start_position().row + 1,
-                            error_message
-                        );
-                        self.record_parse_error_for_node(error_message, full_error_message, *node);
                     }
                 }
                 Storage::Table {
@@ -569,9 +499,9 @@ impl Visitor<'_> {
                             );
                             break;
                         }
-                        let mut args = vec![Arg::Label(parent_id)];
+                        let mut args = vec![trap::Arg::Label(parent_id)];
                         if *has_index {
-                            args.push(Arg::Int(index))
+                            args.push(trap::Arg::Int(index))
                         }
                         args.push(child_value.clone());
                         self.trap_writer.add_tuple(table_name, args);
@@ -585,55 +515,18 @@ impl Visitor<'_> {
             None
         }
     }
-
-    fn type_matches(&self, tp: &TypeName, type_info: &node_types::FieldTypeInfo) -> bool {
-        match type_info {
-            node_types::FieldTypeInfo::Single(single_type) => {
-                if tp == single_type {
-                    return true;
-                }
-                if let EntryKind::Union { members } = &self.schema.get(single_type).unwrap().kind {
-                    if self.type_matches_set(tp, members) {
-                        return true;
-                    }
-                }
-            }
-            node_types::FieldTypeInfo::Multiple { types, .. } => {
-                return self.type_matches_set(tp, types);
-            }
-
-            node_types::FieldTypeInfo::ReservedWordInt(int_mapping) => {
-                return !tp.named && int_mapping.contains_key(&tp.kind)
-            }
-        }
-        false
-    }
-
-    fn type_matches_set(&self, tp: &TypeName, types: &Set<TypeName>) -> bool {
-        if types.contains(tp) {
-            return true;
-        }
-        for other in types.iter() {
-            if let EntryKind::Union { members } = &self.schema.get(other).unwrap().kind {
-                if self.type_matches_set(tp, members) {
-                    return true;
-                }
-            }
-        }
-        false
-    }
 }
 
-// Emit a slice of a source file as an Arg.
-fn sliced_source_arg(source: &[u8], n: Node) -> Arg {
+// Emit a slice of a source file as a trap::Arg.
+fn sliced_source_arg(source: &[u8], n: &Node) -> trap::Arg {
     let range = n.byte_range();
-    Arg::String(String::from_utf8_lossy(&source[range.start..range.end]).into_owned())
+    trap::Arg::String(String::from_utf8_lossy(&source[range.start..range.end]).into_owned())
 }
 
 // Emit a pair of `TrapEntry`s for the provided node, appropriately calibrated.
 // The first is the location and label definition, and the second is the
 // 'Located' entry.
-fn location_for(source: &[u8], n: Node) -> (usize, usize, usize, usize) {
+fn location_for(source: &[u8], n: &Node) -> (usize, usize, usize, usize) {
     // Tree-sitter row, column values are 0-based while CodeQL starts
     // counting at 1. In addition Tree-sitter's row and column for the
     // end position are exclusive while CodeQL's end positions are inclusive.
@@ -680,16 +573,16 @@ fn location_for(source: &[u8], n: Node) -> (usize, usize, usize, usize) {
 
 fn traverse(tree: &Tree, visitor: &mut Visitor) {
     let cursor = &mut tree.walk();
-    visitor.enter_node(cursor.node());
+    visitor.enter_node(&cursor.node());
     let mut recurse = true;
     loop {
         if recurse && cursor.goto_first_child() {
-            recurse = visitor.enter_node(cursor.node());
+            recurse = visitor.enter_node(&cursor.node());
         } else {
-            visitor.leave_node(cursor.field_name(), cursor.node());
+            visitor.leave_node(cursor.field_name(), &cursor.node());
 
             if cursor.goto_next_sibling() {
-                recurse = visitor.enter_node(cursor.node());
+                recurse = visitor.enter_node(&cursor.node());
             } else if cursor.goto_parent() {
                 recurse = false;
             } else {
@@ -699,59 +592,6 @@ fn traverse(tree: &Tree, visitor: &mut Visitor) {
     }
 }
 
-pub struct Program(Vec<TrapEntry>);
-
-impl fmt::Display for Program {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        let mut text = String::new();
-        for trap_entry in &self.0 {
-            text.push_str(&format!("{}\n", trap_entry));
-        }
-        write!(f, "{}", text)
-    }
-}
-
-enum TrapEntry {
-    /// Maps the label to a fresh id, e.g. `#123=*`.
-    FreshId(Label),
-    /// Maps the label to a key, e.g. `#7=@"foo"`.
-    MapLabelToKey(Label, String),
-    /// foo_bar(arg*)
-    GenericTuple(String, Vec<Arg>),
-    Comment(String),
-}
-impl fmt::Display for TrapEntry {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        match self {
-            TrapEntry::FreshId(label) => write!(f, "{}=*", label),
-            TrapEntry::MapLabelToKey(label, key) => {
-                write!(f, "{}=@\"{}\"", label, key.replace("\"", "\"\""))
-            }
-            TrapEntry::GenericTuple(name, args) => {
-                write!(f, "{}(", name)?;
-                for (index, arg) in args.iter().enumerate() {
-                    if index > 0 {
-                        write!(f, ",")?;
-                    }
-                    write!(f, "{}", arg)?;
-                }
-                write!(f, ")")
-            }
-            TrapEntry::Comment(line) => write!(f, "// {}", line),
-        }
-    }
-}
-
-#[derive(Debug, Copy, Clone)]
-// Identifiers of the form #0, #1...
-struct Label(u32);
-
-impl fmt::Display for Label {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        write!(f, "#{:x}", self.0)
-    }
-}
-
 // Numeric indices.
 #[derive(Debug, Copy, Clone)]
 struct Index(usize);
@@ -761,69 +601,3 @@ impl fmt::Display for Index {
         write!(f, "{}", self.0)
     }
 }
-
-// Some untyped argument to a TrapEntry.
-#[derive(Debug, Clone)]
-enum Arg {
-    Label(Label),
-    Int(usize),
-    String(String),
-}
-
-const MAX_STRLEN: usize = 1048576;
-
-impl fmt::Display for Arg {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        match self {
-            Arg::Label(x) => write!(f, "{}", x),
-            Arg::Int(x) => write!(f, "{}", x),
-            Arg::String(x) => write!(
-                f,
-                "\"{}\"",
-                limit_string(x, MAX_STRLEN).replace("\"", "\"\"")
-            ),
-        }
-    }
-}
-
-/// Limit the length (in bytes) of a string. If the string's length in bytes is
-/// less than or equal to the limit then the entire string is returned. Otherwise
-/// the string is sliced at the provided limit. If there is a multi-byte character
-/// at the limit then the returned slice will be slightly shorter than the limit to
-/// avoid splitting that multi-byte character.
-fn limit_string(string: &str, max_size: usize) -> &str {
-    if string.len() <= max_size {
-        return string;
-    }
-    let p = string.as_bytes();
-    let mut index = max_size;
-    // We want to clip the string at [max_size]; however, the character at that position
-    // may span several bytes. We need to find the first byte of the character. In UTF-8
-    // encoded data any byte that matches the bit pattern 10XXXXXX is not a start byte.
-    // Therefore we decrement the index as long as there are bytes matching this pattern.
-    // This ensures we cut the string at the border between one character and another.
-    while index > 0 && (p[index] & 0b11000000) == 0b10000000 {
-        index -= 1;
-    }
-    &string[0..index]
-}
-
-#[test]
-fn limit_string_test() {
-    assert_eq!("hello", limit_string(&"hello world".to_owned(), 5));
-    assert_eq!("hi ☹", limit_string(&"hi ☹☹".to_owned(), 6));
-    assert_eq!("hi ", limit_string(&"hi ☹☹".to_owned(), 5));
-}
-
-#[test]
-fn escape_key_test() {
-    assert_eq!("foo!", escape_key("foo!"));
-    assert_eq!("foo&lbrace;&rbrace;", escape_key("foo{}"));
-    assert_eq!("&lbrace;&rbrace;", escape_key("{}"));
-    assert_eq!("", escape_key(""));
-    assert_eq!("/path/to/foo.rb", escape_key("/path/to/foo.rb"));
-    assert_eq!(
-        "/path/to/foo&amp;&lbrace;&rbrace;&quot;&commat;&num;.rb",
-        escape_key("/path/to/foo&{}\"@#.rb")
-    );
-}

ruby/extractor/src/main.rs

@@ -1,51 +1,15 @@
 mod extractor;
+mod trap;
 
 extern crate num_cpus;
 
 use clap::arg;
-use flate2::write::GzEncoder;
 use rayon::prelude::*;
 use std::fs;
-use std::io::{BufRead, BufWriter};
+use std::io::BufRead;
 use std::path::{Path, PathBuf};
 use tree_sitter::{Language, Parser, Range};
 
-enum TrapCompression {
-    None,
-    Gzip,
-}
-
-impl TrapCompression {
-    fn from_env() -> TrapCompression {
-        match std::env::var("CODEQL_RUBY_TRAP_COMPRESSION") {
-            Ok(method) => match TrapCompression::from_string(&method) {
-                Some(c) => c,
-                None => {
-                    tracing::error!("Unknown compression method '{}'; using gzip.", &method);
-                    TrapCompression::Gzip
-                }
-            },
-            // Default compression method if the env var isn't set:
-            Err(_) => TrapCompression::Gzip,
-        }
-    }
-
-    fn from_string(s: &str) -> Option<TrapCompression> {
-        match s.to_lowercase().as_ref() {
-            "none" => Some(TrapCompression::None),
-            "gzip" => Some(TrapCompression::Gzip),
-            _ => None,
-        }
-    }
-
-    fn extension(&self) -> &str {
-        match self {
-            TrapCompression::None => "trap",
-            TrapCompression::Gzip => "trap.gz",
-        }
-    }
-}
-
 /**
  * Gets the number of threads the extractor should use, by reading the
  * CODEQL_THREADS environment variable and using it as described in the
@@ -118,7 +82,7 @@ fn main() -> std::io::Result<()> {
         .value_of("output-dir")
         .expect("missing --output-dir");
     let trap_dir = PathBuf::from(trap_dir);
-    let trap_compression = TrapCompression::from_env();
+    let trap_compression = trap::Compression::from_env("CODEQL_RUBY_TRAP_COMPRESSION");
 
     let file_list = matches.value_of("file-list").expect("missing --file-list");
     let file_list = fs::File::open(file_list)?;
@@ -141,7 +105,7 @@ fn main() -> std::io::Result<()> {
             let src_archive_file = path_for(&src_archive_dir, &path, "");
             let mut source = std::fs::read(&path)?;
             let code_ranges;
-            let mut trap_writer = extractor::new_trap_writer();
+            let mut trap_writer = trap::Writer::new();
             if path.extension().map_or(false, |x| x == "erb") {
                 tracing::info!("scanning: {}", path.display());
                 extractor::extract(
@@ -186,28 +150,20 @@ fn main() -> std::io::Result<()> {
         .expect("failed to extract files");
 
     let path = PathBuf::from("extras");
-    let mut trap_writer = extractor::new_trap_writer();
-    trap_writer.populate_empty_location();
+    let mut trap_writer = trap::Writer::new();
+    extractor::populate_empty_location(&mut trap_writer);
     write_trap(&trap_dir, path, trap_writer, &trap_compression)
 }
 
 fn write_trap(
     trap_dir: &Path,
     path: PathBuf,
-    trap_writer: extractor::TrapWriter,
-    trap_compression: &TrapCompression,
+    trap_writer: trap::Writer,
+    trap_compression: &trap::Compression,
 ) -> std::io::Result<()> {
     let trap_file = path_for(trap_dir, &path, trap_compression.extension());
     std::fs::create_dir_all(&trap_file.parent().unwrap())?;
-    let trap_file = std::fs::File::create(&trap_file)?;
-    let mut trap_file = BufWriter::new(trap_file);
-    match trap_compression {
-        TrapCompression::None => trap_writer.output(&mut trap_file),
-        TrapCompression::Gzip => {
-            let mut compressed_writer = GzEncoder::new(trap_file, flate2::Compression::fast());
-            trap_writer.output(&mut compressed_writer)
-        }
-    }
+    trap_writer.write_to_file(&trap_file, trap_compression)
 }
 
 fn scan_erb(

ruby/extractor/src/trap.rs

@@ -0,0 +1,247 @@
+use std::borrow::Cow;
+use std::fmt;
+use std::io::BufWriter;
+use std::path::Path;
+
+use flate2::write::GzEncoder;
+use indexmap::IndexMap;
+
+pub struct Writer {
+    /// Labels that should be assigned fresh ids, e.g. `#123=*`.
+    fresh_ids: Vec<Label>,
+
+    /// Labels that should be assigned trap keys, e.g. `#7=@"foo"`.
+    global_keys: IndexMap<String, Label>,
+
+    /// Database rows to emit. Each key is the tuple name, each value is a list.
+    /// Each member of *that* list represents an instance of that tuple,
+    /// containing a list of the arguments/column values.
+    tuples: IndexMap<String, Vec<Vec<Arg>>>,
+
+    /// A counter for generating fresh labels
+    counter: u32,
+}
+
+impl Writer {
+    pub fn new() -> Writer {
+        Writer {
+            fresh_ids: Vec::new(),
+            tuples: IndexMap::new(),
+            global_keys: IndexMap::new(),
+            counter: 0,
+        }
+    }
+
+    ///  Gets a label that will hold the unique ID of the passed string at import time.
+    ///  This can be used for incrementally importable TRAP files -- use globally unique
+    ///  strings to compute a unique ID for table tuples.
+    ///
+    ///  Note: You probably want to make sure that the key strings that you use are disjoint
+    ///  for disjoint column types; the standard way of doing this is to prefix (or append)
+    ///  the column type name to the ID. Thus, you might identify methods in Java by the
+    ///  full ID "methods_com.method.package.DeclaringClass.method(argumentList)".
+    pub fn fresh_id(&mut self) -> Label {
+        let label = Label(self.counter);
+        self.counter += 1;
+        self.fresh_ids.push(label);
+        label
+    }
+
+    pub fn global_id(&mut self, key: String) -> (Label, bool) {
+        if let Some(label) = self.global_keys.get(&key) {
+            return (*label, false);
+        }
+        let label = Label(self.counter);
+        self.counter += 1;
+        self.global_keys.insert(key, label);
+        (label, true)
+    }
+
+    pub fn add_tuple(&mut self, table_name: &str, args: Vec<Arg>) {
+        self.tuples
+            .entry(table_name.to_owned())
+            .or_insert_with(Vec::new)
+            .push(args);
+    }
+
+    fn write<T: std::io::Write>(&self, dest: &mut T) -> std::io::Result<()> {
+        for label in &self.fresh_ids {
+            writeln!(dest, "{}=*", label)?;
+        }
+        for (key, label) in &self.global_keys {
+            writeln!(dest, "{}=@\"{}\"", label, key.replace("\"", "\"\""))?;
+        }
+        for (name, instances) in &self.tuples {
+            for instance in instances {
+                write!(dest, "{}(", name)?;
+                for (index, arg) in instance.iter().enumerate() {
+                    if index > 0 {
+                        write!(dest, ",")?;
+                    }
+                    write!(dest, "{}", arg)?;
+                }
+                writeln!(dest, ")")?;
+            }
+        }
+        Ok(())
+    }
+
+    pub fn write_to_file(&self, path: &Path, compression: &Compression) -> std::io::Result<()> {
+        let trap_file = std::fs::File::create(path)?;
+        let mut trap_file = BufWriter::new(trap_file);
+        match compression {
+            Compression::None => self.write(&mut trap_file),
+            Compression::Gzip => {
+                let mut compressed_writer = GzEncoder::new(trap_file, flate2::Compression::fast());
+                self.write(&mut compressed_writer)
+            }
+        }
+    }
+}
+
+#[derive(Debug, Copy, Clone)]
+// Identifiers of the form #0, #1...
+pub struct Label(u32);
+
+impl fmt::Display for Label {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        write!(f, "#{:x}", self.0)
+    }
+}
+
+// Some untyped argument to a TrapEntry.
+#[derive(Debug, Clone)]
+pub enum Arg {
+    Label(Label),
+    Int(usize),
+    String(String),
+}
+
+const MAX_STRLEN: usize = 1048576;
+
+impl fmt::Display for Arg {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        match self {
+            Arg::Label(x) => write!(f, "{}", x),
+            Arg::Int(x) => write!(f, "{}", x),
+            Arg::String(x) => write!(
+                f,
+                "\"{}\"",
+                limit_string(x, MAX_STRLEN).replace("\"", "\"\"")
+            ),
+        }
+    }
+}
+
+pub fn full_id_for_file(normalized_path: &str) -> String {
+    format!("{};sourcefile", escape_key(normalized_path))
+}
+
+pub fn full_id_for_folder(normalized_path: &str) -> String {
+    format!("{};folder", escape_key(normalized_path))
+}
+
+/// Escapes a string for use in a TRAP key, by replacing special characters with
+/// HTML entities.
+fn escape_key<'a, S: Into<Cow<'a, str>>>(key: S) -> Cow<'a, str> {
+    fn needs_escaping(c: char) -> bool {
+        matches!(c, '&' | '{' | '}' | '"' | '@' | '#')
+    }
+
+    let key = key.into();
+    if key.contains(needs_escaping) {
+        let mut escaped = String::with_capacity(2 * key.len());
+        for c in key.chars() {
+            match c {
+                '&' => escaped.push_str("&amp;"),
+                '{' => escaped.push_str("&lbrace;"),
+                '}' => escaped.push_str("&rbrace;"),
+                '"' => escaped.push_str("&quot;"),
+                '@' => escaped.push_str("&commat;"),
+                '#' => escaped.push_str("&num;"),
+                _ => escaped.push(c),
+            }
+        }
+        Cow::Owned(escaped)
+    } else {
+        key
+    }
+}
+
+/// Limit the length (in bytes) of a string. If the string's length in bytes is
+/// less than or equal to the limit then the entire string is returned. Otherwise
+/// the string is sliced at the provided limit. If there is a multi-byte character
+/// at the limit then the returned slice will be slightly shorter than the limit to
+/// avoid splitting that multi-byte character.
+fn limit_string(string: &str, max_size: usize) -> &str {
+    if string.len() <= max_size {
+        return string;
+    }
+    let p = string.as_bytes();
+    let mut index = max_size;
+    // We want to clip the string at [max_size]; however, the character at that position
+    // may span several bytes. We need to find the first byte of the character. In UTF-8
+    // encoded data any byte that matches the bit pattern 10XXXXXX is not a start byte.
+    // Therefore we decrement the index as long as there are bytes matching this pattern.
+    // This ensures we cut the string at the border between one character and another.
+    while index > 0 && (p[index] & 0b11000000) == 0b10000000 {
+        index -= 1;
+    }
+    &string[0..index]
+}
+
+pub enum Compression {
+    None,
+    Gzip,
+}
+
+impl Compression {
+    pub fn from_env(var_name: &str) -> Compression {
+        match std::env::var(var_name) {
+            Ok(method) => match Compression::from_string(&method) {
+                Some(c) => c,
+                None => {
+                    tracing::error!("Unknown compression method '{}'; using gzip.", &method);
+                    Compression::Gzip
+                }
+            },
+            // Default compression method if the env var isn't set:
+            Err(_) => Compression::Gzip,
+        }
+    }
+
+    pub fn from_string(s: &str) -> Option<Compression> {
+        match s.to_lowercase().as_ref() {
+            "none" => Some(Compression::None),
+            "gzip" => Some(Compression::Gzip),
+            _ => None,
+        }
+    }
+
+    pub fn extension(&self) -> &str {
+        match self {
+            Compression::None => "trap",
+            Compression::Gzip => "trap.gz",
+        }
+    }
+}
+
+#[test]
+fn limit_string_test() {
+    assert_eq!("hello", limit_string(&"hello world".to_owned(), 5));
+    assert_eq!("hi ☹", limit_string(&"hi ☹☹".to_owned(), 6));
+    assert_eq!("hi ", limit_string(&"hi ☹☹".to_owned(), 5));
+}
+
+#[test]
+fn escape_key_test() {
+    assert_eq!("foo!", escape_key("foo!"));
+    assert_eq!("foo&lbrace;&rbrace;", escape_key("foo{}"));
+    assert_eq!("&lbrace;&rbrace;", escape_key("{}"));
+    assert_eq!("", escape_key(""));
+    assert_eq!("/path/to/foo.rb", escape_key("/path/to/foo.rb"));
+    assert_eq!(
+        "/path/to/foo&amp;&lbrace;&rbrace;&quot;&commat;&num;.rb",
+        escape_key("/path/to/foo&{}\"@#.rb")
+    );
+}

ruby/generator/src/main.rs

@@ -20,8 +20,8 @@ fn make_field_type<'a>(
     field: &'a node_types::Field,
     nodes: &'a node_types::NodeTypeMap,
 ) -> (ql::Type<'a>, Option<dbscheme::Entry<'a>>) {
-    match &field.type_info {
-        node_types::FieldTypeInfo::Multiple {
+    match &field.type_info.kind {
+        node_types::FieldTypeKind::Multiple {
             types,
             dbscheme_union,
             ql_class: _,
@@ -40,11 +40,11 @@ fn make_field_type<'a>(
                 })),
             )
         }
-        node_types::FieldTypeInfo::Single(t) => {
+        node_types::FieldTypeKind::Single(t) => {
             let dbscheme_name = &nodes.get(t).unwrap().dbscheme_name;
             (ql::Type::At(dbscheme_name), None)
         }
-        node_types::FieldTypeInfo::ReservedWordInt(int_mapping) => {
+        node_types::FieldTypeKind::ReservedWordInt(int_mapping) => {
             // The field will be an `int` in the db, and we add a case split to
             // create other db types for each integer value.
             let mut branches: Vec<(usize, &'a str)> = Vec::new();

ruby/generator/src/ql_gen.rs

@@ -345,16 +345,16 @@ fn create_field_getters<'a>(
     field: &'a node_types::Field,
     nodes: &'a node_types::NodeTypeMap,
 ) -> (ql::Predicate<'a>, Option<ql::Expression<'a>>) {
-    let return_type = match &field.type_info {
-        node_types::FieldTypeInfo::Single(t) => {
+    let return_type = match &field.type_info.kind {
+        node_types::FieldTypeKind::Single(t) => {
             Some(ql::Type::Normal(&nodes.get(t).unwrap().ql_class_name))
         }
-        node_types::FieldTypeInfo::Multiple {
+        node_types::FieldTypeKind::Multiple {
             types: _,
             dbscheme_union: _,
             ql_class,
         } => Some(ql::Type::Normal(ql_class)),
-        node_types::FieldTypeInfo::ReservedWordInt(_) => Some(ql::Type::String),
+        node_types::FieldTypeKind::ReservedWordInt(_) => Some(ql::Type::String),
     };
     let formal_parameters = match &field.storage {
         node_types::Storage::Column { .. } => vec![],
@@ -372,10 +372,10 @@ fn create_field_getters<'a>(
 
     // For the expression to get a value, what variable name should the result
     // be bound to?
-    let get_value_result_var_name = match &field.type_info {
-        node_types::FieldTypeInfo::ReservedWordInt(_) => "value",
-        node_types::FieldTypeInfo::Single(_) => "result",
-        node_types::FieldTypeInfo::Multiple { .. } => "result",
+    let get_value_result_var_name = match &field.type_info.kind {
+        node_types::FieldTypeKind::ReservedWordInt(_) => "value",
+        node_types::FieldTypeKind::Single(_) => "result",
+        node_types::FieldTypeKind::Multiple { .. } => "result",
     };
 
     // Two expressions for getting the value. One that's suitable use in the
@@ -418,8 +418,8 @@ fn create_field_getters<'a>(
             ),
         ),
     };
-    let (body, optional_expr) = match &field.type_info {
-        node_types::FieldTypeInfo::ReservedWordInt(int_mapping) => {
+    let (body, optional_expr) = match &field.type_info.kind {
+        node_types::FieldTypeKind::ReservedWordInt(int_mapping) => {
             // Create an expression that binds the corresponding string to `result` for each `value`, e.g.:
             //   result = "foo" and value = 0 or
             //   result = "bar" and value = 1 or
@@ -454,7 +454,7 @@ fn create_field_getters<'a>(
                 None,
             )
         }
-        node_types::FieldTypeInfo::Single(_) | node_types::FieldTypeInfo::Multiple { .. } => {
+        node_types::FieldTypeKind::Single(_) | node_types::FieldTypeKind::Multiple { .. } => {
             (get_value, Some(get_value_any_index))
         }
     };

ruby/node-types/src/lib.rs

@@ -1,5 +1,5 @@
 use serde::Deserialize;
-use std::collections::BTreeMap;
+use std::collections::{BTreeMap, HashMap, HashSet};
 use std::path::Path;
 
 use std::collections::BTreeSet as Set;
@@ -22,14 +22,23 @@ pub enum EntryKind {
     Token { kind_id: usize },
 }
 
-#[derive(Debug, Ord, PartialOrd, Eq, PartialEq)]
+#[derive(Debug, Ord, PartialOrd, Eq, PartialEq, Hash, Clone)]
 pub struct TypeName {
     pub kind: String,
     pub named: bool,
 }
 
-#[derive(Debug)]
-pub enum FieldTypeInfo {
+#[derive(Debug, Eq, PartialEq)]
+pub struct FieldTypeInfo {
+    pub kind: FieldTypeKind,
+
+    /// The set of types this field is allowed to take, after recursively
+    /// expanding subtypes.
+    pub valid_types: HashSet<TypeName>,
+}
+
+#[derive(Debug, Eq, PartialEq)]
+pub enum FieldTypeKind {
     /// The field has a single type.
     Single(TypeName),
 
@@ -103,7 +112,49 @@ fn convert_types(node_types: &[NodeType]) -> Set<TypeName> {
     node_types.iter().map(convert_type).collect()
 }
 
+fn get_matching_types(
+    node: &NodeInfo,
+    type_map: &HashMap<NodeType, &NodeInfo>,
+) -> HashSet<TypeName> {
+    let mut result = HashSet::new();
+    let node_type_name = TypeName {
+        kind: node.kind.clone(),
+        named: node.named,
+    };
+    result.insert(node_type_name);
+    if let Some(subtypes) = &node.subtypes {
+        for subtype in subtypes {
+            let subtype = type_map.get(subtype).unwrap();
+            result.extend(get_matching_types(subtype, type_map));
+        }
+    }
+    result
+}
+
 pub fn convert_nodes(prefix: &str, nodes: &[NodeInfo]) -> NodeTypeMap {
+    // Since the nodes contain only weak references to their subtypes, build a
+    // map so we can resolve them.
+    let mut type_map: HashMap<NodeType, &NodeInfo> = HashMap::new();
+    for node in nodes {
+        let node_type = NodeType {
+            kind: node.kind.clone(),
+            named: node.named,
+        };
+        type_map.insert(node_type, node);
+    }
+
+    // Now recursively expand subtypes so that for each tree-sitter node type,
+    // we have a set of all matching types.
+    let mut transitive_type_map = HashMap::new();
+    for node in nodes {
+        let type_name = TypeName {
+            kind: node.kind.clone(),
+            named: node.named,
+        };
+        let matching_types = get_matching_types(node, &type_map);
+        transitive_type_map.insert(type_name, matching_types);
+    }
+
     let mut entries = NodeTypeMap::new();
     let mut token_kinds = Set::new();
 
@@ -166,6 +217,7 @@ pub fn convert_nodes(prefix: &str, nodes: &[NodeInfo]) -> NodeTypeMap {
                         field_info,
                         &mut fields,
                         &token_kinds,
+                        &transitive_type_map,
                     );
                 }
             }
@@ -178,6 +230,7 @@ pub fn convert_nodes(prefix: &str, nodes: &[NodeInfo]) -> NodeTypeMap {
                     children,
                     &mut fields,
                     &token_kinds,
+                    &transitive_type_map,
                 );
             }
             entries.insert(
@@ -222,6 +275,7 @@ fn add_field(
     field_info: &FieldInfo,
     fields: &mut Vec<Field>,
     token_kinds: &Set<TypeName>,
+    transitive_type_map: &HashMap<TypeName, HashSet<TypeName>>,
 ) {
     let parent_flattened_name = node_type_name(&parent_type_name.kind, parent_type_name.named);
     let column_name = escape_name(&name_for_field_or_child(&field_name));
@@ -245,6 +299,18 @@ fn add_field(
         }
     };
     let converted_types = convert_types(&field_info.types);
+
+    // Use the transitive type map we built earlier to create the set of all
+    // possible types this field could take.
+    let mut valid_types: HashSet<TypeName> = HashSet::new();
+    for type_name in &converted_types {
+        if let Some(types) = transitive_type_map.get(type_name) {
+            for t in types {
+                valid_types.insert(t.clone());
+            }
+        }
+    }
+
     let type_info = if field_info
         .types
         .iter()
@@ -259,20 +325,29 @@ fn add_field(
                 escape_name(&format!("{}_{}_{}", &prefix, parent_flattened_name, t.kind));
             field_token_ints.insert(t.kind.to_owned(), (counter, dbscheme_variant_name));
         }
-        FieldTypeInfo::ReservedWordInt(field_token_ints)
+        FieldTypeInfo {
+            kind: FieldTypeKind::ReservedWordInt(field_token_ints),
+            valid_types,
+        }
     } else if field_info.types.len() == 1 {
-        FieldTypeInfo::Single(converted_types.into_iter().next().unwrap())
+        FieldTypeInfo {
+            kind: FieldTypeKind::Single(converted_types.into_iter().next().unwrap()),
+            valid_types,
+        }
     } else {
         // The dbscheme type for this field will be a union. In QL, it'll just be AstNode.
-        FieldTypeInfo::Multiple {
-            types: converted_types,
-            dbscheme_union: format!(
-                "{}_{}_{}_type",
-                &prefix,
-                &parent_flattened_name,
-                &name_for_field_or_child(&field_name)
-            ),
-            ql_class: "AstNode".to_owned(),
+        FieldTypeInfo {
+            kind: FieldTypeKind::Multiple {
+                types: converted_types,
+                dbscheme_union: format!(
+                    "{}_{}_{}_type",
+                    &prefix,
+                    &parent_flattened_name,
+                    &name_for_field_or_child(&field_name)
+                ),
+                ql_class: "AstNode".to_owned(),
+            },
+            valid_types,
         }
     };
     let getter_name = format!(
@@ -303,7 +378,7 @@ pub struct NodeInfo {
     pub subtypes: Option<Vec<NodeType>>,
 }
 
-#[derive(Deserialize)]
+#[derive(Deserialize, Hash, Eq, PartialEq)]
 pub struct NodeType {
     #[serde(rename = "type")]
     pub kind: String,

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

@@ -2,6 +2,7 @@ private import codeql.ruby.AST
 private import codeql.ruby.security.performance.RegExpTreeView as RETV
 private import internal.AST
 private import internal.Constant
+private import internal.Literal
 private import internal.Scope
 private import internal.TreeSitter
 private import codeql.ruby.controlflow.CfgNodes
@@ -36,9 +37,9 @@ class NumericLiteral extends Literal, TNumericLiteral { }
  *  0xff
  * ```
  */
-class IntegerLiteral extends NumericLiteral, TIntegerLiteral {
+class IntegerLiteral extends NumericLiteral instanceof IntegerLiteralImpl {
   /** Gets the numerical value of this integer literal. */
-  int getValue() { none() }
+  final int getValue() { result = super.getValueImpl() }
 
   final override ConstantValue::ConstantIntegerValue getConstantValue() {
     result.isInt(this.getValue())
@@ -47,76 +48,6 @@ class IntegerLiteral extends NumericLiteral, TIntegerLiteral {
   final override string getAPrimaryQlClass() { result = "IntegerLiteral" }
 }
 
-private int parseInteger(Ruby::Integer i) {
-  exists(string s | s = i.getValue().toLowerCase().replaceAll("_", "") |
-    s.charAt(0) != "0" and
-    result = s.toInt()
-    or
-    exists(string str, string values, int shift |
-      s.matches("0b%") and
-      values = "01" and
-      str = s.suffix(2) and
-      shift = 1
-      or
-      s.matches("0x%") and
-      values = "0123456789abcdef" and
-      str = s.suffix(2) and
-      shift = 4
-      or
-      s.charAt(0) = "0" and
-      not s.charAt(1) = ["b", "x", "o"] and
-      values = "01234567" and
-      str = s.suffix(1) and
-      shift = 3
-      or
-      s.matches("0o%") and
-      values = "01234567" and
-      str = s.suffix(2) and
-      shift = 3
-    |
-      result =
-        sum(int index, string c, int v, int exp |
-          c = str.charAt(index) and
-          v = values.indexOf(c.toLowerCase()) and
-          exp = str.length() - index - 1
-        |
-          v.bitShiftLeft((str.length() - index - 1) * shift)
-        )
-    )
-  )
-}
-
-private class RequiredIntegerLiteralConstantValue extends RequiredConstantValue {
-  override predicate requiredInt(int i) { i = any(IntegerLiteral il).getValue() }
-}
-
-private class IntegerLiteralReal extends IntegerLiteral, TIntegerLiteralReal {
-  private Ruby::Integer g;
-
-  IntegerLiteralReal() { this = TIntegerLiteralReal(g) }
-
-  final override int getValue() { result = parseInteger(g) }
-
-  final override string toString() { result = g.getValue() }
-}
-
-private class IntegerLiteralSynth extends IntegerLiteral, TIntegerLiteralSynth {
-  private int value;
-
-  IntegerLiteralSynth() { this = TIntegerLiteralSynth(_, _, value) }
-
-  final override int getValue() { result = value }
-
-  final override string toString() { result = value.toString() }
-}
-
-// TODO: implement properly
-private float parseFloat(Ruby::Float f) { result = f.getValue().toFloat() }
-
-private class RequiredFloatConstantValue extends RequiredConstantValue {
-  override predicate requiredFloat(float f) { f = parseFloat(_) }
-}
-
 /**
  * A floating-point literal.
  *
@@ -139,26 +70,6 @@ class FloatLiteral extends NumericLiteral, TFloatLiteral {
   final override string getAPrimaryQlClass() { result = "FloatLiteral" }
 }
 
-private predicate isRationalValue(Ruby::Rational r, int numerator, int denominator) {
-  numerator = parseInteger(r.getChild()) and
-  denominator = 1
-  or
-  exists(Ruby::Float f, string regex, string before, string after |
-    f = r.getChild() and
-    regex = "([^.]*)\\.(.*)" and
-    before = f.getValue().regexpCapture(regex, 1) and
-    after = f.getValue().regexpCapture(regex, 2) and
-    numerator = before.toInt() * denominator + after.toInt() and
-    denominator = 10.pow(after.length())
-  )
-}
-
-private class RequiredRationalConstantValue extends RequiredConstantValue {
-  override predicate requiredRational(int numerator, int denominator) {
-    isRationalValue(_, numerator, denominator)
-  }
-}
-
 /**
  * A rational literal.
  *
@@ -183,20 +94,6 @@ class RationalLiteral extends NumericLiteral, TRationalLiteral {
   final override string getAPrimaryQlClass() { result = "RationalLiteral" }
 }
 
-private float getComplexValue(Ruby::Complex c) {
-  exists(string s |
-    s = c.getValue() and
-    result = s.prefix(s.length() - 1).toFloat()
-  )
-}
-
-private class RequiredComplexConstantValue extends RequiredConstantValue {
-  override predicate requiredComplex(float real, float imaginary) {
-    real = 0 and
-    imaginary = getComplexValue(_)
-  }
-}
-
 /**
  * A complex literal.
  *
@@ -261,30 +158,6 @@ class BooleanLiteral extends Literal, TBooleanLiteral {
   }
 }
 
-private class TrueLiteral extends BooleanLiteral, TTrueLiteral {
-  private Ruby::True g;
-
-  TrueLiteral() { this = TTrueLiteral(g) }
-
-  final override string toString() { result = g.getValue() }
-
-  final override predicate isTrue() { any() }
-}
-
-private class FalseLiteral extends BooleanLiteral, TFalseLiteral {
-  private Ruby::False g;
-
-  FalseLiteral() { this = TFalseLiteral(g) }
-
-  final override string toString() { result = g.getValue() }
-
-  final override predicate isFalse() { any() }
-}
-
-private class RequiredEncodingLiteralConstantValue extends RequiredConstantValue {
-  override predicate requiredString(string s) { s = "UTF-8" }
-}
-
 /**
  * An `__ENCODING__` literal.
  */
@@ -297,10 +170,6 @@ class EncodingLiteral extends Literal, TEncoding {
   override ConstantValue::ConstantStringValue getConstantValue() { result.isString("UTF-8") }
 }
 
-private class RequiredLineLiteralConstantValue extends RequiredConstantValue {
-  override predicate requiredInt(int i) { i = any(LineLiteral ll).getLocation().getStartLine() }
-}
-
 /**
  * A `__LINE__` literal.
  */
@@ -314,12 +183,6 @@ class LineLiteral extends Literal, TLine {
   }
 }
 
-private class RequiredFileLiteralConstantValue extends RequiredConstantValue {
-  override predicate requiredString(string s) {
-    s = any(FileLiteral fl).getLocation().getFile().getAbsolutePath()
-  }
-}
-
 /**
  * A `__FILE__` literal.
  */
@@ -350,12 +213,6 @@ class StringComponent extends AstNode, TStringComponent {
   ConstantValue::ConstantStringValue getConstantValue() { none() }
 }
 
-private class RequiredStringTextComponentConstantValue extends RequiredConstantValue {
-  override predicate requiredString(string s) {
-    s = any(Ruby::Token t | exists(TStringTextComponentNonRegexp(t))).getValue()
-  }
-}
-
 /**
  * A component of a string (or string-like) literal that is simply text.
  *
@@ -382,12 +239,6 @@ class StringTextComponent extends StringComponent, TStringTextComponentNonRegexp
   final override string getAPrimaryQlClass() { result = "StringTextComponent" }
 }
 
-private class RequiredStringEscapeSequenceComponentConstantValue extends RequiredConstantValue {
-  override predicate requiredString(string s) {
-    s = any(Ruby::Token t | exists(TStringEscapeSequenceComponentNonRegexp(t))).getValue()
-  }
-}
-
 /**
  * An escape sequence component of a string or string-like literal.
  */
@@ -425,10 +276,6 @@ class StringInterpolationComponent extends StringComponent, StmtSequence,
   final override string getAPrimaryQlClass() { result = "StringInterpolationComponent" }
 }
 
-private class TRegExpComponent =
-  TStringTextComponentRegexp or TStringEscapeSequenceComponentRegexp or
-      TStringInterpolationComponentRegexp;
-
 /**
  * The base class for a component of a regular expression literal.
  */
@@ -444,20 +291,6 @@ class RegExpComponent extends AstNode, TRegExpComponent {
   ConstantValue::ConstantStringValue getConstantValue() { none() }
 }
 
-// Exclude components that are children of a free-spacing regex.
-// We do this because `ParseRegExp.qll` cannot handle free-spacing regexes.
-private string getRegExpTextComponentValue(RegExpTextComponent c) {
-  exists(Ruby::Token t |
-    c = TStringTextComponentRegexp(t) and
-    not c.getParent().(RegExpLiteral).hasFreeSpacingFlag() and
-    result = t.getValue()
-  )
-}
-
-private class RequiredRegExpTextComponentConstantValue extends RequiredConstantValue {
-  override predicate requiredString(string s) { s = getRegExpTextComponentValue(_) }
-}
-
 /**
  * A component of a regex literal that is simply text.
  *
@@ -484,20 +317,6 @@ class RegExpTextComponent extends RegExpComponent, TStringTextComponentRegexp {
   final override string getAPrimaryQlClass() { result = "RegExpTextComponent" }
 }
 
-// Exclude components that are children of a free-spacing regex.
-// We do this because `ParseRegExp.qll` cannot handle free-spacing regexes.
-private string getRegExpEscapeSequenceComponentValue(RegExpEscapeSequenceComponent c) {
-  exists(Ruby::EscapeSequence e |
-    c = TStringEscapeSequenceComponentRegexp(e) and
-    not c.getParent().(RegExpLiteral).hasFreeSpacingFlag() and
-    result = e.getValue()
-  )
-}
-
-private class RequiredRegExpEscapeSequenceComponentConstantValue extends RequiredConstantValue {
-  override predicate requiredString(string s) { s = getRegExpEscapeSequenceComponentValue(_) }
-}
-
 /**
  * An escape sequence component of a regex literal.
  */
@@ -662,22 +481,6 @@ class StringLiteral extends StringlikeLiteral, TStringLiteral {
   final override string getAPrimaryQlClass() { result = "StringLiteral" }
 }
 
-private class RegularStringLiteral extends StringLiteral, TRegularStringLiteral {
-  private Ruby::String g;
-
-  RegularStringLiteral() { this = TRegularStringLiteral(g) }
-
-  final override StringComponent getComponent(int n) { toGenerated(result) = g.getChild(n) }
-}
-
-private class BareStringLiteral extends StringLiteral, TBareStringLiteral {
-  private Ruby::BareString g;
-
-  BareStringLiteral() { this = TBareStringLiteral(g) }
-
-  final override StringComponent getComponent(int n) { toGenerated(result) = g.getChild(n) }
-}
-
 /**
  * A regular expression literal.
  *
@@ -762,13 +565,6 @@ class SymbolLiteral extends StringlikeLiteral, TSymbolLiteral {
   }
 }
 
-// Tree-sitter gives us value text including the colon, which we skip.
-private string getSimpleSymbolValue(Ruby::SimpleSymbol ss) { result = ss.getValue().suffix(1) }
-
-private class RequiredSimpleSymbolConstantValue extends RequiredConstantValue {
-  override predicate requiredSymbol(string s) { s = getSimpleSymbolValue(_) }
-}
-
 private class SimpleSymbolLiteral extends SymbolLiteral, TSimpleSymbolLiteral {
   private Ruby::SimpleSymbol g;
 
@@ -781,40 +577,6 @@ private class SimpleSymbolLiteral extends SymbolLiteral, TSimpleSymbolLiteral {
   final override string toString() { result = g.getValue() }
 }
 
-private class ComplexSymbolLiteral extends SymbolLiteral, TComplexSymbolLiteral { }
-
-private class DelimitedSymbolLiteral extends ComplexSymbolLiteral, TDelimitedSymbolLiteral {
-  private Ruby::DelimitedSymbol g;
-
-  DelimitedSymbolLiteral() { this = TDelimitedSymbolLiteral(g) }
-
-  final override StringComponent getComponent(int i) { toGenerated(result) = g.getChild(i) }
-}
-
-private class BareSymbolLiteral extends ComplexSymbolLiteral, TBareSymbolLiteral {
-  private Ruby::BareSymbol g;
-
-  BareSymbolLiteral() { this = TBareSymbolLiteral(g) }
-
-  final override StringComponent getComponent(int i) { toGenerated(result) = g.getChild(i) }
-}
-
-private class RequiredHashKeySymbolConstantValue extends RequiredConstantValue {
-  override predicate requiredSymbol(string s) { s = any(Ruby::HashKeySymbol h).getValue() }
-}
-
-private class HashKeySymbolLiteral extends SymbolLiteral, THashKeySymbolLiteral {
-  private Ruby::HashKeySymbol g;
-
-  HashKeySymbolLiteral() { this = THashKeySymbolLiteral(g) }
-
-  final override ConstantValue::ConstantSymbolValue getConstantValue() {
-    result.isSymbol(g.getValue())
-  }
-
-  final override string toString() { result = ":" + g.getValue() }
-}
-
 /**
  * A subshell literal.
  *
@@ -933,67 +695,25 @@ class HereDoc extends StringlikeLiteral, THereDoc {
  * %i(foo bar)
  * ```
  */
-class ArrayLiteral extends Literal, TArrayLiteral {
+class ArrayLiteral extends Literal instanceof ArrayLiteralImpl {
   final override string getAPrimaryQlClass() { result = "ArrayLiteral" }
 
   /** Gets the `n`th element in this array literal. */
-  final Expr getElement(int n) { result = this.(ArrayLiteralImpl).getElementImpl(n) }
+  final Expr getElement(int n) { result = super.getElementImpl(n) }
 
   /** Gets an element in this array literal. */
   final Expr getAnElement() { result = this.getElement(_) }
 
   /** Gets the number of elements in this array literal. */
-  final int getNumberOfElements() { result = this.(ArrayLiteralImpl).getNumberOfElementsImpl() }
+  final int getNumberOfElements() { result = super.getNumberOfElementsImpl() }
 
   final override AstNode getAChild(string pred) {
-    result = super.getAChild(pred)
+    result = Literal.super.getAChild(pred)
     or
     pred = "getElement" and result = this.getElement(_)
   }
 }
 
-abstract private class ArrayLiteralImpl extends ArrayLiteral {
-  abstract Expr getElementImpl(int n);
-
-  abstract int getNumberOfElementsImpl();
-}
-
-private class RegularArrayLiteral extends ArrayLiteralImpl, TRegularArrayLiteral {
-  private Ruby::Array g;
-
-  RegularArrayLiteral() { this = TRegularArrayLiteral(g) }
-
-  final override Expr getElementImpl(int i) { toGenerated(result) = g.getChild(i) }
-
-  final override int getNumberOfElementsImpl() { result = count(g.getChild(_)) }
-
-  final override string toString() { result = "[...]" }
-}
-
-private class StringArrayLiteral extends ArrayLiteralImpl, TStringArrayLiteral {
-  private Ruby::StringArray g;
-
-  StringArrayLiteral() { this = TStringArrayLiteral(g) }
-
-  final override Expr getElementImpl(int i) { toGenerated(result) = g.getChild(i) }
-
-  final override int getNumberOfElementsImpl() { result = count(g.getChild(_)) }
-
-  final override string toString() { result = "%w(...)" }
-}
-
-private class SymbolArrayLiteral extends ArrayLiteralImpl, TSymbolArrayLiteral {
-  private Ruby::SymbolArray g;
-
-  SymbolArrayLiteral() { this = TSymbolArrayLiteral(g) }
-
-  final override Expr getElementImpl(int i) { toGenerated(result) = g.getChild(i) }
-
-  final override int getNumberOfElementsImpl() { result = count(g.getChild(_)) }
-
-  final override string toString() { result = "%i(...)" }
-}
-
 /**
  * A hash literal.
  *
@@ -1001,7 +721,7 @@ private class SymbolArrayLiteral extends ArrayLiteralImpl, TSymbolArrayLiteral {
  * { foo: 123, bar: 456 }
  * ```
  */
-class HashLiteral extends Literal, THashLiteral {
+class HashLiteral extends Literal instanceof HashLiteralImpl {
   private Ruby::Hash g;
 
   HashLiteral() { this = THashLiteral(g) }
@@ -1027,12 +747,12 @@ class HashLiteral extends Literal, THashLiteral {
   final Pair getAKeyValuePair() { result = this.getAnElement() }
 
   /** Gets the number of elements in this hash literal. */
-  final int getNumberOfElements() { result = count(this.getAnElement()) }
+  final int getNumberOfElements() { result = super.getNumberOfElementsImpl() }
 
   final override string toString() { result = "{...}" }
 
   final override AstNode getAChild(string pred) {
-    result = super.getAChild(pred)
+    result = Literal.super.getAChild(pred)
     or
     pred = "getElement" and result = this.getElement(_)
   }
@@ -1086,34 +806,6 @@ class RangeLiteral extends Literal, TRangeLiteral {
   }
 }
 
-private class RangeLiteralReal extends RangeLiteral, TRangeLiteralReal {
-  private Ruby::Range g;
-
-  RangeLiteralReal() { this = TRangeLiteralReal(g) }
-
-  final override Expr getBegin() { toGenerated(result) = g.getBegin() }
-
-  final override Expr getEnd() { toGenerated(result) = g.getEnd() }
-
-  final override predicate isInclusive() { g instanceof @ruby_range_dotdot }
-
-  final override predicate isExclusive() { g instanceof @ruby_range_dotdotdot }
-}
-
-private class RangeLiteralSynth extends RangeLiteral, TRangeLiteralSynth {
-  private boolean inclusive;
-
-  RangeLiteralSynth() { this = TRangeLiteralSynth(_, _, inclusive) }
-
-  final override Expr getBegin() { result = TIntegerLiteralSynth(this, 0, _) }
-
-  final override Expr getEnd() { result = TIntegerLiteralSynth(this, 1, _) }
-
-  final override predicate isInclusive() { inclusive = true }
-
-  final override predicate isExclusive() { inclusive = false }
-}
-
 /**
  * A method name literal. For example:
  * ```rb
@@ -1128,25 +820,3 @@ class MethodName extends Literal {
 
   final override string getAPrimaryQlClass() { result = "MethodName" }
 }
-
-private string getMethodName(MethodName::Token t) {
-  result = t.(Ruby::Token).getValue()
-  or
-  result = t.(Ruby::Setter).getName().getValue() + "="
-}
-
-private class RequiredMethodNameConstantValue extends RequiredConstantValue {
-  override predicate requiredString(string s) { s = getMethodName(_) }
-}
-
-private class TokenMethodName extends MethodName, TTokenMethodName {
-  private MethodName::Token g;
-
-  TokenMethodName() { this = TTokenMethodName(g) }
-
-  final override ConstantValue::ConstantStringValue getConstantValue() {
-    result.isString(getMethodName(g))
-  }
-
-  final override string toString() { result = getMethodName(g) }
-}

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

@@ -0,0 +1,364 @@
+private import codeql.ruby.AST
+private import AST
+private import Constant
+private import TreeSitter
+private import codeql.ruby.controlflow.CfgNodes
+
+int parseInteger(Ruby::Integer i) {
+  exists(string s | s = i.getValue().toLowerCase().replaceAll("_", "") |
+    s.charAt(0) != "0" and
+    result = s.toInt()
+    or
+    exists(string str, string values, int shift |
+      s.matches("0b%") and
+      values = "01" and
+      str = s.suffix(2) and
+      shift = 1
+      or
+      s.matches("0x%") and
+      values = "0123456789abcdef" and
+      str = s.suffix(2) and
+      shift = 4
+      or
+      s.charAt(0) = "0" and
+      not s.charAt(1) = ["b", "x", "o"] and
+      values = "01234567" and
+      str = s.suffix(1) and
+      shift = 3
+      or
+      s.matches("0o%") and
+      values = "01234567" and
+      str = s.suffix(2) and
+      shift = 3
+    |
+      result =
+        sum(int index, string c, int v, int exp |
+          c = str.charAt(index) and
+          v = values.indexOf(c.toLowerCase()) and
+          exp = str.length() - index - 1
+        |
+          v.bitShiftLeft((str.length() - index - 1) * shift)
+        )
+    )
+  )
+}
+
+private class RequiredIntegerLiteralConstantValue extends RequiredConstantValue {
+  override predicate requiredInt(int i) { i = any(IntegerLiteral il).getValue() }
+}
+
+abstract class IntegerLiteralImpl extends Expr, TIntegerLiteral {
+  abstract int getValueImpl();
+}
+
+class IntegerLiteralReal extends IntegerLiteralImpl, TIntegerLiteralReal {
+  private Ruby::Integer g;
+
+  IntegerLiteralReal() { this = TIntegerLiteralReal(g) }
+
+  final override int getValueImpl() { result = parseInteger(g) }
+
+  final override string toString() { result = g.getValue() }
+}
+
+class IntegerLiteralSynth extends IntegerLiteralImpl, TIntegerLiteralSynth {
+  private int value;
+
+  IntegerLiteralSynth() { this = TIntegerLiteralSynth(_, _, value) }
+
+  final override int getValueImpl() { result = value }
+
+  final override string toString() { result = value.toString() }
+}
+
+// TODO: implement properly
+float parseFloat(Ruby::Float f) { result = f.getValue().toFloat() }
+
+private class RequiredFloatConstantValue extends RequiredConstantValue {
+  override predicate requiredFloat(float f) { f = parseFloat(_) }
+}
+
+predicate isRationalValue(Ruby::Rational r, int numerator, int denominator) {
+  numerator = parseInteger(r.getChild()) and
+  denominator = 1
+  or
+  exists(Ruby::Float f, string regex, string before, string after |
+    f = r.getChild() and
+    regex = "([^.]*)\\.(.*)" and
+    before = f.getValue().regexpCapture(regex, 1) and
+    after = f.getValue().regexpCapture(regex, 2) and
+    numerator = before.toInt() * denominator + after.toInt() and
+    denominator = 10.pow(after.length())
+  )
+}
+
+private class RequiredRationalConstantValue extends RequiredConstantValue {
+  override predicate requiredRational(int numerator, int denominator) {
+    isRationalValue(_, numerator, denominator)
+  }
+}
+
+float getComplexValue(Ruby::Complex c) {
+  exists(string s |
+    s = c.getValue() and
+    result = s.prefix(s.length() - 1).toFloat()
+  )
+}
+
+private class RequiredComplexConstantValue extends RequiredConstantValue {
+  override predicate requiredComplex(float real, float imaginary) {
+    real = 0 and
+    imaginary = getComplexValue(_)
+  }
+}
+
+class TrueLiteral extends BooleanLiteral, TTrueLiteral {
+  private Ruby::True g;
+
+  TrueLiteral() { this = TTrueLiteral(g) }
+
+  final override string toString() { result = g.getValue() }
+
+  final override predicate isTrue() { any() }
+}
+
+class FalseLiteral extends BooleanLiteral, TFalseLiteral {
+  private Ruby::False g;
+
+  FalseLiteral() { this = TFalseLiteral(g) }
+
+  final override string toString() { result = g.getValue() }
+
+  final override predicate isFalse() { any() }
+}
+
+private class RequiredEncodingLiteralConstantValue extends RequiredConstantValue {
+  override predicate requiredString(string s) { s = "UTF-8" }
+}
+
+private class RequiredLineLiteralConstantValue extends RequiredConstantValue {
+  override predicate requiredInt(int i) { i = any(LineLiteral ll).getLocation().getStartLine() }
+}
+
+private class RequiredFileLiteralConstantValue extends RequiredConstantValue {
+  override predicate requiredString(string s) {
+    s = any(FileLiteral fl).getLocation().getFile().getAbsolutePath()
+  }
+}
+
+private class RequiredStringTextComponentConstantValue extends RequiredConstantValue {
+  override predicate requiredString(string s) {
+    s = any(Ruby::Token t | exists(TStringTextComponentNonRegexp(t))).getValue()
+  }
+}
+
+private class RequiredStringEscapeSequenceComponentConstantValue extends RequiredConstantValue {
+  override predicate requiredString(string s) {
+    s = any(Ruby::Token t | exists(TStringEscapeSequenceComponentNonRegexp(t))).getValue()
+  }
+}
+
+class TRegExpComponent =
+  TStringTextComponentRegexp or TStringEscapeSequenceComponentRegexp or
+      TStringInterpolationComponentRegexp;
+
+// Exclude components that are children of a free-spacing regex.
+// We do this because `ParseRegExp.qll` cannot handle free-spacing regexes.
+string getRegExpTextComponentValue(RegExpTextComponent c) {
+  exists(Ruby::Token t |
+    c = TStringTextComponentRegexp(t) and
+    not c.getParent().(RegExpLiteral).hasFreeSpacingFlag() and
+    result = t.getValue()
+  )
+}
+
+private class RequiredRegExpTextComponentConstantValue extends RequiredConstantValue {
+  override predicate requiredString(string s) { s = getRegExpTextComponentValue(_) }
+}
+
+// Exclude components that are children of a free-spacing regex.
+// We do this because `ParseRegExp.qll` cannot handle free-spacing regexes.
+string getRegExpEscapeSequenceComponentValue(RegExpEscapeSequenceComponent c) {
+  exists(Ruby::EscapeSequence e |
+    c = TStringEscapeSequenceComponentRegexp(e) and
+    not c.getParent().(RegExpLiteral).hasFreeSpacingFlag() and
+    result = e.getValue()
+  )
+}
+
+private class RequiredRegExpEscapeSequenceComponentConstantValue extends RequiredConstantValue {
+  override predicate requiredString(string s) { s = getRegExpEscapeSequenceComponentValue(_) }
+}
+
+class RegularStringLiteral extends StringLiteral, TRegularStringLiteral {
+  private Ruby::String g;
+
+  RegularStringLiteral() { this = TRegularStringLiteral(g) }
+
+  final override StringComponent getComponent(int n) { toGenerated(result) = g.getChild(n) }
+}
+
+class BareStringLiteral extends StringLiteral, TBareStringLiteral {
+  private Ruby::BareString g;
+
+  BareStringLiteral() { this = TBareStringLiteral(g) }
+
+  final override StringComponent getComponent(int n) { toGenerated(result) = g.getChild(n) }
+}
+
+// Tree-sitter gives us value text including the colon, which we skip.
+string getSimpleSymbolValue(Ruby::SimpleSymbol ss) { result = ss.getValue().suffix(1) }
+
+private class RequiredSimpleSymbolConstantValue extends RequiredConstantValue {
+  override predicate requiredSymbol(string s) { s = getSimpleSymbolValue(_) }
+}
+
+private class SimpleSymbolLiteral extends SymbolLiteral, TSimpleSymbolLiteral {
+  private Ruby::SimpleSymbol g;
+
+  SimpleSymbolLiteral() { this = TSimpleSymbolLiteral(g) }
+
+  final override ConstantValue::ConstantSymbolValue getConstantValue() {
+    result.isSymbol(getSimpleSymbolValue(g))
+  }
+
+  final override string toString() { result = g.getValue() }
+}
+
+class ComplexSymbolLiteral extends SymbolLiteral, TComplexSymbolLiteral { }
+
+class DelimitedSymbolLiteral extends ComplexSymbolLiteral, TDelimitedSymbolLiteral {
+  private Ruby::DelimitedSymbol g;
+
+  DelimitedSymbolLiteral() { this = TDelimitedSymbolLiteral(g) }
+
+  final override StringComponent getComponent(int i) { toGenerated(result) = g.getChild(i) }
+}
+
+class BareSymbolLiteral extends ComplexSymbolLiteral, TBareSymbolLiteral {
+  private Ruby::BareSymbol g;
+
+  BareSymbolLiteral() { this = TBareSymbolLiteral(g) }
+
+  final override StringComponent getComponent(int i) { toGenerated(result) = g.getChild(i) }
+}
+
+private class RequiredHashKeySymbolConstantValue extends RequiredConstantValue {
+  override predicate requiredSymbol(string s) { s = any(Ruby::HashKeySymbol h).getValue() }
+}
+
+private class HashKeySymbolLiteral extends SymbolLiteral, THashKeySymbolLiteral {
+  private Ruby::HashKeySymbol g;
+
+  HashKeySymbolLiteral() { this = THashKeySymbolLiteral(g) }
+
+  final override ConstantValue::ConstantSymbolValue getConstantValue() {
+    result.isSymbol(g.getValue())
+  }
+
+  final override string toString() { result = ":" + g.getValue() }
+}
+
+private class RequiredCharacterConstantValue extends RequiredConstantValue {
+  override predicate requiredString(string s) { s = any(Ruby::Character c).getValue() }
+}
+
+abstract class ArrayLiteralImpl extends Literal, TArrayLiteral {
+  abstract Expr getElementImpl(int n);
+
+  abstract int getNumberOfElementsImpl();
+}
+
+class RegularArrayLiteral extends ArrayLiteralImpl, TRegularArrayLiteral {
+  private Ruby::Array g;
+
+  RegularArrayLiteral() { this = TRegularArrayLiteral(g) }
+
+  final override Expr getElementImpl(int i) { toGenerated(result) = g.getChild(i) }
+
+  final override int getNumberOfElementsImpl() { result = count(g.getChild(_)) }
+
+  final override string toString() { result = "[...]" }
+}
+
+class StringArrayLiteral extends ArrayLiteralImpl, TStringArrayLiteral {
+  private Ruby::StringArray g;
+
+  StringArrayLiteral() { this = TStringArrayLiteral(g) }
+
+  final override Expr getElementImpl(int i) { toGenerated(result) = g.getChild(i) }
+
+  final override int getNumberOfElementsImpl() { result = count(g.getChild(_)) }
+
+  final override string toString() { result = "%w(...)" }
+}
+
+class SymbolArrayLiteral extends ArrayLiteralImpl, TSymbolArrayLiteral {
+  private Ruby::SymbolArray g;
+
+  SymbolArrayLiteral() { this = TSymbolArrayLiteral(g) }
+
+  final override Expr getElementImpl(int i) { toGenerated(result) = g.getChild(i) }
+
+  final override int getNumberOfElementsImpl() { result = count(g.getChild(_)) }
+
+  final override string toString() { result = "%i(...)" }
+}
+
+class HashLiteralImpl extends Literal, THashLiteral {
+  Ruby::Hash g;
+
+  HashLiteralImpl() { this = THashLiteral(g) }
+
+  final int getNumberOfElementsImpl() { result = count(g.getChild(_)) }
+}
+
+class RangeLiteralReal extends RangeLiteral, TRangeLiteralReal {
+  private Ruby::Range g;
+
+  RangeLiteralReal() { this = TRangeLiteralReal(g) }
+
+  final override Expr getBegin() { toGenerated(result) = g.getBegin() }
+
+  final override Expr getEnd() { toGenerated(result) = g.getEnd() }
+
+  final override predicate isInclusive() { g instanceof @ruby_range_dotdot }
+
+  final override predicate isExclusive() { g instanceof @ruby_range_dotdotdot }
+}
+
+class RangeLiteralSynth extends RangeLiteral, TRangeLiteralSynth {
+  private boolean inclusive;
+
+  RangeLiteralSynth() { this = TRangeLiteralSynth(_, _, inclusive) }
+
+  final override Expr getBegin() { result = TIntegerLiteralSynth(this, 0, _) }
+
+  final override Expr getEnd() { result = TIntegerLiteralSynth(this, 1, _) }
+
+  final override predicate isInclusive() { inclusive = true }
+
+  final override predicate isExclusive() { inclusive = false }
+}
+
+private string getMethodName(MethodName::Token t) {
+  result = t.(Ruby::Token).getValue()
+  or
+  result = t.(Ruby::Setter).getName().getValue() + "="
+}
+
+private class RequiredMethodNameConstantValue extends RequiredConstantValue {
+  override predicate requiredString(string s) { s = getMethodName(_) }
+}
+
+class TokenMethodName extends MethodName, TTokenMethodName {
+  private MethodName::Token g;
+
+  TokenMethodName() { this = TTokenMethodName(g) }
+
+  final override ConstantValue::ConstantStringValue getConstantValue() {
+    result.isString(getMethodName(g))
+  }
+
+  final override string toString() { result = getMethodName(g) }
+}

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

@@ -794,14 +794,13 @@ private module ArrayLiteralDesugar {
     exists(ArrayLiteral al |
       parent = al and
       i = -1 and
-      child = SynthChild(MethodCallKind("[]", false, al.getNumberOfElements() + 1))
+      child = SynthChild(MethodCallKind("[]", false, al.getNumberOfElements()))
       or
-      exists(AstNode mc | mc = TMethodCallSynth(al, -1, _, _, _) |
-        parent = mc and
+      parent = TMethodCallSynth(al, -1, _, _, _) and
+      (
         i = 0 and
         child = SynthChild(ConstantReadAccessKind("::Array"))
         or
-        parent = mc and
         child = childRef(al.getElement(i - 1))
       )
     )
@@ -825,13 +824,56 @@ private module ArrayLiteralDesugar {
     final override predicate methodCall(string name, boolean setter, int arity) {
       name = "[]" and
       setter = false and
-      arity = any(ArrayLiteral al).getNumberOfElements() + 1
+      arity = any(ArrayLiteral al).getNumberOfElements()
     }
 
     final override predicate constantReadAccess(string name) { name = "::Array" }
   }
 }
 
+private module HashLiteralDesugar {
+  pragma[nomagic]
+  private predicate hashLiteralSynthesis(AstNode parent, int i, Child child) {
+    exists(HashLiteral hl |
+      parent = hl and
+      i = -1 and
+      child = SynthChild(MethodCallKind("[]", false, hl.getNumberOfElements()))
+      or
+      parent = TMethodCallSynth(hl, -1, _, _, _) and
+      (
+        i = 0 and
+        child = SynthChild(ConstantReadAccessKind("::Hash"))
+        or
+        child = childRef(hl.getElement(i - 1))
+      )
+    )
+  }
+
+  /**
+   * ```rb
+   * { a: 1, **splat, b: 2 }
+   * ```
+   * desugars to
+   *
+   * ```rb
+   * ::Hash.[](a: 1, **splat, b: 2)
+   * ```
+   */
+  private class HashLiteralSynthesis extends Synthesis {
+    final override predicate child(AstNode parent, int i, Child child) {
+      hashLiteralSynthesis(parent, i, child)
+    }
+
+    final override predicate methodCall(string name, boolean setter, int arity) {
+      name = "[]" and
+      setter = false and
+      arity = any(HashLiteral hl).getNumberOfElements()
+    }
+
+    final override predicate constantReadAccess(string name) { name = "::Hash" }
+  }
+}
+
 /**
  * ```rb
  * for x in xs

ruby/ql/lib/codeql/ruby/controlflow/CfgNodes.qll

@@ -484,6 +484,9 @@ module ExprNodes {
     /** Gets the `n`th argument of this call. */
     final ExprCfgNode getArgument(int n) { e.hasCfgChild(e.getArgument(n), this, result) }
 
+    /** Gets an argument of this call. */
+    final ExprCfgNode getAnArgument() { result = this.getArgument(_) }
+
     /** Gets the the keyword argument whose key is `keyword` of this call. */
     final ExprCfgNode getKeywordArgument(string keyword) {
       exists(PairCfgNode n |
@@ -940,4 +943,39 @@ module ExprNodes {
 
     final override ElementReference getExpr() { result = super.getExpr() }
   }
+
+  /**
+   * A control-flow node that wraps an array literal. Array literals are desugared
+   * into calls to `Array.[]`, so this includes both desugared calls as well as
+   * explicit calls.
+   */
+  class ArrayLiteralCfgNode extends MethodCallCfgNode {
+    ArrayLiteralCfgNode() {
+      exists(ConstantReadAccess array |
+        array = this.getReceiver().getExpr() and
+        e.(MethodCall).getMethodName() = "[]" and
+        array.getName() = "Array" and
+        array.hasGlobalScope()
+      )
+    }
+  }
+
+  /**
+   * A control-flow node that wraps a hash literal. Hash literals are desugared
+   * into calls to `Hash.[]`, so this includes both desugared calls as well as
+   * explicit calls.
+   */
+  class HashLiteralCfgNode extends MethodCallCfgNode {
+    HashLiteralCfgNode() {
+      exists(ConstantReadAccess array |
+        array = this.getReceiver().getExpr() and
+        e.(MethodCall).getMethodName() = "[]" and
+        array.getName() = "Hash" and
+        array.hasGlobalScope()
+      )
+    }
+
+    /** Gets a pair of this hash literal. */
+    PairCfgNode getAKeyValuePair() { result = this.getAnArgument() }
+  }
 }

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

@@ -986,10 +986,6 @@ module Trees {
 
   private class GlobalVariableTree extends LeafTree, GlobalVariableAccess { }
 
-  private class HashLiteralTree extends StandardPostOrderTree, HashLiteral {
-    final override ControlFlowTree getChildElement(int i) { result = this.getElement(i) }
-  }
-
   private class HashSplatNilParameterTree extends LeafTree, HashSplatNilParameter { }
 
   private class HashSplatParameterTree extends NonDefaultValueParameterTree, HashSplatParameter { }

ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowDispatch.qll

@@ -278,16 +278,9 @@ private DataFlow::LocalSourceNode trackInstance(Module tp, TypeTracker t) {
     or
     result.asExpr().getExpr() instanceof StringlikeLiteral and tp = TResolved("String")
     or
-    exists(ConstantReadAccess array, MethodCall mc |
-      result.asExpr().getExpr() = mc and
-      mc.getMethodName() = "[]" and
-      mc.getReceiver() = array and
-      array.getName() = "Array" and
-      array.hasGlobalScope() and
-      tp = TResolved("Array")
-    )
+    result.asExpr() instanceof CfgNodes::ExprNodes::ArrayLiteralCfgNode and tp = TResolved("Array")
     or
-    result.asExpr().getExpr() instanceof HashLiteral and tp = TResolved("Hash")
+    result.asExpr() instanceof CfgNodes::ExprNodes::HashLiteralCfgNode and tp = TResolved("Hash")
     or
     result.asExpr().getExpr() instanceof MethodBase and tp = TResolved("Symbol")
     or

ruby/ql/lib/codeql/ruby/frameworks/XmlParsing.qll

@@ -52,14 +52,15 @@ private class LibXmlRubyXmlParserCall extends XmlParserCall::Range, DataFlow::Ca
   override DataFlow::Node getInput() { result = this.getArgument(0) }
 
   override predicate externalEntitiesEnabled() {
-    exists(Pair pair |
-      pair = this.getArgument(1).asExpr().getExpr().(HashLiteral).getAKeyValuePair() and
+    exists(CfgNodes::ExprNodes::PairCfgNode pair |
+      pair =
+        this.getArgument(1).asExpr().(CfgNodes::ExprNodes::HashLiteralCfgNode).getAKeyValuePair() and
       pair.getKey().getConstantValue().isStringOrSymbol("options") and
       pair.getValue() =
         [
           trackEnableFeature(TNOENT()), trackEnableFeature(TDTDLOAD()),
           trackDisableFeature(TNONET())
-        ].asExpr().getExpr()
+        ].asExpr()
     )
   }
 }

ruby/ql/lib/codeql/ruby/frameworks/http_clients/Excon.qll

@@ -1,4 +1,5 @@
 private import ruby
+private import codeql.ruby.CFG
 private import codeql.ruby.Concepts
 private import codeql.ruby.ApiGraphs
 
@@ -91,16 +92,16 @@ class ExconHttpRequest extends HTTP::Client::Request::Range {
 predicate argSetsVerifyPeer(DataFlow::Node arg, boolean value, DataFlow::Node kvNode) {
   // Either passed as an individual key:value argument, e.g.:
   // Excon.get(..., ssl_verify_peer: false)
-  isSslVerifyPeerPair(arg.asExpr().getExpr(), value) and
+  isSslVerifyPeerPair(arg.asExpr(), value) and
   kvNode = arg
   or
   // Or as a single hash argument, e.g.:
   // Excon.get(..., { ssl_verify_peer: false, ... })
-  exists(DataFlow::LocalSourceNode optionsNode, Pair p |
-    p = optionsNode.asExpr().getExpr().(HashLiteral).getAKeyValuePair() and
+  exists(DataFlow::LocalSourceNode optionsNode, CfgNodes::ExprNodes::PairCfgNode p |
+    p = optionsNode.asExpr().(CfgNodes::ExprNodes::HashLiteralCfgNode).getAKeyValuePair() and
     isSslVerifyPeerPair(p, value) and
     optionsNode.flowsTo(arg) and
-    kvNode.asExpr().getExpr() = p
+    kvNode.asExpr() = p
   )
 }
 
@@ -133,10 +134,10 @@ private predicate hasBooleanValue(DataFlow::Node node, boolean value) {
 }
 
 /** Holds if `p` is the pair `ssl_verify_peer: <value>`. */
-private predicate isSslVerifyPeerPair(Pair p, boolean value) {
+private predicate isSslVerifyPeerPair(CfgNodes::ExprNodes::PairCfgNode p, boolean value) {
   exists(DataFlow::Node key, DataFlow::Node valueNode |
-    key.asExpr().getExpr() = p.getKey() and valueNode.asExpr().getExpr() = p.getValue()
-  |
+    key.asExpr() = p.getKey() and
+    valueNode.asExpr() = p.getValue() and
     isSslVerifyPeerLiteral(key) and
     hasBooleanValue(valueNode, value)
   )

ruby/ql/lib/codeql/ruby/frameworks/http_clients/Faraday.qll

@@ -1,4 +1,5 @@
 private import ruby
+private import codeql.ruby.CFG
 private import codeql.ruby.Concepts
 private import codeql.ruby.ApiGraphs
 
@@ -56,16 +57,16 @@ class FaradayHttpRequest extends HTTP::Client::Request::Range {
     |
       // Either passed as an individual key:value argument, e.g.:
       // Faraday.new(..., ssl: {...})
-      isSslOptionsPairDisablingValidation(arg.asExpr().getExpr()) and
+      isSslOptionsPairDisablingValidation(arg.asExpr()) and
       disablingNode = arg
       or
       // Or as a single hash argument, e.g.:
       // Faraday.new(..., { ssl: {...} })
-      exists(DataFlow::LocalSourceNode optionsNode, Pair p |
-        p = optionsNode.asExpr().getExpr().(HashLiteral).getAKeyValuePair() and
+      exists(DataFlow::LocalSourceNode optionsNode, CfgNodes::ExprNodes::PairCfgNode p |
+        p = optionsNode.asExpr().(CfgNodes::ExprNodes::HashLiteralCfgNode).getAKeyValuePair() and
         isSslOptionsPairDisablingValidation(p) and
         optionsNode.flowsTo(arg) and
-        disablingNode.asExpr().getExpr() = p
+        disablingNode.asExpr() = p
       )
     )
   }
@@ -78,10 +79,10 @@ class FaradayHttpRequest extends HTTP::Client::Request::Range {
  * containing either `verify: false` or
  * `verify_mode: OpenSSL::SSL::VERIFY_NONE`.
  */
-private predicate isSslOptionsPairDisablingValidation(Pair p) {
+private predicate isSslOptionsPairDisablingValidation(CfgNodes::ExprNodes::PairCfgNode p) {
   exists(DataFlow::Node key, DataFlow::Node value |
-    key.asExpr().getExpr() = p.getKey() and value.asExpr().getExpr() = p.getValue()
-  |
+    key.asExpr() = p.getKey() and
+    value.asExpr() = p.getValue() and
     isSymbolLiteral(key, "ssl") and
     (isHashWithVerifyFalse(value) or isHashWithVerifyModeNone(value))
   )
@@ -101,7 +102,7 @@ private predicate isSymbolLiteral(DataFlow::Node node, string valueText) {
  */
 private predicate isHashWithVerifyFalse(DataFlow::Node node) {
   exists(DataFlow::LocalSourceNode hash |
-    isVerifyFalsePair(hash.asExpr().getExpr().(HashLiteral).getAKeyValuePair()) and
+    isVerifyFalsePair(hash.asExpr().(CfgNodes::ExprNodes::HashLiteralCfgNode).getAKeyValuePair()) and
     hash.flowsTo(node)
   )
 }
@@ -112,7 +113,7 @@ private predicate isHashWithVerifyFalse(DataFlow::Node node) {
  */
 private predicate isHashWithVerifyModeNone(DataFlow::Node node) {
   exists(DataFlow::LocalSourceNode hash |
-    isVerifyModeNonePair(hash.asExpr().getExpr().(HashLiteral).getAKeyValuePair()) and
+    isVerifyModeNonePair(hash.asExpr().(CfgNodes::ExprNodes::HashLiteralCfgNode).getAKeyValuePair()) and
     hash.flowsTo(node)
   )
 }
@@ -121,10 +122,10 @@ private predicate isHashWithVerifyModeNone(DataFlow::Node node) {
  * Holds if the pair `p` has the key `:verify_mode` and the value
  * `OpenSSL::SSL::VERIFY_NONE`.
  */
-private predicate isVerifyModeNonePair(Pair p) {
+private predicate isVerifyModeNonePair(CfgNodes::ExprNodes::PairCfgNode p) {
   exists(DataFlow::Node key, DataFlow::Node value |
-    key.asExpr().getExpr() = p.getKey() and value.asExpr().getExpr() = p.getValue()
-  |
+    key.asExpr() = p.getKey() and
+    value.asExpr() = p.getValue() and
     isSymbolLiteral(key, "verify_mode") and
     value = API::getTopLevelMember("OpenSSL").getMember("SSL").getMember("VERIFY_NONE").getAUse()
   )
@@ -133,10 +134,10 @@ private predicate isVerifyModeNonePair(Pair p) {
 /**
  * Holds if the pair `p` has the key `:verify` and the value `false`.
  */
-private predicate isVerifyFalsePair(Pair p) {
+private predicate isVerifyFalsePair(CfgNodes::ExprNodes::PairCfgNode p) {
   exists(DataFlow::Node key, DataFlow::Node value |
-    key.asExpr().getExpr() = p.getKey() and value.asExpr().getExpr() = p.getValue()
-  |
+    key.asExpr() = p.getKey() and
+    value.asExpr() = p.getValue() and
     isSymbolLiteral(key, "verify") and
     isFalse(value)
   )

ruby/ql/lib/codeql/ruby/frameworks/http_clients/Httparty.qll

@@ -1,4 +1,5 @@
 private import ruby
+private import codeql.ruby.CFG
 private import codeql.ruby.Concepts
 private import codeql.ruby.ApiGraphs
 
@@ -48,20 +49,21 @@ class HttpartyRequest extends HTTP::Client::Request::Range {
     // argument, and we're looking for `{ verify: false }` or
     // `{ verify_peer: false }`.
     exists(DataFlow::Node arg, int i |
-      i > 0 and arg.asExpr().getExpr() = requestUse.asExpr().getExpr().(MethodCall).getArgument(i)
+      i > 0 and
+      arg.asExpr() = requestUse.asExpr().(CfgNodes::ExprNodes::MethodCallCfgNode).getArgument(i)
     |
       // Either passed as an individual key:value argument, e.g.:
       // HTTParty.get(..., verify: false)
-      isVerifyFalsePair(arg.asExpr().getExpr()) and
+      isVerifyFalsePair(arg.asExpr()) and
       disablingNode = arg
       or
       // Or as a single hash argument, e.g.:
       // HTTParty.get(..., { verify: false, ... })
-      exists(DataFlow::LocalSourceNode optionsNode, Pair p |
-        p = optionsNode.asExpr().getExpr().(HashLiteral).getAKeyValuePair() and
+      exists(DataFlow::LocalSourceNode optionsNode, CfgNodes::ExprNodes::PairCfgNode p |
+        p = optionsNode.asExpr().(CfgNodes::ExprNodes::HashLiteralCfgNode).getAKeyValuePair() and
         isVerifyFalsePair(p) and
         optionsNode.flowsTo(arg) and
-        disablingNode.asExpr().getExpr() = p
+        disablingNode.asExpr() = p
       )
     )
   }
@@ -88,10 +90,10 @@ private predicate isFalse(DataFlow::Node node) {
 /**
  * Holds if `p` is the pair `verify: false` or `verify_peer: false`.
  */
-private predicate isVerifyFalsePair(Pair p) {
+private predicate isVerifyFalsePair(CfgNodes::ExprNodes::PairCfgNode p) {
   exists(DataFlow::Node key, DataFlow::Node value |
-    key.asExpr().getExpr() = p.getKey() and value.asExpr().getExpr() = p.getValue()
-  |
+    key.asExpr() = p.getKey() and
+    value.asExpr() = p.getValue() and
     isVerifyLiteral(key) and
     isFalse(value)
   )

ruby/ql/lib/codeql/ruby/frameworks/http_clients/OpenURI.qll

@@ -1,4 +1,5 @@
 private import ruby
+private import codeql.ruby.CFG
 private import codeql.ruby.Concepts
 private import codeql.ruby.ApiGraphs
 private import codeql.ruby.frameworks.StandardLibrary
@@ -32,8 +33,7 @@ class OpenUriRequest extends HTTP::Client::Request::Range {
 
   override predicate disablesCertificateValidation(DataFlow::Node disablingNode) {
     exists(DataFlow::Node arg |
-      arg.asExpr().getExpr() = requestUse.asExpr().getExpr().(MethodCall).getArgument(_)
-    |
+      arg.asExpr() = requestUse.asExpr().(CfgNodes::ExprNodes::MethodCallCfgNode).getAnArgument() and
       argumentDisablesValidation(arg, disablingNode)
     )
   }
@@ -68,8 +68,7 @@ class OpenUriKernelOpenRequest extends HTTP::Client::Request::Range {
   override predicate disablesCertificateValidation(DataFlow::Node disablingNode) {
     exists(DataFlow::Node arg, int i |
       i > 0 and
-      arg.asExpr().getExpr() = requestUse.asExpr().getExpr().(MethodCall).getArgument(i)
-    |
+      arg.asExpr() = requestUse.asExpr().(CfgNodes::ExprNodes::MethodCallCfgNode).getArgument(i) and
       argumentDisablesValidation(arg, disablingNode)
     )
   }
@@ -85,24 +84,24 @@ class OpenUriKernelOpenRequest extends HTTP::Client::Request::Range {
 private predicate argumentDisablesValidation(DataFlow::Node arg, DataFlow::Node disablingNode) {
   // Either passed as an individual key:value argument, e.g.:
   // URI.open(..., ssl_verify_mode: OpenSSL::SSL::VERIFY_NONE)
-  isSslVerifyModeNonePair(arg.asExpr().getExpr()) and
+  isSslVerifyModeNonePair(arg.asExpr()) and
   disablingNode = arg
   or
   // Or as a single hash argument, e.g.:
   // URI.open(..., { ssl_verify_mode: OpenSSL::SSL::VERIFY_NONE, ... })
-  exists(DataFlow::LocalSourceNode optionsNode, Pair p |
-    p = optionsNode.asExpr().getExpr().(HashLiteral).getAKeyValuePair() and
+  exists(DataFlow::LocalSourceNode optionsNode, CfgNodes::ExprNodes::PairCfgNode p |
+    p = optionsNode.asExpr().(CfgNodes::ExprNodes::HashLiteralCfgNode).getAKeyValuePair() and
     isSslVerifyModeNonePair(p) and
     optionsNode.flowsTo(arg) and
-    disablingNode.asExpr().getExpr() = p
+    disablingNode.asExpr() = p
   )
 }
 
 /** Holds if `p` is the pair `ssl_verify_mode: OpenSSL::SSL::VERIFY_NONE`. */
-private predicate isSslVerifyModeNonePair(Pair p) {
+private predicate isSslVerifyModeNonePair(CfgNodes::ExprNodes::PairCfgNode p) {
   exists(DataFlow::Node key, DataFlow::Node value |
-    key.asExpr().getExpr() = p.getKey() and value.asExpr().getExpr() = p.getValue()
-  |
+    key.asExpr() = p.getKey() and
+    value.asExpr() = p.getValue() and
     isSslVerifyModeLiteral(key) and
     value = API::getTopLevelMember("OpenSSL").getMember("SSL").getMember("VERIFY_NONE").getAUse()
   )

ruby/ql/lib/codeql/ruby/frameworks/http_clients/RestClient.qll

@@ -1,4 +1,5 @@
 private import ruby
+private import codeql.ruby.CFG
 private import codeql.ruby.Concepts
 private import codeql.ruby.ApiGraphs
 
@@ -50,16 +51,16 @@ class RestClientHttpRequest extends HTTP::Client::Request::Range {
     |
       // Either passed as an individual key:value argument, e.g.:
       // RestClient::Resource.new(..., verify_ssl: OpenSSL::SSL::VERIFY_NONE)
-      isVerifySslNonePair(arg.asExpr().getExpr()) and
+      isVerifySslNonePair(arg.asExpr()) and
       disablingNode = arg
       or
       // Or as a single hash argument, e.g.:
       // RestClient::Resource.new(..., { verify_ssl: OpenSSL::SSL::VERIFY_NONE })
-      exists(DataFlow::LocalSourceNode optionsNode, Pair p |
-        p = optionsNode.asExpr().getExpr().(HashLiteral).getAKeyValuePair() and
+      exists(DataFlow::LocalSourceNode optionsNode, CfgNodes::ExprNodes::PairCfgNode p |
+        p = optionsNode.asExpr().(CfgNodes::ExprNodes::HashLiteralCfgNode).getAKeyValuePair() and
         isVerifySslNonePair(p) and
         optionsNode.flowsTo(arg) and
-        disablingNode.asExpr().getExpr() = p
+        disablingNode.asExpr() = p
       )
     )
   }
@@ -68,10 +69,10 @@ class RestClientHttpRequest extends HTTP::Client::Request::Range {
 }
 
 /** Holds if `p` is the pair `verify_ssl: OpenSSL::SSL::VERIFY_NONE`. */
-private predicate isVerifySslNonePair(Pair p) {
+private predicate isVerifySslNonePair(CfgNodes::ExprNodes::PairCfgNode p) {
   exists(DataFlow::Node key, DataFlow::Node value |
-    key.asExpr().getExpr() = p.getKey() and value.asExpr().getExpr() = p.getValue()
-  |
+    key.asExpr() = p.getKey() and
+    value.asExpr() = p.getValue() and
     isSslVerifyModeLiteral(key) and
     value = API::getTopLevelMember("OpenSSL").getMember("SSL").getMember("VERIFY_NONE").getAUse()
   )

ruby/ql/lib/codeql/ruby/frameworks/http_clients/Typhoeus.qll

@@ -1,4 +1,5 @@
 private import ruby
+private import codeql.ruby.CFG
 private import codeql.ruby.Concepts
 private import codeql.ruby.ApiGraphs
 
@@ -31,16 +32,16 @@ class TyphoeusHttpRequest extends HTTP::Client::Request::Range {
     |
       // Either passed as an individual key:value argument, e.g.:
       // Typhoeus.get(..., ssl_verifypeer: false)
-      isSslVerifyPeerFalsePair(arg.asExpr().getExpr()) and
+      isSslVerifyPeerFalsePair(arg.asExpr()) and
       disablingNode = arg
       or
       // Or as a single hash argument, e.g.:
       // Typhoeus.get(..., { ssl_verifypeer: false, ... })
-      exists(DataFlow::LocalSourceNode optionsNode, Pair p |
-        p = optionsNode.asExpr().getExpr().(HashLiteral).getAKeyValuePair() and
+      exists(DataFlow::LocalSourceNode optionsNode, CfgNodes::ExprNodes::PairCfgNode p |
+        p = optionsNode.asExpr().(CfgNodes::ExprNodes::HashLiteralCfgNode).getAKeyValuePair() and
         isSslVerifyPeerFalsePair(p) and
         optionsNode.flowsTo(arg) and
-        disablingNode.asExpr().getExpr() = p
+        disablingNode.asExpr() = p
       )
     )
   }
@@ -49,11 +50,10 @@ class TyphoeusHttpRequest extends HTTP::Client::Request::Range {
 }
 
 /** Holds if `p` is the pair `ssl_verifypeer: false`. */
-private predicate isSslVerifyPeerFalsePair(Pair p) {
+private predicate isSslVerifyPeerFalsePair(CfgNodes::ExprNodes::PairCfgNode p) {
   exists(DataFlow::Node key, DataFlow::Node value |
-    key.asExpr().getExpr() = p.getKey() and
-    value.asExpr().getExpr() = p.getValue()
-  |
+    key.asExpr() = p.getKey() and
+    value.asExpr() = p.getValue() and
     isSslVerifyPeerLiteral(key) and
     isFalse(value)
   )

ruby/ql/lib/codeql/ruby/security/UnsafeDeserializationCustomizations.qll

@@ -73,12 +73,12 @@ module UnsafeDeserialization {
     result = "object" and isSafe = false
   }
 
-  private predicate isOjModePair(Pair p, string modeValue) {
+  private predicate isOjModePair(CfgNodes::ExprNodes::PairCfgNode p, string modeValue) {
     p.getKey().getConstantValue().isStringOrSymbol("mode") and
     exists(DataFlow::LocalSourceNode symbolLiteral, DataFlow::Node value |
       symbolLiteral.asExpr().getExpr().getConstantValue().isSymbol(modeValue) and
       symbolLiteral.flowsTo(value) and
-      value.asExpr().getExpr() = p.getValue()
+      value.asExpr() = p.getValue()
     )
   }
 
@@ -91,7 +91,8 @@ module UnsafeDeserialization {
     OjOptionsHashWithModeKey() {
       exists(DataFlow::LocalSourceNode options |
         options.flowsTo(this) and
-        isOjModePair(options.asExpr().getExpr().(HashLiteral).getAKeyValuePair(), modeValue)
+        isOjModePair(options.asExpr().(CfgNodes::ExprNodes::HashLiteralCfgNode).getAKeyValuePair(),
+          modeValue)
       )
     }
 
@@ -143,7 +144,7 @@ module UnsafeDeserialization {
       exists(DataFlow::Node arg, int i | i >= 1 and arg = this.getArgument(i) |
         arg.(OjOptionsHashWithModeKey).hasKnownMode(isSafe)
         or
-        isOjModePair(arg.asExpr().getExpr(), getAKnownOjModeName(isSafe))
+        isOjModePair(arg.asExpr(), getAKnownOjModeName(isSafe))
       )
     }
   }

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

@@ -47,6 +47,19 @@ calls/calls.rb:
 #  230|         getReceiver: [ConstantReadAccess] X
 #  229|     getReceiver: [MethodCall] call to bar
 #  229|       getReceiver: [ConstantReadAccess] X
+#  249| [HashLiteral] {...}
+#  249|   getDesugared: [MethodCall] call to []
+#  249|     getReceiver: [ConstantReadAccess] Hash
+#  249|     getArgument: [Pair] Pair
+#  249|       getKey: [MethodCall] call to foo
+#  249|         getReceiver: [Self, SelfVariableAccess] self
+#  249|       getValue: [MethodCall] call to bar
+#  249|         getReceiver: [Self, SelfVariableAccess] self
+#  249|     getArgument: [Pair] Pair
+#  249|       getKey: [MethodCall] call to foo
+#  249|         getReceiver: [ConstantReadAccess] X
+#  249|       getValue: [MethodCall] call to bar
+#  249|         getReceiver: [ConstantReadAccess] X
 #  314| [AssignExpr] ... = ...
 #  314|   getDesugared: [StmtSequence] ...
 #  314|     getStmt: [SetterMethodCall] call to foo=
@@ -292,6 +305,13 @@ constants/constants.rb:
 #   20|     getArgument: [StringLiteral] "Dave"
 #   20|       getComponent: [StringTextComponent] Dave
 literals/literals.rb:
+#   88| [HashLiteral] {...}
+#   88|   getDesugared: [MethodCall] call to []
+#   88|     getReceiver: [ConstantReadAccess] Hash
+#   88|     getArgument: [Pair] Pair
+#   88|       getKey: [SymbolLiteral] :foo
+#   88|       getValue: [StringLiteral] "bar"
+#   88|         getComponent: [StringTextComponent] bar
 #   96| [ArrayLiteral] [...]
 #   96|   getDesugared: [MethodCall] call to []
 #   96|     getReceiver: [ConstantReadAccess] Array
@@ -432,6 +452,31 @@ literals/literals.rb:
 #  113|       getComponent: [StringTextComponent] #{BAR}
 #  113|     getArgument: [SymbolLiteral] :"baz"
 #  113|       getComponent: [StringTextComponent] baz
+#  116| [HashLiteral] {...}
+#  116|   getDesugared: [MethodCall] call to []
+#  116|     getReceiver: [ConstantReadAccess] Hash
+#  117| [HashLiteral] {...}
+#  117|   getDesugared: [MethodCall] call to []
+#  117|     getReceiver: [ConstantReadAccess] Hash
+#  117|     getArgument: [Pair] Pair
+#  117|       getKey: [SymbolLiteral] :foo
+#  117|       getValue: [IntegerLiteral] 1
+#  117|     getArgument: [Pair] Pair
+#  117|       getKey: [SymbolLiteral] :bar
+#  117|       getValue: [IntegerLiteral] 2
+#  117|     getArgument: [Pair] Pair
+#  117|       getKey: [StringLiteral] "baz"
+#  117|         getComponent: [StringTextComponent] baz
+#  117|       getValue: [IntegerLiteral] 3
+#  118| [HashLiteral] {...}
+#  118|   getDesugared: [MethodCall] call to []
+#  118|     getReceiver: [ConstantReadAccess] Hash
+#  118|     getArgument: [Pair] Pair
+#  118|       getKey: [SymbolLiteral] :foo
+#  118|       getValue: [IntegerLiteral] 7
+#  118|     getArgument: [HashSplatExpr] ** ...
+#  118|       getAnOperand/getOperand/getReceiver: [MethodCall] call to baz
+#  118|         getReceiver: [Self, SelfVariableAccess] self
 control/loops.rb:
 #    9| [ForExpr] for ... in ...
 #    9|   getDesugared: [MethodCall] call to each
@@ -511,12 +556,14 @@ control/loops.rb:
 #   24|             getAnOperand/getLeftOperand/getReceiver: [LocalVariableAccess] foo
 #   24|             getAnOperand/getArgument/getRightOperand: [LocalVariableAccess] value
 #   22|     getReceiver: [HashLiteral] {...}
-#   22|       getElement: [Pair] Pair
-#   22|         getKey: [SymbolLiteral] :foo
-#   22|         getValue: [IntegerLiteral] 0
-#   22|       getElement: [Pair] Pair
-#   22|         getKey: [SymbolLiteral] :bar
-#   22|         getValue: [IntegerLiteral] 1
+#   22|       getDesugared: [MethodCall] call to []
+#   22|         getReceiver: [ConstantReadAccess] Hash
+#   22|         getArgument: [Pair] Pair
+#   22|           getKey: [SymbolLiteral] :foo
+#   22|           getValue: [IntegerLiteral] 0
+#   22|         getArgument: [Pair] Pair
+#   22|           getKey: [SymbolLiteral] :bar
+#   22|           getValue: [IntegerLiteral] 1
 #   28| [ForExpr] for ... in ...
 #   28|   getDesugared: [MethodCall] call to each
 #   28|     getBlock: [BraceBlock] { ... }
@@ -553,12 +600,14 @@ control/loops.rb:
 #   30|             getAnOperand/getArgument/getRightOperand: [LocalVariableAccess] value
 #   31|       getStmt: [BreakStmt] break
 #   28|     getReceiver: [HashLiteral] {...}
-#   28|       getElement: [Pair] Pair
-#   28|         getKey: [SymbolLiteral] :foo
-#   28|         getValue: [IntegerLiteral] 0
-#   28|       getElement: [Pair] Pair
-#   28|         getKey: [SymbolLiteral] :bar
-#   28|         getValue: [IntegerLiteral] 1
+#   28|       getDesugared: [MethodCall] call to []
+#   28|         getReceiver: [ConstantReadAccess] Hash
+#   28|         getArgument: [Pair] Pair
+#   28|           getKey: [SymbolLiteral] :foo
+#   28|           getValue: [IntegerLiteral] 0
+#   28|         getArgument: [Pair] Pair
+#   28|           getKey: [SymbolLiteral] :bar
+#   28|           getValue: [IntegerLiteral] 1
 #   36| [AssignAddExpr] ... += ...
 #   36|   getDesugared: [AssignExpr] ... = ...
 #   36|     getAnOperand/getLeftOperand: [LocalVariableAccess] x
@@ -624,6 +673,15 @@ operations/operations.rb:
 #   29|   getDesugared: [MethodCall] call to []
 #   29|     getReceiver: [ConstantReadAccess] Array
 #   29|     getArgument: [IntegerLiteral] 2
+#   29| [HashLiteral] {...}
+#   29|   getDesugared: [MethodCall] call to []
+#   29|     getReceiver: [ConstantReadAccess] Hash
+#   29|     getArgument: [Pair] Pair
+#   29|       getKey: [SymbolLiteral] :b
+#   29|       getValue: [IntegerLiteral] 4
+#   29|     getArgument: [Pair] Pair
+#   29|       getKey: [SymbolLiteral] :c
+#   29|       getValue: [IntegerLiteral] 5
 #   69| [AssignAddExpr] ... += ...
 #   69|   getDesugared: [AssignExpr] ... = ...
 #   69|     getAnOperand/getLeftOperand: [LocalVariableAccess] x
@@ -721,6 +779,9 @@ operations/operations.rb:
 #   96|       getAnOperand/getLeftOperand/getReceiver: [GlobalVariableAccess] $global_var
 #   96|       getAnOperand/getArgument/getRightOperand: [IntegerLiteral] 6
 params/params.rb:
+#    8| [HashLiteral] {...}
+#    8|   getDesugared: [MethodCall] call to []
+#    8|     getReceiver: [ConstantReadAccess] Hash
 #   21| [ArrayLiteral] [...]
 #   21|   getDesugared: [MethodCall] call to []
 #   21|     getReceiver: [ConstantReadAccess] Array

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

@@ -11,6 +11,8 @@ hashSplatExpr
 | calls.rb:274:5:274:9 | ** ... | calls.rb:274:7:274:9 | call to bar |
 | calls.rb:275:5:275:12 | ** ... | calls.rb:275:7:275:12 | call to bar |
 keywordArguments
+| calls.rb:249:3:249:12 | Pair | calls.rb:249:3:249:5 | call to foo | calls.rb:249:10:249:12 | call to bar |
+| 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 |
 keywordArgumentsByKeyword

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

@@ -22,6 +22,8 @@ callsWithArguments
 | calls.rb:85:1:85:12 | ... + ... | + | 0 | calls.rb:85:7:85:12 | call to bar |
 | calls.rb:234:1:234:8 | ...[...] | [] | 0 | calls.rb:234:5:234:7 | call to bar |
 | calls.rb:235:1:235:14 | ...[...] | [] | 0 | calls.rb:235:8:235:13 | call to bar |
+| calls.rb:249:1:249:32 | call to [] | [] | 0 | calls.rb:249:3:249:12 | Pair |
+| calls.rb:249:1:249:32 | call to [] | [] | 1 | calls.rb:249:15:249:30 | Pair |
 | calls.rb:266:1:266:9 | call to foo | foo | 0 | calls.rb:266:5:266:8 | &... |
 | calls.rb:267:1:267:12 | call to foo | foo | 0 | calls.rb:267:5:267:11 | &... |
 | calls.rb:270:1:270:9 | call to foo | foo | 0 | calls.rb:270:5:270:8 | * ... |
@@ -248,6 +250,7 @@ callsWithReceiver
 | calls.rb:245:6:245:8 | call to bar | calls.rb:245:6:245:8 | self |
 | calls.rb:246:1:246:6 | call to foo | calls.rb:246:1:246:1 | X |
 | calls.rb:246:9:246:14 | call to bar | calls.rb:246:9:246:9 | X |
+| calls.rb:249:1:249:32 | call to [] | calls.rb:249:1:249:32 | Hash |
 | calls.rb:249:3:249:5 | call to foo | calls.rb:249:3:249:5 | self |
 | calls.rb:249:10:249:12 | call to bar | calls.rb:249:10:249:12 | self |
 | calls.rb:249:15:249:20 | call to foo | calls.rb:249:15:249:15 | X |

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

@@ -2434,11 +2434,14 @@ cfg.rb:
 #-----|  -> map2
 
 #   97| map1
-#-----|  -> a
+#-----|  -> Hash
 
-#   97| {...}
+#   97| call to []
 #-----|  -> ... = ...
 
+#   97| Hash
+#-----|  -> a
+
 #   97| Pair
 #-----|  -> c
 
@@ -2473,7 +2476,7 @@ cfg.rb:
 #-----|  -> f
 
 #   97| Pair
-#-----|  -> {...}
+#-----|  -> call to []
 
 #   97| "f"
 #-----|  -> Pair
@@ -2485,11 +2488,14 @@ cfg.rb:
 #-----|  -> parameters
 
 #   98| map2
-#-----|  -> map1
+#-----|  -> Hash
 
-#   98| {...}
+#   98| call to []
 #-----|  -> ... = ...
 
+#   98| Hash
+#-----|  -> map1
+
 #   98| ** ...
 #-----|  -> x
 
@@ -2512,7 +2518,7 @@ cfg.rb:
 #-----|  -> "y"
 
 #   98| ** ...
-#-----|  -> {...}
+#-----|  -> call to []
 
 #   98| map1
 #-----|  -> ** ...

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

@@ -148,6 +148,12 @@ positionalArguments
 | cfg.rb:90:10:90:26 | call to [] | cfg.rb:90:21:90:25 | 3.4e5 |
 | cfg.rb:91:6:91:10 | ... > ... | cfg.rb:91:10:91:10 | 3 |
 | cfg.rb:92:3:92:8 | call to puts | cfg.rb:92:8:92:8 | x |
+| cfg.rb:97:8:97:39 | call to [] | cfg.rb:97:10:97:19 | Pair |
+| cfg.rb:97:8:97:39 | call to [] | cfg.rb:97:22:97:29 | Pair |
+| cfg.rb:97:8:97:39 | call to [] | cfg.rb:97:32:97:37 | Pair |
+| cfg.rb:98:8:98:36 | call to [] | cfg.rb:98:10:98:15 | ** ... |
+| cfg.rb:98:8:98:36 | call to [] | cfg.rb:98:18:98:27 | Pair |
+| cfg.rb:98:8:98:36 | call to [] | cfg.rb:98:30:98:35 | ** ... |
 | cfg.rb:102:3:102:12 | call to puts | cfg.rb:102:8:102:12 | value |
 | cfg.rb:103:10:103:20 | ...[...] | cfg.rb:103:17:103:19 | key |
 | cfg.rb:108:1:108:12 | call to puts | cfg.rb:108:6:108:12 | ( ... ) |
@@ -344,3 +350,4 @@ positionalArguments
 keywordArguments
 | cfg.html.erb:6:9:6:58 | call to stylesheet_link_tag | media | cfg.html.erb:6:54:6:58 | "all" |
 | cfg.html.erb:12:11:12:33 | call to link_to | id | cfg.html.erb:12:31:12:33 | "a" |
+| cfg.rb:97:8:97:39 | call to [] | e | cfg.rb:97:35:97:37 | "f" |