From aa7aca1e695ef9026b7aa0cc1907e8e1d936575d Mon Sep 17 00:00:00 2001 From: "Randall Reed, Jr" Date: Mon, 17 Oct 2016 17:54:32 -0400 Subject: [PATCH] Allow secondary alphabetical sorting * After sorting imports by group, allow sorting alphabetically within a group * import/order rule now accepts sort-order option that may be 'ignore' (default) or 'alphabetical' * Fixes #389 --- CHANGELOG.md | 1 + docs/rules/order.md | 36 ++++++++++++++++++ src/rules/order.js | 50 +++++++++++++++++++++++++ tests/src/rules/order.js | 81 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 168 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7ce7ac7734..b8e8548635 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel ## [Unreleased] ### Fixed - [`prefer-default-export`] handles re-exported default exports ([#609]) +- [`order`] allows secondary alphabetical sort ([#389]) ## [2.0.1] - 2016-10-06 ### Fixed diff --git a/docs/rules/order.md b/docs/rules/order.md index 46a1dbd7f8..8fb239555a 100644 --- a/docs/rules/order.md +++ b/docs/rules/order.md @@ -141,6 +141,42 @@ import index from './'; import sibling from './foo'; ``` +### `sort-order: [ignore|alphabetical]`: + + +Enforces alphabetical sorting within import groups: + +- If set to `ignore`, no errors related to order within import groups will be reported (default). +- If set to `alphabetical`, imports within a group must be alphabetized. Imports across groups will not be compared. + +With the default group setting, the following will be invalid: + +```js +/* eslint import/order: ["error", {"sort-order": "alphabetical"}] */ +import path from 'path'; +import fs from 'fs'; +import index from './'; +import sibling from './foo'; +``` + +while this will be valid: + +```js +/* eslint import/order: ["error", {"sort-order": "alphabetical"}] */ +import fs from 'fs'; +import path from 'path'; +import index from './'; +import sibling from './foo'; +``` + +```js +/* eslint import/order: ["error", {"sort-order": "ignore"}] */ +import path from 'path'; +import fs from 'fs'; +import index from './'; +import sibling from './foo'; +``` + ## Related - [`import/external-module-folders`] setting diff --git a/src/rules/order.js b/src/rules/order.js index da4cae1aeb..f99b170bd6 100644 --- a/src/rules/order.js +++ b/src/rules/order.js @@ -31,6 +31,26 @@ function findOutOfOrder(imported) { }) } +function findOutOfAlphabeticalOrder(imported, sortOrder = 'ignore') { + if (imported.length === 0 || sortOrder === 'ignore') { + return [] + } + let maxSeenRankNode = imported[0] + let maxSeenAlphabeticalNode = imported[0] + return imported.filter(function (importedModule) { + if (maxSeenRankNode.rank < importedModule.rank) { + maxSeenRankNode = importedModule + // New group, reset max alphabetical node + maxSeenAlphabeticalNode = importedModule + } + const resAlphabetical = (importedModule.name < maxSeenAlphabeticalNode.name) + if (maxSeenAlphabeticalNode.name < importedModule.name) { + maxSeenAlphabeticalNode = importedModule + } + return resAlphabetical + }) +} + function reportOutOfOrder(context, imported, outOfOrder, order) { outOfOrder.forEach(function (imp) { const found = imported.find(function hasHigherRank(importedItem) { @@ -41,6 +61,16 @@ function reportOutOfOrder(context, imported, outOfOrder, order) { }) } +function reportOutOfAlphabeticalOrder(context, imported, outOfOrder, order) { + outOfOrder.forEach(function (imp) { + const found = imported.find(function hasHigherAlphabeticalOrder(importedItem) { + return importedItem.name > imp.name && importedItem.rank === imp.rank + }) + context.report(imp.node, '`' + imp.name + '` import should occur ' + order + + ' import of `' + found.name + '`') + }) +} + function makeOutOfOrderReport(context, imported) { const outOfOrder = findOutOfOrder(imported) if (!outOfOrder.length) { @@ -56,6 +86,21 @@ function makeOutOfOrderReport(context, imported) { reportOutOfOrder(context, imported, outOfOrder, 'before') } +function makeOutOfAlphabeticalOrderReport(context, imported, sortOrder) { + const outOfOrder = findOutOfAlphabeticalOrder(imported, sortOrder) + if (!outOfOrder.length) { + return + } + // There are things to report. Try to minimize the number of reported errors. + const reversedImported = reverse(imported) + const reversedOrder = findOutOfAlphabeticalOrder(reversedImported, sortOrder) + if (reversedOrder.length < outOfOrder.length && reversedOrder.length > 0) { + reportOutOfAlphabeticalOrder(context, reversedImported, reversedOrder, 'after') + return + } + reportOutOfAlphabeticalOrder(context, imported, outOfOrder, 'before') +} + // DETECTING function computeRank(context, ranks, name, type) { @@ -158,6 +203,9 @@ module.exports = { 'newlines-between': { enum: [ 'ignore', 'always', 'never' ], }, + 'sort-order': { + enum: [ 'alphabetical', 'ignore' ], + }, }, additionalProperties: false, }, @@ -167,6 +215,7 @@ module.exports = { create: function importOrderRule (context) { const options = context.options[0] || {} const newlinesBetweenImports = options['newlines-between'] || 'ignore' + const sortOrder = options['sort-order'] || 'ignore' let ranks try { @@ -205,6 +254,7 @@ module.exports = { }, 'Program:exit': function reportAndReset() { makeOutOfOrderReport(context, imported) + makeOutOfAlphabeticalOrderReport(context, imported, sortOrder) if (newlinesBetweenImports !== 'ignore') { makeNewlinesBetweenReport(context, imported, newlinesBetweenImports) diff --git a/tests/src/rules/order.js b/tests/src/rules/order.js index 68f06c05fc..6a0b646831 100644 --- a/tests/src/rules/order.js +++ b/tests/src/rules/order.js @@ -376,6 +376,39 @@ ruleTester.run('order', rule, { `, options: [{ 'newlines-between': 'always' }] }), + // Alphabetical order + test({ + code: ` + import fs from 'fs'; + import path from 'path'; + `, + options: [{ 'sort-order': 'alphabetical' }], + }), + // Alphabetical order with multiple groups + test({ + code: ` + import fs from 'fs'; + import path from 'path'; + import async from 'async'; + `, + options: [{ 'sort-order': 'alphabetical' }], + }), + // Ignore alphabetical order + test({ + code: ` + import path from 'path'; + import fs from 'fs'; + `, + options: [{ 'sort-order': 'ignore' }], + }), + // Ignore alphabetical order across groups + test({ + code: ` + import fs from 'fs'; + import async from 'async'; + `, + options: [{ 'sort-order': 'alphabetical' }], + }), ], invalid: [ // builtin before external module (require) @@ -760,5 +793,53 @@ ruleTester.run('order', rule, { }, ], }), + // Bad alphabetical order + test({ + code: ` + import path from 'path'; + import fs from 'fs'; + `, + options: [{ 'sort-order': 'alphabetical' }], + errors: [ + { + ruleId: 'order', + message: '`fs` import should occur before import of `path`', + }, + ], + }), + // Good alphabetical order with incorrect group order + test({ + code: ` + import async from 'async' + import fs from 'fs'; + import path from 'path'; + `, + options: [{ 'sort-order': 'alphabetical' }], + errors: [ + { + ruleId: 'order', + message: '`async` import should occur after import of `path`', + }, + ], + }), + // Bad alphabetical order with incorrect group order + test({ + code: ` + import async from 'async' + import path from 'path'; + import fs from 'fs'; + `, + options: [{ 'sort-order': 'alphabetical' }], + errors: [ + { + ruleId: 'order', + message: '`async` import should occur after import of `fs`', + }, + { + ruleId: 'order', + message: '`fs` import should occur before import of `path`', + }, + ], + }), ], })