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

[Fix] no-unused-modules: fix usage of import/extensions settings #1560

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 @@ -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])
stekycz marked this conversation as resolved.
Show resolved Hide resolved

### Fixed
- [`import/extensions`]: ignore non-main modules ([#1563], thanks [@saschanaz])
Expand Down Expand Up @@ -624,6 +626,7 @@ for info on changes for earlier releases.
[`memo-parser`]: ./memo-parser/README.md

[#1563]: https://github.com/benmosher/eslint-plugin-import/pull/1563
[#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
Expand Down Expand Up @@ -1042,3 +1045,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
64 changes: 51 additions & 13 deletions src/rules/no-unused-modules.js
Expand Up @@ -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'
Expand All @@ -16,19 +17,40 @@ 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) {
// 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
let originalListFilesToProcess
try {
listFilesToProcess = require('eslint/lib/util/glob-utils').listFilesToProcess
originalListFilesToProcess = require('eslint/lib/util/glob-utils').listFilesToProcess
listFilesToProcess = function (src, extensions) {
return originalListFilesToProcess(src, {
extensions: extensions,
})
}
} catch (e2) {
listFilesToProcess = require('eslint/lib/util/glob-util').listFilesToProcess
originalListFilesToProcess = require('eslint/lib/util/glob-util').listFilesToProcess

listFilesToProcess = function (src, extensions) {
const patterns = src.reduce((carry, pattern) => {
return carry.concat(extensions.map((extension) => {
return /\*\*|\*\./.test(pattern) ? pattern : `${pattern}/**/*${extension}`
}))
}, src.slice())

return originalListFilesToProcess(patterns)
}
}
}

Expand All @@ -44,7 +66,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()
Expand All @@ -59,12 +80,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
Expand Down Expand Up @@ -200,11 +223,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 =>
Expand Down Expand Up @@ -340,7 +378,7 @@ module.exports = {
unusedExports,
} = context.options[0] || {}

if (unusedExports && !preparationDone) {
if (unusedExports) {
doPreparation(src, ignoreExports, context)
}

Expand Down Expand Up @@ -389,7 +427,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
Expand Down
3 changes: 3 additions & 0 deletions 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;
1 change: 1 addition & 0 deletions tests/files/no-unused-modules/jsx/file-jsx-b.jsx
@@ -0,0 +1 @@
export const b = 2;
3 changes: 3 additions & 0 deletions 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;
1 change: 1 addition & 0 deletions tests/files/no-unused-modules/typescript/file-ts-b.ts
@@ -0,0 +1 @@
export const b = 2;
64 changes: 64 additions & 0 deletions 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 })
Expand All @@ -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: [
Expand Down Expand Up @@ -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('babel-eslint'),
filename: testFilePath('./no-unused-modules/typescript/file-ts-a.ts'),
}),
],
invalid: [
test({
options: unusedExportsTypescriptOptions,
code: `export const b = 2;`,
parser: require.resolve('babel-eslint'),
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`),
],
}),
],
})
})