From 6f5cbbc81622f558d3ccc2ce1b697bea8fd88712 Mon Sep 17 00:00:00 2001 From: Ian VanSchooten Date: Thu, 27 Oct 2022 16:05:31 -0400 Subject: [PATCH] Fix type/non-type sorting (#47) We had a bug, identified in https://github.com/IanVS/prettier-plugin-sort-imports/issues/35#issuecomment-1293903705, which was causing the `"^[./]"` group to include both type and value imports, instead of only value imports. This PR fixes that bug, and expands both the unit and e2e tests, which fail without this change and pass with it. --- .../get-import-nodes-matched-group.spec.ts | 34 +++++++++++++++++++ src/utils/get-import-nodes-matched-group.ts | 9 ++++- .../__snapshots__/ppsi.spec.js.snap | 10 +++--- .../imports-with-mixed-declarations.ts | 1 + tests/TypesSpecialWord/ppsi.spec.js | 2 +- 5 files changed, 50 insertions(+), 6 deletions(-) diff --git a/src/utils/__tests__/get-import-nodes-matched-group.spec.ts b/src/utils/__tests__/get-import-nodes-matched-group.spec.ts index b7dd837..76fa064 100644 --- a/src/utils/__tests__/get-import-nodes-matched-group.spec.ts +++ b/src/utils/__tests__/get-import-nodes-matched-group.spec.ts @@ -12,6 +12,16 @@ import j from './j'; import l from './l'; import a from '@core/a'; `; + +const codeWithTypes = ` +import z from 'z'; +import type T from 't'; +import {b} from 'b'; +import a from './a'; +import type {F} from 'f'; +import type Local from './local'; +`; + test('should return correct matched groups', () => { const importNodes = getImportNodes(code); const importOrder = [ @@ -53,3 +63,27 @@ test('should return THIRD_PARTY_MODULES as matched group with empty order list', expect(matchedGroup).toEqual(''); } }); + +test('should sort non-types separately from types if group is present', () => { + const importNodes = getImportNodes(codeWithTypes, { + plugins: ['typescript'], + }); + const importOrder = ['^[./]', '^[./]', '']; + + let matchedGroups: string[] = []; + for (const importNode of importNodes) { + const matchedGroup = getImportNodesMatchedGroup( + importNode, + importOrder, + ); + matchedGroups.push(matchedGroup); + } + expect(matchedGroups).toEqual([ + '', + '', + '', + '^[./]', + '', + '^[./]', + ]); +}); diff --git a/src/utils/get-import-nodes-matched-group.ts b/src/utils/get-import-nodes-matched-group.ts index cd87c9b..8dd8eea 100644 --- a/src/utils/get-import-nodes-matched-group.ts +++ b/src/utils/get-import-nodes-matched-group.ts @@ -7,6 +7,9 @@ import { /** * Get the regexp group to keep the import nodes. + * + * This comes near the end of processing, after import declaration nodes have been combined or exploded. + * * @param node * @param importOrder */ @@ -36,7 +39,11 @@ export const getImportNodesMatchedGroup = ( node.importKind === 'type' && node.source.value.match(regExp) !== null; } else { - matched = node.source.value.match(regExp) !== null; + // If is being used for any group, and this group doesn't have it, only look for value imports + matched = includesTypesSpecialWord + ? node.importKind !== 'type' && + node.source.value.match(regExp) !== null + : node.source.value.match(regExp) !== null; } if (matched) return group; diff --git a/tests/TypesSpecialWord/__snapshots__/ppsi.spec.js.snap b/tests/TypesSpecialWord/__snapshots__/ppsi.spec.js.snap index 43e4376..c0ee00e 100644 --- a/tests/TypesSpecialWord/__snapshots__/ppsi.spec.js.snap +++ b/tests/TypesSpecialWord/__snapshots__/ppsi.spec.js.snap @@ -2,6 +2,7 @@ exports[`imports-with-mixed-declarations.ts - typescript-verify: imports-with-mixed-declarations.ts 1`] = ` import a, {type LocalType} from './local-file'; +import type Another from './another-local-file'; import value, {tp} from 'third-party'; import {specifier, type ThirdPartyType} from 'third-party'; ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ @@ -9,10 +10,11 @@ import type { ThirdPartyType } from "third-party"; import value, { tp, specifier } from "third-party"; -import type { LocalType } from "./local-file"; - import a from "./local-file"; +import type Another from "./another-local-file"; +import type { LocalType } from "./local-file"; + `; exports[`imports-with-third-party-types.ts - typescript-verify: imports-with-third-party-types.ts 1`] = ` @@ -29,9 +31,9 @@ import type { SpecifierType } from "third-party"; import value, { specifier } from "third-party"; +import a from "./local-file"; + import type LocalType from "./local-file"; import type { LocalSpecifierType } from "./local-file"; -import a from "./local-file"; - `; diff --git a/tests/TypesSpecialWord/imports-with-mixed-declarations.ts b/tests/TypesSpecialWord/imports-with-mixed-declarations.ts index 7522dd4..c2231e7 100644 --- a/tests/TypesSpecialWord/imports-with-mixed-declarations.ts +++ b/tests/TypesSpecialWord/imports-with-mixed-declarations.ts @@ -1,3 +1,4 @@ import a, {type LocalType} from './local-file'; +import type Another from './another-local-file'; import value, {tp} from 'third-party'; import {specifier, type ThirdPartyType} from 'third-party'; diff --git a/tests/TypesSpecialWord/ppsi.spec.js b/tests/TypesSpecialWord/ppsi.spec.js index 2207056..1a578b6 100644 --- a/tests/TypesSpecialWord/ppsi.spec.js +++ b/tests/TypesSpecialWord/ppsi.spec.js @@ -2,8 +2,8 @@ run_spec(__dirname, ['typescript'], { importOrder: [ '', '', - '^[./]', '^[./]', + '^[./]', ], importOrderSeparation: true, importOrderMergeDuplicateImports: true,