diff --git a/CHANGELOG.md b/CHANGELOG.md index 6aff8cda1..56bb20c34 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange - add [`enforce-node-protocol-usage`] rule and `import/node-version` setting ([#3024], thanks [@GoldStrikeArch] and [@sevenc-nanashi]) - add TypeScript types ([#3097], thanks [@G-Rath]) - [`extensions`]: add `pathGroupOverrides to allow enforcement decision overrides based on specifier ([#3105], thanks [@Xunnamius]) +- [`order`]: add options (`sortTypesGroup`, `newlines-between-types`, `consolidateIslands`) to allow intragroup sorting of type-only imports ([#3104], thanks [@Xunnamius]) ### Fixed - [`no-unused-modules`]: provide more meaningful error message when no .eslintrc is present ([#3116], thanks [@michaelfaith]) diff --git a/docs/rules/order.md b/docs/rules/order.md index 02a2b4bed..1a9e048cf 100644 --- a/docs/rules/order.md +++ b/docs/rules/order.md @@ -106,6 +106,9 @@ This rule supports the following options (none of which are required): - [`alphabetize`][30] - [`named`][33] - [`warnOnUnassignedImports`][5] + - [`sortTypesGroup`][7] + - [`newlines-between-types`][27] + - [`consolidateIslands`][25] --- @@ -156,7 +159,7 @@ Roughly speaking, the grouping algorithm is as follows: 1. If the import has no corresponding identifiers (e.g. `import './my/thing.js'`), is otherwise "unassigned," or is an unsupported use of `require()`, and [`warnOnUnassignedImports`][5] is disabled, it will be ignored entirely since the order of these imports may be important for their [side-effects][31] 2. If the import is part of an arcane TypeScript declaration (e.g. `import log = console.log`), it will be considered **object**. However, note that external module references (e.g. `import x = require('z')`) are treated as normal `require()`s and import-exports (e.g. `export import w = y;`) are ignored entirely -3. If the import is [type-only][6], and `"type"` is in `groups`, it will be considered **type** (with additional implications if using [`pathGroups`][8] and `"type"` is in [`pathGroupsExcludedImportTypes`][9]) +3. If the import is [type-only][6], `"type"` is in `groups`, and [`sortTypesGroup`][7] is disabled, it will be considered **type** (with additional implications if using [`pathGroups`][8] and `"type"` is in [`pathGroupsExcludedImportTypes`][9]) 4. If the import's specifier matches [`import/internal-regex`][28], it will be considered **internal** 5. If the import's specifier is an absolute path, it will be considered **unknown** 6. If the import's specifier has the name of a Node.js core module (using [is-core-module][10]), it will be considered **builtin** @@ -171,7 +174,7 @@ Roughly speaking, the grouping algorithm is as follows: 15. If the import's specifier has a name that starts with a word character, it will be considered **external** 16. If this point is reached, the import will be ignored entirely -At the end of the process, if they co-exist in the same file, all top-level `require()` statements that haven't been ignored are shifted (with respect to their order) below any ES6 `import` or similar declarations. +At the end of the process, if they co-exist in the same file, all top-level `require()` statements that haven't been ignored are shifted (with respect to their order) below any ES6 `import` or similar declarations. Finally, any type-only declarations are potentially reorganized according to [`sortTypesGroup`][7]. ### `pathGroups` @@ -533,6 +536,457 @@ import path from 'path'; import './styles.css'; ``` +### `sortTypesGroup` + +Valid values: `boolean` \ +Default: `false` + +> \[!NOTE] +> +> This setting is only meaningful when `"type"` is included in [`groups`][18]. + +Sort [type-only imports][6] separately from normal non-type imports. + +When enabled, the intragroup sort order of [type-only imports][6] will mirror the intergroup ordering of normal imports as defined by [`groups`][18], [`pathGroups`][8], etc. + +#### Example + +Given the following settings: + +```jsonc +{ + "import/order": ["error", { + "groups": ["type", "builtin", "parent", "sibling", "index"], + "alphabetize": { "order": "asc" } + }] +} +``` + +This will fail the rule check even though it's logically ordered as we expect (builtins come before parents, parents come before siblings, siblings come before indices), the only difference is we separated type-only imports from normal imports: + +```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 "./"; +``` + +This happens because [type-only imports][6] are considered part of one global +[`"type"` group](#how-imports-are-grouped) by default. However, if we set +`sortTypesGroup` to `true`: + +```jsonc +{ + "import/order": ["error", { + "groups": ["type", "builtin", "parent", "sibling", "index"], + "alphabetize": { "order": "asc" }, + "sortTypesGroup": true + }] +} +``` + +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`][25] 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 "./"; +``` + +### `consolidateIslands` + +Valid values: `"inside-groups" | "never"` \ +Default: `"never"` + +> \[!NOTE] +> +> This setting is only meaningful when [`newlines-between`][20] and/or [`newlines-between-types`][27] is set to `"always-and-inside-groups"`. + +When set to `"inside-groups"`, this ensures imports spanning multiple lines are separated from other imports with a new line while single-line imports are grouped together (and the space between them consolidated) if they belong to the same [group][18] or [`pathGroups`][8]. + +> \[!IMPORTANT] +> +> When all of the following are true: +> +> - `consolidateIslands` is set to `"inside-groups"` +> - [`newlines-between`][20] is set to `"always-and-inside-groups"` +> - [`newlines-between-types`][27] is set to `"never"` +> - [`sortTypesGroup`][7] is set to `true` +> +> Then [`newlines-between-types`][27] will yield to `consolidateIslands` and allow new lines to separate multi-line imports and a single new line to separate all [type-only imports][6] from all normal imports. Other than that, [`newlines-between-types: "never"`][27] functions as described. +> +> This configuration is useful to keep type-only imports stacked tightly +> together at the bottom of your import block to preserve space while still +> logically organizing normal imports for quick and pleasant reference. + +#### Example + +Given the following settings: + +```jsonc +{ + "import/order": ["error", { + "newlines-between": "always-and-inside-groups", + "consolidateIslands": "inside-groups" + }] +} +``` + +This will fail the rule check: + +```ts +var fs = require('fs'); +var path = require('path'); +var { util1, util2, util3 } = require('util'); +var async = require('async'); +var relParent1 = require('../foo'); +var { + relParent21, + relParent22, + relParent23, + relParent24, +} = require('../'); +var relParent3 = require('../bar'); +var { sibling1, + sibling2, sibling3 } = require('./foo'); +var sibling2 = require('./bar'); +var sibling3 = require('./foobar'); +``` + +While this will succeed (and is what `--fix` would yield): + +```ts +var fs = require('fs'); +var path = require('path'); +var { util1, util2, util3 } = require('util'); + +var async = require('async'); + +var relParent1 = require('../foo'); + +var { + relParent21, + relParent22, + relParent23, + relParent24, +} = require('../'); + +var relParent3 = require('../bar'); + +var { sibling1, + sibling2, sibling3 } = require('./foo'); + +var sibling2 = require('./bar'); +var sibling3 = require('./foobar'); +``` + +Note the intragroup "islands" of grouped single-line imports, as well as multi-line imports, are surrounded by new lines. At the same time, note the typical new lines separating different groups are still maintained thanks to [`newlines-between`][20]. + +The same holds true for the next example; when given the following settings: + +```jsonc +{ + "import/order": ["error", { + "alphabetize": { "order": "asc" }, + "groups": ["external", "internal", "index", "type"], + "pathGroups": [ + { + "pattern": "dirA/**", + "group": "internal", + "position": "after" + }, + { + "pattern": "dirB/**", + "group": "internal", + "position": "before" + }, + { + "pattern": "dirC/**", + "group": "internal" + } + ], + "newlines-between": "always-and-inside-groups", + "newlines-between-types": "never", + "pathGroupsExcludedImportTypes": [], + "sortTypesGroup": true, + "consolidateIslands": "inside-groups" + }] +} +``` + +> [!IMPORTANT] +> +> **Pay special attention to the value of [`pathGroupsExcludedImportTypes`][9]** in this example's settings. +> Without it, the successful example below would fail. +> This is because the imports with specifiers starting with "dirA/", "dirB/", and "dirC/" are all [considered part of the `"external"` group](#how-imports-are-grouped), and imports in that group are excluded from [`pathGroups`][8] matching by default. +> +> The fix is to remove `"external"` (and, in this example, the others) from [`pathGroupsExcludedImportTypes`][9]. + +This will fail the rule check: + +```ts +import c from 'Bar'; +import d from 'bar'; +import { + aa, + bb, + cc, + dd, + ee, + ff, + gg +} from 'baz'; +import { + hh, + ii, + jj, + kk, + ll, + mm, + nn +} from 'fizz'; +import a from 'foo'; +import b from 'dirA/bar'; +import index from './'; +import type { AA, + BB, CC } from 'abc'; +import type { Z } from 'fizz'; +import type { + A, + B +} from 'foo'; +import type { C2 } from 'dirB/Bar'; +import type { + D2, + X2, + Y2 +} from 'dirB/bar'; +import type { E2 } from 'dirB/baz'; +import type { C3 } from 'dirC/Bar'; +import type { + D3, + X3, + Y3 +} from 'dirC/bar'; +import type { E3 } from 'dirC/baz'; +import type { F3 } from 'dirC/caz'; +import type { C1 } from 'dirA/Bar'; +import type { + D1, + X1, + Y1 +} from 'dirA/bar'; +import type { E1 } from 'dirA/baz'; +import type { F } from './index.js'; +import type { G } from './aaa.js'; +import type { H } from './bbb'; +``` + +While this will succeed (and is what `--fix` would yield): + +```ts +import c from 'Bar'; +import d from 'bar'; + +import { + aa, + bb, + cc, + dd, + ee, + ff, + gg +} from 'baz'; + +import { + hh, + ii, + jj, + kk, + ll, + mm, + nn +} from 'fizz'; + +import a from 'foo'; + +import b from 'dirA/bar'; + +import index from './'; + +import type { AA, + BB, CC } from 'abc'; + +import type { Z } from 'fizz'; + +import type { + A, + B +} from 'foo'; + +import type { C2 } from 'dirB/Bar'; + +import type { + D2, + X2, + Y2 +} from 'dirB/bar'; + +import type { E2 } from 'dirB/baz'; +import type { C3 } from 'dirC/Bar'; + +import type { + D3, + X3, + Y3 +} from 'dirC/bar'; + +import type { E3 } from 'dirC/baz'; +import type { F3 } from 'dirC/caz'; +import type { C1 } from 'dirA/Bar'; + +import type { + D1, + X1, + Y1 +} from 'dirA/bar'; + +import type { E1 } from 'dirA/baz'; +import type { F } from './index.js'; +import type { G } from './aaa.js'; +import type { H } from './bbb'; +``` + ## Related - [`import/external-module-folders`][29] @@ -543,6 +997,7 @@ import './styles.css'; [4]: https://nodejs.org/api/esm.html#terminology [5]: #warnonunassignedimports [6]: https://www.typescriptlang.org/docs/handbook/release-notes/typescript-3-8.html#type-only-imports-and-export +[7]: #sortTypesGroup [8]: #pathgroups [9]: #pathgroupsexcludedimporttypes [10]: https://www.npmjs.com/package/is-core-module diff --git a/src/rules/order.js b/src/rules/order.js index d6f25ddd3..a80ee268d 100644 --- a/src/rules/order.js +++ b/src/rules/order.js @@ -513,22 +513,34 @@ function computePathRank(ranks, pathGroups, path, maxPosition) { } } -function computeRank(context, ranks, importEntry, excludedImportTypes) { +function computeRank(context, ranks, importEntry, excludedImportTypes, isSortingTypesGroup) { let impType; let rank; + + const isTypeGroupInGroups = ranks.omittedTypes.indexOf('type') === -1; + const isTypeOnlyImport = importEntry.node.importKind === 'type'; + const isExcludedFromPathRank = isTypeOnlyImport && isTypeGroupInGroups && excludedImportTypes.has('type'); + if (importEntry.type === 'import:object') { impType = 'object'; - } else if (importEntry.node.importKind === 'type' && ranks.omittedTypes.indexOf('type') === -1) { + } else if (isTypeOnlyImport && isTypeGroupInGroups && !isSortingTypesGroup) { impType = 'type'; } else { impType = importType(importEntry.value, context); } - if (!excludedImportTypes.has(impType)) { + + if (!excludedImportTypes.has(impType) && !isExcludedFromPathRank) { rank = computePathRank(ranks.groups, ranks.pathGroups, importEntry.value, ranks.maxPosition); } + if (typeof rank === 'undefined') { rank = ranks.groups[impType]; } + + if (isTypeOnlyImport && isSortingTypesGroup) { + rank = ranks.groups.type + rank / 10; + } + if (importEntry.type !== 'import' && !importEntry.type.startsWith('import:')) { rank += 100; } @@ -536,8 +548,8 @@ function computeRank(context, ranks, importEntry, excludedImportTypes) { return rank; } -function registerNode(context, importEntry, ranks, imported, excludedImportTypes) { - const rank = computeRank(context, ranks, importEntry, excludedImportTypes); +function registerNode(context, importEntry, ranks, imported, excludedImportTypes, isSortingTypesGroup) { + const rank = computeRank(context, ranks, importEntry, excludedImportTypes, isSortingTypesGroup); if (rank !== -1) { imported.push({ ...importEntry, rank }); } @@ -681,8 +693,10 @@ function makeNewlinesBetweenReport(context, imported, newlinesBetweenImports, di const emptyLinesBetween = getNumberOfEmptyLinesBetween(currentImport, previousImport); const isStartOfDistinctGroup = getIsStartOfDistinctGroup(currentImport, previousImport); - if (newlinesBetweenImports === 'always' - || newlinesBetweenImports === 'always-and-inside-groups') { + if ( + newlinesBetweenImports === 'always' + || newlinesBetweenImports === 'always-and-inside-groups' + ) { if (currentImport.rank !== previousImport.rank && emptyLinesBetween === 0) { if (distinctGroup || !distinctGroup && isStartOfDistinctGroup) { context.report({ @@ -691,8 +705,10 @@ function makeNewlinesBetweenReport(context, imported, newlinesBetweenImports, di fix: fixNewLineAfterImport(context, previousImport), }); } - } else if (emptyLinesBetween > 0 - && newlinesBetweenImports !== 'always-and-inside-groups') { + } else if ( + emptyLinesBetween > 0 + && newlinesBetweenImports !== 'always-and-inside-groups' + ) { if (distinctGroup && currentImport.rank === previousImport.rank || !distinctGroup && !isStartOfDistinctGroup) { context.report({ node: previousImport.node, @@ -781,6 +797,10 @@ module.exports = { 'never', ], }, + sortTypesGroup: { + type: 'boolean', + default: false, + }, named: { default: false, oneOf: [{ @@ -837,6 +857,7 @@ module.exports = { const options = context.options[0] || {}; const newlinesBetweenImports = options['newlines-between'] || 'ignore'; const pathGroupsExcludedImportTypes = new Set(options.pathGroupsExcludedImportTypes || ['builtin', 'external', 'object']); + const sortTypesGroup = options.sortTypesGroup; const named = { types: 'mixed', @@ -879,6 +900,9 @@ module.exports = { const importMap = new Map(); const exportMap = new Map(); + const isTypeGroupInGroups = ranks.omittedTypes.indexOf('type') === -1; + const isSortingTypesGroup = isTypeGroupInGroups && sortTypesGroup; + function getBlockImports(node) { if (!importMap.has(node)) { importMap.set(node, []); @@ -932,6 +956,7 @@ module.exports = { ranks, getBlockImports(node.parent), pathGroupsExcludedImportTypes, + isSortingTypesGroup, ); if (named.import) { @@ -983,6 +1008,7 @@ module.exports = { ranks, getBlockImports(node.parent), pathGroupsExcludedImportTypes, + isSortingTypesGroup, ); }, CallExpression(node) { @@ -1005,6 +1031,7 @@ module.exports = { ranks, getBlockImports(block), pathGroupsExcludedImportTypes, + isSortingTypesGroup, ); }, ...named.require && { diff --git a/tests/src/rules/order.js b/tests/src/rules/order.js index 1f60e2ad1..f9e1043a8 100644 --- a/tests/src/rules/order.js +++ b/tests/src/rules/order.js @@ -3313,6 +3313,188 @@ context('TypeScript', function () { ], }), ] : [], + // Option sortTypesGroup: false (default) + test({ + code: ` + import c from 'Bar'; + import a from 'foo'; + + import type { C } from 'dirA/Bar'; + import b from 'dirA/bar'; + import type { D } from 'dirA/bar'; + + import index from './'; + + import type { AA } from 'abc'; + import type { A } from 'foo'; + `, + ...parserConfig, + options: [ + { + alphabetize: { order: 'asc' }, + groups: ['external', 'internal', 'index', 'type'], + pathGroups: [ + { + pattern: 'dirA/**', + group: 'internal', + }, + ], + 'newlines-between': 'always', + pathGroupsExcludedImportTypes: [], + }, + ], + }), + test({ + code: ` + import c from 'Bar'; + import a from 'foo'; + + import type { C } from 'dirA/Bar'; + import b from 'dirA/bar'; + import type { D } from 'dirA/bar'; + + import index from './'; + + import type { AA } from 'abc'; + import type { A } from 'foo'; + `, + ...parserConfig, + options: [ + { + alphabetize: { order: 'asc' }, + groups: ['external', 'internal', 'index', 'type'], + pathGroups: [ + { + pattern: 'dirA/**', + group: 'internal', + }, + ], + 'newlines-between': 'always', + pathGroupsExcludedImportTypes: [], + sortTypesGroup: false, + }, + ], + }), + // Option sortTypesGroup: true and 'type' in pathGroupsExcludedImportTypes + test({ + code: ` + import c from 'Bar'; + import a from 'foo'; + + import b from 'dirA/bar'; + + import index from './'; + + import type { AA } from 'abc'; + import type { C } from 'dirA/Bar'; + import type { D } from 'dirA/bar'; + import type { A } from 'foo'; + `, + ...parserConfig, + options: [ + { + alphabetize: { order: 'asc' }, + groups: ['external', 'internal', 'index', 'type'], + pathGroups: [ + { + pattern: 'dirA/**', + group: 'internal', + }, + ], + 'newlines-between': 'always', + pathGroupsExcludedImportTypes: ['type'], + sortTypesGroup: true, + }, + ], + }), + // Option sortTypesGroup: true and 'type' omitted from groups + test({ + code: ` + import c from 'Bar'; + import type { AA } from 'abc'; + import a from 'foo'; + import type { A } from 'foo'; + + import type { C } from 'dirA/Bar'; + import b from 'dirA/bar'; + import type { D } from 'dirA/bar'; + + import index from './'; + `, + ...parserConfig, + options: [ + { + alphabetize: { order: 'asc' }, + groups: ['external', 'internal', 'index'], + pathGroups: [ + { + pattern: 'dirA/**', + group: 'internal', + }, + ], + 'newlines-between': 'always', + pathGroupsExcludedImportTypes: [], + // Becomes a no-op without "type" in groups + sortTypesGroup: true, + }, + ], + }), + test({ + code: ` + import c from 'Bar'; + import type { AA } from 'abc'; + import a from 'foo'; + import type { A } from 'foo'; + + import type { C } from 'dirA/Bar'; + import b from 'dirA/bar'; + import type { D } from 'dirA/bar'; + + import index from './'; + `, + ...parserConfig, + options: [ + { + alphabetize: { order: 'asc' }, + groups: ['external', 'internal', 'index'], + pathGroups: [ + { + pattern: 'dirA/**', + group: 'internal', + }, + ], + 'newlines-between': 'always', + pathGroupsExcludedImportTypes: [], + }, + ], + }), + // Option: sortTypesGroup: true puts type imports in the same order as regular imports (from issue #2441, PR #2615) + test({ + code: ` + 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 "./"; + `, + ...parserConfig, + options: [ + { + groups: ['type', 'builtin', 'parent', 'sibling', 'index'], + alphabetize: { + order: 'asc', + caseInsensitive: true, + }, + sortTypesGroup: true, + }, + ], + }), ), invalid: [].concat( // Option alphabetize: {order: 'asc'}