From 4cbdbb87eb664d5c51b16e828133b74897c9a2c2 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 | 65 ++++++++++++++++++++++++ tests/src/rules/order.js | 105 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 207 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7ce7ac773..b8e854863 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 46a1dbd7f..8fb239555 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 da4cae1ae..414b6ae40 100644 --- a/src/rules/order.js +++ b/src/rules/order.js @@ -17,6 +17,16 @@ function reverse(array) { }).reverse() } +function alphabetical(firstName, secondName, direction) { + if (direction === 'a-z') { + return firstName < secondName + } else if (direction === 'z-a') { + return firstName > secondName + } else { + throw new Error('Invalid alphabetical direction' + direction) + } +} + function findOutOfOrder(imported) { if (imported.length === 0) { return [] @@ -31,6 +41,31 @@ function findOutOfOrder(imported) { }) } +function findOutOfAlphabeticalOrder(imported, sortOrder, direction) { + 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 = alphabetical( + importedModule.name, + maxSeenAlphabeticalNode.name, + direction + ) + const reversedDirection = direction.split('').reverse().join('') + if (alphabetical(importedModule.name, maxSeenAlphabeticalNode.name, reversedDirection)) { + maxSeenAlphabeticalNode = importedModule + } + return resAlphabetical + }) +} + function reportOutOfOrder(context, imported, outOfOrder, order) { outOfOrder.forEach(function (imp) { const found = imported.find(function hasHigherRank(importedItem) { @@ -41,6 +76,16 @@ function reportOutOfOrder(context, imported, outOfOrder, order) { }) } +function reportOutOfAlphabeticalOrder(context, imported, outOfOrder, order, direction) { + outOfOrder.forEach(function (imp) { + const found = imported.find(function hasHigherAlphabeticalOrder(importedItem) { + return alphabetical(imp.name, importedItem.name, direction) && 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 +101,21 @@ function makeOutOfOrderReport(context, imported) { reportOutOfOrder(context, imported, outOfOrder, 'before') } +function makeOutOfAlphabeticalOrderReport(context, imported, sortOrder) { + const outOfOrder = findOutOfAlphabeticalOrder(imported, sortOrder, 'a-z') + 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, 'z-a') + if (reversedOrder.length < outOfOrder.length) { + reportOutOfAlphabeticalOrder(context, reversedImported, reversedOrder, 'after', 'z-a') + return + } + reportOutOfAlphabeticalOrder(context, imported, outOfOrder, 'before', 'a-z') +} + // DETECTING function computeRank(context, ranks, name, type) { @@ -158,6 +218,9 @@ module.exports = { 'newlines-between': { enum: [ 'ignore', 'always', 'never' ], }, + 'sort-order': { + enum: [ 'alphabetical', 'ignore' ], + }, }, additionalProperties: false, }, @@ -167,6 +230,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 +269,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 68f06c05f..5bd4298d0 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,77 @@ 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`', + }, + ], + }), + // Bad alphabetical order reverse + test({ + code: ` + import path from 'path'; + import index from './'; + import fs from 'fs'; + `, + options: [ + { + groups: [ + ['builtin', 'index'], + ['sibling'], + ['parent', 'external'], + ], + 'sort-order': 'alphabetical', + } + ], + errors: [ + { + ruleId: 'order', + message: '`path` import should occur after import of `fs`', + }, + ], + }), + // 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`', + }, + ], + }), ], })