Skip to content

Commit

Permalink
Allow secondary alphabetical sort of imports
Browse files Browse the repository at this point in the history
* Define comparators that take into account rank and name
  • Loading branch information
randallreedjr committed Oct 21, 2016
1 parent d9605a0 commit ba90260
Show file tree
Hide file tree
Showing 3 changed files with 3,839 additions and 18 deletions.
53 changes: 36 additions & 17 deletions src/rules/order.js
Expand Up @@ -11,49 +11,51 @@ function reverse(array) {
return array.map(function (v) {
return {
name: v.name,
rank: -v.rank,
rank: v.rank,
node: v.node,
}
}).reverse()
}

function findOutOfOrder(imported) {
function findOutOfOrder(imported, comparator) {
if (imported.length === 0) {
return []
}
let maxSeenRankNode = imported[0]
return imported.filter(function (importedModule) {
const res = importedModule.rank < maxSeenRankNode.rank
if (maxSeenRankNode.rank < importedModule.rank) {
// const res = importedModule.rank < maxSeenRankNode.rank
const res = comparator(importedModule, maxSeenRankNode)
// if (maxSeenRankNode.rank < importedModule.rank) {
if (comparator(maxSeenRankNode, importedModule)) {
maxSeenRankNode = importedModule
}
return res
})
}

function reportOutOfOrder(context, imported, outOfOrder, order) {
outOfOrder.forEach(function (imp) {
const found = imported.find(function hasHigherRank(importedItem) {
return importedItem.rank > imp.rank
})
function reportOutOfOrder(context, imported, outOfOrder, order, comparator) {
for (let imp of outOfOrder) {
const found = imported.find(importedItem => comparator(importedItem, imp))

context.report(imp.node, '`' + imp.name + '` import should occur ' + order +
' import of `' + found.name + '`')
})
}
}

function makeOutOfOrderReport(context, imported) {
const outOfOrder = findOutOfOrder(imported)
function makeOutOfOrderReport(context, imported, forwardComparator, reverseComparator) {
const outOfOrder = findOutOfOrder(imported, forwardComparator)
if (!outOfOrder.length) {
return
}
// There are things to report. Try to minimize the number of reported errors.
const reversedImported = reverse(imported)
const reversedOrder = findOutOfOrder(reversedImported)
const reversedOrder = findOutOfOrder(reversedImported, reverseComparator)
const sortedImports = [...imported].sort(reverseComparator)
if (reversedOrder.length < outOfOrder.length) {
reportOutOfOrder(context, reversedImported, reversedOrder, 'after')
return
reportOutOfOrder(context, sortedImports.reverse(), reversedOrder, 'after', forwardComparator)
} else {
reportOutOfOrder(context, sortedImports, outOfOrder, 'before', reverseComparator)
}
reportOutOfOrder(context, imported, outOfOrder, 'before')
}

// DETECTING
Expand Down Expand Up @@ -158,6 +160,9 @@ module.exports = {
'newlines-between': {
enum: [ 'ignore', 'always', 'never' ],
},
'sort': {
enum: [ 'ignore', 'alphabetical' ],
},
},
additionalProperties: false,
},
Expand Down Expand Up @@ -189,6 +194,18 @@ module.exports = {
level--
}

function determineComparators(alphabetize) {
let forwardComparator, reverseComparator
if (alphabetize) {
forwardComparator = (a, b) => a.rank < b.rank || (a.rank === b.rank && a.name < b.name)
reverseComparator = (a, b) => a.rank > b.rank || (a.rank === b.rank && a.name > b.name)
} else {
forwardComparator = (a, b) => a.rank < b.rank
reverseComparator = (a, b) => a.rank > b.rank
}
return [forwardComparator, reverseComparator]
}

return {
ImportDeclaration: function handleImports(node) {
if (node.specifiers.length) { // Ignoring unassigned imports
Expand All @@ -204,7 +221,9 @@ module.exports = {
registerNode(context, node, name, 'require', ranks, imported)
},
'Program:exit': function reportAndReset() {
makeOutOfOrderReport(context, imported)
const alphabetize = (options['sort'] === 'alphabetical')
let [forwardComparator, reverseComparator] = determineComparators(alphabetize)
makeOutOfOrderReport(context, imported, forwardComparator, reverseComparator)

if (newlinesBetweenImports !== 'ignore') {
makeNewlinesBetweenReport(context, imported, newlinesBetweenImports)
Expand Down
130 changes: 129 additions & 1 deletion tests/src/rules/order.js
Expand Up @@ -376,6 +376,47 @@ ruleTester.run('order', rule, {
`,
options: [{ 'newlines-between': 'always' }]
}),
// Alphabetical order
test({
code: `
import fs from 'fs';
import path from 'path';
`,
options: [{ 'sort': 'alphabetical' }],
}),
// Alphabetical order with duplicate import
test({
code: `
import fs from 'fs';
import fs from 'fs';
`,
options: [{ 'sort': 'alphabetical' }],
}),
// Alphabetical order with multiple groups
test({
code: `
import fs from 'fs';
import path from 'path';
import async from 'async';
`,
options: [{ 'sort': 'alphabetical' }],
}),
// Ignore alphabetical order
test({
code: `
import path from 'path';
import fs from 'fs';
`,
options: [{ 'sort': 'ignore' }],
}),
// Ignore alphabetical order across groups
test({
code: `
import fs from 'fs';
import async from 'async';
`,
options: [{ 'sort': 'alphabetical' }],
}),
],
invalid: [
// builtin before external module (require)
Expand Down Expand Up @@ -456,7 +497,7 @@ ruleTester.run('order', rule, {
message: '`async` import should occur before import of `./sibling`',
}, {
ruleId: 'order',
message: '`fs` import should occur before import of `./sibling`',
message: '`fs` import should occur before import of `async`',
}],
}),
// Uses 'after' wording if it creates less errors
Expand Down Expand Up @@ -760,5 +801,92 @@ ruleTester.run('order', rule, {
},
],
}),
// Bad alphabetical order
test({
code: `
import path from 'path';
import fs from 'fs';
`,
options: [{ 'sort': 'alphabetical' }],
errors: [
{
ruleId: 'order',
message: '`fs` import should occur before import of `path`',
},
],
}),
// Bad alphabetical order with duplicate import
test({
code: `
import fs from 'fs';
import path from 'path';
import fs from 'fs';
`,
options: [{ 'sort': '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': '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': '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': 'alphabetical' }],
errors: [
{
ruleId: 'order',
message: '`path` import should occur before import of `async`',
},
{
ruleId: 'order',
message: '`fs` import should occur before import of `path`',
},
],
}),
],
})

0 comments on commit ba90260

Please sign in to comment.