From 12ee9573314969ee6c7ecfd95afaf7c1c30ed517 Mon Sep 17 00:00:00 2001
From: Arthur Baars <aibaars@github.com>
Date: Wed, 14 Apr 2021 10:15:24 +0200
Subject: [PATCH 1/2] Add test cases

---
 ql/test/library-tests/ast/Ast.expected        | 11 ++++++
 .../ast/modules/classes.expected              |  3 ++
 .../ast/modules/module_base.expected          | 36 ++++++++++++-------
 .../ast/modules/modules.expected              | 19 +++++++++-
 ql/test/library-tests/ast/modules/modules.rb  | 17 +++++++++
 .../ast/modules/toplevel.expected             |  2 +-
 6 files changed, 74 insertions(+), 14 deletions(-)

diff --git a/ql/test/library-tests/ast/Ast.expected b/ql/test/library-tests/ast/Ast.expected
index aaa2a31f68a..7078ea3c675 100644
--- a/ql/test/library-tests/ast/Ast.expected
+++ b/ql/test/library-tests/ast/Ast.expected
@@ -1406,6 +1406,17 @@ modules/modules.rb:
 #  102|       getArgument: [ConstantReadAccess] Test
 #  103|     getStmt: [ModuleDeclaration] Y
 #  103|       getScopeExpr: [ConstantReadAccess] Foo2
+#  107|   getStmt: [ModuleDeclaration] MM
+#  108|     getStmt: [ModuleDeclaration] MM
+#  108|       getScopeExpr: [ConstantReadAccess] MM
+#  112|   getStmt: [ClassDeclaration] YY
+#  115|   getStmt: [ModuleDeclaration] XX
+#  116|     getStmt: [ClassDeclaration] YY
+#  116|       getSuperclassExpr: [ConstantReadAccess] YY
+#  120|   getStmt: [ModuleDeclaration] Baz
+#  120|     getScopeExpr: [ConstantReadAccess] Bar
+#  120|       getScopeExpr: [ConstantReadAccess] Foo1
+#  120|         getScopeExpr: [ConstantReadAccess] Test
 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 0691684b668..e81722ecfa2 100644
--- a/ql/test/library-tests/ast/modules/classes.expected
+++ b/ql/test/library-tests/ast/modules/classes.expected
@@ -13,6 +13,8 @@ classes
 | 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 |
+| modules.rb:112:1:113:3 | YY | ClassDeclaration | YY |
+| modules.rb:116:7:117:9 | YY | ClassDeclaration | YY |
 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 |
@@ -37,3 +39,4 @@ modulesInClasses
 classesWithASuperclass
 | classes.rb:7:1:8:3 | Bar | classes.rb:7:13:7:21 | BaseClass |
 | classes.rb:11:1:12:3 | Baz | classes.rb:11:13:11:32 | call to superclass_for |
+| modules.rb:116:7:117:9 | YY | modules.rb:116:18:116:19 | YY |
diff --git a/ql/test/library-tests/ast/modules/module_base.expected b/ql/test/library-tests/ast/modules/module_base.expected
index 50d003cd544..109ffa5cf75 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:105:4 | modules.rb | Toplevel |
+| modules.rb:1:1:122:1 | 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 |
@@ -38,6 +38,12 @@ moduleBases
 | modules.rb:97:3:98:5 | Z | ModuleDeclaration |
 | modules.rb:101:1:105:3 | PrependTest | ModuleDeclaration |
 | modules.rb:103:3:104:5 | Y | ModuleDeclaration |
+| modules.rb:107:1:110:3 | MM | ModuleDeclaration |
+| modules.rb:108:3:109:5 | MM | ModuleDeclaration |
+| modules.rb:112:1:113:3 | YY | ClassDeclaration |
+| modules.rb:115:1:118:3 | XX | ModuleDeclaration |
+| modules.rb:116:7:117:9 | YY | ClassDeclaration |
+| modules.rb:120:1:121:3 | Baz | 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 |
@@ -47,6 +53,7 @@ moduleBaseClasses
 | classes.rb:2:1:56:3 | classes.rb | classes.rb:20:1:37:3 | Wibble |
 | classes.rb:2:1:56:3 | classes.rb | classes.rb:55:1:56:3 | MyClassInGlobalScope |
 | classes.rb:20:1:37:3 | Wibble | classes.rb:32:3:33:5 | ClassInWibble |
+| modules.rb:1:1:122:1 | modules.rb | modules.rb:112:1:113:3 | YY |
 | modules.rb:4:1:24:3 | Foo | modules.rb:19:3:20:5 | ClassInFoo |
 | 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 |
@@ -54,6 +61,7 @@ moduleBaseClasses
 | 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 |
+| modules.rb:115:1:118:3 | XX | modules.rb:116:7:117:9 | YY |
 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 |
@@ -68,17 +76,20 @@ 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:105:4 | modules.rb | modules.rb:1:1:2:3 | Empty |
-| modules.rb:1:1:105:4 | modules.rb | modules.rb:4:1:24:3 | Foo |
-| modules.rb:1:1:105:4 | modules.rb | modules.rb:26:1:35:3 | Foo |
-| modules.rb:1:1:105:4 | modules.rb | modules.rb:37:1:46:3 | Bar |
-| modules.rb:1:1:105:4 | modules.rb | modules.rb:48:1:57:3 | Bar |
-| modules.rb:1:1:105:4 | modules.rb | modules.rb:60:1:61:3 | MyModuleInGlobalScope |
-| modules.rb:1:1:105:4 | modules.rb | modules.rb:63:1:81:3 | Test |
-| modules.rb:1:1:105:4 | modules.rb | modules.rb:83:1:86:3 | Other |
-| modules.rb:1:1:105:4 | modules.rb | modules.rb:88:1:93:3 | IncludeTest |
-| modules.rb:1:1:105:4 | modules.rb | modules.rb:95:1:99:3 | IncludeTest2 |
-| modules.rb:1:1:105:4 | modules.rb | modules.rb:101:1:105:3 | PrependTest |
+| modules.rb:1:1:122:1 | modules.rb | modules.rb:1:1:2:3 | Empty |
+| modules.rb:1:1:122:1 | modules.rb | modules.rb:4:1:24:3 | Foo |
+| modules.rb:1:1:122:1 | modules.rb | modules.rb:26:1:35:3 | Foo |
+| modules.rb:1:1:122:1 | modules.rb | modules.rb:37:1:46:3 | Bar |
+| modules.rb:1:1:122:1 | modules.rb | modules.rb:48:1:57:3 | Bar |
+| modules.rb:1:1:122:1 | modules.rb | modules.rb:60:1:61:3 | MyModuleInGlobalScope |
+| modules.rb:1:1:122:1 | modules.rb | modules.rb:63:1:81:3 | Test |
+| modules.rb:1:1:122:1 | modules.rb | modules.rb:83:1:86:3 | Other |
+| modules.rb:1:1:122:1 | modules.rb | modules.rb:88:1:93:3 | IncludeTest |
+| modules.rb:1:1:122:1 | modules.rb | modules.rb:95:1:99:3 | IncludeTest2 |
+| modules.rb:1:1:122:1 | modules.rb | modules.rb:101:1:105:3 | PrependTest |
+| modules.rb:1:1:122:1 | modules.rb | modules.rb:107:1:110:3 | MM |
+| modules.rb:1:1:122:1 | modules.rb | modules.rb:115:1:118:3 | XX |
+| modules.rb:1:1:122:1 | modules.rb | modules.rb:120:1:121:3 | Baz |
 | 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 |
@@ -88,3 +99,4 @@ moduleBaseModules
 | modules.rb:88:1:93:3 | IncludeTest | modules.rb:91:3:92:5 | Y |
 | modules.rb:95:1:99:3 | IncludeTest2 | modules.rb:97:3:98:5 | Z |
 | modules.rb:101:1:105:3 | PrependTest | modules.rb:103:3:104:5 | Y |
+| modules.rb:107:1:110:3 | MM | modules.rb:108:3:109:5 | MM |
diff --git a/ql/test/library-tests/ast/modules/modules.expected b/ql/test/library-tests/ast/modules/modules.expected
index b395f09e63d..b3fe57cfc02 100644
--- a/ql/test/library-tests/ast/modules/modules.expected
+++ b/ql/test/library-tests/ast/modules/modules.expected
@@ -21,11 +21,17 @@ modules
 | modules.rb:97:3:98:5 | Z | ModuleDeclaration | Z |
 | modules.rb:101:1:105:3 | PrependTest | ModuleDeclaration | PrependTest |
 | modules.rb:103:3:104:5 | Y | ModuleDeclaration | Y |
+| modules.rb:107:1:110:3 | MM | ModuleDeclaration | MM |
+| modules.rb:108:3:109:5 | MM | ModuleDeclaration | MM |
+| modules.rb:115:1:118:3 | XX | ModuleDeclaration | XX |
+| modules.rb:120:1:121:3 | Baz | ModuleDeclaration | Baz |
 modulesWithScopeExprs
 | modules.rb:48:1:57:3 | Bar | modules.rb:48:8:48:10 | Foo |
 | modules.rb:91:3:92:5 | Y | modules.rb:91:10:91:13 | Foo1 |
 | modules.rb:97:3:98:5 | Z | modules.rb:97:10:97:13 | Foo1 |
 | modules.rb:103:3:104:5 | Y | modules.rb:103:10:103:13 | Foo2 |
+| modules.rb:108:3:109:5 | MM | modules.rb:108:10:108:11 | MM |
+| modules.rb:120:1:121:3 | Baz | modules.rb:120:8:120:22 | Bar |
 modulesWithGlobalNameScopeExprs
 | modules.rb:60:1:61:3 | MyModuleInGlobalScope |
 exprsInModules
@@ -66,6 +72,8 @@ exprsInModules
 | modules.rb:95:1:99:3 | IncludeTest2 | 1 | modules.rb:97:3:98:5 | Z | ModuleDeclaration |
 | modules.rb:101:1:105:3 | PrependTest | 0 | modules.rb:102:3:102:16 | call to prepend | MethodCall |
 | modules.rb:101:1:105:3 | PrependTest | 1 | modules.rb:103:3:104:5 | Y | ModuleDeclaration |
+| modules.rb:107:1:110:3 | MM | 0 | modules.rb:108:3:109:5 | MM | ModuleDeclaration |
+| modules.rb:115:1:118:3 | XX | 0 | modules.rb:116:7:117:9 | YY | 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 |
@@ -81,6 +89,7 @@ classesInModules
 | 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 |
+| modules.rb:115:1:118:3 | XX | modules.rb:116:7:117:9 | YY | YY |
 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 |
@@ -91,6 +100,7 @@ modulesInModules
 | modules.rb:88:1:93:3 | IncludeTest | modules.rb:91:3:92:5 | Y | Y |
 | modules.rb:95:1:99:3 | IncludeTest2 | modules.rb:97:3:98:5 | Z | Z |
 | modules.rb:101:1:105:3 | PrependTest | modules.rb:103:3:104:5 | Y | Y |
+| modules.rb:107:1:110:3 | MM | modules.rb:108:3:109:5 | MM | MM |
 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 |
@@ -103,7 +113,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:105:4 | modules.rb | file://:0:0:0:0 | Object |
+| modules.rb:1:1:122: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 |
@@ -130,4 +140,11 @@ moduleTypes
 | modules.rb:97:3:98:5 | Z | modules.rb:97:3:98:5 | Test::Foo1::Z |
 | modules.rb:101:1:105:3 | PrependTest | modules.rb:101:1:105:3 | PrependTest |
 | modules.rb:103:3:104:5 | Y | modules.rb:103:3:104:5 | Test::Foo2::Y |
+| modules.rb:107:1:110:3 | MM | modules.rb:107:1:110:3 | MM |
+| modules.rb:108:3:109:5 | MM | modules.rb:108:3:109:5 | ...::MM |
+| modules.rb:112:1:113:3 | YY | modules.rb:112:1:113:3 | YY |
+| modules.rb:115:1:118:3 | XX | modules.rb:115:1:118:3 | XX |
+| modules.rb:116:7:117:9 | YY | modules.rb:116:7:117:9 | XX::YY |
+| modules.rb:120:1:121:3 | Baz | modules.rb:120:1:121:3 | Bar::Baz |
+| modules.rb:120:1:121:3 | Baz | modules.rb:120:1:121:3 | Test::Foo1::Bar::Baz |
 | 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 a7ff5935c13..8287b0a1bc4 100644
--- a/ql/test/library-tests/ast/modules/modules.rb
+++ b/ql/test/library-tests/ast/modules/modules.rb
@@ -103,3 +103,20 @@ module PrependTest
   module Foo2::Y
   end
 end
+
+module MM
+  module MM::MM
+  end
+end
+
+class YY
+end
+
+module XX
+      class YY < YY
+      end
+end
+
+module Test::Foo1::Bar::Baz
+end
+
diff --git a/ql/test/library-tests/ast/modules/toplevel.expected b/ql/test/library-tests/ast/modules/toplevel.expected
index a1e5b4c3f05..c15df468299 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:105:4 | modules.rb | Toplevel |
+| modules.rb:1:1:122: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 24bb11b20aabf0cf8565997a089516740d4e1b57 Mon Sep 17 00:00:00 2001
From: Arthur Baars <aibaars@github.com>
Date: Tue, 13 Apr 2021 16:38:07 +0200
Subject: [PATCH 2/2] Improve module/class resolution

---
 ql/src/codeql_ruby/ast/internal/Module.qll    | 32 ++++++++++++-------
 .../ast/modules/modules.expected              |  3 +-
 2 files changed, 21 insertions(+), 14 deletions(-)

diff --git a/ql/src/codeql_ruby/ast/internal/Module.qll b/ql/src/codeql_ruby/ast/internal/Module.qll
index 2d2c904b0d6..a1f128d0a08 100644
--- a/ql/src/codeql_ruby/ast/internal/Module.qll
+++ b/ql/src/codeql_ruby/ast/internal/Module.qll
@@ -63,7 +63,13 @@ private TResolved resolveScopeExpr(ConstantReadAccess r) {
     qname =
       min(string qn, int p |
         isDefinedConstant(qn) and
-        qn = resolveScopeExpr(r, p)
+        qn = resolveScopeExpr(r, p) and
+        // prevent classes/modules that contain/extend themselves
+        not exists(ConstantWriteAccess w | qn = constantDefinition0(w) |
+          r = w.getScopeExpr()
+          or
+          r = w.(ClassDeclaration).getSuperclassExpr()
+        )
       |
         qn order by p
       )
@@ -100,18 +106,20 @@ private string resolveScopeExpr(ConstantReadAccess c, int priority) {
   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)
+  (
+    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"
   )
-  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]
diff --git a/ql/test/library-tests/ast/modules/modules.expected b/ql/test/library-tests/ast/modules/modules.expected
index b3fe57cfc02..f7f52051674 100644
--- a/ql/test/library-tests/ast/modules/modules.expected
+++ b/ql/test/library-tests/ast/modules/modules.expected
@@ -141,10 +141,9 @@ moduleTypes
 | modules.rb:101:1:105:3 | PrependTest | modules.rb:101:1:105:3 | PrependTest |
 | modules.rb:103:3:104:5 | Y | modules.rb:103:3:104:5 | Test::Foo2::Y |
 | modules.rb:107:1:110:3 | MM | modules.rb:107:1:110:3 | MM |
-| modules.rb:108:3:109:5 | MM | modules.rb:108:3:109:5 | ...::MM |
+| modules.rb:108:3:109:5 | MM | modules.rb:108:3:109:5 | MM::MM |
 | modules.rb:112:1:113:3 | YY | modules.rb:112:1:113:3 | YY |
 | modules.rb:115:1:118:3 | XX | modules.rb:115:1:118:3 | XX |
 | modules.rb:116:7:117:9 | YY | modules.rb:116:7:117:9 | XX::YY |
-| modules.rb:120:1:121:3 | Baz | modules.rb:120:1:121:3 | Bar::Baz |
 | modules.rb:120:1:121:3 | Baz | modules.rb:120:1:121:3 | Test::Foo1::Bar::Baz |
 | toplevel.rb:1:1:5:23 | toplevel.rb | file://:0:0:0:0 | Object |