JS: Factor into parameterised module, show test failures

Author: asgerf

javascript/ql/lib/semmle/javascript/Modules.qll

@@ -6,7 +6,7 @@
 
 import javascript
 private import semmle.javascript.internal.CachedStages
-private import semmle.javascript.internal.paths.PathExprResolver as PathExprResolver
+private import semmle.javascript.internal.paths.PathExprResolver
 
 /**
  * A module, which may either be an ECMAScript 2015-style module,
@@ -115,6 +115,10 @@ abstract class Module extends TopLevel {
   }
 }
 
+private predicate shouldResolveExpr(Expr e) { e = any(Import imprt).getImportedPath() }
+
+private module Resolver = ResolveExpr<shouldResolveExpr/1>;
+
 /**
  * An import in a module, which may be an ECMAScript 2015-style
  * `import` statement, a CommonJS-style `require` import, or an AMD dependency.
@@ -139,9 +143,7 @@ abstract class Import extends AstNode {
   /**
    * Gets the module the path of this import resolves to.
    */
-  Module resolveImportedPath() {
-    result.getFile() = PathExprResolver::resolvePathExpr(this.getImportedPath())
-  }
+  Module resolveImportedPath() { result.getFile() = Resolver::resolveExpr(this.getImportedPath()) }
 
   /**
    * Gets a module with a `@providesModule` JSDoc tag that matches

javascript/ql/lib/semmle/javascript/internal/paths/PathExprResolver.qll

@@ -3,7 +3,6 @@ private import semmle.javascript.TSConfig
 private import semmle.javascript.internal.paths.PackageJsonEx
 private import semmle.javascript.internal.paths.JSPaths
 
-final private class FinalPathExpr = PathExpr;
 File getFileFromFolderImport(Folder folder) {
   result = folder.getJavaScriptFileOrTypings("index")
   or
@@ -15,10 +14,16 @@ File getFileFromFolderImport(Folder folder) {
   )
 }
 
+private signature predicate exprSig(Expr e);
 
-private class RelevantPathExpr extends FinalPathExpr {
-  pragma[nomagic]
-  RelevantPathExpr() { this = any(Import imprt).getImportedPath() }
+module ResolveExpr<exprSig/1 shouldResolveExpr> {
+
+private final class FinalExpr = Expr;
+
+private class RelevantPathExpr extends FinalExpr {
+  RelevantPathExpr() { shouldResolveExpr(this) }
+
+  string getValue() { result = this.getStringValue() }
 }
 
 /**
@@ -65,7 +70,7 @@ private string getPackagePrefixFromPathExpr(RelevantPathExpr expr) {
 private Variable dirname() { result.getName() = "__dirname" }
 
 /** Holds if `add` is a relevant path expression of form `__dirname + expr`. */
-private predicate prefixedByDirname(PathExpr expr) {
+private predicate prefixedByDirname(Expr expr) {
   expr = dirname().getAnAccess()
   or
   prefixedByDirname(expr.(AddExpr).getLeftOperand())
@@ -175,7 +180,7 @@ private Container resolvePathExpr1(RelevantPathExpr expr) {
   )
 }
 
-File resolvePathExpr(PathExpr expr) {
+File resolveExpr(RelevantPathExpr expr) {
   result = resolvePathExpr1(expr)
   or
   result = getFileFromFolderImport(resolvePathExpr1(expr))
@@ -206,7 +211,7 @@ module Debug {
 
   query Container resolvePathExpr1_(PathExprToDebug expr) { result = resolvePathExpr1(expr) }
 
-  query File resolvePathExpr_(PathExprToDebug expr) { result = resolvePathExpr(expr) }
+  query File resolveExpr_(PathExprToDebug expr) { result = resolveExpr(expr) }
 
   // Some predicates that are usually small enough that they don't need restriction
   query File getPackageMainFile(PackageJsonEx pkg) { result = pkg.getMainFile() }
@@ -217,3 +222,4 @@ module Debug {
 
   query predicate getFileFromFolderImport_ = getFileFromFolderImport/1;
 }
+}

javascript/ql/test/library-tests/PathResolution/DirnameImports/main.js

@@ -2,20 +2,20 @@
 const path = require('path');
 
 // Using __dirname directly
-const direct = require(__dirname + '/target.js'); // $ importTarget=DirnameImports/target.js
+const direct = require(__dirname + '/target.js'); // $ MISSING: importTarget=DirnameImports/target.js
 
 // Using __dirname with path.join
-const withPathJoin = require(path.join(__dirname, 'target.js')); // $ importTarget=DirnameImports/target.js
+const withPathJoin = require(path.join(__dirname, 'target.js')); // $ MISSING: importTarget=DirnameImports/target.js
 
 // Using __dirname with nested path
-const nested = require(__dirname + '/nested/target.js'); // $ importTarget=DirnameImports/nested/target.js
+const nested = require(__dirname + '/nested/target.js'); // $ MISSING: importTarget=DirnameImports/nested/target.js
 
 // Using __dirname with parent directory
-const parent = require(__dirname + '/../import-packages.ts'); // $ importTarget=import-packages.ts
+const parent = require(__dirname + '/../import-packages.ts'); // $ MISSING: importTarget=import-packages.ts
 
 // Using __dirname with path concat and variable
 const subdir = 'nested';
-const dynamic = require(__dirname + '/' + subdir + '/target.js'); // $ importTarget=DirnameImports/nested/target.js
+const dynamic = require(__dirname + '/' + subdir + '/target.js'); // $ MISSING: importTarget=DirnameImports/nested/target.js
 
 // Using __dirname in an AddExpr chain
-const chainedAdd = require(__dirname + '/' + 'target.js'); // $ importTarget=DirnameImports/target.js
+const chainedAdd = require(__dirname + '/' + 'target.js'); // $ MISSING: importTarget=DirnameImports/target.js

javascript/ql/test/library-tests/PathResolution/test.expected

@@ -40,12 +40,6 @@
 | DeclarationFiles/src/main.ts:5:1:5:27 | import  ... cript"; | DeclarationFiles/lib/typescript.ts |
 | DeclarationFiles/src/main.ts:6:1:6:30 | import  ... pt.js"; | DeclarationFiles/lib/typescript.ts |
 | DeclarationFiles/src/main.ts:7:1:7:32 | import  ... .d.ts"; | DeclarationFiles/lib/typescript.d.ts |
-| DirnameImports/main.js:5:16:5:48 | require ... et.js') | DirnameImports/target.js |
-| DirnameImports/main.js:8:22:8:63 | require ... t.js')) | DirnameImports/target.js |
-| DirnameImports/main.js:11:16:11:55 | require ... et.js') | DirnameImports/nested/target.js |
-| DirnameImports/main.js:14:16:14:60 | require ... es.ts') | import-packages.ts |
-| DirnameImports/main.js:18:17:18:64 | require ... et.js') | DirnameImports/nested/target.js |
-| DirnameImports/main.js:21:20:21:57 | require ... et.js') | DirnameImports/target.js |
 | Extended/src/main.ts:2:1:2:21 | import  ... /file"; | Extended/lib/file.ts |
 | Extended/src/main.ts:3:1:3:24 | import  ... le.ts"; | Extended/lib/file.ts |
 | Extended/src/main.ts:4:1:4:24 | import  ... le.js"; | Extended/lib/file.ts |