diff --git a/package.json b/package.json index f9bc620..7eac2b6 100644 --- a/package.json +++ b/package.json @@ -39,7 +39,7 @@ "babel-plugin-istanbul": "^2.0.1", "babel-preset-es2015-argon": "latest", "babel-register": "6.16.3", - "eslint": "3.x", + "eslint": "4.x", "eslint-import-resolver-node": "file:./resolvers/node", "eslint-import-resolver-webpack": "file:./resolvers/webpack", "eslint-module-utils": "file:./utils", @@ -52,12 +52,12 @@ "nyc": "^8.3.0" }, "peerDependencies": { - "eslint": "2.x - 3.x" + "eslint": "2.x - 4.x" }, "dependencies": { "builtin-modules": "^1.1.1", - "eslint-import-resolver-node": "^0.2.0", - "eslint-module-utils": "^2.0.0", + "eslint-import-resolver-node": "^0.3.1", + "eslint-module-utils": "^2.1.1", "gulp-watch": "^4.3.11", "jscodeshift": "^0.3.30", "lodash.cond": "^4.3.0" diff --git a/src/rules/order.js b/src/rules/order.js index 0e8b3c7..860a00e 100644 --- a/src/rules/order.js +++ b/src/rules/order.js @@ -1,5 +1,4 @@ 'use strict' - require('./../core/add-types'); import { EOL } from 'os'; @@ -7,14 +6,6 @@ import importType from '../core/importType' import isStaticRequire from '../core/staticRequire' import parser from 'babel-eslint'; -import jscodeshift from 'jscodeshift'; -const j = jscodeshift.withParser(parser); - -jscodeshift.types.Type.def('ExperimentalSpreadProperty').bases('Node'); -jscodeshift.types.Type.def('ExperimentalRestProperty').bases('Node'); - -jscodeshift.types.finalize(); - const defaultGroups = ['builtin', 'external', 'parent', 'sibling', 'index'] // REPORTING @@ -44,54 +35,46 @@ function findOutOfOrder(imported) { } -function findRootNode(j, root, node) { - let result = null; - - root - .find(j.Node) - .filter((p) => p.node === node).forEach(p => { - let parent = p; - - while (parent.parent != null && parent.parent.value.body == null) { - parent = parent.parent; - } +function findRootNode(node) { + let result = node; - result = parent; - }); + while (result.parent != null && result.parent.body == null) { + result = result.parent; + } return result; } function fixOutOfOrder(context, firstNode, secondNode, order) { const sourceCode = context.getSourceCode(); - const root = j(sourceCode.ast); - const firstRoot = findRootNode(j, root, firstNode.node); - const secondRoot = findRootNode(j, root, secondNode.node); - const newCode = sourceCode.getText(secondRoot.node); + const firstRoot = findRootNode(firstNode.node); + const secondRoot = findRootNode(secondNode.node); + const beforeToken = sourceCode.getTokenBefore(secondRoot); + const newCode = sourceCode.getText(secondRoot); - const msg = () => `\`${secondNode.name}\` import should occur ${order}` + - ` import \`${firstNode.name}\``; + const msg = () => '`' + secondNode.name + '` import should occur ' + order + + ' import of `' + firstNode.name + '`'; if (order === 'before') { context.report({ node: secondNode.node, message: msg(), - fix: fixer => fixer.insertTextBefore(firstRoot.node, newCode) + fix: fixer => [ + fixer.remove(secondRoot), + fixer.insertTextBefore(firstRoot, newCode), + ], }); } else if (order === 'after') { context.report({ node: secondNode.node, message: msg(), - fix: fixer => fixer.insertTextAfter(firstRoot.node, newCode) + fix: fixer => [ + fixer.remove(secondRoot), + fixer.insertTextAfter(firstRoot, newCode), + ] }); } - - context.report({ - node: secondNode.node, - message: msg(), - fix: fixer => fixer.remove(secondRoot.node) - }); } function reportOutOfOrder(context, imported, outOfOrder, order) { @@ -171,20 +154,16 @@ function convertGroupsToRanks(groups) { } function fixNewLineAfterImport(context, previousImport) { - const root = j(context.getSourceCode().ast); - - const prevRoot = findRootNode(j, root, previousImport.node); + const prevRoot = findRootNode(previousImport.node); - return (fixer) => fixer.insertTextAfter(prevRoot.node, EOL); + return (fixer) => fixer.insertTextAfter(prevRoot, EOL); } function removeNewLineAfterImport(context, currentImport, previousImport) { - const root = j(context.getSourceCode().ast); + const prevRoot = findRootNode(previousImport.node); + const currRoot = findRootNode(currentImport.node); - const prevRoot = findRootNode(j, root, previousImport.node); - const currRoot = findRootNode(j, root, currentImport.node); - - return (fixer) => fixer.removeRange([prevRoot.node.range[1] + 1, currRoot.node.range[0]]); + return (fixer) => fixer.removeRange([prevRoot.range[1] + 1, currRoot.range[0]]); } function makeNewlinesBetweenReport (context, imported, newlinesBetweenImports) { @@ -196,26 +175,29 @@ function makeNewlinesBetweenReport (context, imported, newlinesBetweenImports) { return linesBetweenImports.filter((line) => !line.trim().length).length } - let previousImport = imported[0] imported.slice(1).forEach(function(currentImport) { - const emptyLinesCount = getNumberOfEmptyLinesBetween(currentImport, previousImport); - if (newlinesBetweenImports === 'always') { - if (currentImport.rank !== previousImport.rank && emptyLinesCount === 0) { + const emptyLinesBetween = getNumberOfEmptyLinesBetween(currentImport, previousImport) + + if (newlinesBetweenImports === 'always' + || newlinesBetweenImports === 'always-and-inside-groups') { + if (currentImport.rank !== previousImport.rank && emptyLinesBetween === 0) { context.report({ node: previousImport.node, message: 'There should be at least one empty line between import groups', fix: fixNewLineAfterImport(context, previousImport) }); - } else if (currentImport.rank === previousImport.rank && emptyLinesCount > 0) { + } else if (currentImport.rank === previousImport.rank + && emptyLinesBetween > 0 + && newlinesBetweenImports !== 'always-and-inside-groups') { context.report({ node: previousImport.node, message: 'There should be no empty line within import group', fix: removeNewLineAfterImport(context, currentImport, previousImport) }); } - } else if (emptyLinesCount > 0) { + } else if (emptyLinesBetween > 0) { context.report({ node: previousImport.node, message: 'There should be no empty line between import groups', @@ -239,7 +221,7 @@ module.exports = { type: 'array', }, 'newlines-between': { - enum: [ 'ignore', 'always', 'never' ], + enum: [ 'ignore', 'always', 'never', 'always-and-inside-groups' ], }, }, additionalProperties: false, diff --git a/tests/rules/order.test.js b/tests/rules/order.test.js new file mode 100644 index 0000000..7fa8ec7 --- /dev/null +++ b/tests/rules/order.test.js @@ -0,0 +1,818 @@ +import { EOL } from 'os'; + +function test(t) { + return Object.assign({ + filename: 'order.test.js', + parserOptions: { + sourceType: 'module', + ecmaVersion: 6, + }, + }, t); +} + +import { RuleTester } from 'eslint' + +const ruleTester = new RuleTester() + , rule = require('../../src/rules/order') + +ruleTester.run('order', rule, { + valid: [ + // Default order using require + test({ + code: ` + var fs = require('fs'); + var async = require('async'); + var relParent1 = require('../foo'); + var relParent2 = require('../foo/bar'); + var relParent3 = require('../'); + var sibling = require('./foo'); + var index = require('./');`, + }), + // Default order using import + test({ + code: ` + import fs from 'fs'; + import async, {foo1} from 'async'; + import relParent1 from '../foo'; + import relParent2, {foo2} from '../foo/bar'; + import relParent3 from '../'; + import sibling, {foo3} from './foo'; + import index from './';`, + }), + // Multiple module of the same rank next to each other + test({ + code: ` + var fs = require('fs'); + var fs = require('fs'); + var path = require('path'); + var _ = require('lodash'); + var async = require('async');`, + }), + // Overriding order to be the reverse of the default order + test({ + code: ` + var index = require('./'); + var sibling = require('./foo'); + var relParent3 = require('../'); + var relParent2 = require('../foo/bar'); + var relParent1 = require('../foo'); + var async = require('async'); + var fs = require('fs'); + `, + options: [{groups: ['index', 'sibling', 'parent', 'external', 'builtin']}], + }), + // Ignore dynamic requires + test({ + code: ` + var path = require('path'); + var _ = require('lodash'); + var async = require('async'); + var fs = require('f' + 's');`, + }), + // Ignore non-require call expressions + test({ + code: ` + var path = require('path'); + var result = add(1, 2); + var _ = require('lodash');`, + }), + // Ignore requires that are not at the top-level + test({ + code: ` + var index = require('./'); + function foo() { + var fs = require('fs'); + } + () => require('fs'); + if (a) { + require('fs'); + }`, + }), + // Ignore unknown/invalid cases + test({ + code: ` + var unknown1 = require('/unknown1'); + var fs = require('fs'); + var unknown2 = require('/unknown2'); + var async = require('async'); + var unknown3 = require('/unknown3'); + var foo = require('../foo'); + var unknown4 = require('/unknown4'); + var bar = require('../foo/bar'); + var unknown5 = require('/unknown5'); + var parent = require('../'); + var unknown6 = require('/unknown6'); + var foo = require('./foo'); + var unknown7 = require('/unknown7'); + var index = require('./'); + var unknown8 = require('/unknown8'); + `}), + // Ignoring unassigned values by default (require) + test({ + code: ` + require('./foo'); + require('fs'); + var path = require('path'); + `}), + // Ignoring unassigned values by default (import) + test({ + code: ` + import './foo'; + import 'fs'; + import path from 'path'; + `}), + // No imports + test({ + code: ` + function add(a, b) { + return a + b; + } + 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' + ]}], + }), + // Mixing require and import should have import up top + test({ + code: ` + import async, {foo1} from 'async'; + import relParent2, {foo2} from '../foo/bar'; + import sibling, {foo3} from './foo'; + var fs = require('fs'); + var relParent1 = require('../foo'); + var relParent3 = require('../'); + var index = require('./'); + `, + }), + // Option: newlines-between: 'always' + test({ + code: ` + var fs = require('fs'); + var index = require('./'); + var path = require('path'); + + + + var sibling = require('./foo'); + + + var relParent1 = require('../foo'); + var relParent3 = require('../'); + var async = require('async'); + `, + options: [ + { + groups: [ + ['builtin', 'index'], + ['sibling'], + ['parent', 'external'], + ], + 'newlines-between': 'always', + }, + ], + }), + // Option: newlines-between: 'never' + test({ + code: ` + var fs = require('fs'); + var index = require('./'); + var path = require('path'); + var sibling = require('./foo'); + var relParent1 = require('../foo'); + var relParent3 = require('../'); + var async = require('async'); + `, + options: [ + { + groups: [ + ['builtin', 'index'], + ['sibling'], + ['parent', 'external'], + ], + 'newlines-between': 'never', + }, + ], + }), + // Option: newlines-between: 'ignore' + test({ + code: ` + var fs = require('fs'); + + var index = require('./'); + var path = require('path'); + var sibling = require('./foo'); + + + var relParent1 = require('../foo'); + + var relParent3 = require('../'); + var async = require('async'); + `, + options: [ + { + groups: [ + ['builtin', 'index'], + ['sibling'], + ['parent', 'external'], + ], + 'newlines-between': 'ignore', + }, + ], + }), + // 'ignore' should be the default value for `newlines-between` + test({ + code: ` + var fs = require('fs'); + + var index = require('./'); + var path = require('path'); + var sibling = require('./foo'); + + + var relParent1 = require('../foo'); + + var relParent3 = require('../'); + + var async = require('async'); + `, + options: [ + { + groups: [ + ['builtin', 'index'], + ['sibling'], + ['parent', 'external'], + ], + }, + ], + }), + // Option newlines-between: 'always' with multiline imports #1 + test({ + code: ` + import path from 'path'; + + import { + I, + Want, + Couple, + Imports, + Here + } from 'bar'; + import external from 'external' + `, + options: [{ 'newlines-between': 'always' }] + }), + // Option newlines-between: 'always' with multiline imports #2 + test({ + code: ` + import path from 'path'; + import net + from 'net'; + + import external from 'external' + `, + options: [{ 'newlines-between': 'always' }] + }), + // Option newlines-between: 'always' with multiline imports #3 + test({ + code: ` + import foo + from '../../../../this/will/be/very/long/path/and/therefore/this/import/has/to/be/in/two/lines'; + + import bar + from './sibling'; + `, + options: [{ 'newlines-between': 'always' }] + }), + // Option newlines-between: 'always' with not assigned import #1 + test({ + code: ` + import path from 'path'; + + import 'loud-rejection'; + import 'something-else'; + + import _ from 'lodash'; + `, + options: [{ 'newlines-between': 'always' }] + }), + // Option newlines-between: 'never' with not assigned import #2 + test({ + code: ` + import path from 'path'; + import 'loud-rejection'; + import 'something-else'; + import _ from 'lodash'; + `, + options: [{ 'newlines-between': 'never' }] + }), + // Option newlines-between: 'always' with not assigned require #1 + test({ + code: ` + var path = require('path'); + + require('loud-rejection'); + require('something-else'); + + var _ = require('lodash'); + `, + options: [{ 'newlines-between': 'always' }] + }), + // Option newlines-between: 'never' with not assigned require #2 + test({ + code: ` + var path = require('path'); + require('loud-rejection'); + require('something-else'); + var _ = require('lodash'); + `, + options: [{ 'newlines-between': 'never' }] + }), + // Option newlines-between: 'never' should ignore nested require statement's #1 + test({ + code: ` + var some = require('asdas'); + var config = { + port: 4444, + runner: { + server_path: require('runner-binary').path, + + cli_args: { + 'webdriver.chrome.driver': require('browser-binary').path + } + } + } + `, + options: [{ 'newlines-between': 'never' }] + }), + // Option newlines-between: 'always' should ignore nested require statement's #2 + test({ + code: ` + var some = require('asdas'); + var config = { + port: 4444, + runner: { + server_path: require('runner-binary').path, + cli_args: { + 'webdriver.chrome.driver': require('browser-binary').path + } + } + } + `, + options: [{ 'newlines-between': 'always' }] + }), + // Option: newlines-between: 'always-and-inside-groups' + test({ + code: ` + var fs = require('fs'); + var path = require('path'); + + var util = require('util'); + + var async = require('async'); + + var relParent1 = require('../foo'); + var relParent2 = require('../'); + + var relParent3 = require('../bar'); + + var sibling = require('./foo'); + var sibling2 = require('./bar'); + + var sibling3 = require('./foobar'); + `, + options: [ + { + 'newlines-between': 'always-and-inside-groups', + }, + ], + }), + ], + invalid: [ + // builtin before external module (require) + test({ + code: ` + var async = require('async'); + var fs = require('fs'); + `, + errors: [{ + ruleId: 'order', + message: '`fs` import should occur before import of `async`', + }], + }), + // builtin before external module (import) + test({ + code: ` + import async from 'async'; + import fs from 'fs'; + `, + errors: [{ + ruleId: 'order', + message: '`fs` import should occur before import of `async`', + }], + }), + // builtin before external module (mixed import and require) + test({ + code: ` + var async = require('async'); + import fs from 'fs'; + `, + errors: [{ + ruleId: 'order', + message: '`fs` import should occur before import of `async`', + }], + }), + // external before parent + test({ + code: ` + var parent = require('../parent'); + var async = require('async'); + `, + errors: [{ + ruleId: 'order', + message: '`async` import should occur before import of `../parent`', + }], + }), + // parent before sibling + test({ + code: ` + var sibling = require('./sibling'); + var parent = require('../parent'); + `, + errors: [{ + ruleId: 'order', + message: '`../parent` import should occur before import of `./sibling`', + }], + }), + // sibling before index + test({ + code: ` + var index = require('./'); + var sibling = require('./sibling'); + `, + errors: [{ + ruleId: 'order', + message: '`./sibling` import should occur before import of `./`', + }], + }), + // Multiple errors + test({ + code: ` + var sibling = require('./sibling'); + var async = require('async'); + var fs = require('fs'); + `, + errors: [{ + ruleId: 'order', + message: '`async` import should occur before import of `./sibling`', + }, { + ruleId: 'order', + message: '`fs` import should occur before import of `./sibling`', + }], + }), + // Uses 'after' wording if it creates less errors + test({ + code: ` + var index = require('./'); + var fs = require('fs'); + var path = require('path'); + var _ = require('lodash'); + var foo = require('foo'); + var bar = require('bar'); + `, + errors: [{ + ruleId: 'order', + message: '`./` import should occur after import of `bar`', + }], + }), + // Overriding order to be the reverse of the default order + test({ + code: ` + var fs = require('fs'); + var index = require('./'); + `, + options: [{groups: ['index', 'sibling', 'parent', 'external', 'builtin']}], + errors: [{ + ruleId: 'order', + message: '`./` import should occur before import of `fs`', + }], + }), + // member expression of require + test({ + code: ` + var foo = require('./foo').bar; + var fs = require('fs'); + `, + errors: [{ + ruleId: 'order', + message: '`fs` import should occur before import of `./foo`', + }], + }), + // nested member expression of require + test({ + code: ` + var foo = require('./foo').bar.bar.bar; + var fs = require('fs'); + `, + errors: [{ + ruleId: 'order', + 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: '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: '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: '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: '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: '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: 'order', + message: 'Incorrect configuration of the rule: `parent` is duplicated', + }], + }), + // Mixing require and import should have import up top + test({ + code: ` + import async, {foo1} from 'async'; + import relParent2, {foo2} from '../foo/bar'; + var fs = require('fs'); + var relParent1 = require('../foo'); + var relParent3 = require('../'); + import sibling, {foo3} from './foo'; + var index = require('./'); + `, + errors: [{ + ruleId: 'order', + message: '`./foo` import should occur before import of `fs`', + }], + }), + test({ + code: ` + var fs = require('fs'); + import async, {foo1} from 'async'; + import relParent2, {foo2} from '../foo/bar'; + `, + errors: [{ + ruleId: 'order', + message: '`fs` import should occur after import of `../foo/bar`', + }], + }), + // Option newlines-between: 'never' - should report unnecessary line between groups + test({ + code: ` + var fs = require('fs'); + var index = require('./'); + var path = require('path'); + + var sibling = require('./foo'); + + var relParent1 = require('../foo'); + var relParent3 = require('../'); + var async = require('async'); + `, + output: ` + var fs = require('fs'); + var index = require('./'); + var path = require('path'); +var sibling = require('./foo'); +var relParent1 = require('../foo'); + var relParent3 = require('../'); + var async = require('async'); + `, + options: [ + { + groups: [ + ['builtin', 'index'], + ['sibling'], + ['parent', 'external'], + ], + 'newlines-between': 'never', + }, + ], + errors: [ + { + line: 4, + message: 'There should be no empty line between import groups', + }, + { + line: 6, + message: 'There should be no empty line between import groups', + }, + ], + }), + // Option newlines-between: 'always' - should report lack of newline between groups + test({ + code: ` + var fs = require('fs'); + var index = require('./'); + var path = require('path'); + var sibling = require('./foo'); + var relParent1 = require('../foo'); + var relParent3 = require('../'); + var async = require('async'); + `, + output: ` + var fs = require('fs'); + var index = require('./'); + var path = require('path');${EOL} + var sibling = require('./foo');${EOL} + var relParent1 = require('../foo'); + var relParent3 = require('../'); + var async = require('async'); + `, + options: [ + { + groups: [ + ['builtin', 'index'], + ['sibling'], + ['parent', 'external'], + ], + 'newlines-between': 'always', + }, + ], + errors: [ + { + line: 4, + message: 'There should be at least one empty line between import groups', + }, + { + line: 5, + message: 'There should be at least one empty line between import groups', + }, + ], + }), + //Option newlines-between: 'always' should report unnecessary empty lines space between import groups + test({ + code: ` + var fs = require('fs'); + + var path = require('path'); + var index = require('./'); + + var sibling = require('./foo'); + + var async = require('async'); + `, + options: [ + { + groups: [ + ['builtin', 'index'], + ['sibling', 'parent', 'external'] + ], + 'newlines-between': 'always', + }, + ], + errors: [ + { + line: 2, + message: 'There should be no empty line within import group', + }, + { + line: 7, + message: 'There should be no empty line within import group', + }, + ], + }), + // Option newlines-between: 'never' should report unnecessary empty lines when using not assigned imports + test({ + code: ` + import path from 'path'; + import 'loud-rejection'; + + import 'something-else'; + import _ from 'lodash'; + `, + options: [{ 'newlines-between': 'never' }], + errors: [ + { + line: 2, + message: 'There should be no empty line between import groups', + }, + ], + }), + // Option newlines-between: 'always' should report missing empty lines when using not assigned imports + test({ + code: ` + import path from 'path'; + import 'loud-rejection'; + import 'something-else'; + import _ from 'lodash'; + `, + options: [{ 'newlines-between': 'always' }], + errors: [ + { + line: 2, + message: 'There should be at least one empty line between import groups', + }, + ], + }), + ], +}) \ No newline at end of file