Merge pull request #3470 from asger-semmle/js/cache-module-import

Approved by esbena

javascript/ql/src/semmle/javascript/AMD.qll

@@ -192,7 +192,7 @@ private class AmdDependencyPath extends PathExprCandidate {
 }
 
 /** A constant path element appearing in an AMD dependency expression. */
-private class ConstantAmdDependencyPathElement extends PathExprInModule, ConstantString {
+private class ConstantAmdDependencyPathElement extends PathExpr, ConstantString {
   ConstantAmdDependencyPathElement() { this = any(AmdDependencyPath amd).getAPart() }
 
   override string getValue() { result = getStringValue() }

javascript/ql/src/semmle/javascript/ES2015Modules.qll

@@ -53,7 +53,7 @@ class ES2015Module extends Module {
 class ImportDeclaration extends Stmt, Import, @importdeclaration {
   override ES2015Module getEnclosingModule() { result = getTopLevel() }
 
-  override PathExprInModule getImportedPath() { result = getChildExpr(-1) }
+  override PathExpr getImportedPath() { result = getChildExpr(-1) }
 
   /** Gets the `i`th import specifier of this import declaration. */
   ImportSpecifier getSpecifier(int i) { result = getChildExpr(i) }
@@ -82,7 +82,7 @@ class ImportDeclaration extends Stmt, Import, @importdeclaration {
 }
 
 /** A literal path expression appearing in an `import` declaration. */
-private class LiteralImportPath extends PathExprInModule, ConstantString {
+private class LiteralImportPath extends PathExpr, ConstantString {
   LiteralImportPath() { exists(ImportDeclaration req | this = req.getChildExpr(-1)) }
 
   override string getValue() { result = getStringValue() }
@@ -622,7 +622,7 @@ abstract class ReExportDeclaration extends ExportDeclaration {
 }
 
 /** A literal path expression appearing in a re-export declaration. */
-private class LiteralReExportPath extends PathExprInModule, ConstantString {
+private class LiteralReExportPath extends PathExpr, ConstantString {
   LiteralReExportPath() { exists(ReExportDeclaration bred | this = bred.getImportedPath()) }
 
   override string getValue() { result = getStringValue() }

javascript/ql/src/semmle/javascript/Expr.qll

@@ -2614,7 +2614,7 @@ class DynamicImportExpr extends @dynamicimport, Expr, Import {
 }
 
 /** A literal path expression appearing in a dynamic import. */
-private class LiteralDynamicImportPath extends PathExprInModule, ConstantString {
+private class LiteralDynamicImportPath extends PathExpr, ConstantString {
   LiteralDynamicImportPath() {
     exists(DynamicImportExpr di | this.getParentExpr*() = di.getSource())
   }

javascript/ql/src/semmle/javascript/Modules.qll

@@ -185,12 +185,14 @@ abstract class Import extends ASTNode {
 }
 
 /**
+ * DEPRECATED. Use `PathExpr` instead.
+ *
  * A path expression that appears in a module and is resolved relative to it.
  */
-abstract class PathExprInModule extends PathExpr {
-  PathExprInModule() { exists(getEnclosingModule()) }
-
-  override Folder getSearchRoot(int priority) {
-    getEnclosingModule().searchRoot(this, result, priority)
+abstract deprecated class PathExprInModule extends PathExpr {
+  PathExprInModule() {
+    this.(Expr).getTopLevel() instanceof Module
+    or
+    this.(Comment).getTopLevel() instanceof Module
   }
 }

javascript/ql/src/semmle/javascript/NodeJS.qll

@@ -266,14 +266,14 @@ private class RequirePath extends PathExprCandidate {
 }
 
 /** A constant path element appearing in a call to `require` or `require.resolve`. */
-private class ConstantRequirePathElement extends PathExprInModule, ConstantString {
+private class ConstantRequirePathElement extends PathExpr, ConstantString {
   ConstantRequirePathElement() { this = any(RequirePath rp).getAPart() }
 
   override string getValue() { result = getStringValue() }
 }
 
 /** A `__dirname` path expression. */
-private class DirNamePath extends PathExprInModule, VarAccess {
+private class DirNamePath extends PathExpr, VarAccess {
   DirNamePath() {
     getName() = "__dirname" and
     getVariable().getScope() instanceof ModuleScope
@@ -283,7 +283,7 @@ private class DirNamePath extends PathExprInModule, VarAccess {
 }
 
 /** A `__filename` path expression. */
-private class FileNamePath extends PathExprInModule, VarAccess {
+private class FileNamePath extends PathExpr, VarAccess {
   FileNamePath() {
     getName() = "__filename" and
     getVariable().getScope() instanceof ModuleScope
@@ -296,7 +296,7 @@ private class FileNamePath extends PathExprInModule, VarAccess {
  * A path expression of the form `path.join(p, "...")` where
  * `p` is also a path expression.
  */
-private class JoinedPath extends PathExprInModule, @callexpr {
+private class JoinedPath extends PathExpr, @callexpr {
   JoinedPath() {
     exists(MethodCallExpr call | call = this |
       call.getReceiver().(VarAccess).getName() = "path" and

javascript/ql/src/semmle/javascript/NodeModuleResolutionImpl.qll

@@ -112,7 +112,7 @@ class MainModulePath extends PathExpr, @json_string {
 
   override string getValue() { result = this.(JSONString).getValue() }
 
-  override Folder getSearchRoot(int priority) {
+  override Folder getAdditionalSearchRoot(int priority) {
     priority = 0 and
     result = pkg.getFile().getParentContainer()
   }

javascript/ql/src/semmle/javascript/Paths.qll

@@ -174,17 +174,6 @@ abstract class PathString extends string {
   Path resolve(Folder root) { result = resolveUpTo(getNumComponent(), root) }
 }
 
-/**
- * Non-abstract base class for path expressions.
- */
-private class PathExprBase extends Locatable {
-  // We must put getEnclosingModule here for it to be usable in the characteristic predicate of PathExprInModule
-  /** Gets the module containing this path expression, if any. */
-  Module getEnclosingModule() {
-    result = this.(Expr).getTopLevel() or result = this.(Comment).getTopLevel()
-  }
-}
-
 /**
  * An expression whose value represents a (relative or absolute) file system path.
  *
@@ -197,12 +186,25 @@ private class PathExprBase extends Locatable {
  * as their highest-priority root, with default library paths as additional roots
  * of lower priority.
  */
-abstract class PathExpr extends PathExprBase {
+abstract class PathExpr extends Locatable {
   /** Gets the (unresolved) path represented by this expression. */
   abstract string getValue();
 
   /** Gets the root folder of priority `priority` associated with this path expression. */
-  abstract Folder getSearchRoot(int priority);
+  Folder getSearchRoot(int priority) {
+    // We default to the enclosing module's search root, though this may be overridden.
+    getEnclosingModule().searchRoot(this, result, priority)
+    or
+    result = getAdditionalSearchRoot(priority)
+  }
+
+  /**
+   * INTERNAL. Use `getSearchRoot` instead.
+   *
+   * Can be overridden by subclasses of `PathExpr` to provide additional search roots
+   * without overriding `getSearchRoot`.
+   */
+  Folder getAdditionalSearchRoot(int priority) { none() }
 
   /** Gets the `i`th component of this path. */
   string getComponent(int i) { result = getValue().(PathString).getComponent(i) }
@@ -246,6 +248,11 @@ abstract class PathExpr extends PathExprBase {
 
   /** Gets the file or folder that this path refers to. */
   Container resolve() { result = resolveUpTo(getNumComponent()) }
+
+  /** Gets the module containing this path expression, if any. */
+  Module getEnclosingModule() {
+    result = this.(Expr).getTopLevel() or result = this.(Comment).getTopLevel()
+  }
 }
 
 /** A path string derived from a path expression. */
@@ -279,8 +286,8 @@ private class ConcatPath extends PathExpr {
     )
   }
 
-  override Folder getSearchRoot(int priority) {
-    result = this.(AddExpr).getAnOperand().(PathExpr).getSearchRoot(priority)
+  override Folder getAdditionalSearchRoot(int priority) {
+    result = this.(AddExpr).getAnOperand().(PathExpr).getAdditionalSearchRoot(priority)
   }
 }
 

javascript/ql/src/semmle/javascript/TypeScript.qll

@@ -213,7 +213,7 @@ class ExternalModuleReference extends Expr, Import, @externalmodulereference {
 }
 
 /** A literal path expression appearing in an external module reference. */
-private class LiteralExternalModulePath extends PathExprInModule, ConstantString {
+private class LiteralExternalModulePath extends PathExpr, ConstantString {
   LiteralExternalModulePath() {
     exists(ExternalModuleReference emr | this.getParentExpr*() = emr.getExpression())
   }

javascript/ql/src/semmle/javascript/dataflow/Nodes.qll

@@ -697,6 +697,7 @@ module ModuleImportNode {
  *
  * This predicate can be extended by subclassing `ModuleImportNode::Range`.
  */
+cached
 ModuleImportNode moduleImport(string path) { result.getPath() = path }
 
 /**

javascript/ql/src/semmle/javascript/frameworks/LazyCache.qll

@@ -74,7 +74,7 @@ module LazyCache {
   }
 
   /** A constant path element appearing in a call to a lazy-cache object. */
-  private class LazyCachePathExpr extends PathExprInModule, ConstantString {
+  private class LazyCachePathExpr extends PathExpr, ConstantString {
     LazyCachePathExpr() { this = any(LazyCacheImport rp).getArgument(0) }
 
     override string getValue() { result = getStringValue() }

javascript/ql/test/library-tests/Modules/tests.ql

@@ -22,7 +22,7 @@ query predicate test_ImportNamespaceSpecifier(ImportNamespaceSpecifier ins) { an
 
 query predicate test_ImportSpecifiers(ImportSpecifier is, VarDecl res) { res = is.getLocal() }
 
-query predicate test_Imports(ImportDeclaration id, PathExprInModule res0, int res1) {
+query predicate test_Imports(ImportDeclaration id, PathExpr res0, int res1) {
   res0 = id.getImportedPath() and res1 = count(id.getASpecifier())
 }