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/core/getExports.js b/tests/src/core/getExports.js index 2c9f70d5b..604ae5cf2 100644 --- a/tests/src/core/getExports.js +++ b/tests/src/core/getExports.js @@ -8,7 +8,7 @@ import ExportMap from '../../../src/ExportMap'; import * as fs from 'fs'; import { getFilename } from '../utils'; -import * as unambiguous from 'eslint-module-utils/unambiguous'; +import { test as testUnambiguous } from 'eslint-module-utils/unambiguous'; describe('ExportMap', function () { const fakeContext = Object.assign( @@ -438,7 +438,6 @@ describe('ExportMap', function () { // todo: move to utils describe('unambiguous regex', function () { - const testFiles = [ ['deep/b.js', true], ['bar.js', true], @@ -449,10 +448,8 @@ describe('ExportMap', function () { for (const [testFile, expectedRegexResult] of testFiles) { it(`works for ${testFile} (${expectedRegexResult})`, function () { const content = fs.readFileSync('./tests/files/' + testFile, 'utf8'); - expect(unambiguous.test(content)).to.equal(expectedRegexResult); + expect(testUnambiguous(content)).to.equal(expectedRegexResult); }); } - }); - }); diff --git a/tests/src/core/parse.js b/tests/src/core/parse.js index 7344d94f2..407070aa2 100644 --- a/tests/src/core/parse.js +++ b/tests/src/core/parse.js @@ -69,5 +69,4 @@ describe('parse(content, { settings, ecmaFeatures })', function () { expect(parse.bind(null, path, content, { settings: { 'import/parsers': { [parseStubParserPath]: [ '.js' ] } }, parserPath: null, parserOptions })).not.to.throw(Error); expect(parseSpy.callCount, 'custom parser to be called once').to.equal(1); }); - }); 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({ diff --git a/utils/CHANGELOG.md b/utils/CHANGELOG.md index f33001b6f..241a205b4 100644 --- a/utils/CHANGELOG.md +++ b/utils/CHANGELOG.md @@ -7,6 +7,7 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel ### Added - `fileExistsWithCaseSync`: add `strict` argument ([#1262], thanks [@sergei-startsev]) +- add `visit`, to support dynamic imports ([#1660], [#2212], thanks [@maxkomarychev], [@aladdin-add], [@Hypnosphi]) ## v2.6.2 - 2021-08-08 @@ -93,10 +94,12 @@ Yanked due to critical issue with cache key resulting from #839. ### Fixed - `unambiguous.test()` regex is now properly in multiline mode +[#2212]: https://github.com/import-js/eslint-plugin-import/pull/2212 [#2160]: https://github.com/import-js/eslint-plugin-import/pull/2160 [#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 @@ -126,4 +129,7 @@ Yanked due to critical issue with cache key resulting from #839. [@sergei-startsev]: https://github.com/sergei-startsev [@sompylasar]: https://github.com/sompylasar [@timkraut]: https://github.com/timkraut -[@vikr01]: https://github.com/vikr01 \ No newline at end of file +[@vikr01]: https://github.com/vikr01 +[@maxkomarychev]: https://github.com/maxkomarychev +[@aladdin-add]: https://github.com/aladdin-add +[@Hypnosphi]: https://github.com/Hypnosphi \ No newline at end of file diff --git a/utils/parse.js b/utils/parse.js index 3b2ac028f..d1dd4ef03 100644 --- a/utils/parse.js +++ b/utils/parse.js @@ -3,9 +3,42 @@ 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) { + if (parserPath.endsWith('index.js')) { + const hypotheticalLocation = parserPath.replace('index.js', 'visitor-keys.js'); + if (fs.existsSync(hypotheticalLocation)) { + const keys = moduleRequire(hypotheticalLocation); + return keys.default || keys; + } + } else if (parserPath.endsWith('index.cjs')) { + const hypotheticalLocation = parserPath.replace('index.cjs', 'worker/ast-info.cjs'); + if (fs.existsSync(hypotheticalLocation)) { + const astInfo = moduleRequire(hypotheticalLocation); + return astInfo.getVisitorKeys(); + } + } + return null; +} + +function keysFromParser(parserPath, parserInstance, parsedResult) { + if (/.*espree.*/.test(parserPath)) { + return parserInstance.VisitorKeys; + } + if (/.*(babel-eslint|@babel\/eslint-parser).*/.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,20 +78,36 @@ 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); console.warn('Line ' + e.lineNumber + ', column ' + e.column + ': ' + e.message); } if (!ast || typeof ast !== 'object') { - console.warn('`parseForESLint` from parser `' + parserPath + '` is invalid and will just be ignored'); + 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 1446632f3..75f21693b 100644 --- a/utils/unambiguous.js +++ b/utils/unambiguous.js @@ -1,8 +1,7 @@ 'use strict'; exports.__esModule = true; - -const pattern = /(^|;)\s*(export|import)((\s+\w)|(\s*[{*=]))/m; +const pattern = /(^|;)\s*(export|import)((\s+\w)|(\s*[{*=]))|import\(/m; /** * detect possible imports/exports without a full parse. * @@ -26,5 +25,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 000000000..77b09850a --- /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); + } +};