diff --git a/CHANGELOG.md b/CHANGELOG.md index 1cb9b5627..a2a436a38 100644 --- 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 index 996df915c..3dee4b6de 100644 --- 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 index ead2fdff9..17e8735e1 100644 --- 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, ); } @@ -535,6 +535,10 @@ function computeRank(context, ranks, importEntry, excludedImportTypes, isSorting if (typeof rank === 'undefined') { rank = ranks.groups[impType]; + + if (typeof rank === 'undefined') { + return -1; + } } if (isTypeOnlyImport && isSortingTypesGroup) { @@ -677,7 +681,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 +694,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 +834,14 @@ module.exports = { 'never', ], }, + 'newlines-between-types': { + enum: [ + 'ignore', + 'always', + 'always-and-inside-groups', + 'never', + ], + }, sortTypesGroup: { type: 'boolean', default: false, @@ -845,6 +894,14 @@ module.exports = { }, }, additionalProperties: false, + dependencies: { + 'newlines-between-types': { + properties: { + sortTypesGroup: { enum: [true] }, + }, + required: ['sortTypesGroup'], + }, + }, }, ], }, @@ -852,6 +909,7 @@ module.exports = { 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 +1173,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') { diff --git a/tests/src/rules/order.js b/tests/src/rules/order.js index 6da42474c..bde928f42 100644 --- a/tests/src/rules/order.js +++ b/tests/src/rules/order.js @@ -3115,7 +3115,6 @@ context('TypeScript', function () { }), // Option alphabetize: {order: 'asc'} with type group & path group test({ - // only: true, code: ` import c from 'Bar'; import a from 'foo'; @@ -3145,7 +3144,6 @@ context('TypeScript', function () { }), // Option alphabetize: {order: 'asc'} with path group test({ - // only: true, code: ` import c from 'Bar'; import type { A } from 'foo'; @@ -3440,6 +3438,168 @@ context('TypeScript', function () { }, ], }), + // Option sortTypesGroup: true and newlines-between-types defaults to the value of newlines-between + 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 { A } from 'foo'; + + import type { C } from 'dirA/Bar'; + import type { D } from 'dirA/bar'; + `, + ...parserConfig, + options: [ + { + alphabetize: { order: 'asc' }, + groups: ['external', 'internal', 'index', 'type'], + pathGroups: [ + { + pattern: 'dirA/**', + group: 'internal', + }, + ], + 'newlines-between': 'always', + pathGroupsExcludedImportTypes: [], + sortTypesGroup: true, + }, + ], + }), + // Option: sortTypesGroup: true and newlines-between-types: 'always' (takes precedence over newlines-between between type-only and normal imports) + 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 { A } from 'foo'; + + import type { C } from 'dirA/Bar'; + import type { D } from 'dirA/bar'; + `, + ...parserConfig, + options: [ + { + alphabetize: { order: 'asc' }, + groups: ['external', 'internal', 'index', 'type'], + pathGroups: [ + { + pattern: 'dirA/**', + group: 'internal', + }, + ], + 'newlines-between': 'never', + 'newlines-between-types': 'always', + pathGroupsExcludedImportTypes: [], + sortTypesGroup: true, + }, + ], + }), + // Option: sortTypesGroup: true and newlines-between-types: 'never' (takes precedence over newlines-between between type-only and normal imports) + 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 { A } from 'foo'; + import type { C } from 'dirA/Bar'; + import type { D } from 'dirA/bar'; + `, + ...parserConfig, + options: [ + { + alphabetize: { order: 'asc' }, + groups: ['external', 'internal', 'index', 'type'], + pathGroups: [ + { + pattern: 'dirA/**', + group: 'internal', + }, + ], + 'newlines-between': 'always', + 'newlines-between-types': 'never', + pathGroupsExcludedImportTypes: [], + sortTypesGroup: true, + }, + ], + }), + // Option: sortTypesGroup: true and newlines-between-types: 'ignore' + 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 { A } from 'foo'; + import type { C } from 'dirA/Bar'; + import type { D } from 'dirA/bar'; + `, + ...parserConfig, + options: [ + { + alphabetize: { order: 'asc' }, + groups: ['external', 'internal', 'index', 'type'], + pathGroups: [ + { + pattern: 'dirA/**', + group: 'internal', + }, + ], + 'newlines-between': 'never', + 'newlines-between-types': 'ignore', + pathGroupsExcludedImportTypes: [], + sortTypesGroup: true, + }, + ], + }), + // Option: sortTypesGroup: true and newlines-between-types: 'always-and-inside-groups' + 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 { A } from 'foo'; + + import type { C } from 'dirA/Bar'; + + import type { D } from 'dirA/bar'; + `, + ...parserConfig, + options: [ + { + alphabetize: { order: 'asc' }, + groups: ['external', 'internal', 'index', 'type'], + pathGroups: [ + { + pattern: 'dirA/**', + group: 'internal', + }, + ], + 'newlines-between': 'never', + 'newlines-between-types': 'always-and-inside-groups', + pathGroupsExcludedImportTypes: [], + sortTypesGroup: true, + }, + ], + }), // Option: sortTypesGroup: true puts type imports in the same order as regular imports (from issue #2441, PR #2615) test({ code: ` @@ -3467,6 +3627,146 @@ context('TypeScript', function () { }, ], }), + // Options: sortTypesGroup + newlines-between-types example #1 from the documentation (pass) + 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'], + sortTypesGroup: true, + 'newlines-between': 'always', + 'newlines-between-types': 'ignore', + }, + ], + }), + test({ + code: ` + import a from "fs"; + import b from "path"; + + import c from "../foo.js"; + + import d from "./bar.js"; + + import e from "./"; + + 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 './'; + `, + ...parserConfig, + options: [ + { + groups: ['builtin', 'parent', 'sibling', 'index', 'type'], + sortTypesGroup: true, + 'newlines-between': 'always', + 'newlines-between-types': 'ignore', + }, + ], + }), + // Options: sortTypesGroup + newlines-between-types example #2 from the documentation (pass) + 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'], + sortTypesGroup: true, + 'newlines-between': 'never', + 'newlines-between-types': 'always', + }, + ], + }), + test({ + code: ` + import a from "fs"; + import b from "path"; + import c from "../foo.js"; + import d from "./bar.js"; + import e from "./"; + + 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 './'; + `, + ...parserConfig, + options: [ + { + groups: ['builtin', 'parent', 'sibling', 'index', 'type'], + sortTypesGroup: true, + 'newlines-between': 'never', + 'newlines-between-types': 'always', + }, + ], + }), + // Ensure the rule doesn't choke and die on absolute paths trying to pass NaN around + test({ + code: ` + import fs from 'fs'; + + import '@scoped/package'; + import type { B } from 'fs'; + + import type { A1 } from '/bad/bad/bad/bad'; + import './a/b/c'; + import type { A2 } from '/bad/bad/bad/bad'; + import type { A3 } from '/bad/bad/bad/bad'; + import type { D1 } from '/bad/bad/not/good'; + import type { D2 } from '/bad/bad/not/good'; + import type { D3 } from '/bad/bad/not/good'; + + import type { C } from '@something/else'; + + import type { E } from './index.js'; + `, + ...parserConfig, + options: [ + { + alphabetize: { order: 'asc' }, + groups: ['builtin', 'type', 'unknown', 'external'], + sortTypesGroup: true, + 'newlines-between': 'always', + }, + ], + }), ), invalid: [].concat( // Option alphabetize: {order: 'asc'} @@ -3721,6 +4021,117 @@ context('TypeScript', function () { }], }), + // Options: sortTypesGroup + newlines-between-types example #1 from the documentation (fail) + 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 "./"; + `, + output: ` + 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'], + sortTypesGroup: true, + 'newlines-between': 'always', + }, + ], + errors: [ + { + message: 'There should be at least one empty line between import groups', + line: 3, + }, + { + message: 'There should be at least one empty line between import groups', + line: 4, + }, + { + message: 'There should be at least one empty line between import groups', + line: 5, + }, + ], + }), + + // Options: sortTypesGroup + newlines-between-types example #2 from the documentation (fail) + 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 "./"; + `, + output: ` + 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'], + sortTypesGroup: true, + 'newlines-between': 'always', + 'newlines-between-types': 'never', + }, + ], + errors: [ + { + message: 'There should be no empty line between import groups', + line: 6, + }, + ], + }), + supportsExportTypeSpecifiers ? [ test({ code: `