Skip to content

Commit

Permalink
[New] no-unused-modules: support dynamic imports
Browse files Browse the repository at this point in the history
All occurences of `import('...')` are treated as namespace imports
(`import * as X from '...'`)

See #1660, #2212.

Co-authored-by: Max Komarychev <maxkomarychev@gmail.com>
Co-authored-by: Filipp Riabchun <filipp.riabchun@jetbrains.com>
Co-authored-by: 薛定谔的猫 <weiran.zsd@outlook.com>
  • Loading branch information
3 people authored and ljharb committed Jun 20, 2020
1 parent 7579748 commit 7c382f0
Show file tree
Hide file tree
Showing 11 changed files with 198 additions and 7 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion docs/rules/no-unused-modules.md
Expand Up @@ -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

Expand Down
49 changes: 46 additions & 3 deletions src/ExportMap.js
Expand Up @@ -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';

Expand Down Expand Up @@ -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 = {};
Expand Down
36 changes: 34 additions & 2 deletions src/rules/no-unused-modules.js
Expand Up @@ -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';
Expand Down Expand 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();

Expand Down Expand Up @@ -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 => {
Expand Down Expand Up @@ -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;

Expand Down
13 changes: 13 additions & 0 deletions 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')
}
}
5 changes: 5 additions & 0 deletions tests/files/no-unused-modules/dynamic-import-js.js
@@ -0,0 +1,5 @@
class A {
method() {
const c = import('./exports-for-dynamic-js')
}
}
5 changes: 5 additions & 0 deletions 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;
5 changes: 5 additions & 0 deletions 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
6 changes: 6 additions & 0 deletions tests/files/no-unused-modules/typescript/dynamic-import-ts.ts
@@ -0,0 +1,6 @@
class A {
method() {
const c = import('./exports-for-dynamic-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
75 changes: 74 additions & 1 deletion tests/src/rules/no-unused-modules.js
Expand Up @@ -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: [
Expand Down Expand Up @@ -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({
Expand Down

0 comments on commit 7c382f0

Please sign in to comment.