add support for module instantiations in import statements. Rework the import resolution logic to reuse the logic from ModuleExpr

Author: erik-krogh

ql/Cargo.lock

@@ -10,7 +10,7 @@ edition = "2018"
 flate2 = "1.0"
 node-types = { path = "../node-types" }
 tree-sitter = ">= 0.20, < 0.21"
-tree-sitter-ql = { git = "https://github.com/erik-krogh/tree-sitter-ql.git", rev = "dcf0139af9908dbc53efa96408b017133d19e6b2"}
+tree-sitter-ql = { git = "https://github.com/erik-krogh/tree-sitter-ql.git", rev = "13155b740d4040e74727a9a698277805498a6a85"}
 tree-sitter-ql-dbscheme = { git = "https://github.com/erik-krogh/tree-sitter-ql-dbscheme.git", rev = "63e1344353f63931e88bfbc2faa2e78e1421b213"}
 tree-sitter-ql-yaml = {git = "https://github.com/erik-krogh/tree-sitter-ql.git", rev = "cf704bf3671e1ae148e173464fb65a4d2bbf5f99"}
 clap = "2.33"

ql/extractor/Cargo.toml

@@ -11,6 +11,6 @@ clap = "2.33"
 node-types = { path = "../node-types" }
 tracing = "0.1"
 tracing-subscriber = { version = "0.3.3", features = ["env-filter"] }
-tree-sitter-ql = { git = "https://github.com/erik-krogh/tree-sitter-ql.git", rev = "dcf0139af9908dbc53efa96408b017133d19e6b2"}
+tree-sitter-ql = { git = "https://github.com/erik-krogh/tree-sitter-ql.git", rev = "13155b740d4040e74727a9a698277805498a6a85"}
 tree-sitter-ql-dbscheme = { git = "https://github.com/erik-krogh/tree-sitter-ql-dbscheme.git", rev = "63e1344353f63931e88bfbc2faa2e78e1421b213"}
 tree-sitter-ql-yaml = {git = "https://github.com/erik-krogh/tree-sitter-ql.git", rev = "cf704bf3671e1ae148e173464fb65a4d2bbf5f99"}

ql/generator/Cargo.toml

@@ -1175,13 +1175,27 @@ class Import extends TImport, ModuleMember, TypeRef {
   string importedAs() { result = imp.getChild(1).(QL::ModuleName).getChild().getValue() }
 
   /**
-   * Gets the `i`th selected name from the imported module.
+   * Gets the qualified name of the module selected in the import statement.
    * E.g. for
    * `import foo.bar::Baz::Qux`
-   * It is true that `getSelectionName(0) = "Baz"` and `getSelectionName(1) = "Qux"`.
+   * It is true that `getSelectionName() = "Baz::Qux"`.
+   *
+   * Does NOT include type arguments!
+   */
+  string getSelectionName() { result = getModuleExpr().getQualifiedName() }
+
+  /**
+   * Gets the module expression selected in the import statement.
+   * E.g. for
+   * `import foo.Bar::Baz::Qux`
+   * The module expression is the `Bar::Baz::Qux` part.
    */
-  string getSelectionName(int i) {
-    result = imp.getChild(0).(QL::ImportModuleExpr).getName(i).getValue()
+  ModuleExpr getModuleExpr() { toQL(result) = imp.getChild(0).(QL::ImportModuleExpr).getChild() }
+
+  override AstNode getAChild(string pred) {
+    result = super.getAChild(pred)
+    or
+    pred = directMember("getModuleExpr") and result = this.getModuleExpr()
   }
 
   /**
@@ -1191,27 +1205,25 @@ class Import extends TImport, ModuleMember, TypeRef {
    * It is true that `getQualifiedName(0) = "foo"` and `getQualifiedName(1) = "bar"`.
    */
   string getQualifiedName(int i) {
-    result = imp.getChild(0).(QL::ImportModuleExpr).getChild().getName(i).getValue()
+    result = imp.getChild(0).(QL::ImportModuleExpr).getQualName(i).getValue()
   }
 
   /**
-   * Gets the full string specifying the module being imported.
+   * Gets a full string specifying the module being imported.
+   *
+   * Does NOT include type arguments!
    */
   string getImportString() {
-    exists(string selec |
-      not exists(this.getSelectionName(_)) and selec = ""
+    exists(string qual |
+      not exists(this.getQualifiedName(_)) and qual = ""
       or
-      selec =
-        "::" + strictconcat(int i, string q | q = this.getSelectionName(i) | q, "::" order by i)
+      qual = strictconcat(int i, string q | q = this.getQualifiedName(i) | q, "." order by i) + "."
     |
-      result =
-        strictconcat(int i, string q | q = this.getQualifiedName(i) | q, "." order by i) + selec
+      result = qual + this.getSelectionName()
     )
   }
 
-  override Type getResolvedType() {
-    exists(FileOrModule mod | resolve(this, mod) | result = mod.toType())
-  }
+  override Type getResolvedType() { result = this.getModuleExpr().getResolvedType() }
 }
 
 /** A formula, such as `x = 6 and y < 5`. */
@@ -2286,6 +2298,19 @@ class ModuleExpr extends TModuleExpr, TypeRef {
   SignatureExpr getArgument(int i) {
     result.toQL() = me.getAFieldOrChild().(QL::ModuleInstantiation).getChild(i)
   }
+
+  /**
+   * Gets the qualified name for this module expression, which does not include the type arguments.
+   */
+  string getQualifiedName() {
+    exists(string qual |
+      not exists(this.getQualifier()) and qual = ""
+      or
+      qual = this.getQualifier().getQualifiedName() + "::"
+    |
+      result = qual + this.getName()
+    )
+  }
 }
 
 /** A signature expression, either a `PredicateExpr` or a `TypeExpr`. */

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

@@ -120,7 +120,7 @@ class Module_ extends FileOrModule, TModule {
   }
 }
 
-private predicate resolveQualifiedName(Import imp, ContainerOrModule m, int i) {
+private predicate resolveImportQualifier(Import imp, ContainerOrModule m, int i) {
   not m = TFile(any(File f | f.getExtension() = "ql")) and
   exists(string q | q = imp.getQualifiedName(i) |
     i = 0 and
@@ -139,7 +139,6 @@ private predicate resolveQualifiedName(Import imp, ContainerOrModule m, int i) {
         m = TFolder(c)
       )
       or
-      q = imp.getQualifiedName(i) and
       exists(ContainerOrModule container | container = getEnclosingModule(imp).getEnclosing+() |
         definesModule(container, q, m, _) and
         (
@@ -154,25 +153,18 @@ private predicate resolveQualifiedName(Import imp, ContainerOrModule m, int i) {
     )
     or
     exists(Folder_ mid |
-      resolveQualifiedName(imp, mid, i - 1) and
+      resolveImportQualifier(imp, mid, i - 1) and
       m.getEnclosing() = mid and
       q = m.getName()
     )
   )
 }
 
-private predicate resolveSelectionName(Import imp, ContainerOrModule m, int i) {
+private predicate resolveImportQualifier(Import imp, ContainerOrModule m) {
   (m.(File_).getFile().getExtension() = "qll" or not m instanceof File_) and
   exists(int last |
-    resolveQualifiedName(imp, m, last) and
-    last = count(int j | exists(imp.getQualifiedName(j))) - 1
-  ) and
-  not m instanceof Folder_ and
-  i = -1
-  or
-  exists(ContainerOrModule mid |
-    resolveSelectionName(imp, mid, i - 1) and
-    definesModule(mid, imp.getSelectionName(i), m, true)
+    last = max(int j | exists(imp.getQualifiedName(j))) and
+    resolveImportQualifier(imp, m, last)
   )
 }
 
@@ -202,18 +194,10 @@ private module Cached {
       TModule(Module m)
   }
 
-  /** Holds if import statement `imp` resolves to `m`. */
-  cached
-  predicate resolve(Import imp, FileOrModule m) {
-    exists(int last |
-      resolveSelectionName(imp, m, last) and
-      last = count(int j | exists(imp.getSelectionName(j))) - 1
-    )
-  }
-
   /** Holds if module expression `me` resolves to `m`. */
   cached
   predicate resolveModuleRef(TypeRef me, FileOrModule m) {
+    // base case, resolving a typeref without a qualifier (only moduleexpr can have qualifiers)
     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) |
@@ -234,17 +218,69 @@ private module Cached {
       )
     )
     or
+    // recursive case, resolving the qualifier.
     exists(FileOrModule mid |
       resolveModuleRef(me.(ModuleExpr).getQualifier(), mid) and
       definesModule(mid, me.(ModuleExpr).getName(), m, true)
     )
   }
 
+  /**
+   * Gets the module that the lookup for `ref` should start at.
+   * For most type references this will simply be the enclosing module.
+   *
+   * However, for module expressions inside imports, this will be determined
+   * by the qualified name of the import (everything before the module expression).
+   *
+   * E.g. for an import `import foo.Bar::Baz`, the qualified name of the import is "foo",
+   * and the module expression is "Bar::Baz".
+   */
+  private ContainerOrModule getStartModule(TypeRef ref) {
+    if isInsideImport(ref)
+    then
+      exists(Import i | ref = i.getModuleExpr().getQualifier*() |
+        resolveImportQualifier(i, result)
+        or
+        not exists(i.getQualifiedName(_)) and
+        (
+          result = getEnclosingModule(ref)
+          or
+          exists(YAML::QLPack pack |
+            pack.getAFileInPack() = ref.getLocation().getFile() and
+            result = TFolder(pack.getADependency*().getFile().getParentContainer())
+          )
+        )
+      )
+    else result = getEnclosingModule(ref)
+  }
+
+  /** Holds of `me` is part of an import statement. */
+  pragma[noinline]
+  private predicate isInsideImport(ModuleExpr me) {
+    me = any(Import i).getModuleExpr().getQualifier*()
+  }
+
+  /** Gets the enclosing module/container, but stops after the first folder (so no folder -> folder step). */
+  private ContainerOrModule getEnclosingModuleNoFolderStep(ContainerOrModule m) {
+    result = m.getEnclosing() and
+    not (
+      result instanceof Folder_ and
+      m instanceof Folder_
+    )
+  }
+
   pragma[noinline]
   private predicate resolveModuleRefHelper(TypeRef me, ContainerOrModule enclosing, string name) {
-    enclosing = getEnclosingModule(me).getEnclosing*() and
+    // The scope is all enclosing modules, the immidiatly containing folder, not the parent folders.
+    enclosing = getEnclosingModuleNoFolderStep*(getStartModule(me)) and
     name = [me.(ModuleExpr).getName(), me.(TypeExpr).getClassName()] and
-    (not me instanceof ModuleExpr or not enclosing instanceof Folder_) // module expressions are not imports, so they can't resolve to a file (which is contained in a folder).
+    not exists(me.(ModuleExpr).getQualifier()) and
+    (
+      // module expressions are not imports, so they can't resolve to a file (which is contained in a folder).
+      (not me instanceof ModuleExpr or not enclosing instanceof Folder_)
+      or
+      isInsideImport(me) // unless it actually is an import.
+    )
   }
 }
 
@@ -296,7 +332,7 @@ private predicate definesModule(
   // import X
   exists(Import imp, ContainerOrModule m0 |
     container = getEnclosingModule(imp) and
-    resolve(imp, m0) and
+    resolveModuleRef(imp.getModuleExpr(), m0) and
     not exists(imp.importedAs()) and
     definesModule(m0, name, m, true) and
     public = getPublicBool(imp)
@@ -306,7 +342,7 @@ private predicate definesModule(
   exists(Import imp |
     container = getEnclosingModule(imp) and
     name = imp.importedAs() and
-    resolve(imp, m) and
+    resolveModuleRef(imp.getModuleExpr(), m) and
     public = getPublicBool(imp)
   )
   or
@@ -320,24 +356,6 @@ private predicate definesModule(
 }
 
 module ModConsistency {
-  query predicate noResolve(Import imp) {
-    not resolve(imp, _) and
-    not imp.getLocation()
-        .getFile()
-        .getAbsolutePath()
-        .regexpMatch(".*/(test|examples|ql-training|recorded-call-graph-metrics)/.*")
-  }
-
-  query predicate multipleResolve(Import imp, int c, ContainerOrModule m) {
-    c = strictcount(ContainerOrModule m0 | resolve(imp, m0)) and
-    c > 1 and
-    resolve(imp, m) and
-    not imp.getLocation()
-        .getFile()
-        .getAbsolutePath()
-        .regexpMatch(".*/(test|examples|ql-training|recorded-call-graph-metrics)/.*")
-  }
-
   // This can happen with parameterized modules.
   /*
    * query predicate multipleResolveModuleRef(ModuleExpr me, int c, ContainerOrModule m) {
@@ -362,4 +380,16 @@ module ModConsistency {
     mod instanceof ModuleExpr and
     count(mod.(ModuleExpr).getName()) >= 2
   }
+
+  query predicate uniqueResolve(Import i) {
+    count(FileOrModule mod |
+      mod = i.getResolvedModule() and
+      // don't count the alias reference, only the resolved.
+      not exists(mod.asModule().getAlias())
+    ) >= 2 and
+    // paramerized modules are not treated nicely, so we ignore them here.
+    not i.getResolvedModule().getEnclosing*().asModule().hasParameter(_, _, _)
+  }
+
+  query predicate noResolve(Import i) { not exists(i.getResolvedModule()) }
 }

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

@@ -3,6 +3,7 @@ private import Builtins
 private import codeql_ql.ast.internal.Module
 private import codeql_ql.ast.internal.AstNodes
 
+pragma[nomagic]
 private predicate definesPredicate(
   FileOrModule m, string name, int arity, Predicate p, boolean public
 ) {

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

@@ -602,15 +602,15 @@ module QL {
     /** Gets the name of the primary QL class for this element. */
     final override string getAPrimaryQlClass() { result = "ImportModuleExpr" }
 
-    /** Gets the node corresponding to the field `name`. */
-    final SimpleId getName(int i) { ql_import_module_expr_name(this, i, result) }
+    /** Gets the node corresponding to the field `qualName`. */
+    final SimpleId getQualName(int i) { ql_import_module_expr_qual_name(this, i, result) }
 
     /** Gets the child of this node. */
-    final QualModuleExpr getChild() { ql_import_module_expr_def(this, result) }
+    final ModuleExpr getChild() { ql_import_module_expr_def(this, result) }
 
     /** Gets a field or child node of this node. */
     final override AstNode getAFieldOrChild() {
-      ql_import_module_expr_name(this, _, result) or ql_import_module_expr_def(this, result)
+      ql_import_module_expr_qual_name(this, _, result) or ql_import_module_expr_def(this, result)
     }
   }
 
@@ -956,18 +956,6 @@ module QL {
     final override string getAPrimaryQlClass() { result = "Qldoc" }
   }
 
-  /** A class representing `qualModuleExpr` nodes. */
-  class QualModuleExpr extends @ql_qual_module_expr, AstNode {
-    /** Gets the name of the primary QL class for this element. */
-    final override string getAPrimaryQlClass() { result = "QualModuleExpr" }
-
-    /** Gets the node corresponding to the field `name`. */
-    final SimpleId getName(int i) { ql_qual_module_expr_name(this, i, result) }
-
-    /** Gets a field or child node of this node. */
-    final override AstNode getAFieldOrChild() { ql_qual_module_expr_name(this, _, result) }
-  }
-
   /** A class representing `qualifiedRhs` nodes. */
   class QualifiedRhs extends @ql_qualified_rhs, AstNode {
     /** Gets the name of the primary QL class for this element. */

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

@@ -327,6 +327,12 @@ private predicate defines(FileOrModule m, string name, Type t, boolean public) {
     public = getPublicBool(ty.getParent())
   )
   or
+  exists(Module mod | t = TModule(mod) |
+    getEnclosingModule(mod) = m and
+    mod.getName() = name and
+    public = getPublicBool(mod)
+  )
+  or
   exists(Class ty | t = TUnion(ty) |
     getEnclosingModule(ty) = m and
     ty.getName() = name and

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

@@ -83,6 +83,8 @@ private AstNode aliveStep(AstNode prev) {
   //
   // The recursive cases.
   //
+  result = prev.(Import).getModuleExpr()
+  or
   result.getEnclosingPredicate() = prev
   or
   result = prev.(Call).getTarget()

ql/ql/src/codeql_ql/style/DeadCodeQuery.qll

@@ -35,6 +35,7 @@ predicate importsFromSameFolder(Import a, Import b) {
 predicate problem(Import imp, Import redundant, string message) {
   not exists(imp.importedAs()) and
   not exists(redundant.importedAs()) and
+  not exists(imp.getModuleExpr().getQualifier*().getArgument(_)) and // any type-arguments, and we ignore, they might be different.
   // skip the top-level language files, they have redundant imports, and that's fine.
   not exists(imp.getLocation().getFile().getParentContainer().getFile("qlpack.yml")) and
   // skip the DataFlowImpl.qll and similar, they have redundant imports in some copies.