Ruby: Fix ModuleBase::get(A)Method for private methods

Author: hvitved

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

@@ -1,4 +1,5 @@
 private import codeql.ruby.AST
+private import codeql.ruby.CFG
 private import internal.AST
 private import internal.Module
 private import internal.TreeSitter
@@ -49,24 +50,54 @@ class Module extends TModule {
   }
 }
 
+/**
+ * Gets the enclosing module of `s`, but only if `s` and the module are in the
+ * same CFG scope. For example, in
+ *
+ * ```rb
+ * module M
+ *   def pub; end
+ *   private def priv; end
+ * end
+ * ```
+ *
+ * `M` is the enclosing module of `pub` and `priv`, in the same CFG scope, while
+ * in
+ *
+ * ```rb
+ * module M
+ *   def m
+ *     def nested; end
+ *   end
+ * end
+ * ```
+ *
+ * `M` is the enclosing module of `m`, in the same CFG scope, while `nested` is not.
+ */
+pragma[nomagic]
+private ModuleBase getEnclosingModuleInSameCfgScope(Stmt s) {
+  result = s.getEnclosingModule() and
+  s.getCfgScope() = [result.(CfgScope), result.getCfgScope()]
+}
+
 /**
  * The base class for classes, singleton classes, and modules.
  */
 class ModuleBase extends BodyStmt, Scope, TModuleBase {
   /** Gets a method defined in this module/class. */
-  MethodBase getAMethod() { result = this.getAStmt() }
+  MethodBase getAMethod() { this = getEnclosingModuleInSameCfgScope(result) }
 
   /** Gets the method named `name` in this module/class, if any. */
   MethodBase getMethod(string name) { result = this.getAMethod() and result.getName() = name }
 
   /** Gets a class defined in this module/class. */
-  ClassDeclaration getAClass() { result = this.getAStmt() }
+  ClassDeclaration getAClass() { this = getEnclosingModuleInSameCfgScope(result) }
 
   /** Gets the class named `name` in this module/class, if any. */
   ClassDeclaration getClass(string name) { result = this.getAClass() and result.getName() = name }
 
   /** Gets a module defined in this module/class. */
-  ModuleDeclaration getAModule() { result = this.getAStmt() }
+  ModuleDeclaration getAModule() { this = getEnclosingModuleInSameCfgScope(result) }
 
   /** Gets the module named `name` in this module/class, if any. */
   ModuleDeclaration getModule(string name) {

ruby/ql/test/library-tests/modules/callgraph.expected

@@ -190,8 +190,10 @@ getTarget
 | private.rb:64:7:64:32 | call to puts | calls.rb:102:5:102:30 | puts |
 | private.rb:67:3:69:5 | call to private | calls.rb:109:5:109:20 | private |
 | private.rb:68:7:68:32 | call to puts | calls.rb:102:5:102:30 | puts |
+| private.rb:72:7:72:8 | call to m1 | private.rb:63:11:65:5 | m1 |
 | private.rb:77:3:81:5 | call to private | calls.rb:109:5:109:20 | private |
 | private.rb:78:7:78:32 | call to puts | calls.rb:102:5:102:30 | puts |
+| private.rb:79:7:79:8 | call to m2 | private.rb:67:11:69:5 | m2 |
 | private.rb:80:7:80:26 | call to new | calls.rb:114:5:114:16 | new |
 | private.rb:84:1:84:20 | call to new | calls.rb:114:5:114:16 | new |
 | private.rb:84:1:84:28 | call to call_m1 | private.rb:71:3:73:5 | call_m1 |
@@ -240,8 +242,6 @@ unresolvedCall
 | private.rb:35:1:35:14 | call to private2 |
 | private.rb:36:1:36:14 | call to private3 |
 | private.rb:37:1:37:14 | call to private4 |
-| private.rb:72:7:72:8 | call to m1 |
-| private.rb:79:7:79:8 | call to m2 |
 | private.rb:80:7:80:29 | call to m1 |
 | private.rb:85:1:85:23 | call to m1 |
 privateMethod

ruby/ql/test/library-tests/modules/methods.expected

@@ -46,15 +46,20 @@ getMethod
 | modules.rb:5:3:14:5 | Foo::Bar | method_in_foo_bar | modules.rb:9:5:10:7 | method_in_foo_bar |
 | modules.rb:37:1:46:3 | Bar | method_a | modules.rb:38:3:39:5 | method_a |
 | modules.rb:37:1:46:3 | Bar | method_b | modules.rb:41:3:42:5 | method_b |
+| private.rb:1:1:29:3 | E | private1 | private.rb:2:11:3:5 | private1 |
 | private.rb:1:1:29:3 | E | private2 | private.rb:8:3:9:5 | private2 |
 | private.rb:1:1:29:3 | E | private3 | private.rb:14:3:15:5 | private3 |
 | private.rb:1:1:29:3 | E | private4 | private.rb:17:3:18:5 | private4 |
 | private.rb:1:1:29:3 | E | public | private.rb:5:3:6:5 | public |
+| private.rb:42:1:60:3 | F | private1 | private.rb:43:11:44:5 | private1 |
 | private.rb:42:1:60:3 | F | private2 | private.rb:49:3:50:5 | private2 |
 | private.rb:42:1:60:3 | F | private3 | private.rb:55:3:56:5 | private3 |
 | private.rb:42:1:60:3 | F | private4 | private.rb:58:3:59:5 | private4 |
 | private.rb:42:1:60:3 | F | public | private.rb:46:3:47:5 | public |
 | private.rb:62:1:74:3 | PrivateOverride1 | call_m1 | private.rb:71:3:73:5 | call_m1 |
+| private.rb:62:1:74:3 | PrivateOverride1 | m1 | private.rb:63:11:65:5 | m1 |
+| private.rb:62:1:74:3 | PrivateOverride1 | m2 | private.rb:67:11:69:5 | m2 |
+| private.rb:76:1:82:3 | PrivateOverride2 | m1 | private.rb:77:11:81:5 | m1 |
 lookupMethod
 | calls.rb:21:1:34:3 | M | instance_m | calls.rb:22:5:24:7 | instance_m |
 | calls.rb:43:1:58:3 | C | add_singleton | calls.rb:364:1:368:3 | add_singleton |
@@ -406,23 +411,29 @@ lookupMethod
 | modules_rec.rb:4:1:5:3 | A::B | puts | calls.rb:102:5:102:30 | puts |
 | modules_rec.rb:4:1:5:3 | A::B | to_s | calls.rb:169:5:170:7 | to_s |
 | private.rb:1:1:29:3 | E | new | calls.rb:114:5:114:16 | new |
+| private.rb:1:1:29:3 | E | private1 | private.rb:2:11:3:5 | private1 |
 | private.rb:1:1:29:3 | E | private2 | private.rb:8:3:9:5 | private2 |
 | private.rb:1:1:29:3 | E | private3 | private.rb:14:3:15:5 | private3 |
 | private.rb:1:1:29:3 | E | private4 | private.rb:17:3:18:5 | private4 |
 | private.rb:1:1:29:3 | E | private_on_main | private.rb:31:1:32:3 | private_on_main |
 | private.rb:1:1:29:3 | E | public | private.rb:5:3:6:5 | public |
 | private.rb:1:1:29:3 | E | puts | calls.rb:102:5:102:30 | puts |
 | private.rb:1:1:29:3 | E | to_s | calls.rb:169:5:170:7 | to_s |
+| private.rb:42:1:60:3 | F | private1 | private.rb:43:11:44:5 | private1 |
 | private.rb:42:1:60:3 | F | private2 | private.rb:49:3:50:5 | private2 |
 | private.rb:42:1:60:3 | F | private3 | private.rb:55:3:56:5 | private3 |
 | private.rb:42:1:60:3 | F | private4 | private.rb:58:3:59:5 | private4 |
 | private.rb:42:1:60:3 | F | public | private.rb:46:3:47:5 | public |
 | private.rb:62:1:74:3 | PrivateOverride1 | call_m1 | private.rb:71:3:73:5 | call_m1 |
+| private.rb:62:1:74:3 | PrivateOverride1 | m1 | private.rb:63:11:65:5 | m1 |
+| private.rb:62:1:74:3 | PrivateOverride1 | m2 | private.rb:67:11:69:5 | m2 |
 | private.rb:62:1:74:3 | PrivateOverride1 | new | calls.rb:114:5:114:16 | new |
 | private.rb:62:1:74:3 | PrivateOverride1 | private_on_main | private.rb:31:1:32:3 | private_on_main |
 | private.rb:62:1:74:3 | PrivateOverride1 | puts | calls.rb:102:5:102:30 | puts |
 | private.rb:62:1:74:3 | PrivateOverride1 | to_s | calls.rb:169:5:170:7 | to_s |
 | private.rb:76:1:82:3 | PrivateOverride2 | call_m1 | private.rb:71:3:73:5 | call_m1 |
+| private.rb:76:1:82:3 | PrivateOverride2 | m1 | private.rb:77:11:81:5 | m1 |
+| private.rb:76:1:82:3 | PrivateOverride2 | m2 | private.rb:67:11:69:5 | m2 |
 | private.rb:76:1:82:3 | PrivateOverride2 | new | calls.rb:114:5:114:16 | new |
 | private.rb:76:1:82:3 | PrivateOverride2 | private_on_main | private.rb:31:1:32:3 | private_on_main |
 | private.rb:76:1:82:3 | PrivateOverride2 | puts | calls.rb:102:5:102:30 | puts |