diff --git a/README.md b/README.md index 41bbff0a0..7695ac368 100644 --- a/README.md +++ b/README.md @@ -256,6 +256,17 @@ A list of file extensions that will be parsed as modules and inspected for This defaults to `['.js']`, unless you are using the `react` shared config, in which case it is specified as `['.js', '.jsx']`. +```js +"settings": { + "import/extensions": [ + ".js", + ".jsx" + ] +} +``` + +If you require more granular extension definitions, you can use: + ```js "settings": { "import/resolver": { diff --git a/docs/rules/no-useless-path-segments.md b/docs/rules/no-useless-path-segments.md index b2ae82a3a..6a02eab9f 100644 --- a/docs/rules/no-useless-path-segments.md +++ b/docs/rules/no-useless-path-segments.md @@ -11,6 +11,9 @@ my-project ├── app.js ├── footer.js ├── header.js +└── helpers.js +└── helpers + └── index.js └── pages ├── about.js ├── contact.js @@ -30,6 +33,8 @@ import "../pages/about.js"; // should be "./pages/about.js" import "../pages/about"; // should be "./pages/about" import "./pages//about"; // should be "./pages/about" import "./pages/"; // should be "./pages" +import "./pages/index"; // should be "./pages" (except if there is a ./pages.js file) +import "./pages/index.js"; // should be "./pages" (except if there is a ./pages.js file) ``` The following patterns are NOT considered problems: @@ -46,3 +51,25 @@ import "."; import ".."; import fs from "fs"; ``` + +## Options + +### noUselessIndex + +If you want to detect unnecessary `/index` or `/index.js` (depending on the specified file extensions, see below) imports in your paths, you can enable the option `noUselessIndex`. By default it is set to `false`: +```js +"import/no-useless-path-segments": ["error", { + noUselessIndex: true, +}] +``` + +Additionally to the patterns described above, the following imports are considered problems if `noUselessIndex` is enabled: + +```js +// in my-project/app.js +import "./helpers/index"; // should be "./helpers/" (not auto-fixable to `./helpers` because this would lead to an ambiguous import of `./helpers.js` and `./helpers/index.js`) +import "./pages/index"; // should be "./pages" (auto-fixable) +import "./pages/index.js"; // should be "./pages" (auto-fixable) +``` + +Note: `noUselessIndex` only avoids ambiguous imports for `.js` files if you haven't specified other resolved file extensions. See [Settings: import/extensions](https://github.com/benmosher/eslint-plugin-import#importextensions) for details. diff --git a/src/rules/no-useless-path-segments.js b/src/rules/no-useless-path-segments.js index 3fdb397c6..ea72e6c54 100644 --- a/src/rules/no-useless-path-segments.js +++ b/src/rules/no-useless-path-segments.js @@ -3,6 +3,7 @@ * @author Thomas Grainger */ +import { getFileExtensions } from 'eslint-module-utils/ignore' import moduleVisitor from 'eslint-module-utils/moduleVisitor' import resolve from 'eslint-module-utils/resolve' import path from 'path' @@ -31,9 +32,9 @@ function normalize(fn) { return toRelativePath(path.posix.normalize(fn)) } -const countRelativeParents = (pathSegments) => pathSegments.reduce( - (sum, pathSegment) => pathSegment === '..' ? sum + 1 : sum, 0 -) +function countRelativeParents(pathSegments) { + return pathSegments.reduce((sum, pathSegment) => pathSegment === '..' ? sum + 1 : sum, 0) +} module.exports = { meta: { @@ -47,33 +48,28 @@ module.exports = { type: 'object', properties: { commonjs: { type: 'boolean' }, + noUselessIndex: { type: 'boolean' }, }, additionalProperties: false, }, ], fixable: 'code', - messages: { - uselessPath: 'Useless path segments for "{{ path }}", should be "{{ proposedPath }}"', - }, }, create(context) { const currentDir = path.dirname(context.getFilename()) - const config = context.options[0] + const options = context.options[0] function checkSourceValue(source) { const { value: importPath } = source - function report(proposedPath) { + function reportWithProposedPath(proposedPath) { context.report({ node: source, - messageId: 'uselessPath', - data: { - path: importPath, - proposedPath, - }, - fix: fixer => fixer.replaceText(source, JSON.stringify(proposedPath)), + // Note: Using messageIds is not possible due to the support for ESLint 2 and 3 + message: `Useless path segments for "${importPath}", should be "${proposedPath}"`, + fix: fixer => proposedPath && fixer.replaceText(source, JSON.stringify(proposedPath)), }) } @@ -87,7 +83,28 @@ module.exports = { const normedPath = normalize(importPath) const resolvedNormedPath = resolve(normedPath, context) if (normedPath !== importPath && resolvedPath === resolvedNormedPath) { - return report(normedPath) + return reportWithProposedPath(normedPath) + } + + const fileExtensions = getFileExtensions(context.settings) + const regexUnnecessaryIndex = new RegExp( + `.*\\/index(\\${Array.from(fileExtensions).join('|\\')})?$` + ) + + // Check if path contains unnecessary index (including a configured extension) + if (options && options.noUselessIndex && regexUnnecessaryIndex.test(importPath)) { + const parentDirectory = path.dirname(importPath) + + // Try to find ambiguous imports + if (parentDirectory !== '.' && parentDirectory !== '..') { + for (let fileExtension of fileExtensions) { + if (resolve(`${parentDirectory}${fileExtension}`, context)) { + return reportWithProposedPath(`${parentDirectory}/`) + } + } + } + + return reportWithProposedPath(parentDirectory) } // Path is shortest possible + starts from the current directory --> Return directly @@ -113,7 +130,7 @@ module.exports = { } // Report and propose minimal number of required relative parents - return report( + return reportWithProposedPath( toRelativePath( importPathSplit .slice(0, countExpectedRelativeParents) @@ -123,6 +140,6 @@ module.exports = { ) } - return moduleVisitor(checkSourceValue, config) + return moduleVisitor(checkSourceValue, options) }, } diff --git a/tests/src/core/ignore.js b/tests/src/core/ignore.js index cc89f8454..8870158a5 100644 --- a/tests/src/core/ignore.js +++ b/tests/src/core/ignore.js @@ -1,6 +1,6 @@ import { expect } from 'chai' -import isIgnored, { hasValidExtension } from 'eslint-module-utils/ignore' +import isIgnored, { getFileExtensions, hasValidExtension } from 'eslint-module-utils/ignore' import * as utils from '../utils' @@ -55,4 +55,36 @@ describe('ignore', function () { }) }) + describe('getFileExtensions', function () { + it('returns a set with the file extension ".js" if "import/extensions" is not configured', function () { + const fileExtensions = getFileExtensions({}) + + expect(fileExtensions).to.include('.js') + }) + + it('returns a set with the file extensions configured in "import/extension"', function () { + const settings = { + 'import/extensions': ['.js', '.jsx'], + } + + const fileExtensions = getFileExtensions(settings) + + expect(fileExtensions).to.include('.js') + expect(fileExtensions).to.include('.jsx') + }) + + it('returns a set with the file extensions configured in "import/extension" and "import/parsers"', function () { + const settings = { + 'import/parsers': { + 'typescript-eslint-parser': ['.ts', '.tsx'], + }, + } + + const fileExtensions = getFileExtensions(settings) + + expect(fileExtensions).to.include('.js') // If "import/extensions" is not configured, this is the default + expect(fileExtensions).to.include('.ts') + expect(fileExtensions).to.include('.tsx') + }) + }) }) diff --git a/tests/src/rules/no-useless-path-segments.js b/tests/src/rules/no-useless-path-segments.js index ed2044001..923c7efe7 100644 --- a/tests/src/rules/no-useless-path-segments.js +++ b/tests/src/rules/no-useless-path-segments.js @@ -7,20 +7,31 @@ const rule = require('rules/no-useless-path-segments') function runResolverTests(resolver) { ruleTester.run(`no-useless-path-segments (${resolver})`, rule, { valid: [ - // commonjs with default options + // CommonJS modules with default options test({ code: 'require("./../files/malformed.js")' }), - // esmodule + // ES modules with default options test({ code: 'import "./malformed.js"' }), test({ code: 'import "./test-module"' }), test({ code: 'import "./bar/"' }), test({ code: 'import "."' }), test({ code: 'import ".."' }), test({ code: 'import fs from "fs"' }), + test({ code: 'import fs from "fs"' }), + + // ES modules + noUselessIndex + test({ code: 'import "../index"' }), // noUselessIndex is false by default + test({ code: 'import "../my-custom-index"', options: [{ noUselessIndex: true }] }), + test({ code: 'import "./bar.js"', options: [{ noUselessIndex: true }] }), // ./bar/index.js exists + test({ code: 'import "./bar"', options: [{ noUselessIndex: true }] }), + test({ code: 'import "./bar/"', options: [{ noUselessIndex: true }] }), // ./bar.js exists + test({ code: 'import "./malformed.js"', options: [{ noUselessIndex: true }] }), // ./malformed directory does not exist + test({ code: 'import "./malformed"', options: [{ noUselessIndex: true }] }), // ./malformed directory does not exist + test({ code: 'import "./importType"', options: [{ noUselessIndex: true }] }), // ./importType.js does not exist ], invalid: [ - // commonjs + // CommonJS modules test({ code: 'require("./../files/malformed.js")', options: [{ commonjs: true }], @@ -62,7 +73,49 @@ function runResolverTests(resolver) { errors: [ 'Useless path segments for "./deep//a", should be "./deep/a"'], }), - // esmodule + // CommonJS modules + noUselessIndex + test({ + code: 'require("./bar/index.js")', + options: [{ commonjs: true, noUselessIndex: true }], + errors: ['Useless path segments for "./bar/index.js", should be "./bar/"'], // ./bar.js exists + }), + test({ + code: 'require("./bar/index")', + options: [{ commonjs: true, noUselessIndex: true }], + errors: ['Useless path segments for "./bar/index", should be "./bar/"'], // ./bar.js exists + }), + test({ + code: 'require("./importPath/")', + options: [{ commonjs: true, noUselessIndex: true }], + errors: ['Useless path segments for "./importPath/", should be "./importPath"'], // ./importPath.js does not exist + }), + test({ + code: 'require("./importPath/index.js")', + options: [{ commonjs: true, noUselessIndex: true }], + errors: ['Useless path segments for "./importPath/index.js", should be "./importPath"'], // ./importPath.js does not exist + }), + test({ + code: 'require("./importType/index")', + options: [{ commonjs: true, noUselessIndex: true }], + errors: ['Useless path segments for "./importType/index", should be "./importType"'], // ./importPath.js does not exist + }), + test({ + code: 'require("./index")', + options: [{ commonjs: true, noUselessIndex: true }], + errors: ['Useless path segments for "./index", should be "."'], + }), + test({ + code: 'require("../index")', + options: [{ commonjs: true, noUselessIndex: true }], + errors: ['Useless path segments for "../index", should be ".."'], + }), + test({ + code: 'require("../index.js")', + options: [{ commonjs: true, noUselessIndex: true }], + errors: ['Useless path segments for "../index.js", should be ".."'], + }), + + // ES modules test({ code: 'import "./../files/malformed.js"', errors: [ 'Useless path segments for "./../files/malformed.js", should be "../files/malformed.js"'], @@ -95,8 +148,50 @@ function runResolverTests(resolver) { code: 'import "./deep//a"', errors: [ 'Useless path segments for "./deep//a", should be "./deep/a"'], }), - ], - }) + + // ES modules + noUselessIndex + test({ + code: 'import "./bar/index.js"', + options: [{ noUselessIndex: true }], + errors: ['Useless path segments for "./bar/index.js", should be "./bar/"'], // ./bar.js exists + }), + test({ + code: 'import "./bar/index"', + options: [{ noUselessIndex: true }], + errors: ['Useless path segments for "./bar/index", should be "./bar/"'], // ./bar.js exists + }), + test({ + code: 'import "./importPath/"', + options: [{ noUselessIndex: true }], + errors: ['Useless path segments for "./importPath/", should be "./importPath"'], // ./importPath.js does not exist + }), + test({ + code: 'import "./importPath/index.js"', + options: [{ noUselessIndex: true }], + errors: ['Useless path segments for "./importPath/index.js", should be "./importPath"'], // ./importPath.js does not exist + }), + test({ + code: 'import "./importPath/index"', + options: [{ noUselessIndex: true }], + errors: ['Useless path segments for "./importPath/index", should be "./importPath"'], // ./importPath.js does not exist + }), + test({ + code: 'import "./index"', + options: [{ noUselessIndex: true }], + errors: ['Useless path segments for "./index", should be "."'], + }), + test({ + code: 'import "../index"', + options: [{ noUselessIndex: true }], + errors: ['Useless path segments for "../index", should be ".."'], + }), + test({ + code: 'import "../index.js"', + options: [{ noUselessIndex: true }], + errors: ['Useless path segments for "../index.js", should be ".."'], + }), + ], + }) } ['node', 'webpack'].forEach(runResolverTests) diff --git a/utils/ignore.js b/utils/ignore.js index c1ddc8699..47af8122d 100644 --- a/utils/ignore.js +++ b/utils/ignore.js @@ -34,6 +34,7 @@ function makeValidExtensionSet(settings) { return exts } +exports.getFileExtensions = makeValidExtensionSet exports.default = function ignore(path, context) { // check extension whitelist first (cheap)