fix: avoid dynamic paths provided to require imports

antoine-coulon · Oct 25, 2022 · 82d3ee6 · 82d3ee6
1 parent a802fb9
commit 82d3ee6
Show file tree

Hide file tree

Showing 4 changed files with 55 additions and 26 deletions.
diff --git a/packages/skott/src/modules/walkers/ecmascript/javascript/walker.ts b/packages/skott/src/modules/walkers/ecmascript/javascript/walker.ts
@@ -1,9 +1,7 @@
 import { walk } from "estree-walker";
 
 import { ModuleWalker, ModuleWalkerResult } from "../../common.js";
-import { isEcmaScriptModuleDeclaration } from "../module-declaration.js";
-
-import { isCommonJSModuleImport } from "./cjs.js";
+import { extractModuleDeclarations } from "../module-declaration.js";
 
 export class JavaScriptModuleWalker implements ModuleWalker {
   public async walk(fileContent: string): Promise<ModuleWalkerResult> {
@@ -19,17 +17,7 @@ export class JavaScriptModuleWalker implements ModuleWalker {
 
     walk(isRootNode ? node.body : node, {
       enter(node) {
-        if (isCommonJSModuleImport(node)) {
-          moduleDeclarations.add(node.arguments[0].value);
-        }
-
-        if (isEcmaScriptModuleDeclaration(node)) {
-          moduleDeclarations.add(node.source.value);
-        }
-
-        if (node.type === "ImportExpression") {
-          moduleDeclarations.add(node.source.value);
-        }
+        extractModuleDeclarations(node, moduleDeclarations);
       }
     });
 

diff --git a/packages/skott/src/modules/walkers/ecmascript/module-declaration.ts b/packages/skott/src/modules/walkers/ecmascript/module-declaration.ts
@@ -1,3 +1,5 @@
+import { isCommonJSModuleImport } from "./javascript/cjs.js";
+
 /**
  * Searching for named exports with no local variable binding such as
  * export { foo } from "./foo.js" as this export from another file is creating
@@ -29,8 +31,33 @@ function isEcmaScriptModuleImport(estreeNode: any): boolean {
   return estreeNode.type === "ImportDeclaration";
 }
 
-export function isEcmaScriptModuleDeclaration(estreeNode: any): boolean {
+function isEcmaScriptModuleDeclaration(estreeNode: any): boolean {
   return (
     isEcmaScriptModuleImport(estreeNode) || isEcmaScriptModuleExport(estreeNode)
   );
 }
+
+export function extractModuleDeclarations(
+  node: any,
+  moduleDeclarations: Set<string>
+): void {
+  if (isCommonJSModuleImport(node)) {
+    /**
+     * Just trying to track static paths from dynamic require statements such as
+     * `require('./index.js')`. Consequently every "require(someVar)" will be
+     * discarded.
+     */
+    const isStaticPath = node.arguments[0].value;
+    if (isStaticPath) {
+      moduleDeclarations.add(isStaticPath);
+    }
+  }
+
+  if (isEcmaScriptModuleDeclaration(node)) {
+    moduleDeclarations.add(node.source.value);
+  }
+
+  if (node.type === "ImportExpression") {
+    moduleDeclarations.add(node.source.value);
+  }
+}
diff --git a/packages/skott/src/modules/walkers/ecmascript/typescript/walker.ts b/packages/skott/src/modules/walkers/ecmascript/typescript/walker.ts
@@ -1,8 +1,7 @@
 import { walk } from "estree-walker";
 
 import { ModuleWalker, ModuleWalkerResult } from "../../common.js";
-import { isCommonJSModuleImport } from "../javascript/cjs.js";
-import { isEcmaScriptModuleDeclaration } from "../module-declaration.js";
+import { extractModuleDeclarations } from "../module-declaration.js";
 
 async function tryOrElse(
   tryFn: () => void,
@@ -35,15 +34,7 @@ export class TypeScriptModuleWalker implements ModuleWalker {
 
       walk(isRootNode ? node.body : node, {
         enter(node) {
-          if (isCommonJSModuleImport(node)) {
-            moduleDeclarations.add(node.arguments[0].value);
-          }
-          if (isEcmaScriptModuleDeclaration(node)) {
-            moduleDeclarations.add(node.source.value);
-          }
-          if (node.type === "ImportExpression") {
-            moduleDeclarations.add(node.source.value);
-          }
+          extractModuleDeclarations(node, moduleDeclarations);
         }
       });
     }

diff --git a/packages/skott/test/ecmascript/agnostic.spec.ts b/packages/skott/test/ecmascript/agnostic.spec.ts
@@ -269,6 +269,29 @@ describe("When dealing with ECMAScript standards agnostic of TypeScript and Java
   });
 });
 
+describe("When extracting CommonJS dynamic import declarations using variables", () => {
+  it("should ignore the require statement as we can't resolve statically the file", async () => {
+    mountFakeFileSystem({
+      "index.js": `
+        const a = "./something.js";
+      
+        require(a);
+      `,
+      "index.ts": `
+        const a = "./something.js";
+      
+        require(a);
+      `
+    });
+
+    const { files } = await buildSkottProjectUsingInMemoryFileExplorer({
+      includeBaseDir: false
+    });
+
+    expect(files).to.be.deep.equal(["index.js", "index.ts"]);
+  });
+});
+
 describe("When a global analysis without any entrypoint is requested", () => {
   it("should collect all files at the root directory level", async () => {
     mountFakeFileSystem({