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.

Sign up for GitHub

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

Jump to bottom

Add newlines-between option to order rule #298

Merged
merged 1 commit into from
May 5, 2016
+269 −6
Merged

Add newlines-between option to order rule #298

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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',
},
],
}),
],
})