From 13e5c42f3bb2545c0715567470c4eca7d273bd12 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20S=CC=8Ctekl?= Date: Mon, 9 Dec 2019 11:55:34 +0100 Subject: [PATCH] [Fix] `no-unused-modules`: fix usage of `import/extensions` settings --- CHANGELOG.md | 4 ++ src/rules/no-unused-modules.js | 52 +++++++++++---- .../no-unused-modules/jsx/file-jsx-a.jsx | 3 + .../no-unused-modules/jsx/file-jsx-b.jsx | 1 + .../no-unused-modules/typescript/file-ts-a.ts | 3 + .../no-unused-modules/typescript/file-ts-b.ts | 1 + tests/src/rules/no-unused-modules.js | 64 +++++++++++++++++++ 7 files changed, 115 insertions(+), 13 deletions(-) create mode 100644 tests/files/no-unused-modules/jsx/file-jsx-a.jsx create mode 100644 tests/files/no-unused-modules/jsx/file-jsx-b.jsx create mode 100644 tests/files/no-unused-modules/typescript/file-ts-a.ts create mode 100644 tests/files/no-unused-modules/typescript/file-ts-b.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 18409a9c13..a0000de766 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,8 @@ This project adheres to [Semantic Versioning](http://semver.org/). This change log adheres to standards from [Keep a CHANGELOG](http://keepachangelog.com). ## [Unreleased] +### Fixed +- [`no-unused-modules`]: fix usage of `import/extensions` settings ([#1560], thanks [@stekycz]) ## [2.19.1] - 2019-12-08 ### Fixed @@ -620,6 +622,7 @@ for info on changes for earlier releases. [`memo-parser`]: ./memo-parser/README.md +[#1560]: https://github.com/benmosher/eslint-plugin-import/pull/1560 [#1551]: https://github.com/benmosher/eslint-plugin-import/pull/1551 [#1542]: https://github.com/benmosher/eslint-plugin-import/pull/1542 [#1521]: https://github.com/benmosher/eslint-plugin-import/pull/1521 @@ -1038,3 +1041,4 @@ for info on changes for earlier releases. [@christophercurrie]: https://github.com/christophercurrie [@randallreedjr]: https://github.com/randallreedjr [@Pessimistress]: https://github.com/Pessimistress +[@stekycz]: https://github.com/stekycz diff --git a/src/rules/no-unused-modules.js b/src/rules/no-unused-modules.js index 9bbafe99db..1a80bdebc6 100644 --- a/src/rules/no-unused-modules.js +++ b/src/rules/no-unused-modules.js @@ -5,6 +5,7 @@ */ import Exports from '../ExportMap' +import { getFileExtensions } from 'eslint-module-utils/ignore' import resolve from 'eslint-module-utils/resolve' import docsUrl from '../docsUrl' import { dirname, join } from 'path' @@ -16,19 +17,28 @@ import includes from 'array-includes' // and has been moved to eslint/lib/cli-engine/file-enumerator in version 6 let listFilesToProcess try { - var FileEnumerator = require('eslint/lib/cli-engine/file-enumerator').FileEnumerator - listFilesToProcess = function (src) { - var e = new FileEnumerator() + const FileEnumerator = require('eslint/lib/cli-engine/file-enumerator').FileEnumerator + listFilesToProcess = function (src, extensions) { + const e = new FileEnumerator({ + extensions: extensions, + }) return Array.from(e.iterateFiles(src), ({ filePath, ignored }) => ({ ignored, filename: filePath, })) } } catch (e1) { + let originalListFilesToProcess try { - listFilesToProcess = require('eslint/lib/util/glob-utils').listFilesToProcess + originalListFilesToProcess = require('eslint/lib/util/glob-utils').listFilesToProcess } catch (e2) { - listFilesToProcess = require('eslint/lib/util/glob-util').listFilesToProcess + originalListFilesToProcess = require('eslint/lib/util/glob-util').listFilesToProcess + } + // Prevent passing invalid options (extensions array) to old versions of the function. + // https://github.com/eslint/eslint/blob/v5.16.0/lib/util/glob-utils.js#L178-L280 + // https://github.com/eslint/eslint/blob/v5.2.0/lib/util/glob-util.js#L174-L269 + listFilesToProcess = function (src) { + return originalListFilesToProcess(src) } } @@ -44,7 +54,6 @@ const CLASS_DECLARATION = 'ClassDeclaration' const DEFAULT = 'default' const TYPE_ALIAS = 'TypeAlias' -let preparationDone = false const importList = new Map() const exportList = new Map() const ignoredFiles = new Set() @@ -59,12 +68,14 @@ const isNodeModule = path => { * * return all files matching src pattern, which are not matching the ignoreExports pattern */ -const resolveFiles = (src, ignoreExports) => { +const resolveFiles = (src, ignoreExports, context) => { + const extensions = Array.from(getFileExtensions(context.settings)) + const srcFiles = new Set() - const srcFileList = listFilesToProcess(src) + const srcFileList = listFilesToProcess(src, extensions) // prepare list of ignored files - const ignoredFilesList = listFilesToProcess(ignoreExports) + const ignoredFilesList = listFilesToProcess(ignoreExports, extensions) ignoredFilesList.forEach(({ filename }) => ignoredFiles.add(filename)) // prepare list of source files, don't consider files from node_modules @@ -200,11 +211,26 @@ const getSrc = src => { * the start of a new eslint run */ let srcFiles +let lastPrepareKey const doPreparation = (src, ignoreExports, context) => { - srcFiles = resolveFiles(getSrc(src), ignoreExports) + const prepareKey = JSON.stringify({ + src: (src || []).sort(), + ignoreExports: (ignoreExports || []).sort(), + extensions: Array.from(getFileExtensions(context.settings)).sort(), + }) + if (prepareKey === lastPrepareKey) { + return + } + + importList.clear() + exportList.clear() + ignoredFiles.clear() + filesOutsideSrc.clear() + + srcFiles = resolveFiles(getSrc(src), ignoreExports, context) prepareImportsAndExports(srcFiles, context) determineUsage() - preparationDone = true + lastPrepareKey = prepareKey } const newNamespaceImportExists = specifiers => @@ -340,7 +366,7 @@ module.exports = { unusedExports, } = context.options[0] || {} - if (unusedExports && !preparationDone) { + if (unusedExports) { doPreparation(src, ignoreExports, context) } @@ -389,7 +415,7 @@ module.exports = { // make sure file to be linted is included in source files if (!srcFiles.has(file)) { - srcFiles = resolveFiles(getSrc(src), ignoreExports) + srcFiles = resolveFiles(getSrc(src), ignoreExports, context) if (!srcFiles.has(file)) { filesOutsideSrc.add(file) return diff --git a/tests/files/no-unused-modules/jsx/file-jsx-a.jsx b/tests/files/no-unused-modules/jsx/file-jsx-a.jsx new file mode 100644 index 0000000000..1de6d020c8 --- /dev/null +++ b/tests/files/no-unused-modules/jsx/file-jsx-a.jsx @@ -0,0 +1,3 @@ +import {b} from './file-jsx-b'; + +export const a = b + 1; diff --git a/tests/files/no-unused-modules/jsx/file-jsx-b.jsx b/tests/files/no-unused-modules/jsx/file-jsx-b.jsx new file mode 100644 index 0000000000..202103085c --- /dev/null +++ b/tests/files/no-unused-modules/jsx/file-jsx-b.jsx @@ -0,0 +1 @@ +export const b = 2; diff --git a/tests/files/no-unused-modules/typescript/file-ts-a.ts b/tests/files/no-unused-modules/typescript/file-ts-a.ts new file mode 100644 index 0000000000..a4272256e6 --- /dev/null +++ b/tests/files/no-unused-modules/typescript/file-ts-a.ts @@ -0,0 +1,3 @@ +import {b} from './file-ts-b'; + +export const a = b + 1; diff --git a/tests/files/no-unused-modules/typescript/file-ts-b.ts b/tests/files/no-unused-modules/typescript/file-ts-b.ts new file mode 100644 index 0000000000..202103085c --- /dev/null +++ b/tests/files/no-unused-modules/typescript/file-ts-b.ts @@ -0,0 +1 @@ +export const b = 2; diff --git a/tests/src/rules/no-unused-modules.js b/tests/src/rules/no-unused-modules.js index 5afae4dfac..8d7660c143 100644 --- a/tests/src/rules/no-unused-modules.js +++ b/tests/src/rules/no-unused-modules.js @@ -1,9 +1,13 @@ import { test, testFilePath } from '../utils' +import jsxConfig from '../../../config/react' +import typescriptConfig from '../../../config/typescript' import { RuleTester } from 'eslint' import fs from 'fs' const ruleTester = new RuleTester() + , typescriptRuleTester = new RuleTester(typescriptConfig) + , jsxRuleTester = new RuleTester(jsxConfig) , rule = require('rules/no-unused-modules') const error = message => ({ ruleId: 'no-unused-modules', message }) @@ -18,6 +22,18 @@ const unusedExportsOptions = [{ ignoreExports: [testFilePath('./no-unused-modules/*ignored*.js')], }] +const unusedExportsTypescriptOptions = [{ + unusedExports: true, + src: [testFilePath('./no-unused-modules/typescript')], + ignoreExports: undefined, +}] + +const unusedExportsJsxOptions = [{ + unusedExports: true, + src: [testFilePath('./no-unused-modules/jsx')], + ignoreExports: undefined, +}] + // tests for missing exports ruleTester.run('no-unused-modules', rule, { valid: [ @@ -686,3 +702,51 @@ describe('Avoid errors if re-export all from umd compiled library', () => { invalid: [], }) }) + +describe('correctly work with Typescript only files', () => { + typescriptRuleTester.run('no-unused-modules', rule, { + valid: [ + test({ + options: unusedExportsTypescriptOptions, + code: 'import a from "file-ts-a";', + parser: require.resolve('@typescript-eslint/parser'), + filename: testFilePath('./no-unused-modules/typescript/file-ts-a.ts'), + }), + ], + invalid: [ + test({ + options: unusedExportsTypescriptOptions, + code: `export const b = 2;`, + parser: require.resolve('@typescript-eslint/parser'), + filename: testFilePath('./no-unused-modules/typescript/file-ts-b.ts'), + errors: [ + error(`exported declaration 'b' not used within other modules`), + ], + }), + ], + }) +}) + +describe('correctly work with JSX only files', () => { + jsxRuleTester.run('no-unused-modules', rule, { + valid: [ + test({ + options: unusedExportsJsxOptions, + code: 'import a from "file-jsx-a";', + parser: require.resolve('babel-eslint'), + filename: testFilePath('./no-unused-modules/jsx/file-jsx-a.jsx'), + }), + ], + invalid: [ + test({ + options: unusedExportsJsxOptions, + code: `export const b = 2;`, + parser: require.resolve('babel-eslint'), + filename: testFilePath('./no-unused-modules/jsx/file-jsx-b.jsx'), + errors: [ + error(`exported declaration 'b' not used within other modules`), + ], + }), + ], + }) +})