Skip to content

Commit

Permalink
fix: ensure consolidateIslands overrides both newlines-between and ne…
Browse files Browse the repository at this point in the history
…wlines-between-types when in conflict
  • Loading branch information
Xunnamius committed Jan 26, 2025
1 parent 630ffef commit 5afa43a
Show file tree
Hide file tree
Showing 3 changed files with 188 additions and 11 deletions.
16 changes: 9 additions & 7 deletions docs/rules/order.md
Original file line number Diff line number Diff line change
Expand Up @@ -738,16 +738,18 @@ When set to `"inside-groups"`, this ensures imports spanning multiple lines are
>
> 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`
> - `consolidateIslands` is set to `"inside-groups"`
> - [`newlines-between`][20] is set to `"always-and-inside-groups"` when [`newlines-between-types`][27] is set to `"never"` (or vice-versa)
>
> 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.
> Then [`newlines-between`][20]/[`newlines-between-types`][27] will yield to
> `consolidateIslands` and allow new lines to separate multi-line imports
> regardless of the `"never"` setting.
>
> 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.
> This configuration is useful, for instance, to keep single-line 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

Expand Down
15 changes: 12 additions & 3 deletions src/rules/order.js
Original file line number Diff line number Diff line change
Expand Up @@ -691,7 +691,7 @@ function removeNewLineAfterImport(context, currentImport, previousImport) {
return undefined;
}

function makeNewlinesBetweenReport(context, imported, newlinesBetweenImports, newlinesBetweenTypeOnlyImports_, distinctGroup, isSortingTypesGroup, isConsolidatingSpaceBetweenImports) {
function makeNewlinesBetweenReport(context, imported, newlinesBetweenImports_, newlinesBetweenTypeOnlyImports_, distinctGroup, isSortingTypesGroup, isConsolidatingSpaceBetweenImports) {
const getNumberOfEmptyLinesBetween = (currentImport, previousImport) => {
const linesBetweenImports = getSourceCode(context).lines.slice(
previousImport.node.loc.end.line,
Expand Down Expand Up @@ -721,14 +721,23 @@ function makeNewlinesBetweenReport(context, imported, newlinesBetweenImports, ne

const isTypeOnlyImportAndRelevant = isTypeOnlyImport && isSortingTypesGroup;

// In the special case where newlinesBetweenImports and consolidateIslands
// want the opposite thing, consolidateIslands wins
const newlinesBetweenImports = isSortingTypesGroup
&& isConsolidatingSpaceBetweenImports
&& (previousImport.isMultiline || currentImport.isMultiline)
&& newlinesBetweenImports_ === 'never'
? 'always-and-inside-groups'
: newlinesBetweenImports_;

// In the special case where newlinesBetweenTypeOnlyImports and
// consolidateIslands want the opposite thing, consolidateIslands wins
const newlinesBetweenTypeOnlyImports = newlinesBetweenTypeOnlyImports_ === 'never'
const newlinesBetweenTypeOnlyImports = isSortingTypesGroup
&& isConsolidatingSpaceBetweenImports
&& isSortingTypesGroup
&& (isNormalImportNextToTypeOnlyImportAndRelevant
|| previousImport.isMultiline
|| currentImport.isMultiline)
&& newlinesBetweenTypeOnlyImports_ === 'never'
? 'always-and-inside-groups'
: newlinesBetweenTypeOnlyImports_;

Expand Down
168 changes: 167 additions & 1 deletion tests/src/rules/order.js
Original file line number Diff line number Diff line change
Expand Up @@ -4158,7 +4158,7 @@ context('TypeScript', function () {
},
],
}),
// Ensure consolidateOptions: 'inside-groups', newlines-between: 'always-and-inside-groups', and newlines-between-types: 'never' do not fight for dominance
// Ensure consolidateIslands: 'inside-groups', newlines-between: 'always-and-inside-groups', and newlines-between-types: 'never' do not fight for dominance
test({
code: `
import makeVanillaYargs from 'yargs/yargs';
Expand Down Expand Up @@ -4228,6 +4228,172 @@ context('TypeScript', function () {
},
],
}),
// Ensure consolidateIslands: 'inside-groups', newlines-between: 'never', and newlines-between-types: 'always-and-inside-groups' do not fight for dominance
test({
code: `
import makeVanillaYargs from 'yargs/yargs';
import { createDebugLogger } from 'multiverse+rejoinder';
import { globalDebuggerNamespace } from 'rootverse+bfe:src/constant.ts';
import { ErrorMessage, type KeyValueEntry } from 'rootverse+bfe:src/error.ts';
import { $artificiallyInvoked } from 'rootverse+bfe:src/symbols.ts';
import type {
Entries,
LiteralUnion,
OmitIndexSignature,
Promisable,
StringKeyOf
} from 'type-fest';
`,
...parserConfig,
options: [
{
alphabetize: {
order: 'asc',
orderImportKind: 'asc',
caseInsensitive: true,
},
named: {
enabled: true,
types: 'types-last',
},
groups: [
'builtin',
'external',
'internal',
['parent', 'sibling', 'index'],
['object', 'type'],
],
pathGroups: [
{
pattern: 'multiverse{*,*/**}',
group: 'external',
position: 'after',
},
{
pattern: 'rootverse{*,*/**}',
group: 'external',
position: 'after',
},
{
pattern: 'universe{*,*/**}',
group: 'external',
position: 'after',
},
],
distinctGroup: true,
pathGroupsExcludedImportTypes: ['builtin', 'object'],
'newlines-between': 'never',
'newlines-between-types': 'always-and-inside-groups',
sortTypesGroup: true,
consolidateIslands: 'inside-groups',
},
],
}),
test({
code: `
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';
`,
...parserConfig,
options: [
{
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': 'never',
'newlines-between-types': 'always-and-inside-groups',
pathGroupsExcludedImportTypes: [],
sortTypesGroup: true,
consolidateIslands: 'inside-groups',
},
],
}),
test({
code: `
import assert from 'assert';
Expand Down

0 comments on commit 5afa43a

Please sign in to comment.