From 995c12c80016e9d2cc5b3026884c34b5d4b1ad13 Mon Sep 17 00:00:00 2001 From: pri1311 Date: Tue, 8 Mar 2022 17:19:58 +0530 Subject: [PATCH] [New] `newline-after-import`: add `considerComments` option --- CHANGELOG.md | 5 + docs/rules/newline-after-import.md | 29 ++++- src/rules/newline-after-import.js | 42 +++++++- tests/src/rules/newline-after-import.js | 136 ++++++++++++++++++++++++ 4 files changed, 207 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index db482f619..89e7f66e8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,9 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange ## [Unreleased] +### Added +- [`newline-after-import`]: add `considerComments` option ([#2399], thanks [@pri1311]) + ### Changed - [Tests] `named`: Run all TypeScript test ([#2427], thanks [@ProdigySim]) @@ -983,6 +986,7 @@ for info on changes for earlier releases. [#2427]: https://github.com/import-js/eslint-plugin-import/pull/2427 [#2417]: https://github.com/import-js/eslint-plugin-import/pull/2417 [#2411]: https://github.com/import-js/eslint-plugin-import/pull/2411 +[#2399]: https://github.com/import-js/eslint-plugin-import/pull/2399 [#2393]: https://github.com/import-js/eslint-plugin-import/pull/2393 [#2388]: https://github.com/import-js/eslint-plugin-import/pull/2388 [#2381]: https://github.com/import-js/eslint-plugin-import/pull/2381 @@ -1628,6 +1632,7 @@ for info on changes for earlier releases. [@Pessimistress]: https://github.com/Pessimistress [@pmcelhaney]: https://github.com/pmcelhaney [@preco21]: https://github.com/preco21 +[@pri1311]: https://github.com/pri1311 [@ProdigySim]: https://github.com/ProdigySim [@pzhine]: https://github.com/pzhine [@ramasilveyra]: https://github.com/ramasilveyra diff --git a/docs/rules/newline-after-import.md b/docs/rules/newline-after-import.md index 4883776c9..ab454e4bd 100644 --- a/docs/rules/newline-after-import.md +++ b/docs/rules/newline-after-import.md @@ -5,7 +5,10 @@ Enforces having one or more empty lines after the last top-level import statemen ## 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`. +This rule supports the following options: +- `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`. + +- `considerComments` which enforces the rule on comments after the last import-statement as well when set to true. This option defaults to `false`. Valid: @@ -71,6 +74,30 @@ import defaultExport from './foo' const FOO = 'BAR' ``` +With `considerComments` set to `false` this will be considered valid: + +```js +import defaultExport from './foo' +// some comment here. +const FOO = 'BAR' +``` + +With `considerComments` set to `true` this will be considered valid: + +```js +import defaultExport from './foo' + +// some comment here. +const FOO = 'BAR' +``` + +With `considerComments` set to `true` this will be considered invalid: + +```js +import defaultExport from './foo' +// some comment here. +const FOO = 'BAR' +``` ## Example options usage ```json diff --git a/src/rules/newline-after-import.js b/src/rules/newline-after-import.js index a3e0a11e1..3f285345f 100644 --- a/src/rules/newline-after-import.js +++ b/src/rules/newline-after-import.js @@ -67,6 +67,7 @@ module.exports = { 'type': 'integer', 'minimum': 1, }, + 'considerComments': { 'type': 'boolean' }, }, 'additionalProperties': false, }, @@ -75,6 +76,7 @@ module.exports = { create(context) { let level = 0; const requireCalls = []; + const options = Object.assign({ count: 1, considerComments: false }, context.options[0]); function checkForNewLine(node, nextNode, type) { if (isExportDefaultClass(nextNode) || isExportNameClass(nextNode)) { @@ -87,7 +89,6 @@ module.exports = { nextNode = nextNode.decorators[0]; } - const options = context.options[0] || { count: 1 }; const lineDifference = getLineDifference(node, nextNode); const EXPECTED_LINE_DIFFERENCE = options.count + 1; @@ -103,8 +104,32 @@ module.exports = { line: node.loc.end.line, column, }, - message: `Expected ${options.count} empty line${options.count > 1 ? 's' : ''} \ -after ${type} statement not followed by another ${type}.`, + message: `Expected ${options.count} empty line${options.count > 1 ? 's' : ''} after ${type} statement not followed by another ${type}.`, + fix: fixer => fixer.insertTextAfter( + node, + '\n'.repeat(EXPECTED_LINE_DIFFERENCE - lineDifference), + ), + }); + } + } + + function commentAfterImport(node, nextComment) { + const lineDifference = getLineDifference(node, nextComment); + const EXPECTED_LINE_DIFFERENCE = options.count + 1; + + if (lineDifference < EXPECTED_LINE_DIFFERENCE) { + let column = node.loc.start.column; + + if (node.loc.start.line !== node.loc.end.line) { + column = 0; + } + + context.report({ + loc: { + line: node.loc.end.line, + column, + }, + message: `Expected ${options.count} empty line${options.count > 1 ? 's' : ''} after import statement not followed by another import.`, fix: fixer => fixer.insertTextAfter( node, '\n'.repeat(EXPECTED_LINE_DIFFERENCE - lineDifference), @@ -124,13 +149,22 @@ after ${type} statement not followed by another ${type}.`, const { parent } = node; const nodePosition = parent.body.indexOf(node); const nextNode = parent.body[nodePosition + 1]; + const endLine = node.loc.end.line; + let nextComment; + + if (typeof parent.comments !== 'undefined' && options.considerComments) { + nextComment = parent.comments.find(o => o.loc.start.line === endLine + 1); + } + // skip "export import"s if (node.type === 'TSImportEqualsDeclaration' && node.isExport) { return; } - if (nextNode && nextNode.type !== 'ImportDeclaration' && (nextNode.type !== 'TSImportEqualsDeclaration' || nextNode.isExport)) { + if (nextComment && typeof nextComment !== 'undefined') { + commentAfterImport(node, nextComment); + } else if (nextNode && nextNode.type !== 'ImportDeclaration' && (nextNode.type !== 'TSImportEqualsDeclaration' || nextNode.isExport)) { checkForNewLine(node, nextNode, 'import'); } } diff --git a/tests/src/rules/newline-after-import.js b/tests/src/rules/newline-after-import.js index 80cc076ce..bf91064f8 100644 --- a/tests/src/rules/newline-after-import.js +++ b/tests/src/rules/newline-after-import.js @@ -24,10 +24,41 @@ ruleTester.run('newline-after-import', require('rules/newline-after-import'), { , y = () => require('bar')`, parserOptions: { ecmaVersion: 6 } , }, + { + code: ` + const x = () => require('baz') + , y = () => require('bar') + + // some comment here + `, + parserOptions: { ecmaVersion: 6 } , + options: [{ considerComments: true }], + }, { code: `const x = () => require('baz') && require('bar')`, parserOptions: { ecmaVersion: 6 } , }, + { + code: ` + const x = () => require('baz') && require('bar') + + // Some random single line comment + var bar = 42; + `, + parserOptions: { ecmaVersion: 6 } , + options: [{ 'considerComments': true }], + }, + { + code: ` + const x = () => require('baz') && require('bar') + /** + * some multiline comment here + * another line of comment + **/ + var bar = 42; + `, + parserOptions: { ecmaVersion: 6 } , + }, `function x() { require('baz'); }`, `a(require('b'), require('c'), require('d'));`, `function foo() { @@ -255,9 +286,114 @@ ruleTester.run('newline-after-import', require('rules/newline-after-import'), { `, parserOptions: { ecmaVersion: 2015, sourceType: 'module' }, }, + { + code: ` + import path from 'path'; + import foo from 'foo'; + /** + * some multiline comment here + * another line of comment + **/ + var bar = 42; + `, + parserOptions: { ecmaVersion: 2015, sourceType: 'module' } , + }, + { + code: ` + import path from 'path';import foo from 'foo'; + + /** + * some multiline comment here + * another line of comment + **/ + var bar = 42; + `, + parserOptions: { ecmaVersion: 2015, sourceType: 'module' } , + options: [{ 'considerComments': true }], + }, + { + code: ` + import path from 'path'; + import foo from 'foo'; + + // Some random single line comment + var bar = 42; + `, + parserOptions: { ecmaVersion: 2015, sourceType: 'module' } , + }, ), invalid: [].concat( + { + code: ` + import { A, B, C, D } from + '../path/to/my/module/in/very/far/directory' + // some comment + var foo = 'bar'; + `, + output: ` + import { A, B, C, D } from + '../path/to/my/module/in/very/far/directory' + + // some comment + var foo = 'bar'; + `, + errors: [ { + line: 3, + column: 1, + message: IMPORT_ERROR_MESSAGE, + } ], + parserOptions: { ecmaVersion: 2015, sourceType: 'module' }, + options: [{ 'considerComments': true }], + }, + { + code: ` + import path from 'path'; + import foo from 'foo'; + /** + * some multiline comment here + * another line of comment + **/ + var bar = 42; + `, + output: ` + import path from 'path'; + import foo from 'foo';\n + /** + * some multiline comment here + * another line of comment + **/ + var bar = 42; + `, + errors: [ { + line: 3, + column: 9, + message: IMPORT_ERROR_MESSAGE, + } ], + parserOptions: { ecmaVersion: 2015, sourceType: 'module' } , + options: [{ 'considerComments': true }], + }, + { + code: ` + import path from 'path'; + import foo from 'foo'; + // Some random single line comment + var bar = 42; + `, + output: ` + import path from 'path'; + import foo from 'foo';\n + // Some random single line comment + var bar = 42; + `, + errors: [ { + line: 3, + column: 9, + message: IMPORT_ERROR_MESSAGE, + } ], + parserOptions: { ecmaVersion: 2015, sourceType: 'module' } , + options: [{ 'considerComments': true, 'count': 1 }], + }, { code: `import foo from 'foo';\nexport default function() {};`, output: `import foo from 'foo';\n\nexport default function() {};`,