From 0af1d2828d27885483737867653ba1659af72005 Mon Sep 17 00:00:00 2001 From: Milos Djermanovic Date: Tue, 7 Jul 2020 22:26:16 +0200 Subject: [PATCH] Update: add allowSeparatedGroups option to sort-imports (fixes #12951) (#13455) --- docs/rules/sort-imports.md | 51 +++++++++- lib/rules/sort-imports.js | 28 ++++++ tests/lib/rules/sort-imports.js | 159 +++++++++++++++++++++++++++++++- 3 files changed, 236 insertions(+), 2 deletions(-) diff --git a/docs/rules/sort-imports.md b/docs/rules/sort-imports.md index f6fd04f0683..637c9fcf3f3 100644 --- a/docs/rules/sort-imports.md +++ b/docs/rules/sort-imports.md @@ -41,6 +41,7 @@ This rule accepts an object with its properties as * `all` = import all members provided by exported bindings. * `multiple` = import multiple members. * `single` = import single member. +* `allowSeparatedGroups` (default: `false`) Default option settings are: @@ -50,7 +51,8 @@ Default option settings are: "ignoreCase": false, "ignoreDeclarationSort": false, "ignoreMemberSort": false, - "memberSyntaxSortOrder": ["none", "all", "multiple", "single"] + "memberSyntaxSortOrder": ["none", "all", "multiple", "single"], + "allowSeparatedGroups": false }] } ``` @@ -226,6 +228,53 @@ import {a, b} from 'foo.js'; Default is `["none", "all", "multiple", "single"]`. +### `allowSeparatedGroups` + +When `true` the rule checks the sorting of import declaration statements only for those that appear on consecutive lines. + +In other words, a blank line or a comment line or line with any other statement after an import declaration statement will reset the sorting of import declaration statements. + +Examples of **incorrect** code for this rule with the `{ "allowSeparatedGroups": true }` option: + +```js +/*eslint sort-imports: ["error", { "allowSeparatedGroups": true }]*/ + +import b from 'foo.js'; +import c from 'bar.js'; +import a from 'baz.js'; +``` + +Examples of **correct** code for this rule with the `{ "allowSeparatedGroups": true }` option: + +```js +/*eslint sort-imports: ["error", { "allowSeparatedGroups": true }]*/ + +import b from 'foo.js'; +import c from 'bar.js'; + +import a from 'baz.js'; +``` + +```js +/*eslint sort-imports: ["error", { "allowSeparatedGroups": true }]*/ + +import b from 'foo.js'; +import c from 'bar.js'; +// comment +import a from 'baz.js'; +``` + +```js +/*eslint sort-imports: ["error", { "allowSeparatedGroups": true }]*/ + +import b from 'foo.js'; +import c from 'bar.js'; +quux(); +import a from 'baz.js'; +``` + +Default is `false`. + ## When Not To Use It This rule is a formatting preference and not following it won't negatively affect the quality of your code. If alphabetizing imports isn't a part of your coding standards, then you can leave this rule disabled. diff --git a/lib/rules/sort-imports.js b/lib/rules/sort-imports.js index 65ad9a18a93..4c3ddec7669 100644 --- a/lib/rules/sort-imports.js +++ b/lib/rules/sort-imports.js @@ -44,6 +44,10 @@ module.exports = { ignoreMemberSort: { type: "boolean", default: false + }, + allowSeparatedGroups: { + type: "boolean", + default: false } }, additionalProperties: false @@ -66,6 +70,7 @@ module.exports = { ignoreDeclarationSort = configuration.ignoreDeclarationSort || false, ignoreMemberSort = configuration.ignoreMemberSort || false, memberSyntaxSortOrder = configuration.memberSyntaxSortOrder || ["none", "all", "multiple", "single"], + allowSeparatedGroups = configuration.allowSeparatedGroups || false, sourceCode = context.getSourceCode(); let previousDeclaration = null; @@ -115,9 +120,32 @@ module.exports = { } + /** + * Calculates number of lines between two nodes. It is assumed that the given `left` node appears before + * the given `right` node in the source code. Lines are counted from the end of the `left` node till the + * start of the `right` node. If the given nodes are on the same line, it returns `0`, same as if they were + * on two consecutive lines. + * @param {ASTNode} left node that appears before the given `right` node. + * @param {ASTNode} right node that appears after the given `left` node. + * @returns {number} number of lines between nodes. + */ + function getNumberOfLinesBetween(left, right) { + return Math.max(right.loc.start.line - left.loc.end.line - 1, 0); + } + return { ImportDeclaration(node) { if (!ignoreDeclarationSort) { + if ( + previousDeclaration && + allowSeparatedGroups && + getNumberOfLinesBetween(previousDeclaration, node) > 0 + ) { + + // reset declaration sort + previousDeclaration = null; + } + if (previousDeclaration) { const currentMemberSyntaxGroupIndex = getMemberParameterGroupIndex(node), previousMemberSyntaxGroupIndex = getMemberParameterGroupIndex(previousDeclaration); diff --git a/tests/lib/rules/sort-imports.js b/tests/lib/rules/sort-imports.js index 51f3ee3a804..b5da5018919 100644 --- a/tests/lib/rules/sort-imports.js +++ b/tests/lib/rules/sort-imports.js @@ -101,7 +101,45 @@ ruleTester.run("sort-imports", rule, { }, // https://github.com/eslint/eslint/issues/5305 - "import React, {Component} from 'react';" + "import React, {Component} from 'react';", + + // allowSeparatedGroups + { + code: "import b from 'b';\n\nimport a from 'a';", + options: [{ allowSeparatedGroups: true }] + }, + { + code: "import a from 'a';\n\nimport 'b';", + options: [{ allowSeparatedGroups: true }] + }, + { + code: "import { b } from 'b';\n\n\nimport { a } from 'a';", + options: [{ allowSeparatedGroups: true }] + }, + { + code: "import b from 'b';\n// comment\nimport a from 'a';", + options: [{ allowSeparatedGroups: true }] + }, + { + code: "import b from 'b';\nfoo();\nimport a from 'a';", + options: [{ allowSeparatedGroups: true }] + }, + { + code: "import { b } from 'b';/*\n comment \n*/import { a } from 'a';", + options: [{ allowSeparatedGroups: true }] + }, + { + code: "import b from\n'b';\n\nimport\n a from 'a';", + options: [{ allowSeparatedGroups: true }] + }, + { + code: "import c from 'c';\n\nimport a from 'a';\nimport b from 'b';", + options: [{ allowSeparatedGroups: true }] + }, + { + code: "import c from 'c';\n\nimport b from 'b';\n\nimport a from 'a';", + options: [{ allowSeparatedGroups: true }] + } ], invalid: [ { @@ -287,6 +325,125 @@ ruleTester.run("sort-imports", rule, { data: { memberName: "qux" }, type: "ImportSpecifier" }] + }, + + // allowSeparatedGroups + { + code: "import b from 'b';\nimport a from 'a';", + output: null, + errors: [{ + messageId: "sortImportsAlphabetically", + type: "ImportDeclaration" + }] + }, + { + code: "import b from 'b';\nimport a from 'a';", + output: null, + options: [{}], + errors: [{ + messageId: "sortImportsAlphabetically", + type: "ImportDeclaration" + }] + }, + { + code: "import b from 'b';\nimport a from 'a';", + output: null, + options: [{ allowSeparatedGroups: false }], + errors: [{ + messageId: "sortImportsAlphabetically", + type: "ImportDeclaration" + }] + }, + { + code: "import b from 'b';import a from 'a';", + output: null, + options: [{ allowSeparatedGroups: false }], + errors: [{ + messageId: "sortImportsAlphabetically", + type: "ImportDeclaration" + }] + }, + { + code: "import b from 'b'; /* comment */ import a from 'a';", + output: null, + options: [{ allowSeparatedGroups: false }], + errors: [{ + messageId: "sortImportsAlphabetically", + type: "ImportDeclaration" + }] + }, + { + code: "import b from 'b'; // comment\nimport a from 'a';", + output: null, + options: [{ allowSeparatedGroups: false }], + errors: [{ + messageId: "sortImportsAlphabetically", + type: "ImportDeclaration" + }] + }, + { + code: "import b from 'b'; // comment 1\n/* comment 2 */import a from 'a';", + output: null, + options: [{ allowSeparatedGroups: false }], + errors: [{ + messageId: "sortImportsAlphabetically", + type: "ImportDeclaration" + }] + }, + { + code: "import { b } from 'b'; /* comment line 1 \n comment line 2 */ import { a } from 'a';", + output: null, + options: [{ allowSeparatedGroups: false }], + errors: [{ + messageId: "sortImportsAlphabetically", + type: "ImportDeclaration" + }] + }, + { + code: "import b\nfrom 'b'; import a\nfrom 'a';", + output: null, + options: [{ allowSeparatedGroups: false }], + errors: [{ + messageId: "sortImportsAlphabetically", + type: "ImportDeclaration" + }] + }, + { + code: "import { b } from \n'b'; /* comment */ import\n { a } from 'a';", + output: null, + options: [{ allowSeparatedGroups: false }], + errors: [{ + messageId: "sortImportsAlphabetically", + type: "ImportDeclaration" + }] + }, + { + code: "import { b } from \n'b';\nimport\n { a } from 'a';", + output: null, + options: [{ allowSeparatedGroups: false }], + errors: [{ + messageId: "sortImportsAlphabetically", + type: "ImportDeclaration" + }] + }, + { + code: "import c from 'c';\n\nimport b from 'b';\nimport a from 'a';", + output: null, + options: [{ allowSeparatedGroups: true }], + errors: [{ + messageId: "sortImportsAlphabetically", + type: "ImportDeclaration", + line: 4 + }] + }, + { + code: "import b from 'b';\n\nimport { c, a } from 'c';", + output: "import b from 'b';\n\nimport { a, c } from 'c';", + options: [{ allowSeparatedGroups: true }], + errors: [{ + messageId: "sortMembersAlphabetically", + type: "ImportSpecifier" + }] } ] });