Change detection in organizeImports

src/compiler/core.ts

@@ -848,38 +848,6 @@ export function arrayIsSorted<T>(array: readonly T[], comparer: Comparer<T>) {
     return true;
 }
 
-/** @internal */
-export const enum SortKind {
-    None = 0,
-    CaseSensitive = 1 << 0,
-    CaseInsensitive = 1 << 1,
-    Both = CaseSensitive | CaseInsensitive,
-}
-
-/** @internal */
-export function detectSortCaseSensitivity<T>(
-    array: readonly T[],
-    getString: (element: T) => string,
-    compareStringsCaseSensitive: Comparer<string>,
-    compareStringsCaseInsensitive: Comparer<string>,
-): SortKind {
-    let kind = SortKind.Both;
-    if (array.length < 2) return kind;
-
-    let prevElement = getString(array[0]);
-    for (let i = 1, len = array.length; i < len && kind !== SortKind.None; i++) {
-        const element = getString(array[i]);
-        if (kind & SortKind.CaseSensitive && compareStringsCaseSensitive(prevElement, element) > 0) {
-            kind &= ~SortKind.CaseSensitive;
-        }
-        if (kind & SortKind.CaseInsensitive && compareStringsCaseInsensitive(prevElement, element) > 0) {
-            kind &= ~SortKind.CaseInsensitive;
-        }
-        prevElement = element;
-    }
-    return kind;
-}
-
 /** @internal */
 export function arrayIsEqualTo<T>(array1: readonly T[] | undefined, array2: readonly T[] | undefined, equalityComparer: (a: T, b: T, index: number) => boolean = equateValues): boolean {
     if (!array1 || !array2) {

src/compiler/types.ts

@@ -9969,7 +9969,7 @@ export interface UserPreferences {
      *
      * Default: `last`
      */
-    readonly organizeImportsTypeOrder?: "first" | "last" | "inline";
+    readonly organizeImportsTypeOrder?: OrganizeImportsTypeOrder;
     /**
      * Indicates whether to exclude standard library and node_modules file symbols from navTo results.
      */
@@ -9980,6 +9980,8 @@ export interface UserPreferences {
     readonly disableLineTextInReferences?: boolean;
 }
 
+export type OrganizeImportsTypeOrder = "last" | "inline" | "first";
+
 /** Represents a bigint literal value without requiring bigint support */
 export interface PseudoBigInt {
     negative: boolean;

src/services/codefixes/importFixes.ts

@@ -114,14 +114,14 @@ import {
     removeFileExtension,
     removeSuffix,
     RequireVariableStatement,
+    sameMap,
     ScriptTarget,
     SemanticMeaning,
     shouldUseUriStyleNodeCoreModules,
     single,
     skipAlias,
     some,
     sort,
-    SortKind,
     SourceFile,
     stableSort,
     startsWith,
@@ -1394,11 +1394,10 @@ function promoteFromTypeOnly(
     switch (aliasDeclaration.kind) {
         case SyntaxKind.ImportSpecifier:
             if (aliasDeclaration.isTypeOnly) {
-                const sortKind = OrganizeImports.detectImportSpecifierSorting(aliasDeclaration.parent.elements, preferences);
-                if (aliasDeclaration.parent.elements.length > 1 && sortKind) {
+                if (aliasDeclaration.parent.elements.length > 1) {
                     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, preferences);
+                    const { specifierComparer } = OrganizeImports.getNamedImportSpecifierComparerWithDetection(aliasDeclaration.parent.parent.parent, preferences, sourceFile);
+                    const insertionIndex = OrganizeImports.getImportSpecifierInsertionIndex(aliasDeclaration.parent.elements, newSpecifier, specifierComparer);
                     if (insertionIndex !== aliasDeclaration.parent.elements.indexOf(aliasDeclaration)) {
                         changes.delete(sourceFile, aliasDeclaration);
                         changes.insertImportSpecifierAtIndex(sourceFile, newSpecifier, aliasDeclaration.parent, insertionIndex);
@@ -1440,8 +1439,9 @@ function promoteFromTypeOnly(
         if (convertExistingToTypeOnly) {
             const namedImports = tryCast(importClause.namedBindings, isNamedImports);
             if (namedImports && namedImports.elements.length > 1) {
+                const sortState = OrganizeImports.getNamedImportSpecifierComparerWithDetection(importClause.parent, preferences, sourceFile);
                 if (
-                    OrganizeImports.detectImportSpecifierSorting(namedImports.elements, preferences) &&
+                    (sortState.isSorted !== false) &&
                     aliasDeclaration.kind === SyntaxKind.ImportSpecifier &&
                     namedImports.elements.indexOf(aliasDeclaration) !== 0
                 ) {
@@ -1478,6 +1478,7 @@ function doAddExistingFix(
         return;
     }
 
+    // promoteFromTypeOnly = true if we need to promote the entire original clause from type only
     const promoteFromTypeOnly = clause.isTypeOnly && some([defaultImport, ...namedImports], i => i?.addAsTypeOnly === AddAsTypeOnly.NotAllowed);
     const existingSpecifiers = clause.namedBindings && tryCast(clause.namedBindings, isNamedImports)?.elements;
 
@@ -1487,25 +1488,7 @@ function doAddExistingFix(
     }
 
     if (namedImports.length) {
-        // sort case sensitivity:
-        // - if the user preference is explicit, use that
-        // - otherwise, if there are enough existing import specifiers in this import to detect unambiguously, use that
-        // - otherwise, detect from other imports in the file
-        let ignoreCaseForSorting: boolean | undefined;
-        if (typeof preferences.organizeImportsIgnoreCase === "boolean") {
-            ignoreCaseForSorting = preferences.organizeImportsIgnoreCase;
-        }
-        else if (existingSpecifiers) {
-            const targetImportSorting = OrganizeImports.detectImportSpecifierSorting(existingSpecifiers, preferences);
-            if (targetImportSorting !== SortKind.Both) {
-                ignoreCaseForSorting = targetImportSorting === SortKind.CaseInsensitive;
-            }
-        }
-        if (ignoreCaseForSorting === undefined) {
-            ignoreCaseForSorting = OrganizeImports.detectSorting(sourceFile, preferences) === SortKind.CaseInsensitive;
-        }
-
-        const comparer = OrganizeImports.getOrganizeImportsComparer(preferences, ignoreCaseForSorting);
+        const { specifierComparer, isSorted } = OrganizeImports.getNamedImportSpecifierComparerWithDetection(clause.parent, preferences, sourceFile);
         const newSpecifiers = stableSort(
             namedImports.map(namedImport =>
                 factory.createImportSpecifier(
@@ -1514,23 +1497,24 @@ function doAddExistingFix(
                     factory.createIdentifier(namedImport.name),
                 )
             ),
-            (s1, s2) => OrganizeImports.compareImportOrExportSpecifiers(s1, s2, comparer),
+            specifierComparer,
         );
 
         // The sorting preference computed earlier may or may not have validated that these particular
         // import specifiers are sorted. If they aren't, `getImportSpecifierInsertionIndex` will return
         // nonsense. So if there are existing specifiers, even if we know the sorting preference, we
         // need to ensure that the existing specifiers are sorted according to the preference in order
         // to do a sorted insertion.
-        const specifierSort = existingSpecifiers?.length && OrganizeImports.detectImportSpecifierSorting(existingSpecifiers, preferences);
-        if (specifierSort && !(ignoreCaseForSorting && specifierSort === SortKind.CaseSensitive)) {
+
+        // changed to check if existing specifiers are sorted
+        if (existingSpecifiers?.length && isSorted !== false) {
+            // if we're promoting the clause from type-only, we need to transform the existing imports before attempting to insert the new named imports
+            const transformedExistingSpecifiers = (promoteFromTypeOnly && existingSpecifiers) ? factory.updateNamedImports(
+                clause.namedBindings as NamedImports,
+                sameMap(existingSpecifiers, e => factory.updateImportSpecifier(e, /*isTypeOnly*/ true, e.propertyName, e.name)),
+            ).elements : existingSpecifiers;
             for (const spec of newSpecifiers) {
-                // Organize imports puts type-only import specifiers last, so if we're
-                // adding a non-type-only specifier and converting all the other ones to
-                // 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, preferences);
+                const insertionIndex = OrganizeImports.getImportSpecifierInsertionIndex(transformedExistingSpecifiers, spec, specifierComparer);
                 changes.insertImportSpecifierAtIndex(sourceFile, spec, clause.namedBindings as NamedImports, insertionIndex);
             }
         }