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.

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

Add count option to newline-after-import #742

Merged
merged 9 commits into from
Feb 16, 2017
1 change: 1 addition & 0 deletions CHANGELOG.md
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
- [`no-anonymous-default-export`] rule: report anonymous default exports ([#712], thanks [@duncanbeevers]).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please revert all the changes to this file, except for your addition of course

- Add new value to `order`'s `newlines-between` option to allow newlines inside import groups ([#627], [#628], thanks [@giodamelio])
- Add `count` option to the `newline-after-import` rule to allow configuration of number of newlines expected ([#742])

### Changed
- [`no-extraneous-dependencies`]: use `read-pkg-up` to simplify finding + loading `package.json` ([#680], thanks [@wtgtybhertgeghgtwtg])
Expand Down
40 changes: 38 additions & 2 deletions docs/rules/newline-after-import.md
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
# newline-after-import

Enforces having an empty line after the last top-level import statement or require call.
Enforces having one or more empty line after the last top-level import statement or require call.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"one or more empty lines" (ie, plural)


## Rule Details

This rule has one option, `count` which sets the number of newlines that are enforced after the last top-level import statement or require call. This option defaults to `1`.

Valid:

```js
Expand All @@ -26,7 +28,7 @@ const BAR = require('./bar')
const BAZ = 1
```

...whereas here imports will be reported:
Invalid:

```js
import * as foo from 'foo'
Expand All @@ -46,6 +48,40 @@ const BAZ = 1
const BAR = require('./bar')
```

With `count` set to `2` this will be considered valid:

```js
import defaultExport from './foo'


const FOO = 'BAR'
```

With `count` set to `2` these will be considered invalid:

```js
import defaultExport from './foo'
const FOO = 'BAR'
```

```js
import defaultExport from './foo'

const FOO = 'BAR'
```


## Example options usage
```
{
...
"rules": {
"import/newline-after-import": [{ "count": 2 }]
}
}
```


## When Not To Use It

If you like to visually group module imports with its usage, you don't want to use this rule.
15 changes: 14 additions & 1 deletion src/rules/newline-after-import.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,18 @@ function isClassWithDecorator(node) {
module.exports = {
meta: {
docs: {},
schema: [
{
'type': 'object',
'properties': {
'count': {
'type': 'integer',
'minimum': 1,
},
},
'additionalProperties': false,
},
],
},
create: function (context) {
let level = 0
Expand All @@ -55,7 +67,8 @@ module.exports = {
nextNode = nextNode.decorators[0]
}

if (getLineDifference(node, nextNode) < 2) {
const options = context.options[0] || { count: 1 }
if (getLineDifference(node, nextNode) < options.count + 1) {
let column = node.loc.start.column

if (node.loc.start.line !== node.loc.end.line) {
Expand Down
148 changes: 94 additions & 54 deletions tests/src/rules/newline-after-import.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@ const ruleTester = new RuleTester()

ruleTester.run('newline-after-import', require('rules/newline-after-import'), {
valid: [
"var path = require('path');\nvar foo = require('foo');\n",
"require('foo');",
"switch ('foo') { case 'bar': require('baz'); }",
`var path = require('path');\nvar foo = require('foo');\n`,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't mind changing this to backticks, but then we might as well replace the \n by in-code line breaks.
(Not necessary for this PR, but if you feel like changing that at some point later, that's would be nice :D).

`require('foo');`,
`switch ('foo') { case 'bar': require('baz'); }`,
{
code: `
const x = () => require('baz')
Expand All @@ -20,8 +20,8 @@ ruleTester.run('newline-after-import', require('rules/newline-after-import'), {
code: `const x = () => require('baz') && require('bar')`,
parserOptions: { ecmaVersion: 6 } ,
},
"function x(){ require('baz'); }",
"a(require('b'), require('c'), require('d'));",
`function x(){ require('baz'); }`,
`a(require('b'), require('c'), require('d'));`,
`function foo() {
switch (renderData.modalViewKey) {
case 'value':
Expand Down Expand Up @@ -65,40 +65,60 @@ ruleTester.run('newline-after-import', require('rules/newline-after-import'), {
parserOptions: { sourceType: 'module' },
},
{
code: "import path from 'path';\nimport foo from 'foo';\n",
code: `import path from 'path';\nimport foo from 'foo';\n`,
parserOptions: { sourceType: 'module' },
},
{
code: "import path from 'path';import foo from 'foo';\n",
code: `import path from 'path';import foo from 'foo';\n`,
parserOptions: { sourceType: 'module' },
},
{
code: "import path from 'path';import foo from 'foo';\n\nvar bar = 42;",
code: `import path from 'path';import foo from 'foo';\n\nvar bar = 42;`,
parserOptions: { sourceType: 'module' },
},
{
code: "import foo from 'foo';\n\nvar foo = 'bar';",
parserOptions: { sourceType: 'module' }
code: `import foo from 'foo';\n\nvar foo = 'bar';`,
parserOptions: { sourceType: 'module' },
},
{
code: `import foo from 'foo';\n\n\nvar foo = 'bar';`,
parserOptions: { sourceType: 'module' },
options: [{ 'count': 2 }],
},
{
code: `import foo from 'foo';\n\n\n\n\nvar foo = 'bar';`,
parserOptions: { sourceType: 'module' },
options: [{ 'count': 4 }],
},
{
code: `var foo = require('foo-module');\n\nvar foo = 'bar';`,
parserOptions: { sourceType: 'module' },
},
{
code: `var foo = require('foo-module');\n\n\nvar foo = 'bar';`,
parserOptions: { sourceType: 'module' },
options: [{ 'count': 2 }],
},
{
code: "var foo = require('foo-module');\n\nvar foo = 'bar';",
parserOptions: { sourceType: 'module' }
code: `var foo = require('foo-module');\n\n\n\n\nvar foo = 'bar';`,
parserOptions: { sourceType: 'module' },
options: [{ 'count': 4 }],
},
{
code: "require('foo-module');\n\nvar foo = 'bar';",
parserOptions: { sourceType: 'module' }
code: `require('foo-module');\n\nvar foo = 'bar';`,
parserOptions: { sourceType: 'module' },
},
{
code: "import foo from 'foo';\nimport { bar } from './bar-lib';",
parserOptions: { sourceType: 'module' }
code: `import foo from 'foo';\nimport { bar } from './bar-lib';`,
parserOptions: { sourceType: 'module' },
},
{
code: "import foo from 'foo';\n\nvar a = 123;\n\nimport { bar } from './bar-lib';",
parserOptions: { sourceType: 'module' }
code: `import foo from 'foo';\n\nvar a = 123;\n\nimport { bar } from './bar-lib';`,
parserOptions: { sourceType: 'module' },
},
{
code: "var foo = require('foo-module');\n\nvar a = 123;\n\nvar bar = require('bar-lib');",
parserOptions: { sourceType: 'module' }
code: `var foo = require('foo-module');\n\nvar a = 123;\n\nvar bar = require('bar-lib');`,
parserOptions: { sourceType: 'module' },
},
{
code: `
Expand Down Expand Up @@ -137,114 +157,134 @@ ruleTester.run('newline-after-import', require('rules/newline-after-import'), {
parser: 'babel-eslint',
},
{
code: "var foo = require('foo');\n\n@SomeDecorator(foo)\nclass Foo {}",
code: `var foo = require('foo');\n\n@SomeDecorator(foo)\nclass Foo {}`,
parserOptions: { sourceType: 'module' },
parser: 'babel-eslint',
},
],

invalid: [
{
code: "import foo from 'foo';\nexport default function() {};",
code: `import foo from 'foo';\nexport default function() {};`,
errors: [ {
line: 1,
column: 1,
message: IMPORT_ERROR_MESSAGE
message: IMPORT_ERROR_MESSAGE,
} ],
parserOptions: { sourceType: 'module' }
parserOptions: { sourceType: 'module' },
},
{
code: "var foo = require('foo-module');\nvar something = 123;",
code: `import foo from 'foo';\n\nexport default function() {};`,
options: [{ 'count': 2 }],
errors: [ {
line: 1,
column: 1,
message: REQUIRE_ERROR_MESSAGE
message: IMPORT_ERROR_MESSAGE,
} ],
parserOptions: { sourceType: 'module' }
parserOptions: { sourceType: 'module' },
},
{
code: "import foo from 'foo';\nvar a = 123;\n\nimport { bar } from './bar-lib';\nvar b=456;",
code: `import foo from 'foo';\nexport default function() {};`,
options: [{ 'count': 1 }],
errors: [ {
line: 1,
column: 1,
message: IMPORT_ERROR_MESSAGE,
} ],
parserOptions: { sourceType: 'module' },
},
{
code: `var foo = require('foo-module');\nvar something = 123;`,
errors: [ {
line: 1,
column: 1,
message: REQUIRE_ERROR_MESSAGE,
} ],
parserOptions: { sourceType: 'module' },
},
{
code: `import foo from 'foo';\nvar a = 123;\n\nimport { bar } from './bar-lib';\nvar b=456;`,
errors: [
{
line: 1,
column: 1,
message: IMPORT_ERROR_MESSAGE
message: IMPORT_ERROR_MESSAGE,
},
{
line: 4,
column: 1,
message: IMPORT_ERROR_MESSAGE
message: IMPORT_ERROR_MESSAGE,
}],
parserOptions: { sourceType: 'module' }
parserOptions: { sourceType: 'module' },
},
{
code: "var foo = require('foo-module');\nvar a = 123;\n\nvar bar = require('bar-lib');\nvar b=456;",
code: `var foo = require('foo-module');\nvar a = 123;\n\nvar bar = require('bar-lib');\nvar b=456;`,
errors: [
{
line: 1,
column: 1,
message: REQUIRE_ERROR_MESSAGE
message: REQUIRE_ERROR_MESSAGE,
},
{
line: 4,
column: 1,
message: REQUIRE_ERROR_MESSAGE
message: REQUIRE_ERROR_MESSAGE,
}],
parserOptions: { sourceType: 'module' }
parserOptions: { sourceType: 'module' },
},
{
code: "var foo = require('foo-module');\nvar a = 123;\n\nrequire('bar-lib');\nvar b=456;",
code: `var foo = require('foo-module');\nvar a = 123;\n\nrequire('bar-lib');\nvar b=456;`,
errors: [
{
line: 1,
column: 1,
message: REQUIRE_ERROR_MESSAGE
message: REQUIRE_ERROR_MESSAGE,
},
{
line: 4,
column: 1,
message: REQUIRE_ERROR_MESSAGE
message: REQUIRE_ERROR_MESSAGE,
}],
parserOptions: { sourceType: 'module' }
parserOptions: { sourceType: 'module' },
},
{
code: "var path = require('path');\nvar foo = require('foo');\nvar bar = 42;",
code: `var path = require('path');\nvar foo = require('foo');\nvar bar = 42;`,
errors: [ {
line: 2,
column: 1,
message: REQUIRE_ERROR_MESSAGE,
} ]
} ],
},
{
code: "var assign = Object.assign || require('object-assign');\nvar foo = require('foo');\nvar bar = 42;",
code: `var assign = Object.assign || require('object-assign');\nvar foo = require('foo');\nvar bar = 42;`,
errors: [ {
line: 2,
column: 1,
message: REQUIRE_ERROR_MESSAGE,
} ]
} ],
},
{
code: "require('a');\nfoo(require('b'), require('c'), require('d'));\nrequire('d');\nvar foo = 'bar';",
code: `require('a');\nfoo(require('b'), require('c'), require('d'));\nrequire('d');\nvar foo = 'bar';`,
errors: [
{
line: 3,
column: 1,
message: REQUIRE_ERROR_MESSAGE
message: REQUIRE_ERROR_MESSAGE,
},
]
],
},
{
code: "require('a');\nfoo(\nrequire('b'),\nrequire('c'),\nrequire('d')\n);\nvar foo = 'bar';",
code: `require('a');\nfoo(\nrequire('b'),\nrequire('c'),\nrequire('d')\n);\nvar foo = 'bar';`,
errors: [
{
line: 6,
column: 1,
message: REQUIRE_ERROR_MESSAGE
}
]
message: REQUIRE_ERROR_MESSAGE,
},
],
},
{
code: "import path from 'path';\nimport foo from 'foo';\nvar bar = 42;",
code: `import path from 'path';\nimport foo from 'foo';\nvar bar = 42;`,
errors: [ {
line: 2,
column: 1,
Expand All @@ -253,7 +293,7 @@ ruleTester.run('newline-after-import', require('rules/newline-after-import'), {
parserOptions: { sourceType: 'module' },
},
{
code: "import path from 'path';import foo from 'foo';var bar = 42;",
code: `import path from 'path';import foo from 'foo';var bar = 42;`,
errors: [ {
line: 1,
column: 25,
Expand All @@ -262,7 +302,7 @@ ruleTester.run('newline-after-import', require('rules/newline-after-import'), {
parserOptions: { sourceType: 'module' },
},
{
code: "import foo from 'foo';\n@SomeDecorator(foo)\nclass Foo {}",
code: `import foo from 'foo';\n@SomeDecorator(foo)\nclass Foo {}`,
errors: [ {
line: 1,
column: 1,
Expand All @@ -272,7 +312,7 @@ ruleTester.run('newline-after-import', require('rules/newline-after-import'), {
parser: 'babel-eslint',
},
{
code: "var foo = require('foo');\n@SomeDecorator(foo)\nclass Foo {}",
code: `var foo = require('foo');\n@SomeDecorator(foo)\nclass Foo {}`,
errors: [ {
line: 1,
column: 1,
Expand Down