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

Add count option to newline-after-import #742

Merged
merged 9 commits into from
Feb 16, 2017
+150 −58
Merged

Add count option to newline-after-import #742

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
14605f8
Add `newlines` option to `newline-after-import`
ntdb Feb 12, 2017
3e5acab
Add tests for `newline-after-import` option and fix linter errors in …
ntdb Feb 12, 2017
ba3a5c0
Fix broken tests, add docs
ntdb Feb 12, 2017
8c7ea26
Update CHANGELOG
ntdb Feb 12, 2017
1d757dc
Add schema and further testing for `newlines` option on `newline-afte…
ntdb Feb 13, 2017
c0f733b
Change `newlines` option to `count` for `newline-after-import` rule
ntdb Feb 16, 2017
2323ece
Fix missing plural in docs
ntdb Feb 16, 2017
639c227
Add links in changelog and thank @ntdb :)
jfmengels Feb 16, 2017
49ea883
Add links to rules in changelog
jfmengels Feb 16, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
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
- [`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 `newlines` 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
View file
Open in desktop
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, `newlines` 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 `newlines` set to `2` this will be considered valid:

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


const FOO = 'BAR'
```

With `newlines` 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": [{ "newlines": 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
View file
Open in desktop
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': {
'newlines': {
'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] || { newlines: 1 }
Copy link
Member

Choose a reason for hiding this comment

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

let's add a rule schema that enforces that newlines is an integer? (ideally a positive one)

if (getLineDifference(node, nextNode) < options.newlines + 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
View file
Open in desktop
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: [{ 'newlines': 2 }],
},
{
code: `import foo from 'foo';\n\n\n\n\nvar foo = 'bar';`,
parserOptions: { sourceType: 'module' },
options: [{ 'newlines': 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: [{ 'newlines': 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: [{ 'newlines': 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: [{ 'newlines': 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: [{ 'newlines': 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