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

[New] newline-after-import: add considerComments option #2399

Merged
merged 1 commit into from Apr 12, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions CHANGELOG.md
Expand Up @@ -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])

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
29 changes: 28 additions & 1 deletion docs/rules/newline-after-import.md
Expand Up @@ -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:

Expand Down Expand Up @@ -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
Expand Down
42 changes: 38 additions & 4 deletions src/rules/newline-after-import.js
Expand Up @@ -67,6 +67,7 @@ module.exports = {
'type': 'integer',
'minimum': 1,
},
'considerComments': { 'type': 'boolean' },
},
'additionalProperties': false,
},
Expand All @@ -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)) {
Expand All @@ -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;

Expand All @@ -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;
pri1311 marked this conversation as resolved.
Show resolved Hide resolved
}

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),
Expand All @@ -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');
}
}
Expand Down
136 changes: 136 additions & 0 deletions tests/src/rules/newline-after-import.js
Expand Up @@ -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() {
Expand Down Expand Up @@ -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() {};`,
Expand Down