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
import-js · Oct 18, 2016 · 4cbdbb8 · 4cbdbb8
1 parent d9605a0
commit 4cbdbb8
Show file tree

Hide file tree

Showing 4 changed files with 207 additions and 0 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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`',
+        },
+      ],
+    }),
   ],
 })