Add import-order rule

CHANGELOG.md

@@ -7,6 +7,7 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel
 ### Added
 - [`no-named-as-default-member`] to `warnings` canned config
 - add [`no-extraneous-dependencies`] rule ([#241], thanks [@jfmengels])
+- add [`import-order`] rule ([#247], thanks [@jfmengels])
 
 ## [1.5.0] - 2016-04-18
 ### Added

README.md

@@ -50,10 +50,12 @@ This plugin intends to support linting of ES2015+ (ES6+) import/export syntax, a
 * Ensure all imports appear before other statements ([`imports-first`])
 * Report repeated import of the same module in multiple places ([`no-duplicates`])
 * Report namespace imports ([`no-namespace`])
+* Enforce a convention in module import order ([`import-order`])
 
 [`imports-first`]: ./docs/rules/imports-first.md
 [`no-duplicates`]: ./docs/rules/no-duplicates.md
 [`no-namespace`]: ./docs/rules/no-namespace.md
+[`import-order`]: ./docs/rules/import-order.md
 
 
 ## Installation

docs/rules/import-order.md

@@ -0,0 +1,66 @@
+# 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. 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
+import bar from './bar';
+import baz from './bar/baz';
+// 5. "index" of the current directory
+import main from './';
+```
+
+Unassigned imports are not accounted for, as the order they are imported in may be important.
+
+
+## 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`
+```
+
+
+## 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');
+```
+
+## Options
+
+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.
+
+You can set the options like this:
+
+```js
+"import-order/import-order": ["error", {"order": ["index", "sibling", "parent", "external", "builtin"]}]
+```

package.json

@@ -72,6 +72,7 @@
     "es6-symbol": "*",
     "eslint-import-resolver-node": "^0.2.0",
     "lodash.cond": "^4.3.0",
+    "lodash.find": "^4.3.0",
     "object-assign": "^4.0.1",
     "pkg-up": "^1.0.0"
   }

src/core/importType.js

@@ -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'
 
@@ -28,8 +28,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
 }
 

src/core/staticRequire.js

@@ -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 &&

src/index.js

@@ -14,6 +14,7 @@ export const rules = {
   'no-duplicates': require('./rules/no-duplicates'),
   'imports-first': require('./rules/imports-first'),
   'no-extraneous-dependencies': require('./rules/no-extraneous-dependencies'),
+  'import-order': require('./rules/import-order'),
 
   // metadata-based
   'no-deprecated': require('./rules/no-deprecated'),

src/rules/import-order.js

@@ -0,0 +1,135 @@
+'use strict'
+
+import find from 'lodash.find'
+import importType from '../core/importType'
+import isStaticRequire from '../core/staticRequire'
+
+const defaultOrder = ['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, order, name) {
+  return order.indexOf(importType(name, context))
+}
+
+function registerNode(context, node, name, order, imported) {
+  const rank = computeRank(context, order, name)
+  if (rank !== -1) {
+    imported.push({name: name, rank: rank, node: node})
+  }
+}
+
+function isInVariableDeclarator(node) {
+  return node &&
+    (node.type === 'VariableDeclarator' || isInVariableDeclarator(node.parent))
+}
+
+module.exports = function importOrderRule (context) {
+  const options = context.options[0] || {}
+  const order = options.order || defaultOrder
+  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, order, 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)
+    },
+    '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: {
+      order: {
+        type: 'array',
+        uniqueItems: true,
+        length: 5,
+        items: {
+          enum: defaultOrder,
+        },
+      },
+    },
+    additionalProperties: false,
+  },
+]

tests/src/core/importType.js

@@ -36,21 +36,17 @@ describe('importType(name)', function () {
   it("should return 'sibling' for internal modules that are connected to one of the siblings", function() {
     expect(importType('./foo', context)).to.equal('sibling')
     expect(importType('./foo/bar', context)).to.equal('sibling')
+    expect(importType('./importType', context)).to.equal('sibling')
+    expect(importType('./importType/', context)).to.equal('sibling')
+    expect(importType('./importType/index', context)).to.equal('sibling')
+    expect(importType('./importType/index.js', context)).to.equal('sibling')
   })
 
-  describe("should return 'index' for sibling index file when", function() {
-    it("resolves", function() {
-      expect(importType('./importType', context)).to.equal('index')
-      expect(importType('./importType/', context)).to.equal('index')
-      expect(importType('./importType/index', context)).to.equal('index')
-      expect(importType('./importType/index.js', context)).to.equal('index')
-    })
-    it("doesn't resolve", function() {
-      expect(importType('.', context)).to.equal('index')
-      expect(importType('./', context)).to.equal('index')
-      expect(importType('./index', context)).to.equal('index')
-      expect(importType('./index.js', context)).to.equal('index')
-    })
+  describe("should return 'index' for sibling index file", function() {
+    expect(importType('.', context)).to.equal('index')
+    expect(importType('./', context)).to.equal('index')
+    expect(importType('./index', context)).to.equal('index')
+    expect(importType('./index.js', context)).to.equal('index')
   })
 
   it("should return 'unknown' for any unhandled cases", function() {