diff --git a/docs/rules/no-unused-modules.md b/docs/rules/no-unused-modules.md index 4302bc8458..4c07a2fd54 100644 --- a/docs/rules/no-unused-modules.md +++ b/docs/rules/no-unused-modules.md @@ -2,9 +2,8 @@ Reports: - modules without any exports - - individual exports not being statically `import`ed or `require`ed from other modules in the same project + - individual exports not `import`ed or `require`ed from other modules in the same project -Note: dynamic imports are currently not supported. ## Rule Details diff --git a/src/ExportMap.js b/src/ExportMap.js index 525f64a48a..656bafe441 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' @@ -344,14 +345,49 @@ ExportMap.parse = function (path, content, context) { var m = new ExportMap(path) try { - var ast = parse(path, content, context) + var { 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 + if (!node.arguments.length) { + return null + } + if (node.arguments[0].type !== 'Literal') { + return null + } + const literal = node.arguments[0] + if (!literal) { + return null + } + const p = remotePath(literal.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: literal.value, + loc: literal.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.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.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 ac15fd9154..f2fd5b5501 100644 --- a/tests/src/rules/no-unused-modules.js +++ b/tests/src/rules/no-unused-modules.js @@ -1,4 +1,4 @@ -import { test, testFilePath } from '../utils' +import { test, testFilePath } from '../../src/utils' import jsxConfig from '../../../config/react' import typescriptConfig from '../../../config/typescript' @@ -71,32 +71,39 @@ 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', - filename: testFilePath('./no-unused-modules/file-a.js')}), + 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')}), + 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')}), + 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')}), + 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')}), + 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')}), + 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')}), + 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')}), - ], + filename: testFilePath('./no-unused-modules/file-o.js'), + parser: require.resolve('babel-eslint')}), + ], invalid: [ test({ options: unusedExportsOptions, code: `import eslint from 'eslint' @@ -164,6 +171,40 @@ ruleTester.run('no-unused-modules', rule, { ], }) +// 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: [ + ], +}) +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: [ diff --git a/utils/parse.js b/utils/parse.js index b3a469221d..091489aa08 100644 --- a/utils/parse.js +++ b/utils/parse.js @@ -3,9 +3,33 @@ 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(parserPath.replace('index.js', 'visitor-keys.js')) + return keys + } else { + return null + } +} + +function keysFromParser(parserPath, parserInstance, parsedResult) { + if (/.*espree.*/.test(parserPath)) { + return parserInstance.VisitorKeys + } else if (/.*babel-eslint.*/.test(parserPath)) { + return getBabelVisitorKeys(parserPath) + } else 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 +69,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) @@ -55,16 +84,25 @@ exports.default = function parse(path, content, context) { console.warn( '`parseForESLint` from parser `' + parserPath + - '` is invalid and will just be ignored' + '` is invalid and will just be ignored', + path ) } 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) { const parsers = context.settings['import/parsers'] if (parsers != null) { diff --git a/utils/unambiguous.js b/utils/unambiguous.js index 1dae1d6167..e505be3719 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 0000000000..45a1f147d3 --- /dev/null +++ b/utils/visit.js @@ -0,0 +1,28 @@ +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 + } + for (const fieldName of childFields) { + const field = node[fieldName] + if (Array.isArray(field)) { + for (const item of field) { + visit(item, keys, visitorSpec) + } + } else { + visit(field, keys, visitorSpec) + } + } + if (typeof visitorSpec[`${type}:Exit`] === 'function') { + visitorSpec[`${type}:Exit`](node) + } +}