Skip to content

Commit

Permalink
Allow grouping in import-order, rename project type to internal
Browse files Browse the repository at this point in the history
  • Loading branch information
jfmengels committed Apr 19, 2016
1 parent a8bf7a0 commit 125e884
Show file tree
Hide file tree
Showing 5 changed files with 199 additions and 25 deletions.
22 changes: 17 additions & 5 deletions docs/rules/import-order.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,16 @@ import path from 'path';
// 2. "external" modules
import _ from 'lodash';
import chalk from 'chalk';
// 3. modules from a "parent" directory
// 3. "internal" modules
// (if you have configured your path or webpack to handle your internal paths differently)
import foo from 'src/foo';
// 4. modules from a "parent" directory
import foo from '../foo';
import qux from '../../foo/qux';
// 4. "sibling" modules from the same or a sibling's directory
// 5. "sibling" modules from the same or a sibling's directory
import bar from './bar';
import baz from './bar/baz';
// 5. "index" of the current directory
// 6. "index" of the current directory
import main from './';
```

Expand Down Expand Up @@ -57,10 +60,19 @@ var path = require('path');

This rule supports the following options:

`order`: The order to respect. It needs to contain only and all of the following elements: `"builtin", "external", "parent", "sibling", "index"`, which is the default value.
`groups`: How groups are defined, and the order to respect. `groups` must be an array of `string` or [`string`]. The only allowed `string`s are: `"builtin"`, `"external"`, `"internal"`, `"parent"`, `"sibling"`, `"index"`. The enforced order is the same as the order of each element in a group. Omitted types are implicitly grouped together as the last element. Example:
```js
[
'builtin', // Built-in types are first
['sibling', 'parent'], // Then sibling and parent types. They can be mingled together
'index', // Then the index file
// Then the rest: internal and external type
]
```
The default value is `["builtin", "external", "internal", "parent", "sibling", "index"]`.

You can set the options like this:

```js
"import-order/import-order": ["error", {"order": ["index", "sibling", "parent", "external", "builtin"]}]
"import-order/import-order": ["error", {"groups": ["index", "sibling", "parent", "internal", "external", "builtin"]}]
```
4 changes: 2 additions & 2 deletions src/core/importType.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ function isExternalModule(name, path) {
return (!path || -1 < path.indexOf(join('node_modules', name)))
}

function isProjectModule(name, path) {
function isInternalModule(name, path) {
if (!externalModuleRegExp.test(name)) return false
return (path && -1 === path.indexOf(join('node_modules', name)))
}
Expand All @@ -39,7 +39,7 @@ function isRelativeToSibling(name) {
const typeTest = cond([
[isBuiltIn, constant('builtin')],
[isExternalModule, constant('external')],
[isProjectModule, constant('project')],
[isInternalModule, constant('internal')],
[isRelativeToParent, constant('parent')],
[isIndex, constant('index')],
[isRelativeToSibling, constant('sibling')],
Expand Down
67 changes: 53 additions & 14 deletions src/rules/import-order.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import find from 'lodash.find'
import importType from '../core/importType'
import isStaticRequire from '../core/staticRequire'

const defaultOrder = ['builtin', 'external', 'parent', 'sibling', 'index']
const defaultGroups = ['builtin', 'external', 'parent', 'sibling', 'index']

// REPORTING

Expand Down Expand Up @@ -59,12 +59,12 @@ function makeReport(context, imported) {

// DETECTING

function computeRank(context, order, name) {
return order.indexOf(importType(name, context))
function computeRank(context, ranks, name) {
return ranks[importType(name, context)]
}

function registerNode(context, node, name, order, imported) {
const rank = computeRank(context, order, name)
function registerNode(context, node, name, ranks, imported) {
const rank = computeRank(context, ranks, name)
if (rank !== -1) {
imported.push({name: name, rank: rank, node: node})
}
Expand All @@ -75,9 +75,53 @@ function isInVariableDeclarator(node) {
(node.type === 'VariableDeclarator' || isInVariableDeclarator(node.parent))
}

const types = ['builtin', 'external', 'internal', 'parent', 'sibling', 'index']

// Creates an object with type-rank pairs.
// Example: { index: 0, sibling: 1, parent: 1, external: 1, builtin: 2, internal: 2 }
// Will throw an error if it contains a type that does not exist, or has a duplicate
function convertGroupsToRanks(groups) {
const rankObject = groups.reduce(function(res, group, index) {
if (typeof group === 'string') {
group = [group]
}
group.forEach(function(groupItem) {
if (types.indexOf(groupItem) === -1) {
throw new Error('Incorrect configuration of the rule: Unknown type `' +
JSON.stringify(groupItem) + '`')
}
if (res[groupItem] !== undefined) {
throw new Error('Incorrect configuration of the rule: `' + groupItem + '` is duplicated')
}
res[groupItem] = index
})
return res
}, {})

const omittedTypes = types.filter(function(type) {
return rankObject[type] === undefined
})

return omittedTypes.reduce(function(res, type) {
res[type] = groups.length
return res
}, rankObject)
}

module.exports = function importOrderRule (context) {
const options = context.options[0] || {}
const order = options.order || defaultOrder
let ranks

try {
ranks = convertGroupsToRanks(options.groups || defaultGroups)
} catch (error) {
// Malformed configuration
return {
Program: function(node) {
context.report(node, error.message)
},
}
}
let imported = []
let level = 0

Expand All @@ -92,15 +136,15 @@ module.exports = function importOrderRule (context) {
ImportDeclaration: function handleImports(node) {
if (node.specifiers.length) { // Ignoring unassigned imports
const name = node.source.value
registerNode(context, node, name, order, imported)
registerNode(context, node, name, ranks, imported)
}
},
CallExpression: function handleRequires(node) {
if (level !== 0 || !isStaticRequire(node) || !isInVariableDeclarator(node.parent)) {
return
}
const name = node.arguments[0].value
registerNode(context, node, name, order, imported)
registerNode(context, node, name, ranks, imported)
},
'Program:exit': function reportAndReset() {
makeReport(context, imported)
Expand All @@ -121,13 +165,8 @@ module.exports.schema = [
{
type: 'object',
properties: {
order: {
groups: {
type: 'array',
uniqueItems: true,
length: 5,
items: {
enum: defaultOrder,
},
},
},
additionalProperties: false,
Expand Down
4 changes: 2 additions & 2 deletions tests/src/core/importType.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,9 @@ describe('importType(name)', function () {
expect(importType('lodash/fp', context)).to.equal('external')
})

it("should return 'project' for non-builtins resolved outside of node_modules", function () {
it("should return 'internal' for non-builtins resolved outside of node_modules", function () {
const pathContext = testContext({ "import/resolver": { node: { paths: [ path.join(__dirname, '..', '..', 'files') ] } } })
expect(importType('importType', pathContext)).to.equal('project')
expect(importType('importType', pathContext)).to.equal('internal')
})

it("should return 'parent' for internal modules that go through the parent", function() {
Expand Down
127 changes: 125 additions & 2 deletions tests/src/rules/import-order.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ ruleTester.run('import-order', rule, {
var async = require('async');
var fs = require('fs');
`,
options: [{order: ['index', 'sibling', 'parent', 'external', 'builtin']}],
options: [{groups: ['index', 'sibling', 'parent', 'external', 'builtin']}],
}),
// Ignore dynamic requires
test({
Expand Down Expand Up @@ -130,6 +130,35 @@ ruleTester.run('import-order', rule, {
}
var foo;
`}),
// Grouping import types
test({
code: `
var fs = require('fs');
var index = require('./');
var path = require('path');
var sibling = require('./foo');
var relParent3 = require('../');
var async = require('async');
var relParent1 = require('../foo');
`,
options: [{groups: [
['builtin', 'index'],
['sibling', 'parent', 'external'],
]}],
}),
// Omitted types should implicitly be considered as the last type
test({
code: `
var index = require('./');
var path = require('path');
`,
options: [{groups: [
'index',
['sibling', 'parent', 'external'],
// missing 'builtin'
]}],
}),
],
invalid: [
// builtin before external module (require)
Expand Down Expand Up @@ -234,7 +263,7 @@ ruleTester.run('import-order', rule, {
var fs = require('fs');
var index = require('./');
`,
options: [{order: ['index', 'sibling', 'parent', 'external', 'builtin']}],
options: [{groups: ['index', 'sibling', 'parent', 'external', 'builtin']}],
errors: [{
ruleId: 'import-order',
message: '`./` import should occur before import of `fs`',
Expand Down Expand Up @@ -262,5 +291,99 @@ ruleTester.run('import-order', rule, {
message: '`fs` import should occur before import of `./foo`',
}],
}),
// Grouping import types
test({
code: `
var fs = require('fs');
var index = require('./');
var sibling = require('./foo');
var path = require('path');
`,
options: [{groups: [
['builtin', 'index'],
['sibling', 'parent', 'external'],
]}],
errors: [{
ruleId: 'import-order',
message: '`path` import should occur before import of `./foo`',
}],
}),
// Omitted types should implicitly be considered as the last type
test({
code: `
var path = require('path');
var async = require('async');
`,
options: [{groups: [
'index',
['sibling', 'parent', 'external', 'internal'],
// missing 'builtin'
]}],
errors: [{
ruleId: 'import-order',
message: '`async` import should occur before import of `path`',
}],
}),
// Setting the order for an unknown type
// should make the rule trigger an error and do nothing else
test({
code: `
var async = require('async');
var index = require('./');
`,
options: [{groups: [
'index',
['sibling', 'parent', 'UNKNOWN', 'internal'],
]}],
errors: [{
ruleId: 'import-order',
message: 'Incorrect configuration of the rule: Unknown type `"UNKNOWN"`',
}],
}),
// Type in an array can't be another array, too much nesting
test({
code: `
var async = require('async');
var index = require('./');
`,
options: [{groups: [
'index',
['sibling', 'parent', ['builtin'], 'internal'],
]}],
errors: [{
ruleId: 'import-order',
message: 'Incorrect configuration of the rule: Unknown type `["builtin"]`',
}],
}),
// No numbers
test({
code: `
var async = require('async');
var index = require('./');
`,
options: [{groups: [
'index',
['sibling', 'parent', 2, 'internal'],
]}],
errors: [{
ruleId: 'import-order',
message: 'Incorrect configuration of the rule: Unknown type `2`',
}],
}),
// Duplicate
test({
code: `
var async = require('async');
var index = require('./');
`,
options: [{groups: [
'index',
['sibling', 'parent', 'parent', 'internal'],
]}],
errors: [{
ruleId: 'import-order',
message: 'Incorrect configuration of the rule: `parent` is duplicated',
}],
}),
],
})

0 comments on commit 125e884

Please sign in to comment.