just use TypeExpr to resolve modules

Author: erik-krogh

ql/ql/src/codeql_ql/ast/Ast.qll

@@ -2191,17 +2191,14 @@ class DontCare extends TDontCare, Expr {
   override string getAPrimaryQlClass() { result = "DontCare" }
 }
 
-/** A reference to a module as part of a parameterized module (or it's instantiation) */
-class ModuleParameterRef extends ModuleRef, TModuleParameterRef {
-  QL::TypeExpr type;
-
-  ModuleParameterRef() { this = TModuleParameterRef(type) }
-
+/**
+ * A type expression seen as a reference to a module as part of a parameterized module (or it's instantiation).
+ * This might not be a reference to a module, but we assume so until we find out in the resolve phase.
+ */
+class ModuleParameterRef extends ModuleRef instanceof TypeExpr {
   final override FileOrModule getResolvedModule() { resolveModuleRef(this, result) }
 
-  override string getName() { result = type.getName().getValue() }
-
-  override string getAPrimaryQlClass() { result = "ModuleParameterRef" }
+  override string getName() { result = TypeExpr.super.getClassName() }
 }
 
 /** A module expression. Such as `DataFlow` in `DataFlow::Node` */
@@ -2219,7 +2216,15 @@ class ModuleExpr extends TModuleExpr, ModuleRef {
    *
    * is `Bar`.
    */
-  override string getName() { result = getNameForModuleExpr(me) }
+  override string getName() {
+    result = me.getName().(QL::SimpleId).getValue()
+    or
+    not exists(me.getName()) and result = me.getChild().(QL::SimpleId).getValue()
+    or
+    exists(QL::ModuleInstantiation instantiation | instantiation.getParent() = me |
+      result = instantiation.getName().getChild().getValue()
+    )
+  }
 
   /**
    * Gets the qualifier of this module expression. For example, the qualifier of

ql/ql/src/codeql_ql/ast/internal/AstNodes.qll

@@ -19,11 +19,7 @@ newtype TAstNode =
   TNewType(QL::Datatype dt) or
   TNewTypeBranch(QL::DatatypeBranch branch) or
   TImport(QL::ImportDirective imp) or
-  // splitting up the TypeExpr based on whether they are a reference to a type or to a module, with some duplicates. 
-  TType(QL::TypeExpr type) { not isDefinitelyModuleParameter(type) } or
-  TModuleParameterRef(QL::TypeExpr type) {
-    isDefinitelyModuleParameter(type) or mightBeModuleParameter(type)
-  } or
+  TType(QL::TypeExpr type) or
   TDisjunction(QL::Disjunction disj) or
   TConjunction(QL::Conjunction conj) or
   TComparisonFormula(QL::CompTerm comp) or
@@ -87,43 +83,11 @@ class TExpr =
 
 class TCall = TPredicateCall or TMemberCall or TNoneCall or TAnyCall;
 
-class TModuleRef = TImport or TModuleExpr or TModuleParameterRef;
+class TModuleRef = TImport or TModuleExpr or TType;
 
 class TYamlNode = TYamlCommemt or TYamlEntry or TYamlKey or TYamlListitem or TYamlValue;
 
-class TSignatureExpr = TPredicateExpr or TType or TModuleParameterRef;
-
-private predicate isDefinitelyModuleParameter(QL::TypeExpr type) {
-  // the signature of a parameterized module
-  exists(QL::SignatureExpr expr, QL::Module m, QL::ModuleParam param, string name |
-    param = m.getParameter(_) and
-    name = param.getParameter().getValue() and
-    expr = param.getSignature() and
-    type = expr.getTypeExpr() and
-    // we have a ref to it, to confirm that it's actually a module.
-    exists(QL::ModuleExpr ref | getNameForModuleExpr(ref) = name | ref.getParent+() = m)
-  )
-  or
-  // the implements clause of a module.
-  exists(QL::Module qlmod | qlmod.getImplements(_).getTypeExpr() = type)
-}
-
-private predicate mightBeModuleParameter(QL::TypeExpr type) {
-  // or it's in an instantiation. The only way to know for sure if it's a module ref or not is to try, so we just have duplicates in the AST.
-  // we spuriously have both TypeExpr and ModuleParameterRef for the same thing.
-  exists(QL::ModuleInstantiation inst | type = inst.getChild(_).getTypeExpr())
-}
-
-// ensuring non-monotonic recursion by outlining predicate out of `ModuleExpr`
-string getNameForModuleExpr(QL::ModuleExpr me) {
-  result = me.getName().(QL::SimpleId).getValue()
-  or
-  not exists(me.getName()) and result = me.getChild().(QL::SimpleId).getValue()
-  or
-  exists(QL::ModuleInstantiation instantiation | instantiation.getParent() = me |
-    result = instantiation.getName().getChild().getValue()
-  )
-}
+class TSignatureExpr = TPredicateExpr or TType;
 
 /** DEPRECATED: Alias for TYamlNode */
 deprecated class TYAMLNode = TYamlNode;
@@ -212,8 +176,6 @@ QL::AstNode toQL(AST::AstNode n) {
   or
   n = TNewTypeBranch(result)
   or
-  n = TModuleParameterRef(result)
-  or
   n = TImport(result)
   or
   n = TType(result)

ql/ql/src/codeql_ql/ast/internal/Module.qll

@@ -24,11 +24,11 @@ private class ContainerOrModule extends TContainerOrModule {
     this = TFolder(_) and result = "folder"
   }
 
-    /** Gets the module for this imported module. */
-    Module asModule() { this = TModule(result) }
+  /** Gets the module for this imported module. */
+  Module asModule() { this = TModule(result) }
 
-    /** Gets the file for this file. */
-    File asFile() { this = TFile(result) }
+  /** Gets the file for this file. */
+  File asFile() { this = TFile(result) }
 }
 
 private class TFileOrModule = TFile or TModule;
@@ -115,7 +115,6 @@ class Module_ extends FileOrModule, TModule {
 }
 
 // class ModuleRef = AstNodes::TModuleExpr or AstNodes::TType;
-
 private predicate resolveQualifiedName(Import imp, ContainerOrModule m, int i) {
   not m = TFile(any(File f | f.getExtension() = "ql")) and
   exists(string q | q = imp.getQualifiedName(i) |
@@ -209,7 +208,7 @@ private module Cached {
 
   /** Holds if module expression `me` resolves to `m`. */
   cached
-  predicate resolveModuleRef(ModuleRef me, FileOrModule m) { // TODO: name.
+  predicate resolveModuleRef(ModuleRef me, FileOrModule m) {
     not m = TFile(any(File f | f.getExtension() = "ql")) and
     not exists(me.(ModuleExpr).getQualifier()) and
     exists(ContainerOrModule enclosing, string name | resolveModuleRefHelper(me, enclosing, name) |
@@ -257,17 +256,17 @@ private predicate definesModule(
     or
     m = TModule(any(Module mod | public = getPublicBool(mod)))
   )
-   or
+  or
   // signature module in a paramertized module
-  exists(Module mod, SignatureExpr sig, ModuleParameterRef ty, int i | 
+  exists(Module mod, SignatureExpr sig, ModuleParameterRef ty, int i |
     mod = container.asModule() and
     mod.hasParameter(i, name, sig) and
     public = false and
-    ty = sig.asModuleRef() 
-    |
+    ty = sig.asModuleRef()
+  |
     m = ty.getResolvedModule()
     or
-    exists(ModuleExpr inst | inst.getResolvedModule().asModule() = mod | 
+    exists(ModuleExpr inst | inst.getResolvedModule().asModule() = mod |
       m = inst.getArgument(i).asModuleRef().getResolvedModule()
     )
   )
@@ -307,12 +306,14 @@ module ModConsistency {
         .regexpMatch(".*/(test|examples|ql-training|recorded-call-graph-metrics)/.*")
   }
 
-  query predicate noResolveModuleRef(ModuleRef me) { // TODO: name?
+  query predicate noResolveModuleRef(ModuleRef me) {
     not exists(me.getResolvedModule()) and
     not me.getLocation()
         .getFile()
         .getAbsolutePath()
-        .regexpMatch(".*/(test|examples|ql-training|recorded-call-graph-metrics)/.*")
+        .regexpMatch(".*/(test|examples|ql-training|recorded-call-graph-metrics)/.*") and
+    // this ModuleRef might really be a type.
+    not exists(me.(TypeExpr).getResolvedType())
   }
 
   query predicate multipleResolve(Import imp, int c, ContainerOrModule m) {

ql/ql/src/codeql_ql/ast/internal/Type.qll

@@ -350,7 +350,6 @@ module TyConsistency {
         .getAbsolutePath()
         .regexpMatch(".*/(test|examples|ql-training|recorded-call-graph-metrics)/.*") and
     // we have some duplicate with moduleRef, so that might be resolved correctly.
-    // TODO: Collapse both ModuleRef and TypeExpr into one class?
     not exists(ModuleRef ref | AstNodes::toQL(te) = AstNodes::toQL(ref) |
       exists(ref.getResolvedModule())
     )