Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add import-order rule #247

Merged
merged 5 commits into from
Apr 25, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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