Skip to content

Commit

Permalink
Merge pull request #247 from jfmengels/import-order
Browse files Browse the repository at this point in the history
Add `order` rule
  • Loading branch information
jfmengels committed Apr 25, 2016
2 parents 9e5ea9b + 38f3935 commit 055201e
Show file tree
Hide file tree
Showing 10 changed files with 715 additions and 25 deletions.
6 changes: 1 addition & 5 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel
- add [`no-extraneous-dependencies`] rule ([#241], thanks [@jfmengels])
- add [`extensions`] rule ([#250], thanks [@lo1tuma])
- add [`no-nodejs-modules`] rule ([#261], thanks [@jfmengels])
- add [`order`] rule ([#247], thanks [@jfmengels])
- consider `resolve.fallback` config option in the webpack resolver ([#254])

### Changed
Expand Down Expand Up @@ -155,9 +156,6 @@ for info on changes for earlier releases.
[`imports-first`]: ./docs/rules/imports-first.md
[`no-nodejs-modules`]: ./docs/rules/no-nodejs-modules.md

[#256]: https://github.com/benmosher/eslint-plugin-import/pull/256
[#254]: https://github.com/benmosher/eslint-plugin-import/pull/254
[#250]: https://github.com/benmosher/eslint-plugin-import/pull/250
[#243]: https://github.com/benmosher/eslint-plugin-import/pull/243
[#241]: https://github.com/benmosher/eslint-plugin-import/pull/241
[#239]: https://github.com/benmosher/eslint-plugin-import/pull/239
Expand Down Expand Up @@ -198,5 +196,3 @@ for info on changes for earlier releases.
[@singles]: https://github.com/singles
[@jfmengels]: https://github.com/jfmengels
[@dmnd]: https://github.com/dmnd
[@lo1tuma]: https://github.com/lo1tuma
[@lemonmade]: https://github.com/lemonmade
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,13 @@ This plugin intends to support linting of ES2015+ (ES6+) import/export syntax, a
* Report repeated import of the same module in multiple places ([`no-duplicates`])
* Report namespace imports ([`no-namespace`])
* Ensure consistent use of file extension within the import path ([`extensions`])
* Enforce a convention in module import order ([`order`])

[`imports-first`]: ./docs/rules/imports-first.md
[`no-duplicates`]: ./docs/rules/no-duplicates.md
[`no-namespace`]: ./docs/rules/no-namespace.md
[`extensions`]: ./docs/rules/extensions.md
[`order`]: ./docs/rules/order.md


## Installation
Expand Down
91 changes: 91 additions & 0 deletions docs/rules/order.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
# Enforce a convention in module import order

Enforce a convention in the order of `require()` / `import` statements. The order is as shown in the following example:

```js
// 1. node "builtins"
import fs from 'fs';
import path from 'path';
// 2. "external" modules
import _ from 'lodash';
import chalk from 'chalk';
// 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';
// 5. "sibling" modules from the same or a sibling's directory
import bar from './bar';
import baz from './bar/baz';
// 6. "index" of the current directory
import main from './';
```

Unassigned imports are ignored, as the order they are imported in may be important.

Statements using the ES6 `import` syntax must appear before any `require()` statements.


## Fail

```js
import _ from 'lodash';
import path from 'path'; // `path` import should occur before import of `lodash`

// -----

var _ = require('lodash');
var path = require('path'); // `path` import should occur before import of `lodash`

// -----

var path = require('path');
import foo from './foo'; // `import` statements must be before `require` statement
```


## Pass

```js
import path from 'path';
import _ from 'lodash';

// -----

var path = require('path');
var _ = require('lodash');

// -----

// Allowed as ̀`babel-register` is not assigned.
require('babel-register');
var path = require('path');

// -----

// Allowed as `import` must be before `require`
import foo from './foo';
var path = require('path');
```

## Options

This rule supports the following options:

`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": ["error", {"groups": ["index", "sibling", "parent", "internal", "external", "builtin"]}]
```
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@
"eslint-import-resolver-node": "^0.2.0",
"lodash.cond": "^4.3.0",
"lodash.endswith": "^4.0.1",
"lodash.find": "^4.3.0",
"object-assign": "^4.0.1",
"pkg-up": "^1.0.0"
}
Expand Down
15 changes: 10 additions & 5 deletions src/core/importType.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import cond from 'lodash.cond'
import builtinModules from 'builtin-modules'
import { basename, join } from 'path'
import { join } from 'path'

import resolve from './resolve'

Expand All @@ -18,7 +18,12 @@ function isExternalModule(name, path) {
return (!path || -1 < path.indexOf(join('node_modules', name)))
}

function isProjectModule(name, path) {
const scopedRegExp = /^@\w+\/\w+/
function isScoped(name) {
return scopedRegExp.test(name)
}

function isInternalModule(name, path) {
if (!externalModuleRegExp.test(name)) return false
return (path && -1 === path.indexOf(join('node_modules', name)))
}
Expand All @@ -28,8 +33,7 @@ function isRelativeToParent(name) {
}

const indexFiles = ['.', './', './index', './index.js']
function isIndex(name, path) {
if (path) return basename(path).split('.')[0] === 'index'
function isIndex(name) {
return indexFiles.indexOf(name) !== -1
}

Expand All @@ -40,7 +44,8 @@ function isRelativeToSibling(name) {
const typeTest = cond([
[isBuiltIn, constant('builtin')],
[isExternalModule, constant('external')],
[isProjectModule, constant('project')],
[isScoped, constant('external')],
[isInternalModule, constant('internal')],
[isRelativeToParent, constant('parent')],
[isIndex, constant('index')],
[isRelativeToSibling, constant('sibling')],
Expand Down
1 change: 1 addition & 0 deletions src/core/staticRequire.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// todo: merge with module visitor
export default function isStaticRequire(node) {
return node &&
node.callee &&
node.callee.type === 'Identifier' &&
node.callee.name === 'require' &&
node.arguments.length === 1 &&
Expand Down
1 change: 1 addition & 0 deletions src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ export const rules = {
'imports-first': require('./rules/imports-first'),
'no-extraneous-dependencies': require('./rules/no-extraneous-dependencies'),
'no-nodejs-modules': require('./rules/no-nodejs-modules'),
'order': require('./rules/order'),

// metadata-based
'no-deprecated': require('./rules/no-deprecated'),
Expand Down
175 changes: 175 additions & 0 deletions src/rules/order.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,175 @@
'use strict'

import find from 'lodash.find'
import importType from '../core/importType'
import isStaticRequire from '../core/staticRequire'

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

// REPORTING

function reverse(array) {
return array.map(function (v) {
return {
name: v.name,
rank: -v.rank,
node: v.node,
}
}).reverse()
}

function findOutOfOrder(imported) {
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) {
maxSeenRankNode = importedModule
}
return res
})
}

function report(context, imported, outOfOrder, order) {
outOfOrder.forEach(function (imp) {
const found = find(imported, function hasHigherRank(importedItem) {
return importedItem.rank > imp.rank
})
context.report(imp.node, '`' + imp.name + '` import should occur ' + order +
' import of `' + found.name + '`')
})
}

function makeReport(context, imported) {
const outOfOrder = findOutOfOrder(imported)
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)
if (reversedOrder.length < outOfOrder.length) {
report(context, reversedImported, reversedOrder, 'after')
return
}
report(context, imported, outOfOrder, 'before')
}

// DETECTING

function computeRank(context, ranks, name, type) {
return ranks[importType(name, context)] +
(type === 'import' ? 0 : 100)
}

function registerNode(context, node, name, type, ranks, imported) {
const rank = computeRank(context, ranks, name, type)
if (rank !== -1) {
imported.push({name, rank, node})
}
}

function isInVariableDeclarator(node) {
return 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] || {}
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

function incrementLevel() {
level++
}
function decrementLevel() {
level--
}

return {
ImportDeclaration: function handleImports(node) {
if (node.specifiers.length) { // Ignoring unassigned imports
const name = node.source.value
registerNode(context, node, name, 'import', 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, 'require', ranks, imported)
},
'Program:exit': function reportAndReset() {
makeReport(context, imported)
imported = []
},
FunctionDeclaration: incrementLevel,
FunctionExpression: incrementLevel,
ArrowFunctionExpression: incrementLevel,
BlockStatement: incrementLevel,
'FunctionDeclaration:exit': decrementLevel,
'FunctionExpression:exit': decrementLevel,
'ArrowFunctionExpression:exit': decrementLevel,
'BlockStatement:exit': decrementLevel,
}
}

module.exports.schema = [
{
type: 'object',
properties: {
groups: {
type: 'array',
},
},
additionalProperties: false,
},
]
Loading

0 comments on commit 055201e

Please sign in to comment.