Merge pull request github#10928 from asgerf/rb/assumed-global-const

Ruby: assume some global constants are defined

Author: asgerf

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

@@ -25,7 +25,12 @@ class Module extends TModule {
 
   /** Holds if this module is a class. */
   pragma[noinline]
-  predicate isClass() { this.getADeclaration() instanceof ClassDeclaration }
+  predicate isClass() {
+    this.getADeclaration() instanceof ClassDeclaration
+    or
+    // If another class extends this, but we can't see the class declaration, assume it's a class
+    getSuperClass(_) = this
+  }
 
   /** Gets a textual representation of this module. */
   string toString() {

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

@@ -1,11 +1,13 @@
 private import codeql.ruby.AST
+private import Scope as Scope
 
 // Names of built-in modules and classes
 private string builtin() {
   result =
     [
       "Object", "Kernel", "BasicObject", "Class", "Module", "NilClass", "FalseClass", "TrueClass",
-      "Numeric", "Integer", "Float", "Rational", "Complex", "Array", "Hash", "Symbol", "Proc"
+      "Numeric", "Integer", "Float", "Rational", "Complex", "Array", "Hash", "String", "Symbol",
+      "Proc",
     ]
 }
 
@@ -16,6 +18,8 @@ private module Cached {
     TResolved(string qName) {
       qName = builtin()
       or
+      qName = getAnAssumedGlobalConst()
+      or
       qName = namespaceDeclaration(_)
     } or
     TUnresolved(Namespace n) { not exists(namespaceDeclaration(n)) }
@@ -38,7 +42,10 @@ private module Cached {
   Module getSuperClass(Module cls) {
     cls = TResolved("Object") and result = TResolved("BasicObject")
     or
-    cls = TResolved(["Module", "Numeric", "Array", "Hash", "FalseClass", "TrueClass", "NilClass"]) and
+    cls =
+      TResolved([
+          "Module", "Numeric", "Array", "Hash", "FalseClass", "TrueClass", "NilClass", "String"
+        ]) and
     result = TResolved("Object")
     or
     cls = TResolved(["Integer", "Float", "Rational", "Complex"]) and
@@ -58,14 +65,20 @@ private module Cached {
       forex(ClassDeclaration d | d = cls.getADeclaration() |
         not exists(resolveConstantReadAccess(d.getSuperclassExpr()))
       )
+      or
+      // If a module is used as a base class of another class, but we don't see its class declaration
+      // treat it as a class extending Object, so its subclasses transitively extend Object.
+      result = TResolved("Object") and
+      not cls.getADeclaration() instanceof ClassDeclaration and
+      cls = resolveConstantReadAccess(any(ClassDeclaration d).getSuperclassExpr())
     )
   }
 
   private Module getACludedModule(IncludeOrPrependCall c, Module m) {
     (
       m = resolveConstantReadAccess(c.getReceiver())
       or
-      m = enclosingModule(c).getModule() and
+      m = enclosingModuleNoBlock(c).getModule() and
       c.getReceiver() instanceof SelfVariableAccess
     ) and
     result = resolveConstantReadAccess(c.getAnArgument())
@@ -388,11 +401,23 @@ private module ResolveImpl {
     result = resolveConstantWriteAccessRec(c, _, _)
   }
 
+  /**
+   * Gets the name of a constant `C` that we assume to be defined in the top-level because
+   * it is referenced in a way that can only resolve to a top-level constant.
+   */
+  string getAnAssumedGlobalConst() {
+    exists(ConstantAccess access |
+      not exists(access.getScopeExpr()) and
+      result = access.getName() and
+      isToplevel(access)
+    )
+  }
+
   pragma[nomagic]
   private string isDefinedConstantNonRec(string container, string name) {
     result = resolveConstantWriteAccessNonRec(_, container, name)
     or
-    result = builtin() and
+    result = [builtin(), getAnAssumedGlobalConst()] and
     name = result and
     container = "Object"
   }
@@ -447,7 +472,7 @@ private module ResolveImpl {
       result = resolveConstantReadAccess(this.getReceiver(), _)
       or
       exists(ModuleBase encl |
-        encl = enclosingModule(this) and
+        encl = enclosingModuleNoBlock(this) and
         result = [qualifiedModuleNameNonRec(encl, _, _), qualifiedModuleNameRec(encl, _, _)]
       |
         this.getReceiver() instanceof SelfVariableAccess
@@ -495,23 +520,31 @@ private module ResolveImpl {
 private import ResolveImpl
 
 /**
- * A variant of AstNode::getEnclosingModule that excludes
+ * Gets an enclosing scope of `scope`, stopping at the first module or block.
+ *
+ * Includes `scope` itself and the final module/block.
+ */
+private Scope enclosingScopesNoBlock(Scope scope) {
+  result = scope
+  or
+  not scope instanceof ModuleBase and
+  not scope instanceof Block and
+  result = enclosingScopesNoBlock(scope.getOuterScope())
+}
+
+/**
+ * A variant of `AstNode::getEnclosingModule` that excludes
  * results that are enclosed in a block. This is a bit wrong because
  * it could lead to false negatives. However, `include` statements in
  * blocks are very rare in normal code. The majority of cases are in calls
  * to methods like `module_eval` and `Rspec.describe` / `Rspec.context`. These
  * methods evaluate the block in the context of some other module/class instead of
  * the enclosing one.
  */
-private ModuleBase enclosingModule(AstNode node) {
-  result = node.getParent()
-  or
-  exists(AstNode mid |
-    result = enclosingModule(mid) and
-    mid = node.getParent() and
-    not mid instanceof ModuleBase and
-    not mid instanceof Block
-  )
+private ModuleBase enclosingModuleNoBlock(Stmt node) {
+  // Note: don't rely on AstNode.getParent() here.
+  // Instead use Scope.getOuterScope() to correctly handle the scoping of things like Namespace.getScopeExpr().
+  result = enclosingScopesNoBlock(Scope::scopeOfInclSynth(node))
 }
 
 private Module getAncestors(Module m) {

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

@@ -7,7 +7,14 @@ private import internal.ControlFlowGraphImpl
 private import internal.Splitting
 private import internal.Completion
 
-/** An AST node with an associated control-flow graph. */
+/**
+ * An AST node with an associated control-flow graph.
+ *
+ * Top-levels, methods, blocks, and lambdas are all CFG scopes.
+ *
+ * Note that module declarations are not themselves CFG scopes, as they are part of
+ * the CFG of the enclosing top-level or callable.
+ */
 class CfgScope extends Scope instanceof CfgScopeImpl {
   /** Gets the CFG scope that this scope is nested under, if any. */
   final CfgScope getOuterCfgScope() {

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

@@ -1,6 +1,14 @@
 #-----| Class
 #-----| super -> Module
 
+#-----| EsotericInstanceMethods
+
+#-----| MyStruct
+
+#-----| Struct
+
+#-----| UnresolvedNamespace
+
 #-----| BasicObject
 
 #-----| Complex
@@ -239,3 +247,16 @@ toplevel_self_singleton.rb:
 #-----| super -> Object
 
 #   24| Good
+
+unresolved_subclass.rb:
+#    1| ResolvableBaseClass
+#-----| super -> Object
+
+#    4| UnresolvedNamespace::Subclass1
+#-----| super -> ResolvableBaseClass
+
+#    7| UnresolvedNamespace::Subclass2
+#-----| super -> UnresolvedNamespace::Subclass1
+
+#   11| UnresolvedNamespace::A
+#-----| super -> Object

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

@@ -281,6 +281,7 @@ getTarget
 | private.rb:104:1:104:20 | call to new | calls.rb:117:5:117:16 | new |
 | private.rb:104:1:104:28 | call to call_m1 | private.rb:91:3:93:5 | call_m1 |
 | private.rb:105:1:105:20 | call to new | calls.rb:117:5:117:16 | new |
+| toplevel_self_singleton.rb:18:12:22:1 | call to new | calls.rb:117:5:117:16 | new |
 | toplevel_self_singleton.rb:30:13:30:19 | call to call_me | toplevel_self_singleton.rb:26:9:27:11 | call_me |
 | toplevel_self_singleton.rb:31:13:31:20 | call to call_you | toplevel_self_singleton.rb:29:9:32:11 | call_you |
 unresolvedCall
@@ -372,7 +373,6 @@ unresolvedCall
 | toplevel_self_singleton.rb:8:1:16:3 | call to do_something |
 | toplevel_self_singleton.rb:10:9:10:27 | call to ab_singleton_method |
 | toplevel_self_singleton.rb:14:9:14:27 | call to ab_singleton_method |
-| toplevel_self_singleton.rb:18:12:22:1 | call to new |
 | toplevel_self_singleton.rb:20:9:20:27 | call to ab_singleton_method |
 privateMethod
 | calls.rb:1:1:3:3 | foo |

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

@@ -599,6 +599,18 @@ lookupMethod
 | toplevel_self_singleton.rb:2:5:5:7 | A::B | new | calls.rb:117:5:117:16 | new |
 | toplevel_self_singleton.rb:2:5:5:7 | A::B | puts | calls.rb:102:5:102:30 | puts |
 | toplevel_self_singleton.rb:2:5:5:7 | A::B | to_s | calls.rb:172:5:173:7 | to_s |
+| unresolved_subclass.rb:1:1:2:3 | ResolvableBaseClass | new | calls.rb:117:5:117:16 | new |
+| unresolved_subclass.rb:1:1:2:3 | ResolvableBaseClass | puts | calls.rb:102:5:102:30 | puts |
+| unresolved_subclass.rb:1:1:2:3 | ResolvableBaseClass | to_s | calls.rb:172:5:173:7 | to_s |
+| unresolved_subclass.rb:4:1:5:3 | UnresolvedNamespace::Subclass1 | new | calls.rb:117:5:117:16 | new |
+| unresolved_subclass.rb:4:1:5:3 | UnresolvedNamespace::Subclass1 | puts | calls.rb:102:5:102:30 | puts |
+| unresolved_subclass.rb:4:1:5:3 | UnresolvedNamespace::Subclass1 | to_s | calls.rb:172:5:173:7 | to_s |
+| unresolved_subclass.rb:7:1:8:3 | UnresolvedNamespace::Subclass2 | new | calls.rb:117:5:117:16 | new |
+| unresolved_subclass.rb:7:1:8:3 | UnresolvedNamespace::Subclass2 | puts | calls.rb:102:5:102:30 | puts |
+| unresolved_subclass.rb:7:1:8:3 | UnresolvedNamespace::Subclass2 | to_s | calls.rb:172:5:173:7 | to_s |
+| unresolved_subclass.rb:11:1:12:3 | UnresolvedNamespace::A | new | calls.rb:117:5:117:16 | new |
+| unresolved_subclass.rb:11:1:12:3 | UnresolvedNamespace::A | puts | calls.rb:102:5:102:30 | puts |
+| unresolved_subclass.rb:11:1:12:3 | UnresolvedNamespace::A | to_s | calls.rb:172:5:173:7 | to_s |
 enclosingMethod
 | calls.rb:2:5:2:14 | call to puts | calls.rb:1:1:3:3 | foo |
 | calls.rb:2:5:2:14 | self | calls.rb:1:1:3:3 | foo |

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

@@ -35,14 +35,18 @@ getModule
 | file://:0:0:0:0 | BasicObject |
 | file://:0:0:0:0 | Class |
 | file://:0:0:0:0 | Complex |
+| file://:0:0:0:0 | EsotericInstanceMethods |
 | file://:0:0:0:0 | FalseClass |
 | file://:0:0:0:0 | Float |
+| file://:0:0:0:0 | MyStruct |
 | file://:0:0:0:0 | NilClass |
 | file://:0:0:0:0 | Numeric |
 | file://:0:0:0:0 | Proc |
 | file://:0:0:0:0 | Rational |
+| file://:0:0:0:0 | Struct |
 | file://:0:0:0:0 | Symbol |
 | file://:0:0:0:0 | TrueClass |
+| file://:0:0:0:0 | UnresolvedNamespace |
 | hello.rb:1:1:8:3 | EnglishWords |
 | hello.rb:11:1:16:3 | Greeting |
 | hello.rb:18:1:22:3 | HelloWorld |
@@ -85,6 +89,10 @@ getModule
 | private.rb:96:1:102:3 | PrivateOverride2 |
 | toplevel_self_singleton.rb:2:5:5:7 | A::B |
 | toplevel_self_singleton.rb:24:1:34:3 | Good |
+| unresolved_subclass.rb:1:1:2:3 | ResolvableBaseClass |
+| unresolved_subclass.rb:4:1:5:3 | UnresolvedNamespace::Subclass1 |
+| unresolved_subclass.rb:7:1:8:3 | UnresolvedNamespace::Subclass2 |
+| unresolved_subclass.rb:11:1:12:3 | UnresolvedNamespace::A |
 getADeclaration
 | calls.rb:21:1:34:3 | M | calls.rb:21:1:34:3 | M |
 | calls.rb:43:1:58:3 | C | calls.rb:43:1:58:3 | C |
@@ -101,6 +109,7 @@ getADeclaration
 | calls.rb:115:1:118:3 | Object | modules_rec.rb:1:1:11:26 | modules_rec.rb |
 | calls.rb:115:1:118:3 | Object | private.rb:1:1:105:40 | private.rb |
 | calls.rb:115:1:118:3 | Object | toplevel_self_singleton.rb:1:1:34:4 | toplevel_self_singleton.rb |
+| calls.rb:115:1:118:3 | Object | unresolved_subclass.rb:1:1:12:4 | unresolved_subclass.rb |
 | calls.rb:120:1:123:3 | Hash | calls.rb:120:1:123:3 | Hash |
 | calls.rb:125:1:138:3 | Array | calls.rb:125:1:138:3 | Array |
 | calls.rb:165:1:169:3 | S | calls.rb:165:1:169:3 | S |
@@ -176,6 +185,10 @@ getADeclaration
 | toplevel_self_singleton.rb:2:5:5:7 | A::B | modules_rec.rb:4:1:5:3 | B |
 | toplevel_self_singleton.rb:2:5:5:7 | A::B | toplevel_self_singleton.rb:2:5:5:7 | B |
 | toplevel_self_singleton.rb:24:1:34:3 | Good | toplevel_self_singleton.rb:24:1:34:3 | Good |
+| unresolved_subclass.rb:1:1:2:3 | ResolvableBaseClass | unresolved_subclass.rb:1:1:2:3 | ResolvableBaseClass |
+| unresolved_subclass.rb:4:1:5:3 | UnresolvedNamespace::Subclass1 | unresolved_subclass.rb:4:1:5:3 | Subclass1 |
+| unresolved_subclass.rb:7:1:8:3 | UnresolvedNamespace::Subclass2 | unresolved_subclass.rb:7:1:8:3 | Subclass2 |
+| unresolved_subclass.rb:11:1:12:3 | UnresolvedNamespace::A | unresolved_subclass.rb:11:1:12:3 | A |
 getSuperClass
 | calls.rb:43:1:58:3 | C | calls.rb:115:1:118:3 | Object |
 | calls.rb:65:1:69:3 | D | calls.rb:43:1:58:3 | C |
@@ -231,6 +244,10 @@ getSuperClass
 | private.rb:82:1:94:3 | PrivateOverride1 | calls.rb:115:1:118:3 | Object |
 | private.rb:96:1:102:3 | PrivateOverride2 | private.rb:82:1:94:3 | PrivateOverride1 |
 | toplevel_self_singleton.rb:2:5:5:7 | A::B | calls.rb:115:1:118:3 | Object |
+| unresolved_subclass.rb:1:1:2:3 | ResolvableBaseClass | calls.rb:115:1:118:3 | Object |
+| unresolved_subclass.rb:4:1:5:3 | UnresolvedNamespace::Subclass1 | unresolved_subclass.rb:1:1:2:3 | ResolvableBaseClass |
+| unresolved_subclass.rb:7:1:8:3 | UnresolvedNamespace::Subclass2 | unresolved_subclass.rb:4:1:5:3 | UnresolvedNamespace::Subclass1 |
+| unresolved_subclass.rb:11:1:12:3 | UnresolvedNamespace::A | calls.rb:115:1:118:3 | Object |
 getAPrependedModule
 | calls.rb:115:1:118:3 | Object | calls.rb:171:1:174:3 | A |
 | calls.rb:171:1:174:3 | A | toplevel_self_singleton.rb:2:5:5:7 | A::B |
@@ -308,6 +325,11 @@ resolveConstantReadAccess
 | calls.rb:471:5:471:11 | Array | Array |
 | calls.rb:477:5:477:9 | Class | Class |
 | calls.rb:483:5:483:11 | Array | Array |
+| calls.rb:490:1:490:23 | EsotericInstanceMethods | EsotericInstanceMethods |
+| calls.rb:491:1:491:23 | EsotericInstanceMethods | EsotericInstanceMethods |
+| calls.rb:492:1:492:23 | EsotericInstanceMethods | EsotericInstanceMethods |
+| calls.rb:493:1:493:23 | EsotericInstanceMethods | EsotericInstanceMethods |
+| calls.rb:494:1:494:23 | EsotericInstanceMethods | EsotericInstanceMethods |
 | calls.rb:504:1:504:21 | ExtendSingletonMethod | ExtendSingletonMethod |
 | calls.rb:507:12:507:32 | ExtendSingletonMethod | ExtendSingletonMethod |
 | calls.rb:510:1:510:22 | ExtendSingletonMethod2 | ExtendSingletonMethod2 |
@@ -373,6 +395,14 @@ resolveConstantReadAccess
 | private.rb:100:7:100:22 | PrivateOverride1 | PrivateOverride1 |
 | private.rb:104:1:104:16 | PrivateOverride2 | PrivateOverride2 |
 | private.rb:105:1:105:16 | PrivateOverride2 | PrivateOverride2 |
+| toplevel_self_singleton.rb:18:12:18:17 | Struct | Struct |
+| unresolved_subclass.rb:4:7:4:25 | UnresolvedNamespace | UnresolvedNamespace |
+| unresolved_subclass.rb:4:40:4:58 | ResolvableBaseClass | ResolvableBaseClass |
+| unresolved_subclass.rb:7:7:7:25 | UnresolvedNamespace | UnresolvedNamespace |
+| unresolved_subclass.rb:7:40:7:58 | UnresolvedNamespace | UnresolvedNamespace |
+| unresolved_subclass.rb:7:40:7:69 | Subclass1 | UnresolvedNamespace::Subclass1 |
+| unresolved_subclass.rb:11:7:11:25 | UnresolvedNamespace | UnresolvedNamespace |
+| unresolved_subclass.rb:11:32:11:50 | UnresolvedNamespace | UnresolvedNamespace |
 resolveConstantWriteAccess
 | calls.rb:21:1:34:3 | M | M |
 | calls.rb:43:1:58:3 | C | C |
@@ -469,6 +499,10 @@ resolveConstantWriteAccess
 | toplevel_self_singleton.rb:2:5:5:7 | B | A::B |
 | toplevel_self_singleton.rb:18:1:18:8 | MyStruct | MyStruct |
 | toplevel_self_singleton.rb:24:1:34:3 | Good | Good |
+| unresolved_subclass.rb:1:1:2:3 | ResolvableBaseClass | ResolvableBaseClass |
+| unresolved_subclass.rb:4:1:5:3 | Subclass1 | UnresolvedNamespace::Subclass1 |
+| unresolved_subclass.rb:7:1:8:3 | Subclass2 | UnresolvedNamespace::Subclass2 |
+| unresolved_subclass.rb:11:1:12:3 | A | UnresolvedNamespace::A |
 enclosingModule
 | calls.rb:1:1:3:3 | foo | calls.rb:1:1:616:32 | calls.rb |
 | calls.rb:2:5:2:14 | call to puts | calls.rb:1:1:616:32 | calls.rb |
@@ -1792,3 +1826,15 @@ enclosingModule
 | toplevel_self_singleton.rb:30:13:30:19 | self | toplevel_self_singleton.rb:25:5:33:7 | class << ... |
 | toplevel_self_singleton.rb:31:13:31:20 | call to call_you | toplevel_self_singleton.rb:25:5:33:7 | class << ... |
 | toplevel_self_singleton.rb:31:13:31:20 | self | toplevel_self_singleton.rb:25:5:33:7 | class << ... |
+| unresolved_subclass.rb:1:1:2:3 | ResolvableBaseClass | unresolved_subclass.rb:1:1:12:4 | unresolved_subclass.rb |
+| unresolved_subclass.rb:4:1:5:3 | Subclass1 | unresolved_subclass.rb:1:1:12:4 | unresolved_subclass.rb |
+| unresolved_subclass.rb:4:7:4:25 | UnresolvedNamespace | unresolved_subclass.rb:1:1:12:4 | unresolved_subclass.rb |
+| unresolved_subclass.rb:4:40:4:58 | ResolvableBaseClass | unresolved_subclass.rb:1:1:12:4 | unresolved_subclass.rb |
+| unresolved_subclass.rb:7:1:8:3 | Subclass2 | unresolved_subclass.rb:1:1:12:4 | unresolved_subclass.rb |
+| unresolved_subclass.rb:7:7:7:25 | UnresolvedNamespace | unresolved_subclass.rb:1:1:12:4 | unresolved_subclass.rb |
+| unresolved_subclass.rb:7:40:7:58 | UnresolvedNamespace | unresolved_subclass.rb:1:1:12:4 | unresolved_subclass.rb |
+| unresolved_subclass.rb:7:40:7:69 | Subclass1 | unresolved_subclass.rb:1:1:12:4 | unresolved_subclass.rb |
+| unresolved_subclass.rb:11:1:12:3 | A | unresolved_subclass.rb:1:1:12:4 | unresolved_subclass.rb |
+| unresolved_subclass.rb:11:7:11:25 | UnresolvedNamespace | unresolved_subclass.rb:1:1:12:4 | unresolved_subclass.rb |
+| unresolved_subclass.rb:11:32:11:50 | UnresolvedNamespace | unresolved_subclass.rb:1:1:12:4 | unresolved_subclass.rb |
+| unresolved_subclass.rb:11:32:11:53 | B | unresolved_subclass.rb:1:1:12:4 | unresolved_subclass.rb |

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

@@ -1,6 +1,14 @@
 #-----| Class
 #-----|  -> Module
 
+#-----| EsotericInstanceMethods
+
+#-----| MyStruct
+
+#-----| Struct
+
+#-----| UnresolvedNamespace
+
 #-----| BasicObject
 
 #-----| Complex
@@ -230,3 +238,16 @@ toplevel_self_singleton.rb:
 #-----|  -> Object
 
 #   24| Good
+
+unresolved_subclass.rb:
+#    1| ResolvableBaseClass
+#-----|  -> Object
+
+#    4| UnresolvedNamespace::Subclass1
+#-----|  -> ResolvableBaseClass
+
+#    7| UnresolvedNamespace::Subclass2
+#-----|  -> UnresolvedNamespace::Subclass1
+
+#   11| UnresolvedNamespace::A
+#-----|  -> Object

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

@@ -0,0 +1,12 @@
+class ResolvableBaseClass
+end
+
+class UnresolvedNamespace::Subclass1 < ResolvableBaseClass
+end
+
+class UnresolvedNamespace::Subclass2 < UnresolvedNamespace::Subclass1
+end
+
+# Ensure Object is a transitive superclass of this
+class UnresolvedNamespace::A < UnresolvedNamespace::B
+end