diff --git a/CHANGELOG.md b/CHANGELOG.md index a6d4d6b84a..463eb6fcdc 100644 --- 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 @@ -1427,8 +1429,8 @@ for info on changes for earlier releases. [@kiwka]: https://github.com/kiwka [@klimashkin]: https://github.com/klimashkin [@kmui2]: https://github.com/kmui2 -[@KostyaZgara]: https://github.com/KostyaZgara [@knpwrs]: https://github.com/knpwrs +[@KostyaZgara]: https://github.com/KostyaZgara [@laysent]: https://github.com/laysent [@le0nik]: https://github.com/le0nik [@lemonmade]: https://github.com/lemonmade @@ -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 index 01c13557cf..0bd805612b 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 46d3cfe88c..563741ae39 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,46 @@ 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; + let hasDynamicImports = false; + + visit(ast, visitorKeys, { + CallExpression(node) { + if (node.callee.type === 'Import') { + hasDynamicImports = true; + const firstArgument = node.arguments[0]; + if (firstArgument.type !== 'Literal') { + return null; + } + const p = remotePath(firstArgument.value); + if (p == null) { + return null; + } + const importedSpecifiers = new Set(); + importedSpecifiers.add('ImportNamespaceSpecifier'); + const getter = thunkFor(p, context); + m.imports.set(p, { + getter, + source: { + // capturing actual node reference holds full AST in memory! + value: firstArgument.value, + loc: firstArgument.loc, + }, + importedSpecifiers, + }); + } + }, + }); + + if (!unambiguous.isModule(ast) && !hasDynamicImports) return null; const docstyle = (context.settings && context.settings['import/docstyle']) || ['jsdoc']; const docStyleParsers = {}; 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 0000000000..3de28a65d5 --- /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 0000000000..36bf5c313c --- /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 0000000000..19082862fc --- /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 0000000000..06d938e40b --- /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 0000000000..10a17c3b19 --- /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 0000000000..566eb7c7d4 --- /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 2e7d09b3da..7feef9fd75 100644 --- a/tests/src/rules/no-unused-modules.js +++ b/tests/src/rules/no-unused-modules.js @@ -4,8 +4,8 @@ import typescriptConfig from '../../../config/typescript'; import { RuleTester } from 'eslint'; import fs from 'fs'; -import semver from 'semver'; import eslintPkg from 'eslint/package.json'; +import semver from 'semver'; // TODO: figure out why these tests fail in eslint 4 const isESLint4TODO = semver.satisfies(eslintPkg.version, '^4'); @@ -108,7 +108,6 @@ ruleTester.run('no-unused-modules', rule, { // tests for exports ruleTester.run('no-unused-modules', rule, { valid: [ - test({ options: unusedExportsOptions, code: 'import { o2 } from "./file-o";export default () => 12', @@ -149,6 +148,54 @@ 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'), + }), ], invalid: [ test({ @@ -235,7 +282,123 @@ ruleTester.run('no-unused-modules', rule, { ], }); -// // test for export from +// 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: [ + ], +}); + +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`), + ], + }), + ], + }); + 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({ diff --git a/utils/CHANGELOG.md b/utils/CHANGELOG.md index 9160055145..070a3bc596 100644 --- a/utils/CHANGELOG.md +++ b/utils/CHANGELOG.md @@ -5,6 +5,9 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel ## Unreleased +### Added +- [`no-unused-modules`]: support dynamic imports ([#1660], thanks [@maxkomarychev]) + ## v2.6.2 - 2021-08-08 ### Fixed @@ -94,6 +97,7 @@ Yanked due to critical issue with cache key resulting from #839. [#2026]: https://github.com/import-js/eslint-plugin-import/pull/2026 [#1786]: https://github.com/import-js/eslint-plugin-import/pull/1786 [#1671]: https://github.com/import-js/eslint-plugin-import/pull/1671 +[#1660]: https://github.com/import-js/eslint-plugin-import/pull/1660 [#1606]: https://github.com/import-js/eslint-plugin-import/pull/1606 [#1602]: https://github.com/import-js/eslint-plugin-import/pull/1602 [#1591]: https://github.com/import-js/eslint-plugin-import/pull/1591 @@ -118,6 +122,7 @@ Yanked due to critical issue with cache key resulting from #839. [@JounQin]: https://github.com/JounQin [@kaiyoma]: https://github.com/kaiyoma [@manuth]: https://github.com/manuth +[@maxkomarychev]: https://github.com/maxkomarychev [@pmcelhaney]: https://github.com/pmcelhaney [@sompylasar]: https://github.com/sompylasar [@timkraut]: https://github.com/timkraut diff --git a/utils/parse.js b/utils/parse.js index 3b2ac028f0..fe764c7032 100644 --- a/utils/parse.js +++ b/utils/parse.js @@ -3,9 +3,34 @@ exports.__esModule = true; const moduleRequire = require('./module-require').default; const extname = require('path').extname; +const fs = require('fs'); const log = require('debug')('eslint-plugin-import:parse'); +function getBabelVisitorKeys(parserPath) { + const hypotheticalLocation = parserPath.replace('index.js', 'visitor-keys.js'); + if (fs.existsSync(hypotheticalLocation)) { + const keys = moduleRequire(hypotheticalLocation); + return keys; + } + return null; +} + +function keysFromParser(parserPath, parserInstance, parsedResult) { + if (/.*espree.*/.test(parserPath)) { + return parserInstance.VisitorKeys; + } + if (/.*babel-eslint.*/.test(parserPath)) { + return getBabelVisitorKeys(parserPath); + } + if (/.*@typescript-eslint\/parser/.test(parserPath)) { + if (parsedResult) { + return parsedResult.visitorKeys; + } + } + return null; +} + exports.default = function parse(path, content, context) { if (context == null) throw new Error('need context to parse properly'); @@ -45,7 +70,12 @@ exports.default = function parse(path, content, context) { if (typeof parser.parseForESLint === 'function') { let ast; try { - ast = parser.parseForESLint(content, parserOptions).ast; + const parserRaw = parser.parseForESLint(content, parserOptions); + ast = parserRaw.ast; + return { + ast, + visitorKeys: keysFromParser(parserPath, parser, parserRaw), + }; } catch (e) { console.warn(); console.warn('Error while parsing ' + parserOptions.filePath); @@ -54,11 +84,18 @@ exports.default = function parse(path, content, context) { if (!ast || typeof ast !== 'object') { console.warn('`parseForESLint` from parser `' + parserPath + '` is invalid and will just be ignored'); } else { - return ast; + return { + ast, + visitorKeys: keysFromParser(parserPath, parser, undefined), + }; } } - return parser.parse(content, parserOptions); + const keys = keysFromParser(parserPath, parser, undefined); + return { + ast: parser.parse(content, parserOptions), + visitorKeys: keys, + }; }; function getParserPath(path, context) { diff --git a/utils/unambiguous.js b/utils/unambiguous.js index 1446632f39..a04b51f902 100644 --- a/utils/unambiguous.js +++ b/utils/unambiguous.js @@ -1,8 +1,8 @@ 'use strict'; exports.__esModule = true; +const pattern = /(^|;)\s*(export|import)((\s+\w)|(\s*[{*=]))|import\(/m; -const pattern = /(^|;)\s*(export|import)((\s+\w)|(\s*[{*=]))/m; /** * detect possible imports/exports without a full parse. * @@ -26,5 +26,5 @@ const unambiguousNodeType = /^(?:(?:Exp|Imp)ort.*Declaration|TSExportAssignment) * @return {Boolean} */ exports.isModule = function isUnambiguousModule(ast) { - return ast.body.some(node => unambiguousNodeType.test(node.type)); + return ast.body && ast.body.some(node => unambiguousNodeType.test(node.type)); }; diff --git a/utils/visit.js b/utils/visit.js new file mode 100644 index 0000000000..0a26cf2b91 --- /dev/null +++ b/utils/visit.js @@ -0,0 +1,24 @@ +'use strict' +exports.__esModule = true + +exports.default = function visit(node, keys, visitorSpec) { + if (!node || !keys) { + return + } + const type = node.type + if (typeof visitorSpec[type] === 'function') { + visitorSpec[type](node) + } + const childFields = keys[type] + if (!childFields) { + return + } + childFields.forEach((fieldName) => { + [].concat(node[fieldName]).forEach((item) => { + visit(item, keys, visitorSpec) + }) + }) + if (typeof visitorSpec[`${type}:Exit`] === 'function') { + visitorSpec[`${type}:Exit`](node) + } +}