Skip to content

Commit

Permalink
[Fix] newline-after-import: fix considerComments option when require
Browse files Browse the repository at this point in the history
  • Loading branch information
developer-bandi authored and ljharb committed Jan 12, 2024
1 parent 806e3c2 commit ee1ea02
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 11 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange
- [`no-extraneous-dependencies`]: allow wrong path ([#3012], thanks [@chabb])
- [`no-cycle`]: use scc algorithm to optimize ([#2998], thanks [@soryy708])
- [`no-duplicates`]: Removing duplicates breaks in TypeScript ([#3033], thanks [@yesl-kim])
- [`newline-after-import`]: fix considerComments option when require ([#2952], thanks [@developer-bandi])

### Changed
- [Docs] `no-extraneous-dependencies`: Make glob pattern description more explicit ([#2944], thanks [@mulztob])
Expand Down Expand Up @@ -1136,6 +1137,7 @@ for info on changes for earlier releases.
[#2987]: https://github.com/import-js/eslint-plugin-import/pull/2987
[#2985]: https://github.com/import-js/eslint-plugin-import/pull/2985
[#2982]: https://github.com/import-js/eslint-plugin-import/pull/2982
[#2952]: https://github.com/import-js/eslint-plugin-import/pull/2952
[#2944]: https://github.com/import-js/eslint-plugin-import/pull/2944
[#2942]: https://github.com/import-js/eslint-plugin-import/pull/2942
[#2919]: https://github.com/import-js/eslint-plugin-import/pull/2919
Expand Down Expand Up @@ -1742,6 +1744,7 @@ for info on changes for earlier releases.
[@bicstone]: https://github.com/bicstone
[@Blasz]: https://github.com/Blasz
[@bmish]: https://github.com/bmish
[@developer-bandi]: https://github.com/developer-bandi
[@borisyankov]: https://github.com/borisyankov
[@bradennapier]: https://github.com/bradennapier
[@bradzacher]: https://github.com/bradzacher
Expand Down
20 changes: 15 additions & 5 deletions src/rules/newline-after-import.js
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ module.exports = {
}
}

function commentAfterImport(node, nextComment) {
function commentAfterImport(node, nextComment, type) {
const lineDifference = getLineDifference(node, nextComment);
const EXPECTED_LINE_DIFFERENCE = options.count + 1;

Expand All @@ -140,7 +140,7 @@ module.exports = {
line: node.loc.end.line,
column,
},
message: `Expected ${options.count} empty line${options.count > 1 ? 's' : ''} after import statement not followed by another import.`,
message: `Expected ${options.count} empty line${options.count > 1 ? 's' : ''} after ${type} statement not followed by another ${type}.`,
fix: options.exactCount && EXPECTED_LINE_DIFFERENCE < lineDifference ? undefined : (fixer) => fixer.insertTextAfter(
node,
'\n'.repeat(EXPECTED_LINE_DIFFERENCE - lineDifference),
Expand Down Expand Up @@ -178,7 +178,7 @@ module.exports = {
}

if (nextComment && typeof nextComment !== 'undefined') {
commentAfterImport(node, nextComment);
commentAfterImport(node, nextComment, 'import');
} else if (nextNode && nextNode.type !== 'ImportDeclaration' && (nextNode.type !== 'TSImportEqualsDeclaration' || nextNode.isExport)) {
checkForNewLine(node, nextNode, 'import');
}
Expand Down Expand Up @@ -215,8 +215,18 @@ module.exports = {
|| !containsNodeOrEqual(nextStatement, nextRequireCall)
)
) {

checkForNewLine(statementWithRequireCall, nextStatement, 'require');
let nextComment;
if (typeof statementWithRequireCall.parent.comments !== 'undefined' && options.considerComments) {
const endLine = node.loc.end.line;
nextComment = statementWithRequireCall.parent.comments.find((o) => o.loc.start.line >= endLine && o.loc.start.line <= endLine + options.count + 1);
}

if (nextComment && typeof nextComment !== 'undefined') {

commentAfterImport(statementWithRequireCall, nextComment, 'require');
} else {
checkForNewLine(statementWithRequireCall, nextStatement, 'require');
}
}
});
},
Expand Down
38 changes: 32 additions & 6 deletions tests/src/rules/newline-after-import.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { getTSParsers, parsers, testVersion } from '../utils';
const IMPORT_ERROR_MESSAGE = 'Expected 1 empty line after import statement not followed by another import.';
const IMPORT_ERROR_MESSAGE_MULTIPLE = (count) => `Expected ${count} empty lines after import statement not followed by another import.`;
const REQUIRE_ERROR_MESSAGE = 'Expected 1 empty line after require statement not followed by another require.';
const REQUIRE_ERROR_MESSAGE_MULTIPLE = (count) => `Expected ${count} empty lines after require statement not followed by another require.`;

const ruleTester = new RuleTester();

Expand Down Expand Up @@ -202,7 +203,7 @@ ruleTester.run('newline-after-import', require('rules/newline-after-import'), {
options: [{ count: 4, exactCount: true }],
},
{
code: `var foo = require('foo-module');\n\n\n\n// Some random comment\nvar foo = 'bar';`,
code: `var foo = require('foo-module');\n\n\n\n\n// Some random comment\nvar foo = 'bar';`,
parserOptions: { ecmaVersion: 2015, sourceType: 'module' },
options: [{ count: 4, exactCount: true, considerComments: true }],
},
Expand Down Expand Up @@ -394,6 +395,19 @@ ruleTester.run('newline-after-import', require('rules/newline-after-import'), {
`,
parserOptions: { ecmaVersion: 2015, sourceType: 'module' },
},
{
code: `var foo = require('foo-module');\n\n\n// Some random comment\nvar foo = 'bar';`,
options: [{ count: 2, considerComments: true }],
},
{
code: `var foo = require('foo-module');\n\n\n/**\n * Test comment\n */\nvar foo = 'bar';`,
options: [{ count: 2, considerComments: true }],
},
{
code: `const foo = require('foo');\n\n\n// some random comment\nconst bar = function() {};`,
options: [{ count: 2, exactCount: true, considerComments: true }],
parserOptions: { ecmaVersion: 2015 },
},
),

invalid: [].concat(
Expand Down Expand Up @@ -825,7 +839,7 @@ ruleTester.run('newline-after-import', require('rules/newline-after-import'), {
errors: [{
line: 1,
column: 1,
message: 'Expected 2 empty lines after require statement not followed by another require.',
message: REQUIRE_ERROR_MESSAGE_MULTIPLE(2),
}],
parserOptions: { ecmaVersion: 2015 },
},
Expand All @@ -836,7 +850,7 @@ ruleTester.run('newline-after-import', require('rules/newline-after-import'), {
errors: [{
line: 1,
column: 1,
message: 'Expected 2 empty lines after require statement not followed by another require.',
message: REQUIRE_ERROR_MESSAGE_MULTIPLE(2),
}],
parserOptions: { ecmaVersion: 2015 },
},
Expand All @@ -852,14 +866,26 @@ ruleTester.run('newline-after-import', require('rules/newline-after-import'), {
parserOptions: { ecmaVersion: 2015, considerComments: true, sourceType: 'module' },
},
{
code: `const foo = require('foo');\n\n\n// some random comment\nconst bar = function() {};`,
options: [{ count: 2, exactCount: true, considerComments: true }],
code: `var foo = require('foo-module');\nvar foo = require('foo-module');\n\n// Some random comment\nvar foo = 'bar';`,
output: `var foo = require('foo-module');\nvar foo = require('foo-module');\n\n\n// Some random comment\nvar foo = 'bar';`,
errors: [{
line: 2,
column: 1,
message: REQUIRE_ERROR_MESSAGE_MULTIPLE(2),
}],
parserOptions: { ecmaVersion: 2015, sourceType: 'module' },
options: [{ considerComments: true, count: 2 }],
},
{
code: `var foo = require('foo-module');\n\n/**\n * Test comment\n */\nvar foo = 'bar';`,
output: `var foo = require('foo-module');\n\n\n/**\n * Test comment\n */\nvar foo = 'bar';`,
errors: [{
line: 1,
column: 1,
message: 'Expected 2 empty lines after require statement not followed by another require.',
message: REQUIRE_ERROR_MESSAGE_MULTIPLE(2),
}],
parserOptions: { ecmaVersion: 2015 },
options: [{ considerComments: true, count: 2 }],
},
),
});

0 comments on commit ee1ea02

Please sign in to comment.