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 '...'`)
  • Loading branch information
maxkomarychev authored and ljharb committed Jun 20, 2020
1 parent b02885d commit eee0cc8
Show file tree
Hide file tree
Showing 14 changed files with 315 additions and 12 deletions.
5 changes: 4 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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])
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion docs/rules/no-unused-modules.md
Original file line number Diff line number Diff line change
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
36 changes: 34 additions & 2 deletions src/ExportMap.js
Original file line number Diff line number Diff line change
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,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 = {};
Expand Down
13 changes: 13 additions & 0 deletions tests/files/no-unused-modules/dynamic-import-js-2.js
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
class A {
method() {
const c = import('./exports-for-dynamic-ts')
}
}

Original file line number Diff line number Diff line change
@@ -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
169 changes: 166 additions & 3 deletions tests/src/rules/no-unused-modules.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -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',
Expand Down Expand Up @@ -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({
Expand Down Expand Up @@ -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({
Expand Down
5 changes: 5 additions & 0 deletions utils/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
Loading

0 comments on commit eee0cc8

Please sign in to comment.