From cfe2475ff50559218fad0564d298a550101fa9c6 Mon Sep 17 00:00:00 2001
From: Jean <110341611+jean-michelet@users.noreply.github.com>
Date: Thu, 25 Apr 2024 14:04:45 +0200
Subject: [PATCH] Handle `ImportExpression` nodes (#261)

* feat: Analyse ImportExpression nodes

* test: add deeper test for entryFiles non .js extensions

* refactor: replace assert.strictEqual by assert.ok

* add empty line at end of file
---
 src/probes/isImportDeclaration.js          |  2 +-
 test/EntryFilesAnalyser.spec.js            | 43 ++++++++++++----------
 test/fixtures/entryFiles/deps/default.cjs  |  1 +
 test/fixtures/entryFiles/deps/default.jsx  |  3 +-
 test/fixtures/entryFiles/deps/default.mjs  |  1 +
 test/fixtures/entryFiles/deps/default.node |  2 +-
 test/fixtures/entryFiles/deps/dep.cjs      |  1 +
 test/fixtures/entryFiles/deps/dep.jsx      |  7 ++++
 test/fixtures/entryFiles/deps/dep.mjs      |  1 +
 test/fixtures/entryFiles/deps/dep.node     |  1 +
 test/probes/isImportDeclaration.spec.js    | 30 +++++++++++----
 11 files changed, 63 insertions(+), 29 deletions(-)
 create mode 100644 test/fixtures/entryFiles/deps/dep.cjs
 create mode 100644 test/fixtures/entryFiles/deps/dep.jsx
 create mode 100644 test/fixtures/entryFiles/deps/dep.mjs
 create mode 100644 test/fixtures/entryFiles/deps/dep.node

diff --git a/src/probes/isImportDeclaration.js b/src/probes/isImportDeclaration.js
index de381f2..3412fab 100644
--- a/src/probes/isImportDeclaration.js
+++ b/src/probes/isImportDeclaration.js
@@ -9,7 +9,7 @@
 function validateNode(node) {
   return [
     // Note: the source property is the right-side Literal part of the Import
-    node.type === "ImportDeclaration" && node.source.type === "Literal"
+    ["ImportDeclaration", "ImportExpression"].includes(node.type) && node.source.type === "Literal"
   ];
 }
 
diff --git a/test/EntryFilesAnalyser.spec.js b/test/EntryFilesAnalyser.spec.js
index 2308595..2f711bb 100644
--- a/test/EntryFilesAnalyser.spec.js
+++ b/test/EntryFilesAnalyser.spec.js
@@ -18,14 +18,14 @@ describe("EntryFilesAnalyser", () => {
     const generator = entryFilesAnalyser.analyse([entryUrl, deepEntryUrl]);
 
     // First entry
-    await assertReport(generator, entryUrl, true);
-    await assertReport(generator, new URL("deps/dep1.js", FIXTURE_URL), true);
-    await assertReport(generator, new URL("shared.js", FIXTURE_URL), true);
-    await assertReport(generator, new URL("deps/dep2.js", FIXTURE_URL), true);
+    await assertReport(generator, entryUrl);
+    await assertReport(generator, new URL("deps/dep1.js", FIXTURE_URL));
+    await assertReport(generator, new URL("shared.js", FIXTURE_URL));
+    await assertReport(generator, new URL("deps/dep2.js", FIXTURE_URL));
 
     // Second entry
-    await assertReport(generator, deepEntryUrl, true);
-    await assertReport(generator, new URL("deps/dep3.js", FIXTURE_URL), true);
+    await assertReport(generator, deepEntryUrl);
+    await assertReport(generator, new URL("deps/dep3.js", FIXTURE_URL));
 
     await assertAllReportsYielded(generator);
 
@@ -40,15 +40,15 @@ describe("EntryFilesAnalyser", () => {
 
     const generator = entryFilesAnalyser.analyse([entryUrl]);
 
-    await assertReport(generator, entryUrl, true);
+    await assertReport(generator, entryUrl);
 
     const invalidDepReport = await generator.next();
     assert.ok(!invalidDepReport.value.ok);
     assert.strictEqual(invalidDepReport.value.url, new URL("deps/invalidDep.js", FIXTURE_URL).pathname);
     assert.strictEqual(invalidDepReport.value.warnings[0].kind, "parsing-error");
 
-    await assertReport(generator, new URL("deps/dep1.js", FIXTURE_URL), true);
-    await assertReport(generator, new URL("shared.js", FIXTURE_URL), true);
+    await assertReport(generator, new URL("deps/dep1.js", FIXTURE_URL));
+    await assertReport(generator, new URL("shared.js", FIXTURE_URL));
 
     await assertAllReportsYielded(generator);
   });
@@ -60,12 +60,16 @@ describe("EntryFilesAnalyser", () => {
     const entryUrl = new URL("entryWithVariousDepExtensions.js", FIXTURE_URL);
     const generator = entryFilesAnalyser.analyse([entryUrl]);
 
-    await assertReport(generator, entryUrl, true);
-    await assertReport(generator, new URL("deps/default.js", FIXTURE_URL), true);
-    await assertReport(generator, new URL("deps/default.cjs", FIXTURE_URL), true);
-    await assertReport(generator, new URL("deps/default.mjs", FIXTURE_URL), true);
-    await assertReport(generator, new URL("deps/default.node", FIXTURE_URL), true);
-    await assertReport(generator, new URL("deps/default.jsx", FIXTURE_URL), true);
+    await assertReport(generator, entryUrl);
+    await assertReport(generator, new URL("deps/default.js", FIXTURE_URL));
+    await assertReport(generator, new URL("deps/default.cjs", FIXTURE_URL));
+    await assertReport(generator, new URL("deps/dep.cjs", FIXTURE_URL));
+    await assertReport(generator, new URL("deps/default.mjs", FIXTURE_URL));
+    await assertReport(generator, new URL("deps/dep.mjs", FIXTURE_URL));
+    await assertReport(generator, new URL("deps/default.node", FIXTURE_URL));
+    await assertReport(generator, new URL("deps/dep.node", FIXTURE_URL));
+    await assertReport(generator, new URL("deps/default.jsx", FIXTURE_URL));
+    await assertReport(generator, new URL("deps/dep.jsx", FIXTURE_URL));
 
     await assertAllReportsYielded(generator);
   });
@@ -77,16 +81,17 @@ describe("EntryFilesAnalyser", () => {
     const entryUrl = new URL("entryWithVariousDepExtensions.js", FIXTURE_URL);
     const generator = entryFilesAnalyser.analyse([entryUrl]);
 
-    await assertReport(generator, entryUrl, true);
-    await assertReport(generator, new URL("deps/default.jsx", FIXTURE_URL), true);
+    await assertReport(generator, entryUrl);
+    await assertReport(generator, new URL("deps/default.jsx", FIXTURE_URL));
+    await assertReport(generator, new URL("deps/dep.jsx", FIXTURE_URL));
 
     await assertAllReportsYielded(generator);
   });
 
-  async function assertReport(generator, expectedUrl, expectedOk) {
+  async function assertReport(generator, expectedUrl) {
     const report = await generator.next();
     assert.strictEqual(report.value.url, expectedUrl.pathname);
-    assert.strictEqual(report.value.ok, expectedOk);
+    assert.ok(report.value.ok);
   }
 
   async function assertAllReportsYielded(generator) {
diff --git a/test/fixtures/entryFiles/deps/default.cjs b/test/fixtures/entryFiles/deps/default.cjs
index 7c72f39..ac8f024 100644
--- a/test/fixtures/entryFiles/deps/default.cjs
+++ b/test/fixtures/entryFiles/deps/default.cjs
@@ -1 +1,2 @@
 require('externalDep')
+require('dep.cjs')
diff --git a/test/fixtures/entryFiles/deps/default.jsx b/test/fixtures/entryFiles/deps/default.jsx
index 607e7bd..06faa68 100644
--- a/test/fixtures/entryFiles/deps/default.jsx
+++ b/test/fixtures/entryFiles/deps/default.jsx
@@ -1,7 +1,8 @@
 import React from 'react'
+import Bar from './dep.jsx'
 
 export default function Foo() {
   return (
-    <div></div>
+    <Bar />
   )
 }
diff --git a/test/fixtures/entryFiles/deps/default.mjs b/test/fixtures/entryFiles/deps/default.mjs
index a2a416a..24476d0 100644
--- a/test/fixtures/entryFiles/deps/default.mjs
+++ b/test/fixtures/entryFiles/deps/default.mjs
@@ -1 +1,2 @@
 import externalDep from 'externalDep';
+import('dep.mjs')
diff --git a/test/fixtures/entryFiles/deps/default.node b/test/fixtures/entryFiles/deps/default.node
index 2138e72..edfa69c 100644
--- a/test/fixtures/entryFiles/deps/default.node
+++ b/test/fixtures/entryFiles/deps/default.node
@@ -1 +1 @@
-module.exports = require('some/addon');
+module.exports = require('dep.node');
diff --git a/test/fixtures/entryFiles/deps/dep.cjs b/test/fixtures/entryFiles/deps/dep.cjs
new file mode 100644
index 0000000..4ba52ba
--- /dev/null
+++ b/test/fixtures/entryFiles/deps/dep.cjs
@@ -0,0 +1 @@
+module.exports = {}
diff --git a/test/fixtures/entryFiles/deps/dep.jsx b/test/fixtures/entryFiles/deps/dep.jsx
new file mode 100644
index 0000000..a7600ff
--- /dev/null
+++ b/test/fixtures/entryFiles/deps/dep.jsx
@@ -0,0 +1,7 @@
+import React from 'react'
+
+export default function Bar() {
+  return (
+    <div></div>
+  )
+}
diff --git a/test/fixtures/entryFiles/deps/dep.mjs b/test/fixtures/entryFiles/deps/dep.mjs
new file mode 100644
index 0000000..336ce12
--- /dev/null
+++ b/test/fixtures/entryFiles/deps/dep.mjs
@@ -0,0 +1 @@
+export {}
diff --git a/test/fixtures/entryFiles/deps/dep.node b/test/fixtures/entryFiles/deps/dep.node
new file mode 100644
index 0000000..2138e72
--- /dev/null
+++ b/test/fixtures/entryFiles/deps/dep.node
@@ -0,0 +1 @@
+module.exports = require('some/addon');
diff --git a/test/probes/isImportDeclaration.spec.js b/test/probes/isImportDeclaration.spec.js
index e2bcf75..4758ef6 100644
--- a/test/probes/isImportDeclaration.spec.js
+++ b/test/probes/isImportDeclaration.spec.js
@@ -46,16 +46,32 @@ test("should detect 1 dependency with no specificiers", () => {
   assert.ok(dependencies.has("bar"));
 });
 
+test("should detect 1 dependency for an ImportExpression", () => {
+  const str = "import(\"bar\")";
+  const ast = parseScript(str);
+  const { sourceFile } = getSastAnalysis(str, isImportDeclaration)
+    .execute(ast.body);
+
+  const { dependencies } = sourceFile;
+  assert.ok(dependencies.has("bar"));
+});
+
 test("should detect an unsafe import using data:text/javascript and throw a unsafe-import warning", () => {
   const expectedValue = "data:text/javascript;base64,Y29uc29sZS5sb2coJ2hlbGxvIHdvcmxkJyk7Cg==";
-  const str = `import '${expectedValue}';`;
 
-  const ast = parseScript(str);
-  const sastAnalysis = getSastAnalysis(str, isImportDeclaration)
-    .execute(ast.body);
+  const importNodes = [
+    `import '${expectedValue}';`,
+    `import('${expectedValue}');`
+  ];
+
+  importNodes.forEach((str) => {
+    const ast = parseScript(str);
+    const sastAnalysis = getSastAnalysis(str, isImportDeclaration)
+      .execute(ast.body);
 
-  assert.strictEqual(sastAnalysis.warnings().length, 1);
+    assert.strictEqual(sastAnalysis.warnings().length, 1);
 
-  const unsafeImport = sastAnalysis.getWarning("unsafe-import");
-  assert.strictEqual(unsafeImport.value, expectedValue);
+    const unsafeImport = sastAnalysis.getWarning("unsafe-import");
+    assert.strictEqual(unsafeImport.value, expectedValue);
+  });
 });