Skip to content

Commit cff6dbb

Browse files
committed
no-unused-modules: support dynamic imports
All occurences of `import('...')` are treated as namespace imports (`import * as X from '...'`)
1 parent 40d1e67 commit cff6dbb

13 files changed

+217
-22
lines changed

CHANGELOG.md

+4
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,10 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel
66

77
## [Unreleased]
88

9+
### Added
10+
11+
- [`no-unused-modules`]: support dynamic imports ([#1660], thanks [@maxkomarychev])
12+
913
### Fixed
1014
- [`group-exports`]: Flow type export awareness ([#1702], thanks [@ernestostifano])
1115
- [`order`]: Recognize pathGroup config for first group ([#1719], [#1724], thanks [@forivall], [@xpl])

docs/rules/no-unused-modules.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@
33
Reports:
44
- modules without any exports
55
- individual exports not being statically `import`ed or `require`ed from other modules in the same project
6+
- dynamic imports are supported if argument is a literal string
67

7-
Note: dynamic imports are currently not supported.
88

99
## Rule Details
1010

src/ExportMap.js

+33-3
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import debug from 'debug'
77
import { SourceCode } from 'eslint'
88

99
import parse from 'eslint-module-utils/parse'
10+
import visit from 'eslint-module-utils/visit'
1011
import resolve from 'eslint-module-utils/resolve'
1112
import isIgnored, { hasValidExtension } from 'eslint-module-utils/ignore'
1213

@@ -344,14 +345,43 @@ ExportMap.parse = function (path, content, context) {
344345
var m = new ExportMap(path)
345346

346347
try {
347-
var ast = parse(path, content, context)
348+
var { ast, visitorKeys } = parse(path, content, context)
348349
} catch (err) {
349-
log('parse error:', path, err)
350350
m.errors.push(err)
351351
return m // can't continue
352352
}
353353

354-
if (!unambiguous.isModule(ast)) return null
354+
let hasDynamicImports = false
355+
356+
visit(ast, visitorKeys, {
357+
CallExpression(node) {
358+
if (node.callee.type === 'Import') {
359+
hasDynamicImports = true
360+
const firstArgument = node.arguments[0]
361+
if (firstArgument.type !== 'Literal') {
362+
return null
363+
}
364+
const p = remotePath(firstArgument.value)
365+
if (p == null) {
366+
return null
367+
}
368+
const importedSpecifiers = new Set()
369+
importedSpecifiers.add('ImportNamespaceSpecifier')
370+
const getter = thunkFor(p, context)
371+
m.imports.set(p, {
372+
getter,
373+
source: {
374+
// capturing actual node reference holds full AST in memory!
375+
value: firstArgument.value,
376+
loc: firstArgument.loc,
377+
},
378+
importedSpecifiers,
379+
})
380+
}
381+
},
382+
})
383+
384+
if (!unambiguous.isModule(ast) && !hasDynamicImports) return null
355385

356386
const docstyle = (context.settings && context.settings['import/docstyle']) || ['jsdoc']
357387
const docStyleParsers = {}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
const importPath = './exports-for-dynamic-js';
2+
class A {
3+
method() {
4+
const c = import(importPath)
5+
}
6+
}
7+
8+
9+
class B {
10+
method() {
11+
const c = import('i-do-not-exist')
12+
}
13+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
class A {
2+
method() {
3+
const c = import('./exports-for-dynamic-js')
4+
}
5+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
export const a = 10;
2+
export const b = 20;
3+
export const c = 30;
4+
const d = 40;
5+
export default d;
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
export const a = 10
2+
export const b = 20
3+
export const c = 30
4+
const d = 40
5+
export default d
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
class A {
2+
method() {
3+
const c = import('./exports-for-dynamic-ts')
4+
}
5+
}
6+
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
export const ts_a = 10
2+
export const ts_b = 20
3+
export const ts_c = 30
4+
const ts_d = 40
5+
export default ts_d

tests/src/rules/no-unused-modules.js

+68-11
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { test, testFilePath } from '../utils'
1+
import { test, testFilePath } from '../../src/utils'
22
import jsxConfig from '../../../config/react'
33
import typescriptConfig from '../../../config/typescript'
44

@@ -71,32 +71,39 @@ ruleTester.run('no-unused-modules', rule, {
7171
// tests for exports
7272
ruleTester.run('no-unused-modules', rule, {
7373
valid: [
74-
7574
test({ options: unusedExportsOptions,
7675
code: 'import { o2 } from "./file-o";export default () => 12',
77-
filename: testFilePath('./no-unused-modules/file-a.js')}),
76+
filename: testFilePath('./no-unused-modules/file-a.js'),
77+
parser: require.resolve('babel-eslint')}),
7878
test({ options: unusedExportsOptions,
7979
code: 'export const b = 2',
80-
filename: testFilePath('./no-unused-modules/file-b.js')}),
80+
filename: testFilePath('./no-unused-modules/file-b.js'),
81+
parser: require.resolve('babel-eslint')}),
8182
test({ options: unusedExportsOptions,
8283
code: 'const c1 = 3; function c2() { return 3 }; export { c1, c2 }',
83-
filename: testFilePath('./no-unused-modules/file-c.js')}),
84+
filename: testFilePath('./no-unused-modules/file-c.js'),
85+
parser: require.resolve('babel-eslint')}),
8486
test({ options: unusedExportsOptions,
8587
code: 'export function d() { return 4 }',
86-
filename: testFilePath('./no-unused-modules/file-d.js')}),
88+
filename: testFilePath('./no-unused-modules/file-d.js'),
89+
parser: require.resolve('babel-eslint')}),
8790
test({ options: unusedExportsOptions,
8891
code: 'export class q { q0() {} }',
89-
filename: testFilePath('./no-unused-modules/file-q.js')}),
92+
filename: testFilePath('./no-unused-modules/file-q.js'),
93+
parser: require.resolve('babel-eslint')}),
9094
test({ options: unusedExportsOptions,
9195
code: 'const e0 = 5; export { e0 as e }',
92-
filename: testFilePath('./no-unused-modules/file-e.js')}),
96+
filename: testFilePath('./no-unused-modules/file-e.js'),
97+
parser: require.resolve('babel-eslint')}),
9398
test({ options: unusedExportsOptions,
9499
code: 'const l0 = 5; const l = 10; export { l0 as l1, l }; export default () => {}',
95-
filename: testFilePath('./no-unused-modules/file-l.js')}),
100+
filename: testFilePath('./no-unused-modules/file-l.js'),
101+
parser: require.resolve('babel-eslint')}),
96102
test({ options: unusedExportsOptions,
97103
code: 'const o0 = 0; const o1 = 1; export { o0, o1 as o2 }; export default () => {}',
98-
filename: testFilePath('./no-unused-modules/file-o.js')}),
99-
],
104+
filename: testFilePath('./no-unused-modules/file-o.js'),
105+
parser: require.resolve('babel-eslint')}),
106+
],
100107
invalid: [
101108
test({ options: unusedExportsOptions,
102109
code: `import eslint from 'eslint'
@@ -164,6 +171,56 @@ ruleTester.run('no-unused-modules', rule, {
164171
],
165172
})
166173

174+
// test for unused exports with `import()`
175+
ruleTester.run('no-unused-modules', rule, {
176+
valid: [
177+
test({ options: unusedExportsOptions,
178+
code: `
179+
export const a = 10
180+
export const b = 20
181+
export const c = 30
182+
const d = 40
183+
export default d
184+
`,
185+
parser: require.resolve('babel-eslint'),
186+
filename: testFilePath('./no-unused-modules/exports-for-dynamic-js.js')}),
187+
],
188+
invalid: [
189+
test({ options: unusedExportsOptions,
190+
code: `
191+
export const a = 10
192+
export const b = 20
193+
export const c = 30
194+
const d = 40
195+
export default d
196+
`,
197+
parser: require.resolve('babel-eslint'),
198+
filename: testFilePath('./no-unused-modules/exports-for-dynamic-js-2.js'),
199+
errors: [
200+
error(`exported declaration 'a' not used within other modules`),
201+
error(`exported declaration 'b' not used within other modules`),
202+
error(`exported declaration 'c' not used within other modules`),
203+
error(`exported declaration 'default' not used within other modules`),
204+
]}),
205+
],
206+
})
207+
typescriptRuleTester.run('no-unused-modules', rule, {
208+
valid: [
209+
test({ options: unusedExportsTypescriptOptions,
210+
code: `
211+
export const ts_a = 10
212+
export const ts_b = 20
213+
export const ts_c = 30
214+
const ts_d = 40
215+
export default ts_d
216+
`,
217+
parser: require.resolve('@typescript-eslint/parser'),
218+
filename: testFilePath('./no-unused-modules/typescript/exports-for-dynamic-ts.ts')}),
219+
],
220+
invalid: [
221+
],
222+
})
223+
167224
// // test for export from
168225
ruleTester.run('no-unused-modules', rule, {
169226
valid: [

utils/parse.js

+42-4
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,33 @@ exports.__esModule = true
33

44
const moduleRequire = require('./module-require').default
55
const extname = require('path').extname
6+
const fs = require('fs')
67

78
const log = require('debug')('eslint-plugin-import:parse')
89

10+
function getBabelVisitorKeys(parserPath) {
11+
const hypotheticalLocation = parserPath.replace('index.js', 'visitor-keys.js')
12+
if (fs.existsSync(hypotheticalLocation)) {
13+
const keys = moduleRequire(parserPath.replace('index.js', 'visitor-keys.js'))
14+
return keys
15+
} else {
16+
return null
17+
}
18+
}
19+
20+
function keysFromParser(parserPath, parserInstance, parsedResult) {
21+
if (/.*espree.*/.test(parserPath)) {
22+
return parserInstance.VisitorKeys
23+
} else if (/.*babel-eslint.*/.test(parserPath)) {
24+
return getBabelVisitorKeys(parserPath)
25+
} else if (/.*@typescript-eslint\/parser/.test(parserPath)) {
26+
if (parsedResult) {
27+
return parsedResult.visitorKeys
28+
}
29+
}
30+
return null
31+
}
32+
933
exports.default = function parse(path, content, context) {
1034

1135
if (context == null) throw new Error('need context to parse properly')
@@ -45,7 +69,12 @@ exports.default = function parse(path, content, context) {
4569
if (typeof parser.parseForESLint === 'function') {
4670
let ast
4771
try {
48-
ast = parser.parseForESLint(content, parserOptions).ast
72+
const parserRaw = parser.parseForESLint(content, parserOptions)
73+
ast = parserRaw.ast
74+
return {
75+
ast,
76+
visitorKeys: keysFromParser(parserPath, parser, parserRaw),
77+
}
4978
} catch (e) {
5079
console.warn()
5180
console.warn('Error while parsing ' + parserOptions.filePath)
@@ -55,16 +84,25 @@ exports.default = function parse(path, content, context) {
5584
console.warn(
5685
'`parseForESLint` from parser `' +
5786
parserPath +
58-
'` is invalid and will just be ignored'
87+
'` is invalid and will just be ignored',
88+
path
5989
)
6090
} else {
61-
return ast
91+
return {
92+
ast,
93+
visitorKeys: keysFromParser(parserPath, parser, undefined),
94+
}
6295
}
6396
}
6497

65-
return parser.parse(content, parserOptions)
98+
const keys = keysFromParser(parserPath, parser, undefined)
99+
return {
100+
ast: parser.parse(content, parserOptions),
101+
visitorKeys: keys,
102+
}
66103
}
67104

105+
68106
function getParserPath(path, context) {
69107
const parsers = context.settings['import/parsers']
70108
if (parsers != null) {

utils/unambiguous.js

+2-3
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,7 @@
11
'use strict'
22
exports.__esModule = true
33

4-
5-
const pattern = /(^|;)\s*(export|import)((\s+\w)|(\s*[{*=]))/m
4+
const pattern = /(^|;)\s*(export|import)((\s+\w)|(\s*[{*=]))|import\(/m
65
/**
76
* detect possible imports/exports without a full parse.
87
*
@@ -26,5 +25,5 @@ const unambiguousNodeType = /^(?:(?:Exp|Imp)ort.*Declaration|TSExportAssignment)
2625
* @return {Boolean}
2726
*/
2827
exports.isModule = function isUnambiguousModule(ast) {
29-
return ast.body.some(node => unambiguousNodeType.test(node.type))
28+
return ast.body && ast.body.some(node => unambiguousNodeType.test(node.type))
3029
}

utils/visit.js

+28
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
exports.__esModule = true
2+
3+
exports.default = function visit(node, keys, visitorSpec) {
4+
if (!node || !keys) {
5+
return
6+
}
7+
const type = node.type
8+
if (typeof visitorSpec[type] === 'function') {
9+
visitorSpec[type](node)
10+
}
11+
const childFields = keys[type]
12+
if (!childFields) {
13+
return
14+
}
15+
for (const fieldName of childFields) {
16+
const field = node[fieldName]
17+
if (Array.isArray(field)) {
18+
for (const item of field) {
19+
visit(item, keys, visitorSpec)
20+
}
21+
} else {
22+
visit(field, keys, visitorSpec)
23+
}
24+
}
25+
if (typeof visitorSpec[`${type}:Exit`] === 'function') {
26+
visitorSpec[`${type}:Exit`](node)
27+
}
28+
}

0 commit comments

Comments
 (0)