[New] no-unused-modules: support dynamic imports

All occurences of `import('...')` are treated as namespace imports (`import * as X from '...'`) Co-authored-by: Max Komarychev <[email protected]> Co-authored-by: Filipp Riabchun <[email protected]> Co-authored-by: 薛定谔的猫 <[email protected]>
import-js · Jun 20, 2020 · 4f338bf · 4f338bf
1 parent 513bb0b
commit 4f338bf
Show file tree

Hide file tree

Showing 15 changed files with 348 additions and 11 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -16,6 +16,7 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel
 - [`named`]: add `commonjs` option ([#1222], thanks [@vikr01])
 - [`no-namespace`]: Add `ignore` option ([#2112], thanks [@aberezkin])
 - [`max-dependencies`]: add option `ignoreTypeImports` ([#1847], thanks [@rfermann])
+- [`no-unused-modules`]: support dynamic imports ([#1660], thanks [@maxkomarychev])
 
 ### Fixed
 - [`no-duplicates`]: ensure autofix avoids excessive newlines ([#2028], thanks [@ertrzyiks])
@@ -959,6 +960,7 @@ for info on changes for earlier releases.
 [#1676]: https://github.com/import-js/eslint-plugin-import/pull/1676
 [#1666]: https://github.com/import-js/eslint-plugin-import/pull/1666
 [#1664]: https://github.com/import-js/eslint-plugin-import/pull/1664
+[#1660]: https://github.com/import-js/eslint-plugin-import/pull/1660
 [#1658]: https://github.com/import-js/eslint-plugin-import/pull/1658
 [#1651]: https://github.com/import-js/eslint-plugin-import/pull/1651
 [#1626]: https://github.com/import-js/eslint-plugin-import/pull/1626
@@ -1454,6 +1456,7 @@ for info on changes for earlier releases.
 [@MatthiasKunnen]: https://github.com/MatthiasKunnen
 [@mattijsbliek]: https://github.com/mattijsbliek
 [@Maxim-Mazurok]: https://github.com/Maxim-Mazurok
+[@maxkomarychev]: https://github.com/maxkomarychev
 [@maxmalov]: https://github.com/maxmalov
 [@MikeyBeLike]: https://github.com/MikeyBeLike
 [@mplewis]: https://github.com/mplewis

diff --git a/docs/rules/no-unused-modules.md b/docs/rules/no-unused-modules.md
@@ -3,8 +3,8 @@
 Reports:
   - modules without any exports
   - individual exports not being statically `import`ed or `require`ed from other modules in the same project
+  - dynamic imports are supported if argument is a literal string
 
-Note: dynamic imports are currently not supported.
 
 ## Rule Details
 

diff --git a/src/ExportMap.js b/src/ExportMap.js
@@ -7,6 +7,7 @@ import debug from 'debug';
 import { SourceCode } from 'eslint';
 
 import parse from 'eslint-module-utils/parse';
+import visit from 'eslint-module-utils/visit';
 import resolve from 'eslint-module-utils/resolve';
 import isIgnored, { hasValidExtension } from 'eslint-module-utils/ignore';
 
@@ -354,15 +355,55 @@ ExportMap.parse = function (path, content, context) {
   const isEsModuleInteropTrue = isEsModuleInterop();
 
   let ast;
+  let visitorKeys;
   try {
-    ast = parse(path, content, context);
+    ({ ast, visitorKeys } = parse(path, content, context));
   } catch (err) {
-    log('parse error:', path, err);
     m.errors.push(err);
     return m; // can't continue
   }
 
-  if (!unambiguous.isModule(ast)) return null;
+  m.visitorKeys = visitorKeys;
+
+  let hasDynamicImports = false;
+
+  function processDynamicImport(source) {
+    hasDynamicImports = true;
+    if (source.type !== 'Literal') {
+      return null;
+    }
+    const p = remotePath(source.value);
+    if (p == null) {
+      return null;
+    }
+    const importedSpecifiers = new Set();
+    importedSpecifiers.add('ImportNamespaceSpecifier');
+    const getter = thunkFor(p, context);
+    m.imports.set(p, {
+      getter,
+      declarations: new Set([{
+        source: {
+        // capturing actual node reference holds full AST in memory!
+          value: source.value,
+          loc: source.loc,
+        },
+        importedSpecifiers,
+      }]),
+    });
+  }
+
+  visit(ast, visitorKeys, {
+    ImportExpression(node) {
+      processDynamicImport(node.source);
+    },
+    CallExpression(node) {
+      if (node.callee.type === 'Import') {
+        processDynamicImport(node.arguments[0]);
+      }
+    },
+  });
+
+  if (!unambiguous.isModule(ast) && !hasDynamicImports) return null;
 
   const docstyle = (context.settings && context.settings['import/docstyle']) || ['jsdoc'];
   const docStyleParsers = {};

diff --git a/src/rules/no-unused-modules.js b/src/rules/no-unused-modules.js
@@ -7,6 +7,7 @@
 import Exports, { recursivePatternCapture } from '../ExportMap';
 import { getFileExtensions } from 'eslint-module-utils/ignore';
 import resolve from 'eslint-module-utils/resolve';
+import visit from 'eslint-module-utils/visit';
 import docsUrl from '../docsUrl';
 import { dirname, join } from 'path';
 import readPkgUp from 'read-pkg-up';
@@ -144,6 +145,8 @@ const importList = new Map();
  */
 const exportList = new Map();
 
+const visitorKeyMap = new Map();
+
 const ignoredFiles = new Set();
 const filesOutsideSrc = new Set();
 
@@ -183,8 +186,15 @@ const prepareImportsAndExports = (srcFiles, context) => {
     const imports = new Map();
     const currentExports = Exports.get(file, context);
     if (currentExports) {
-      const { dependencies, reexports, imports: localImportList, namespace  } = currentExports;
-
+      const {
+        dependencies,
+        reexports,
+        imports: localImportList,
+        namespace,
+        visitorKeys,
+      } = currentExports;
+
+      visitorKeyMap.set(file, visitorKeys);
       // dependencies === export * from
       const currentExportAll = new Set();
       dependencies.forEach(getDependency => {
@@ -665,6 +675,28 @@ module.exports = {
         });
       });
 
+      function processDynamicImport(source) {
+        if (source.type !== 'Literal') {
+          return null;
+        }
+        const p = resolve(source.value, context);
+        if (p == null) {
+          return null;
+        }
+        newNamespaceImports.add(p);
+      }
+
+      visit(node, visitorKeyMap.get(file), {
+        ImportExpression(child) {
+          processDynamicImport(child.source);
+        },
+        CallExpression(child) {
+          if (child.callee.type === 'Import') {
+            processDynamicImport(child.arguments[0]);
+          }
+        },
+      });
+
       node.body.forEach(astNode => {
         let resolvedPath;
 

diff --git a/tests/files/no-unused-modules/dynamic-import-js-2.js b/tests/files/no-unused-modules/dynamic-import-js-2.js
@@ -0,0 +1,13 @@
+const importPath = './exports-for-dynamic-js';
+class A {
+    method() {
+        const c = import(importPath)
+    }
+}
+
+
+class B {
+    method() {
+        const c = import('i-do-not-exist')
+    }
+}
diff --git a/tests/files/no-unused-modules/dynamic-import-js.js b/tests/files/no-unused-modules/dynamic-import-js.js
@@ -0,0 +1,5 @@
+class A {
+    method() {
+        const c = import('./exports-for-dynamic-js')
+    }
+}
diff --git a/tests/files/no-unused-modules/exports-for-dynamic-js-2.js b/tests/files/no-unused-modules/exports-for-dynamic-js-2.js
@@ -0,0 +1,5 @@
+export const a = 10;
+export const b = 20;
+export const c = 30;
+const d = 40;
+export default d;
diff --git a/tests/files/no-unused-modules/exports-for-dynamic-js.js b/tests/files/no-unused-modules/exports-for-dynamic-js.js
@@ -0,0 +1,5 @@
+export const a = 10
+export const b = 20
+export const c = 30
+const d = 40
+export default d
diff --git a/tests/files/no-unused-modules/typescript/dynamic-import-ts.ts b/tests/files/no-unused-modules/typescript/dynamic-import-ts.ts
@@ -0,0 +1,6 @@
+class A {
+    method() {
+        const c = import('./exports-for-dynamic-ts')
+    }
+}
+
diff --git a/tests/files/no-unused-modules/typescript/exports-for-dynamic-ts.ts b/tests/files/no-unused-modules/typescript/exports-for-dynamic-ts.ts
@@ -0,0 +1,5 @@
+export const ts_a = 10
+export const ts_b = 20
+export const ts_c = 30
+const ts_d = 40
+export default ts_d
diff --git a/tests/src/rules/no-unused-modules.js b/tests/src/rules/no-unused-modules.js
@@ -148,6 +148,59 @@ ruleTester.run('no-unused-modules', rule, {
       code: 'const o0 = 0; const o1 = 1; export { o0, o1 as o2 }; export default () => {}',
       filename: testFilePath('./no-unused-modules/file-o.js'),
     }),
+    test({
+      options: unusedExportsOptions,
+      code: 'import { o2 } from "./file-o";export default () => 12',
+      filename: testFilePath('./no-unused-modules/file-a.js'),
+      parser: require.resolve('babel-eslint'),
+    }),
+    test({
+      options: unusedExportsOptions,
+      code: 'export const b = 2',
+      filename: testFilePath('./no-unused-modules/file-b.js'),
+      parser: require.resolve('babel-eslint'),
+    }),
+    test({
+      options: unusedExportsOptions,
+      code: 'const c1 = 3; function c2() { return 3 }; export { c1, c2 }',
+      filename: testFilePath('./no-unused-modules/file-c.js'),
+      parser: require.resolve('babel-eslint'),
+    }),
+    test({
+      options: unusedExportsOptions,
+      code: 'export function d() { return 4 }',
+      filename: testFilePath('./no-unused-modules/file-d.js'),
+      parser: require.resolve('babel-eslint'),
+    }),
+    test({
+      options: unusedExportsOptions,
+      code: 'export class q { q0() {} }',
+      filename: testFilePath('./no-unused-modules/file-q.js'),
+      parser: require.resolve('babel-eslint'),
+    }),
+    test({
+      options: unusedExportsOptions,
+      code: 'const e0 = 5; export { e0 as e }',
+      filename: testFilePath('./no-unused-modules/file-e.js'),
+      parser: require.resolve('babel-eslint'),
+    }),
+    test({
+      options: unusedExportsOptions,
+      code: 'const l0 = 5; const l = 10; export { l0 as l1, l }; export default () => {}',
+      filename: testFilePath('./no-unused-modules/file-l.js'),
+      parser: require.resolve('babel-eslint'),
+    }),
+    test({
+      options: unusedExportsOptions,
+      code: 'const o0 = 0; const o1 = 1; export { o0, o1 as o2 }; export default () => {}',
+      filename: testFilePath('./no-unused-modules/file-o.js'),
+      parser: require.resolve('babel-eslint'),
+    }),
+    test({ options: unusedExportsOptions,
+      code: 'export class q { q0() {} }',
+      filename: testFilePath('./no-unused-modules/file-q.js'),
+      parser: require.resolve('babel-eslint'),
+    }),
   ],
   invalid: [
     test({
@@ -234,6 +287,89 @@ ruleTester.run('no-unused-modules', rule, {
   ],
 });
 
+// test for export from
+ruleTester.run('no-unused-modules', rule, {
+  valid: [
+    test({
+      options: unusedExportsOptions,
+      code: `
+        export const a = 10
+        export const b = 20
+        export const c = 30
+        const d = 40
+        export default d
+      `,
+      parser: require.resolve('babel-eslint'),
+      filename: testFilePath('./no-unused-modules/exports-for-dynamic-js.js'),
+    }),
+  ],
+  invalid: [
+    test({
+      options: unusedExportsOptions,
+      code: `
+        export const a = 10
+        export const b = 20
+        export const c = 30
+        const d = 40
+        export default d
+      `,
+      parser: require.resolve('babel-eslint'),
+      filename: testFilePath('./no-unused-modules/exports-for-dynamic-js-2.js'),
+      errors: [
+        error(`exported declaration 'a' not used within other modules`),
+        error(`exported declaration 'b' not used within other modules`),
+        error(`exported declaration 'c' not used within other modules`),
+        error(`exported declaration 'default' not used within other modules`),
+      ],
+    }),
+  ],
+});
+
+describe('dynamic imports', () => {
+  if (semver.satisfies(eslintPkg.version, '< 6')) {
+    this.skip();
+    return;
+  }
+
+  // test for unused exports with `import()`
+  ruleTester.run('no-unused-modules', rule, {
+    valid: [
+      test({
+        options: unusedExportsOptions,
+        code: `
+          export const a = 10
+          export const b = 20
+          export const c = 30
+          const d = 40
+          export default d
+        `,
+        parser: require.resolve('babel-eslint'),
+        filename: testFilePath('./no-unused-modules/exports-for-dynamic-js.js'),
+      }),
+    ],
+    invalid: [
+      test({
+        options: unusedExportsOptions,
+        code: `
+          export const a = 10
+          export const b = 20
+          export const c = 30
+          const d = 40
+          export default d
+        `,
+        parser: require.resolve('babel-eslint'),
+        filename: testFilePath('./no-unused-modules/exports-for-dynamic-js-2.js'),
+        errors: [
+          error(`exported declaration 'a' not used within other modules`),
+          error(`exported declaration 'b' not used within other modules`),
+          error(`exported declaration 'c' not used within other modules`),
+          error(`exported declaration 'default' not used within other modules`),
+        ],
+      }),
+    ],
+  });
+});
+
 // test for export from
 ruleTester.run('no-unused-modules', rule, {
   valid: [
@@ -951,6 +1087,18 @@ context('TypeScript', function () {
   getTSParsers().forEach((parser) => {
     typescriptRuleTester.run('no-unused-modules', rule, {
       valid: [].concat(
+        test({
+          options: unusedExportsTypescriptOptions,
+          code: `
+            export const ts_a = 10;
+            export const ts_b = 20;
+            export const ts_c = 30;
+            const ts_d = 40;
+            export default ts_d;
+          `,
+          parser: require.resolve('@typescript-eslint/parser'),
+          filename: testFilePath('./no-unused-modules/typescript/exports-for-dynamic-ts.ts'),
+        }),
         test({
           options: unusedExportsTypescriptOptions,
           code: `