Skip to content

Commit e1e5538

Browse files
Xunnamiusljharb
Xunnamius
authored and
ljharb
committed
[New] order: add newlines-between-types option to control intragroup sorting of type-only imports
Closes import-js#2912 Closes import-js#2347 Closes import-js#2441 Subsumes import-js#2615
1 parent 668d493 commit e1e5538

File tree

4 files changed

+603
-33
lines changed

4 files changed

+603
-33
lines changed

‎CHANGELOG.md

+2
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange
1111
- add TypeScript types ([#3097], thanks [@G-Rath])
1212
- [`extensions`]: add `pathGroupOverrides to allow enforcement decision overrides based on specifier ([#3105], thanks [@Xunnamius])
1313
- [`order`]: add `sortTypesGroup` option to allow intragroup sorting of type-only imports ([#3104], thanks [@Xunnamius])
14+
- [`order`]: add `newlines-between-types` option to control intragroup sorting of type-only imports ([#3127], thanks [@Xunnamius])
1415

1516
### Fixed
1617
- [`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.
11721173

11731174
[#3151]: https://github.com/import-js/eslint-plugin-import/pull/3151
11741175
[#3138]: https://github.com/import-js/eslint-plugin-import/pull/3138
1176+
[#3127]: https://github.com/import-js/eslint-plugin-import/pull/3127
11751177
[#3125]: https://github.com/import-js/eslint-plugin-import/pull/3125
11761178
[#3122]: https://github.com/import-js/eslint-plugin-import/pull/3122
11771179
[#3116]: https://github.com/import-js/eslint-plugin-import/pull/3116

‎docs/rules/order.md

+131
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,7 @@ This rule supports the following options (none of which are required):
107107
- [`named`][33]
108108
- [`warnOnUnassignedImports`][5]
109109
- [`sortTypesGroup`][7]
110+
- [`newlines-between-types`][27]
110111

111112
---
112113

@@ -592,6 +593,135 @@ This happens because [type-only imports][6] are considered part of one global
592593

593594
The same example will pass.
594595

596+
### `newlines-between-types`
597+
598+
Valid values: `"ignore" | "always" | "always-and-inside-groups" | "never"` \
599+
Default: the value of [`newlines-between`][20]
600+
601+
> \[!NOTE]
602+
>
603+
> This setting is only meaningful when [`sortTypesGroup`][7] is enabled.
604+
605+
`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.
606+
607+
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].
608+
609+
#### Example
610+
611+
Given the following settings:
612+
613+
```jsonc
614+
{
615+
"import/order": ["error", {
616+
"groups": ["type", "builtin", "parent", "sibling", "index"],
617+
"sortTypesGroup": true,
618+
"newlines-between": "always"
619+
}]
620+
}
621+
```
622+
623+
This will fail the rule check:
624+
625+
```ts
626+
import type A from "fs";
627+
import type B from "path";
628+
import type C from "../foo.js";
629+
import type D from "./bar.js";
630+
import type E from './';
631+
632+
import a from "fs";
633+
import b from "path";
634+
635+
import c from "../foo.js";
636+
637+
import d from "./bar.js";
638+
639+
import e from "./";
640+
```
641+
642+
However, if we set `newlines-between-types` to `"ignore"`:
643+
644+
```jsonc
645+
{
646+
"import/order": ["error", {
647+
"groups": ["type", "builtin", "parent", "sibling", "index"],
648+
"sortTypesGroup": true,
649+
"newlines-between": "always",
650+
"newlines-between-types": "ignore"
651+
}]
652+
}
653+
```
654+
655+
The same example will pass.
656+
657+
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`_.
658+
659+
> \[!IMPORTANT]
660+
>
661+
> 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.
662+
663+
The next example will pass even though there's a new line preceding the normal import and [`newlines-between`][20] is set to `"never"`:
664+
665+
```jsonc
666+
{
667+
"import/order": ["error", {
668+
"groups": ["type", "builtin", "parent", "sibling", "index"],
669+
"sortTypesGroup": true,
670+
"newlines-between": "never",
671+
"newlines-between-types": "always"
672+
}]
673+
}
674+
```
675+
676+
```ts
677+
import type A from "fs";
678+
679+
import type B from "path";
680+
681+
import type C from "../foo.js";
682+
683+
import type D from "./bar.js";
684+
685+
import type E from './';
686+
687+
import a from "fs";
688+
import b from "path";
689+
import c from "../foo.js";
690+
import d from "./bar.js";
691+
import e from "./";
692+
```
693+
694+
While the following fails due to the new line between the last type import and the first normal import:
695+
696+
```jsonc
697+
{
698+
"import/order": ["error", {
699+
"groups": ["type", "builtin", "parent", "sibling", "index"],
700+
"sortTypesGroup": true,
701+
"newlines-between": "always",
702+
"newlines-between-types": "never"
703+
}]
704+
}
705+
```
706+
707+
```ts
708+
import type A from "fs";
709+
import type B from "path";
710+
import type C from "../foo.js";
711+
import type D from "./bar.js";
712+
import type E from './';
713+
714+
import a from "fs";
715+
716+
import b from "path";
717+
718+
import c from "../foo.js";
719+
720+
import d from "./bar.js";
721+
722+
import e from "./";
723+
```
724+
595725
## Related
596726

597727
- [`import/external-module-folders`][29]
@@ -617,6 +747,7 @@ The same example will pass.
617747
[21]: https://eslint.org/docs/latest/rules/no-multiple-empty-lines
618748
[22]: https://prettier.io
619749
[23]: https://www.typescriptlang.org/docs/handbook/release-notes/typescript-4-5.html#type-modifiers-on-import-names
750+
[27]: #newlines-between-types
620751
[28]: ../../README.md#importinternal-regex
621752
[29]: ../../README.md#importexternal-module-folders
622753
[30]: #alphabetize

‎src/rules/order.js

+87-33
Original file line numberDiff line numberDiff line change
@@ -416,7 +416,7 @@ const compareString = (a, b) => {
416416
};
417417

418418
/** Some parsers (languages without types) don't provide ImportKind */
419-
const DEAFULT_IMPORT_KIND = 'value';
419+
const DEFAULT_IMPORT_KIND = 'value';
420420
const getNormalizedValue = (node, toLowerCase) => {
421421
const value = node.value;
422422
return toLowerCase ? String(value).toLowerCase() : value;
@@ -462,8 +462,8 @@ function getSorter(alphabetizeOptions) {
462462
// In case the paths are equal (result === 0), sort them by importKind
463463
if (!result && multiplierImportKind) {
464464
result = multiplierImportKind * compareString(
465-
nodeA.node.importKind || DEAFULT_IMPORT_KIND,
466-
nodeB.node.importKind || DEAFULT_IMPORT_KIND,
465+
nodeA.node.importKind || DEFAULT_IMPORT_KIND,
466+
nodeB.node.importKind || DEFAULT_IMPORT_KIND,
467467
);
468468
}
469469

@@ -677,7 +677,7 @@ function removeNewLineAfterImport(context, currentImport, previousImport) {
677677
return undefined;
678678
}
679679

680-
function makeNewlinesBetweenReport(context, imported, newlinesBetweenImports, distinctGroup) {
680+
function makeNewlinesBetweenReport(context, imported, newlinesBetweenImports, newlinesBetweenTypeOnlyImports, distinctGroup, isSortingTypesGroup) {
681681
const getNumberOfEmptyLinesBetween = (currentImport, previousImport) => {
682682
const linesBetweenImports = getSourceCode(context).lines.slice(
683683
previousImport.node.loc.end.line,
@@ -690,35 +690,72 @@ function makeNewlinesBetweenReport(context, imported, newlinesBetweenImports, di
690690
let previousImport = imported[0];
691691

692692
imported.slice(1).forEach(function (currentImport) {
693-
const emptyLinesBetween = getNumberOfEmptyLinesBetween(currentImport, previousImport);
694-
const isStartOfDistinctGroup = getIsStartOfDistinctGroup(currentImport, previousImport);
695-
696-
if (newlinesBetweenImports === 'always'
697-
|| newlinesBetweenImports === 'always-and-inside-groups') {
698-
if (currentImport.rank !== previousImport.rank && emptyLinesBetween === 0) {
699-
if (distinctGroup || !distinctGroup && isStartOfDistinctGroup) {
700-
context.report({
701-
node: previousImport.node,
702-
message: 'There should be at least one empty line between import groups',
703-
fix: fixNewLineAfterImport(context, previousImport),
704-
});
705-
}
706-
} else if (emptyLinesBetween > 0
707-
&& newlinesBetweenImports !== 'always-and-inside-groups') {
708-
if (distinctGroup && currentImport.rank === previousImport.rank || !distinctGroup && !isStartOfDistinctGroup) {
709-
context.report({
710-
node: previousImport.node,
711-
message: 'There should be no empty line within import group',
712-
fix: removeNewLineAfterImport(context, currentImport, previousImport),
713-
});
693+
const emptyLinesBetween = getNumberOfEmptyLinesBetween(
694+
currentImport,
695+
previousImport,
696+
);
697+
698+
const isStartOfDistinctGroup = getIsStartOfDistinctGroup(
699+
currentImport,
700+
previousImport,
701+
);
702+
703+
const isTypeOnlyImport = currentImport.node.importKind === 'type';
704+
const isPreviousImportTypeOnlyImport = previousImport.node.importKind === 'type';
705+
706+
const isNormalImportFollowingTypeOnlyImportAndRelevant = !isTypeOnlyImport && isPreviousImportTypeOnlyImport && isSortingTypesGroup;
707+
708+
const isTypeOnlyImportAndRelevant = isTypeOnlyImport && isSortingTypesGroup;
709+
710+
const isNotIgnored = isTypeOnlyImportAndRelevant
711+
&& newlinesBetweenTypeOnlyImports !== 'ignore'
712+
|| !isTypeOnlyImportAndRelevant && newlinesBetweenImports !== 'ignore';
713+
714+
if (isNotIgnored) {
715+
const shouldAssertNewlineBetweenGroups = (isTypeOnlyImportAndRelevant || isNormalImportFollowingTypeOnlyImportAndRelevant)
716+
&& (newlinesBetweenTypeOnlyImports === 'always'
717+
|| newlinesBetweenTypeOnlyImports === 'always-and-inside-groups')
718+
|| !isTypeOnlyImportAndRelevant && !isNormalImportFollowingTypeOnlyImportAndRelevant
719+
&& (newlinesBetweenImports === 'always'
720+
|| newlinesBetweenImports === 'always-and-inside-groups');
721+
722+
const shouldAssertNoNewlineWithinGroup = (isTypeOnlyImportAndRelevant || isNormalImportFollowingTypeOnlyImportAndRelevant)
723+
&& newlinesBetweenTypeOnlyImports !== 'always-and-inside-groups'
724+
|| !isTypeOnlyImportAndRelevant && !isNormalImportFollowingTypeOnlyImportAndRelevant
725+
&& newlinesBetweenImports !== 'always-and-inside-groups';
726+
727+
const shouldAssertNoNewlineBetweenGroup = !isSortingTypesGroup
728+
|| !isNormalImportFollowingTypeOnlyImportAndRelevant
729+
|| newlinesBetweenTypeOnlyImports === 'never';
730+
731+
if (shouldAssertNewlineBetweenGroups) {
732+
if (currentImport.rank !== previousImport.rank && emptyLinesBetween === 0) {
733+
if (distinctGroup || !distinctGroup && isStartOfDistinctGroup) {
734+
context.report({
735+
node: previousImport.node,
736+
message: 'There should be at least one empty line between import groups',
737+
fix: fixNewLineAfterImport(context, previousImport),
738+
});
739+
}
740+
} else if (emptyLinesBetween > 0 && shouldAssertNoNewlineWithinGroup) {
741+
if (
742+
distinctGroup && currentImport.rank === previousImport.rank
743+
|| !distinctGroup && !isStartOfDistinctGroup
744+
) {
745+
context.report({
746+
node: previousImport.node,
747+
message: 'There should be no empty line within import group',
748+
fix: removeNewLineAfterImport(context, currentImport, previousImport),
749+
});
750+
}
714751
}
752+
} else if (emptyLinesBetween > 0 && shouldAssertNoNewlineBetweenGroup) {
753+
context.report({
754+
node: previousImport.node,
755+
message: 'There should be no empty line between import groups',
756+
fix: removeNewLineAfterImport(context, currentImport, previousImport),
757+
});
715758
}
716-
} else if (emptyLinesBetween > 0) {
717-
context.report({
718-
node: previousImport.node,
719-
message: 'There should be no empty line between import groups',
720-
fix: removeNewLineAfterImport(context, currentImport, previousImport),
721-
});
722759
}
723760

724761
previousImport = currentImport;
@@ -793,6 +830,14 @@ module.exports = {
793830
'never',
794831
],
795832
},
833+
'newlines-between-types': {
834+
enum: [
835+
'ignore',
836+
'always',
837+
'always-and-inside-groups',
838+
'never',
839+
],
840+
},
796841
sortTypesGroup: {
797842
type: 'boolean',
798843
default: false,
@@ -845,13 +890,22 @@ module.exports = {
845890
},
846891
},
847892
additionalProperties: false,
893+
dependencies: {
894+
'newlines-between-types': {
895+
properties: {
896+
sortTypesGroup: { enum: [true] },
897+
},
898+
required: ['sortTypesGroup'],
899+
},
900+
},
848901
},
849902
],
850903
},
851904

852905
create(context) {
853906
const options = context.options[0] || {};
854907
const newlinesBetweenImports = options['newlines-between'] || 'ignore';
908+
const newlinesBetweenTypeOnlyImports = options['newlines-between-types'] || newlinesBetweenImports;
855909
const pathGroupsExcludedImportTypes = new Set(options.pathGroupsExcludedImportTypes || ['builtin', 'external', 'object']);
856910
const sortTypesGroup = options.sortTypesGroup;
857911

@@ -1115,8 +1169,8 @@ module.exports = {
11151169
},
11161170
'Program:exit'() {
11171171
importMap.forEach((imported) => {
1118-
if (newlinesBetweenImports !== 'ignore') {
1119-
makeNewlinesBetweenReport(context, imported, newlinesBetweenImports, distinctGroup);
1172+
if (newlinesBetweenImports !== 'ignore' || newlinesBetweenTypeOnlyImports !== 'ignore') {
1173+
makeNewlinesBetweenReport(context, imported, newlinesBetweenImports, newlinesBetweenTypeOnlyImports, distinctGroup, isSortingTypesGroup);
11201174
}
11211175

11221176
if (alphabetize.order !== 'ignore') {

0 commit comments

Comments
 (0)