diff --git a/CHANGELOG.md b/CHANGELOG.md index b2eab0a42..cbf13112c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel ### Added - [`newline-after-import`], new rule. ([#245], thanks [@singles]) - Added an `optionalDependencies` option to [`no-extraneous-dependencies`] to allow/forbid optional dependencies ([#266], thanks [@jfmengels]). +- Added `newlines-between` option to [`order`] rule ([#298], thanks [@singles]) ## resolvers/webpack/0.2.4 - 2016-04-29 ### Changed diff --git a/docs/rules/order.md b/docs/rules/order.md index ef5a95393..4f1b256e8 100644 --- a/docs/rules/order.md +++ b/docs/rules/order.md @@ -73,7 +73,9 @@ var path = require('path'); 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: +### `groups: [array]`: + +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 @@ -89,3 +91,53 @@ You can set the options like this: ```js "import/order": ["error", {"groups": ["index", "sibling", "parent", "internal", "external", "builtin"]}] ``` + +### `newlines-between: [always|never]`: + + +Enforces or forbids new lines between import groups: + +- If omitted, assertion messages will be neither enforced nor forbidden. +- If set to `always`, a new line between each group will be enforced, and new lines inside a group will be forbidden. +- If set to `never`, no new lines are allowed in the entire import section. + +With the default group setting, the following will be invalid: + +```js +/* eslint import/order: ["error", {"newlines-between": "always"}] */ +import fs from 'fs'; +import path from 'path'; +import index from './'; +import sibling from './foo'; +``` + +```js +/* eslint import/order: ["error", {"newlines-between": "never"}] */ +import fs from 'fs'; +import path from 'path'; + +import index from './'; + +import sibling from './foo'; +``` + +while those will be valid: + +```js +/* eslint import/order: ["error", {"newlines-between": "always"}] */ +import fs from 'fs'; +import path from 'path'; + +import index from './'; + +import sibling from './foo'; +``` + +```js +/* eslint import/order: ["error", {"newlines-between": "never"}] */ +import fs from 'fs'; +import path from 'path'; +import index from './'; +import sibling from './foo'; +``` + diff --git a/src/rules/order.js b/src/rules/order.js index 980c6534e..dffb6a5bd 100644 --- a/src/rules/order.js +++ b/src/rules/order.js @@ -32,7 +32,7 @@ function findOutOfOrder(imported) { }) } -function report(context, imported, outOfOrder, order) { +function reportOutOfOrder(context, imported, outOfOrder, order) { outOfOrder.forEach(function (imp) { const found = find(imported, function hasHigherRank(importedItem) { return importedItem.rank > imp.rank @@ -42,7 +42,7 @@ function report(context, imported, outOfOrder, order) { }) } -function makeReport(context, imported) { +function makeOutOfOrderReport(context, imported) { const outOfOrder = findOutOfOrder(imported) if (!outOfOrder.length) { return @@ -51,10 +51,10 @@ function makeReport(context, imported) { const reversedImported = reverse(imported) const reversedOrder = findOutOfOrder(reversedImported) if (reversedOrder.length < outOfOrder.length) { - report(context, reversedImported, reversedOrder, 'after') + reportOutOfOrder(context, reversedImported, reversedOrder, 'after') return } - report(context, imported, outOfOrder, 'before') + reportOutOfOrder(context, imported, outOfOrder, 'before') } // DETECTING @@ -109,6 +109,37 @@ function convertGroupsToRanks(groups) { }, rankObject) } +function makeNewlinesBetweenReport (context, imported, newlinesBetweenImports) { + const getLineDifference = (currentImport, previousImport) => { + return currentImport.node.loc.start.line - previousImport.node.loc.start.line + } + let previousImport = imported[0] + + imported.slice(1).forEach(function(currentImport) { + if (newlinesBetweenImports === 'always') { + if (currentImport.rank !== previousImport.rank + && getLineDifference(currentImport, previousImport) !== 2) + { + context.report( + previousImport.node, 'There should be one empty line between import groups' + ) + } else if (currentImport.rank === previousImport.rank + && getLineDifference(currentImport, previousImport) >= 2) + { + context.report( + previousImport.node, 'There should be no empty line within import group' + ) + } + } else { + if (getLineDifference(currentImport, previousImport) > 1) { + context.report(previousImport.node, 'There should be no empty line between import groups') + } + } + + previousImport = currentImport + }) +} + module.exports = function importOrderRule (context) { const options = context.options[0] || {} let ranks @@ -148,7 +179,12 @@ module.exports = function importOrderRule (context) { registerNode(context, node, name, 'require', ranks, imported) }, 'Program:exit': function reportAndReset() { - makeReport(context, imported) + makeOutOfOrderReport(context, imported) + + if ('newlines-between' in options) { + makeNewlinesBetweenReport(context, imported, options['newlines-between']) + } + imported = [] }, FunctionDeclaration: incrementLevel, @@ -169,6 +205,9 @@ module.exports.schema = [ groups: { type: 'array', }, + 'newlines-between': { + enum: [ 'always', 'never' ], + }, }, additionalProperties: false, }, diff --git a/tests/src/rules/order.js b/tests/src/rules/order.js index 645cddf0c..5310cac16 100644 --- a/tests/src/rules/order.js +++ b/tests/src/rules/order.js @@ -160,6 +160,52 @@ ruleTester.run('order', rule, { 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', + }, + ], + }), ], invalid: [ // builtin before external module (require) @@ -413,5 +459,130 @@ ruleTester.run('order', rule, { 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'); + `, + 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'); + `, + options: [ + { + groups: [ + ['builtin', 'index'], + ['sibling'], + ['parent', 'external'], + ], + 'newlines-between': 'always', + }, + ], + errors: [ + { + line: 4, + message: 'There should be one empty line between import groups', + }, + { + line: 5, + message: 'There should be one empty line between import groups', + }, + ], + }), + //Option newlines-between: 'always' should report too many empty lines between import groups + test({ + code: ` + var fs = require('fs'); + var index = require('./'); + + + + var sibling = require('./foo'); + var async = require('async'); + `, + options: [ + { + groups: [ + ['builtin', 'index'], + ['sibling', 'parent', 'external'] + ], + 'newlines-between': 'always', + }, + ], + errors: [ + { + line: 3, + message: 'There should be 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', + }, + ], + }), ], })