Organize type imports

src/compiler/types.ts

@@ -10074,6 +10074,7 @@ export interface UserPreferences {
     readonly organizeImportsNumericCollation?: boolean;
     readonly organizeImportsAccentCollation?: boolean;
     readonly organizeImportsCaseFirst?: "upper" | "lower" | false;
+    readonly organizeImportsTypeOrder?: "first" | "last" | "inline";
     readonly excludeLibrarySymbolsInNavTo?: boolean;
 }
 

src/harness/fourslashImpl.ts

@@ -3521,10 +3521,12 @@ export class TestState {
             actualTextArray.push(text);
 
             // Undo changes to perform next fix
-            const span = change.textChanges[0].span;
-            const deletedText = originalContent.substr(span.start, change.textChanges[0].span.length);
-            const insertedText = change.textChanges[0].newText;
-            this.editScriptAndUpdateMarkers(fileName, span.start, span.start + insertedText.length, deletedText);
+            for (const textChange of change.textChanges) {
+                const span = textChange.span;
+                const deletedText = originalContent.slice(span.start, span.start + textChange.span.length);
+                const insertedText = textChange.newText;
+                this.editScriptAndUpdateMarkers(fileName, span.start, span.start + insertedText.length, deletedText);
+            }
         }
         if (expectedTextArray.length !== actualTextArray.length) {
             this.raiseError(`Expected ${expectedTextArray.length} import fixes, got ${actualTextArray.length}:\n\n${actualTextArray.join("\n\n" + "-".repeat(20) + "\n\n")}`);

src/server/protocol.ts

@@ -3643,6 +3643,13 @@ export interface UserPreferences {
      * Default: `false`
      */
     readonly organizeImportsCaseFirst?: "upper" | "lower" | false;
+    /**
+     * Indicates where named type-only imports should sort. "inline" sorts named imports without regard to if the import is
+     * type-only.
+     *
+     * Default: `last`
+     */
+    readonly organizeImportsTypeOrder?: "last" | "first" | "inline";
 
     /**
      * Indicates whether {@link ReferencesResponseItem.lineText} is supported.

src/services/codefixes/importFixes.ts

@@ -53,6 +53,7 @@ import {
     getSourceFileOfNode,
     getSymbolId,
     getTokenAtPosition,
+    getTokenPosOfNode,
     getTypeKeywordOfTypeOnlyImport,
     getUniqueSymbolId,
     hostGetCanonicalFileName,
@@ -1406,14 +1407,14 @@ function promoteFromTypeOnly(
                 if (aliasDeclaration.parent.elements.length > 1 && sortKind) {
                     const newSpecifier = factory.updateImportSpecifier(aliasDeclaration, /*isTypeOnly*/ false, aliasDeclaration.propertyName, aliasDeclaration.name);
                     const comparer = OrganizeImports.getOrganizeImportsComparer(preferences, sortKind === SortKind.CaseInsensitive);
-                    const insertionIndex = OrganizeImports.getImportSpecifierInsertionIndex(aliasDeclaration.parent.elements, newSpecifier, comparer);
-                    if (aliasDeclaration.parent.elements.indexOf(aliasDeclaration) !== insertionIndex) {
+                    const insertionIndex = OrganizeImports.getImportSpecifierInsertionIndex(aliasDeclaration.parent.elements, newSpecifier, comparer, preferences);
+                    if (insertionIndex !== aliasDeclaration.parent.elements.indexOf(aliasDeclaration)) {
                         changes.delete(sourceFile, aliasDeclaration);
                         changes.insertImportSpecifierAtIndex(sourceFile, newSpecifier, aliasDeclaration.parent, insertionIndex);
                         return aliasDeclaration;
                     }
                 }
-                changes.deleteRange(sourceFile, aliasDeclaration.getFirstToken()!);
+                changes.deleteRange(sourceFile, { pos: getTokenPosOfNode(aliasDeclaration.getFirstToken()!), end: getTokenPosOfNode(aliasDeclaration.propertyName ?? aliasDeclaration.name) });
                 return aliasDeclaration;
             }
             else {
@@ -1538,7 +1539,7 @@ function doAddExistingFix(
                 // type-only, there's no need to ask for the insertion index - it's 0.
                 const insertionIndex = promoteFromTypeOnly && !spec.isTypeOnly
                     ? 0
-                    : OrganizeImports.getImportSpecifierInsertionIndex(existingSpecifiers, spec, comparer);
+                    : OrganizeImports.getImportSpecifierInsertionIndex(existingSpecifiers, spec, comparer, preferences);
                 changes.insertImportSpecifierAtIndex(sourceFile, spec, clause.namedBindings as NamedImports, insertionIndex);
             }
         }

src/services/organizeImports.ts

@@ -23,8 +23,10 @@ import {
     flatMap,
     formatting,
     getNewLineOrDefaultFromHost,
+    getStringComparer,
     getUILocale,
     group,
+    groupBy,
     Identifier,
     identity,
     ImportClause,
@@ -94,7 +96,7 @@ export function organizeImports(
 
     const processImportsOfSameModuleSpecifier = (importGroup: readonly ImportDeclaration[]) => {
         if (shouldRemove) importGroup = removeUnusedImports(importGroup, sourceFile, program);
-        if (shouldCombine) importGroup = coalesceImportsWorker(importGroup, comparer, sourceFile);
+        if (shouldCombine) importGroup = coalesceImportsWorker(importGroup, comparer, sourceFile, preferences);
         if (shouldSort) importGroup = stableSort(importGroup, (s1, s2) => compareImportsOrRequireStatements(s1, s2, comparer));
         return importGroup;
     };
@@ -104,7 +106,7 @@ export function organizeImports(
     // Exports are always used
     if (mode !== OrganizeImportsMode.RemoveUnused) {
         // All of the old ExportDeclarations in the file, in syntactic order.
-        getTopLevelExportGroups(sourceFile).forEach(exportGroupDecl => organizeImportsWorker(exportGroupDecl, group => coalesceExportsWorker(group, comparer)));
+        getTopLevelExportGroups(sourceFile).forEach(exportGroupDecl => organizeImportsWorker(exportGroupDecl, group => coalesceExportsWorker(group, comparer, preferences)));
     }
 
     for (const ambientModule of sourceFile.statements.filter(isAmbientModule)) {
@@ -116,7 +118,7 @@ export function organizeImports(
         // Exports are always used
         if (mode !== OrganizeImportsMode.RemoveUnused) {
             const ambientModuleExportDecls = ambientModule.body.statements.filter(isExportDeclaration);
-            organizeImportsWorker(ambientModuleExportDecls, group => coalesceExportsWorker(group, comparer));
+            organizeImportsWorker(ambientModuleExportDecls, group => coalesceExportsWorker(group, comparer, preferences));
         }
     }
 
@@ -310,12 +312,12 @@ function getExternalModuleName(specifier: Expression | undefined) {
  * @deprecated Only used for testing
  * @internal
  */
-export function coalesceImports(importGroup: readonly ImportDeclaration[], ignoreCase: boolean, sourceFile?: SourceFile): readonly ImportDeclaration[] {
+export function coalesceImports(importGroup: readonly ImportDeclaration[], ignoreCase: boolean, sourceFile?: SourceFile, preferences?: UserPreferences): readonly ImportDeclaration[] {
     const comparer = getOrganizeImportsOrdinalStringComparer(ignoreCase);
-    return coalesceImportsWorker(importGroup, comparer, sourceFile);
+    return coalesceImportsWorker(importGroup, comparer, sourceFile, preferences);
 }
 
-function coalesceImportsWorker(importGroup: readonly ImportDeclaration[], comparer: Comparer<string>, sourceFile?: SourceFile): readonly ImportDeclaration[] {
+function coalesceImportsWorker(importGroup: readonly ImportDeclaration[], comparer: Comparer<string>, sourceFile?: SourceFile, preferences?: UserPreferences): readonly ImportDeclaration[] {
     if (importGroup.length === 0) {
         return importGroup;
     }
@@ -374,7 +376,7 @@ function coalesceImportsWorker(importGroup: readonly ImportDeclaration[], compar
         newImportSpecifiers.push(...getNewImportSpecifiers(namedImports));
 
         const sortedImportSpecifiers = factory.createNodeArray(
-            sortSpecifiers(newImportSpecifiers, comparer),
+            sortSpecifiers(newImportSpecifiers, comparer, preferences),
             firstNamedImport?.importClause.namedBindings.elements.hasTrailingComma,
         );
 
@@ -491,18 +493,17 @@ function getCategorizedImports(importGroup: readonly ImportDeclaration[]) {
  * @deprecated Only used for testing
  * @internal
  */
-export function coalesceExports(exportGroup: readonly ExportDeclaration[], ignoreCase: boolean) {
+export function coalesceExports(exportGroup: readonly ExportDeclaration[], ignoreCase: boolean, preferences?: UserPreferences) {
     const comparer = getOrganizeImportsOrdinalStringComparer(ignoreCase);
-    return coalesceExportsWorker(exportGroup, comparer);
+    return coalesceExportsWorker(exportGroup, comparer, preferences);
 }
 
-function coalesceExportsWorker(exportGroup: readonly ExportDeclaration[], comparer: Comparer<string>) {
+function coalesceExportsWorker(exportGroup: readonly ExportDeclaration[], comparer: Comparer<string>, preferences?: UserPreferences) {
     if (exportGroup.length === 0) {
         return exportGroup;
     }
 
     const { exportWithoutClause, namedExports, typeOnlyExports } = getCategorizedExports(exportGroup);
-
     const coalescedExports: ExportDeclaration[] = [];
 
     if (exportWithoutClause) {
@@ -516,7 +517,7 @@ function coalesceExportsWorker(exportGroup: readonly ExportDeclaration[], compar
         const newExportSpecifiers: ExportSpecifier[] = [];
         newExportSpecifiers.push(...flatMap(exportGroup, i => i.exportClause && isNamedExports(i.exportClause) ? i.exportClause.elements : emptyArray));
 
-        const sortedExportSpecifiers = sortSpecifiers(newExportSpecifiers, comparer);
+        const sortedExportSpecifiers = sortSpecifiers(newExportSpecifiers, comparer, preferences);
 
         const exportDecl = exportGroup[0];
         coalescedExports.push(
@@ -583,13 +584,20 @@ function updateImportDeclarationAndClause(
     );
 }
 
-function sortSpecifiers<T extends ImportOrExportSpecifier>(specifiers: readonly T[], comparer: Comparer<string>) {
-    return stableSort(specifiers, (s1, s2) => compareImportOrExportSpecifiers(s1, s2, comparer));
+function sortSpecifiers<T extends ImportOrExportSpecifier>(specifiers: readonly T[], comparer: Comparer<string>, preferences?: UserPreferences): readonly T[] {
+    return stableSort(specifiers, (s1, s2) => compareImportOrExportSpecifiers(s1, s2, comparer, preferences));
 }
 
 /** @internal */
-export function compareImportOrExportSpecifiers<T extends ImportOrExportSpecifier>(s1: T, s2: T, comparer: Comparer<string>): Comparison {
-    return compareBooleans(s1.isTypeOnly, s2.isTypeOnly) || comparer(s1.name.text, s2.name.text);
+export function compareImportOrExportSpecifiers<T extends ImportOrExportSpecifier>(s1: T, s2: T, comparer: Comparer<string>, preferences?: UserPreferences): Comparison {
+    switch (preferences?.organizeImportsTypeOrder) {
+        case "first":
+            return compareBooleans(s2.isTypeOnly, s1.isTypeOnly) || comparer(s1.name.text, s2.name.text);
+        case "inline":
+            return comparer(s1.name.text, s2.name.text);
+        default:
+            return compareBooleans(s1.isTypeOnly, s2.isTypeOnly) || comparer(s1.name.text, s2.name.text);
+    }
 }
 
 /**
@@ -721,11 +729,51 @@ class ImportSpecifierSortingCache implements MemoizeCache<[readonly ImportSpecif
 
 /** @internal */
 export const detectImportSpecifierSorting = memoizeCached((specifiers: readonly ImportSpecifier[], preferences: UserPreferences): SortKind => {
-    if (!arrayIsSorted(specifiers, (s1, s2) => compareBooleans(s1.isTypeOnly, s2.isTypeOnly))) {
-        return SortKind.None;
+    // If types are not sorted as specified, then imports are assumed to be unsorted.
+    // If there is no type sorting specification, we default to "last" and move on to case sensitivity detection.
+    switch (preferences.organizeImportsTypeOrder) {
+        case "first":
+            if (!arrayIsSorted(specifiers, (s1, s2) => compareBooleans(s2.isTypeOnly, s1.isTypeOnly))) return SortKind.None;
+            break;
+        case "inline":
+            if (
+                !arrayIsSorted(specifiers, (s1, s2) => {
+                    const comparer = getStringComparer(/*ignoreCase*/ true);
+                    return comparer(s1.name.text, s2.name.text);
+                })
+            ) {
+                return SortKind.None;
+            }
+            break;
+        default:
+            if (!arrayIsSorted(specifiers, (s1, s2) => compareBooleans(s1.isTypeOnly, s2.isTypeOnly))) return SortKind.None;
+            break;
     }
+
     const collateCaseSensitive = getOrganizeImportsComparer(preferences, /*ignoreCase*/ false);
     const collateCaseInsensitive = getOrganizeImportsComparer(preferences, /*ignoreCase*/ true);
+
+    if (preferences.organizeImportsTypeOrder !== "inline") {
+        const { type: regularImports, regular: typeImports } = groupBy(specifiers, s => s.isTypeOnly ? "type" : "regular");
+        const regularCaseSensitivity = regularImports?.length
+            ? detectSortCaseSensitivity(regularImports, specifier => specifier.name.text, collateCaseSensitive, collateCaseInsensitive)
+            : undefined;
+        const typeCaseSensitivity = typeImports?.length
+            ? detectSortCaseSensitivity(typeImports, specifier => specifier.name.text ?? "", collateCaseSensitive, collateCaseInsensitive)
+            : undefined;
+        if (regularCaseSensitivity === undefined) {
+            return typeCaseSensitivity ?? SortKind.None;
+        }
+        if (typeCaseSensitivity === undefined) {
+            return regularCaseSensitivity;
+        }
+        if (regularCaseSensitivity === SortKind.None || typeCaseSensitivity === SortKind.None) {
+            return SortKind.None;
+        }
+        return typeCaseSensitivity & regularCaseSensitivity;
+    }
+
+    // else inline
     return detectSortCaseSensitivity(specifiers, specifier => specifier.name.text, collateCaseSensitive, collateCaseInsensitive);
 }, new ImportSpecifierSortingCache());
 
@@ -736,8 +784,8 @@ export function getImportDeclarationInsertionIndex(sortedImports: readonly AnyIm
 }
 
 /** @internal */
-export function getImportSpecifierInsertionIndex(sortedImports: readonly ImportSpecifier[], newImport: ImportSpecifier, comparer: Comparer<string>) {
-    const index = binarySearch(sortedImports, newImport, identity, (s1, s2) => compareImportOrExportSpecifiers(s1, s2, comparer));
+export function getImportSpecifierInsertionIndex(sortedImports: readonly ImportSpecifier[], newImport: ImportSpecifier, comparer: Comparer<string>, preferences: UserPreferences) {
+    const index = binarySearch(sortedImports, newImport, identity, (s1, s2) => compareImportOrExportSpecifiers(s1, s2, comparer, preferences));
     return index < 0 ? ~index : index;
 }
 

src/testRunner/unittests/services/organizeImports.ts

@@ -239,10 +239,10 @@ describe("unittests:: services:: organizeImports", () => {
             assertListEqual(actualCoalescedExports, expectedCoalescedExports);
         });
 
-        it("Sort specifiers - type-only", () => {
+        it("Sort specifiers - type-only-inline", () => {
             const sortedImports = parseImports(`import { type z, y, type x, c, type b, a } from "lib";`);
-            const actualCoalescedImports = ts.OrganizeImports.coalesceImports(sortedImports, /*ignoreCase*/ true);
-            const expectedCoalescedImports = parseImports(`import { a, c, y, type b, type x, type z } from "lib";`);
+            const actualCoalescedImports = ts.OrganizeImports.coalesceImports(sortedImports, /*ignoreCase*/ true, ts.getSourceFileOfNode(sortedImports[0]), { organizeImportsTypeOrder: "inline" });
+            const expectedCoalescedImports = parseImports(`import { a, type b, c, type x, y, type z } from "lib";`);
             assertListEqual(actualCoalescedImports, expectedCoalescedImports);
         });
 

tests/baselines/reference/api/typescript.d.ts

@@ -2933,6 +2933,13 @@ declare namespace ts {
                  * Default: `false`
                  */
                 readonly organizeImportsCaseFirst?: "upper" | "lower" | false;
+                /**
+                 * Indicates where named type-only imports should sort. "inline" sorts named imports without regard to if the import is
+                 * type-only.
+                 *
+                 * Default: `last`
+                 */
+                readonly organizeImportsTypeOrder?: "last" | "first" | "inline";
                 /**
                  * Indicates whether {@link ReferencesResponseItem.lineText} is supported.
                  */
@@ -8784,6 +8791,7 @@ declare namespace ts {
         readonly organizeImportsNumericCollation?: boolean;
         readonly organizeImportsAccentCollation?: boolean;
         readonly organizeImportsCaseFirst?: "upper" | "lower" | false;
+        readonly organizeImportsTypeOrder?: "first" | "last" | "inline";
         readonly excludeLibrarySymbolsInNavTo?: boolean;
     }
     /** Represents a bigint literal value without requiring bigint support */

tests/cases/fourslash/autoImportTypeImport1.ts

@@ -0,0 +1,46 @@
+/// <reference path="fourslash.ts" />
+
+// @verbatimModuleSyntax: true
+// @target: esnext
+
+// @Filename: /foo.ts
+//// export const A = 1;
+//// export type B = { x: number };
+//// export type C = 1;
+//// export class D = { y: string };
+
+// @Filename: /test.ts
+//// import { A, D, type C } from './foo';
+//// const b: B/**/ | C;
+//// console.log(A, D);
+
+goTo.marker("");
+
+// importFixes should only place the import in sorted position if the existing imports are sorted as specified,
+//     otherwise the import should be placed at the end 
+verify.importFixAtPosition([
+`import { A, D, type C, type B } from './foo';
+const b: B | C;
+console.log(A, D);`],
+    /*errorCode*/ undefined,
+    { organizeImportsTypeOrder: "inline" }
+    // `type B` is added to the end since the existing imports are not sorted as specified
+);
+
+verify.importFixAtPosition([
+`import { A, D, type B, type C } from './foo';
+const b: B | C;
+console.log(A, D);`],
+    /*errorCode*/ undefined,
+    { organizeImportsTypeOrder: "last" }
+    // `type B` is added to the sorted position since the existing imports *are* sorted as specified
+);
+
+verify.importFixAtPosition([
+`import { A, D, type C, type B } from './foo';
+const b: B | C;
+console.log(A, D);`],
+    /*errorCode*/ undefined,
+    { organizeImportsTypeOrder: "first" }
+    // `type B` is added to the end (default behavior) since the existing imports are not sorted as specified
+);

tests/cases/fourslash/autoImportTypeImport2.ts

@@ -0,0 +1,43 @@
+/// <reference path="fourslash.ts" />
+
+// @verbatimModuleSyntax: true
+// @target: esnext
+
+// @Filename: /foo.ts
+//// export const A = 1;
+//// export type B = { x: number };
+//// export type C = 1;
+//// export class D = { y: string };
+
+// @Filename: /test.ts
+//// import { A, type C, D } from './foo';
+//// const b: B/**/ | C;
+//// console.log(A, D);
+
+goTo.marker("");
+
+// importFixes should only place the import in sorted position if the existing imports are sorted as specified,
+//     otherwise the import should be placed at the end 
+verify.importFixAtPosition([
+`import { A, type B, type C, D } from './foo';
+const b: B | C;
+console.log(A, D);`],
+    /*errorCode*/ undefined,
+    { organizeImportsTypeOrder: "inline" }
+);
+
+verify.importFixAtPosition([
+`import { A, type C, D, type B } from './foo';
+const b: B | C;
+console.log(A, D);`],
+    /*errorCode*/ undefined,
+    { organizeImportsTypeOrder: "last" }
+);
+
+verify.importFixAtPosition([
+`import { A, type C, D, type B } from './foo';
+const b: B | C;
+console.log(A, D);`],
+    /*errorCode*/ undefined,
+    { organizeImportsTypeOrder: "first" }
+);