From a04ce1e0402335e321646b03f898df1c76eb45eb Mon Sep 17 00:00:00 2001 From: Christophe Porteneuve Date: Sun, 5 Nov 2017 23:12:36 +0100 Subject: [PATCH] feat: Allow regex groups *after* keyword groups (#335) --- src/extension/managers/ImportManager.ts | 5 ++- src/extension/utilities/utilityFunctions.ts | 19 ++++++++ test/_workspace/.vscode/settings.json | 8 ++++ .../extension/managers/ImportManager.test.ts | 44 ++++++++++++++++--- .../utilities/utilityFunctions.test.ts | 25 +++++++++++ 5 files changed, 93 insertions(+), 8 deletions(-) create mode 100644 test/single-workspace-tests/extension/utilities/utilityFunctions.test.ts diff --git a/src/extension/managers/ImportManager.ts b/src/extension/managers/ImportManager.ts index 9ce1622..d2691c8 100644 --- a/src/extension/managers/ImportManager.ts +++ b/src/extension/managers/ImportManager.ts @@ -29,7 +29,7 @@ import { importRange } from '../helpers'; import { ImportGroup } from '../import-grouping'; import { Container } from '../IoC'; import { iocSymbols } from '../IoCSymbols'; -import { importSort, specifierSort } from '../utilities/utilityFunctions'; +import { importGroupSortForPrecedence, importSort, specifierSort } from '../utilities/utilityFunctions'; import { Logger } from '../utilities/winstonLogger'; import { ObjectManager } from './ObjectManager'; @@ -409,8 +409,9 @@ export class ImportManager implements ObjectManager { * @memberof ImportManager */ private addImportsToGroups(imports: Import[]): void { + const importGroupsWithPrecedence = importGroupSortForPrecedence(this.importGroups); for (const tsImport of imports) { - for (const group of this.importGroups) { + for (const group of importGroupsWithPrecedence) { if (group.processImport(tsImport)) { break; } diff --git a/src/extension/utilities/utilityFunctions.ts b/src/extension/utilities/utilityFunctions.ts index f4e0456..8242db9 100644 --- a/src/extension/utilities/utilityFunctions.ts +++ b/src/extension/utilities/utilityFunctions.ts @@ -1,3 +1,4 @@ +import { ImportGroup, RegexImportGroup } from '../import-grouping'; import { ClassDeclaration, ConstructorDeclaration, @@ -41,6 +42,24 @@ export function stringSort(strA: string, strB: string, order: 'asc' | 'desc' = ' return result; } +/** + * Orders import groups by matching precedence (regex first). This is used internally by + * `ImportManager` when assigning imports to groups, so regex groups can appear later than + * keyword groups yet capture relevant imports nonetheless. + * + * @export + * @param {ImportGroup[]} importGroups The original import groups (as per extension configuration) + * @returns {ImportGroup[]} The same list, with Regex import groups appearing first. + */ +export function importGroupSortForPrecedence(importGroups: ImportGroup[]): ImportGroup[] { + const regexGroups: ImportGroup[] = []; + const otherGroups: ImportGroup[] = []; + for (const ig of importGroups) { + (ig instanceof RegexImportGroup ? regexGroups : otherGroups).push(ig); + } + return regexGroups.concat(otherGroups); +} + /** * Order imports by library name. * diff --git a/test/_workspace/.vscode/settings.json b/test/_workspace/.vscode/settings.json index 5b32623..7196faf 100755 --- a/test/_workspace/.vscode/settings.json +++ b/test/_workspace/.vscode/settings.json @@ -9,6 +9,14 @@ "dist", "resourceParser" ], + "typescriptHero.resolver.importGroups": [ + "Plains", + "Modules", + "/cool-library/", + "/cooler-library/", + "Workspace" + ], + "editor.tabSize": 4, "files.eol": "\n", "typescriptHero.verbosity": "warn", "tslint.enable": false, diff --git a/test/single-workspace-tests/extension/managers/ImportManager.test.ts b/test/single-workspace-tests/extension/managers/ImportManager.test.ts index 749c7f2..6dd92eb 100644 --- a/test/single-workspace-tests/extension/managers/ImportManager.test.ts +++ b/test/single-workspace-tests/extension/managers/ImportManager.test.ts @@ -257,11 +257,13 @@ describe('ImportManager', () => { const ctrl = await ImportManager.create(document); const declaration = index.declarationInfos.find(o => o.declaration.name === 'NotBarelExported'); - (ctrl as any).importGroups[2].imports.should.have.lengthOf(1); + // Import group 4 in single-workspace test settings is "Workspace" + (ctrl as any).importGroups[4].imports.should.have.lengthOf(1); ctrl.addDeclarationImport(declaration!); - (ctrl as any).importGroups[2].imports.should.have.lengthOf(2); + // Import group 4 in single-workspace test settings is "Workspace" + (ctrl as any).importGroups[4].imports.should.have.lengthOf(2); }); it('should add an import to an existing import group', async () => { @@ -269,11 +271,13 @@ describe('ImportManager', () => { const declaration = index.declarationInfos.find(o => o.declaration.name === 'Class2'); const declaration2 = index.declarationInfos.find(o => o.declaration.name === 'Class3'); - (ctrl as any).importGroups[2].imports.should.have.lengthOf(1); + // Import group 4 in single-workspace test settings is "Workspace" + (ctrl as any).importGroups[4].imports.should.have.lengthOf(1); ctrl.addDeclarationImport(declaration!).addDeclarationImport(declaration2!); - (ctrl as any).importGroups[2].imports.should.have.lengthOf(1); + // Import group 4 in single-workspace test settings is "Workspace" + (ctrl as any).importGroups[4].imports.should.have.lengthOf(1); }); }); @@ -348,11 +352,13 @@ describe('ImportManager', () => { const declaration = index.declarationInfos.find(o => o.declaration.name === 'myComponent'); ctrl.addDeclarationImport(declaration!); - (ctrl as any).importGroups[2].imports.should.have.lengthOf(2); + // Import group 4 in single-workspace test settings is "Workspace" + (ctrl as any).importGroups[4].imports.should.have.lengthOf(2); ctrl.organizeImports(); - (ctrl as any).importGroups[2].imports.should.have.lengthOf(1); + // Import group 4 in single-workspace test settings is "Workspace" + (ctrl as any).importGroups[4].imports.should.have.lengthOf(1); }); it('should remove an unused specifier from an import', async () => { @@ -543,6 +549,32 @@ describe('ImportManager', () => { (ctrl as any).imports.should.have.lengthOf(1); }); + it('should honor regex groups even if they appear later than keyword groups', async () => { + await window.activeTextEditor!.edit((builder) => { + builder.insert( + new Position(0, 0), + ` + import mainFx from 'some-regular-module' + import { CoolerStuff, CoolerTitle } from 'cooler-library/items' + import CoolLib from 'cool-library' + import React, { Component } from 'react' + + import '../plain.ts' + `.replace(/\n[ \t]+/g, '\n') + ); + // Ensure imports are not stripped because they’re unused + builder.insert( + new Position(12, 0), + 'console.log(mainFx, CoolerStuff, CoolerTitle, CoolLib, React, Component)\n' + ) + }); + const ctrl = await ImportManager.create(document); + + await ctrl.organizeImports().commit(); + const names = (ctrl as any).imports.map((l) => l.libraryName) + names.should.deep.equal(['../plain.ts', 'react', 'some-regular-module', 'cool-library', 'cooler-library/items', '../../server/indices']) + }); + }); describe('commit()', () => { diff --git a/test/single-workspace-tests/extension/utilities/utilityFunctions.test.ts b/test/single-workspace-tests/extension/utilities/utilityFunctions.test.ts new file mode 100644 index 0000000..932dba7 --- /dev/null +++ b/test/single-workspace-tests/extension/utilities/utilityFunctions.test.ts @@ -0,0 +1,25 @@ +import * as chai from 'chai'; +chai.should(); + +import { KeywordImportGroup, ImportGroupKeyword, RegexImportGroup } from '../../../../src/extension/import-grouping'; +import { importGroupSortForPrecedence } from '../../../../src/extension/utilities/utilityFunctions'; + +describe('utilityFunctions', () => { + describe('importGroupSortForPrecedence', () => { + it('should prioritize regexes, leaving original order untouched besides that', () => { + const initialList = [ + new KeywordImportGroup(ImportGroupKeyword.Modules), + new KeywordImportGroup(ImportGroupKeyword.Plains), + new RegexImportGroup("/cool-library/"), + new RegexImportGroup("/cooler-library/"), + new KeywordImportGroup(ImportGroupKeyword.Workspace), + ] + const expectedList = initialList.slice(2, 4) + .concat(initialList.slice(0, 2)) + .concat(initialList.slice(4)) + + importGroupSortForPrecedence(initialList).should.deep.equal(expectedList, + 'Regex Import Groups should appear first (and that should be the only change)') + }); + }); +});