diff --git a/README.md b/README.md index 41bbff0a0d..7695ac368d 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 b2ae82a3a7..a15abdea40 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,24 @@ 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) +``` + +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 2ad207fada..73038c7860 100644 --- a/src/rules/no-useless-path-segments.js +++ b/src/rules/no-useless-path-segments.js @@ -3,10 +3,10 @@ * @author Thomas Grainger */ -import path from 'path' -import sumBy from 'lodash/sumBy' -import resolve from 'eslint-module-utils/resolve' +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' import docsUrl from '../docsUrl' /** @@ -19,19 +19,22 @@ import docsUrl from '../docsUrl' * ..foo/bar -> ./..foo/bar * foo/bar -> ./foo/bar * - * @param rel {string} relative posix path potentially missing leading './' + * @param relativePath {string} relative posix path potentially missing leading './' * @returns {string} relative posix path that always starts with a ./ **/ -function toRel(rel) { - const stripped = rel.replace(/\/$/g, '') +function toRelativePath(relativePath) { + const stripped = relativePath.replace(/\/$/g, '') // Remove trailing / + return /^((\.\.)|(\.))($|\/)/.test(stripped) ? stripped : `./${stripped}` } function normalize(fn) { - return toRel(path.posix.normalize(fn)) + return toRelativePath(path.posix.normalize(fn)) } -const countRelParent = x => sumBy(x, v => v === '..') +function countRelativeParents(pathSegments) { + return pathSegments.reduce((sum, pathSegment) => pathSegment === '..' ? sum + 1 : sum, 0) +} module.exports = { meta: { @@ -45,65 +48,104 @@ 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: function (context) { + create(context) { const currentDir = path.dirname(context.getFilename()) + const options = context.options[0] function checkSourceValue(source) { - const { value } = source + const { value: importPath } = source - function report(proposed) { + function reportWithProposedPath(proposedPath) { context.report({ node: source, - message: `Useless path segments for "${value}", should be "${proposed}"`, - fix: fixer => fixer.replaceText(source, JSON.stringify(proposed)), + messageId: 'uselessPath', + data: { + path: importPath, + proposedPath, + }, + fix: fixer => proposedPath && fixer.replaceText(source, JSON.stringify(proposedPath)), }) } - if (!value.startsWith('.')) { + // Only relative imports are relevant for this rule --> Skip checking + if (!importPath.startsWith('.')) { return } - const resolvedPath = resolve(value, context) - const normed = normalize(value) - if (normed !== value && resolvedPath === resolve(normed, context)) { - return report(normed) + // Report rule violation if path is not the shortest possible + const resolvedPath = resolve(importPath, context) + const normedPath = normalize(importPath) + const resolvedNormedPath = resolve(normedPath, context) + if (normedPath !== importPath && resolvedPath === resolvedNormedPath) { + 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) } - if (value.startsWith('./')) { + // Path is shortest possible + starts from the current directory --> Return directly + if (importPath.startsWith('./')) { return } + // Path is not existing --> Return directly (following code requires path to be defined) if (resolvedPath === undefined) { return } - const expected = path.relative(currentDir, resolvedPath) - const expectedSplit = expected.split(path.sep) - const valueSplit = value.replace(/^\.\//, '').split('/') - const valueNRelParents = countRelParent(valueSplit) - const expectedNRelParents = countRelParent(expectedSplit) - const diff = valueNRelParents - expectedNRelParents + const expected = path.relative(currentDir, resolvedPath) // Expected import path + const expectedSplit = expected.split(path.sep) // Split by / or \ (depending on OS) + const importPathSplit = importPath.replace(/^\.\//, '').split('/') + const countImportPathRelativeParents = countRelativeParents(importPathSplit) + const countExpectedRelativeParents = countRelativeParents(expectedSplit) + const diff = countImportPathRelativeParents - countExpectedRelativeParents + // Same number of relative parents --> Paths are the same --> Return directly if (diff <= 0) { return } - return report( - toRel(valueSplit - .slice(0, expectedNRelParents) - .concat(valueSplit.slice(valueNRelParents + diff)) - .join('/')) + // Report and propose minimal number of required relative parents + return reportWithProposedPath( + toRelativePath( + importPathSplit + .slice(0, countExpectedRelativeParents) + .concat(importPathSplit.slice(countImportPathRelativeParents + diff)) + .join('/') + ) ) } - return moduleVisitor(checkSourceValue, context.options[0]) + return moduleVisitor(checkSourceValue, options) }, } diff --git a/tests/src/rules/no-useless-path-segments.js b/tests/src/rules/no-useless-path-segments.js index ed20440012..923c7efe79 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 91cc731a81..799b8582b2 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)