use resolveConstanteWriteAccess instead, add a few more test cases

Author: jeffgran-dox

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

@@ -40,16 +40,6 @@ class ConstantAccess extends Expr, TConstantAccess {
    */
   predicate hasGlobalScope() { none() }
 
-  // gets the full name
-  string getFullName() {
-    exists(ConstantAccess ca | this.getScopeExpr() = ca |
-    result = ca.getFullName() + "::" + this.getName())
-    or
-    // TODO if the getScopeExpr is not a constant, try to figure out which constants it could be?
-    not exists(ConstantAccess ca | this.getScopeExpr() = ca) and
-    result = this.getName()
-  }
-
   override string toString() { result = this.getName() }
 
   override AstNode getAChild(string pred) {
@@ -194,16 +184,34 @@ class ConstantWriteAccess extends ConstantAccess {
    *
    * the constant `Baz` has the fully qualified name `Foo::Bar::Baz`, and
    * `CONST_A` has the fully qualified name `Foo::CONST_A`.
+   *
+   * Important note: This can return more than one value, because there are
+   * situations where there can be multiple possible "fully qualified" names.
+   * For example:
+   * ```
+   * module Mod4
+   *   include Mod1
+   *   module Mod3::Mod5 end
+   * end
+   * ```
+   * In the above snippet, `Mod5` has two valid fully qualified names it can be
+   * referred to by: `Mod1::Mod3::Mod5`, or `Mod4::Mod3::Mod5`.
+   *
+   * Another example has to do with the order in which module definitions are
+   * executed at runtime. Because of the way that ruby dynamically looks up
+   * constants up the namespace chain, the fully qualified name of a nested
+   * constant can be ambiguous from just statically looking at the AST.
    */
-  string getQualifiedName() {
-    /* get the qualified name for the parent module, then append w */
-    exists(ConstantWriteAccess parent | parent = this.getEnclosingModule() |
-      result = parent.getQualifiedName() + "::" + this.getFullName()
-    )
-    or
-    /* base case - there's no parent module */
-    not exists(ConstantWriteAccess parent | parent = this.getEnclosingModule()) and
-    result = this.getFullName()
+  string getAQualifiedName() {
+    result = resolveConstantWriteAccess(this)
+  }
+
+  /**
+   * gets a qualified name for this constant. Deprecated in favor of
+   * `getAQualifiedName` because this can return more than one value
+   */
+  deprecated string getQualifiedName() {
+    result = this.getAQualifiedName()
   }
 }
 

ruby/ql/test/library-tests/ast/constants/constants.expected

@@ -42,6 +42,12 @@ constantAccess
 | constants.rb:50:3:51:5 | ClassB | write | ClassB | ClassDeclaration |
 | constants.rb:50:9:50:15 | ModuleB | read | ModuleB | ConstantReadAccess |
 | constants.rb:50:27:50:30 | Base | read | Base | ConstantReadAccess |
+| constants.rb:54:1:56:3 | Mod1 | write | Mod1 | ModuleDeclaration |
+| constants.rb:55:3:55:17 | Mod3 | write | Mod3 | ModuleDeclaration |
+| constants.rb:58:1:61:3 | Mod4 | write | Mod4 | ModuleDeclaration |
+| constants.rb:59:11:59:14 | Mod1 | read | Mod1 | ConstantReadAccess |
+| constants.rb:60:3:60:23 | Mod5 | write | Mod5 | ModuleDeclaration |
+| constants.rb:60:10:60:13 | Mod3 | read | Mod3 | ConstantReadAccess |
 getConst
 | constants.rb:1:1:15:3 | ModuleA | CONST_B | constants.rb:6:15:6:23 | "const_b" |
 | constants.rb:2:5:4:7 | ModuleA::ClassA | CONST_A | constants.rb:3:19:3:27 | "const_a" |
@@ -79,4 +85,11 @@ constantWriteAccessQualifiedName
 | constants.rb:44:1:47:3 | ModuleB | ModuleA::ModuleB |
 | constants.rb:45:3:46:5 | ClassB | ModuleA::ModuleB::ClassB |
 | constants.rb:49:1:52:3 | ModuleA | ModuleA |
-| constants.rb:50:3:51:5 | ClassB | ModuleA::ModuleB::ClassB |
+| constants.rb:50:3:51:5 | ClassB | ModuleA::ModuleB::ClassB |
+| constants.rb:50:3:51:5 | ClassB | ModuleB::ClassB |
+| constants.rb:54:1:56:3 | Mod1 | Mod1 |
+| constants.rb:55:3:55:17 | Mod3 | Mod1::Mod3 |
+| constants.rb:58:1:61:3 | Mod4 | Mod4 |
+| constants.rb:60:3:60:23 | Mod5 | Mod1::Mod3::Mod5 |
+| constants.rb:60:3:60:23 | Mod5 | Mod4::Mod3::Mod5 |
+| constants.rb:60:3:60:23 | Mod5 | Mod3::Mod5 |

ruby/ql/test/library-tests/ast/constants/constants.ql

@@ -18,5 +18,5 @@ query Expr lookupConst(Module m, string name) { result = M::lookupConst(m, name)
 query predicate constantValue(ConstantReadAccess a, Expr e) { e = a.getValue() }
 
 query predicate constantWriteAccessQualifiedName(ConstantWriteAccess w, string qualifiedName) {
-  w.getQualifiedName() = qualifiedName
+  w.getAQualifiedName() = qualifiedName
 }

ruby/ql/test/library-tests/ast/constants/constants.rb

@@ -50,3 +50,12 @@ module ModuleA
   class ModuleB::ClassB < Base
   end
 end
+
+module Mod1
+  module Mod3 end
+end
+
+module Mod4
+  include Mod1
+  module Mod3::Mod5 end
+end