From 7c382f02a6cafe676ee754a896092040dad8daf6 Mon Sep 17 00:00:00 2001 From: Max Komarychev Date: Sat, 20 Jun 2020 15:10:40 +0300 Subject: [PATCH] [New] `no-unused-modules`: support dynamic imports MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All occurences of `import('...')` are treated as namespace imports (`import * as X from '...'`) See #1660, #2212. Co-authored-by: Max Komarychev Co-authored-by: Filipp Riabchun Co-authored-by: 薛定谔的猫 --- CHANGELOG.md | 4 + docs/rules/no-unused-modules.md | 2 +- src/ExportMap.js | 49 +++++++++++- src/rules/no-unused-modules.js | 36 ++++++++- .../no-unused-modules/dynamic-import-js-2.js | 13 ++++ .../no-unused-modules/dynamic-import-js.js | 5 ++ .../exports-for-dynamic-js-2.js | 5 ++ .../exports-for-dynamic-js.js | 5 ++ .../typescript/dynamic-import-ts.ts | 6 ++ .../typescript/exports-for-dynamic-ts.ts | 5 ++ tests/src/rules/no-unused-modules.js | 75 ++++++++++++++++++- 11 files changed, 198 insertions(+), 7 deletions(-) create mode 100644 tests/files/no-unused-modules/dynamic-import-js-2.js create mode 100644 tests/files/no-unused-modules/dynamic-import-js.js create mode 100644 tests/files/no-unused-modules/exports-for-dynamic-js-2.js create mode 100644 tests/files/no-unused-modules/exports-for-dynamic-js.js create mode 100644 tests/files/no-unused-modules/typescript/dynamic-import-ts.ts create mode 100644 tests/files/no-unused-modules/typescript/exports-for-dynamic-ts.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 426b1114a..9203739fd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel - [`no-unresolved`]: add `caseSensitiveStrict` option ([#1262], thanks [@sergei-startsev]) - [`no-unused-modules`]: add eslint v8 support ([#2194], thanks [@coderaiser]) - [`no-restricted-paths`]: add/restore glob pattern support ([#2219], thanks [@stropho]) +- [`no-unused-modules`]: support dynamic imports ([#1660], [#2212], thanks [@maxkomarychev], [@aladdin-add], [@Hypnosphi]) ## [2.24.2] - 2021-08-24 @@ -906,6 +907,7 @@ for info on changes for earlier releases. [`memo-parser`]: ./memo-parser/README.md [#2219]: https://github.com/import-js/eslint-plugin-import/pull/2219 +[#2212]: https://github.com/import-js/eslint-plugin-import/pull/2212 [#2196]: https://github.com/import-js/eslint-plugin-import/pull/2196 [#2194]: https://github.com/import-js/eslint-plugin-import/pull/2194 [#2184]: https://github.com/import-js/eslint-plugin-import/pull/2184 @@ -983,6 +985,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 @@ -1487,6 +1490,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 index 01c13557c..0bd805612 100644 --- 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 index 5bda83dd3..53091e466 100644 --- 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,57 @@ ExportMap.parse = function (path, content, context) { const isEsModuleInteropTrue = isEsModuleInterop(); let ast; + let visitorKeys; try { - ast = parse(path, content, context); + const result = parse(path, content, context); + ast = result.ast; + visitorKeys = result.visitorKeys; } 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 index ae6646f87..ab347bd8c 100644 --- 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'; @@ -154,6 +155,8 @@ const importList = new Map(); */ const exportList = new Map(); +const visitorKeyMap = new Map(); + const ignoredFiles = new Set(); const filesOutsideSrc = new Set(); @@ -193,8 +196,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 => { @@ -675,6 +685,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 new file mode 100644 index 000000000..3de28a65d --- /dev/null +++ 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 new file mode 100644 index 000000000..36bf5c313 --- /dev/null +++ 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 new file mode 100644 index 000000000..19082862f --- /dev/null +++ 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 new file mode 100644 index 000000000..06d938e40 --- /dev/null +++ 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 new file mode 100644 index 000000000..10a17c3b1 --- /dev/null +++ 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 new file mode 100644 index 000000000..566eb7c7d --- /dev/null +++ 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 index 837f7e575..6f87058c4 100644 --- a/tests/src/rules/no-unused-modules.js +++ b/tests/src/rules/no-unused-modules.js @@ -112,41 +112,49 @@ ruleTester.run('no-unused-modules', rule, { 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'), }), ], invalid: [ @@ -234,7 +242,72 @@ ruleTester.run('no-unused-modules', rule, { ], }); -// test for export from + +describe('dynamic imports', () => { + if (semver.satisfies(eslintPkg.version, '< 6')) { + beforeEach(function () { + 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`), + ] }), + ], + }); + typescriptRuleTester.run('no-unused-modules', rule, { + valid: [ + 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'), + }), + ], + invalid: [ + ], + }); +}); + +// // test for export from ruleTester.run('no-unused-modules', rule, { valid: [ test({