From b5a962f6c80d0e0ec71a895a603e8e8096292f0e Mon Sep 17 00:00:00 2001 From: Nate Bailey Date: Thu, 16 Feb 2017 13:14:10 -0800 Subject: [PATCH] Add `count` option to `newline-after-import` (#742) * Add `newlines` option to `newline-after-import` * Add tests for `newline-after-import` option and fix linter errors in test file * Fix broken tests, add docs * Update CHANGELOG * Add schema and further testing for `newlines` option on `newline-after-import` * Change `newlines` option to `count` for `newline-after-import` rule * Fix missing plural in docs * Add links in changelog and thank @ntdb :) * Add links to rules in changelog --- CHANGELOG.md | 5 +- docs/rules/newline-after-import.md | 40 ++++++- src/rules/newline-after-import.js | 15 ++- tests/src/rules/newline-after-import.js | 148 +++++++++++++++--------- 4 files changed, 150 insertions(+), 58 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3d1686fd3..648e481df 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,8 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel ## [Unreleased] ### Added - [`no-anonymous-default-export`] rule: report anonymous default exports ([#712], thanks [@duncanbeevers]). -- Add new value to `order`'s `newlines-between` option to allow newlines inside import groups ([#627], [#628], thanks [@giodamelio]) +- 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], thanks [@ntdb]) ### Changed - [`no-extraneous-dependencies`]: use `read-pkg-up` to simplify finding + loading `package.json` ([#680], thanks [@wtgtybhertgeghgtwtg]) @@ -382,6 +383,7 @@ for info on changes for earlier releases. [`unambiguous`]: ./docs/rules/unambiguous.md [`no-anonymous-default-export`]: ./docs/rules/no-anonymous-default-export.md +[#742]: https://github.com/benmosher/eslint-plugin-import/pull/742 [#712]: https://github.com/benmosher/eslint-plugin-import/pull/712 [#680]: https://github.com/benmosher/eslint-plugin-import/pull/680 [#654]: https://github.com/benmosher/eslint-plugin-import/pull/654 @@ -571,3 +573,4 @@ for info on changes for earlier releases. [@wtgtybhertgeghgtwtg]: https://github.com/wtgtybhertgeghgtwtg [@duncanbeevers]: https://github.com/duncanbeevers [@giodamelio]: https://github.com/giodamelio +[@ntdb]: https://github.com/ntdb diff --git a/docs/rules/newline-after-import.md b/docs/rules/newline-after-import.md index 0ad2cba01..42e73f656 100644 --- a/docs/rules/newline-after-import.md +++ b/docs/rules/newline-after-import.md @@ -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 lines after the last top-level import statement or require call. ## 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 @@ -26,7 +28,7 @@ const BAR = require('./bar') const BAZ = 1 ``` -...whereas here imports will be reported: +Invalid: ```js import * as foo from 'foo' @@ -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. diff --git a/src/rules/newline-after-import.js b/src/rules/newline-after-import.js index 3198a9579..868904ba3 100644 --- a/src/rules/newline-after-import.js +++ b/src/rules/newline-after-import.js @@ -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 @@ -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) { diff --git a/tests/src/rules/newline-after-import.js b/tests/src/rules/newline-after-import.js index afd942ef0..e5291ba1e 100644 --- a/tests/src/rules/newline-after-import.js +++ b/tests/src/rules/newline-after-import.js @@ -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`, + `require('foo');`, + `switch ('foo') { case 'bar': require('baz'); }`, { code: ` const x = () => require('baz') @@ -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': @@ -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: ` @@ -137,7 +157,7 @@ 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', }, @@ -145,106 +165,126 @@ ruleTester.run('newline-after-import', require('rules/newline-after-import'), { 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, @@ -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, @@ -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, @@ -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,