[New] order: add newlines-between-types option to control intragr…

…oup sorting of type-only imports Closes #2912 Closes #2347 Closes #2441 Subsumes #2615
import-js · Dec 23, 2024 · e1e5538 · e1e5538
1 parent 668d493
commit e1e5538
Show file tree

Hide file tree

Showing 4 changed files with 603 additions and 33 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -11,6 +11,7 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange
 - add TypeScript types ([#3097], thanks [@G-Rath])
 - [`extensions`]: add `pathGroupOverrides to allow enforcement decision overrides based on specifier ([#3105], thanks [@Xunnamius])
 - [`order`]: add `sortTypesGroup` option to allow intragroup sorting of type-only imports ([#3104], thanks [@Xunnamius])
+- [`order`]: add `newlines-between-types` option to control intragroup sorting of type-only imports ([#3127], thanks [@Xunnamius])
 
 ### Fixed
 - [`no-unused-modules`]: provide more meaningful error message when no .eslintrc is present ([#3116], thanks [@michaelfaith])
@@ -1172,6 +1173,7 @@ for info on changes for earlier releases.
 
 [#3151]: https://github.com/import-js/eslint-plugin-import/pull/3151
 [#3138]: https://github.com/import-js/eslint-plugin-import/pull/3138
+[#3127]: https://github.com/import-js/eslint-plugin-import/pull/3127
 [#3125]: https://github.com/import-js/eslint-plugin-import/pull/3125
 [#3122]: https://github.com/import-js/eslint-plugin-import/pull/3122
 [#3116]: https://github.com/import-js/eslint-plugin-import/pull/3116

diff --git a/docs/rules/order.md b/docs/rules/order.md
@@ -107,6 +107,7 @@ This rule supports the following options (none of which are required):
  - [`named`][33]
  - [`warnOnUnassignedImports`][5]
  - [`sortTypesGroup`][7]
+ - [`newlines-between-types`][27]
 
 ---
 
@@ -592,6 +593,135 @@ This happens because [type-only imports][6] are considered part of one global
 
 The same example will pass.
 
+### `newlines-between-types`
+
+Valid values: `"ignore" | "always" | "always-and-inside-groups" | "never"` \
+Default: the value of [`newlines-between`][20]
+
+> \[!NOTE]
+>
+> This setting is only meaningful when [`sortTypesGroup`][7] is enabled.
+
+`newlines-between-types` is functionally identical to [`newlines-between`][20] except it only enforces or forbids new lines between _[type-only][6] import groups_, which exist only when [`sortTypesGroup`][7] is enabled.
+
+In addition, when determining if a new line is enforceable or forbidden between the type-only imports and the normal imports, `newlines-between-types` takes precedence over [`newlines-between`][20].
+
+#### Example
+
+Given the following settings:
+
+```jsonc
+{
+  "import/order": ["error", {
+    "groups": ["type", "builtin", "parent", "sibling", "index"],
+    "sortTypesGroup": true,
+    "newlines-between": "always"
+  }]
+}
+```
+
+This will fail the rule check:
+
+```ts
+import type A from "fs";
+import type B from "path";
+import type C from "../foo.js";
+import type D from "./bar.js";
+import type E from './';
+
+import a from "fs";
+import b from "path";
+
+import c from "../foo.js";
+
+import d from "./bar.js";
+
+import e from "./";
+```
+
+However, if we set `newlines-between-types` to `"ignore"`:
+
+```jsonc
+{
+  "import/order": ["error", {
+    "groups": ["type", "builtin", "parent", "sibling", "index"],
+    "sortTypesGroup": true,
+    "newlines-between": "always",
+    "newlines-between-types": "ignore"
+  }]
+}
+```
+
+The same example will pass.
+
+Note the new line after `import type E from './';` but before `import a from "fs";`. This new line separates the type-only imports from the normal imports. Its existence is governed by [`newlines-between-types`][27] and _not `newlines-between`_.
+
+> \[!IMPORTANT]
+>
+> In certain situations, `consolidateIslands: true` will take precedence over `newlines-between-types: "never"`, if used, when it comes to the new line separating type-only imports from normal imports.
+
+The next example will pass even though there's a new line preceding the normal import and [`newlines-between`][20] is set to `"never"`:
+
+```jsonc
+{
+  "import/order": ["error", {
+    "groups": ["type", "builtin", "parent", "sibling", "index"],
+    "sortTypesGroup": true,
+    "newlines-between": "never",
+    "newlines-between-types": "always"
+  }]
+}
+```
+
+```ts
+import type A from "fs";
+
+import type B from "path";
+
+import type C from "../foo.js";
+
+import type D from "./bar.js";
+
+import type E from './';
+
+import a from "fs";
+import b from "path";
+import c from "../foo.js";
+import d from "./bar.js";
+import e from "./";
+```
+
+While the following fails due to the new line between the last type import and the first normal import:
+
+```jsonc
+{
+  "import/order": ["error", {
+    "groups": ["type", "builtin", "parent", "sibling", "index"],
+    "sortTypesGroup": true,
+    "newlines-between": "always",
+    "newlines-between-types": "never"
+  }]
+}
+```
+
+```ts
+import type A from "fs";
+import type B from "path";
+import type C from "../foo.js";
+import type D from "./bar.js";
+import type E from './';
+
+import a from "fs";
+
+import b from "path";
+
+import c from "../foo.js";
+
+import d from "./bar.js";
+
+import e from "./";
+```
+
 ## Related
 
  - [`import/external-module-folders`][29]
@@ -617,6 +747,7 @@ The same example will pass.
 [21]: https://eslint.org/docs/latest/rules/no-multiple-empty-lines
 [22]: https://prettier.io
 [23]: https://www.typescriptlang.org/docs/handbook/release-notes/typescript-4-5.html#type-modifiers-on-import-names
+[27]: #newlines-between-types
 [28]: ../../README.md#importinternal-regex
 [29]: ../../README.md#importexternal-module-folders
 [30]: #alphabetize

diff --git a/src/rules/order.js b/src/rules/order.js
@@ -416,7 +416,7 @@ const compareString = (a, b) => {
 };
 
 /** Some parsers (languages without types) don't provide ImportKind */
-const DEAFULT_IMPORT_KIND = 'value';
+const DEFAULT_IMPORT_KIND = 'value';
 const getNormalizedValue = (node, toLowerCase) => {
   const value = node.value;
   return toLowerCase ? String(value).toLowerCase() : value;
@@ -462,8 +462,8 @@ function getSorter(alphabetizeOptions) {
     // In case the paths are equal (result === 0), sort them by importKind
     if (!result && multiplierImportKind) {
       result = multiplierImportKind * compareString(
-        nodeA.node.importKind || DEAFULT_IMPORT_KIND,
-        nodeB.node.importKind || DEAFULT_IMPORT_KIND,
+        nodeA.node.importKind || DEFAULT_IMPORT_KIND,
+        nodeB.node.importKind || DEFAULT_IMPORT_KIND,
       );
     }
 
@@ -677,7 +677,7 @@ function removeNewLineAfterImport(context, currentImport, previousImport) {
   return undefined;
 }
 
-function makeNewlinesBetweenReport(context, imported, newlinesBetweenImports, distinctGroup) {
+function makeNewlinesBetweenReport(context, imported, newlinesBetweenImports, newlinesBetweenTypeOnlyImports, distinctGroup, isSortingTypesGroup) {
   const getNumberOfEmptyLinesBetween = (currentImport, previousImport) => {
     const linesBetweenImports = getSourceCode(context).lines.slice(
       previousImport.node.loc.end.line,
@@ -690,35 +690,72 @@ function makeNewlinesBetweenReport(context, imported, newlinesBetweenImports, di
   let previousImport = imported[0];
 
   imported.slice(1).forEach(function (currentImport) {
-    const emptyLinesBetween = getNumberOfEmptyLinesBetween(currentImport, previousImport);
-    const isStartOfDistinctGroup = getIsStartOfDistinctGroup(currentImport, previousImport);
-
-    if (newlinesBetweenImports === 'always'
-        || newlinesBetweenImports === 'always-and-inside-groups') {
-      if (currentImport.rank !== previousImport.rank && emptyLinesBetween === 0) {
-        if (distinctGroup || !distinctGroup && isStartOfDistinctGroup) {
-          context.report({
-            node: previousImport.node,
-            message: 'There should be at least one empty line between import groups',
-            fix: fixNewLineAfterImport(context, previousImport),
-          });
-        }
-      } else if (emptyLinesBetween > 0
-        && newlinesBetweenImports !== 'always-and-inside-groups') {
-        if (distinctGroup && currentImport.rank === previousImport.rank || !distinctGroup && !isStartOfDistinctGroup) {
-          context.report({
-            node: previousImport.node,
-            message: 'There should be no empty line within import group',
-            fix: removeNewLineAfterImport(context, currentImport, previousImport),
-          });
+    const emptyLinesBetween = getNumberOfEmptyLinesBetween(
+      currentImport,
+      previousImport,
+    );
+
+    const isStartOfDistinctGroup = getIsStartOfDistinctGroup(
+      currentImport,
+      previousImport,
+    );
+
+    const isTypeOnlyImport = currentImport.node.importKind === 'type';
+    const isPreviousImportTypeOnlyImport = previousImport.node.importKind === 'type';
+
+    const isNormalImportFollowingTypeOnlyImportAndRelevant = !isTypeOnlyImport && isPreviousImportTypeOnlyImport && isSortingTypesGroup;
+
+    const isTypeOnlyImportAndRelevant = isTypeOnlyImport && isSortingTypesGroup;
+
+    const isNotIgnored = isTypeOnlyImportAndRelevant
+        && newlinesBetweenTypeOnlyImports !== 'ignore'
+      || !isTypeOnlyImportAndRelevant && newlinesBetweenImports !== 'ignore';
+
+    if (isNotIgnored) {
+      const shouldAssertNewlineBetweenGroups = (isTypeOnlyImportAndRelevant || isNormalImportFollowingTypeOnlyImportAndRelevant)
+          && (newlinesBetweenTypeOnlyImports === 'always'
+            || newlinesBetweenTypeOnlyImports === 'always-and-inside-groups')
+        || !isTypeOnlyImportAndRelevant && !isNormalImportFollowingTypeOnlyImportAndRelevant
+          && (newlinesBetweenImports === 'always'
+            || newlinesBetweenImports === 'always-and-inside-groups');
+
+      const shouldAssertNoNewlineWithinGroup = (isTypeOnlyImportAndRelevant || isNormalImportFollowingTypeOnlyImportAndRelevant)
+          && newlinesBetweenTypeOnlyImports !== 'always-and-inside-groups'
+        || !isTypeOnlyImportAndRelevant && !isNormalImportFollowingTypeOnlyImportAndRelevant
+          && newlinesBetweenImports !== 'always-and-inside-groups';
+
+      const shouldAssertNoNewlineBetweenGroup = !isSortingTypesGroup
+        || !isNormalImportFollowingTypeOnlyImportAndRelevant
+        || newlinesBetweenTypeOnlyImports === 'never';
+
+      if (shouldAssertNewlineBetweenGroups) {
+        if (currentImport.rank !== previousImport.rank && emptyLinesBetween === 0) {
+          if (distinctGroup || !distinctGroup && isStartOfDistinctGroup) {
+            context.report({
+              node: previousImport.node,
+              message: 'There should be at least one empty line between import groups',
+              fix: fixNewLineAfterImport(context, previousImport),
+            });
+          }
+        } else if (emptyLinesBetween > 0 && shouldAssertNoNewlineWithinGroup) {
+          if (
+            distinctGroup && currentImport.rank === previousImport.rank
+            || !distinctGroup && !isStartOfDistinctGroup
+          ) {
+            context.report({
+              node: previousImport.node,
+              message: 'There should be no empty line within import group',
+              fix: removeNewLineAfterImport(context, currentImport, previousImport),
+            });
+          }
         }
+      } else if (emptyLinesBetween > 0 && shouldAssertNoNewlineBetweenGroup) {
+        context.report({
+          node: previousImport.node,
+          message: 'There should be no empty line between import groups',
+          fix: removeNewLineAfterImport(context, currentImport, previousImport),
+        });
       }
-    } else if (emptyLinesBetween > 0) {
-      context.report({
-        node: previousImport.node,
-        message: 'There should be no empty line between import groups',
-        fix: removeNewLineAfterImport(context, currentImport, previousImport),
-      });
     }
 
     previousImport = currentImport;
@@ -793,6 +830,14 @@ module.exports = {
               'never',
             ],
           },
+          'newlines-between-types': {
+            enum: [
+              'ignore',
+              'always',
+              'always-and-inside-groups',
+              'never',
+            ],
+          },
           sortTypesGroup: {
             type: 'boolean',
             default: false,
@@ -845,13 +890,22 @@ module.exports = {
           },
         },
         additionalProperties: false,
+        dependencies: {
+          'newlines-between-types': {
+            properties: {
+              sortTypesGroup: { enum: [true] },
+            },
+            required: ['sortTypesGroup'],
+          },
+        },
       },
     ],
   },
 
   create(context) {
     const options = context.options[0] || {};
     const newlinesBetweenImports = options['newlines-between'] || 'ignore';
+    const newlinesBetweenTypeOnlyImports = options['newlines-between-types'] || newlinesBetweenImports;
     const pathGroupsExcludedImportTypes = new Set(options.pathGroupsExcludedImportTypes || ['builtin', 'external', 'object']);
     const sortTypesGroup = options.sortTypesGroup;
 
@@ -1115,8 +1169,8 @@ module.exports = {
       },
       'Program:exit'() {
         importMap.forEach((imported) => {
-          if (newlinesBetweenImports !== 'ignore') {
-            makeNewlinesBetweenReport(context, imported, newlinesBetweenImports, distinctGroup);
+          if (newlinesBetweenImports !== 'ignore' || newlinesBetweenTypeOnlyImports !== 'ignore') {
+            makeNewlinesBetweenReport(context, imported, newlinesBetweenImports, newlinesBetweenTypeOnlyImports, distinctGroup, isSortingTypesGroup);
           }
 
           if (alphabetize.order !== 'ignore') {