Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[New] no-unused-modules: support dynamic imports #1660

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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) {
ljharb marked this conversation as resolved.
Show resolved Hide resolved
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
7 changes: 2 additions & 5 deletions tests/src/core/getExports.js
Expand Up @@ -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(
Expand Down Expand Up @@ -438,7 +438,6 @@ describe('ExportMap', function () {

// todo: move to utils
describe('unambiguous regex', function () {

const testFiles = [
['deep/b.js', true],
['bar.js', true],
Expand All @@ -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);
});
}

});

});
1 change: 0 additions & 1 deletion tests/src/core/parse.js
Expand Up @@ -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);
});

});
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