Skip to content

Commit

Permalink
Merge pull request #298 from singles/order-option-newlines-between
Browse files Browse the repository at this point in the history 
Add `newlines-between` option to `order` rule
  • Loading branch information
@benmosher
benmosher committed May 5, 2016
2 parents 73164f8 + c3d5ada commit f75d0af
Show file tree
Hide file tree
Showing 4 changed files with 269 additions and 6 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
54 changes: 53 additions & 1 deletion docs/rules/order.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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';
```

49 changes: 44 additions & 5 deletions src/rules/order.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -169,6 +205,9 @@ module.exports.schema = [
groups: {
type: 'array',
},
'newlines-between': {
enum: [ 'always', 'never' ],
},
},
additionalProperties: false,
},
Expand Down
171 changes: 171 additions & 0 deletions tests/src/rules/order.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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',
},
],
}),
],
})

0 comments on commit f75d0af

Please sign in to comment.