Skip to content
This repository has been archived by the owner on Oct 10, 2018. It is now read-only.

Commit

Permalink
feat: Option to allow first-specifier/alias import ordering (#336)
Browse files Browse the repository at this point in the history
Many users (e.g. me, or people at #316) like their imports sorted not
by library path, but by first specifier/alias name.  This adds a
`typescriptHero.resolver.organizeSortsByFirstSpecifier` boolean option,
defaulting to `false`, that enables just such a behavior.  Import
grouping is unaffected: sorting happens within groups.

Sorting is locale-aware, by the way, so not just lexicographical.  When
looking for specifier-ordering, we usually are in a "human language"
mindset, so case shifts and diacritics should work in natural order.
  • Loading branch information
tdd authored and buehler committed Nov 6, 2017
1 parent 06b739c commit ddf4418
Show file tree
Hide file tree
Showing 12 changed files with 226 additions and 21 deletions.
3 changes: 3 additions & 0 deletions .vscode/settings.json
@@ -0,0 +1,3 @@
{
"editor.tabSize": 4
}
4 changes: 4 additions & 0 deletions README.md
Expand Up @@ -82,6 +82,7 @@ The following settings do have the prefix `codeCompletion`. So an example settin
`typescriptHero.codeCompletion.completionSortOrder`.

| Setting | Description |
| ------------------- | ------------------------------------------------------------------------------|
| completionSortOrder | The order of import completions in suggestion list, `bottom` pushes them down |

### Import resolver
Expand All @@ -103,6 +104,7 @@ The following settings do have the prefix `resolver`. So an example setting coul
| ignoreImportsForOrganize | Imports that are never removed during organize import (e.g. react) |
| resolverMode | Which files should be considered to index for TypeScript Hero |
| organizeOnSave | Enable or disable the `organizeImports` action on a save of a document |
| organizeSortsByFirstSpecifier | When organizing runs, sort by first specifier/alias (if any) instead of module path |
| promptForSpecifiers | If the extension should ask the user for aliases and duplicate specifiers |

### Code outline view
Expand All @@ -126,6 +128,8 @@ TypeScript Hero can manage your imports. It is capable of:
- Remove unused imports and sort the remaining ones by alphabet
- Do organize the imports when a document is saved
- Note that this feature is only enabled if the vscode setting `editor.formatOnSave` is enabled as well!
- Organizing used module paths by default, sorted lexicographically. An option lets you use first
import specifier/alias instead, in natural-language order.

#### Import groups

Expand Down
6 changes: 6 additions & 0 deletions package.json
Expand Up @@ -259,6 +259,12 @@
"description": "Defines if the imports should be organized on save.",
"scope": "resource"
},
"typescriptHero.resolver.organizeSortsByFirstSpecifier": {
"type": "boolean",
"default": false,
"description": "Defines if the imports are organized by first specifier/alias instead of module path.",
"scope": "resource"
},
"typescriptHero.resolver.ignoreImportsForOrganize": {
"type": "array",
"items": {
Expand Down
9 changes: 9 additions & 0 deletions src/common/config/ResolverConfig.ts
Expand Up @@ -166,6 +166,15 @@ export interface ResolverConfig {
*/
resolverModeLanguages: string[];

/**
* Sorts imports by first specifier / alias / default alias (when available),
* instead of by library names (module paths).
*
* @type {boolean}
* @memberof ResolverConfig
*/
organizeSortsByFirstSpecifier: boolean;

/**
* Defines if typescript hero tries to organize your imports of a
* file as soon as the file would be saved.
Expand Down
13 changes: 13 additions & 0 deletions src/extension/config/VscodeResolverConfig.ts
Expand Up @@ -270,6 +270,19 @@ export class VscodeResolverConfig implements ResolverConfig {
return typescriptHeroValue && editorValue;
}

/**
* Defines if typescript hero import organization (sorting) uses first
* available specifier/alias, when available, instead of library names
* (module paths).
*
* @readonly
* @type {boolean}
* @memberof VscodeResolverConfig
*/
public get organizeSortsByFirstSpecifier(): boolean {
return this.workspaceSection.get('organizeSortsByFirstSpecifier', false);
}

/**
* Defines if typescript hero should ask the user for default specifiers or duplicate specifier aliases.
* If true, tsh does ask the user.
Expand Down
15 changes: 12 additions & 3 deletions src/extension/managers/ImportManager.ts
Expand Up @@ -29,7 +29,12 @@ import { importRange } from '../helpers';
import { ImportGroup } from '../import-grouping';
import { Container } from '../IoC';
import { iocSymbols } from '../IoCSymbols';
import { importGroupSortForPrecedence, importSort, specifierSort } from '../utilities/utilityFunctions';
import {
importGroupSortForPrecedence,
importSort,
importSortByFirstSpecifier,
specifierSort,
} from '../utilities/utilityFunctions';
import { Logger } from '../utilities/winstonLogger';
import { ObjectManager } from './ObjectManager';

Expand Down Expand Up @@ -273,9 +278,13 @@ export class ImportManager implements ObjectManager {
}

if (!this.config.resolver.disableImportSorting) {
const sorter = this.config.resolver.organizeSortsByFirstSpecifier
? importSortByFirstSpecifier
: importSort;

keep = [
...keep.filter(o => o instanceof StringImport).sort(importSort),
...keep.filter(o => !(o instanceof StringImport)).sort(importSort),
...keep.filter(o => o instanceof StringImport).sort(sorter),
...keep.filter(o => !(o instanceof StringImport)).sort(sorter),
];
}

Expand Down
70 changes: 70 additions & 0 deletions src/extension/utilities/utilityFunctions.ts
@@ -1,19 +1,24 @@
import { ImportGroup, RegexImportGroup } from '../import-grouping';
import { basename } from 'path';
import {
ClassDeclaration,
ConstructorDeclaration,
Declaration,
DefaultDeclaration,
EnumDeclaration,
ExternalModuleImport,
FunctionDeclaration,
GetterDeclaration,
Import,
InterfaceDeclaration,
MethodDeclaration,
ModuleDeclaration,
NamedImport,
NamespaceImport,
ParameterDeclaration,
PropertyDeclaration,
SetterDeclaration,
StringImport,
SymbolSpecifier,
TypeAliasDeclaration,
VariableDeclaration,
Expand Down Expand Up @@ -60,6 +65,22 @@ export function importGroupSortForPrecedence(importGroups: ImportGroup[]): Impor
return regexGroups.concat(otherGroups);
}

/**
* Locale-sensitive ("Human-compatible") String-Sort function.
*
* @param {string} strA
* @param {string} strB
* @param {('asc' | 'desc')} [order='asc']
* @returns {number}
*/
function localeStringSort(strA: string, strB: string, order: 'asc' | 'desc' = 'asc'): number {
let result: number = strA.localeCompare(strB);
if (order === 'desc') {
result *= -1;
}
return result;
}

/**
* Order imports by library name.
*
Expand All @@ -76,6 +97,55 @@ export function importSort(i1: Import, i2: Import, order: 'asc' | 'desc' = 'asc'
return stringSort(strA, strB, order);
}

/**
* Order imports by first specifier name. Does not re-sort specifiers internally:
* assumes they were sorted AOT (which happens in `ImportManager#organizeImports`,
* indeed).
*
* @export
* @param {Import} i1
* @param {Import} i2
* @param {('asc' | 'desc')} [order='asc']
* @returns {number}
*/
export function importSortByFirstSpecifier(i1: Import, i2: Import, order: 'asc' | 'desc' = 'asc'): number {
const strA = getImportFirstSpecifier(i1);
const strB = getImportFirstSpecifier(i2);

return localeStringSort(strA, strB, order);
}

/**
* Computes the first specifier/alias of an import, falling back ot its
* module path (for StringImports, basically). Does not re-sort specifiers
* internally: assumes they were sorted AOT (which happens in
* `ImportManager#organizeImports`, indeed).
*
* @param {Import} imp
* @returns {String}
*/
function getImportFirstSpecifier(imp: Import): string {
if (imp instanceof NamespaceImport || imp instanceof ExternalModuleImport) {
return imp.alias;
}

if (imp instanceof StringImport) {
return basename(imp.libraryName);
}

if (imp instanceof NamedImport) {
const namedSpecifiers = (imp as NamedImport).specifiers
.map(s => s.alias || s.specifier)
.filter(Boolean);
const marker = namedSpecifiers[0] || imp.defaultAlias;
if (marker) {
return marker;
}
}

return basename(imp.libraryName);
}

/**
* Order specifiers by name.
*
Expand Down
3 changes: 2 additions & 1 deletion test/_workspace/.vscode/settings.json
Expand Up @@ -25,5 +25,6 @@
"typescriptHero.resolver.promptForSpecifiers": true,
"editor.formatOnSave": true,
"typescriptHero.codeCompletion.completionSortMode": "default",
"typescriptHero.resolver.disableImportRemovalOnOrganize": false
"typescriptHero.resolver.disableImportRemovalOnOrganize": false,
"typescriptHero.resolver.organizeSortsByFirstSpecifier": false
}
@@ -0,0 +1,6 @@
import 'coolEffectLib';
import './workspaceSideEffectLib';
import { Foobar, Genero } from './myFile';
import { AnotherFoobar } from './anotherFile';
import ModuleFoobar from 'myLib';
import { AnotherModuleFoo as MuchFurtherSorted } from 'anotherLib';
Expand Up @@ -14,6 +14,7 @@ describe('OrganizeImportsOnSaveExtension', () => {

const rootPath = vscode.workspace.workspaceFolders![0].uri.fsPath;
let document: vscode.TextDocument;
let documentText: string;
let index: DeclarationIndex;

before(async () => {
Expand All @@ -22,6 +23,7 @@ describe('OrganizeImportsOnSaveExtension', () => {
'extension/extensions/organizeImportsOnSaveExtension/organizeFile.ts',
);
document = await vscode.workspace.openTextDocument(file);
documentText = document.getText()

await vscode.window.showTextDocument(document);

Expand All @@ -42,6 +44,14 @@ describe('OrganizeImportsOnSaveExtension', () => {
after(async () => {
const config = vscode.workspace.getConfiguration('typescriptHero');
await config.update('resolver.organizeOnSave', false);
await vscode.window.activeTextEditor!.edit((builder) => {
builder.delete(new vscode.Range(
new vscode.Position(0, 0),
document.lineAt(document.lineCount - 1).rangeIncludingLineBreak.end,
));
builder.insert(new vscode.Position(0, 0), documentText);
});
await document.save()
});

afterEach(async () => {
Expand Down
Expand Up @@ -575,6 +575,49 @@ describe('ImportManager', () => {
names.should.deep.equal(['../plain.ts', 'react', 'some-regular-module', 'cool-library', 'cooler-library/items', '../../server/indices'])
});

describe('resolver.organizeSortsByFirstSpecifier: true', () => {
before(async () => {
const config = workspace.getConfiguration('typescriptHero');
await config.update('resolver.organizeSortsByFirstSpecifier', true);
});

after(async () => {
const config = workspace.getConfiguration('typescriptHero');
await config.update('resolver.organizeSortsByFirstSpecifier', false);
});

it('should sort imports by first specifier/alias', async () => {
const demoSource = `
import { Foobar, Genero } from 'someLib';
import { AnotherFoobar } from 'someOtherLib';
import ModuleFoobar from 'myLib';
import { AnotherModuleFoo as MuchFurtherSorted } from 'anotherLib';
console.log(AnotherFoobar, Foobar, Genero, ModuleFoobar, MuchFurtherSorted)
`.replace(/\n[ \t]+/g, '\n').trim();

await window.activeTextEditor!.edit((builder) => {
builder.delete(new Range(
new Position(0, 0),
document.lineAt(document.lineCount - 1).rangeIncludingLineBreak.end,
));
builder.insert(new Position(0, 0), demoSource);
});

const ctrl = await ImportManager.create(document);

(ctrl as any).imports[0].libraryName.should.equal('someLib');

ctrl.organizeImports();

(ctrl as any).imports.map((i) => i.libraryName).should.deep.equal([
'someOtherLib', // { AnotherFoobar }
'someLib', // { Foobar, Genero }
'myLib', // ModuleFoobar
'anotherLib', // { AnotherModuleFoo as MuchFurtherSorted }
]);
})
})
});

describe('commit()', () => {
Expand Down
@@ -1,25 +1,56 @@
import * as chai from 'chai';
chai.should();
import { Container } from '../../../../src/extension/IoC';
import { TypescriptParser } from 'typescript-parser';
import { iocSymbols } from '../../../../src/extension/IoCSymbols';
import { join } from 'path';
import { workspace } from 'vscode';

import { KeywordImportGroup, ImportGroupKeyword, RegexImportGroup } from '../../../../src/extension/import-grouping';
import { importGroupSortForPrecedence } from '../../../../src/extension/utilities/utilityFunctions';
import { importGroupSortForPrecedence, importSortByFirstSpecifier } from '../../../../src/extension/utilities/utilityFunctions';

chai.should();

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))
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)')
});
});

describe('importSortByFirstSpecifier', () => {
const parser = Container.get<TypescriptParser>(iocSymbols.typescriptParser);
const rootPath = workspace.workspaceFolders![0].uri.fsPath;

it('should sort according to first specifier/alias, falling back to module path', async () => {
const file = await parser.parseFile(
join(
rootPath,
'extension/utilities/importsForSpecifierSort.ts',
),
rootPath,
);

importGroupSortForPrecedence(initialList).should.deep.equal(expectedList,
'Regex Import Groups should appear first (and that should be the only change)')
const result = [...file.imports].sort(importSortByFirstSpecifier);
result.map((i) => i.libraryName).should.deep.equal([
'./anotherFile', // { AnotherFoobar }
'coolEffectLib', // 'coolEffectLib'
'./myFile', // { Foobar, Genero }
'myLib', // ModuleFoobar
'anotherLib', // { AnotherModuleFoo as MuchFurtherSorted }
'./workspaceSideEffectLib', // './workspaceSideEffectLib
]);
});
});
});
});

0 comments on commit ddf4418

Please sign in to comment.