From eebbc7e505471625cb9b1e86fbb7e454d1b16ad9 Mon Sep 17 00:00:00 2001
From: Arthur Baars <aibaars@github.com>
Date: Fri, 26 Mar 2021 16:00:24 +0100
Subject: [PATCH 01/14] AST: rename Class/Module to
 ClassDefinition/ModuleDefinition

---
 ql/src/codeql_ruby/ast/Module.qll             | 24 +++----
 .../internal/ControlFlowGraphImpl.qll         |  8 +--
 ql/test/library-tests/ast/Ast.expected        | 72 +++++++++----------
 .../ast/constants/constants.expected          | 14 ++--
 .../ast/modules/classes.expected              | 26 +++----
 ql/test/library-tests/ast/modules/classes.ql  | 22 +++---
 .../ast/modules/module_base.expected          | 42 +++++------
 .../library-tests/ast/modules/module_base.ql  |  4 +-
 .../ast/modules/modules.expected              | 28 ++++----
 ql/test/library-tests/ast/modules/modules.ql  | 14 ++--
 .../ast/modules/singleton_classes.expected    |  2 +-
 11 files changed, 131 insertions(+), 125 deletions(-)

diff --git a/ql/src/codeql_ruby/ast/Module.qll b/ql/src/codeql_ruby/ast/Module.qll
index 0eff778dbb8..6279a7e8a15 100644
--- a/ql/src/codeql_ruby/ast/Module.qll
+++ b/ql/src/codeql_ruby/ast/Module.qll
@@ -14,16 +14,16 @@ class ModuleBase extends BodyStmt, TModuleBase {
   MethodBase getMethod(string name) { result = this.getAMethod() and result.getName() = name }
 
   /** Gets a class defined in this module/class. */
-  Class getAClass() { result = this.getAStmt() }
+  ClassDefinition getAClass() { result = this.getAStmt() }
 
   /** Gets the class named `name` in this module/class, if any. */
-  Class getClass(string name) { result = this.getAClass() and result.getName() = name }
+  ClassDefinition getClass(string name) { result = this.getAClass() and result.getName() = name }
 
   /** Gets a module defined in this module/class. */
-  Module getAModule() { result = this.getAStmt() }
+  ModuleDefinition getAModule() { result = this.getAStmt() }
 
   /** Gets the module named `name` in this module/class, if any. */
-  Module getModule(string name) { result = this.getAModule() and result.getName() = name }
+  ModuleDefinition getModule(string name) { result = this.getAModule() and result.getName() = name }
 }
 
 /**
@@ -150,12 +150,12 @@ class Namespace extends ModuleBase, ConstantWriteAccess, TNamespace {
  * end
  * ```
  */
-class Class extends Namespace, TClass {
+class ClassDefinition extends Namespace, TClass {
   private Generated::Class g;
 
-  Class() { this = TClass(g) }
+  ClassDefinition() { this = TClass(g) }
 
-  final override string getAPrimaryQlClass() { result = "Class" }
+  final override string getAPrimaryQlClass() { result = "ClassDefinition" }
 
   /**
    * Gets the `Expr` used as the superclass in the class definition, if any.
@@ -214,7 +214,7 @@ class SingletonClass extends ModuleBase, TSingletonClass {
 
   SingletonClass() { this = TSingletonClass(g) }
 
-  final override string getAPrimaryQlClass() { result = "Class" }
+  final override string getAPrimaryQlClass() { result = "ClassDefinition" }
 
   /**
    * Gets the expression resulting in the object on which the singleton class
@@ -249,7 +249,7 @@ class SingletonClass extends ModuleBase, TSingletonClass {
  * N.B. this class represents a single instance of a module definition. In the
  * following example, classes `Bar` and `Baz` are both defined in the module
  * `Foo`, but in two syntactically distinct definitions, meaning that there
- * will be two instances of `Module` in the database.
+ * will be two instances of `ModuleDefinition` in the database.
  *
  * ```rb
  * module Foo
@@ -261,12 +261,12 @@ class SingletonClass extends ModuleBase, TSingletonClass {
  * end
  * ```
  */
-class Module extends Namespace, TModule {
+class ModuleDefinition extends Namespace, TModule {
   private Generated::Module g;
 
-  Module() { this = TModule(g) }
+  ModuleDefinition() { this = TModule(g) }
 
-  final override string getAPrimaryQlClass() { result = "Module" }
+  final override string getAPrimaryQlClass() { result = "ModuleDefinition" }
 
   final override string getName() {
     result = g.getName().(Generated::Token).getValue() or
diff --git a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll
index 15097a58d85..3f54a5d49a0 100644
--- a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll
+++ b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll
@@ -598,7 +598,7 @@ module Trees {
 
   private class CharacterTree extends LeafTree, CharacterLiteral { }
 
-  private class ClassTree extends BodyStmtPreOrderTree, Class {
+  private class ClassTree extends BodyStmtPreOrderTree, ClassDefinition {
     /** Gets the `i`th child in the body of this block. */
     final override AstNode getBodyChild(int i, boolean rescuable) {
       result = this.getScopeExpr() and i = 0 and rescuable = false
@@ -673,8 +673,8 @@ module Trees {
 
   private class ConstantAccessTree extends PostOrderTree, ConstantAccess {
     ConstantAccessTree() {
-      not this instanceof Class and
-      not this instanceof Module
+      not this instanceof ClassDefinition and
+      not this instanceof ModuleDefinition
     }
 
     final override predicate propagatesAbnormal(AstNode child) { child = this.getScopeExpr() }
@@ -934,7 +934,7 @@ module Trees {
     }
   }
 
-  private class ModuleTree extends BodyStmtPreOrderTree, Module {
+  private class ModuleTree extends BodyStmtPreOrderTree, ModuleDefinition {
     /** Gets the `i`th child in the body of this block. */
     final override AstNode getBodyChild(int i, boolean rescuable) {
       result = this.getScopeExpr() and i = 0 and rescuable = false
diff --git a/ql/test/library-tests/ast/Ast.expected b/ql/test/library-tests/ast/Ast.expected
index 5c411c581b7..1085019743b 100644
--- a/ql/test/library-tests/ast/Ast.expected
+++ b/ql/test/library-tests/ast/Ast.expected
@@ -128,19 +128,19 @@ calls/calls.rb:
 #  111|       getBody: [StmtSequence] then ...
 #  112|         getStmt: [MethodCall] call to baz
 #  112|           getReceiver: [ConstantReadAccess] X
-#  116|   getStmt: [Class] MyClass
+#  116|   getStmt: [ClassDefinition] MyClass
 #  117|     getStmt: [MethodCall] call to foo
 #  118|     getStmt: [MethodCall] call to bar
 #  118|       getReceiver: [ConstantReadAccess] X
-#  122|   getStmt: [Class] MyClass
+#  122|   getStmt: [ClassDefinition] MyClass
 #  122|     getSuperclassExpr: [MethodCall] call to foo
-#  124|   getStmt: [Class] MyClass2
+#  124|   getStmt: [ClassDefinition] MyClass2
 #  124|     getSuperclassExpr: [MethodCall] call to foo
 #  124|       getReceiver: [ConstantReadAccess] X
-#  128|   getStmt: [Class] class << ...
+#  128|   getStmt: [ClassDefinition] class << ...
 #  128|     getValue: [MethodCall] call to foo
 #  129|     getStmt: [MethodCall] call to bar
-#  131|   getStmt: [Class] class << ...
+#  131|   getStmt: [ClassDefinition] class << ...
 #  131|     getValue: [MethodCall] call to foo
 #  131|       getReceiver: [ConstantReadAccess] X
 #  132|     getStmt: [MethodCall] call to bar
@@ -172,7 +172,7 @@ calls/calls.rb:
 #  156|       getDefiningAccess: [LocalVariableAccess] param
 #  156|       getDefaultValue: [MethodCall] call to foo
 #  156|         getReceiver: [ConstantReadAccess] X
-#  160|   getStmt: [Module] SomeModule
+#  160|   getStmt: [ModuleDefinition] SomeModule
 #  161|     getStmt: [MethodCall] call to foo
 #  162|     getStmt: [MethodCall] call to bar
 #  162|       getReceiver: [ConstantReadAccess] X
@@ -376,7 +376,7 @@ calls/calls.rb:
 #  279|       getKey: [SymbolLiteral] :blah
 #  279|       getValue: [MethodCall] call to bar
 #  279|         getReceiver: [ConstantReadAccess] X
-#  284|   getStmt: [Class] MyClass
+#  284|   getStmt: [ClassDefinition] MyClass
 #  285|     getStmt: [Method] my_method
 #  286|       getStmt: [SuperCall] call to super
 #  287|       getStmt: [SuperCall] call to super
@@ -419,7 +419,7 @@ calls/calls.rb:
 #  293|           getStmt: [AddExpr] ... + ...
 #  293|             getAnOperand/getLeftOperand: [LocalVariableAccess] x
 #  293|             getAnOperand/getRightOperand: [IntegerLiteral] 200
-#  301|   getStmt: [Class] AnotherClass
+#  301|   getStmt: [ClassDefinition] AnotherClass
 #  302|     getStmt: [Method] another_method
 #  303|       getStmt: [MethodCall] call to super
 #  303|         getReceiver: [MethodCall] call to foo
@@ -522,16 +522,16 @@ control/cases.rb:
 #   21|         getStmt: [IntegerLiteral] 30
 modules/classes.rb:
 #    2| [Toplevel] classes.rb
-#    3|   getStmt: [Class] Foo
-#    7|   getStmt: [Class] Bar
+#    3|   getStmt: [ClassDefinition] Foo
+#    7|   getStmt: [ClassDefinition] Bar
 #    7|     getSuperclassExpr: [ConstantReadAccess] BaseClass
-#   11|   getStmt: [Class] Baz
+#   11|   getStmt: [ClassDefinition] Baz
 #   11|     getSuperclassExpr: [MethodCall] call to superclass_for
 #   11|       getArgument: [SymbolLiteral] :baz
-#   15|   getStmt: [Module] MyModule
-#   16|   getStmt: [Class] MyClass
+#   15|   getStmt: [ModuleDefinition] MyModule
+#   16|   getStmt: [ClassDefinition] MyClass
 #   16|     getScopeExpr: [ConstantReadAccess] MyModule
-#   20|   getStmt: [Class] Wibble
+#   20|   getStmt: [ClassDefinition] Wibble
 #   21|     getStmt: [Method] method_a
 #   22|       getStmt: [MethodCall] call to puts
 #   22|         getArgument: [StringLiteral] "a"
@@ -544,13 +544,13 @@ modules/classes.rb:
 #   30|     getStmt: [AssignExpr] ... = ...
 #   30|       getAnOperand/getLeftOperand: [GlobalVariableAccess] $global_var
 #   30|       getAnOperand/getRightOperand: [IntegerLiteral] 123
-#   32|     getStmt: [Class] ClassInWibble
-#   35|     getStmt: [Module] ModuleInWibble
+#   32|     getStmt: [ClassDefinition] ClassInWibble
+#   35|     getStmt: [ModuleDefinition] ModuleInWibble
 #   40|   getStmt: [AssignExpr] ... = ...
 #   40|     getAnOperand/getLeftOperand: [LocalVariableAccess] x
 #   40|     getAnOperand/getRightOperand: [StringLiteral] "hello"
 #   40|       getComponent: [StringTextComponent] hello
-#   41|   getStmt: [Class] class << ...
+#   41|   getStmt: [ClassDefinition] class << ...
 #   41|     getValue: [LocalVariableAccess] x
 #   42|     getStmt: [Method] length
 #   43|       getStmt: [MulExpr] ... * ...
@@ -564,7 +564,7 @@ modules/classes.rb:
 #   51|     getStmt: [AssignExpr] ... = ...
 #   51|       getAnOperand/getLeftOperand: [GlobalVariableAccess] $global_var2
 #   51|       getAnOperand/getRightOperand: [IntegerLiteral] 456
-#   55|   getStmt: [Class] MyClassInGlobalScope
+#   55|   getStmt: [ClassDefinition] MyClassInGlobalScope
 control/conditionals.rb:
 #    1| [Toplevel] conditionals.rb
 #    2|   getStmt: [AssignExpr] ... = ...
@@ -687,12 +687,12 @@ control/conditionals.rb:
 #   69|       getStmt: [LocalVariableAccess] c
 constants/constants.rb:
 #    1| [Toplevel] constants.rb
-#    1|   getStmt: [Module] ModuleA
-#    2|     getStmt: [Class] ClassA
-#    5|     getStmt: [Module] ModuleB
-#    6|       getStmt: [Class] ClassB
+#    1|   getStmt: [ModuleDefinition] ModuleA
+#    2|     getStmt: [ClassDefinition] ClassA
+#    5|     getStmt: [ModuleDefinition] ModuleB
+#    6|       getStmt: [ClassDefinition] ClassB
 #    6|         getSuperclassExpr: [ConstantReadAccess] Base
-#    9|       getStmt: [Class] ClassC
+#    9|       getStmt: [ClassDefinition] ClassC
 #    9|         getSuperclassExpr: [ConstantReadAccess] Z
 #    9|           getScopeExpr: [ConstantReadAccess] Y
 #    9|             getScopeExpr: [ConstantReadAccess] X
@@ -725,9 +725,9 @@ constants/constants.rb:
 #   25|     getStmt: [MethodCall] call to Array
 #   25|       getArgument: [StringLiteral] "foo"
 #   25|         getComponent: [StringTextComponent] foo
-#   28|   getStmt: [Class] ClassD
+#   28|   getStmt: [ClassDefinition] ClassD
 #   28|     getScopeExpr: [ConstantReadAccess] ModuleA
-#   31|   getStmt: [Module] ModuleC
+#   31|   getStmt: [ModuleDefinition] ModuleC
 #   31|     getScopeExpr: [ConstantReadAccess] ModuleA
 #   34|   getStmt: [AssignExpr] ... = ...
 #   34|     getAnOperand/getLeftOperand: [ConstantAssignment] MAX_SIZE
@@ -1322,10 +1322,10 @@ misc/misc.rb:
 #   10|       getComponent: [StringTextComponent] foo
 modules/modules.rb:
 #    1| [Toplevel] modules.rb
-#    1|   getStmt: [Module] Empty
-#    4|   getStmt: [Module] Foo
-#    5|     getStmt: [Module] Bar
-#    6|       getStmt: [Class] ClassInFooBar
+#    1|   getStmt: [ModuleDefinition] Empty
+#    4|   getStmt: [ModuleDefinition] Foo
+#    5|     getStmt: [ModuleDefinition] Bar
+#    6|       getStmt: [ClassDefinition] ClassInFooBar
 #    9|       getStmt: [Method] method_in_foo_bar
 #   12|       getStmt: [MethodCall] call to puts
 #   12|         getArgument: [StringLiteral] "module Foo::Bar"
@@ -1334,23 +1334,23 @@ modules/modules.rb:
 #   13|         getAnOperand/getLeftOperand: [GlobalVariableAccess] $global_var
 #   13|         getAnOperand/getRightOperand: [IntegerLiteral] 0
 #   16|     getStmt: [Method] method_in_foo
-#   19|     getStmt: [Class] ClassInFoo
+#   19|     getStmt: [ClassDefinition] ClassInFoo
 #   22|     getStmt: [MethodCall] call to puts
 #   22|       getArgument: [StringLiteral] "module Foo"
 #   22|         getComponent: [StringTextComponent] module Foo
 #   23|     getStmt: [AssignExpr] ... = ...
 #   23|       getAnOperand/getLeftOperand: [GlobalVariableAccess] $global_var
 #   23|       getAnOperand/getRightOperand: [IntegerLiteral] 1
-#   26|   getStmt: [Module] Foo
+#   26|   getStmt: [ModuleDefinition] Foo
 #   27|     getStmt: [Method] method_in_another_definition_of_foo
-#   30|     getStmt: [Class] ClassInAnotherDefinitionOfFoo
+#   30|     getStmt: [ClassDefinition] ClassInAnotherDefinitionOfFoo
 #   33|     getStmt: [MethodCall] call to puts
 #   33|       getArgument: [StringLiteral] "module Foo again"
 #   33|         getComponent: [StringTextComponent] module Foo again
 #   34|     getStmt: [AssignExpr] ... = ...
 #   34|       getAnOperand/getLeftOperand: [GlobalVariableAccess] $global_var
 #   34|       getAnOperand/getRightOperand: [IntegerLiteral] 2
-#   37|   getStmt: [Module] Bar
+#   37|   getStmt: [ModuleDefinition] Bar
 #   38|     getStmt: [Method] method_a
 #   41|     getStmt: [Method] method_b
 #   44|     getStmt: [MethodCall] call to puts
@@ -1359,9 +1359,9 @@ modules/modules.rb:
 #   45|     getStmt: [AssignExpr] ... = ...
 #   45|       getAnOperand/getLeftOperand: [GlobalVariableAccess] $global_var
 #   45|       getAnOperand/getRightOperand: [IntegerLiteral] 3
-#   48|   getStmt: [Module] Bar
+#   48|   getStmt: [ModuleDefinition] Bar
 #   48|     getScopeExpr: [ConstantReadAccess] Foo
-#   49|     getStmt: [Class] ClassInAnotherDefinitionOfFooBar
+#   49|     getStmt: [ClassDefinition] ClassInAnotherDefinitionOfFooBar
 #   52|     getStmt: [Method] method_in_another_definition_of_foo_bar
 #   55|     getStmt: [MethodCall] call to puts
 #   55|       getArgument: [StringLiteral] "module Foo::Bar again"
@@ -1369,7 +1369,7 @@ modules/modules.rb:
 #   56|     getStmt: [AssignExpr] ... = ...
 #   56|       getAnOperand/getLeftOperand: [GlobalVariableAccess] $global_var
 #   56|       getAnOperand/getRightOperand: [IntegerLiteral] 4
-#   60|   getStmt: [Module] MyModuleInGlobalScope
+#   60|   getStmt: [ModuleDefinition] MyModuleInGlobalScope
 operations/operations.rb:
 #    1| [Toplevel] operations.rb
 #    3|   getStmt: [AssignExpr] ... = ...
diff --git a/ql/test/library-tests/ast/constants/constants.expected b/ql/test/library-tests/ast/constants/constants.expected
index c29abf34181..60362f161f4 100644
--- a/ql/test/library-tests/ast/constants/constants.expected
+++ b/ql/test/library-tests/ast/constants/constants.expected
@@ -1,9 +1,9 @@
-| constants.rb:1:1:12:3 | ModuleA | write | ModuleA | Module |
-| constants.rb:2:5:3:7 | ClassA | write | ClassA | Class |
-| constants.rb:5:5:11:7 | ModuleB | write | ModuleB | Module |
-| constants.rb:6:9:7:11 | ClassB | write | ClassB | Class |
+| constants.rb:1:1:12:3 | ModuleA | write | ModuleA | ModuleDefinition |
+| constants.rb:2:5:3:7 | ClassA | write | ClassA | ClassDefinition |
+| constants.rb:5:5:11:7 | ModuleB | write | ModuleB | ModuleDefinition |
+| constants.rb:6:9:7:11 | ClassB | write | ClassB | ClassDefinition |
 | constants.rb:6:24:6:27 | Base | read | Base | ConstantReadAccess |
-| constants.rb:9:9:10:11 | ClassC | write | ClassC | Class |
+| constants.rb:9:9:10:11 | ClassC | write | ClassC | ClassDefinition |
 | constants.rb:9:24:9:24 | X | read | X | ConstantReadAccess |
 | constants.rb:9:24:9:27 | Y | read | Y | ConstantReadAccess |
 | constants.rb:9:24:9:30 | Z | read | Z | ConstantReadAccess |
@@ -11,9 +11,9 @@
 | constants.rb:17:5:17:9 | Names | write | Names | ConstantAssignment |
 | constants.rb:19:5:19:9 | Names | read | Names | ConstantReadAccess |
 | constants.rb:20:18:20:25 | GREETING | read | GREETING | ConstantReadAccess |
-| constants.rb:28:1:29:3 | ClassD | write | ClassD | Class |
+| constants.rb:28:1:29:3 | ClassD | write | ClassD | ClassDefinition |
 | constants.rb:28:7:28:13 | ModuleA | read | ModuleA | ConstantReadAccess |
-| constants.rb:31:1:32:3 | ModuleC | write | ModuleC | Module |
+| constants.rb:31:1:32:3 | ModuleC | write | ModuleC | ModuleDefinition |
 | constants.rb:31:8:31:14 | ModuleA | read | ModuleA | ConstantReadAccess |
 | constants.rb:34:1:34:7 | ModuleA | read | ModuleA | ConstantReadAccess |
 | constants.rb:34:1:34:16 | ModuleB | read | ModuleB | ConstantReadAccess |
diff --git a/ql/test/library-tests/ast/modules/classes.expected b/ql/test/library-tests/ast/modules/classes.expected
index 9eb7604f0df..5d9b7dfae33 100644
--- a/ql/test/library-tests/ast/modules/classes.expected
+++ b/ql/test/library-tests/ast/modules/classes.expected
@@ -1,15 +1,15 @@
 classes
-| classes.rb:3:1:4:3 | Foo | Class | Foo |
-| classes.rb:7:1:8:3 | Bar | Class | Bar |
-| classes.rb:11:1:12:3 | Baz | Class | Baz |
-| classes.rb:16:1:17:3 | MyClass | Class | MyClass |
-| classes.rb:20:1:37:3 | Wibble | Class | Wibble |
-| classes.rb:32:3:33:5 | ClassInWibble | Class | ClassInWibble |
-| classes.rb:55:1:56:3 | MyClassInGlobalScope | Class | MyClassInGlobalScope |
-| modules.rb:6:5:7:7 | ClassInFooBar | Class | ClassInFooBar |
-| modules.rb:19:3:20:5 | ClassInFoo | Class | ClassInFoo |
-| modules.rb:30:3:31:5 | ClassInAnotherDefinitionOfFoo | Class | ClassInAnotherDefinitionOfFoo |
-| modules.rb:49:3:50:5 | ClassInAnotherDefinitionOfFooBar | Class | ClassInAnotherDefinitionOfFooBar |
+| classes.rb:3:1:4:3 | Foo | ClassDefinition | Foo |
+| classes.rb:7:1:8:3 | Bar | ClassDefinition | Bar |
+| classes.rb:11:1:12:3 | Baz | ClassDefinition | Baz |
+| classes.rb:16:1:17:3 | MyClass | ClassDefinition | MyClass |
+| classes.rb:20:1:37:3 | Wibble | ClassDefinition | Wibble |
+| classes.rb:32:3:33:5 | ClassInWibble | ClassDefinition | ClassInWibble |
+| classes.rb:55:1:56:3 | MyClassInGlobalScope | ClassDefinition | MyClassInGlobalScope |
+| modules.rb:6:5:7:7 | ClassInFooBar | ClassDefinition | ClassInFooBar |
+| modules.rb:19:3:20:5 | ClassInFoo | ClassDefinition | ClassInFoo |
+| modules.rb:30:3:31:5 | ClassInAnotherDefinitionOfFoo | ClassDefinition | ClassInAnotherDefinitionOfFoo |
+| modules.rb:49:3:50:5 | ClassInAnotherDefinitionOfFooBar | ClassDefinition | ClassInAnotherDefinitionOfFooBar |
 classesWithNameScopeExprs
 | classes.rb:16:1:17:3 | MyClass | classes.rb:16:7:16:14 | MyModule |
 classesWithGlobalNameScopeExprs
@@ -19,8 +19,8 @@ exprsInClasses
 | classes.rb:20:1:37:3 | Wibble | 1 | classes.rb:25:3:27:5 | method_b | Method |
 | classes.rb:20:1:37:3 | Wibble | 2 | classes.rb:29:3:29:20 | call to some_method_call | MethodCall |
 | classes.rb:20:1:37:3 | Wibble | 3 | classes.rb:30:3:30:19 | ... = ... | AssignExpr |
-| classes.rb:20:1:37:3 | Wibble | 4 | classes.rb:32:3:33:5 | ClassInWibble | Class |
-| classes.rb:20:1:37:3 | Wibble | 5 | classes.rb:35:3:36:5 | ModuleInWibble | Module |
+| classes.rb:20:1:37:3 | Wibble | 4 | classes.rb:32:3:33:5 | ClassInWibble | ClassDefinition |
+| classes.rb:20:1:37:3 | Wibble | 5 | classes.rb:35:3:36:5 | ModuleInWibble | ModuleDefinition |
 methodsInClasses
 | classes.rb:20:1:37:3 | Wibble | classes.rb:21:3:23:5 | method_a | method_a |
 | classes.rb:20:1:37:3 | Wibble | classes.rb:25:3:27:5 | method_b | method_b |
diff --git a/ql/test/library-tests/ast/modules/classes.ql b/ql/test/library-tests/ast/modules/classes.ql
index 52d6ed21e51..ce347830b06 100644
--- a/ql/test/library-tests/ast/modules/classes.ql
+++ b/ql/test/library-tests/ast/modules/classes.ql
@@ -1,21 +1,27 @@
 import ruby
 
-query predicate classes(Class c, string pClass, string name) {
+query predicate classes(ClassDefinition c, string pClass, string name) {
   pClass = c.getAPrimaryQlClass() and name = c.getName()
 }
 
-query predicate classesWithNameScopeExprs(Class c, Expr se) { se = c.getScopeExpr() }
+query predicate classesWithNameScopeExprs(ClassDefinition c, Expr se) { se = c.getScopeExpr() }
 
-query predicate classesWithGlobalNameScopeExprs(Class c) { c.hasGlobalScope() }
+query predicate classesWithGlobalNameScopeExprs(ClassDefinition c) { c.hasGlobalScope() }
 
-query predicate exprsInClasses(Class c, int i, Expr e, string eClass) {
+query predicate exprsInClasses(ClassDefinition c, int i, Expr e, string eClass) {
   e = c.getStmt(i) and eClass = e.getAPrimaryQlClass()
 }
 
-query predicate methodsInClasses(Class c, Method m, string name) { m = c.getMethod(name) }
+query predicate methodsInClasses(ClassDefinition c, Method m, string name) { m = c.getMethod(name) }
 
-query predicate classesInClasses(Class c, Class child, string name) { child = c.getClass(name) }
+query predicate classesInClasses(ClassDefinition c, ClassDefinition child, string name) {
+  child = c.getClass(name)
+}
 
-query predicate modulesInClasses(Class c, Module m, string name) { m = c.getModule(name) }
+query predicate modulesInClasses(ClassDefinition c, ModuleDefinition m, string name) {
+  m = c.getModule(name)
+}
 
-query predicate classesWithASuperclass(Class c, Expr scExpr) { scExpr = c.getSuperclassExpr() }
+query predicate classesWithASuperclass(ClassDefinition c, Expr scExpr) {
+  scExpr = c.getSuperclassExpr()
+}
diff --git a/ql/test/library-tests/ast/modules/module_base.expected b/ql/test/library-tests/ast/modules/module_base.expected
index 1e26e369d5f..99d40f49e22 100644
--- a/ql/test/library-tests/ast/modules/module_base.expected
+++ b/ql/test/library-tests/ast/modules/module_base.expected
@@ -1,27 +1,27 @@
 moduleBases
 | classes.rb:2:1:56:3 | classes.rb | Toplevel |
-| classes.rb:3:1:4:3 | Foo | Class |
-| classes.rb:7:1:8:3 | Bar | Class |
-| classes.rb:11:1:12:3 | Baz | Class |
-| classes.rb:15:1:15:20 | MyModule | Module |
-| classes.rb:16:1:17:3 | MyClass | Class |
-| classes.rb:20:1:37:3 | Wibble | Class |
-| classes.rb:32:3:33:5 | ClassInWibble | Class |
-| classes.rb:35:3:36:5 | ModuleInWibble | Module |
-| classes.rb:41:1:52:3 | class << ... | Class |
-| classes.rb:55:1:56:3 | MyClassInGlobalScope | Class |
-| modules.rb:1:1:2:3 | Empty | Module |
+| classes.rb:3:1:4:3 | Foo | ClassDefinition |
+| classes.rb:7:1:8:3 | Bar | ClassDefinition |
+| classes.rb:11:1:12:3 | Baz | ClassDefinition |
+| classes.rb:15:1:15:20 | MyModule | ModuleDefinition |
+| classes.rb:16:1:17:3 | MyClass | ClassDefinition |
+| classes.rb:20:1:37:3 | Wibble | ClassDefinition |
+| classes.rb:32:3:33:5 | ClassInWibble | ClassDefinition |
+| classes.rb:35:3:36:5 | ModuleInWibble | ModuleDefinition |
+| classes.rb:41:1:52:3 | class << ... | ClassDefinition |
+| classes.rb:55:1:56:3 | MyClassInGlobalScope | ClassDefinition |
+| modules.rb:1:1:2:3 | Empty | ModuleDefinition |
 | modules.rb:1:1:61:3 | modules.rb | Toplevel |
-| modules.rb:4:1:24:3 | Foo | Module |
-| modules.rb:5:3:14:5 | Bar | Module |
-| modules.rb:6:5:7:7 | ClassInFooBar | Class |
-| modules.rb:19:3:20:5 | ClassInFoo | Class |
-| modules.rb:26:1:35:3 | Foo | Module |
-| modules.rb:30:3:31:5 | ClassInAnotherDefinitionOfFoo | Class |
-| modules.rb:37:1:46:3 | Bar | Module |
-| modules.rb:48:1:57:3 | Bar | Module |
-| modules.rb:49:3:50:5 | ClassInAnotherDefinitionOfFooBar | Class |
-| modules.rb:60:1:61:3 | MyModuleInGlobalScope | Module |
+| modules.rb:4:1:24:3 | Foo | ModuleDefinition |
+| modules.rb:5:3:14:5 | Bar | ModuleDefinition |
+| modules.rb:6:5:7:7 | ClassInFooBar | ClassDefinition |
+| modules.rb:19:3:20:5 | ClassInFoo | ClassDefinition |
+| modules.rb:26:1:35:3 | Foo | ModuleDefinition |
+| modules.rb:30:3:31:5 | ClassInAnotherDefinitionOfFoo | ClassDefinition |
+| modules.rb:37:1:46:3 | Bar | ModuleDefinition |
+| modules.rb:48:1:57:3 | Bar | ModuleDefinition |
+| modules.rb:49:3:50:5 | ClassInAnotherDefinitionOfFooBar | ClassDefinition |
+| modules.rb:60:1:61:3 | MyModuleInGlobalScope | ModuleDefinition |
 | toplevel.rb:1:1:5:23 | toplevel.rb | Toplevel |
 moduleBaseClasses
 | classes.rb:2:1:56:3 | classes.rb | classes.rb:3:1:4:3 | Foo |
diff --git a/ql/test/library-tests/ast/modules/module_base.ql b/ql/test/library-tests/ast/modules/module_base.ql
index e4e72ed1c69..4b41e4ad03b 100644
--- a/ql/test/library-tests/ast/modules/module_base.ql
+++ b/ql/test/library-tests/ast/modules/module_base.ql
@@ -2,8 +2,8 @@ import ruby
 
 query predicate moduleBases(ModuleBase mb, string pClass) { pClass = mb.getAPrimaryQlClass() }
 
-query predicate moduleBaseClasses(ModuleBase mb, Class c) { c = mb.getAClass() }
+query predicate moduleBaseClasses(ModuleBase mb, ClassDefinition c) { c = mb.getAClass() }
 
 query predicate moduleBaseMethods(ModuleBase mb, Method m) { m = mb.getAMethod() }
 
-query predicate moduleBaseModules(ModuleBase mb, Module m) { m = mb.getAModule() }
+query predicate moduleBaseModules(ModuleBase mb, ModuleDefinition m) { m = mb.getAModule() }
diff --git a/ql/test/library-tests/ast/modules/modules.expected b/ql/test/library-tests/ast/modules/modules.expected
index b21ae05c813..1cb4c50c9e7 100644
--- a/ql/test/library-tests/ast/modules/modules.expected
+++ b/ql/test/library-tests/ast/modules/modules.expected
@@ -1,36 +1,36 @@
 modules
-| classes.rb:15:1:15:20 | MyModule | Module | MyModule |
-| classes.rb:35:3:36:5 | ModuleInWibble | Module | ModuleInWibble |
-| modules.rb:1:1:2:3 | Empty | Module | Empty |
-| modules.rb:4:1:24:3 | Foo | Module | Foo |
-| modules.rb:5:3:14:5 | Bar | Module | Bar |
-| modules.rb:26:1:35:3 | Foo | Module | Foo |
-| modules.rb:37:1:46:3 | Bar | Module | Bar |
-| modules.rb:48:1:57:3 | Bar | Module | Bar |
-| modules.rb:60:1:61:3 | MyModuleInGlobalScope | Module | MyModuleInGlobalScope |
+| classes.rb:15:1:15:20 | MyModule | ModuleDefinition | MyModule |
+| classes.rb:35:3:36:5 | ModuleInWibble | ModuleDefinition | ModuleInWibble |
+| modules.rb:1:1:2:3 | Empty | ModuleDefinition | Empty |
+| modules.rb:4:1:24:3 | Foo | ModuleDefinition | Foo |
+| modules.rb:5:3:14:5 | Bar | ModuleDefinition | Bar |
+| modules.rb:26:1:35:3 | Foo | ModuleDefinition | Foo |
+| modules.rb:37:1:46:3 | Bar | ModuleDefinition | Bar |
+| modules.rb:48:1:57:3 | Bar | ModuleDefinition | Bar |
+| modules.rb:60:1:61:3 | MyModuleInGlobalScope | ModuleDefinition | MyModuleInGlobalScope |
 modulesWithScopeExprs
 | modules.rb:48:1:57:3 | Bar | modules.rb:48:8:48:10 | Foo |
 modulesWithGlobalNameScopeExprs
 | modules.rb:60:1:61:3 | MyModuleInGlobalScope |
 exprsInModules
-| modules.rb:4:1:24:3 | Foo | 0 | modules.rb:5:3:14:5 | Bar | Module |
+| modules.rb:4:1:24:3 | Foo | 0 | modules.rb:5:3:14:5 | Bar | ModuleDefinition |
 | modules.rb:4:1:24:3 | Foo | 1 | modules.rb:16:3:17:5 | method_in_foo | Method |
-| modules.rb:4:1:24:3 | Foo | 2 | modules.rb:19:3:20:5 | ClassInFoo | Class |
+| modules.rb:4:1:24:3 | Foo | 2 | modules.rb:19:3:20:5 | ClassInFoo | ClassDefinition |
 | modules.rb:4:1:24:3 | Foo | 3 | modules.rb:22:3:22:19 | call to puts | MethodCall |
 | modules.rb:4:1:24:3 | Foo | 4 | modules.rb:23:3:23:17 | ... = ... | AssignExpr |
-| modules.rb:5:3:14:5 | Bar | 0 | modules.rb:6:5:7:7 | ClassInFooBar | Class |
+| modules.rb:5:3:14:5 | Bar | 0 | modules.rb:6:5:7:7 | ClassInFooBar | ClassDefinition |
 | modules.rb:5:3:14:5 | Bar | 1 | modules.rb:9:5:10:7 | method_in_foo_bar | Method |
 | modules.rb:5:3:14:5 | Bar | 2 | modules.rb:12:5:12:26 | call to puts | MethodCall |
 | modules.rb:5:3:14:5 | Bar | 3 | modules.rb:13:5:13:19 | ... = ... | AssignExpr |
 | modules.rb:26:1:35:3 | Foo | 0 | modules.rb:27:3:28:5 | method_in_another_definition_of_foo | Method |
-| modules.rb:26:1:35:3 | Foo | 1 | modules.rb:30:3:31:5 | ClassInAnotherDefinitionOfFoo | Class |
+| modules.rb:26:1:35:3 | Foo | 1 | modules.rb:30:3:31:5 | ClassInAnotherDefinitionOfFoo | ClassDefinition |
 | modules.rb:26:1:35:3 | Foo | 2 | modules.rb:33:3:33:25 | call to puts | MethodCall |
 | modules.rb:26:1:35:3 | Foo | 3 | modules.rb:34:3:34:17 | ... = ... | AssignExpr |
 | modules.rb:37:1:46:3 | Bar | 0 | modules.rb:38:3:39:5 | method_a | Method |
 | modules.rb:37:1:46:3 | Bar | 1 | modules.rb:41:3:42:5 | method_b | Method |
 | modules.rb:37:1:46:3 | Bar | 2 | modules.rb:44:3:44:19 | call to puts | MethodCall |
 | modules.rb:37:1:46:3 | Bar | 3 | modules.rb:45:3:45:17 | ... = ... | AssignExpr |
-| modules.rb:48:1:57:3 | Bar | 0 | modules.rb:49:3:50:5 | ClassInAnotherDefinitionOfFooBar | Class |
+| modules.rb:48:1:57:3 | Bar | 0 | modules.rb:49:3:50:5 | ClassInAnotherDefinitionOfFooBar | ClassDefinition |
 | modules.rb:48:1:57:3 | Bar | 1 | modules.rb:52:3:53:5 | method_in_another_definition_of_foo_bar | Method |
 | modules.rb:48:1:57:3 | Bar | 2 | modules.rb:55:3:55:30 | call to puts | MethodCall |
 | modules.rb:48:1:57:3 | Bar | 3 | modules.rb:56:3:56:17 | ... = ... | AssignExpr |
diff --git a/ql/test/library-tests/ast/modules/modules.ql b/ql/test/library-tests/ast/modules/modules.ql
index cce336c8b5c..d0942303fb5 100644
--- a/ql/test/library-tests/ast/modules/modules.ql
+++ b/ql/test/library-tests/ast/modules/modules.ql
@@ -1,25 +1,25 @@
 import ruby
 
-query predicate modules(Module m, string pClass, string name) {
+query predicate modules(ModuleDefinition m, string pClass, string name) {
   pClass = m.getAPrimaryQlClass() and name = m.getName()
 }
 
-query predicate modulesWithScopeExprs(Module m, Expr se) { se = m.getScopeExpr() }
+query predicate modulesWithScopeExprs(ModuleDefinition m, Expr se) { se = m.getScopeExpr() }
 
-query predicate modulesWithGlobalNameScopeExprs(Module m) { m.hasGlobalScope() }
+query predicate modulesWithGlobalNameScopeExprs(ModuleDefinition m) { m.hasGlobalScope() }
 
-query predicate exprsInModules(Module m, int i, Expr e, string eClass) {
+query predicate exprsInModules(ModuleDefinition m, int i, Expr e, string eClass) {
   e = m.getStmt(i) and eClass = e.getAPrimaryQlClass()
 }
 
-query predicate methodsInModules(Module mod, Method method, string name) {
+query predicate methodsInModules(ModuleDefinition mod, Method method, string name) {
   method = mod.getMethod(name)
 }
 
-query predicate classesInModules(Module mod, Class klass, string name) {
+query predicate classesInModules(ModuleDefinition mod, ClassDefinition klass, string name) {
   klass = mod.getClass(name)
 }
 
-query predicate modulesInModules(Module mod, Module child, string name) {
+query predicate modulesInModules(ModuleDefinition mod, ModuleDefinition child, string name) {
   child = mod.getModule(name)
 }
diff --git a/ql/test/library-tests/ast/modules/singleton_classes.expected b/ql/test/library-tests/ast/modules/singleton_classes.expected
index 845afe551e7..c5746395fba 100644
--- a/ql/test/library-tests/ast/modules/singleton_classes.expected
+++ b/ql/test/library-tests/ast/modules/singleton_classes.expected
@@ -1,5 +1,5 @@
 singletonClasses
-| classes.rb:41:1:52:3 | class << ... | Class | classes.rb:41:10:41:10 | x |
+| classes.rb:41:1:52:3 | class << ... | ClassDefinition | classes.rb:41:10:41:10 | x |
 exprsInSingletonClasses
 | classes.rb:41:1:52:3 | class << ... | 0 | classes.rb:42:3:44:5 | length | Method |
 | classes.rb:41:1:52:3 | class << ... | 1 | classes.rb:46:3:48:5 | wibble | Method |

From ea9afcd4e11b58088ef3ffde134b899ed08c5cec Mon Sep 17 00:00:00 2001
From: Arthur Baars <aibaars@github.com>
Date: Fri, 26 Mar 2021 16:01:11 +0100
Subject: [PATCH 02/14] AST: make some classes instance of Scope

---
 ql/src/codeql_ruby/ast/Method.qll | 2 +-
 ql/src/codeql_ruby/ast/Module.qll | 2 +-
 ql/src/codeql_ruby/ast/Scope.qll  | 3 +++
 3 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/ql/src/codeql_ruby/ast/Method.qll b/ql/src/codeql_ruby/ast/Method.qll
index 7c205f174ab..648392b28f0 100644
--- a/ql/src/codeql_ruby/ast/Method.qll
+++ b/ql/src/codeql_ruby/ast/Method.qll
@@ -4,7 +4,7 @@ private import internal.AST
 private import internal.TreeSitter
 
 /** A callable. */
-class Callable extends Expr, TCallable {
+class Callable extends Expr, Scope, TCallable {
   /** Gets the number of parameters of this callable. */
   final int getNumberOfParameters() { result = count(this.getAParameter()) }
 
diff --git a/ql/src/codeql_ruby/ast/Module.qll b/ql/src/codeql_ruby/ast/Module.qll
index 6279a7e8a15..07a36091324 100644
--- a/ql/src/codeql_ruby/ast/Module.qll
+++ b/ql/src/codeql_ruby/ast/Module.qll
@@ -6,7 +6,7 @@ private import internal.TreeSitter
 /**
  * The base class for classes, singleton classes, and modules.
  */
-class ModuleBase extends BodyStmt, TModuleBase {
+class ModuleBase extends BodyStmt, Scope, TModuleBase {
   /** Gets a method defined in this module/class. */
   MethodBase getAMethod() { result = this.getAStmt() }
 
diff --git a/ql/src/codeql_ruby/ast/Scope.qll b/ql/src/codeql_ruby/ast/Scope.qll
index 36b23e880e5..2f4b941a56e 100644
--- a/ql/src/codeql_ruby/ast/Scope.qll
+++ b/ql/src/codeql_ruby/ast/Scope.qll
@@ -17,6 +17,9 @@ class Scope extends AstNode, TScopeType {
   /** Gets the scope in which this scope is nested, if any. */
   Scope getOuterScope() { toGenerated(result) = range.getOuterScope() }
 
+  /** Gets the scope in which this scope is nested, if any. */
+  AstNode getADescendant() { range = scopeOf(toGenerated(result)) }
+
   /** Gets a variable that is declared in this scope. */
   final Variable getAVariable() { result.getDeclaringScope() = this }
 

From 201c1e4b8106b3198e0fc9292d317010a5d45d78 Mon Sep 17 00:00:00 2001
From: Arthur Baars <aibaars@github.com>
Date: Tue, 30 Mar 2021 15:34:20 +0200
Subject: [PATCH 03/14] Basic module resolution

---
 ql/src/codeql_ruby/ast/Module.qll          |  42 ++++++++
 ql/src/codeql_ruby/ast/internal/Module.qll | 106 +++++++++++++++++++++
 2 files changed, 148 insertions(+)
 create mode 100644 ql/src/codeql_ruby/ast/internal/Module.qll

diff --git a/ql/src/codeql_ruby/ast/Module.qll b/ql/src/codeql_ruby/ast/Module.qll
index 07a36091324..1126ba6253e 100644
--- a/ql/src/codeql_ruby/ast/Module.qll
+++ b/ql/src/codeql_ruby/ast/Module.qll
@@ -1,8 +1,39 @@
 private import codeql_ruby.AST
 private import codeql_ruby.ast.Constant
 private import internal.AST
+private import internal.Module
 private import internal.TreeSitter
 
+/**
+ * A representation of a run-time `module` or `class` value.
+ */
+class Module extends TConstant {
+  Module() { this = TResolved(_, true) or this = TUnresolved(any(Namespace n)) }
+
+  string toString() {
+    this = TResolved(result, _)
+    or
+    exists(Namespace n | this = TUnresolved(n) and result = "...::" + n.toString())
+  }
+
+  Location getLocation() {
+    exists(Namespace n | this = TUnresolved(n) and result = n.getLocation())
+    or
+    result =
+      min(Namespace n, string qName, Location loc, int weight |
+        this = TResolved(qName, _) and
+        qName = constantDefinition(n) and
+        loc = n.getLocation() and
+        if exists(loc.getFile().getRelativePath()) then weight = 0 else weight = 1
+      |
+        loc
+        order by
+          weight, count(n.getAStmt()) desc, loc.getFile().getAbsolutePath(), loc.getStartLine(),
+          loc.getStartColumn()
+      )
+  }
+}
+
 /**
  * The base class for classes, singleton classes, and modules.
  */
@@ -24,6 +55,9 @@ class ModuleBase extends BodyStmt, Scope, TModuleBase {
 
   /** Gets the module named `name` in this module/class, if any. */
   ModuleDefinition getModule(string name) { result = this.getAModule() and result.getName() = name }
+
+  /** Gets the representation of the run-time value of this module or class. */
+  Module getModule() { none() }
 }
 
 /**
@@ -62,6 +96,8 @@ class Toplevel extends ModuleBase, TToplevel {
     pred = "getBeginBlock" and result = this.getBeginBlock(_)
   }
 
+  final override Module getModule() { result = TResolved("Object", true) }
+
   final override string toString() { result = g.getLocation().getFile().getBaseName() }
 }
 
@@ -132,6 +168,12 @@ class Namespace extends ModuleBase, ConstantWriteAccess, TNamespace {
    */
   override predicate hasGlobalScope() { none() }
 
+  final override Module getModule() {
+    result = any(string qName | qName = constantDefinition(this) | TResolved(qName, true))
+    or
+    result = TUnresolved(this)
+  }
+
   override AstNode getAChild(string pred) {
     result = ModuleBase.super.getAChild(pred) or
     result = ConstantWriteAccess.super.getAChild(pred)
diff --git a/ql/src/codeql_ruby/ast/internal/Module.qll b/ql/src/codeql_ruby/ast/internal/Module.qll
new file mode 100644
index 00000000000..d5e85d84258
--- /dev/null
+++ b/ql/src/codeql_ruby/ast/internal/Module.qll
@@ -0,0 +1,106 @@
+private import codeql.Locations
+private import codeql_ruby.ast.Constant
+private import codeql_ruby.ast.Module
+private import codeql_ruby.ast.Operation
+private import codeql_ruby.ast.Scope
+
+// Names of built-in modules and classes
+private string builtin() { result = ["Object", "Kernel", "BasicObject", "Class", "Module"] }
+
+newtype TConstant =
+  TResolved(string qName, boolean isModule) {
+    exists(ConstantWriteAccess n |
+      qName = builtin() and isModule = true
+      or
+      qName = constantDefinition(n) and
+      if n instanceof Namespace then isModule = true else isModule = false
+    )
+  } or
+  TUnresolved(ConstantWriteAccess n) { not exists(constantDefinition(n)) }
+
+private predicate isToplevel(ConstantAccess n) {
+  not exists(n.getScopeExpr()) and
+  (
+    n.hasGlobalScope()
+    or
+    exists(Scope x | x.getADescendant() = n and x.getEnclosingModule() instanceof Toplevel)
+  )
+}
+
+private string constantDefinition0(ConstantWriteAccess n) { result = qualifiedNameForConstant0(n) }
+
+/**
+ * Resolve a scope expression
+ */
+private string resolveScopeExpr0(ConstantReadAccess n) {
+  exists(string qname | qname = qualifiedNameForConstant0(n) |
+    qname = builtin() and result = qname
+    or
+    not qname = builtin() and
+    exists(ConstantWriteAccess def | qname = constantDefinition0(def) |
+      result = qname and def instanceof Namespace
+      or
+      result = resolveScopeExpr0(def.getParent().(Assignment).getRightOperand())
+    )
+  )
+}
+
+ModuleBase enclosing(ModuleBase m, int level) {
+  result = m and level = 0
+  or
+  result = enclosing(m.getOuterScope().getEnclosingModule(), level - 1)
+}
+
+private string resolveRelativeToEnclosing(ConstantAccess n, int i) {
+  not isToplevel(n) and
+  not exists(n.getScopeExpr()) and
+  exists(Scope s, ModuleBase enclosing |
+    n = s.getADescendant() and
+    enclosing = enclosing(s.getEnclosingModule(), i) and
+    (
+      result = constantDefinition0(enclosing) + "::" + n.getName()
+      or
+      enclosing instanceof Toplevel and result = n.getName()
+    )
+  )
+}
+
+private string qualifiedNameForConstant0(ConstantAccess n) {
+  isToplevel(n) and
+  result = n.getName()
+  or
+  result = resolveRelativeToEnclosing(n, 0)
+  or
+  result = resolveScopeExpr0(n.getScopeExpr()) + "::" + n.getName()
+}
+
+string constantDefinition(ConstantWriteAccess n) {
+  result = constantDefinition0(n)
+  or
+  result = resolveScopeExpr(n.getScopeExpr()) + "::" + n.getName()
+}
+
+private string resolveScopeExpr(ConstantReadAccess n) {
+  exists(string qname |
+    qname =
+      min(int i, string x |
+        (
+          x = qualifiedNameForConstant0(n) and i = 0
+          or
+          x = resolveRelativeToEnclosing(n, i)
+        ) and
+        (x = builtin() or x = constantDefinition0(_))
+      |
+        x order by i
+      )
+  |
+    qname = builtin() and result = qname
+    or
+    not qname = builtin() and
+    exists(ConstantWriteAccess def | qname = constantDefinition0(def) |
+      result = qname and def instanceof Namespace
+      or
+      result = resolveScopeExpr(def.getParent().(Assignment).getRightOperand())
+    )
+  )
+}

From b2c7185664652c221cff9da9dfdee6e02c4af2d7 Mon Sep 17 00:00:00 2001
From: Arthur Baars <aibaars@github.com>
Date: Tue, 30 Mar 2021 15:49:41 +0200
Subject: [PATCH 04/14] Add tests

---
 ql/test/library-tests/ast/Ast.expected        | 14 +++++++++
 .../ast/modules/classes.expected              |  6 ++++
 .../ast/modules/module_base.expected          | 30 ++++++++++++++-----
 .../ast/modules/modules.expected              | 20 +++++++++++++
 ql/test/library-tests/ast/modules/modules.rb  | 23 +++++++++++++-
 .../ast/modules/toplevel.expected             |  2 +-
 6 files changed, 86 insertions(+), 9 deletions(-)

diff --git a/ql/test/library-tests/ast/Ast.expected b/ql/test/library-tests/ast/Ast.expected
index 1085019743b..c6f4dc44656 100644
--- a/ql/test/library-tests/ast/Ast.expected
+++ b/ql/test/library-tests/ast/Ast.expected
@@ -1370,6 +1370,20 @@ modules/modules.rb:
 #   56|       getAnOperand/getLeftOperand: [GlobalVariableAccess] $global_var
 #   56|       getAnOperand/getRightOperand: [IntegerLiteral] 4
 #   60|   getStmt: [ModuleDefinition] MyModuleInGlobalScope
+#   63|   getStmt: [ModuleDefinition] Test
+#   65|     getStmt: [ModuleDefinition] Foo1
+#   66|       getStmt: [ClassDefinition] Bar
+#   66|         getScopeExpr: [ConstantReadAccess] Foo1
+#   70|     getStmt: [ModuleDefinition] Foo2
+#   71|       getStmt: [ModuleDefinition] Foo2
+#   72|       getStmt: [ClassDefinition] Bar
+#   72|         getScopeExpr: [ConstantReadAccess] Foo2
+#   76|     getStmt: [ModuleDefinition] Foo3
+#   77|       getStmt: [AssignExpr] ... = ...
+#   77|         getAnOperand/getLeftOperand: [ConstantAssignment] Foo3
+#   77|         getAnOperand/getRightOperand: [ConstantReadAccess] Object
+#   78|       getStmt: [ClassDefinition] Bar
+#   78|         getScopeExpr: [ConstantReadAccess] Foo3
 operations/operations.rb:
 #    1| [Toplevel] operations.rb
 #    3|   getStmt: [AssignExpr] ... = ...
diff --git a/ql/test/library-tests/ast/modules/classes.expected b/ql/test/library-tests/ast/modules/classes.expected
index 5d9b7dfae33..55923386fe4 100644
--- a/ql/test/library-tests/ast/modules/classes.expected
+++ b/ql/test/library-tests/ast/modules/classes.expected
@@ -10,8 +10,14 @@ classes
 | modules.rb:19:3:20:5 | ClassInFoo | ClassDefinition | ClassInFoo |
 | modules.rb:30:3:31:5 | ClassInAnotherDefinitionOfFoo | ClassDefinition | ClassInAnotherDefinitionOfFoo |
 | modules.rb:49:3:50:5 | ClassInAnotherDefinitionOfFooBar | ClassDefinition | ClassInAnotherDefinitionOfFooBar |
+| modules.rb:66:5:67:7 | Bar | ClassDefinition | Bar |
+| modules.rb:72:5:73:7 | Bar | ClassDefinition | Bar |
+| modules.rb:78:5:79:7 | Bar | ClassDefinition | Bar |
 classesWithNameScopeExprs
 | classes.rb:16:1:17:3 | MyClass | classes.rb:16:7:16:14 | MyModule |
+| modules.rb:66:5:67:7 | Bar | modules.rb:66:11:66:14 | Foo1 |
+| modules.rb:72:5:73:7 | Bar | modules.rb:72:11:72:14 | Foo2 |
+| modules.rb:78:5:79:7 | Bar | modules.rb:78:11:78:14 | Foo3 |
 classesWithGlobalNameScopeExprs
 | classes.rb:55:1:56:3 | MyClassInGlobalScope |
 exprsInClasses
diff --git a/ql/test/library-tests/ast/modules/module_base.expected b/ql/test/library-tests/ast/modules/module_base.expected
index 99d40f49e22..d63da775d77 100644
--- a/ql/test/library-tests/ast/modules/module_base.expected
+++ b/ql/test/library-tests/ast/modules/module_base.expected
@@ -11,7 +11,7 @@ moduleBases
 | classes.rb:41:1:52:3 | class << ... | ClassDefinition |
 | classes.rb:55:1:56:3 | MyClassInGlobalScope | ClassDefinition |
 | modules.rb:1:1:2:3 | Empty | ModuleDefinition |
-| modules.rb:1:1:61:3 | modules.rb | Toplevel |
+| modules.rb:1:1:82:1 | modules.rb | Toplevel |
 | modules.rb:4:1:24:3 | Foo | ModuleDefinition |
 | modules.rb:5:3:14:5 | Bar | ModuleDefinition |
 | modules.rb:6:5:7:7 | ClassInFooBar | ClassDefinition |
@@ -22,6 +22,14 @@ moduleBases
 | modules.rb:48:1:57:3 | Bar | ModuleDefinition |
 | modules.rb:49:3:50:5 | ClassInAnotherDefinitionOfFooBar | ClassDefinition |
 | modules.rb:60:1:61:3 | MyModuleInGlobalScope | ModuleDefinition |
+| modules.rb:63:1:81:3 | Test | ModuleDefinition |
+| modules.rb:65:3:68:5 | Foo1 | ModuleDefinition |
+| modules.rb:66:5:67:7 | Bar | ClassDefinition |
+| modules.rb:70:3:74:5 | Foo2 | ModuleDefinition |
+| modules.rb:71:5:71:19 | Foo2 | ModuleDefinition |
+| modules.rb:72:5:73:7 | Bar | ClassDefinition |
+| modules.rb:76:3:80:5 | Foo3 | ModuleDefinition |
+| modules.rb:78:5:79:7 | Bar | ClassDefinition |
 | toplevel.rb:1:1:5:23 | toplevel.rb | Toplevel |
 moduleBaseClasses
 | classes.rb:2:1:56:3 | classes.rb | classes.rb:3:1:4:3 | Foo |
@@ -35,6 +43,9 @@ moduleBaseClasses
 | modules.rb:5:3:14:5 | Bar | modules.rb:6:5:7:7 | ClassInFooBar |
 | modules.rb:26:1:35:3 | Foo | modules.rb:30:3:31:5 | ClassInAnotherDefinitionOfFoo |
 | modules.rb:48:1:57:3 | Bar | modules.rb:49:3:50:5 | ClassInAnotherDefinitionOfFooBar |
+| modules.rb:65:3:68:5 | Foo1 | modules.rb:66:5:67:7 | Bar |
+| modules.rb:70:3:74:5 | Foo2 | modules.rb:72:5:73:7 | Bar |
+| modules.rb:76:3:80:5 | Foo3 | modules.rb:78:5:79:7 | Bar |
 moduleBaseMethods
 | classes.rb:20:1:37:3 | Wibble | classes.rb:21:3:23:5 | method_a |
 | classes.rb:20:1:37:3 | Wibble | classes.rb:25:3:27:5 | method_b |
@@ -49,10 +60,15 @@ moduleBaseMethods
 moduleBaseModules
 | classes.rb:2:1:56:3 | classes.rb | classes.rb:15:1:15:20 | MyModule |
 | classes.rb:20:1:37:3 | Wibble | classes.rb:35:3:36:5 | ModuleInWibble |
-| modules.rb:1:1:61:3 | modules.rb | modules.rb:1:1:2:3 | Empty |
-| modules.rb:1:1:61:3 | modules.rb | modules.rb:4:1:24:3 | Foo |
-| modules.rb:1:1:61:3 | modules.rb | modules.rb:26:1:35:3 | Foo |
-| modules.rb:1:1:61:3 | modules.rb | modules.rb:37:1:46:3 | Bar |
-| modules.rb:1:1:61:3 | modules.rb | modules.rb:48:1:57:3 | Bar |
-| modules.rb:1:1:61:3 | modules.rb | modules.rb:60:1:61:3 | MyModuleInGlobalScope |
+| modules.rb:1:1:82:1 | modules.rb | modules.rb:1:1:2:3 | Empty |
+| modules.rb:1:1:82:1 | modules.rb | modules.rb:4:1:24:3 | Foo |
+| modules.rb:1:1:82:1 | modules.rb | modules.rb:26:1:35:3 | Foo |
+| modules.rb:1:1:82:1 | modules.rb | modules.rb:37:1:46:3 | Bar |
+| modules.rb:1:1:82:1 | modules.rb | modules.rb:48:1:57:3 | Bar |
+| modules.rb:1:1:82:1 | modules.rb | modules.rb:60:1:61:3 | MyModuleInGlobalScope |
+| modules.rb:1:1:82:1 | modules.rb | modules.rb:63:1:81:3 | Test |
 | modules.rb:4:1:24:3 | Foo | modules.rb:5:3:14:5 | Bar |
+| modules.rb:63:1:81:3 | Test | modules.rb:65:3:68:5 | Foo1 |
+| modules.rb:63:1:81:3 | Test | modules.rb:70:3:74:5 | Foo2 |
+| modules.rb:63:1:81:3 | Test | modules.rb:76:3:80:5 | Foo3 |
+| modules.rb:70:3:74:5 | Foo2 | modules.rb:71:5:71:19 | Foo2 |
diff --git a/ql/test/library-tests/ast/modules/modules.expected b/ql/test/library-tests/ast/modules/modules.expected
index 1cb4c50c9e7..5bad601a779 100644
--- a/ql/test/library-tests/ast/modules/modules.expected
+++ b/ql/test/library-tests/ast/modules/modules.expected
@@ -8,6 +8,11 @@ modules
 | modules.rb:37:1:46:3 | Bar | ModuleDefinition | Bar |
 | modules.rb:48:1:57:3 | Bar | ModuleDefinition | Bar |
 | modules.rb:60:1:61:3 | MyModuleInGlobalScope | ModuleDefinition | MyModuleInGlobalScope |
+| modules.rb:63:1:81:3 | Test | ModuleDefinition | Test |
+| modules.rb:65:3:68:5 | Foo1 | ModuleDefinition | Foo1 |
+| modules.rb:70:3:74:5 | Foo2 | ModuleDefinition | Foo2 |
+| modules.rb:71:5:71:19 | Foo2 | ModuleDefinition | Foo2 |
+| modules.rb:76:3:80:5 | Foo3 | ModuleDefinition | Foo3 |
 modulesWithScopeExprs
 | modules.rb:48:1:57:3 | Bar | modules.rb:48:8:48:10 | Foo |
 modulesWithGlobalNameScopeExprs
@@ -34,6 +39,14 @@ exprsInModules
 | modules.rb:48:1:57:3 | Bar | 1 | modules.rb:52:3:53:5 | method_in_another_definition_of_foo_bar | Method |
 | modules.rb:48:1:57:3 | Bar | 2 | modules.rb:55:3:55:30 | call to puts | MethodCall |
 | modules.rb:48:1:57:3 | Bar | 3 | modules.rb:56:3:56:17 | ... = ... | AssignExpr |
+| modules.rb:63:1:81:3 | Test | 0 | modules.rb:65:3:68:5 | Foo1 | ModuleDefinition |
+| modules.rb:63:1:81:3 | Test | 1 | modules.rb:70:3:74:5 | Foo2 | ModuleDefinition |
+| modules.rb:63:1:81:3 | Test | 2 | modules.rb:76:3:80:5 | Foo3 | ModuleDefinition |
+| modules.rb:65:3:68:5 | Foo1 | 0 | modules.rb:66:5:67:7 | Bar | ClassDefinition |
+| modules.rb:70:3:74:5 | Foo2 | 0 | modules.rb:71:5:71:19 | Foo2 | ModuleDefinition |
+| modules.rb:70:3:74:5 | Foo2 | 1 | modules.rb:72:5:73:7 | Bar | ClassDefinition |
+| modules.rb:76:3:80:5 | Foo3 | 0 | modules.rb:77:5:77:17 | ... = ... | AssignExpr |
+| modules.rb:76:3:80:5 | Foo3 | 1 | modules.rb:78:5:79:7 | Bar | ClassDefinition |
 methodsInModules
 | modules.rb:4:1:24:3 | Foo | modules.rb:16:3:17:5 | method_in_foo | method_in_foo |
 | modules.rb:5:3:14:5 | Bar | modules.rb:9:5:10:7 | method_in_foo_bar | method_in_foo_bar |
@@ -46,5 +59,12 @@ classesInModules
 | modules.rb:5:3:14:5 | Bar | modules.rb:6:5:7:7 | ClassInFooBar | ClassInFooBar |
 | modules.rb:26:1:35:3 | Foo | modules.rb:30:3:31:5 | ClassInAnotherDefinitionOfFoo | ClassInAnotherDefinitionOfFoo |
 | modules.rb:48:1:57:3 | Bar | modules.rb:49:3:50:5 | ClassInAnotherDefinitionOfFooBar | ClassInAnotherDefinitionOfFooBar |
+| modules.rb:65:3:68:5 | Foo1 | modules.rb:66:5:67:7 | Bar | Bar |
+| modules.rb:70:3:74:5 | Foo2 | modules.rb:72:5:73:7 | Bar | Bar |
+| modules.rb:76:3:80:5 | Foo3 | modules.rb:78:5:79:7 | Bar | Bar |
 modulesInModules
 | modules.rb:4:1:24:3 | Foo | modules.rb:5:3:14:5 | Bar | Bar |
+| modules.rb:63:1:81:3 | Test | modules.rb:65:3:68:5 | Foo1 | Foo1 |
+| modules.rb:63:1:81:3 | Test | modules.rb:70:3:74:5 | Foo2 | Foo2 |
+| modules.rb:63:1:81:3 | Test | modules.rb:76:3:80:5 | Foo3 | Foo3 |
+| modules.rb:70:3:74:5 | Foo2 | modules.rb:71:5:71:19 | Foo2 | Foo2 |
diff --git a/ql/test/library-tests/ast/modules/modules.rb b/ql/test/library-tests/ast/modules/modules.rb
index dfba11f8d6e..2463a699d4d 100644
--- a/ql/test/library-tests/ast/modules/modules.rb
+++ b/ql/test/library-tests/ast/modules/modules.rb
@@ -58,4 +58,25 @@ end
 
 # a module where the name is a scope resolution using the global scope
 module ::MyModuleInGlobalScope
-end
\ No newline at end of file
+end
+
+module Test
+
+  module Foo1
+    class Foo1::Bar
+    end
+  end
+
+  module Foo2
+    module Foo2 end
+    class Foo2::Bar
+    end
+  end
+  
+  module Foo3
+    Foo3 = Object
+    class Foo3::Bar
+    end
+  end
+end
+
diff --git a/ql/test/library-tests/ast/modules/toplevel.expected b/ql/test/library-tests/ast/modules/toplevel.expected
index 9ea0f83b3df..dd4576a21a0 100644
--- a/ql/test/library-tests/ast/modules/toplevel.expected
+++ b/ql/test/library-tests/ast/modules/toplevel.expected
@@ -1,6 +1,6 @@
 toplevel
 | classes.rb:2:1:56:3 | classes.rb | Toplevel |
-| modules.rb:1:1:61:3 | modules.rb | Toplevel |
+| modules.rb:1:1:82:1 | modules.rb | Toplevel |
 | toplevel.rb:1:1:5:23 | toplevel.rb | Toplevel |
 beginBlocks
 | toplevel.rb:1:1:5:23 | toplevel.rb | 0 | toplevel.rb:5:1:5:22 | BEGIN { ... } |

From f12e6ea8ea3e671a360992538ea27d414807882d Mon Sep 17 00:00:00 2001
From: Arthur Baars <aibaars@github.com>
Date: Tue, 30 Mar 2021 16:14:21 +0200
Subject: [PATCH 05/14] Avoid 'Object::' prefixes

---
 ql/src/codeql_ruby/ast/internal/Module.qll    | 11 +++++--
 .../ast/modules/modules.expected              | 32 +++++++++++++++++++
 ql/test/library-tests/ast/modules/modules.ql  |  2 ++
 3 files changed, 42 insertions(+), 3 deletions(-)

diff --git a/ql/src/codeql_ruby/ast/internal/Module.qll b/ql/src/codeql_ruby/ast/internal/Module.qll
index d5e85d84258..3d9d2e3203f 100644
--- a/ql/src/codeql_ruby/ast/internal/Module.qll
+++ b/ql/src/codeql_ruby/ast/internal/Module.qll
@@ -51,6 +51,11 @@ ModuleBase enclosing(ModuleBase m, int level) {
   result = enclosing(m.getOuterScope().getEnclosingModule(), level - 1)
 }
 
+bindingset[qualifier, name]
+private string scopeAppend(string qualifier, string name) {
+  if qualifier = "Object" then result = name else result = qualifier + "::" + name
+}
+
 private string resolveRelativeToEnclosing(ConstantAccess n, int i) {
   not isToplevel(n) and
   not exists(n.getScopeExpr()) and
@@ -58,7 +63,7 @@ private string resolveRelativeToEnclosing(ConstantAccess n, int i) {
     n = s.getADescendant() and
     enclosing = enclosing(s.getEnclosingModule(), i) and
     (
-      result = constantDefinition0(enclosing) + "::" + n.getName()
+      result = scopeAppend(constantDefinition0(enclosing), n.getName())
       or
       enclosing instanceof Toplevel and result = n.getName()
     )
@@ -71,13 +76,13 @@ private string qualifiedNameForConstant0(ConstantAccess n) {
   or
   result = resolveRelativeToEnclosing(n, 0)
   or
-  result = resolveScopeExpr0(n.getScopeExpr()) + "::" + n.getName()
+  result = scopeAppend(resolveScopeExpr0(n.getScopeExpr()), n.getName())
 }
 
 string constantDefinition(ConstantWriteAccess n) {
   result = constantDefinition0(n)
   or
-  result = resolveScopeExpr(n.getScopeExpr()) + "::" + n.getName()
+  result = scopeAppend(resolveScopeExpr(n.getScopeExpr()), n.getName())
 }
 
 private string resolveScopeExpr(ConstantReadAccess n) {
diff --git a/ql/test/library-tests/ast/modules/modules.expected b/ql/test/library-tests/ast/modules/modules.expected
index 5bad601a779..7c877d40c2e 100644
--- a/ql/test/library-tests/ast/modules/modules.expected
+++ b/ql/test/library-tests/ast/modules/modules.expected
@@ -68,3 +68,35 @@ modulesInModules
 | modules.rb:63:1:81:3 | Test | modules.rb:70:3:74:5 | Foo2 | Foo2 |
 | modules.rb:63:1:81:3 | Test | modules.rb:76:3:80:5 | Foo3 | Foo3 |
 | modules.rb:70:3:74:5 | Foo2 | modules.rb:71:5:71:19 | Foo2 | Foo2 |
+moduleTypes
+| classes.rb:2:1:56:3 | classes.rb | file://:0:0:0:0 | Object |
+| classes.rb:3:1:4:3 | Foo | modules.rb:4:1:24:3 | Foo |
+| classes.rb:7:1:8:3 | Bar | modules.rb:37:1:46:3 | Bar |
+| classes.rb:11:1:12:3 | Baz | classes.rb:11:1:12:3 | Baz |
+| classes.rb:15:1:15:20 | MyModule | classes.rb:15:1:15:20 | MyModule |
+| classes.rb:16:1:17:3 | MyClass | classes.rb:16:1:17:3 | MyModule::MyClass |
+| classes.rb:20:1:37:3 | Wibble | classes.rb:20:1:37:3 | Wibble |
+| classes.rb:32:3:33:5 | ClassInWibble | classes.rb:32:3:33:5 | Wibble::ClassInWibble |
+| classes.rb:35:3:36:5 | ModuleInWibble | classes.rb:35:3:36:5 | Wibble::ModuleInWibble |
+| classes.rb:55:1:56:3 | MyClassInGlobalScope | classes.rb:55:1:56:3 | MyClassInGlobalScope |
+| modules.rb:1:1:2:3 | Empty | modules.rb:1:1:2:3 | Empty |
+| modules.rb:1:1:82:1 | modules.rb | file://:0:0:0:0 | Object |
+| modules.rb:4:1:24:3 | Foo | modules.rb:4:1:24:3 | Foo |
+| modules.rb:5:3:14:5 | Bar | modules.rb:5:3:14:5 | Foo::Bar |
+| modules.rb:6:5:7:7 | ClassInFooBar | modules.rb:6:5:7:7 | Foo::Bar::ClassInFooBar |
+| modules.rb:19:3:20:5 | ClassInFoo | modules.rb:19:3:20:5 | Foo::ClassInFoo |
+| modules.rb:26:1:35:3 | Foo | modules.rb:4:1:24:3 | Foo |
+| modules.rb:30:3:31:5 | ClassInAnotherDefinitionOfFoo | modules.rb:30:3:31:5 | Foo::ClassInAnotherDefinitionOfFoo |
+| modules.rb:37:1:46:3 | Bar | modules.rb:37:1:46:3 | Bar |
+| modules.rb:48:1:57:3 | Bar | modules.rb:5:3:14:5 | Foo::Bar |
+| modules.rb:49:3:50:5 | ClassInAnotherDefinitionOfFooBar | modules.rb:49:3:50:5 | Foo::Bar::ClassInAnotherDefinitionOfFooBar |
+| modules.rb:60:1:61:3 | MyModuleInGlobalScope | modules.rb:60:1:61:3 | MyModuleInGlobalScope |
+| modules.rb:63:1:81:3 | Test | modules.rb:63:1:81:3 | Test |
+| modules.rb:65:3:68:5 | Foo1 | modules.rb:65:3:68:5 | Test::Foo1 |
+| modules.rb:66:5:67:7 | Bar | modules.rb:66:5:67:7 | Test::Foo1::Bar |
+| modules.rb:70:3:74:5 | Foo2 | modules.rb:70:3:74:5 | Test::Foo2 |
+| modules.rb:71:5:71:19 | Foo2 | modules.rb:71:5:71:19 | Test::Foo2::Foo2 |
+| modules.rb:72:5:73:7 | Bar | modules.rb:72:5:73:7 | Test::Foo2::Foo2::Bar |
+| modules.rb:76:3:80:5 | Foo3 | modules.rb:76:3:80:5 | Test::Foo3 |
+| modules.rb:78:5:79:7 | Bar | modules.rb:37:1:46:3 | Bar |
+| toplevel.rb:1:1:5:23 | toplevel.rb | file://:0:0:0:0 | Object |
diff --git a/ql/test/library-tests/ast/modules/modules.ql b/ql/test/library-tests/ast/modules/modules.ql
index d0942303fb5..ba42d6365bf 100644
--- a/ql/test/library-tests/ast/modules/modules.ql
+++ b/ql/test/library-tests/ast/modules/modules.ql
@@ -23,3 +23,5 @@ query predicate classesInModules(ModuleDefinition mod, ClassDefinition klass, st
 query predicate modulesInModules(ModuleDefinition mod, ModuleDefinition child, string name) {
   child = mod.getModule(name)
 }
+
+query predicate moduleTypes(ModuleBase def, Module type) { type = def.getModule() }

From 50b8b6b2570fabbc97db47a8e53bfc1f8748c9fd Mon Sep 17 00:00:00 2001
From: Arthur Baars <aibaars@github.com>
Date: Thu, 1 Apr 2021 13:52:20 +0200
Subject: [PATCH 06/14] Also resolve constants with respect to the ancestors of
 the enclosing module.

---
 ql/src/codeql_ruby/ast/internal/Module.qll | 139 ++++++++++++++++++---
 1 file changed, 124 insertions(+), 15 deletions(-)

diff --git a/ql/src/codeql_ruby/ast/internal/Module.qll b/ql/src/codeql_ruby/ast/internal/Module.qll
index 3d9d2e3203f..96629824689 100644
--- a/ql/src/codeql_ruby/ast/internal/Module.qll
+++ b/ql/src/codeql_ruby/ast/internal/Module.qll
@@ -1,5 +1,7 @@
 private import codeql.Locations
+private import codeql_ruby.ast.Call
 private import codeql_ruby.ast.Constant
+private import codeql_ruby.ast.Expr
 private import codeql_ruby.ast.Module
 private import codeql_ruby.ast.Operation
 private import codeql_ruby.ast.Scope
@@ -7,6 +9,7 @@ private import codeql_ruby.ast.Scope
 // Names of built-in modules and classes
 private string builtin() { result = ["Object", "Kernel", "BasicObject", "Class", "Module"] }
 
+cached
 newtype TConstant =
   TResolved(string qName, boolean isModule) {
     exists(ConstantWriteAccess n |
@@ -62,11 +65,122 @@ private string resolveRelativeToEnclosing(ConstantAccess n, int i) {
   exists(Scope s, ModuleBase enclosing |
     n = s.getADescendant() and
     enclosing = enclosing(s.getEnclosingModule(), i) and
-    (
-      result = scopeAppend(constantDefinition0(enclosing), n.getName())
+    result = scopeAppend(constantDefinition0(enclosing), n.getName()) and
+    (result = builtin() or result = constantDefinition0(_) or n instanceof ConstantWriteAccess)
+  )
+}
+
+private class IncludeOrPrependCall extends MethodCall {
+  IncludeOrPrependCall() { this.getMethodName() = ["include", "prepend"] }
+
+  string getAModule() { result = resolveScopeExpr0(this.getAnArgument()) }
+
+  string getTarget() {
+    result = resolveScopeExpr0(this.getReceiver())
+    or
+    exists(Scope s |
+      s.getADescendant() = this and
+      (
+        result = constantDefinition0(s.getEnclosingModule())
+        or
+        result = "Object" and s.getEnclosingModule() instanceof Toplevel
+      )
+    |
+      this.getReceiver() instanceof Self
       or
-      enclosing instanceof Toplevel and result = n.getName()
+      not exists(this.getReceiver())
     )
+  }
+}
+
+private string prepends(string qname) {
+  exists(IncludeOrPrependCall m |
+    m.getMethodName() = "prepend" and
+    qname = m.getTarget() and
+    result = m.getAModule()
+  )
+}
+
+private string includes(string qname) {
+  qname = "Object" and
+  result = "Kernel"
+  or
+  exists(IncludeOrPrependCall m |
+    m.getMethodName() = "include" and
+    qname = m.getTarget() and
+    result = m.getAModule()
+  )
+}
+
+private Expr superexpr(string qname) {
+  exists(ClassDefinition c | qname = constantDefinition0(c) and result = c.getSuperclassExpr())
+}
+
+private string superclass(string qname) {
+  qname = "Object" and result = "BasicObject"
+  or
+  result = resolveScopeExpr(superexpr(qname))
+  or
+  qname = constantDefinition0(_) and
+  not exists(superexpr(qname)) and
+  result = "Object" and
+  qname != "BasicObject"
+}
+
+private string ancestors(string qname) {
+  qname = [builtin(), constantDefinition0(any(Namespace x))] and
+  (
+    result = prepends(qname)
+    or
+    result = qname
+    or
+    result = includes(qname)
+  )
+}
+
+private string contains(string qname, string name) {
+  result = containsIgnoringSuper(qname, name)
+  or
+  not exists(containsIgnoringSuper(qname, name)) and
+  result = contains(superclass(qname), name)
+}
+
+private string qualifiedName(string container, string name) {
+  result = [builtin(), constantDefinition0(_)] and
+  (
+    container = result.regexpCapture("(.+)::([^:]+)", 1) and
+    name = result.regexpCapture("(.+)::([^:]+)", 2)
+    or
+    container = "Object" and name = result
+  )
+}
+
+private string containsIgnoringSuper(string qname, string name) {
+  result =
+    min(string n, string container, int i |
+      n = qualifiedName(container, name) and
+      (
+        container = ancestors*(prepends(qname)) and i = 0
+        or
+        container = qname and i = 1
+        or
+        container = ancestors*(includes(qname)) and i = 2
+      )
+    |
+      n order by i
+    )
+}
+
+private string resolveRelativeToAncestors(ConstantReadAccess n) {
+  not isToplevel(n) and
+  not exists(n.getScopeExpr()) and
+  exists(Scope s, ModuleBase enclosing |
+    n = s.getADescendant() and
+    enclosing = s.getEnclosingModule()
+  |
+    result = contains(constantDefinition(enclosing), n.getName())
+    or
+    enclosing instanceof Toplevel and result = contains("Object", n.getName())
   )
 }
 
@@ -86,23 +200,18 @@ string constantDefinition(ConstantWriteAccess n) {
 }
 
 private string resolveScopeExpr(ConstantReadAccess n) {
+  result = resolveScopeExpr0(n)
+  or
   exists(string qname |
-    qname =
-      min(int i, string x |
-        (
-          x = qualifiedNameForConstant0(n) and i = 0
-          or
-          x = resolveRelativeToEnclosing(n, i)
-        ) and
-        (x = builtin() or x = constantDefinition0(_))
-      |
-        x order by i
-      )
+    qname = min(int i, string x | x = resolveRelativeToEnclosing(n, i) | x order by i)
+    or
+    not exists(resolveRelativeToEnclosing(n, _)) and
+    qname = resolveRelativeToAncestors(n)
   |
     qname = builtin() and result = qname
     or
     not qname = builtin() and
-    exists(ConstantWriteAccess def | qname = constantDefinition0(def) |
+    exists(ConstantWriteAccess def | qname = constantDefinition(def) |
       result = qname and def instanceof Namespace
       or
       result = resolveScopeExpr(def.getParent().(Assignment).getRightOperand())

From 063b085078cde8dd7311e7f78d058919f0598092 Mon Sep 17 00:00:00 2001
From: Arthur Baars <aibaars@github.com>
Date: Wed, 7 Apr 2021 15:57:13 +0200
Subject: [PATCH 07/14] Address comments

---
 ql/src/codeql_ruby/ast/Expr.qll               |  4 +-
 ql/src/codeql_ruby/ast/Module.qll             | 43 +++++----
 ql/src/codeql_ruby/ast/internal/AST.qll       | 10 +--
 ql/src/codeql_ruby/ast/internal/Module.qll    | 17 ++--
 ql/src/codeql_ruby/ast/internal/Scope.qll     |  2 +-
 .../internal/ControlFlowGraphImpl.qll         |  8 +-
 ql/test/library-tests/ast/Ast.expected        | 88 +++++++++----------
 .../ast/constants/constants.expected          | 14 +--
 .../ast/modules/classes.expected              | 32 +++----
 ql/test/library-tests/ast/modules/classes.ql  | 18 ++--
 .../ast/modules/module_base.expected          | 58 ++++++------
 .../library-tests/ast/modules/module_base.ql  |  4 +-
 .../ast/modules/modules.expected              | 52 +++++------
 ql/test/library-tests/ast/modules/modules.ql  | 14 +--
 .../ast/modules/singleton_classes.expected    |  2 +-
 15 files changed, 186 insertions(+), 180 deletions(-)

diff --git a/ql/src/codeql_ruby/ast/Expr.qll b/ql/src/codeql_ruby/ast/Expr.qll
index a113103d09b..4e44bc98a25 100644
--- a/ql/src/codeql_ruby/ast/Expr.qll
+++ b/ql/src/codeql_ruby/ast/Expr.qll
@@ -137,11 +137,11 @@ class BodyStmt extends StmtSequence, TBodyStmt {
     result = any(Generated::Program g | this = TToplevel(g)).getChild(i) and
     not result instanceof Generated::BeginBlock
     or
-    result = any(Generated::Class g | this = TClass(g)).getChild(i)
+    result = any(Generated::Class g | this = TClassDeclaration(g)).getChild(i)
     or
     result = any(Generated::SingletonClass g | this = TSingletonClass(g)).getChild(i)
     or
-    result = any(Generated::Module g | this = TModule(g)).getChild(i)
+    result = any(Generated::Module g | this = TModuleDeclaration(g)).getChild(i)
     or
     result = any(Generated::Begin g | this = TBeginExpr(g)).getChild(i)
   }
diff --git a/ql/src/codeql_ruby/ast/Module.qll b/ql/src/codeql_ruby/ast/Module.qll
index 1126ba6253e..98aee1e1746 100644
--- a/ql/src/codeql_ruby/ast/Module.qll
+++ b/ql/src/codeql_ruby/ast/Module.qll
@@ -7,21 +7,26 @@ private import internal.TreeSitter
 /**
  * A representation of a run-time `module` or `class` value.
  */
-class Module extends TConstant {
-  Module() { this = TResolved(_, true) or this = TUnresolved(any(Namespace n)) }
+class Module extends TModule {
+  Module() { this = TResolved(_) or this = TUnresolved(_) }
 
+  /** Get a declaration of this module, if any. */
+  ModuleBase getADeclaration() { result.getModule() = this }
+
+  /** Gets a textual representation of this module. */
   string toString() {
-    this = TResolved(result, _)
+    this = TResolved(result)
     or
     exists(Namespace n | this = TUnresolved(n) and result = "...::" + n.toString())
   }
 
+  /** Gets the location of this module. */
   Location getLocation() {
     exists(Namespace n | this = TUnresolved(n) and result = n.getLocation())
     or
     result =
       min(Namespace n, string qName, Location loc, int weight |
-        this = TResolved(qName, _) and
+        this = TResolved(qName) and
         qName = constantDefinition(n) and
         loc = n.getLocation() and
         if exists(loc.getFile().getRelativePath()) then weight = 0 else weight = 1
@@ -45,16 +50,18 @@ class ModuleBase extends BodyStmt, Scope, TModuleBase {
   MethodBase getMethod(string name) { result = this.getAMethod() and result.getName() = name }
 
   /** Gets a class defined in this module/class. */
-  ClassDefinition getAClass() { result = this.getAStmt() }
+  ClassDeclaration getAClass() { result = this.getAStmt() }
 
   /** Gets the class named `name` in this module/class, if any. */
-  ClassDefinition getClass(string name) { result = this.getAClass() and result.getName() = name }
+  ClassDeclaration getClass(string name) { result = this.getAClass() and result.getName() = name }
 
   /** Gets a module defined in this module/class. */
-  ModuleDefinition getAModule() { result = this.getAStmt() }
+  ModuleDeclaration getAModule() { result = this.getAStmt() }
 
   /** Gets the module named `name` in this module/class, if any. */
-  ModuleDefinition getModule(string name) { result = this.getAModule() and result.getName() = name }
+  ModuleDeclaration getModule(string name) {
+    result = this.getAModule() and result.getName() = name
+  }
 
   /** Gets the representation of the run-time value of this module or class. */
   Module getModule() { none() }
@@ -96,7 +103,7 @@ class Toplevel extends ModuleBase, TToplevel {
     pred = "getBeginBlock" and result = this.getBeginBlock(_)
   }
 
-  final override Module getModule() { result = TResolved("Object", true) }
+  final override Module getModule() { result = TResolved("Object") }
 
   final override string toString() { result = g.getLocation().getFile().getBaseName() }
 }
@@ -169,7 +176,7 @@ class Namespace extends ModuleBase, ConstantWriteAccess, TNamespace {
   override predicate hasGlobalScope() { none() }
 
   final override Module getModule() {
-    result = any(string qName | qName = constantDefinition(this) | TResolved(qName, true))
+    result = any(string qName | qName = constantDefinition(this) | TResolved(qName))
     or
     result = TUnresolved(this)
   }
@@ -192,12 +199,12 @@ class Namespace extends ModuleBase, ConstantWriteAccess, TNamespace {
  * end
  * ```
  */
-class ClassDefinition extends Namespace, TClass {
+class ClassDeclaration extends Namespace, TClassDeclaration {
   private Generated::Class g;
 
-  ClassDefinition() { this = TClass(g) }
+  ClassDeclaration() { this = TClassDeclaration(g) }
 
-  final override string getAPrimaryQlClass() { result = "ClassDefinition" }
+  final override string getAPrimaryQlClass() { result = "ClassDeclaration" }
 
   /**
    * Gets the `Expr` used as the superclass in the class definition, if any.
@@ -256,7 +263,7 @@ class SingletonClass extends ModuleBase, TSingletonClass {
 
   SingletonClass() { this = TSingletonClass(g) }
 
-  final override string getAPrimaryQlClass() { result = "ClassDefinition" }
+  final override string getAPrimaryQlClass() { result = "ClassDeclaration" }
 
   /**
    * Gets the expression resulting in the object on which the singleton class
@@ -291,7 +298,7 @@ class SingletonClass extends ModuleBase, TSingletonClass {
  * N.B. this class represents a single instance of a module definition. In the
  * following example, classes `Bar` and `Baz` are both defined in the module
  * `Foo`, but in two syntactically distinct definitions, meaning that there
- * will be two instances of `ModuleDefinition` in the database.
+ * will be two instances of `ModuleDeclaration` in the database.
  *
  * ```rb
  * module Foo
@@ -303,12 +310,12 @@ class SingletonClass extends ModuleBase, TSingletonClass {
  * end
  * ```
  */
-class ModuleDefinition extends Namespace, TModule {
+class ModuleDeclaration extends Namespace, TModuleDeclaration {
   private Generated::Module g;
 
-  ModuleDefinition() { this = TModule(g) }
+  ModuleDeclaration() { this = TModuleDeclaration(g) }
 
-  final override string getAPrimaryQlClass() { result = "ModuleDefinition" }
+  final override string getAPrimaryQlClass() { result = "ModuleDeclaration" }
 
   final override string getName() {
     result = g.getName().(Generated::Token).getValue() or
diff --git a/ql/src/codeql_ruby/ast/internal/AST.qll b/ql/src/codeql_ruby/ast/internal/AST.qll
index 37ef071d507..26e2e6149c3 100644
--- a/ql/src/codeql_ruby/ast/internal/AST.qll
+++ b/ql/src/codeql_ruby/ast/internal/AST.qll
@@ -82,7 +82,7 @@ private module Cached {
     TCaseEqExpr(Generated::Binary g) { g instanceof @binary_equalequalequal } or
     TCaseExpr(Generated::Case g) or
     TCharacterLiteral(Generated::Character g) or
-    TClass(Generated::Class g) or
+    TClassDeclaration(Generated::Class g) or
     TClassVariableAccess(Generated::ClassVariable g, AST::ClassVariable v) {
       ClassVariableAccess::range(g, v)
     } or
@@ -139,7 +139,7 @@ private module Cached {
     } or
     TLogicalOrExpr(Generated::Binary g) { g instanceof @binary_or or g instanceof @binary_pipepipe } or
     TMethod(Generated::Method g) or
-    TModule(Generated::Module g) or
+    TModuleDeclaration(Generated::Module g) or
     TModuloExpr(Generated::Binary g) { g instanceof @binary_percent } or
     TMulExpr(Generated::Binary g) { g instanceof @binary_star } or
     TNEExpr(Generated::Binary g) { g instanceof @binary_bangequal } or
@@ -257,7 +257,7 @@ private module Cached {
     n = TCaseEqExpr(result) or
     n = TCaseExpr(result) or
     n = TCharacterLiteral(result) or
-    n = TClass(result) or
+    n = TClassDeclaration(result) or
     n = TClassVariableAccess(result, _) or
     n = TComplementExpr(result) or
     n = TComplexLiteral(result) or
@@ -302,7 +302,7 @@ private module Cached {
     n = TLogicalAndExpr(result) or
     n = TLogicalOrExpr(result) or
     n = TMethod(result) or
-    n = TModule(result) or
+    n = TModuleDeclaration(result) or
     n = TModuloExpr(result) or
     n = TMulExpr(result) or
     n = TNEExpr(result) or
@@ -434,7 +434,7 @@ class TBlock = TDoBlock or TBraceBlock;
 
 class TModuleBase = TToplevel or TNamespace or TSingletonClass;
 
-class TNamespace = TClass or TModule;
+class TNamespace = TClassDeclaration or TModuleDeclaration;
 
 class TOperation = TUnaryOperation or TBinaryOperation or TAssignment;
 
diff --git a/ql/src/codeql_ruby/ast/internal/Module.qll b/ql/src/codeql_ruby/ast/internal/Module.qll
index 96629824689..84a2adc68d7 100644
--- a/ql/src/codeql_ruby/ast/internal/Module.qll
+++ b/ql/src/codeql_ruby/ast/internal/Module.qll
@@ -10,16 +10,13 @@ private import codeql_ruby.ast.Scope
 private string builtin() { result = ["Object", "Kernel", "BasicObject", "Class", "Module"] }
 
 cached
-newtype TConstant =
-  TResolved(string qName, boolean isModule) {
-    exists(ConstantWriteAccess n |
-      qName = builtin() and isModule = true
-      or
-      qName = constantDefinition(n) and
-      if n instanceof Namespace then isModule = true else isModule = false
-    )
+newtype TModule =
+  TResolved(string qName) {
+    qName = builtin()
+    or
+    qName = constantDefinition(_)
   } or
-  TUnresolved(ConstantWriteAccess n) { not exists(constantDefinition(n)) }
+  TUnresolved(Namespace n) { not exists(constantDefinition(n)) }
 
 private predicate isToplevel(ConstantAccess n) {
   not exists(n.getScopeExpr()) and
@@ -113,7 +110,7 @@ private string includes(string qname) {
 }
 
 private Expr superexpr(string qname) {
-  exists(ClassDefinition c | qname = constantDefinition0(c) and result = c.getSuperclassExpr())
+  exists(ClassDeclaration c | qname = constantDefinition0(c) and result = c.getSuperclassExpr())
 }
 
 private string superclass(string qname) {
diff --git a/ql/src/codeql_ruby/ast/internal/Scope.qll b/ql/src/codeql_ruby/ast/internal/Scope.qll
index 7c76ef8affa..fa030b2eb39 100644
--- a/ql/src/codeql_ruby/ast/internal/Scope.qll
+++ b/ql/src/codeql_ruby/ast/internal/Scope.qll
@@ -7,7 +7,7 @@ class TScopeType = TMethodBase or TModuleLike or TBlockLike;
 
 private class TBlockLike = TDoBlock or TLambda or TBlock or TEndBlock;
 
-private class TModuleLike = TToplevel or TModule or TClass or TSingletonClass;
+private class TModuleLike = TToplevel or TModuleDeclaration or TClassDeclaration or TSingletonClass;
 
 module Scope {
   class TypeRange = Callable::TypeRange or ModuleBase::TypeRange or @end_block;
diff --git a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll
index 3f54a5d49a0..32ccfb319e9 100644
--- a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll
+++ b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll
@@ -598,7 +598,7 @@ module Trees {
 
   private class CharacterTree extends LeafTree, CharacterLiteral { }
 
-  private class ClassTree extends BodyStmtPreOrderTree, ClassDefinition {
+  private class ClassTree extends BodyStmtPreOrderTree, ClassDeclaration {
     /** Gets the `i`th child in the body of this block. */
     final override AstNode getBodyChild(int i, boolean rescuable) {
       result = this.getScopeExpr() and i = 0 and rescuable = false
@@ -673,8 +673,8 @@ module Trees {
 
   private class ConstantAccessTree extends PostOrderTree, ConstantAccess {
     ConstantAccessTree() {
-      not this instanceof ClassDefinition and
-      not this instanceof ModuleDefinition
+      not this instanceof ClassDeclaration and
+      not this instanceof ModuleDeclaration
     }
 
     final override predicate propagatesAbnormal(AstNode child) { child = this.getScopeExpr() }
@@ -934,7 +934,7 @@ module Trees {
     }
   }
 
-  private class ModuleTree extends BodyStmtPreOrderTree, ModuleDefinition {
+  private class ModuleTree extends BodyStmtPreOrderTree, ModuleDeclaration {
     /** Gets the `i`th child in the body of this block. */
     final override AstNode getBodyChild(int i, boolean rescuable) {
       result = this.getScopeExpr() and i = 0 and rescuable = false
diff --git a/ql/test/library-tests/ast/Ast.expected b/ql/test/library-tests/ast/Ast.expected
index c6f4dc44656..ae82f967178 100644
--- a/ql/test/library-tests/ast/Ast.expected
+++ b/ql/test/library-tests/ast/Ast.expected
@@ -128,19 +128,19 @@ calls/calls.rb:
 #  111|       getBody: [StmtSequence] then ...
 #  112|         getStmt: [MethodCall] call to baz
 #  112|           getReceiver: [ConstantReadAccess] X
-#  116|   getStmt: [ClassDefinition] MyClass
+#  116|   getStmt: [ClassDeclaration] MyClass
 #  117|     getStmt: [MethodCall] call to foo
 #  118|     getStmt: [MethodCall] call to bar
 #  118|       getReceiver: [ConstantReadAccess] X
-#  122|   getStmt: [ClassDefinition] MyClass
+#  122|   getStmt: [ClassDeclaration] MyClass
 #  122|     getSuperclassExpr: [MethodCall] call to foo
-#  124|   getStmt: [ClassDefinition] MyClass2
+#  124|   getStmt: [ClassDeclaration] MyClass2
 #  124|     getSuperclassExpr: [MethodCall] call to foo
 #  124|       getReceiver: [ConstantReadAccess] X
-#  128|   getStmt: [ClassDefinition] class << ...
+#  128|   getStmt: [ClassDeclaration] class << ...
 #  128|     getValue: [MethodCall] call to foo
 #  129|     getStmt: [MethodCall] call to bar
-#  131|   getStmt: [ClassDefinition] class << ...
+#  131|   getStmt: [ClassDeclaration] class << ...
 #  131|     getValue: [MethodCall] call to foo
 #  131|       getReceiver: [ConstantReadAccess] X
 #  132|     getStmt: [MethodCall] call to bar
@@ -172,7 +172,7 @@ calls/calls.rb:
 #  156|       getDefiningAccess: [LocalVariableAccess] param
 #  156|       getDefaultValue: [MethodCall] call to foo
 #  156|         getReceiver: [ConstantReadAccess] X
-#  160|   getStmt: [ModuleDefinition] SomeModule
+#  160|   getStmt: [ModuleDeclaration] SomeModule
 #  161|     getStmt: [MethodCall] call to foo
 #  162|     getStmt: [MethodCall] call to bar
 #  162|       getReceiver: [ConstantReadAccess] X
@@ -376,7 +376,7 @@ calls/calls.rb:
 #  279|       getKey: [SymbolLiteral] :blah
 #  279|       getValue: [MethodCall] call to bar
 #  279|         getReceiver: [ConstantReadAccess] X
-#  284|   getStmt: [ClassDefinition] MyClass
+#  284|   getStmt: [ClassDeclaration] MyClass
 #  285|     getStmt: [Method] my_method
 #  286|       getStmt: [SuperCall] call to super
 #  287|       getStmt: [SuperCall] call to super
@@ -419,7 +419,7 @@ calls/calls.rb:
 #  293|           getStmt: [AddExpr] ... + ...
 #  293|             getAnOperand/getLeftOperand: [LocalVariableAccess] x
 #  293|             getAnOperand/getRightOperand: [IntegerLiteral] 200
-#  301|   getStmt: [ClassDefinition] AnotherClass
+#  301|   getStmt: [ClassDeclaration] AnotherClass
 #  302|     getStmt: [Method] another_method
 #  303|       getStmt: [MethodCall] call to super
 #  303|         getReceiver: [MethodCall] call to foo
@@ -522,16 +522,16 @@ control/cases.rb:
 #   21|         getStmt: [IntegerLiteral] 30
 modules/classes.rb:
 #    2| [Toplevel] classes.rb
-#    3|   getStmt: [ClassDefinition] Foo
-#    7|   getStmt: [ClassDefinition] Bar
+#    3|   getStmt: [ClassDeclaration] Foo
+#    7|   getStmt: [ClassDeclaration] Bar
 #    7|     getSuperclassExpr: [ConstantReadAccess] BaseClass
-#   11|   getStmt: [ClassDefinition] Baz
+#   11|   getStmt: [ClassDeclaration] Baz
 #   11|     getSuperclassExpr: [MethodCall] call to superclass_for
 #   11|       getArgument: [SymbolLiteral] :baz
-#   15|   getStmt: [ModuleDefinition] MyModule
-#   16|   getStmt: [ClassDefinition] MyClass
+#   15|   getStmt: [ModuleDeclaration] MyModule
+#   16|   getStmt: [ClassDeclaration] MyClass
 #   16|     getScopeExpr: [ConstantReadAccess] MyModule
-#   20|   getStmt: [ClassDefinition] Wibble
+#   20|   getStmt: [ClassDeclaration] Wibble
 #   21|     getStmt: [Method] method_a
 #   22|       getStmt: [MethodCall] call to puts
 #   22|         getArgument: [StringLiteral] "a"
@@ -544,13 +544,13 @@ modules/classes.rb:
 #   30|     getStmt: [AssignExpr] ... = ...
 #   30|       getAnOperand/getLeftOperand: [GlobalVariableAccess] $global_var
 #   30|       getAnOperand/getRightOperand: [IntegerLiteral] 123
-#   32|     getStmt: [ClassDefinition] ClassInWibble
-#   35|     getStmt: [ModuleDefinition] ModuleInWibble
+#   32|     getStmt: [ClassDeclaration] ClassInWibble
+#   35|     getStmt: [ModuleDeclaration] ModuleInWibble
 #   40|   getStmt: [AssignExpr] ... = ...
 #   40|     getAnOperand/getLeftOperand: [LocalVariableAccess] x
 #   40|     getAnOperand/getRightOperand: [StringLiteral] "hello"
 #   40|       getComponent: [StringTextComponent] hello
-#   41|   getStmt: [ClassDefinition] class << ...
+#   41|   getStmt: [ClassDeclaration] class << ...
 #   41|     getValue: [LocalVariableAccess] x
 #   42|     getStmt: [Method] length
 #   43|       getStmt: [MulExpr] ... * ...
@@ -564,7 +564,7 @@ modules/classes.rb:
 #   51|     getStmt: [AssignExpr] ... = ...
 #   51|       getAnOperand/getLeftOperand: [GlobalVariableAccess] $global_var2
 #   51|       getAnOperand/getRightOperand: [IntegerLiteral] 456
-#   55|   getStmt: [ClassDefinition] MyClassInGlobalScope
+#   55|   getStmt: [ClassDeclaration] MyClassInGlobalScope
 control/conditionals.rb:
 #    1| [Toplevel] conditionals.rb
 #    2|   getStmt: [AssignExpr] ... = ...
@@ -687,12 +687,12 @@ control/conditionals.rb:
 #   69|       getStmt: [LocalVariableAccess] c
 constants/constants.rb:
 #    1| [Toplevel] constants.rb
-#    1|   getStmt: [ModuleDefinition] ModuleA
-#    2|     getStmt: [ClassDefinition] ClassA
-#    5|     getStmt: [ModuleDefinition] ModuleB
-#    6|       getStmt: [ClassDefinition] ClassB
+#    1|   getStmt: [ModuleDeclaration] ModuleA
+#    2|     getStmt: [ClassDeclaration] ClassA
+#    5|     getStmt: [ModuleDeclaration] ModuleB
+#    6|       getStmt: [ClassDeclaration] ClassB
 #    6|         getSuperclassExpr: [ConstantReadAccess] Base
-#    9|       getStmt: [ClassDefinition] ClassC
+#    9|       getStmt: [ClassDeclaration] ClassC
 #    9|         getSuperclassExpr: [ConstantReadAccess] Z
 #    9|           getScopeExpr: [ConstantReadAccess] Y
 #    9|             getScopeExpr: [ConstantReadAccess] X
@@ -725,9 +725,9 @@ constants/constants.rb:
 #   25|     getStmt: [MethodCall] call to Array
 #   25|       getArgument: [StringLiteral] "foo"
 #   25|         getComponent: [StringTextComponent] foo
-#   28|   getStmt: [ClassDefinition] ClassD
+#   28|   getStmt: [ClassDeclaration] ClassD
 #   28|     getScopeExpr: [ConstantReadAccess] ModuleA
-#   31|   getStmt: [ModuleDefinition] ModuleC
+#   31|   getStmt: [ModuleDeclaration] ModuleC
 #   31|     getScopeExpr: [ConstantReadAccess] ModuleA
 #   34|   getStmt: [AssignExpr] ... = ...
 #   34|     getAnOperand/getLeftOperand: [ConstantAssignment] MAX_SIZE
@@ -1322,10 +1322,10 @@ misc/misc.rb:
 #   10|       getComponent: [StringTextComponent] foo
 modules/modules.rb:
 #    1| [Toplevel] modules.rb
-#    1|   getStmt: [ModuleDefinition] Empty
-#    4|   getStmt: [ModuleDefinition] Foo
-#    5|     getStmt: [ModuleDefinition] Bar
-#    6|       getStmt: [ClassDefinition] ClassInFooBar
+#    1|   getStmt: [ModuleDeclaration] Empty
+#    4|   getStmt: [ModuleDeclaration] Foo
+#    5|     getStmt: [ModuleDeclaration] Bar
+#    6|       getStmt: [ClassDeclaration] ClassInFooBar
 #    9|       getStmt: [Method] method_in_foo_bar
 #   12|       getStmt: [MethodCall] call to puts
 #   12|         getArgument: [StringLiteral] "module Foo::Bar"
@@ -1334,23 +1334,23 @@ modules/modules.rb:
 #   13|         getAnOperand/getLeftOperand: [GlobalVariableAccess] $global_var
 #   13|         getAnOperand/getRightOperand: [IntegerLiteral] 0
 #   16|     getStmt: [Method] method_in_foo
-#   19|     getStmt: [ClassDefinition] ClassInFoo
+#   19|     getStmt: [ClassDeclaration] ClassInFoo
 #   22|     getStmt: [MethodCall] call to puts
 #   22|       getArgument: [StringLiteral] "module Foo"
 #   22|         getComponent: [StringTextComponent] module Foo
 #   23|     getStmt: [AssignExpr] ... = ...
 #   23|       getAnOperand/getLeftOperand: [GlobalVariableAccess] $global_var
 #   23|       getAnOperand/getRightOperand: [IntegerLiteral] 1
-#   26|   getStmt: [ModuleDefinition] Foo
+#   26|   getStmt: [ModuleDeclaration] Foo
 #   27|     getStmt: [Method] method_in_another_definition_of_foo
-#   30|     getStmt: [ClassDefinition] ClassInAnotherDefinitionOfFoo
+#   30|     getStmt: [ClassDeclaration] ClassInAnotherDefinitionOfFoo
 #   33|     getStmt: [MethodCall] call to puts
 #   33|       getArgument: [StringLiteral] "module Foo again"
 #   33|         getComponent: [StringTextComponent] module Foo again
 #   34|     getStmt: [AssignExpr] ... = ...
 #   34|       getAnOperand/getLeftOperand: [GlobalVariableAccess] $global_var
 #   34|       getAnOperand/getRightOperand: [IntegerLiteral] 2
-#   37|   getStmt: [ModuleDefinition] Bar
+#   37|   getStmt: [ModuleDeclaration] Bar
 #   38|     getStmt: [Method] method_a
 #   41|     getStmt: [Method] method_b
 #   44|     getStmt: [MethodCall] call to puts
@@ -1359,9 +1359,9 @@ modules/modules.rb:
 #   45|     getStmt: [AssignExpr] ... = ...
 #   45|       getAnOperand/getLeftOperand: [GlobalVariableAccess] $global_var
 #   45|       getAnOperand/getRightOperand: [IntegerLiteral] 3
-#   48|   getStmt: [ModuleDefinition] Bar
+#   48|   getStmt: [ModuleDeclaration] Bar
 #   48|     getScopeExpr: [ConstantReadAccess] Foo
-#   49|     getStmt: [ClassDefinition] ClassInAnotherDefinitionOfFooBar
+#   49|     getStmt: [ClassDeclaration] ClassInAnotherDefinitionOfFooBar
 #   52|     getStmt: [Method] method_in_another_definition_of_foo_bar
 #   55|     getStmt: [MethodCall] call to puts
 #   55|       getArgument: [StringLiteral] "module Foo::Bar again"
@@ -1369,20 +1369,20 @@ modules/modules.rb:
 #   56|     getStmt: [AssignExpr] ... = ...
 #   56|       getAnOperand/getLeftOperand: [GlobalVariableAccess] $global_var
 #   56|       getAnOperand/getRightOperand: [IntegerLiteral] 4
-#   60|   getStmt: [ModuleDefinition] MyModuleInGlobalScope
-#   63|   getStmt: [ModuleDefinition] Test
-#   65|     getStmt: [ModuleDefinition] Foo1
-#   66|       getStmt: [ClassDefinition] Bar
+#   60|   getStmt: [ModuleDeclaration] MyModuleInGlobalScope
+#   63|   getStmt: [ModuleDeclaration] Test
+#   65|     getStmt: [ModuleDeclaration] Foo1
+#   66|       getStmt: [ClassDeclaration] Bar
 #   66|         getScopeExpr: [ConstantReadAccess] Foo1
-#   70|     getStmt: [ModuleDefinition] Foo2
-#   71|       getStmt: [ModuleDefinition] Foo2
-#   72|       getStmt: [ClassDefinition] Bar
+#   70|     getStmt: [ModuleDeclaration] Foo2
+#   71|       getStmt: [ModuleDeclaration] Foo2
+#   72|       getStmt: [ClassDeclaration] Bar
 #   72|         getScopeExpr: [ConstantReadAccess] Foo2
-#   76|     getStmt: [ModuleDefinition] Foo3
+#   76|     getStmt: [ModuleDeclaration] Foo3
 #   77|       getStmt: [AssignExpr] ... = ...
 #   77|         getAnOperand/getLeftOperand: [ConstantAssignment] Foo3
 #   77|         getAnOperand/getRightOperand: [ConstantReadAccess] Object
-#   78|       getStmt: [ClassDefinition] Bar
+#   78|       getStmt: [ClassDeclaration] Bar
 #   78|         getScopeExpr: [ConstantReadAccess] Foo3
 operations/operations.rb:
 #    1| [Toplevel] operations.rb
diff --git a/ql/test/library-tests/ast/constants/constants.expected b/ql/test/library-tests/ast/constants/constants.expected
index 60362f161f4..b77e525a21a 100644
--- a/ql/test/library-tests/ast/constants/constants.expected
+++ b/ql/test/library-tests/ast/constants/constants.expected
@@ -1,9 +1,9 @@
-| constants.rb:1:1:12:3 | ModuleA | write | ModuleA | ModuleDefinition |
-| constants.rb:2:5:3:7 | ClassA | write | ClassA | ClassDefinition |
-| constants.rb:5:5:11:7 | ModuleB | write | ModuleB | ModuleDefinition |
-| constants.rb:6:9:7:11 | ClassB | write | ClassB | ClassDefinition |
+| constants.rb:1:1:12:3 | ModuleA | write | ModuleA | ModuleDeclaration |
+| constants.rb:2:5:3:7 | ClassA | write | ClassA | ClassDeclaration |
+| constants.rb:5:5:11:7 | ModuleB | write | ModuleB | ModuleDeclaration |
+| constants.rb:6:9:7:11 | ClassB | write | ClassB | ClassDeclaration |
 | constants.rb:6:24:6:27 | Base | read | Base | ConstantReadAccess |
-| constants.rb:9:9:10:11 | ClassC | write | ClassC | ClassDefinition |
+| constants.rb:9:9:10:11 | ClassC | write | ClassC | ClassDeclaration |
 | constants.rb:9:24:9:24 | X | read | X | ConstantReadAccess |
 | constants.rb:9:24:9:27 | Y | read | Y | ConstantReadAccess |
 | constants.rb:9:24:9:30 | Z | read | Z | ConstantReadAccess |
@@ -11,9 +11,9 @@
 | constants.rb:17:5:17:9 | Names | write | Names | ConstantAssignment |
 | constants.rb:19:5:19:9 | Names | read | Names | ConstantReadAccess |
 | constants.rb:20:18:20:25 | GREETING | read | GREETING | ConstantReadAccess |
-| constants.rb:28:1:29:3 | ClassD | write | ClassD | ClassDefinition |
+| constants.rb:28:1:29:3 | ClassD | write | ClassD | ClassDeclaration |
 | constants.rb:28:7:28:13 | ModuleA | read | ModuleA | ConstantReadAccess |
-| constants.rb:31:1:32:3 | ModuleC | write | ModuleC | ModuleDefinition |
+| constants.rb:31:1:32:3 | ModuleC | write | ModuleC | ModuleDeclaration |
 | constants.rb:31:8:31:14 | ModuleA | read | ModuleA | ConstantReadAccess |
 | constants.rb:34:1:34:7 | ModuleA | read | ModuleA | ConstantReadAccess |
 | constants.rb:34:1:34:16 | ModuleB | read | ModuleB | ConstantReadAccess |
diff --git a/ql/test/library-tests/ast/modules/classes.expected b/ql/test/library-tests/ast/modules/classes.expected
index 55923386fe4..0691684b668 100644
--- a/ql/test/library-tests/ast/modules/classes.expected
+++ b/ql/test/library-tests/ast/modules/classes.expected
@@ -1,18 +1,18 @@
 classes
-| classes.rb:3:1:4:3 | Foo | ClassDefinition | Foo |
-| classes.rb:7:1:8:3 | Bar | ClassDefinition | Bar |
-| classes.rb:11:1:12:3 | Baz | ClassDefinition | Baz |
-| classes.rb:16:1:17:3 | MyClass | ClassDefinition | MyClass |
-| classes.rb:20:1:37:3 | Wibble | ClassDefinition | Wibble |
-| classes.rb:32:3:33:5 | ClassInWibble | ClassDefinition | ClassInWibble |
-| classes.rb:55:1:56:3 | MyClassInGlobalScope | ClassDefinition | MyClassInGlobalScope |
-| modules.rb:6:5:7:7 | ClassInFooBar | ClassDefinition | ClassInFooBar |
-| modules.rb:19:3:20:5 | ClassInFoo | ClassDefinition | ClassInFoo |
-| modules.rb:30:3:31:5 | ClassInAnotherDefinitionOfFoo | ClassDefinition | ClassInAnotherDefinitionOfFoo |
-| modules.rb:49:3:50:5 | ClassInAnotherDefinitionOfFooBar | ClassDefinition | ClassInAnotherDefinitionOfFooBar |
-| modules.rb:66:5:67:7 | Bar | ClassDefinition | Bar |
-| modules.rb:72:5:73:7 | Bar | ClassDefinition | Bar |
-| modules.rb:78:5:79:7 | Bar | ClassDefinition | Bar |
+| classes.rb:3:1:4:3 | Foo | ClassDeclaration | Foo |
+| classes.rb:7:1:8:3 | Bar | ClassDeclaration | Bar |
+| classes.rb:11:1:12:3 | Baz | ClassDeclaration | Baz |
+| classes.rb:16:1:17:3 | MyClass | ClassDeclaration | MyClass |
+| classes.rb:20:1:37:3 | Wibble | ClassDeclaration | Wibble |
+| classes.rb:32:3:33:5 | ClassInWibble | ClassDeclaration | ClassInWibble |
+| classes.rb:55:1:56:3 | MyClassInGlobalScope | ClassDeclaration | MyClassInGlobalScope |
+| modules.rb:6:5:7:7 | ClassInFooBar | ClassDeclaration | ClassInFooBar |
+| modules.rb:19:3:20:5 | ClassInFoo | ClassDeclaration | ClassInFoo |
+| modules.rb:30:3:31:5 | ClassInAnotherDefinitionOfFoo | ClassDeclaration | ClassInAnotherDefinitionOfFoo |
+| modules.rb:49:3:50:5 | ClassInAnotherDefinitionOfFooBar | ClassDeclaration | ClassInAnotherDefinitionOfFooBar |
+| modules.rb:66:5:67:7 | Bar | ClassDeclaration | Bar |
+| modules.rb:72:5:73:7 | Bar | ClassDeclaration | Bar |
+| modules.rb:78:5:79:7 | Bar | ClassDeclaration | Bar |
 classesWithNameScopeExprs
 | classes.rb:16:1:17:3 | MyClass | classes.rb:16:7:16:14 | MyModule |
 | modules.rb:66:5:67:7 | Bar | modules.rb:66:11:66:14 | Foo1 |
@@ -25,8 +25,8 @@ exprsInClasses
 | classes.rb:20:1:37:3 | Wibble | 1 | classes.rb:25:3:27:5 | method_b | Method |
 | classes.rb:20:1:37:3 | Wibble | 2 | classes.rb:29:3:29:20 | call to some_method_call | MethodCall |
 | classes.rb:20:1:37:3 | Wibble | 3 | classes.rb:30:3:30:19 | ... = ... | AssignExpr |
-| classes.rb:20:1:37:3 | Wibble | 4 | classes.rb:32:3:33:5 | ClassInWibble | ClassDefinition |
-| classes.rb:20:1:37:3 | Wibble | 5 | classes.rb:35:3:36:5 | ModuleInWibble | ModuleDefinition |
+| classes.rb:20:1:37:3 | Wibble | 4 | classes.rb:32:3:33:5 | ClassInWibble | ClassDeclaration |
+| classes.rb:20:1:37:3 | Wibble | 5 | classes.rb:35:3:36:5 | ModuleInWibble | ModuleDeclaration |
 methodsInClasses
 | classes.rb:20:1:37:3 | Wibble | classes.rb:21:3:23:5 | method_a | method_a |
 | classes.rb:20:1:37:3 | Wibble | classes.rb:25:3:27:5 | method_b | method_b |
diff --git a/ql/test/library-tests/ast/modules/classes.ql b/ql/test/library-tests/ast/modules/classes.ql
index ce347830b06..a4b392a32cb 100644
--- a/ql/test/library-tests/ast/modules/classes.ql
+++ b/ql/test/library-tests/ast/modules/classes.ql
@@ -1,27 +1,29 @@
 import ruby
 
-query predicate classes(ClassDefinition c, string pClass, string name) {
+query predicate classes(ClassDeclaration c, string pClass, string name) {
   pClass = c.getAPrimaryQlClass() and name = c.getName()
 }
 
-query predicate classesWithNameScopeExprs(ClassDefinition c, Expr se) { se = c.getScopeExpr() }
+query predicate classesWithNameScopeExprs(ClassDeclaration c, Expr se) { se = c.getScopeExpr() }
 
-query predicate classesWithGlobalNameScopeExprs(ClassDefinition c) { c.hasGlobalScope() }
+query predicate classesWithGlobalNameScopeExprs(ClassDeclaration c) { c.hasGlobalScope() }
 
-query predicate exprsInClasses(ClassDefinition c, int i, Expr e, string eClass) {
+query predicate exprsInClasses(ClassDeclaration c, int i, Expr e, string eClass) {
   e = c.getStmt(i) and eClass = e.getAPrimaryQlClass()
 }
 
-query predicate methodsInClasses(ClassDefinition c, Method m, string name) { m = c.getMethod(name) }
+query predicate methodsInClasses(ClassDeclaration c, Method m, string name) {
+  m = c.getMethod(name)
+}
 
-query predicate classesInClasses(ClassDefinition c, ClassDefinition child, string name) {
+query predicate classesInClasses(ClassDeclaration c, ClassDeclaration child, string name) {
   child = c.getClass(name)
 }
 
-query predicate modulesInClasses(ClassDefinition c, ModuleDefinition m, string name) {
+query predicate modulesInClasses(ClassDeclaration c, ModuleDeclaration m, string name) {
   m = c.getModule(name)
 }
 
-query predicate classesWithASuperclass(ClassDefinition c, Expr scExpr) {
+query predicate classesWithASuperclass(ClassDeclaration c, Expr scExpr) {
   scExpr = c.getSuperclassExpr()
 }
diff --git a/ql/test/library-tests/ast/modules/module_base.expected b/ql/test/library-tests/ast/modules/module_base.expected
index d63da775d77..adf8aa07566 100644
--- a/ql/test/library-tests/ast/modules/module_base.expected
+++ b/ql/test/library-tests/ast/modules/module_base.expected
@@ -1,35 +1,35 @@
 moduleBases
 | classes.rb:2:1:56:3 | classes.rb | Toplevel |
-| classes.rb:3:1:4:3 | Foo | ClassDefinition |
-| classes.rb:7:1:8:3 | Bar | ClassDefinition |
-| classes.rb:11:1:12:3 | Baz | ClassDefinition |
-| classes.rb:15:1:15:20 | MyModule | ModuleDefinition |
-| classes.rb:16:1:17:3 | MyClass | ClassDefinition |
-| classes.rb:20:1:37:3 | Wibble | ClassDefinition |
-| classes.rb:32:3:33:5 | ClassInWibble | ClassDefinition |
-| classes.rb:35:3:36:5 | ModuleInWibble | ModuleDefinition |
-| classes.rb:41:1:52:3 | class << ... | ClassDefinition |
-| classes.rb:55:1:56:3 | MyClassInGlobalScope | ClassDefinition |
-| modules.rb:1:1:2:3 | Empty | ModuleDefinition |
+| classes.rb:3:1:4:3 | Foo | ClassDeclaration |
+| classes.rb:7:1:8:3 | Bar | ClassDeclaration |
+| classes.rb:11:1:12:3 | Baz | ClassDeclaration |
+| classes.rb:15:1:15:20 | MyModule | ModuleDeclaration |
+| classes.rb:16:1:17:3 | MyClass | ClassDeclaration |
+| classes.rb:20:1:37:3 | Wibble | ClassDeclaration |
+| classes.rb:32:3:33:5 | ClassInWibble | ClassDeclaration |
+| classes.rb:35:3:36:5 | ModuleInWibble | ModuleDeclaration |
+| classes.rb:41:1:52:3 | class << ... | ClassDeclaration |
+| classes.rb:55:1:56:3 | MyClassInGlobalScope | ClassDeclaration |
+| modules.rb:1:1:2:3 | Empty | ModuleDeclaration |
 | modules.rb:1:1:82:1 | modules.rb | Toplevel |
-| modules.rb:4:1:24:3 | Foo | ModuleDefinition |
-| modules.rb:5:3:14:5 | Bar | ModuleDefinition |
-| modules.rb:6:5:7:7 | ClassInFooBar | ClassDefinition |
-| modules.rb:19:3:20:5 | ClassInFoo | ClassDefinition |
-| modules.rb:26:1:35:3 | Foo | ModuleDefinition |
-| modules.rb:30:3:31:5 | ClassInAnotherDefinitionOfFoo | ClassDefinition |
-| modules.rb:37:1:46:3 | Bar | ModuleDefinition |
-| modules.rb:48:1:57:3 | Bar | ModuleDefinition |
-| modules.rb:49:3:50:5 | ClassInAnotherDefinitionOfFooBar | ClassDefinition |
-| modules.rb:60:1:61:3 | MyModuleInGlobalScope | ModuleDefinition |
-| modules.rb:63:1:81:3 | Test | ModuleDefinition |
-| modules.rb:65:3:68:5 | Foo1 | ModuleDefinition |
-| modules.rb:66:5:67:7 | Bar | ClassDefinition |
-| modules.rb:70:3:74:5 | Foo2 | ModuleDefinition |
-| modules.rb:71:5:71:19 | Foo2 | ModuleDefinition |
-| modules.rb:72:5:73:7 | Bar | ClassDefinition |
-| modules.rb:76:3:80:5 | Foo3 | ModuleDefinition |
-| modules.rb:78:5:79:7 | Bar | ClassDefinition |
+| modules.rb:4:1:24:3 | Foo | ModuleDeclaration |
+| modules.rb:5:3:14:5 | Bar | ModuleDeclaration |
+| modules.rb:6:5:7:7 | ClassInFooBar | ClassDeclaration |
+| modules.rb:19:3:20:5 | ClassInFoo | ClassDeclaration |
+| modules.rb:26:1:35:3 | Foo | ModuleDeclaration |
+| modules.rb:30:3:31:5 | ClassInAnotherDefinitionOfFoo | ClassDeclaration |
+| modules.rb:37:1:46:3 | Bar | ModuleDeclaration |
+| modules.rb:48:1:57:3 | Bar | ModuleDeclaration |
+| modules.rb:49:3:50:5 | ClassInAnotherDefinitionOfFooBar | ClassDeclaration |
+| modules.rb:60:1:61:3 | MyModuleInGlobalScope | ModuleDeclaration |
+| modules.rb:63:1:81:3 | Test | ModuleDeclaration |
+| modules.rb:65:3:68:5 | Foo1 | ModuleDeclaration |
+| modules.rb:66:5:67:7 | Bar | ClassDeclaration |
+| modules.rb:70:3:74:5 | Foo2 | ModuleDeclaration |
+| modules.rb:71:5:71:19 | Foo2 | ModuleDeclaration |
+| modules.rb:72:5:73:7 | Bar | ClassDeclaration |
+| modules.rb:76:3:80:5 | Foo3 | ModuleDeclaration |
+| modules.rb:78:5:79:7 | Bar | ClassDeclaration |
 | toplevel.rb:1:1:5:23 | toplevel.rb | Toplevel |
 moduleBaseClasses
 | classes.rb:2:1:56:3 | classes.rb | classes.rb:3:1:4:3 | Foo |
diff --git a/ql/test/library-tests/ast/modules/module_base.ql b/ql/test/library-tests/ast/modules/module_base.ql
index 4b41e4ad03b..e95faa62969 100644
--- a/ql/test/library-tests/ast/modules/module_base.ql
+++ b/ql/test/library-tests/ast/modules/module_base.ql
@@ -2,8 +2,8 @@ import ruby
 
 query predicate moduleBases(ModuleBase mb, string pClass) { pClass = mb.getAPrimaryQlClass() }
 
-query predicate moduleBaseClasses(ModuleBase mb, ClassDefinition c) { c = mb.getAClass() }
+query predicate moduleBaseClasses(ModuleBase mb, ClassDeclaration c) { c = mb.getAClass() }
 
 query predicate moduleBaseMethods(ModuleBase mb, Method m) { m = mb.getAMethod() }
 
-query predicate moduleBaseModules(ModuleBase mb, ModuleDefinition m) { m = mb.getAModule() }
+query predicate moduleBaseModules(ModuleBase mb, ModuleDeclaration m) { m = mb.getAModule() }
diff --git a/ql/test/library-tests/ast/modules/modules.expected b/ql/test/library-tests/ast/modules/modules.expected
index 7c877d40c2e..7b232112d6d 100644
--- a/ql/test/library-tests/ast/modules/modules.expected
+++ b/ql/test/library-tests/ast/modules/modules.expected
@@ -1,52 +1,52 @@
 modules
-| classes.rb:15:1:15:20 | MyModule | ModuleDefinition | MyModule |
-| classes.rb:35:3:36:5 | ModuleInWibble | ModuleDefinition | ModuleInWibble |
-| modules.rb:1:1:2:3 | Empty | ModuleDefinition | Empty |
-| modules.rb:4:1:24:3 | Foo | ModuleDefinition | Foo |
-| modules.rb:5:3:14:5 | Bar | ModuleDefinition | Bar |
-| modules.rb:26:1:35:3 | Foo | ModuleDefinition | Foo |
-| modules.rb:37:1:46:3 | Bar | ModuleDefinition | Bar |
-| modules.rb:48:1:57:3 | Bar | ModuleDefinition | Bar |
-| modules.rb:60:1:61:3 | MyModuleInGlobalScope | ModuleDefinition | MyModuleInGlobalScope |
-| modules.rb:63:1:81:3 | Test | ModuleDefinition | Test |
-| modules.rb:65:3:68:5 | Foo1 | ModuleDefinition | Foo1 |
-| modules.rb:70:3:74:5 | Foo2 | ModuleDefinition | Foo2 |
-| modules.rb:71:5:71:19 | Foo2 | ModuleDefinition | Foo2 |
-| modules.rb:76:3:80:5 | Foo3 | ModuleDefinition | Foo3 |
+| classes.rb:15:1:15:20 | MyModule | ModuleDeclaration | MyModule |
+| classes.rb:35:3:36:5 | ModuleInWibble | ModuleDeclaration | ModuleInWibble |
+| modules.rb:1:1:2:3 | Empty | ModuleDeclaration | Empty |
+| modules.rb:4:1:24:3 | Foo | ModuleDeclaration | Foo |
+| modules.rb:5:3:14:5 | Bar | ModuleDeclaration | Bar |
+| modules.rb:26:1:35:3 | Foo | ModuleDeclaration | Foo |
+| modules.rb:37:1:46:3 | Bar | ModuleDeclaration | Bar |
+| modules.rb:48:1:57:3 | Bar | ModuleDeclaration | Bar |
+| modules.rb:60:1:61:3 | MyModuleInGlobalScope | ModuleDeclaration | MyModuleInGlobalScope |
+| modules.rb:63:1:81:3 | Test | ModuleDeclaration | Test |
+| modules.rb:65:3:68:5 | Foo1 | ModuleDeclaration | Foo1 |
+| modules.rb:70:3:74:5 | Foo2 | ModuleDeclaration | Foo2 |
+| modules.rb:71:5:71:19 | Foo2 | ModuleDeclaration | Foo2 |
+| modules.rb:76:3:80:5 | Foo3 | ModuleDeclaration | Foo3 |
 modulesWithScopeExprs
 | modules.rb:48:1:57:3 | Bar | modules.rb:48:8:48:10 | Foo |
 modulesWithGlobalNameScopeExprs
 | modules.rb:60:1:61:3 | MyModuleInGlobalScope |
 exprsInModules
-| modules.rb:4:1:24:3 | Foo | 0 | modules.rb:5:3:14:5 | Bar | ModuleDefinition |
+| modules.rb:4:1:24:3 | Foo | 0 | modules.rb:5:3:14:5 | Bar | ModuleDeclaration |
 | modules.rb:4:1:24:3 | Foo | 1 | modules.rb:16:3:17:5 | method_in_foo | Method |
-| modules.rb:4:1:24:3 | Foo | 2 | modules.rb:19:3:20:5 | ClassInFoo | ClassDefinition |
+| modules.rb:4:1:24:3 | Foo | 2 | modules.rb:19:3:20:5 | ClassInFoo | ClassDeclaration |
 | modules.rb:4:1:24:3 | Foo | 3 | modules.rb:22:3:22:19 | call to puts | MethodCall |
 | modules.rb:4:1:24:3 | Foo | 4 | modules.rb:23:3:23:17 | ... = ... | AssignExpr |
-| modules.rb:5:3:14:5 | Bar | 0 | modules.rb:6:5:7:7 | ClassInFooBar | ClassDefinition |
+| modules.rb:5:3:14:5 | Bar | 0 | modules.rb:6:5:7:7 | ClassInFooBar | ClassDeclaration |
 | modules.rb:5:3:14:5 | Bar | 1 | modules.rb:9:5:10:7 | method_in_foo_bar | Method |
 | modules.rb:5:3:14:5 | Bar | 2 | modules.rb:12:5:12:26 | call to puts | MethodCall |
 | modules.rb:5:3:14:5 | Bar | 3 | modules.rb:13:5:13:19 | ... = ... | AssignExpr |
 | modules.rb:26:1:35:3 | Foo | 0 | modules.rb:27:3:28:5 | method_in_another_definition_of_foo | Method |
-| modules.rb:26:1:35:3 | Foo | 1 | modules.rb:30:3:31:5 | ClassInAnotherDefinitionOfFoo | ClassDefinition |
+| modules.rb:26:1:35:3 | Foo | 1 | modules.rb:30:3:31:5 | ClassInAnotherDefinitionOfFoo | ClassDeclaration |
 | modules.rb:26:1:35:3 | Foo | 2 | modules.rb:33:3:33:25 | call to puts | MethodCall |
 | modules.rb:26:1:35:3 | Foo | 3 | modules.rb:34:3:34:17 | ... = ... | AssignExpr |
 | modules.rb:37:1:46:3 | Bar | 0 | modules.rb:38:3:39:5 | method_a | Method |
 | modules.rb:37:1:46:3 | Bar | 1 | modules.rb:41:3:42:5 | method_b | Method |
 | modules.rb:37:1:46:3 | Bar | 2 | modules.rb:44:3:44:19 | call to puts | MethodCall |
 | modules.rb:37:1:46:3 | Bar | 3 | modules.rb:45:3:45:17 | ... = ... | AssignExpr |
-| modules.rb:48:1:57:3 | Bar | 0 | modules.rb:49:3:50:5 | ClassInAnotherDefinitionOfFooBar | ClassDefinition |
+| modules.rb:48:1:57:3 | Bar | 0 | modules.rb:49:3:50:5 | ClassInAnotherDefinitionOfFooBar | ClassDeclaration |
 | modules.rb:48:1:57:3 | Bar | 1 | modules.rb:52:3:53:5 | method_in_another_definition_of_foo_bar | Method |
 | modules.rb:48:1:57:3 | Bar | 2 | modules.rb:55:3:55:30 | call to puts | MethodCall |
 | modules.rb:48:1:57:3 | Bar | 3 | modules.rb:56:3:56:17 | ... = ... | AssignExpr |
-| modules.rb:63:1:81:3 | Test | 0 | modules.rb:65:3:68:5 | Foo1 | ModuleDefinition |
-| modules.rb:63:1:81:3 | Test | 1 | modules.rb:70:3:74:5 | Foo2 | ModuleDefinition |
-| modules.rb:63:1:81:3 | Test | 2 | modules.rb:76:3:80:5 | Foo3 | ModuleDefinition |
-| modules.rb:65:3:68:5 | Foo1 | 0 | modules.rb:66:5:67:7 | Bar | ClassDefinition |
-| modules.rb:70:3:74:5 | Foo2 | 0 | modules.rb:71:5:71:19 | Foo2 | ModuleDefinition |
-| modules.rb:70:3:74:5 | Foo2 | 1 | modules.rb:72:5:73:7 | Bar | ClassDefinition |
+| modules.rb:63:1:81:3 | Test | 0 | modules.rb:65:3:68:5 | Foo1 | ModuleDeclaration |
+| modules.rb:63:1:81:3 | Test | 1 | modules.rb:70:3:74:5 | Foo2 | ModuleDeclaration |
+| modules.rb:63:1:81:3 | Test | 2 | modules.rb:76:3:80:5 | Foo3 | ModuleDeclaration |
+| modules.rb:65:3:68:5 | Foo1 | 0 | modules.rb:66:5:67:7 | Bar | ClassDeclaration |
+| modules.rb:70:3:74:5 | Foo2 | 0 | modules.rb:71:5:71:19 | Foo2 | ModuleDeclaration |
+| modules.rb:70:3:74:5 | Foo2 | 1 | modules.rb:72:5:73:7 | Bar | ClassDeclaration |
 | modules.rb:76:3:80:5 | Foo3 | 0 | modules.rb:77:5:77:17 | ... = ... | AssignExpr |
-| modules.rb:76:3:80:5 | Foo3 | 1 | modules.rb:78:5:79:7 | Bar | ClassDefinition |
+| modules.rb:76:3:80:5 | Foo3 | 1 | modules.rb:78:5:79:7 | Bar | ClassDeclaration |
 methodsInModules
 | modules.rb:4:1:24:3 | Foo | modules.rb:16:3:17:5 | method_in_foo | method_in_foo |
 | modules.rb:5:3:14:5 | Bar | modules.rb:9:5:10:7 | method_in_foo_bar | method_in_foo_bar |
diff --git a/ql/test/library-tests/ast/modules/modules.ql b/ql/test/library-tests/ast/modules/modules.ql
index ba42d6365bf..21adb23e633 100644
--- a/ql/test/library-tests/ast/modules/modules.ql
+++ b/ql/test/library-tests/ast/modules/modules.ql
@@ -1,26 +1,26 @@
 import ruby
 
-query predicate modules(ModuleDefinition m, string pClass, string name) {
+query predicate modules(ModuleDeclaration m, string pClass, string name) {
   pClass = m.getAPrimaryQlClass() and name = m.getName()
 }
 
-query predicate modulesWithScopeExprs(ModuleDefinition m, Expr se) { se = m.getScopeExpr() }
+query predicate modulesWithScopeExprs(ModuleDeclaration m, Expr se) { se = m.getScopeExpr() }
 
-query predicate modulesWithGlobalNameScopeExprs(ModuleDefinition m) { m.hasGlobalScope() }
+query predicate modulesWithGlobalNameScopeExprs(ModuleDeclaration m) { m.hasGlobalScope() }
 
-query predicate exprsInModules(ModuleDefinition m, int i, Expr e, string eClass) {
+query predicate exprsInModules(ModuleDeclaration m, int i, Expr e, string eClass) {
   e = m.getStmt(i) and eClass = e.getAPrimaryQlClass()
 }
 
-query predicate methodsInModules(ModuleDefinition mod, Method method, string name) {
+query predicate methodsInModules(ModuleDeclaration mod, Method method, string name) {
   method = mod.getMethod(name)
 }
 
-query predicate classesInModules(ModuleDefinition mod, ClassDefinition klass, string name) {
+query predicate classesInModules(ModuleDeclaration mod, ClassDeclaration klass, string name) {
   klass = mod.getClass(name)
 }
 
-query predicate modulesInModules(ModuleDefinition mod, ModuleDefinition child, string name) {
+query predicate modulesInModules(ModuleDeclaration mod, ModuleDeclaration child, string name) {
   child = mod.getModule(name)
 }
 
diff --git a/ql/test/library-tests/ast/modules/singleton_classes.expected b/ql/test/library-tests/ast/modules/singleton_classes.expected
index c5746395fba..f1c13b476d6 100644
--- a/ql/test/library-tests/ast/modules/singleton_classes.expected
+++ b/ql/test/library-tests/ast/modules/singleton_classes.expected
@@ -1,5 +1,5 @@
 singletonClasses
-| classes.rb:41:1:52:3 | class << ... | ClassDefinition | classes.rb:41:10:41:10 | x |
+| classes.rb:41:1:52:3 | class << ... | ClassDeclaration | classes.rb:41:10:41:10 | x |
 exprsInSingletonClasses
 | classes.rb:41:1:52:3 | class << ... | 0 | classes.rb:42:3:44:5 | length | Method |
 | classes.rb:41:1:52:3 | class << ... | 1 | classes.rb:46:3:48:5 | wibble | Method |

From 84f6e902eaadb6fb7fb89c1aeb9abb47b1f257fd Mon Sep 17 00:00:00 2001
From: Arthur Baars <aibaars@github.com>
Date: Wed, 7 Apr 2021 16:28:20 +0200
Subject: [PATCH 08/14] AST: move some scope related methods to AstNode

---
 ql/src/codeql_ruby/AST.qll                 | 15 +++++++++++++++
 ql/src/codeql_ruby/ast/Scope.qll           |  9 ---------
 ql/src/codeql_ruby/ast/internal/Module.qll | 22 +++++++++-------------
 3 files changed, 24 insertions(+), 22 deletions(-)

diff --git a/ql/src/codeql_ruby/AST.qll b/ql/src/codeql_ruby/AST.qll
index 11f201ac6dd..518f42c69d9 100644
--- a/ql/src/codeql_ruby/AST.qll
+++ b/ql/src/codeql_ruby/AST.qll
@@ -13,6 +13,7 @@ import ast.Scope
 import ast.Statement
 import ast.Variable
 private import ast.internal.AST
+private import ast.internal.Scope
 
 /**
  * A node in the abstract syntax tree. This class is the base class for all Ruby
@@ -28,6 +29,20 @@ class AstNode extends TAstNode {
    */
   string getAPrimaryQlClass() { result = "???" }
 
+  /** Gets the enclosing module, if any. */
+  ModuleBase getEnclosingModule() {
+    exists(Scope::Range s |
+      s = scopeOf(toGenerated(this)) and toGenerated(result) = s.getEnclosingModule()
+    )
+  }
+
+  /** Gets the enclosing method, if any. */
+  MethodBase getEnclosingMethod() {
+    exists(Scope::Range s |
+      s = scopeOf(toGenerated(this)) and toGenerated(result) = s.getEnclosingMethod()
+    )
+  }
+
   /** Gets a textual representation of this node. */
   cached
   string toString() { none() }
diff --git a/ql/src/codeql_ruby/ast/Scope.qll b/ql/src/codeql_ruby/ast/Scope.qll
index 2f4b941a56e..a726236e410 100644
--- a/ql/src/codeql_ruby/ast/Scope.qll
+++ b/ql/src/codeql_ruby/ast/Scope.qll
@@ -8,18 +8,9 @@ class Scope extends AstNode, TScopeType {
 
   Scope() { range = toGenerated(this) }
 
-  /** Gets the enclosing module, if any. */
-  ModuleBase getEnclosingModule() { toGenerated(result) = range.getEnclosingModule() }
-
-  /** Gets the enclosing method, if any. */
-  MethodBase getEnclosingMethod() { toGenerated(result) = range.getEnclosingMethod() }
-
   /** Gets the scope in which this scope is nested, if any. */
   Scope getOuterScope() { toGenerated(result) = range.getOuterScope() }
 
-  /** Gets the scope in which this scope is nested, if any. */
-  AstNode getADescendant() { range = scopeOf(toGenerated(result)) }
-
   /** Gets a variable that is declared in this scope. */
   final Variable getAVariable() { result.getDeclaringScope() = this }
 
diff --git a/ql/src/codeql_ruby/ast/internal/Module.qll b/ql/src/codeql_ruby/ast/internal/Module.qll
index 84a2adc68d7..b8a45fdd610 100644
--- a/ql/src/codeql_ruby/ast/internal/Module.qll
+++ b/ql/src/codeql_ruby/ast/internal/Module.qll
@@ -23,7 +23,7 @@ private predicate isToplevel(ConstantAccess n) {
   (
     n.hasGlobalScope()
     or
-    exists(Scope x | x.getADescendant() = n and x.getEnclosingModule() instanceof Toplevel)
+    n.getEnclosingModule() instanceof Toplevel
   )
 }
 
@@ -48,7 +48,7 @@ private string resolveScopeExpr0(ConstantReadAccess n) {
 ModuleBase enclosing(ModuleBase m, int level) {
   result = m and level = 0
   or
-  result = enclosing(m.getOuterScope().getEnclosingModule(), level - 1)
+  result = enclosing(m.getEnclosingModule(), level - 1)
 }
 
 bindingset[qualifier, name]
@@ -59,9 +59,8 @@ private string scopeAppend(string qualifier, string name) {
 private string resolveRelativeToEnclosing(ConstantAccess n, int i) {
   not isToplevel(n) and
   not exists(n.getScopeExpr()) and
-  exists(Scope s, ModuleBase enclosing |
-    n = s.getADescendant() and
-    enclosing = enclosing(s.getEnclosingModule(), i) and
+  exists(ModuleBase enclosing |
+    enclosing = enclosing(n.getEnclosingModule(), i) and
     result = scopeAppend(constantDefinition0(enclosing), n.getName()) and
     (result = builtin() or result = constantDefinition0(_) or n instanceof ConstantWriteAccess)
   )
@@ -75,12 +74,12 @@ private class IncludeOrPrependCall extends MethodCall {
   string getTarget() {
     result = resolveScopeExpr0(this.getReceiver())
     or
-    exists(Scope s |
-      s.getADescendant() = this and
+    exists(ModuleBase enclosing |
+      enclosing = this.getEnclosingModule() and
       (
-        result = constantDefinition0(s.getEnclosingModule())
+        result = constantDefinition0(enclosing)
         or
-        result = "Object" and s.getEnclosingModule() instanceof Toplevel
+        result = "Object" and enclosing instanceof Toplevel
       )
     |
       this.getReceiver() instanceof Self
@@ -171,10 +170,7 @@ private string containsIgnoringSuper(string qname, string name) {
 private string resolveRelativeToAncestors(ConstantReadAccess n) {
   not isToplevel(n) and
   not exists(n.getScopeExpr()) and
-  exists(Scope s, ModuleBase enclosing |
-    n = s.getADescendant() and
-    enclosing = s.getEnclosingModule()
-  |
+  exists(ModuleBase enclosing | enclosing = n.getEnclosingModule() |
     result = contains(constantDefinition(enclosing), n.getName())
     or
     enclosing instanceof Toplevel and result = contains("Object", n.getName())

From 039e8b36a5011d18d8ca74aa55f3ba7e23e54f6a Mon Sep 17 00:00:00 2001
From: Arthur Baars <aibaars@github.com>
Date: Wed, 7 Apr 2021 17:27:33 +0200
Subject: [PATCH 09/14] Add some include/prepend tests

---
 ql/test/library-tests/ast/Ast.expected        | 15 ++++++++++
 .../ast/modules/module_base.expected          | 28 +++++++++++++------
 .../ast/modules/modules.expected              | 26 ++++++++++++++++-
 ql/test/library-tests/ast/modules/modules.rb  | 17 +++++++++++
 .../ast/modules/toplevel.expected             |  2 +-
 5 files changed, 78 insertions(+), 10 deletions(-)

diff --git a/ql/test/library-tests/ast/Ast.expected b/ql/test/library-tests/ast/Ast.expected
index ae82f967178..30a91419de1 100644
--- a/ql/test/library-tests/ast/Ast.expected
+++ b/ql/test/library-tests/ast/Ast.expected
@@ -1384,6 +1384,21 @@ modules/modules.rb:
 #   77|         getAnOperand/getRightOperand: [ConstantReadAccess] Object
 #   78|       getStmt: [ClassDeclaration] Bar
 #   78|         getScopeExpr: [ConstantReadAccess] Foo3
+#   83|   getStmt: [ModuleDeclaration] IncludeTest
+#   84|     getStmt: [MethodCall] call to include
+#   84|       getArgument: [ConstantReadAccess] Test
+#   85|     getStmt: [ModuleDeclaration] Y
+#   85|       getScopeExpr: [ConstantReadAccess] Foo1
+#   89|   getStmt: [ModuleDeclaration] IncludeTest2
+#   90|     getStmt: [MethodCall] call to include
+#   90|       getArgument: [ConstantReadAccess] Test
+#   91|     getStmt: [ModuleDeclaration] Z
+#   91|       getScopeExpr: [ConstantReadAccess] Foo1
+#   95|   getStmt: [ModuleDeclaration] PrependTest
+#   96|     getStmt: [MethodCall] call to prepend
+#   96|       getArgument: [ConstantReadAccess] Test
+#   97|     getStmt: [ModuleDeclaration] Y
+#   97|       getScopeExpr: [ConstantReadAccess] Foo2
 operations/operations.rb:
 #    1| [Toplevel] operations.rb
 #    3|   getStmt: [AssignExpr] ... = ...
diff --git a/ql/test/library-tests/ast/modules/module_base.expected b/ql/test/library-tests/ast/modules/module_base.expected
index adf8aa07566..647f94c86f5 100644
--- a/ql/test/library-tests/ast/modules/module_base.expected
+++ b/ql/test/library-tests/ast/modules/module_base.expected
@@ -11,7 +11,7 @@ moduleBases
 | classes.rb:41:1:52:3 | class << ... | ClassDeclaration |
 | classes.rb:55:1:56:3 | MyClassInGlobalScope | ClassDeclaration |
 | modules.rb:1:1:2:3 | Empty | ModuleDeclaration |
-| modules.rb:1:1:82:1 | modules.rb | Toplevel |
+| modules.rb:1:1:99:3 | modules.rb | Toplevel |
 | modules.rb:4:1:24:3 | Foo | ModuleDeclaration |
 | modules.rb:5:3:14:5 | Bar | ModuleDeclaration |
 | modules.rb:6:5:7:7 | ClassInFooBar | ClassDeclaration |
@@ -30,6 +30,12 @@ moduleBases
 | modules.rb:72:5:73:7 | Bar | ClassDeclaration |
 | modules.rb:76:3:80:5 | Foo3 | ModuleDeclaration |
 | modules.rb:78:5:79:7 | Bar | ClassDeclaration |
+| modules.rb:83:1:87:3 | IncludeTest | ModuleDeclaration |
+| modules.rb:85:3:86:5 | Y | ModuleDeclaration |
+| modules.rb:89:1:93:3 | IncludeTest2 | ModuleDeclaration |
+| modules.rb:91:3:92:5 | Z | ModuleDeclaration |
+| modules.rb:95:1:99:3 | PrependTest | ModuleDeclaration |
+| modules.rb:97:3:98:5 | Y | ModuleDeclaration |
 | toplevel.rb:1:1:5:23 | toplevel.rb | Toplevel |
 moduleBaseClasses
 | classes.rb:2:1:56:3 | classes.rb | classes.rb:3:1:4:3 | Foo |
@@ -60,15 +66,21 @@ moduleBaseMethods
 moduleBaseModules
 | classes.rb:2:1:56:3 | classes.rb | classes.rb:15:1:15:20 | MyModule |
 | classes.rb:20:1:37:3 | Wibble | classes.rb:35:3:36:5 | ModuleInWibble |
-| modules.rb:1:1:82:1 | modules.rb | modules.rb:1:1:2:3 | Empty |
-| modules.rb:1:1:82:1 | modules.rb | modules.rb:4:1:24:3 | Foo |
-| modules.rb:1:1:82:1 | modules.rb | modules.rb:26:1:35:3 | Foo |
-| modules.rb:1:1:82:1 | modules.rb | modules.rb:37:1:46:3 | Bar |
-| modules.rb:1:1:82:1 | modules.rb | modules.rb:48:1:57:3 | Bar |
-| modules.rb:1:1:82:1 | modules.rb | modules.rb:60:1:61:3 | MyModuleInGlobalScope |
-| modules.rb:1:1:82:1 | modules.rb | modules.rb:63:1:81:3 | Test |
+| modules.rb:1:1:99:3 | modules.rb | modules.rb:1:1:2:3 | Empty |
+| modules.rb:1:1:99:3 | modules.rb | modules.rb:4:1:24:3 | Foo |
+| modules.rb:1:1:99:3 | modules.rb | modules.rb:26:1:35:3 | Foo |
+| modules.rb:1:1:99:3 | modules.rb | modules.rb:37:1:46:3 | Bar |
+| modules.rb:1:1:99:3 | modules.rb | modules.rb:48:1:57:3 | Bar |
+| modules.rb:1:1:99:3 | modules.rb | modules.rb:60:1:61:3 | MyModuleInGlobalScope |
+| modules.rb:1:1:99:3 | modules.rb | modules.rb:63:1:81:3 | Test |
+| modules.rb:1:1:99:3 | modules.rb | modules.rb:83:1:87:3 | IncludeTest |
+| modules.rb:1:1:99:3 | modules.rb | modules.rb:89:1:93:3 | IncludeTest2 |
+| modules.rb:1:1:99:3 | modules.rb | modules.rb:95:1:99:3 | PrependTest |
 | modules.rb:4:1:24:3 | Foo | modules.rb:5:3:14:5 | Bar |
 | modules.rb:63:1:81:3 | Test | modules.rb:65:3:68:5 | Foo1 |
 | modules.rb:63:1:81:3 | Test | modules.rb:70:3:74:5 | Foo2 |
 | modules.rb:63:1:81:3 | Test | modules.rb:76:3:80:5 | Foo3 |
 | modules.rb:70:3:74:5 | Foo2 | modules.rb:71:5:71:19 | Foo2 |
+| modules.rb:83:1:87:3 | IncludeTest | modules.rb:85:3:86:5 | Y |
+| modules.rb:89:1:93:3 | IncludeTest2 | modules.rb:91:3:92:5 | Z |
+| modules.rb:95:1:99:3 | PrependTest | modules.rb:97:3:98:5 | Y |
diff --git a/ql/test/library-tests/ast/modules/modules.expected b/ql/test/library-tests/ast/modules/modules.expected
index 7b232112d6d..15d0da66801 100644
--- a/ql/test/library-tests/ast/modules/modules.expected
+++ b/ql/test/library-tests/ast/modules/modules.expected
@@ -13,8 +13,17 @@ modules
 | modules.rb:70:3:74:5 | Foo2 | ModuleDeclaration | Foo2 |
 | modules.rb:71:5:71:19 | Foo2 | ModuleDeclaration | Foo2 |
 | modules.rb:76:3:80:5 | Foo3 | ModuleDeclaration | Foo3 |
+| modules.rb:83:1:87:3 | IncludeTest | ModuleDeclaration | IncludeTest |
+| modules.rb:85:3:86:5 | Y | ModuleDeclaration | Y |
+| modules.rb:89:1:93:3 | IncludeTest2 | ModuleDeclaration | IncludeTest2 |
+| modules.rb:91:3:92:5 | Z | ModuleDeclaration | Z |
+| modules.rb:95:1:99:3 | PrependTest | ModuleDeclaration | PrependTest |
+| modules.rb:97:3:98:5 | Y | ModuleDeclaration | Y |
 modulesWithScopeExprs
 | modules.rb:48:1:57:3 | Bar | modules.rb:48:8:48:10 | Foo |
+| modules.rb:85:3:86:5 | Y | modules.rb:85:10:85:13 | Foo1 |
+| modules.rb:91:3:92:5 | Z | modules.rb:91:10:91:13 | Foo1 |
+| modules.rb:97:3:98:5 | Y | modules.rb:97:10:97:13 | Foo2 |
 modulesWithGlobalNameScopeExprs
 | modules.rb:60:1:61:3 | MyModuleInGlobalScope |
 exprsInModules
@@ -47,6 +56,12 @@ exprsInModules
 | modules.rb:70:3:74:5 | Foo2 | 1 | modules.rb:72:5:73:7 | Bar | ClassDeclaration |
 | modules.rb:76:3:80:5 | Foo3 | 0 | modules.rb:77:5:77:17 | ... = ... | AssignExpr |
 | modules.rb:76:3:80:5 | Foo3 | 1 | modules.rb:78:5:79:7 | Bar | ClassDeclaration |
+| modules.rb:83:1:87:3 | IncludeTest | 0 | modules.rb:84:3:84:16 | call to include | MethodCall |
+| modules.rb:83:1:87:3 | IncludeTest | 1 | modules.rb:85:3:86:5 | Y | ModuleDeclaration |
+| modules.rb:89:1:93:3 | IncludeTest2 | 0 | modules.rb:90:3:90:14 | call to include | MethodCall |
+| modules.rb:89:1:93:3 | IncludeTest2 | 1 | modules.rb:91:3:92:5 | Z | ModuleDeclaration |
+| modules.rb:95:1:99:3 | PrependTest | 0 | modules.rb:96:3:96:16 | call to prepend | MethodCall |
+| modules.rb:95:1:99:3 | PrependTest | 1 | modules.rb:97:3:98:5 | Y | ModuleDeclaration |
 methodsInModules
 | modules.rb:4:1:24:3 | Foo | modules.rb:16:3:17:5 | method_in_foo | method_in_foo |
 | modules.rb:5:3:14:5 | Bar | modules.rb:9:5:10:7 | method_in_foo_bar | method_in_foo_bar |
@@ -68,6 +83,9 @@ modulesInModules
 | modules.rb:63:1:81:3 | Test | modules.rb:70:3:74:5 | Foo2 | Foo2 |
 | modules.rb:63:1:81:3 | Test | modules.rb:76:3:80:5 | Foo3 | Foo3 |
 | modules.rb:70:3:74:5 | Foo2 | modules.rb:71:5:71:19 | Foo2 | Foo2 |
+| modules.rb:83:1:87:3 | IncludeTest | modules.rb:85:3:86:5 | Y | Y |
+| modules.rb:89:1:93:3 | IncludeTest2 | modules.rb:91:3:92:5 | Z | Z |
+| modules.rb:95:1:99:3 | PrependTest | modules.rb:97:3:98:5 | Y | Y |
 moduleTypes
 | classes.rb:2:1:56:3 | classes.rb | file://:0:0:0:0 | Object |
 | classes.rb:3:1:4:3 | Foo | modules.rb:4:1:24:3 | Foo |
@@ -80,7 +98,7 @@ moduleTypes
 | classes.rb:35:3:36:5 | ModuleInWibble | classes.rb:35:3:36:5 | Wibble::ModuleInWibble |
 | classes.rb:55:1:56:3 | MyClassInGlobalScope | classes.rb:55:1:56:3 | MyClassInGlobalScope |
 | modules.rb:1:1:2:3 | Empty | modules.rb:1:1:2:3 | Empty |
-| modules.rb:1:1:82:1 | modules.rb | file://:0:0:0:0 | Object |
+| modules.rb:1:1:99:3 | modules.rb | file://:0:0:0:0 | Object |
 | modules.rb:4:1:24:3 | Foo | modules.rb:4:1:24:3 | Foo |
 | modules.rb:5:3:14:5 | Bar | modules.rb:5:3:14:5 | Foo::Bar |
 | modules.rb:6:5:7:7 | ClassInFooBar | modules.rb:6:5:7:7 | Foo::Bar::ClassInFooBar |
@@ -99,4 +117,10 @@ moduleTypes
 | modules.rb:72:5:73:7 | Bar | modules.rb:72:5:73:7 | Test::Foo2::Foo2::Bar |
 | modules.rb:76:3:80:5 | Foo3 | modules.rb:76:3:80:5 | Test::Foo3 |
 | modules.rb:78:5:79:7 | Bar | modules.rb:37:1:46:3 | Bar |
+| modules.rb:83:1:87:3 | IncludeTest | modules.rb:83:1:87:3 | IncludeTest |
+| modules.rb:85:3:86:5 | Y | modules.rb:85:3:86:5 | Test::Foo1::Y |
+| modules.rb:89:1:93:3 | IncludeTest2 | modules.rb:89:1:93:3 | IncludeTest2 |
+| modules.rb:91:3:92:5 | Z | modules.rb:91:3:92:5 | ...::Z |
+| modules.rb:95:1:99:3 | PrependTest | modules.rb:95:1:99:3 | PrependTest |
+| modules.rb:97:3:98:5 | Y | modules.rb:97:3:98:5 | Test::Foo2::Y |
 | toplevel.rb:1:1:5:23 | toplevel.rb | file://:0:0:0:0 | Object |
diff --git a/ql/test/library-tests/ast/modules/modules.rb b/ql/test/library-tests/ast/modules/modules.rb
index 2463a699d4d..f1e08d1bf47 100644
--- a/ql/test/library-tests/ast/modules/modules.rb
+++ b/ql/test/library-tests/ast/modules/modules.rb
@@ -80,3 +80,20 @@ module Test
   end
 end
 
+module IncludeTest
+  include ::Test
+  module Foo1::Y
+  end
+end
+
+module IncludeTest2
+  include Test
+  module Foo1::Z
+  end
+end
+
+module PrependTest
+  prepend ::Test
+  module Foo2::Y
+  end
+end
\ No newline at end of file
diff --git a/ql/test/library-tests/ast/modules/toplevel.expected b/ql/test/library-tests/ast/modules/toplevel.expected
index dd4576a21a0..e189e08db1b 100644
--- a/ql/test/library-tests/ast/modules/toplevel.expected
+++ b/ql/test/library-tests/ast/modules/toplevel.expected
@@ -1,6 +1,6 @@
 toplevel
 | classes.rb:2:1:56:3 | classes.rb | Toplevel |
-| modules.rb:1:1:82:1 | modules.rb | Toplevel |
+| modules.rb:1:1:99:3 | modules.rb | Toplevel |
 | toplevel.rb:1:1:5:23 | toplevel.rb | Toplevel |
 beginBlocks
 | toplevel.rb:1:1:5:23 | toplevel.rb | 0 | toplevel.rb:5:1:5:22 | BEGIN { ... } |

From ceb2eb21d8500b3a1273570f779370554cd2c4ab Mon Sep 17 00:00:00 2001
From: Arthur Baars <aibaars@github.com>
Date: Thu, 8 Apr 2021 15:11:57 +0200
Subject: [PATCH 10/14] Address comments

---
 ql/src/codeql_ruby/ast/Module.qll                             | 2 --
 .../codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll | 4 ++--
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/ql/src/codeql_ruby/ast/Module.qll b/ql/src/codeql_ruby/ast/Module.qll
index 98aee1e1746..6616e29a899 100644
--- a/ql/src/codeql_ruby/ast/Module.qll
+++ b/ql/src/codeql_ruby/ast/Module.qll
@@ -8,8 +8,6 @@ private import internal.TreeSitter
  * A representation of a run-time `module` or `class` value.
  */
 class Module extends TModule {
-  Module() { this = TResolved(_) or this = TUnresolved(_) }
-
   /** Get a declaration of this module, if any. */
   ModuleBase getADeclaration() { result.getModule() = this }
 
diff --git a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll
index 32ccfb319e9..a648a2c1b71 100644
--- a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll
+++ b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll
@@ -598,7 +598,7 @@ module Trees {
 
   private class CharacterTree extends LeafTree, CharacterLiteral { }
 
-  private class ClassTree extends BodyStmtPreOrderTree, ClassDeclaration {
+  private class ClassDeclarationTree extends BodyStmtPreOrderTree, ClassDeclaration {
     /** Gets the `i`th child in the body of this block. */
     final override AstNode getBodyChild(int i, boolean rescuable) {
       result = this.getScopeExpr() and i = 0 and rescuable = false
@@ -934,7 +934,7 @@ module Trees {
     }
   }
 
-  private class ModuleTree extends BodyStmtPreOrderTree, ModuleDeclaration {
+  private class ModuleDeclarationTree extends BodyStmtPreOrderTree, ModuleDeclaration {
     /** Gets the `i`th child in the body of this block. */
     final override AstNode getBodyChild(int i, boolean rescuable) {
       result = this.getScopeExpr() and i = 0 and rescuable = false

From 2db999d0dae249c6e9fa0fa5a4e392837674e6cc Mon Sep 17 00:00:00 2001
From: Arthur Baars <aibaars@github.com>
Date: Thu, 8 Apr 2021 15:17:31 +0200
Subject: [PATCH 11/14] Improve module resolution

---
 ql/src/codeql_ruby/ast/internal/Module.qll    | 199 ++++++++----------
 .../ast/modules/modules.expected              |   2 +-
 2 files changed, 86 insertions(+), 115 deletions(-)

diff --git a/ql/src/codeql_ruby/ast/internal/Module.qll b/ql/src/codeql_ruby/ast/internal/Module.qll
index b8a45fdd610..e204e6aa24b 100644
--- a/ql/src/codeql_ruby/ast/internal/Module.qll
+++ b/ql/src/codeql_ruby/ast/internal/Module.qll
@@ -27,61 +27,116 @@ private predicate isToplevel(ConstantAccess n) {
   )
 }
 
-private string constantDefinition0(ConstantWriteAccess n) { result = qualifiedNameForConstant0(n) }
+string constantDefinition(ConstantWriteAccess n) {
+  isToplevel(n) and result = n.getName()
+  or
+  not isToplevel(n) and
+  not exists(n.getScopeExpr()) and
+  result = scopeAppend(constantDefinition(n.getEnclosingModule()), n.getName())
+  or
+  result = scopeAppend(resolveScopeExpr(n.getScopeExpr()), n.getName())
+}
+
+private predicate isDefinedConstant(string qualifiedModuleName) {
+  qualifiedModuleName = [builtin(), constantDefinition0(_)]
+}
 
 /**
  * Resolve a scope expression
  */
-private string resolveScopeExpr0(ConstantReadAccess n) {
-  exists(string qname | qname = qualifiedNameForConstant0(n) |
-    qname = builtin() and result = qname
+private string resolveScopeExpr(ConstantReadAccess r) {
+  exists(string container |
+    container =
+      min(string c, int p |
+        isDefinedConstant(c) and
+        c = resolveScopeExpr(r, p)
+      |
+        c order by p
+      )
+  |
+    result = container and
+    container = [builtin(), constantDefinition(any(Namespace x))]
     or
-    not qname = builtin() and
-    exists(ConstantWriteAccess def | qname = constantDefinition0(def) |
-      result = qname and def instanceof Namespace
-      or
-      result = resolveScopeExpr0(def.getParent().(Assignment).getRightOperand())
+    exists(ConstantAssignment a |
+      container = constantDefinition(a) and
+      result = resolveScopeExpr(a.getParent().(Assignment).getRightOperand())
     )
   )
 }
 
-ModuleBase enclosing(ModuleBase m, int level) {
+cached
+private int maxDepth() { result = max(ConstantAccess c | | count(c.getEnclosingModule+())) }
+
+private ModuleBase enclosing(ModuleBase m, int level) {
   result = m and level = 0
   or
   result = enclosing(m.getEnclosingModule(), level - 1)
 }
 
+private string resolveScopeExpr(ConstantReadAccess c, int priority) {
+  c.hasGlobalScope() and result = c.getName() and priority = 0
+  or
+  result = qualifiedModuleName(resolveScopeExpr(c.getScopeExpr(), priority), c.getName())
+  or
+  not exists(c.getScopeExpr()) and
+  not c.hasGlobalScope() and
+  exists(Namespace n |
+    result = qualifiedModuleName(constantDefinition0(n), c.getName()) and
+    n = enclosing(c.getEnclosingModule(), priority)
+  )
+  or
+  result =
+    qualifiedModuleName(ancestors(qualifiedModuleName(c.getEnclosingModule()), priority - maxDepth()),
+      c.getName())
+  or
+  result = c.getName() and
+  priority = maxDepth() + 4 and
+  qualifiedModuleName(c.getEnclosingModule()) != "BasicObject"
+}
+
 bindingset[qualifier, name]
 private string scopeAppend(string qualifier, string name) {
   if qualifier = "Object" then result = name else result = qualifier + "::" + name
 }
 
-private string resolveRelativeToEnclosing(ConstantAccess n, int i) {
-  not isToplevel(n) and
-  not exists(n.getScopeExpr()) and
-  exists(ModuleBase enclosing |
-    enclosing = enclosing(n.getEnclosingModule(), i) and
-    result = scopeAppend(constantDefinition0(enclosing), n.getName()) and
-    (result = builtin() or result = constantDefinition0(_) or n instanceof ConstantWriteAccess)
+private string qualifiedModuleName(ModuleBase m) {
+  result = "Object" and m instanceof Toplevel
+  or
+  result = constantDefinition0(m)
+}
+
+private string constantDefinition0(ConstantWriteAccess c) {
+  c.hasGlobalScope() and result = c.getName()
+  or
+  result = scopeAppend(resolveScopeExpr(c.getScopeExpr(), _), c.getName())
+  or
+  not exists(c.getScopeExpr()) and
+  not c.hasGlobalScope() and
+  exists(ModuleBase enclosing | enclosing = c.getEnclosingModule() |
+    result = scopeAppend(qualifiedModuleName(enclosing), c.getName())
   )
 }
 
+private string ancestors(string qname, int priority) {
+  result = ancestors(prepends(qname), _) and priority = 0
+  or
+  result = qname and priority = 1 and isDefinedConstant(qname)
+  or
+  result = ancestors(includes(qname), _) and priority = 2
+  or
+  result = ancestors(superclass(qname), _) and priority = 3
+}
+
 private class IncludeOrPrependCall extends MethodCall {
   IncludeOrPrependCall() { this.getMethodName() = ["include", "prepend"] }
 
-  string getAModule() { result = resolveScopeExpr0(this.getAnArgument()) }
+  string getAModule() { result = resolveScopeExpr(this.getAnArgument(), _) }
 
   string getTarget() {
-    result = resolveScopeExpr0(this.getReceiver())
+    result = resolveScopeExpr(this.getReceiver(), _)
     or
-    exists(ModuleBase enclosing |
-      enclosing = this.getEnclosingModule() and
-      (
-        result = constantDefinition0(enclosing)
-        or
-        result = "Object" and enclosing instanceof Toplevel
-      )
-    |
+    result = qualifiedModuleName(this.getEnclosingModule()) and
+    (
       this.getReceiver() instanceof Self
       or
       not exists(this.getReceiver())
@@ -115,34 +170,11 @@ private Expr superexpr(string qname) {
 private string superclass(string qname) {
   qname = "Object" and result = "BasicObject"
   or
-  result = resolveScopeExpr(superexpr(qname))
-  or
-  qname = constantDefinition0(_) and
-  not exists(superexpr(qname)) and
-  result = "Object" and
-  qname != "BasicObject"
+  result = resolveScopeExpr(superexpr(qname), _)
 }
 
-private string ancestors(string qname) {
-  qname = [builtin(), constantDefinition0(any(Namespace x))] and
-  (
-    result = prepends(qname)
-    or
-    result = qname
-    or
-    result = includes(qname)
-  )
-}
-
-private string contains(string qname, string name) {
-  result = containsIgnoringSuper(qname, name)
-  or
-  not exists(containsIgnoringSuper(qname, name)) and
-  result = contains(superclass(qname), name)
-}
-
-private string qualifiedName(string container, string name) {
-  result = [builtin(), constantDefinition0(_)] and
+private string qualifiedModuleName(string container, string name) {
+  isDefinedConstant(result) and
   (
     container = result.regexpCapture("(.+)::([^:]+)", 1) and
     name = result.regexpCapture("(.+)::([^:]+)", 2)
@@ -150,64 +182,3 @@ private string qualifiedName(string container, string name) {
     container = "Object" and name = result
   )
 }
-
-private string containsIgnoringSuper(string qname, string name) {
-  result =
-    min(string n, string container, int i |
-      n = qualifiedName(container, name) and
-      (
-        container = ancestors*(prepends(qname)) and i = 0
-        or
-        container = qname and i = 1
-        or
-        container = ancestors*(includes(qname)) and i = 2
-      )
-    |
-      n order by i
-    )
-}
-
-private string resolveRelativeToAncestors(ConstantReadAccess n) {
-  not isToplevel(n) and
-  not exists(n.getScopeExpr()) and
-  exists(ModuleBase enclosing | enclosing = n.getEnclosingModule() |
-    result = contains(constantDefinition(enclosing), n.getName())
-    or
-    enclosing instanceof Toplevel and result = contains("Object", n.getName())
-  )
-}
-
-private string qualifiedNameForConstant0(ConstantAccess n) {
-  isToplevel(n) and
-  result = n.getName()
-  or
-  result = resolveRelativeToEnclosing(n, 0)
-  or
-  result = scopeAppend(resolveScopeExpr0(n.getScopeExpr()), n.getName())
-}
-
-string constantDefinition(ConstantWriteAccess n) {
-  result = constantDefinition0(n)
-  or
-  result = scopeAppend(resolveScopeExpr(n.getScopeExpr()), n.getName())
-}
-
-private string resolveScopeExpr(ConstantReadAccess n) {
-  result = resolveScopeExpr0(n)
-  or
-  exists(string qname |
-    qname = min(int i, string x | x = resolveRelativeToEnclosing(n, i) | x order by i)
-    or
-    not exists(resolveRelativeToEnclosing(n, _)) and
-    qname = resolveRelativeToAncestors(n)
-  |
-    qname = builtin() and result = qname
-    or
-    not qname = builtin() and
-    exists(ConstantWriteAccess def | qname = constantDefinition(def) |
-      result = qname and def instanceof Namespace
-      or
-      result = resolveScopeExpr(def.getParent().(Assignment).getRightOperand())
-    )
-  )
-}
diff --git a/ql/test/library-tests/ast/modules/modules.expected b/ql/test/library-tests/ast/modules/modules.expected
index 15d0da66801..fb8d891c4f2 100644
--- a/ql/test/library-tests/ast/modules/modules.expected
+++ b/ql/test/library-tests/ast/modules/modules.expected
@@ -120,7 +120,7 @@ moduleTypes
 | modules.rb:83:1:87:3 | IncludeTest | modules.rb:83:1:87:3 | IncludeTest |
 | modules.rb:85:3:86:5 | Y | modules.rb:85:3:86:5 | Test::Foo1::Y |
 | modules.rb:89:1:93:3 | IncludeTest2 | modules.rb:89:1:93:3 | IncludeTest2 |
-| modules.rb:91:3:92:5 | Z | modules.rb:91:3:92:5 | ...::Z |
+| modules.rb:91:3:92:5 | Z | modules.rb:91:3:92:5 | Test::Foo1::Z |
 | modules.rb:95:1:99:3 | PrependTest | modules.rb:95:1:99:3 | PrependTest |
 | modules.rb:97:3:98:5 | Y | modules.rb:97:3:98:5 | Test::Foo2::Y |
 | toplevel.rb:1:1:5:23 | toplevel.rb | file://:0:0:0:0 | Object |

From 7bc5be93ff0ddabc8f711dcee31740c37de87fbd Mon Sep 17 00:00:00 2001
From: Arthur Baars <aibaars@github.com>
Date: Fri, 9 Apr 2021 13:29:27 +0200
Subject: [PATCH 12/14] Module: make main predicates cached

---
 ql/src/codeql_ruby/ast/internal/Module.qll | 39 ++++++++++++----------
 1 file changed, 22 insertions(+), 17 deletions(-)

diff --git a/ql/src/codeql_ruby/ast/internal/Module.qll b/ql/src/codeql_ruby/ast/internal/Module.qll
index e204e6aa24b..f70a897e12a 100644
--- a/ql/src/codeql_ruby/ast/internal/Module.qll
+++ b/ql/src/codeql_ruby/ast/internal/Module.qll
@@ -10,13 +10,29 @@ private import codeql_ruby.ast.Scope
 private string builtin() { result = ["Object", "Kernel", "BasicObject", "Class", "Module"] }
 
 cached
-newtype TModule =
-  TResolved(string qName) {
-    qName = builtin()
+module Cached {
+  cached
+  newtype TModule =
+    TResolved(string qName) {
+      qName = builtin()
+      or
+      qName = constantDefinition(_)
+    } or
+    TUnresolved(Namespace n) { not exists(constantDefinition(n)) }
+
+  cached
+  string constantDefinition(ConstantWriteAccess n) {
+    isToplevel(n) and result = n.getName()
     or
-    qName = constantDefinition(_)
-  } or
-  TUnresolved(Namespace n) { not exists(constantDefinition(n)) }
+    not isToplevel(n) and
+    not exists(n.getScopeExpr()) and
+    result = scopeAppend(constantDefinition(n.getEnclosingModule()), n.getName())
+    or
+    result = scopeAppend(resolveScopeExpr(n.getScopeExpr()), n.getName())
+  }
+}
+
+import Cached
 
 private predicate isToplevel(ConstantAccess n) {
   not exists(n.getScopeExpr()) and
@@ -27,16 +43,6 @@ private predicate isToplevel(ConstantAccess n) {
   )
 }
 
-string constantDefinition(ConstantWriteAccess n) {
-  isToplevel(n) and result = n.getName()
-  or
-  not isToplevel(n) and
-  not exists(n.getScopeExpr()) and
-  result = scopeAppend(constantDefinition(n.getEnclosingModule()), n.getName())
-  or
-  result = scopeAppend(resolveScopeExpr(n.getScopeExpr()), n.getName())
-}
-
 private predicate isDefinedConstant(string qualifiedModuleName) {
   qualifiedModuleName = [builtin(), constantDefinition0(_)]
 }
@@ -64,7 +70,6 @@ private string resolveScopeExpr(ConstantReadAccess r) {
   )
 }
 
-cached
 private int maxDepth() { result = max(ConstantAccess c | | count(c.getEnclosingModule+())) }
 
 private ModuleBase enclosing(ModuleBase m, int level) {

From a247544fc5ff3969f721177a16f87219496df698 Mon Sep 17 00:00:00 2001
From: Arthur Baars <aibaars@github.com>
Date: Fri, 9 Apr 2021 13:44:15 +0200
Subject: [PATCH 13/14] Add comments

---
 ql/src/codeql_ruby/ast/Module.qll          |  4 +-
 ql/src/codeql_ruby/ast/internal/Module.qll | 63 ++++++++++++++++------
 2 files changed, 48 insertions(+), 19 deletions(-)

diff --git a/ql/src/codeql_ruby/ast/Module.qll b/ql/src/codeql_ruby/ast/Module.qll
index 6616e29a899..d449a576e4a 100644
--- a/ql/src/codeql_ruby/ast/Module.qll
+++ b/ql/src/codeql_ruby/ast/Module.qll
@@ -25,7 +25,7 @@ class Module extends TModule {
     result =
       min(Namespace n, string qName, Location loc, int weight |
         this = TResolved(qName) and
-        qName = constantDefinition(n) and
+        qName = namespaceDeclaration(n) and
         loc = n.getLocation() and
         if exists(loc.getFile().getRelativePath()) then weight = 0 else weight = 1
       |
@@ -174,7 +174,7 @@ class Namespace extends ModuleBase, ConstantWriteAccess, TNamespace {
   override predicate hasGlobalScope() { none() }
 
   final override Module getModule() {
-    result = any(string qName | qName = constantDefinition(this) | TResolved(qName))
+    result = any(string qName | qName = namespaceDeclaration(this) | TResolved(qName))
     or
     result = TUnresolved(this)
   }
diff --git a/ql/src/codeql_ruby/ast/internal/Module.qll b/ql/src/codeql_ruby/ast/internal/Module.qll
index f70a897e12a..fad46e1dfbf 100644
--- a/ql/src/codeql_ruby/ast/internal/Module.qll
+++ b/ql/src/codeql_ruby/ast/internal/Module.qll
@@ -16,19 +16,22 @@ module Cached {
     TResolved(string qName) {
       qName = builtin()
       or
-      qName = constantDefinition(_)
+      qName = namespaceDeclaration(_)
     } or
-    TUnresolved(Namespace n) { not exists(constantDefinition(n)) }
+    TUnresolved(Namespace n) { not exists(namespaceDeclaration(n)) }
 
   cached
-  string constantDefinition(ConstantWriteAccess n) {
+  string namespaceDeclaration(Namespace n) {
     isToplevel(n) and result = n.getName()
     or
     not isToplevel(n) and
     not exists(n.getScopeExpr()) and
-    result = scopeAppend(constantDefinition(n.getEnclosingModule()), n.getName())
+    result = scopeAppend(namespaceDeclaration(n.getEnclosingModule()), n.getName())
     or
-    result = scopeAppend(resolveScopeExpr(n.getScopeExpr()), n.getName())
+    exists(string container |
+      TResolved(container) = resolveScopeExpr(n.getScopeExpr()) and
+      result = scopeAppend(container, n.getName())
+    )
   }
 }
 
@@ -48,29 +51,32 @@ private predicate isDefinedConstant(string qualifiedModuleName) {
 }
 
 /**
- * Resolve a scope expression
+ * Resolve constant read access (typically a scope expression) to a qualified module name.
+ * `resolveScopeExpr/1` picks the best (lowest priority number) result of
+ * `resolveScopeExpr/2` that resolves to a constant definition. If the constant
+ * definition is a Namespace then it is returned, if it's a constant assignment then
+ * the right-hand side of the assignment is resolved.
  */
-private string resolveScopeExpr(ConstantReadAccess r) {
-  exists(string container |
-    container =
-      min(string c, int p |
-        isDefinedConstant(c) and
-        c = resolveScopeExpr(r, p)
+private TResolved resolveScopeExpr(ConstantReadAccess r) {
+  exists(string qname |
+    qname =
+      min(string qn, int p |
+        isDefinedConstant(qn) and
+        qn = resolveScopeExpr(r, p)
       |
-        c order by p
+        qn order by p
       )
   |
-    result = container and
-    container = [builtin(), constantDefinition(any(Namespace x))]
+    result = TResolved(qname)
     or
     exists(ConstantAssignment a |
-      container = constantDefinition(a) and
+      qname = constantDefinition0(a) and
       result = resolveScopeExpr(a.getParent().(Assignment).getRightOperand())
     )
   )
 }
 
-private int maxDepth() { result = max(ConstantAccess c | | count(c.getEnclosingModule+())) }
+private int maxDepth() { result = 1 + max(int level | exists(enclosing(_, level))) }
 
 private ModuleBase enclosing(ModuleBase m, int level) {
   result = m and level = 0
@@ -78,6 +84,14 @@ private ModuleBase enclosing(ModuleBase m, int level) {
   result = enclosing(m.getEnclosingModule(), level - 1)
 }
 
+/**
+ * Resolve constant read access (typically a scope expression) to a qualified name. The
+ * `priority` value indicates the precedence of the solution with respect to the lookup order.
+ * A constant name without scope specifier is resolved against its enclosing modules (inner-most first);
+ * if the constant is not found in any of the enclosing modules, then the constant will be resolved
+ * with respect to the ancestors (prepends, includes, super classes, and their ancestors) of the
+ * directly enclosing module.
+ */
 private string resolveScopeExpr(ConstantReadAccess c, int priority) {
   c.hasGlobalScope() and result = c.getName() and priority = 0
   or
@@ -110,6 +124,12 @@ private string qualifiedModuleName(ModuleBase m) {
   result = constantDefinition0(m)
 }
 
+/**
+ * Get a qualified name for a constant definition. May return multiple qualified
+ * names because we over-approximate when resolving scope resolutions and ignore
+ * lookup order precedence. Taking lookup order into account here would lead to
+ * non-monotonic recursion.
+ */
 private string constantDefinition0(ConstantWriteAccess c) {
   c.hasGlobalScope() and result = c.getName()
   or
@@ -122,6 +142,15 @@ private string constantDefinition0(ConstantWriteAccess c) {
   )
 }
 
+/**
+ * The qualified names of the ancestors of a class/module. The ancestors should be an ordered list
+ * of the ancestores of `prepend`ed modules, the module itself , the ancestors or `include`d modules
+ * and the ancestors of the super class. The priority value only distinguishes the kind of ancestor,
+ * it does not order the ancestors within a group of the same kind. This is an over-approximation, however,
+ * computing the precise order is tricky because it depends on the evaluation/file loading order.
+ */
+// TODO: the order of super classes can be determined more precisely even without knowing the evaluation
+// order, so we should be able to make this more precise.
 private string ancestors(string qname, int priority) {
   result = ancestors(prepends(qname), _) and priority = 0
   or

From cdfabbc95dd99b16b6c0f57d2983abc8e1e47929 Mon Sep 17 00:00:00 2001
From: Arthur Baars <aibaars@github.com>
Date: Fri, 9 Apr 2021 16:47:02 +0200
Subject: [PATCH 14/14] Make Cached module private

---
 ql/src/codeql_ruby/ast/internal/Module.qll | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ql/src/codeql_ruby/ast/internal/Module.qll b/ql/src/codeql_ruby/ast/internal/Module.qll
index fad46e1dfbf..8a0f98182af 100644
--- a/ql/src/codeql_ruby/ast/internal/Module.qll
+++ b/ql/src/codeql_ruby/ast/internal/Module.qll
@@ -10,7 +10,7 @@ private import codeql_ruby.ast.Scope
 private string builtin() { result = ["Object", "Kernel", "BasicObject", "Class", "Module"] }
 
 cached
-module Cached {
+private module Cached {
   cached
   newtype TModule =
     TResolved(string qName) {