NPE reproduction

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

@@ -75,11 +75,11 @@ private module Cached {
   cached
   Module getAnIncludedModule(Module m) {
     m = TResolved("Object") and result = TResolved("Kernel")
-    or
-    exists(IncludeOrPrependCall c |
-      c.getMethodName() = "include" and
-      result = getACludedModule(c, m)
-    )
+    //or
+    //exists(IncludeOrPrependCall c |
+    //  c.getMethodName() = "include" and
+    //  result = getACludedModule(c, m)
+    //)
   }
 
   cached

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

@@ -175,9 +175,27 @@ private predicate instanceMethodCall(CfgNodes::ExprNodes::CallCfgNode call, Modu
   exists(DataFlow::LocalSourceNode sourceNode |
     methodCall(call, sourceNode, method) and
     sourceNode = trackInstance(tp)
+    //(
+    //  not sourceNode instanceof SsaSelfDefinitionNode and sourceNode = trackInstance(tp)
+    //  or
+    //  sourceNode instanceof SsaSelfDefinitionNode and
+    //  exists(ClassDeclaration c | selfHasType(sourceNode, c) | tp = c or tp = c)
+    //)
   )
 }
 
+private predicate selfHasType(SsaSelfDefinitionNode self, Module tp) {
+  // `self` in method
+  tp = self.getSelfScope().(Method).getEnclosingModule().getModule()
+  or
+  // `self` in singleton method
+  flowsToSingletonMethodObject(trackInstance(tp), self.getSelfScope())
+  or
+  // `self` in top-level
+  self.getSelfScope() instanceof Toplevel and
+  tp = TResolved("Object")
+}
+
 cached
 private module Cached {
   cached
@@ -449,13 +467,29 @@ private DataFlow::LocalSourceNode trackModule(Module tp) {
  * qualifier accesses a parameter of the enclosing callable `c` (including
  * the implicit `self` parameter).
  */
-predicate mayBenefitFromCallContext(DataFlowCall call, DataFlowCallable c) { none() }
+predicate mayBenefitFromCallContext(NormalCall call, DataFlowCallable c) {
+  c = call.getEnclosingCallable() and
+  call.asCall().getReceiver().getExpr() instanceof SelfVariableAccess
+}
+
+private predicate impossibleTarget(DataFlowCallable t, DataFlowCall call, DataFlowCall ctx) {
+  
+}
 
 /**
  * Gets a viable dispatch target of `call` in the context `ctx`. This is
  * restricted to those `call`s for which a context might make a difference.
  */
-DataFlowCallable viableImplInCallContext(DataFlowCall call, DataFlowCall ctx) { none() }
+DataFlowCallable viableImplInCallContext(DataFlowCall call, DataFlowCall ctx) {
+  mayBenefitFromCallContext(call, _) and
+  exists(DataFlowCallable enclosing |
+    result = viableCallable(call) and
+    enclosing = call.getEnclosingCallable() and
+    enclosing = viableCallable(ctx)
+  |
+    not impossibleTarget(result, call, ctx) //... <`result` is impossible  target for `call` given `ctx`> ...
+  )
+}
 
 predicate exprNodeReturnedFrom = exprNodeReturnedFromCached/2;
 

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

@@ -81,6 +81,17 @@ hello.rb:
 #   18| HelloWorld
 #-----| super -> Greeting
 
+mixins.rb:
+#    4| BarMixin
+
+#   10| MixinTestA
+#-----| super -> Object
+#-----| include -> BarMixin
+
+#   27| MixinTestB
+#-----| super -> Object
+#-----| include -> BarMixin
+
 modules.rb:
 #    1| Empty
 

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

@@ -65,6 +65,18 @@ getTarget
 | hello.rb:14:16:14:20 | call to hello | hello.rb:2:5:4:7 | hello |
 | hello.rb:20:16:20:20 | call to super | hello.rb:13:5:15:7 | message |
 | hello.rb:20:30:20:34 | call to world | hello.rb:5:5:7:7 | world |
+| mixins.rb:6:5:6:7 | call to baz | mixins.rb:17:3:20:5 | baz |
+| mixins.rb:6:5:6:7 | call to baz | mixins.rb:34:3:37:5 | baz |
+| mixins.rb:11:3:11:18 | call to include | calls.rb:92:5:92:20 | include |
+| mixins.rb:14:5:14:7 | call to bar | mixins.rb:5:3:7:5 | bar |
+| mixins.rb:19:5:19:7 | call to qux | mixins.rb:22:3:24:5 | qux |
+| mixins.rb:19:5:19:7 | call to qux | mixins.rb:39:3:41:5 | qux |
+| mixins.rb:23:5:23:26 | call to puts | calls.rb:87:5:87:17 | puts |
+| mixins.rb:28:3:28:18 | call to include | calls.rb:92:5:92:20 | include |
+| mixins.rb:31:5:31:7 | call to bar | mixins.rb:5:3:7:5 | bar |
+| mixins.rb:36:5:36:7 | call to qux | mixins.rb:22:3:24:5 | qux |
+| mixins.rb:36:5:36:7 | call to qux | mixins.rb:39:3:41:5 | qux |
+| mixins.rb:40:5:40:26 | call to puts | calls.rb:87:5:87:17 | puts |
 | modules.rb:12:5:12:26 | call to puts | calls.rb:87:5:87:17 | puts |
 | modules.rb:22:3:22:19 | call to puts | calls.rb:87:5:87:17 | puts |
 | modules.rb:33:3:33:25 | call to puts | calls.rb:87:5:87:17 | puts |

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

@@ -30,6 +30,13 @@ getMethod
 | hello.rb:1:1:8:3 | EnglishWords | world | hello.rb:5:5:7:7 | world |
 | hello.rb:11:1:16:3 | Greeting | message | hello.rb:13:5:15:7 | message |
 | hello.rb:18:1:22:3 | HelloWorld | message | hello.rb:19:5:21:7 | message |
+| mixins.rb:4:1:8:3 | BarMixin | bar | mixins.rb:5:3:7:5 | bar |
+| mixins.rb:10:1:25:3 | MixinTestA | baz | mixins.rb:17:3:20:5 | baz |
+| mixins.rb:10:1:25:3 | MixinTestA | foo | mixins.rb:13:3:15:5 | foo |
+| mixins.rb:10:1:25:3 | MixinTestA | qux | mixins.rb:22:3:24:5 | qux |
+| mixins.rb:27:1:42:3 | MixinTestB | baz | mixins.rb:34:3:37:5 | baz |
+| mixins.rb:27:1:42:3 | MixinTestB | foo | mixins.rb:30:3:32:5 | foo |
+| mixins.rb:27:1:42:3 | MixinTestB | qux | mixins.rb:39:3:41:5 | qux |
 | modules.rb:4:1:24:3 | Foo | method_in_another_definition_of_foo | modules.rb:27:3:28:5 | method_in_another_definition_of_foo |
 | modules.rb:4:1:24:3 | Foo | method_in_foo | modules.rb:16:3:17:5 | method_in_foo |
 | modules.rb:5:3:14:5 | Foo::Bar | method_in_another_definition_of_foo_bar | modules.rb:52:3:53:5 | method_in_another_definition_of_foo_bar |
@@ -211,6 +218,21 @@ lookupMethod
 | hello.rb:18:1:22:3 | HelloWorld | puts | calls.rb:87:5:87:17 | puts |
 | hello.rb:18:1:22:3 | HelloWorld | to_s | calls.rb:151:5:152:7 | to_s |
 | hello.rb:18:1:22:3 | HelloWorld | world | hello.rb:5:5:7:7 | world |
+| mixins.rb:4:1:8:3 | BarMixin | bar | mixins.rb:5:3:7:5 | bar |
+| mixins.rb:10:1:25:3 | MixinTestA | bar | mixins.rb:5:3:7:5 | bar |
+| mixins.rb:10:1:25:3 | MixinTestA | baz | mixins.rb:17:3:20:5 | baz |
+| mixins.rb:10:1:25:3 | MixinTestA | foo | mixins.rb:13:3:15:5 | foo |
+| mixins.rb:10:1:25:3 | MixinTestA | new | calls.rb:99:5:99:16 | new |
+| mixins.rb:10:1:25:3 | MixinTestA | puts | calls.rb:87:5:87:17 | puts |
+| mixins.rb:10:1:25:3 | MixinTestA | qux | mixins.rb:22:3:24:5 | qux |
+| mixins.rb:10:1:25:3 | MixinTestA | to_s | calls.rb:151:5:152:7 | to_s |
+| mixins.rb:27:1:42:3 | MixinTestB | bar | mixins.rb:5:3:7:5 | bar |
+| mixins.rb:27:1:42:3 | MixinTestB | baz | mixins.rb:34:3:37:5 | baz |
+| mixins.rb:27:1:42:3 | MixinTestB | foo | mixins.rb:30:3:32:5 | foo |
+| mixins.rb:27:1:42:3 | MixinTestB | new | calls.rb:99:5:99:16 | new |
+| mixins.rb:27:1:42:3 | MixinTestB | puts | calls.rb:87:5:87:17 | puts |
+| mixins.rb:27:1:42:3 | MixinTestB | qux | mixins.rb:39:3:41:5 | qux |
+| mixins.rb:27:1:42:3 | MixinTestB | to_s | calls.rb:151:5:152:7 | to_s |
 | modules.rb:4:1:24:3 | Foo | method_in_another_definition_of_foo | modules.rb:27:3:28:5 | method_in_another_definition_of_foo |
 | modules.rb:4:1:24:3 | Foo | method_in_foo | modules.rb:16:3:17:5 | method_in_foo |
 | modules.rb:5:3:14:5 | Foo::Bar | method_in_another_definition_of_foo_bar | modules.rb:52:3:53:5 | method_in_another_definition_of_foo_bar |

ruby/ql/test/library-tests/modules/mixins.rb

@@ -0,0 +1,42 @@
+# Reproduction of spurious call targets caused by over-approximation of the type
+# of `self` flowing through mixin methods.
+
+module BarMixin
+  def bar
+    baz
+  end
+end
+
+class MixinTestA
+  include BarMixin
+
+  def foo
+    bar
+  end
+
+  def baz
+    # This only calls MixinTestA::qux, but we resolve it to both MixinTestA::qux and MixinTestB::qux.
+    qux
+  end
+
+  def qux
+    puts "MixinTestA::qux"
+  end
+end
+
+class MixinTestB
+  include BarMixin
+
+  def foo
+    bar
+  end
+
+  def baz
+    # This only calls MixinTestB::qux, but we resolve it to both MixinTestA::qux and MixinTestB::qux.
+    qux
+  end
+
+  def qux
+    puts "MixinTestB::qux"
+  end
+end

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

@@ -26,6 +26,9 @@ getModule
 | hello.rb:1:1:8:3 | EnglishWords |
 | hello.rb:11:1:16:3 | Greeting |
 | hello.rb:18:1:22:3 | HelloWorld |
+| mixins.rb:4:1:8:3 | BarMixin |
+| mixins.rb:10:1:25:3 | MixinTestA |
+| mixins.rb:27:1:42:3 | MixinTestB |
 | modules.rb:1:1:2:3 | Empty |
 | modules.rb:4:1:24:3 | Foo |
 | modules.rb:5:3:14:5 | Foo::Bar |
@@ -71,6 +74,7 @@ getADeclaration
 | calls.rb:97:1:100:3 | Object | calls.rb:1:1:167:16 | calls.rb |
 | calls.rb:97:1:100:3 | Object | calls.rb:97:1:100:3 | Object |
 | calls.rb:97:1:100:3 | Object | hello.rb:1:1:22:3 | hello.rb |
+| calls.rb:97:1:100:3 | Object | mixins.rb:1:1:42:3 | mixins.rb |
 | calls.rb:97:1:100:3 | Object | modules.rb:1:1:121:4 | modules.rb |
 | calls.rb:97:1:100:3 | Object | modules_rec.rb:1:1:11:10 | modules_rec.rb |
 | calls.rb:97:1:100:3 | Object | private.rb:1:1:60:3 | private.rb |
@@ -83,6 +87,9 @@ getADeclaration
 | hello.rb:1:1:8:3 | EnglishWords | hello.rb:1:1:8:3 | EnglishWords |
 | hello.rb:11:1:16:3 | Greeting | hello.rb:11:1:16:3 | Greeting |
 | hello.rb:18:1:22:3 | HelloWorld | hello.rb:18:1:22:3 | HelloWorld |
+| mixins.rb:4:1:8:3 | BarMixin | mixins.rb:4:1:8:3 | BarMixin |
+| mixins.rb:10:1:25:3 | MixinTestA | mixins.rb:10:1:25:3 | MixinTestA |
+| mixins.rb:27:1:42:3 | MixinTestB | mixins.rb:27:1:42:3 | MixinTestB |
 | modules.rb:1:1:2:3 | Empty | modules.rb:1:1:2:3 | Empty |
 | modules.rb:4:1:24:3 | Foo | modules.rb:4:1:24:3 | Foo |
 | modules.rb:4:1:24:3 | Foo | modules.rb:26:1:35:3 | Foo |
@@ -143,6 +150,8 @@ getSuperClass
 | file://:0:0:0:0 | TrueClass | calls.rb:97:1:100:3 | Object |
 | hello.rb:11:1:16:3 | Greeting | calls.rb:97:1:100:3 | Object |
 | hello.rb:18:1:22:3 | HelloWorld | hello.rb:11:1:16:3 | Greeting |
+| mixins.rb:10:1:25:3 | MixinTestA | calls.rb:97:1:100:3 | Object |
+| mixins.rb:27:1:42:3 | MixinTestB | calls.rb:97:1:100:3 | Object |
 | modules.rb:6:5:7:7 | Foo::Bar::ClassInFooBar | calls.rb:97:1:100:3 | Object |
 | modules.rb:19:3:20:5 | Foo::ClassInFoo | calls.rb:97:1:100:3 | Object |
 | modules.rb:30:3:31:5 | Foo::ClassInAnotherDefinitionOfFoo | calls.rb:97:1:100:3 | Object |
@@ -163,6 +172,8 @@ getAnIncludedModule
 | calls.rb:29:1:44:3 | C | calls.rb:15:1:24:3 | M |
 | calls.rb:97:1:100:3 | Object | calls.rb:86:1:88:3 | Kernel |
 | hello.rb:11:1:16:3 | Greeting | hello.rb:1:1:8:3 | EnglishWords |
+| mixins.rb:10:1:25:3 | MixinTestA | mixins.rb:4:1:8:3 | BarMixin |
+| mixins.rb:27:1:42:3 | MixinTestB | mixins.rb:4:1:8:3 | BarMixin |
 | modules.rb:88:1:93:3 | IncludeTest | modules.rb:63:1:81:3 | Test |
 | modules.rb:95:1:99:3 | IncludeTest2 | modules.rb:63:1:81:3 | Test |
 resolveConstantReadAccess
@@ -186,6 +197,8 @@ resolveConstantReadAccess
 | calls.rb:162:1:162:1 | B | B |
 | hello.rb:12:13:12:24 | EnglishWords | EnglishWords |
 | hello.rb:18:20:18:27 | Greeting | Greeting |
+| mixins.rb:11:11:11:18 | BarMixin | BarMixin |
+| mixins.rb:28:11:28:18 | BarMixin | BarMixin |
 | modules.rb:48:8:48:10 | Foo | Foo |
 | modules.rb:66:11:66:14 | Foo1 | Test::Foo1 |
 | modules.rb:72:11:72:14 | Foo2 | Test::Foo2::Foo2 |
@@ -231,6 +244,9 @@ resolveConstantWriteAccess
 | hello.rb:1:1:8:3 | EnglishWords | EnglishWords |
 | hello.rb:11:1:16:3 | Greeting | Greeting |
 | hello.rb:18:1:22:3 | HelloWorld | HelloWorld |
+| mixins.rb:4:1:8:3 | BarMixin | BarMixin |
+| mixins.rb:10:1:25:3 | MixinTestA | MixinTestA |
+| mixins.rb:27:1:42:3 | MixinTestB | MixinTestB |
 | modules.rb:1:1:2:3 | Empty | Empty |
 | modules.rb:4:1:24:3 | Foo | Foo |
 | modules.rb:5:3:14:5 | Bar | Foo::Bar |

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

@@ -76,6 +76,15 @@ hello.rb:
 #   18| HelloWorld
 #-----|  -> Greeting
 
+mixins.rb:
+#    4| BarMixin
+
+#   10| MixinTestA
+#-----|  -> Object
+
+#   27| MixinTestB
+#-----|  -> Object
+
 modules.rb:
 #    1| Empty