Skip to content

Commit

Permalink
Allow secondary alphabetical sorting
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
Randall Reed, Jr authored and randallreedjr committed Oct 18, 2016
1 parent d9605a0 commit a2a80b5
Show file tree
Hide file tree
Showing 4 changed files with 223 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -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
Expand Down
36 changes: 36 additions & 0 deletions docs/rules/order.md
Expand Up @@ -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
Expand Down
81 changes: 81 additions & 0 deletions src/rules/order.js
Expand Up @@ -31,6 +31,47 @@ function findOutOfOrder(imported) {
})
}

function findOutOfAlphabeticalOrder(imported, sortOrder) {
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 findReversedOutOfAlphabeticalOrder(imported, sortOrder = 'ignore') {
if (imported.length === 0 || sortOrder === 'ignore') {
return []
}
let maxSeenRankNode = imported[0]
let minSeenAlphabeticalNode = imported[0]
return imported.filter(function (importedModule) {
if (maxSeenRankNode.rank < importedModule.rank) {
maxSeenRankNode = importedModule
// New group, reset max alphabetical node
minSeenAlphabeticalNode = importedModule
}

const resAlphabetical = importedModule.name > minSeenAlphabeticalNode.name
if (minSeenAlphabeticalNode.name > importedModule.name) {
minSeenAlphabeticalNode = importedModule
}
return resAlphabetical
})
}

function reportOutOfOrder(context, imported, outOfOrder, order) {
outOfOrder.forEach(function (imp) {
const found = imported.find(function hasHigherRank(importedItem) {
Expand All @@ -41,6 +82,26 @@ 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 reportReversedOutOfAlphabeticalOrder(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) {
Expand All @@ -56,6 +117,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 = findReversedOutOfAlphabeticalOrder(reversedImported, sortOrder)
if (reversedOrder.length < outOfOrder.length) {
reportReversedOutOfAlphabeticalOrder(context, reversedImported, reversedOrder, 'after')
return
}
reportOutOfAlphabeticalOrder(context, imported, outOfOrder, 'before')
}

// DETECTING

function computeRank(context, ranks, name, type) {
Expand Down Expand Up @@ -158,6 +234,9 @@ module.exports = {
'newlines-between': {
enum: [ 'ignore', 'always', 'never' ],
},
'sort-order': {
enum: [ 'alphabetical', 'ignore' ],
},
},
additionalProperties: false,
},
Expand All @@ -167,6 +246,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 {
Expand Down Expand Up @@ -205,6 +285,7 @@ module.exports = {
},
'Program:exit': function reportAndReset() {
makeOutOfOrderReport(context, imported)
makeOutOfAlphabeticalOrderReport(context, imported, sortOrder)

if (newlinesBetweenImports !== 'ignore') {
makeNewlinesBetweenReport(context, imported, newlinesBetweenImports)
Expand Down
105 changes: 105 additions & 0 deletions tests/src/rules/order.js
Expand Up @@ -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)
Expand Down Expand Up @@ -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`',
},
],
}),
],
})

0 comments on commit a2a80b5

Please sign in to comment.