From 6100acdc9fc908c9cbaa1f49e60df15a3be63f7c Mon Sep 17 00:00:00 2001 From: Ron Buckton Date: Wed, 4 Jan 2023 18:08:27 -0500 Subject: [PATCH 01/10] Add configurable collation settings for 'organize imports' --- src/compiler/types.ts | 5 + src/server/protocol.ts | 51 +++++++ src/services/organizeImports.ts | 144 +++++++++++++----- tests/cases/fourslash/fourslash.ts | 5 + .../fourslash/organizeImportsNatural1.ts | 36 +++++ .../fourslash/organizeImportsNatural2.ts | 35 +++++ .../fourslash/organizeImportsNatural3.ts | 35 +++++ .../fourslash/organizeImportsNatural4.ts | 38 +++++ 8 files changed, 310 insertions(+), 39 deletions(-) create mode 100644 tests/cases/fourslash/organizeImportsNatural1.ts create mode 100644 tests/cases/fourslash/organizeImportsNatural2.ts create mode 100644 tests/cases/fourslash/organizeImportsNatural3.ts create mode 100644 tests/cases/fourslash/organizeImportsNatural4.ts diff --git a/src/compiler/types.ts b/src/compiler/types.ts index 44a716a516f7a..6e78b9af54dba 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -9771,6 +9771,11 @@ export interface UserPreferences { readonly allowRenameOfImportPath?: boolean; readonly autoImportFileExcludePatterns?: string[]; readonly organizeImportsIgnoreCase?: "auto" | boolean; + readonly organizeImportsCollation?: "natural" | "ordinal"; + readonly organizeImportsLocale?: string; + readonly organizeImportsNumericCollation?: boolean; + readonly organizeImportsAccentCollation?: boolean; + readonly organizeImportsCaseFirst?: "upper" | "lower" | false; } /** Represents a bigint literal value without requiring bigint support */ diff --git a/src/server/protocol.ts b/src/server/protocol.ts index 76c80264ce968..0d3a1c2831c07 100644 --- a/src/server/protocol.ts +++ b/src/server/protocol.ts @@ -3516,7 +3516,58 @@ export interface UserPreferences { readonly includeInlayFunctionLikeReturnTypeHints?: boolean; readonly includeInlayEnumMemberValueHints?: boolean; readonly autoImportFileExcludePatterns?: string[]; + + /** + * Indicates whether imports should be organized in a case-insensitive manner. + */ readonly organizeImportsIgnoreCase?: "auto" | boolean; + /** + * Indicates whether imports should be organized via an "ordinal" (binary) comparison using the numeric value + * of their code points, or via "natural" collation using rules associated with the locale specified by + * {@link organizeImportsCollationLocale}. + * + * NOTE: When comparing paths `"ordinal"` collation is always used. + * + * Default: `"ordinal"`. + */ + readonly organizeImportsCollation?: "natural" | "ordinal"; + /** + * Indicates the locale to use for "natural" collation. If not specified, the locale `"en"` is used as an invariant + * for the sake of consistent sorting. Use `"auto"` to use the detected UI locale. + * + * This preference is ignored if {@link organizeImportsCollation} is not `"natural"`. + * + * Default: `"en"` + */ + readonly organizeImportsCollationLocale?: string; + /** + * Indicates whether numeric collation should be used for digit sequences in strings. When `true`, will collate + * strings such that `a1z < a2z < a100z`. When `false`, will collate strings such that `a1z < a100z < a2z`. + * + * This preference is ignored if {@link organizeImportsCollation} is not `"natural"`. + * + * Default: `true` + */ + readonly organizeImportsNumericCollation?: boolean; + /** + * Indicates whether accents and other diacritic marks are considered unequal for the purpose of collation. When + * `true`, characters with accents and other diacritics will be collated in the order defined by the locale specified + * in {@link organizeImportsCollationLocale}. + * + * This preference is ignored if {@link organizeImportsCollation} is not `"natural"`. + * + * Default: `true` + */ + readonly organizeImportsAccentCollation?: boolean; + /** + * Indicates whether upper case or lower case should sort first. When `false`, the default order for the locale + * specified in {@link organizeImportsCollationLocale} is used. + * + * This preference is ignored if {@link organizeImportsCollation} is not `"natural"`. + * + * Default: `false` + */ + readonly organizeImportsCaseFirst?: "upper" | "lower" | false; /** * Indicates whether {@link ReferencesResponseItem.lineText} is supported. diff --git a/src/services/organizeImports.ts b/src/services/organizeImports.ts index 881c2d6471501..29eca95e6c6a1 100644 --- a/src/services/organizeImports.ts +++ b/src/services/organizeImports.ts @@ -3,6 +3,7 @@ import { arrayIsSorted, binarySearch, compareBooleans, + Comparer, compareStringsCaseInsensitiveEslintCompatible, compareStringsCaseSensitive, compareValues, @@ -21,9 +22,11 @@ import { flatMap, formatting, getNewLineOrDefaultFromHost, + getUILocale, group, Identifier, identity, + ImportClause, ImportDeclaration, ImportOrExportSpecifier, ImportSpecifier, @@ -83,19 +86,16 @@ export function organizeImports( const shouldSort = mode === OrganizeImportsMode.SortAndCombine || mode === OrganizeImportsMode.All; const shouldCombine = shouldSort; // These are currently inseparable, but I draw a distinction for clarity and in case we add modes in the future. const shouldRemove = mode === OrganizeImportsMode.RemoveUnused || mode === OrganizeImportsMode.All; - const maybeRemove = shouldRemove ? removeUnusedImports : identity; - const maybeCoalesce = shouldCombine ? coalesceImports : identity; // All of the old ImportDeclarations in the file, in syntactic order. const topLevelImportGroupDecls = groupImportsByNewlineContiguous(sourceFile, sourceFile.statements.filter(isImportDeclaration)); - const ignoreCase = typeof preferences.organizeImportsIgnoreCase === "boolean" - ? preferences.organizeImportsIgnoreCase - : shouldSort && detectSortingWorker(topLevelImportGroupDecls) === SortKind.CaseInsensitive; + + const comparer = getOrganizeImportsComparer(preferences, shouldSort ? () => detectSortingWorker(topLevelImportGroupDecls) === SortKind.CaseInsensitive : undefined); const processImportsOfSameModuleSpecifier = (importGroup: readonly ImportDeclaration[]) => { - const processedDeclarations = maybeCoalesce(maybeRemove(importGroup, sourceFile, program), ignoreCase, sourceFile); - return shouldSort - ? stableSort(processedDeclarations, (s1, s2) => compareImportsOrRequireStatements(s1, s2)) - : processedDeclarations; + if (shouldRemove) importGroup = removeUnusedImports(importGroup, sourceFile, program); + if (shouldCombine) importGroup = coalesceImportsWorker(importGroup, comparer, sourceFile); + if (shouldSort) importGroup = stableSort(importGroup, (s1, s2) => compareImportsOrRequireStatements(s1, s2)); + return importGroup; }; topLevelImportGroupDecls.forEach(importGroupDecl => organizeImportsWorker(importGroupDecl, processImportsOfSameModuleSpecifier)); @@ -104,7 +104,7 @@ export function organizeImports( if (mode !== OrganizeImportsMode.RemoveUnused) { // All of the old ExportDeclarations in the file, in syntactic order. const topLevelExportDecls = sourceFile.statements.filter(isExportDeclaration); - organizeImportsWorker(topLevelExportDecls, group => coalesceExports(group, ignoreCase)); + organizeImportsWorker(topLevelExportDecls, group => coalesceExportsWorker(group, comparer)); } for (const ambientModule of sourceFile.statements.filter(isAmbientModule)) { @@ -116,7 +116,7 @@ export function organizeImports( // Exports are always used if (mode !== OrganizeImportsMode.RemoveUnused) { const ambientModuleExportDecls = ambientModule.body.statements.filter(isExportDeclaration); - organizeImportsWorker(ambientModuleExportDecls, group => coalesceExports(group, ignoreCase)); + organizeImportsWorker(ambientModuleExportDecls, group => coalesceExportsWorker(group, comparer)); } } @@ -309,12 +309,16 @@ function getExternalModuleName(specifier: Expression | undefined) { * @internal */ export function coalesceImports(importGroup: readonly ImportDeclaration[], ignoreCase?: boolean, sourceFile?: SourceFile): readonly ImportDeclaration[] { + const comparer = getOrganizeImportsOrdinalStringComparer(!!ignoreCase); + return coalesceImportsWorker(importGroup, comparer, sourceFile); +} + +function coalesceImportsWorker(importGroup: readonly ImportDeclaration[], comparer: Comparer, sourceFile?: SourceFile): readonly ImportDeclaration[] { if (importGroup.length === 0) { return importGroup; } const { importWithoutClause, typeOnlyImports, regularImports } = getCategorizedImports(importGroup); - const compareIdentifiers = ignoreCase ? compareIdentifiersCaseInsensitive : compareIdentifiersCaseSensitive; const coalescedImports: ImportDeclaration[] = []; if (importWithoutClause) { @@ -330,18 +334,18 @@ export function coalesceImports(importGroup: readonly ImportDeclaration[], ignor // Add the namespace import to the existing default ImportDeclaration. const defaultImport = defaultImports[0]; coalescedImports.push( - updateImportDeclarationAndClause(defaultImport, defaultImport.importClause!.name, namespaceImports[0].importClause!.namedBindings)); // TODO: GH#18217 + updateImportDeclarationAndClause(defaultImport, defaultImport.importClause.name, namespaceImports[0].importClause.namedBindings)); continue; } const sortedNamespaceImports = stableSort(namespaceImports, (i1, i2) => - compareIdentifiers((i1.importClause!.namedBindings as NamespaceImport).name, (i2.importClause!.namedBindings as NamespaceImport).name)); // TODO: GH#18217 + comparer(i1.importClause.namedBindings.name.text, i2.importClause.namedBindings.name.text)); for (const namespaceImport of sortedNamespaceImports) { // Drop the name, if any coalescedImports.push( - updateImportDeclarationAndClause(namespaceImport, /*name*/ undefined, namespaceImport.importClause!.namedBindings)); // TODO: GH#18217 + updateImportDeclarationAndClause(namespaceImport, /*name*/ undefined, namespaceImport.importClause.namedBindings)); } if (defaultImports.length === 0 && namedImports.length === 0) { @@ -351,20 +355,20 @@ export function coalesceImports(importGroup: readonly ImportDeclaration[], ignor let newDefaultImport: Identifier | undefined; const newImportSpecifiers: ImportSpecifier[] = []; if (defaultImports.length === 1) { - newDefaultImport = defaultImports[0].importClause!.name; + newDefaultImport = defaultImports[0].importClause.name; } else { for (const defaultImport of defaultImports) { newImportSpecifiers.push( - factory.createImportSpecifier(/*isTypeOnly*/ false, factory.createIdentifier("default"), defaultImport.importClause!.name!)); // TODO: GH#18217 + factory.createImportSpecifier(/*isTypeOnly*/ false, factory.createIdentifier("default"), defaultImport.importClause.name)); } } newImportSpecifiers.push(...getNewImportSpecifiers(namedImports)); const sortedImportSpecifiers = factory.createNodeArray( - sortSpecifiers(newImportSpecifiers, ignoreCase), - (namedImports[0]?.importClause!.namedBindings as NamedImports)?.elements.hasTrailingComma + sortSpecifiers(newImportSpecifiers, comparer), + namedImports[0].importClause.namedBindings.elements.hasTrailingComma ); const importDecl = defaultImports.length > 0 @@ -377,11 +381,12 @@ export function coalesceImports(importGroup: readonly ImportDeclaration[], ignor : factory.createNamedImports(emptyArray) : namedImports.length === 0 ? factory.createNamedImports(sortedImportSpecifiers) - : factory.updateNamedImports(namedImports[0].importClause!.namedBindings as NamedImports, sortedImportSpecifiers); // TODO: GH#18217 + : factory.updateNamedImports(namedImports[0].importClause.namedBindings, sortedImportSpecifiers); if (sourceFile && newNamedImports && - namedImports[0]?.importClause!.namedBindings && + namedImports.length && + namedImports[0].importClause.namedBindings && !rangeIsOnSingleLine(namedImports[0].importClause.namedBindings, sourceFile) ) { setEmitFlags(newNamedImports, EmitFlags.MultiLine); @@ -403,13 +408,30 @@ export function coalesceImports(importGroup: readonly ImportDeclaration[], ignor } return coalescedImports; - } +type ImportDeclarationWithDefaultImport = ImportDeclaration & { + readonly importClause: ImportClause & { + readonly name: Identifier; + }; +}; + +type ImportDeclarationWithNamespaceImport = ImportDeclaration & { + readonly importClause: ImportClause & { + readonly namedBindings: NamespaceImport; + }; +}; + +type ImportDeclarationWithNamedImports = ImportDeclaration & { + readonly importClause: ImportClause & { + readonly namedBindings: NamedImports; + }; +}; + interface ImportGroup { - defaultImports: ImportDeclaration[]; - namespaceImports: ImportDeclaration[]; - namedImports: ImportDeclaration[]; + defaultImports: ImportDeclarationWithDefaultImport[]; + namespaceImports: ImportDeclarationWithNamespaceImport[]; + namedImports: ImportDeclarationWithNamedImports[]; } /* @@ -436,15 +458,15 @@ function getCategorizedImports(importGroup: readonly ImportDeclaration[]) { const { name, namedBindings } = importDeclaration.importClause; if (name) { - group.defaultImports.push(importDeclaration); + group.defaultImports.push(importDeclaration as ImportDeclarationWithDefaultImport); } if (namedBindings) { if (isNamespaceImport(namedBindings)) { - group.namespaceImports.push(importDeclaration); + group.namespaceImports.push(importDeclaration as ImportDeclarationWithNamespaceImport); } else { - group.namedImports.push(importDeclaration); + group.namedImports.push(importDeclaration as ImportDeclarationWithNamedImports); } } } @@ -463,6 +485,11 @@ function getCategorizedImports(importGroup: readonly ImportDeclaration[]) { * @internal */ export function coalesceExports(exportGroup: readonly ExportDeclaration[], ignoreCase: boolean) { + const comparer = getOrganizeImportsOrdinalStringComparer(ignoreCase); + return coalesceExportsWorker(exportGroup, comparer); +} + +function coalesceExportsWorker(exportGroup: readonly ExportDeclaration[], comparer: Comparer) { if (exportGroup.length === 0) { return exportGroup; } @@ -482,7 +509,7 @@ export function coalesceExports(exportGroup: readonly ExportDeclaration[], ignor const newExportSpecifiers: ExportSpecifier[] = []; newExportSpecifiers.push(...flatMap(exportGroup, i => i.exportClause && isNamedExports(i.exportClause) ? i.exportClause.elements : emptyArray)); - const sortedExportSpecifiers = sortSpecifiers(newExportSpecifiers, ignoreCase); + const sortedExportSpecifiers = sortSpecifiers(newExportSpecifiers, comparer); const exportDecl = exportGroup[0]; coalescedExports.push( @@ -546,22 +573,17 @@ function updateImportDeclarationAndClause( importDeclaration.assertClause); } -function sortSpecifiers(specifiers: readonly T[], ignoreCase?: boolean) { - return stableSort(specifiers, (s1, s2) => compareImportOrExportSpecifiers(s1, s2, ignoreCase)); +function sortSpecifiers(specifiers: readonly T[], comparer: Comparer) { + return stableSort(specifiers, (s1, s2) => compareImportOrExportSpecifiersWorker(s1, s2, comparer)); } /** @internal */ export function compareImportOrExportSpecifiers(s1: T, s2: T, ignoreCase?: boolean): Comparison { - const compareIdentifiers = ignoreCase ? compareIdentifiersCaseInsensitive : compareIdentifiersCaseSensitive; - return compareBooleans(s1.isTypeOnly, s2.isTypeOnly) || compareIdentifiers(s1.name, s2.name); -} - -function compareIdentifiersCaseSensitive(s1: Identifier, s2: Identifier) { - return compareStringsCaseSensitive(s1.text, s2.text); + return compareImportOrExportSpecifiersWorker(s1, s2, getOrganizeImportsOrdinalStringComparer(!!ignoreCase)) } -function compareIdentifiersCaseInsensitive(s1: Identifier, s2: Identifier) { - return compareStringsCaseInsensitiveEslintCompatible(s1.text, s2.text); +function compareImportOrExportSpecifiersWorker(s1: T, s2: T, comparer: Comparer): Comparison { + return compareBooleans(s1.isTypeOnly, s2.isTypeOnly) || comparer(s1.name.text, s2.name.text); } /** @@ -699,3 +721,47 @@ function tryGetNamedBindingElements(namedImport: ImportDeclaration) { ? namedImport.importClause.namedBindings.elements : undefined; } + +function getOrganizeImportsOrdinalStringComparer(ignoreCase: boolean) { + return ignoreCase ? compareStringsCaseInsensitiveEslintCompatible : compareStringsCaseSensitive; +} + +function getOrganizeImportsNaturalStringComparer(ignoreCase: boolean, preferences: UserPreferences): Comparer { + const resolvedLocale = getOrganizeImportsLocale(preferences); + const caseFirst = preferences.organizeImportsCaseFirst ?? false; + const numeric = preferences.organizeImportsNumericCollation ?? false; + const accents = preferences.organizeImportsAccentCollation ?? true; + const sensitivity = + ignoreCase ? + accents ? "accent" : "base" : + accents ? "variant" : "case"; + + const collator = new Intl.Collator(resolvedLocale, { + usage: "sort", + caseFirst: caseFirst || "false", + sensitivity, + numeric, + }); + + // `compare` is a bound method, so we do not need to close over `collator`. + return collator.compare; +} + +function getOrganizeImportsLocale(preferences: UserPreferences): string { + let locale = preferences.organizeImportsLocale; + if (locale === "auto") locale = getUILocale(); + if (locale === undefined) locale = "en"; + + const supportedLocales = Intl.Collator.supportedLocalesOf(locale); + const resolvedLocale = supportedLocales.length ? supportedLocales[0] : "en"; + return resolvedLocale; +} + +/** @internal */ +export function getOrganizeImportsComparer(preferences: UserPreferences, detectIgnoreCase?: () => boolean): Comparer { + const collation = preferences.organizeImportsCollation ?? "ordinal"; + const ignoreCase = typeof preferences.organizeImportsIgnoreCase === "boolean" ? preferences.organizeImportsIgnoreCase : detectIgnoreCase?.() ?? false; + return collation === "ordinal" ? + getOrganizeImportsOrdinalStringComparer(ignoreCase) : + getOrganizeImportsNaturalStringComparer(ignoreCase, preferences); +} \ No newline at end of file diff --git a/tests/cases/fourslash/fourslash.ts b/tests/cases/fourslash/fourslash.ts index 262d2327d2ffc..0707dc892bebc 100644 --- a/tests/cases/fourslash/fourslash.ts +++ b/tests/cases/fourslash/fourslash.ts @@ -673,6 +673,11 @@ declare namespace FourSlashInterface { readonly allowRenameOfImportPath?: boolean; readonly autoImportFileExcludePatterns?: readonly string[]; readonly organizeImportsIgnoreCase?: "auto" | boolean; + readonly organizeImportsCollation?: "natural" | "ordinal"; + readonly organizeImportsLocale?: string; + readonly organizeImportsNumericCollation?: boolean; + readonly organizeImportsAccentCollation?: boolean; + readonly organizeImportsCaseFirst?: "upper" | "lower" | false; } interface InlayHintsOptions extends UserPreferences { readonly includeInlayParameterNameHints?: "none" | "literals" | "all"; diff --git a/tests/cases/fourslash/organizeImportsNatural1.ts b/tests/cases/fourslash/organizeImportsNatural1.ts new file mode 100644 index 0000000000000..7ce4df267e983 --- /dev/null +++ b/tests/cases/fourslash/organizeImportsNatural1.ts @@ -0,0 +1,36 @@ +/// + +//// import { +//// Ab, +//// _aB, +//// aB, +//// _Ab, +//// } from './foo'; +//// +//// console.log(_aB, _Ab, aB, Ab); + +verify.organizeImports( +`import { + Ab, + _Ab, + _aB, + aB, +} from './foo'; + +console.log(_aB, _Ab, aB, Ab);`, /*mode*/ undefined, { + organizeImportsIgnoreCase: false, + organizeImportsCollation: "ordinal", +}); + +verify.organizeImports( +`import { + _aB, + _Ab, + aB, + Ab, +} from './foo'; + +console.log(_aB, _Ab, aB, Ab);`, /*mode*/ undefined, { + organizeImportsIgnoreCase: false, + organizeImportsCollation: "natural", +}); diff --git a/tests/cases/fourslash/organizeImportsNatural2.ts b/tests/cases/fourslash/organizeImportsNatural2.ts new file mode 100644 index 0000000000000..524eb6b06eed2 --- /dev/null +++ b/tests/cases/fourslash/organizeImportsNatural2.ts @@ -0,0 +1,35 @@ +/// + +//// import { +//// a2, +//// a100, +//// a1, +//// } from './foo'; +//// +//// console.log(a1, a2, a100); + +verify.organizeImports( +`import { + a1, + a100, + a2, +} from './foo'; + +console.log(a1, a2, a100);`, /*mode*/ undefined, { + organizeImportsIgnoreCase: false, + organizeImportsCollation: "natural", + organizeImportsNumericCollation: false, +}); + +verify.organizeImports( +`import { + a1, + a2, + a100, +} from './foo'; + +console.log(a1, a2, a100);`, /*mode*/ undefined, { + organizeImportsIgnoreCase: false, + organizeImportsCollation: "natural", + organizeImportsNumericCollation: true, +}); diff --git a/tests/cases/fourslash/organizeImportsNatural3.ts b/tests/cases/fourslash/organizeImportsNatural3.ts new file mode 100644 index 0000000000000..8bcb8f54a831e --- /dev/null +++ b/tests/cases/fourslash/organizeImportsNatural3.ts @@ -0,0 +1,35 @@ +/// + +//// import { +//// B, +//// À, +//// A, +//// } from './foo'; +//// +//// console.log(A, À, B); + +verify.organizeImports( +`import { + À, + A, + B, +} from './foo'; + +console.log(A, À, B);`, /*mode*/ undefined, { + organizeImportsIgnoreCase: false, + organizeImportsCollation: "natural", + organizeImportsAccentCollation: false, +}); + +verify.organizeImports( +`import { + A, + À, + B, +} from './foo'; + +console.log(A, À, B);`, /*mode*/ undefined, { + organizeImportsIgnoreCase: false, + organizeImportsCollation: "natural", + organizeImportsAccentCollation: true, +}); diff --git a/tests/cases/fourslash/organizeImportsNatural4.ts b/tests/cases/fourslash/organizeImportsNatural4.ts new file mode 100644 index 0000000000000..f154e45e2b398 --- /dev/null +++ b/tests/cases/fourslash/organizeImportsNatural4.ts @@ -0,0 +1,38 @@ +/// + +//// import { +//// Ab, +//// _aB, +//// aB, +//// _Ab, +//// } from './foo'; +//// +//// console.log(_aB, _Ab, aB, Ab); + +verify.organizeImports( +`import { + _Ab, + _aB, + Ab, + aB, +} from './foo'; + +console.log(_aB, _Ab, aB, Ab);`, /*mode*/ undefined, { + organizeImportsIgnoreCase: false, + organizeImportsCollation: "natural", + organizeImportsCaseFirst: "upper", +}); + +verify.organizeImports( +`import { + _aB, + _Ab, + aB, + Ab, +} from './foo'; + +console.log(_aB, _Ab, aB, Ab);`, /*mode*/ undefined, { + organizeImportsIgnoreCase: false, + organizeImportsCollation: "natural", + organizeImportsCaseFirst: "lower", +}); From 8aaaba4c4c675890ca4ab7547dd1402a8631c0f8 Mon Sep 17 00:00:00 2001 From: Ron Buckton Date: Wed, 4 Jan 2023 21:38:45 -0500 Subject: [PATCH 02/10] Auto-imports should use collation settings --- src/compiler/core.ts | 54 ++++++++++----- src/services/codefixes/importFixes.ts | 20 +++--- src/services/organizeImports.ts | 69 ++++++++++++++----- .../fourslash/importNameCodeFix_order2.ts | 53 ++++++++++++++ 4 files changed, 152 insertions(+), 44 deletions(-) create mode 100644 tests/cases/fourslash/importNameCodeFix_order2.ts diff --git a/src/compiler/core.ts b/src/compiler/core.ts index a5e2407ccb441..a61388991e837 100644 --- a/src/compiler/core.ts +++ b/src/compiler/core.ts @@ -961,32 +961,25 @@ export const enum SortKind { } /** @internal */ -export function detectSortCaseSensitivity(array: readonly string[], useEslintOrdering?: boolean): SortKind; -/** @internal */ -export function detectSortCaseSensitivity(array: readonly T[], useEslintOrdering: boolean, getString: (element: T) => string): SortKind; -/** @internal */ -export function detectSortCaseSensitivity(array: readonly T[], useEslintOrdering: boolean, getString?: (element: T) => string): SortKind { +export function detectSortCaseSensitivity( + array: readonly T[], + getString: (element: T) => string, + compareStringsCaseSensitive: Comparer, + compareStringsCaseInsensitive: Comparer, +): SortKind { let kind = SortKind.Both; if (array.length < 2) return kind; - const caseSensitiveComparer = getString - ? (a: T, b: T) => compareStringsCaseSensitive(getString(a), getString(b)) - : compareStringsCaseSensitive as (a: T | undefined, b: T | undefined) => Comparison; - const compareCaseInsensitive = useEslintOrdering ? compareStringsCaseInsensitiveEslintCompatible : compareStringsCaseInsensitive; - const caseInsensitiveComparer = getString - ? (a: T, b: T) => compareCaseInsensitive(getString(a), getString(b)) - : compareCaseInsensitive as (a: T | undefined, b: T | undefined) => Comparison; + + let prevElement = getString(array[0]); for (let i = 1, len = array.length; i < len; i++) { - const prevElement = array[i - 1]; - const element = array[i]; - if (kind & SortKind.CaseSensitive && caseSensitiveComparer(prevElement, element) === Comparison.GreaterThan) { + const element = getString(array[i]); + if (kind & SortKind.CaseSensitive && compareStringsCaseSensitive(prevElement, element) > 0) { kind &= ~SortKind.CaseSensitive; } - if (kind & SortKind.CaseInsensitive && caseInsensitiveComparer(prevElement, element) === Comparison.GreaterThan) { + if (kind & SortKind.CaseInsensitive && compareStringsCaseInsensitive(prevElement, element) > 0) { kind &= ~SortKind.CaseInsensitive; } - if (kind === SortKind.None) { - return kind; - } + prevElement = element; } return kind; } @@ -2200,6 +2193,29 @@ export function memoizeWeak(callback: (arg: A) => T): (arg: }; } +/** @internal */ +export interface MemoizeCache { + has(args: A): boolean; + get(args: A): T | undefined; + set(args: A, value: T): void; +} + +/** + * A version of `memoize` that supports multiple arguments, backed by a provided cache. + * + * @internal + */ +export function memoizeCached(callback: (...args: A) => T, cache: MemoizeCache): (...args: A) => T { + return (...args: A) => { + let value = cache.get(args); + if (value === undefined && !cache.has(args)) { + value = callback(...args); + cache.set(args, value); + } + return value!; + }; +} + /** * High-order function, composes functions. Note that functions are composed inside-out; * for example, `compose(a, b)` is the equivalent of `x => b(a(x))`. diff --git a/src/services/codefixes/importFixes.ts b/src/services/codefixes/importFixes.ts index ac89579ab0ed5..8ce871555f866 100644 --- a/src/services/codefixes/importFixes.ts +++ b/src/services/codefixes/importFixes.ts @@ -1222,7 +1222,7 @@ function codeActionForFixWorker(changes: textChanges.ChangeTracker, sourceFile: } case ImportFixKind.PromoteTypeOnly: { const { typeOnlyAliasDeclaration } = fix; - const promotedDeclaration = promoteFromTypeOnly(changes, typeOnlyAliasDeclaration, compilerOptions, sourceFile); + const promotedDeclaration = promoteFromTypeOnly(changes, typeOnlyAliasDeclaration, compilerOptions, sourceFile, preferences); return promotedDeclaration.kind === SyntaxKind.ImportSpecifier ? [Diagnostics.Remove_type_from_import_of_0_from_1, symbolName, getModuleSpecifierText(promotedDeclaration.parent.parent)] : [Diagnostics.Remove_type_from_import_declaration_from_0, getModuleSpecifierText(promotedDeclaration)]; @@ -1238,17 +1238,18 @@ function getModuleSpecifierText(promotedDeclaration: ImportClause | ImportEquals : cast(promotedDeclaration.parent.moduleSpecifier, isStringLiteral).text; } -function promoteFromTypeOnly(changes: textChanges.ChangeTracker, aliasDeclaration: TypeOnlyAliasDeclaration, compilerOptions: CompilerOptions, sourceFile: SourceFile) { +function promoteFromTypeOnly(changes: textChanges.ChangeTracker, aliasDeclaration: TypeOnlyAliasDeclaration, compilerOptions: CompilerOptions, sourceFile: SourceFile, preferences: UserPreferences) { // See comment in `doAddExistingFix` on constant with the same name. const convertExistingToTypeOnly = compilerOptions.preserveValueImports && compilerOptions.isolatedModules; switch (aliasDeclaration.kind) { case SyntaxKind.ImportSpecifier: if (aliasDeclaration.isTypeOnly) { - const sortKind = OrganizeImports.detectImportSpecifierSorting(aliasDeclaration.parent.elements); + const sortKind = OrganizeImports.detectImportSpecifierSorting(aliasDeclaration.parent.elements, preferences); if (aliasDeclaration.parent.elements.length > 1 && sortKind) { changes.delete(sourceFile, aliasDeclaration); const newSpecifier = factory.updateImportSpecifier(aliasDeclaration, /*isTypeOnly*/ false, aliasDeclaration.propertyName, aliasDeclaration.name); - const insertionIndex = OrganizeImports.getImportSpecifierInsertionIndex(aliasDeclaration.parent.elements, newSpecifier, sortKind === SortKind.CaseInsensitive); + const comparer = OrganizeImports.getOrganizeImportsComparer(preferences, sortKind === SortKind.CaseInsensitive); + const insertionIndex = OrganizeImports.getImportSpecifierInsertionIndex(aliasDeclaration.parent.elements, newSpecifier, comparer); changes.insertImportSpecifierAtIndex(sourceFile, newSpecifier, aliasDeclaration.parent, insertionIndex); } else { @@ -1279,7 +1280,7 @@ function promoteFromTypeOnly(changes: textChanges.ChangeTracker, aliasDeclaratio if (convertExistingToTypeOnly) { const namedImports = tryCast(importClause.namedBindings, isNamedImports); if (namedImports && namedImports.elements.length > 1) { - if (OrganizeImports.detectImportSpecifierSorting(namedImports.elements) && + if (OrganizeImports.detectImportSpecifierSorting(namedImports.elements, preferences) && aliasDeclaration.kind === SyntaxKind.ImportSpecifier && namedImports.elements.indexOf(aliasDeclaration) !== 0 ) { @@ -1342,13 +1343,13 @@ function doAddExistingFix( ignoreCaseForSorting = preferences.organizeImportsIgnoreCase; } else if (existingSpecifiers) { - const targetImportSorting = OrganizeImports.detectImportSpecifierSorting(existingSpecifiers); + const targetImportSorting = OrganizeImports.detectImportSpecifierSorting(existingSpecifiers, preferences); if (targetImportSorting !== SortKind.Both) { ignoreCaseForSorting = targetImportSorting === SortKind.CaseInsensitive; } } if (ignoreCaseForSorting === undefined) { - ignoreCaseForSorting = OrganizeImports.detectSorting(sourceFile) === SortKind.CaseInsensitive; + ignoreCaseForSorting = OrganizeImports.detectSorting(sourceFile, preferences) === SortKind.CaseInsensitive; } const newSpecifiers = stableSort( @@ -1363,15 +1364,16 @@ function doAddExistingFix( // 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); + const specifierSort = existingSpecifiers?.length && OrganizeImports.detectImportSpecifierSorting(existingSpecifiers, preferences); if (specifierSort && !(ignoreCaseForSorting && specifierSort === SortKind.CaseSensitive)) { + const comparer = OrganizeImports.getOrganizeImportsComparer(preferences, /*ignoreCase*/ false); 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 = convertExistingToTypeOnly && !spec.isTypeOnly ? 0 - : OrganizeImports.getImportSpecifierInsertionIndex(existingSpecifiers, spec, ignoreCaseForSorting); + : OrganizeImports.getImportSpecifierInsertionIndex(existingSpecifiers, spec, comparer); changes.insertImportSpecifierAtIndex(sourceFile, spec, clause.namedBindings as NamedImports, insertionIndex); } } diff --git a/src/services/organizeImports.ts b/src/services/organizeImports.ts index 29eca95e6c6a1..a0db50a871d92 100644 --- a/src/services/organizeImports.ts +++ b/src/services/organizeImports.ts @@ -45,7 +45,8 @@ import { LanguageServiceHost, length, map, - memoizeWeak, + MemoizeCache, + memoizeCached, NamedImportBindings, NamedImports, NamespaceImport, @@ -89,7 +90,7 @@ export function organizeImports( // All of the old ImportDeclarations in the file, in syntactic order. const topLevelImportGroupDecls = groupImportsByNewlineContiguous(sourceFile, sourceFile.statements.filter(isImportDeclaration)); - const comparer = getOrganizeImportsComparer(preferences, shouldSort ? () => detectSortingWorker(topLevelImportGroupDecls) === SortKind.CaseInsensitive : undefined); + const comparer = getOrganizeImportsComparerWithDetection(preferences, shouldSort ? () => detectSortingWorker(topLevelImportGroupDecls, preferences) === SortKind.CaseInsensitive : undefined); const processImportsOfSameModuleSpecifier = (importGroup: readonly ImportDeclaration[]) => { if (shouldRemove) importGroup = removeUnusedImports(importGroup, sourceFile, program); @@ -613,17 +614,18 @@ function getModuleSpecifierExpression(declaration: AnyImportOrRequireStatement): } /** @internal */ -export function detectSorting(sourceFile: SourceFile): SortKind { +export function detectSorting(sourceFile: SourceFile, preferences: UserPreferences): SortKind { return detectSortingWorker( - groupImportsByNewlineContiguous(sourceFile, sourceFile.statements.filter(isImportDeclaration))); + groupImportsByNewlineContiguous(sourceFile, sourceFile.statements.filter(isImportDeclaration)), + preferences); } -function detectSortingWorker(importGroups: ImportDeclaration[][]): SortKind { +function detectSortingWorker(importGroups: ImportDeclaration[][], preferences: UserPreferences): SortKind { let sortState = SortKind.Both; for (const importGroup of importGroups) { // Check module specifiers if (importGroup.length > 1) { - sortState &= detectSortCaseSensitivity(importGroup, /*useEslintOrdering*/ true, i => tryCast(i.moduleSpecifier, isStringLiteral)?.text ?? ""); + sortState &= detectSortCaseSensitivity(importGroup, i => tryCast(i.moduleSpecifier, isStringLiteral)?.text ?? "", compareStringsCaseSensitive, compareStringsCaseInsensitiveEslintCompatible); if (!sortState) { return sortState; } @@ -634,7 +636,7 @@ function detectSortingWorker(importGroups: ImportDeclaration[][]): SortKind { importGroup, i => tryCast(i.importClause?.namedBindings, isNamedImports)?.elements.length! > 1); if (declarationWithNamedImports) { - sortState &= detectImportSpecifierSorting((declarationWithNamedImports.importClause!.namedBindings as NamedImports).elements); + sortState &= detectImportSpecifierSorting((declarationWithNamedImports.importClause!.namedBindings as NamedImports).elements, preferences); if (!sortState) { return sortState; } @@ -652,16 +654,47 @@ function detectSortingWorker(importGroups: ImportDeclaration[][]): SortKind { /** @internal */ export function detectImportDeclarationSorting(imports: readonly AnyImportOrRequireStatement[]): SortKind { - return detectSortCaseSensitivity(imports, /*useEslintOrdering*/ true, s => getExternalModuleName(getModuleSpecifierExpression(s)) || ""); + return detectSortCaseSensitivity( + imports, + s => getExternalModuleName(getModuleSpecifierExpression(s)) || "", + compareStringsCaseInsensitiveEslintCompatible, + compareStringsCaseSensitive + ); +} + +class ImportSpecifierSortingCache implements MemoizeCache<[readonly ImportSpecifier[], UserPreferences], SortKind> { + private _lastPreferences: UserPreferences | undefined; + private _cache: WeakMap | undefined; + + has([specifiers, preferences]: [readonly ImportSpecifier[], UserPreferences]) { + if (this._lastPreferences !== preferences || !this._cache) return false; + return this._cache.has(specifiers); + } + + get([specifiers, preferences]: [readonly ImportSpecifier[], UserPreferences]) { + if (this._lastPreferences !== preferences || !this._cache) return undefined; + return this._cache.get(specifiers); + } + + set([specifiers, preferences]: [readonly ImportSpecifier[], UserPreferences], value: SortKind) { + if (this._lastPreferences !== preferences) { + this._lastPreferences = preferences; + this._cache = undefined; + } + this._cache ??= new WeakMap(); + this._cache.set(specifiers, value); + } } /** @internal */ -export const detectImportSpecifierSorting = memoizeWeak((specifiers: readonly ImportSpecifier[]): SortKind => { +export const detectImportSpecifierSorting = memoizeCached((specifiers: readonly ImportSpecifier[], preferences: UserPreferences): SortKind => { if (!arrayIsSorted(specifiers, (s1, s2) => compareBooleans(s1.isTypeOnly, s2.isTypeOnly))) { return SortKind.None; } - return detectSortCaseSensitivity(specifiers, /*useEslintOrdering*/ true, specifier => specifier.name.text); -}); + const collateCaseSensitive = getOrganizeImportsComparer(preferences, /*ignoreCase*/ false); + const collateCaseInsensitive = getOrganizeImportsComparer(preferences, /*ignoreCase*/ true); + return detectSortCaseSensitivity(specifiers, specifier => specifier.name.text, collateCaseSensitive, collateCaseInsensitive); +}, new ImportSpecifierSortingCache()); /** @internal */ export function getImportDeclarationInsertionIndex(sortedImports: readonly AnyImportOrRequireStatement[], newImport: AnyImportOrRequireStatement, ignoreCase?: boolean) { @@ -670,8 +703,8 @@ export function getImportDeclarationInsertionIndex(sortedImports: readonly AnyIm } /** @internal */ -export function getImportSpecifierInsertionIndex(sortedImports: readonly ImportSpecifier[], newImport: ImportSpecifier, ignoreCase?: boolean) { - const index = binarySearch(sortedImports, newImport, identity, (s1, s2) => compareImportOrExportSpecifiers(s1, s2, ignoreCase)); +export function getImportSpecifierInsertionIndex(sortedImports: readonly ImportSpecifier[], newImport: ImportSpecifier, comparer: Comparer) { + const index = binarySearch(sortedImports, newImport, identity, (s1, s2) => compareImportOrExportSpecifiersWorker(s1, s2, comparer)); return index < 0 ? ~index : index; } @@ -758,10 +791,14 @@ function getOrganizeImportsLocale(preferences: UserPreferences): string { } /** @internal */ -export function getOrganizeImportsComparer(preferences: UserPreferences, detectIgnoreCase?: () => boolean): Comparer { +export function getOrganizeImportsComparer(preferences: UserPreferences, ignoreCase: boolean): Comparer { const collation = preferences.organizeImportsCollation ?? "ordinal"; - const ignoreCase = typeof preferences.organizeImportsIgnoreCase === "boolean" ? preferences.organizeImportsIgnoreCase : detectIgnoreCase?.() ?? false; return collation === "ordinal" ? getOrganizeImportsOrdinalStringComparer(ignoreCase) : getOrganizeImportsNaturalStringComparer(ignoreCase, preferences); -} \ No newline at end of file +} + +function getOrganizeImportsComparerWithDetection(preferences: UserPreferences, detectIgnoreCase?: () => boolean): Comparer { + const ignoreCase = typeof preferences.organizeImportsIgnoreCase === "boolean" ? preferences.organizeImportsIgnoreCase : detectIgnoreCase?.() ?? false; + return getOrganizeImportsComparer(preferences, ignoreCase); +} diff --git a/tests/cases/fourslash/importNameCodeFix_order2.ts b/tests/cases/fourslash/importNameCodeFix_order2.ts new file mode 100644 index 0000000000000..ae9c6bc4fc473 --- /dev/null +++ b/tests/cases/fourslash/importNameCodeFix_order2.ts @@ -0,0 +1,53 @@ +/// + +// @Filename: /a.ts +////export const _aB: number; +////export const _Ab: number; +////export const aB: number; +////export const Ab: number; + +// @Filename: /b.ts +////[|import { +//// _aB, +//// _Ab, +//// Ab, +////} from "./a"; +////aB;|] + +// @Filename: /c.ts +////[|import { +//// _aB, +//// _Ab, +//// Ab, +////} from "./a"; +////aB;|] + +// the import in 'b.ts' isn't sorted per ordinal comparison, so the import is added to the end of the list +goTo.file("/b.ts"); +verify.importFixAtPosition([ +`import { + _aB, + _Ab, + Ab, + aB, +} from "./a"; +aB;`, +], undefined, { + organizeImportsIgnoreCase: false, + organizeImportsCollation: "ordinal" +}); + +// the import in 'c.ts' *is* sorted per natural collation, so the import is added before `Ab` +goTo.file("/c.ts"); +verify.importFixAtPosition([ +`import { + _aB, + _Ab, + aB, + Ab, +} from "./a"; +aB;`, +], undefined, { + organizeImportsIgnoreCase: false, + organizeImportsCollation: "natural" +}); From b083508ea0bdf66b970ecc6665065f320cbfe662 Mon Sep 17 00:00:00 2001 From: Ron Buckton Date: Wed, 4 Jan 2023 22:35:39 -0500 Subject: [PATCH 03/10] Fix failing tests, update baselines --- src/services/codefixes/importFixes.ts | 2 +- src/services/organizeImports.ts | 25 ++++----- .../reference/api/tsserverlibrary.d.ts | 55 +++++++++++++++++++ tests/baselines/reference/api/typescript.d.ts | 5 ++ 4 files changed, 73 insertions(+), 14 deletions(-) diff --git a/src/services/codefixes/importFixes.ts b/src/services/codefixes/importFixes.ts index 8ce871555f866..55f456f3a79f2 100644 --- a/src/services/codefixes/importFixes.ts +++ b/src/services/codefixes/importFixes.ts @@ -1366,7 +1366,7 @@ function doAddExistingFix( // to do a sorted insertion. const specifierSort = existingSpecifiers?.length && OrganizeImports.detectImportSpecifierSorting(existingSpecifiers, preferences); if (specifierSort && !(ignoreCaseForSorting && specifierSort === SortKind.CaseSensitive)) { - const comparer = OrganizeImports.getOrganizeImportsComparer(preferences, /*ignoreCase*/ false); + const comparer = OrganizeImports.getOrganizeImportsComparer(preferences, ignoreCaseForSorting); 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 diff --git a/src/services/organizeImports.ts b/src/services/organizeImports.ts index a0db50a871d92..041a6164d55be 100644 --- a/src/services/organizeImports.ts +++ b/src/services/organizeImports.ts @@ -19,6 +19,7 @@ import { FileTextChanges, find, FindAllReferences, + firstOrUndefined, flatMap, formatting, getNewLineOrDefaultFromHost, @@ -349,7 +350,10 @@ function coalesceImportsWorker(importGroup: readonly ImportDeclaration[], compar updateImportDeclarationAndClause(namespaceImport, /*name*/ undefined, namespaceImport.importClause.namedBindings)); } - if (defaultImports.length === 0 && namedImports.length === 0) { + const firstDefaultImport = firstOrUndefined(defaultImports); + const firstNamedImport = firstOrUndefined(namedImports); + const importDecl = firstDefaultImport ?? firstNamedImport; + if (!importDecl) { continue; } @@ -369,26 +373,21 @@ function coalesceImportsWorker(importGroup: readonly ImportDeclaration[], compar const sortedImportSpecifiers = factory.createNodeArray( sortSpecifiers(newImportSpecifiers, comparer), - namedImports[0].importClause.namedBindings.elements.hasTrailingComma + firstNamedImport?.importClause.namedBindings.elements.hasTrailingComma ); - const importDecl = defaultImports.length > 0 - ? defaultImports[0] - : namedImports[0]; - const newNamedImports = sortedImportSpecifiers.length === 0 ? newDefaultImport ? undefined : factory.createNamedImports(emptyArray) - : namedImports.length === 0 - ? factory.createNamedImports(sortedImportSpecifiers) - : factory.updateNamedImports(namedImports[0].importClause.namedBindings, sortedImportSpecifiers); + : firstNamedImport + ? factory.updateNamedImports(firstNamedImport.importClause.namedBindings, sortedImportSpecifiers) + : factory.createNamedImports(sortedImportSpecifiers); if (sourceFile && newNamedImports && - namedImports.length && - namedImports[0].importClause.namedBindings && - !rangeIsOnSingleLine(namedImports[0].importClause.namedBindings, sourceFile) + firstNamedImport?.importClause.namedBindings && + !rangeIsOnSingleLine(firstNamedImport.importClause.namedBindings, sourceFile) ) { setEmitFlags(newNamedImports, EmitFlags.MultiLine); } @@ -400,7 +399,7 @@ function coalesceImportsWorker(importGroup: readonly ImportDeclaration[], compar coalescedImports.push( updateImportDeclarationAndClause(importDecl, newDefaultImport, /*namedBindings*/ undefined)); coalescedImports.push( - updateImportDeclarationAndClause(namedImports[0] ?? importDecl, /*name*/ undefined, newNamedImports)); + updateImportDeclarationAndClause(firstNamedImport ?? importDecl, /*name*/ undefined, newNamedImports)); } else { coalescedImports.push( diff --git a/tests/baselines/reference/api/tsserverlibrary.d.ts b/tests/baselines/reference/api/tsserverlibrary.d.ts index 6f6e0a8fb4b32..3de836506f79d 100644 --- a/tests/baselines/reference/api/tsserverlibrary.d.ts +++ b/tests/baselines/reference/api/tsserverlibrary.d.ts @@ -2802,7 +2802,57 @@ declare namespace ts { readonly includeInlayFunctionLikeReturnTypeHints?: boolean; readonly includeInlayEnumMemberValueHints?: boolean; readonly autoImportFileExcludePatterns?: string[]; + /** + * Indicates whether imports should be organized in a case-insensitive manner. + */ readonly organizeImportsIgnoreCase?: "auto" | boolean; + /** + * Indicates whether imports should be organized via an "ordinal" (binary) comparison using the numeric value + * of their code points, or via "natural" collation using rules associated with the locale specified by + * {@link organizeImportsCollationLocale}. + * + * NOTE: When comparing paths `"ordinal"` collation is always used. + * + * Default: `"ordinal"`. + */ + readonly organizeImportsCollation?: "natural" | "ordinal"; + /** + * Indicates the locale to use for "natural" collation. If not specified, the locale `"en"` is used as an invariant + * for the sake of consistent sorting. Use `"auto"` to use the detected UI locale. + * + * This preference is ignored if {@link organizeImportsCollation} is not `"natural"`. + * + * Default: `"en"` + */ + readonly organizeImportsCollationLocale?: string; + /** + * Indicates whether numeric collation should be used for digit sequences in strings. When `true`, will collate + * strings such that `a1z < a2z < a100z`. When `false`, will collate strings such that `a1z < a100z < a2z`. + * + * This preference is ignored if {@link organizeImportsCollation} is not `"natural"`. + * + * Default: `true` + */ + readonly organizeImportsNumericCollation?: boolean; + /** + * Indicates whether accents and other diacritic marks are considered unequal for the purpose of collation. When + * `true`, characters with accents and other diacritics will be collated in the order defined by the locale specified + * in {@link organizeImportsCollationLocale}. + * + * This preference is ignored if {@link organizeImportsCollation} is not `"natural"`. + * + * Default: `true` + */ + readonly organizeImportsAccentCollation?: boolean; + /** + * Indicates whether upper case or lower case should sort first. When `false`, the default order for the locale + * specified in {@link organizeImportsCollationLocale} is used. + * + * This preference is ignored if {@link organizeImportsCollation} is not `"natural"`. + * + * Default: `false` + */ + readonly organizeImportsCaseFirst?: "upper" | "lower" | false; /** * Indicates whether {@link ReferencesResponseItem.lineText} is supported. */ @@ -8404,6 +8454,11 @@ declare namespace ts { readonly allowRenameOfImportPath?: boolean; readonly autoImportFileExcludePatterns?: string[]; readonly organizeImportsIgnoreCase?: "auto" | boolean; + readonly organizeImportsCollation?: "natural" | "ordinal"; + readonly organizeImportsLocale?: string; + readonly organizeImportsNumericCollation?: boolean; + readonly organizeImportsAccentCollation?: boolean; + readonly organizeImportsCaseFirst?: "upper" | "lower" | false; } /** Represents a bigint literal value without requiring bigint support */ interface PseudoBigInt { diff --git a/tests/baselines/reference/api/typescript.d.ts b/tests/baselines/reference/api/typescript.d.ts index 7c3088b485c5d..ce249c7c66fd5 100644 --- a/tests/baselines/reference/api/typescript.d.ts +++ b/tests/baselines/reference/api/typescript.d.ts @@ -4469,6 +4469,11 @@ declare namespace ts { readonly allowRenameOfImportPath?: boolean; readonly autoImportFileExcludePatterns?: string[]; readonly organizeImportsIgnoreCase?: "auto" | boolean; + readonly organizeImportsCollation?: "natural" | "ordinal"; + readonly organizeImportsLocale?: string; + readonly organizeImportsNumericCollation?: boolean; + readonly organizeImportsAccentCollation?: boolean; + readonly organizeImportsCaseFirst?: "upper" | "lower" | false; } /** Represents a bigint literal value without requiring bigint support */ interface PseudoBigInt { From d022b95cfa260aaa3f7a15379b3f95205a037cd1 Mon Sep 17 00:00:00 2001 From: Ron Buckton Date: Wed, 4 Jan 2023 23:07:38 -0500 Subject: [PATCH 04/10] Use 'unicode' instead of 'natural' for 'organizeImportsCollation' --- src/compiler/types.ts | 2 +- src/server/protocol.ts | 7 ++++--- src/services/organizeImports.ts | 8 ++++---- tests/baselines/reference/api/tsserverlibrary.d.ts | 9 +++++---- tests/baselines/reference/api/typescript.d.ts | 2 +- tests/cases/fourslash/fourslash.ts | 2 +- tests/cases/fourslash/importNameCodeFix_order2.ts | 2 +- ...nizeImportsNatural1.ts => organizeImportsUnicode1.ts} | 2 +- ...nizeImportsNatural2.ts => organizeImportsUnicode2.ts} | 4 ++-- ...nizeImportsNatural3.ts => organizeImportsUnicode3.ts} | 4 ++-- ...nizeImportsNatural4.ts => organizeImportsUnicode4.ts} | 4 ++-- 11 files changed, 24 insertions(+), 22 deletions(-) rename tests/cases/fourslash/{organizeImportsNatural1.ts => organizeImportsUnicode1.ts} (93%) rename tests/cases/fourslash/{organizeImportsNatural2.ts => organizeImportsUnicode2.ts} (87%) rename tests/cases/fourslash/{organizeImportsNatural3.ts => organizeImportsUnicode3.ts} (87%) rename tests/cases/fourslash/{organizeImportsNatural4.ts => organizeImportsUnicode4.ts} (88%) diff --git a/src/compiler/types.ts b/src/compiler/types.ts index 6e78b9af54dba..47c8380d9f6be 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -9771,7 +9771,7 @@ export interface UserPreferences { readonly allowRenameOfImportPath?: boolean; readonly autoImportFileExcludePatterns?: string[]; readonly organizeImportsIgnoreCase?: "auto" | boolean; - readonly organizeImportsCollation?: "natural" | "ordinal"; + readonly organizeImportsCollation?: "ordinal" | "unicode"; readonly organizeImportsLocale?: string; readonly organizeImportsNumericCollation?: boolean; readonly organizeImportsAccentCollation?: boolean; diff --git a/src/server/protocol.ts b/src/server/protocol.ts index 0d3a1c2831c07..da0006702c49b 100644 --- a/src/server/protocol.ts +++ b/src/server/protocol.ts @@ -3523,14 +3523,15 @@ export interface UserPreferences { readonly organizeImportsIgnoreCase?: "auto" | boolean; /** * Indicates whether imports should be organized via an "ordinal" (binary) comparison using the numeric value - * of their code points, or via "natural" collation using rules associated with the locale specified by - * {@link organizeImportsCollationLocale}. + * of their code points, or via "unicode" collation (via the + * [Unicode Collation Algorithm](https://unicode.org/reports/tr10/#Scope)) using rules associated with the locale + * specified in {@link organizeImportsCollationLocale}. * * NOTE: When comparing paths `"ordinal"` collation is always used. * * Default: `"ordinal"`. */ - readonly organizeImportsCollation?: "natural" | "ordinal"; + readonly organizeImportsCollation?: "ordinal" | "unicode"; /** * Indicates the locale to use for "natural" collation. If not specified, the locale `"en"` is used as an invariant * for the sake of consistent sorting. Use `"auto"` to use the detected UI locale. diff --git a/src/services/organizeImports.ts b/src/services/organizeImports.ts index 041a6164d55be..11bd37ffac977 100644 --- a/src/services/organizeImports.ts +++ b/src/services/organizeImports.ts @@ -758,7 +758,7 @@ function getOrganizeImportsOrdinalStringComparer(ignoreCase: boolean) { return ignoreCase ? compareStringsCaseInsensitiveEslintCompatible : compareStringsCaseSensitive; } -function getOrganizeImportsNaturalStringComparer(ignoreCase: boolean, preferences: UserPreferences): Comparer { +function getOrganizeImportsUnicodeStringComparer(ignoreCase: boolean, preferences: UserPreferences): Comparer { const resolvedLocale = getOrganizeImportsLocale(preferences); const caseFirst = preferences.organizeImportsCaseFirst ?? false; const numeric = preferences.organizeImportsNumericCollation ?? false; @@ -792,9 +792,9 @@ function getOrganizeImportsLocale(preferences: UserPreferences): string { /** @internal */ export function getOrganizeImportsComparer(preferences: UserPreferences, ignoreCase: boolean): Comparer { const collation = preferences.organizeImportsCollation ?? "ordinal"; - return collation === "ordinal" ? - getOrganizeImportsOrdinalStringComparer(ignoreCase) : - getOrganizeImportsNaturalStringComparer(ignoreCase, preferences); + return collation === "unicode" ? + getOrganizeImportsUnicodeStringComparer(ignoreCase, preferences) : + getOrganizeImportsOrdinalStringComparer(ignoreCase); } function getOrganizeImportsComparerWithDetection(preferences: UserPreferences, detectIgnoreCase?: () => boolean): Comparer { diff --git a/tests/baselines/reference/api/tsserverlibrary.d.ts b/tests/baselines/reference/api/tsserverlibrary.d.ts index 3de836506f79d..fe2d68577c3f0 100644 --- a/tests/baselines/reference/api/tsserverlibrary.d.ts +++ b/tests/baselines/reference/api/tsserverlibrary.d.ts @@ -2808,14 +2808,15 @@ declare namespace ts { readonly organizeImportsIgnoreCase?: "auto" | boolean; /** * Indicates whether imports should be organized via an "ordinal" (binary) comparison using the numeric value - * of their code points, or via "natural" collation using rules associated with the locale specified by - * {@link organizeImportsCollationLocale}. + * of their code points, or via "unicode" collation (via the + * [Unicode Collation Algorithm](https://unicode.org/reports/tr10/#Scope)) using rules associated with the locale + * specified in {@link organizeImportsCollationLocale}. * * NOTE: When comparing paths `"ordinal"` collation is always used. * * Default: `"ordinal"`. */ - readonly organizeImportsCollation?: "natural" | "ordinal"; + readonly organizeImportsCollation?: "ordinal" | "unicode"; /** * Indicates the locale to use for "natural" collation. If not specified, the locale `"en"` is used as an invariant * for the sake of consistent sorting. Use `"auto"` to use the detected UI locale. @@ -8454,7 +8455,7 @@ declare namespace ts { readonly allowRenameOfImportPath?: boolean; readonly autoImportFileExcludePatterns?: string[]; readonly organizeImportsIgnoreCase?: "auto" | boolean; - readonly organizeImportsCollation?: "natural" | "ordinal"; + readonly organizeImportsCollation?: "ordinal" | "unicode"; readonly organizeImportsLocale?: string; readonly organizeImportsNumericCollation?: boolean; readonly organizeImportsAccentCollation?: boolean; diff --git a/tests/baselines/reference/api/typescript.d.ts b/tests/baselines/reference/api/typescript.d.ts index ce249c7c66fd5..d81eb53e20ed3 100644 --- a/tests/baselines/reference/api/typescript.d.ts +++ b/tests/baselines/reference/api/typescript.d.ts @@ -4469,7 +4469,7 @@ declare namespace ts { readonly allowRenameOfImportPath?: boolean; readonly autoImportFileExcludePatterns?: string[]; readonly organizeImportsIgnoreCase?: "auto" | boolean; - readonly organizeImportsCollation?: "natural" | "ordinal"; + readonly organizeImportsCollation?: "ordinal" | "unicode"; readonly organizeImportsLocale?: string; readonly organizeImportsNumericCollation?: boolean; readonly organizeImportsAccentCollation?: boolean; diff --git a/tests/cases/fourslash/fourslash.ts b/tests/cases/fourslash/fourslash.ts index 0707dc892bebc..d07fdf2526e61 100644 --- a/tests/cases/fourslash/fourslash.ts +++ b/tests/cases/fourslash/fourslash.ts @@ -673,7 +673,7 @@ declare namespace FourSlashInterface { readonly allowRenameOfImportPath?: boolean; readonly autoImportFileExcludePatterns?: readonly string[]; readonly organizeImportsIgnoreCase?: "auto" | boolean; - readonly organizeImportsCollation?: "natural" | "ordinal"; + readonly organizeImportsCollation?: "unicode" | "ordinal"; readonly organizeImportsLocale?: string; readonly organizeImportsNumericCollation?: boolean; readonly organizeImportsAccentCollation?: boolean; diff --git a/tests/cases/fourslash/importNameCodeFix_order2.ts b/tests/cases/fourslash/importNameCodeFix_order2.ts index ae9c6bc4fc473..c5fcbe57a168f 100644 --- a/tests/cases/fourslash/importNameCodeFix_order2.ts +++ b/tests/cases/fourslash/importNameCodeFix_order2.ts @@ -49,5 +49,5 @@ verify.importFixAtPosition([ aB;`, ], undefined, { organizeImportsIgnoreCase: false, - organizeImportsCollation: "natural" + organizeImportsCollation: "unicode" }); diff --git a/tests/cases/fourslash/organizeImportsNatural1.ts b/tests/cases/fourslash/organizeImportsUnicode1.ts similarity index 93% rename from tests/cases/fourslash/organizeImportsNatural1.ts rename to tests/cases/fourslash/organizeImportsUnicode1.ts index 7ce4df267e983..c673180a638c0 100644 --- a/tests/cases/fourslash/organizeImportsNatural1.ts +++ b/tests/cases/fourslash/organizeImportsUnicode1.ts @@ -32,5 +32,5 @@ verify.organizeImports( console.log(_aB, _Ab, aB, Ab);`, /*mode*/ undefined, { organizeImportsIgnoreCase: false, - organizeImportsCollation: "natural", + organizeImportsCollation: "unicode", }); diff --git a/tests/cases/fourslash/organizeImportsNatural2.ts b/tests/cases/fourslash/organizeImportsUnicode2.ts similarity index 87% rename from tests/cases/fourslash/organizeImportsNatural2.ts rename to tests/cases/fourslash/organizeImportsUnicode2.ts index 524eb6b06eed2..338f3440f6984 100644 --- a/tests/cases/fourslash/organizeImportsNatural2.ts +++ b/tests/cases/fourslash/organizeImportsUnicode2.ts @@ -17,7 +17,7 @@ verify.organizeImports( console.log(a1, a2, a100);`, /*mode*/ undefined, { organizeImportsIgnoreCase: false, - organizeImportsCollation: "natural", + organizeImportsCollation: "unicode", organizeImportsNumericCollation: false, }); @@ -30,6 +30,6 @@ verify.organizeImports( console.log(a1, a2, a100);`, /*mode*/ undefined, { organizeImportsIgnoreCase: false, - organizeImportsCollation: "natural", + organizeImportsCollation: "unicode", organizeImportsNumericCollation: true, }); diff --git a/tests/cases/fourslash/organizeImportsNatural3.ts b/tests/cases/fourslash/organizeImportsUnicode3.ts similarity index 87% rename from tests/cases/fourslash/organizeImportsNatural3.ts rename to tests/cases/fourslash/organizeImportsUnicode3.ts index 8bcb8f54a831e..f1b6e28b849d9 100644 --- a/tests/cases/fourslash/organizeImportsNatural3.ts +++ b/tests/cases/fourslash/organizeImportsUnicode3.ts @@ -17,7 +17,7 @@ verify.organizeImports( console.log(A, À, B);`, /*mode*/ undefined, { organizeImportsIgnoreCase: false, - organizeImportsCollation: "natural", + organizeImportsCollation: "unicode", organizeImportsAccentCollation: false, }); @@ -30,6 +30,6 @@ verify.organizeImports( console.log(A, À, B);`, /*mode*/ undefined, { organizeImportsIgnoreCase: false, - organizeImportsCollation: "natural", + organizeImportsCollation: "unicode", organizeImportsAccentCollation: true, }); diff --git a/tests/cases/fourslash/organizeImportsNatural4.ts b/tests/cases/fourslash/organizeImportsUnicode4.ts similarity index 88% rename from tests/cases/fourslash/organizeImportsNatural4.ts rename to tests/cases/fourslash/organizeImportsUnicode4.ts index f154e45e2b398..5eeb2d2daa05d 100644 --- a/tests/cases/fourslash/organizeImportsNatural4.ts +++ b/tests/cases/fourslash/organizeImportsUnicode4.ts @@ -19,7 +19,7 @@ verify.organizeImports( console.log(_aB, _Ab, aB, Ab);`, /*mode*/ undefined, { organizeImportsIgnoreCase: false, - organizeImportsCollation: "natural", + organizeImportsCollation: "unicode", organizeImportsCaseFirst: "upper", }); @@ -33,6 +33,6 @@ verify.organizeImports( console.log(_aB, _Ab, aB, Ab);`, /*mode*/ undefined, { organizeImportsIgnoreCase: false, - organizeImportsCollation: "natural", + organizeImportsCollation: "unicode", organizeImportsCaseFirst: "lower", }); From 7244b3f5a78b5e28e881df9767a4475074580e6f Mon Sep 17 00:00:00 2001 From: Ron Buckton Date: Wed, 4 Jan 2023 23:11:33 -0500 Subject: [PATCH 05/10] Address lint errors --- src/server/protocol.ts | 2 +- src/services/organizeImports.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/server/protocol.ts b/src/server/protocol.ts index da0006702c49b..5bd17a22f953c 100644 --- a/src/server/protocol.ts +++ b/src/server/protocol.ts @@ -3523,7 +3523,7 @@ export interface UserPreferences { readonly organizeImportsIgnoreCase?: "auto" | boolean; /** * Indicates whether imports should be organized via an "ordinal" (binary) comparison using the numeric value - * of their code points, or via "unicode" collation (via the + * of their code points, or via "unicode" collation (via the * [Unicode Collation Algorithm](https://unicode.org/reports/tr10/#Scope)) using rules associated with the locale * specified in {@link organizeImportsCollationLocale}. * diff --git a/src/services/organizeImports.ts b/src/services/organizeImports.ts index 11bd37ffac977..d8d90545e8058 100644 --- a/src/services/organizeImports.ts +++ b/src/services/organizeImports.ts @@ -579,7 +579,7 @@ function sortSpecifiers(specifiers: readonly /** @internal */ export function compareImportOrExportSpecifiers(s1: T, s2: T, ignoreCase?: boolean): Comparison { - return compareImportOrExportSpecifiersWorker(s1, s2, getOrganizeImportsOrdinalStringComparer(!!ignoreCase)) + return compareImportOrExportSpecifiersWorker(s1, s2, getOrganizeImportsOrdinalStringComparer(!!ignoreCase)); } function compareImportOrExportSpecifiersWorker(s1: T, s2: T, comparer: Comparer): Comparison { From 72678a3dde6be15abe5c583b6e769dceac2073e0 Mon Sep 17 00:00:00 2001 From: Ron Buckton Date: Wed, 18 Jan 2023 16:32:26 -0500 Subject: [PATCH 06/10] Apply collation to paths as well --- src/services/codefixes/importFixes.ts | 8 +-- src/services/organizeImports.ts | 53 +++++++++++-------- src/services/refactors/moveToNewFile.ts | 2 +- src/services/utilities.ts | 12 +++-- .../fourslash/organizeImportsPathsUnicode1.ts | 30 +++++++++++ .../fourslash/organizeImportsPathsUnicode2.ts | 29 ++++++++++ .../fourslash/organizeImportsPathsUnicode3.ts | 29 ++++++++++ .../fourslash/organizeImportsPathsUnicode4.ts | 32 +++++++++++ 8 files changed, 165 insertions(+), 30 deletions(-) create mode 100644 tests/cases/fourslash/organizeImportsPathsUnicode1.ts create mode 100644 tests/cases/fourslash/organizeImportsPathsUnicode2.ts create mode 100644 tests/cases/fourslash/organizeImportsPathsUnicode3.ts create mode 100644 tests/cases/fourslash/organizeImportsPathsUnicode4.ts diff --git a/src/services/codefixes/importFixes.ts b/src/services/codefixes/importFixes.ts index 55f456f3a79f2..2164dc200ede8 100644 --- a/src/services/codefixes/importFixes.ts +++ b/src/services/codefixes/importFixes.ts @@ -375,7 +375,7 @@ function createImportAdderWorker(sourceFile: SourceFile, program: Program, useAu newDeclarations = combine(newDeclarations, declarations); }); if (newDeclarations) { - insertImports(changeTracker, sourceFile, newDeclarations, /*blankLineBetween*/ true); + insertImports(changeTracker, sourceFile, newDeclarations, /*blankLineBetween*/ true, preferences); } } @@ -1215,7 +1215,7 @@ function codeActionForFixWorker(changes: textChanges.ChangeTracker, sourceFile: const defaultImport: Import | undefined = importKind === ImportKind.Default ? { name: symbolName, addAsTypeOnly } : undefined; const namedImports: Import[] | undefined = importKind === ImportKind.Named ? [{ name: symbolName, addAsTypeOnly }] : undefined; const namespaceLikeImport = importKind === ImportKind.Namespace || importKind === ImportKind.CommonJS ? { importKind, name: symbolName, addAsTypeOnly } : undefined; - insertImports(changes, sourceFile, getDeclarations(moduleSpecifier, quotePreference, defaultImport, namedImports, namespaceLikeImport), /*blankLineBetween*/ true); + insertImports(changes, sourceFile, getDeclarations(moduleSpecifier, quotePreference, defaultImport, namedImports, namespaceLikeImport), /*blankLineBetween*/ true, preferences); return includeSymbolNameInDescription ? [Diagnostics.Import_0_from_1, symbolName, moduleSpecifier] : [Diagnostics.Add_import_from_0, moduleSpecifier]; @@ -1352,12 +1352,13 @@ function doAddExistingFix( ignoreCaseForSorting = OrganizeImports.detectSorting(sourceFile, preferences) === SortKind.CaseInsensitive; } + const comparer = OrganizeImports.getOrganizeImportsComparer(preferences, ignoreCaseForSorting); const newSpecifiers = stableSort( namedImports.map(namedImport => factory.createImportSpecifier( (!clause.isTypeOnly || promoteFromTypeOnly) && needsTypeOnly(namedImport), /*propertyName*/ undefined, factory.createIdentifier(namedImport.name))), - (s1, s2) => OrganizeImports.compareImportOrExportSpecifiers(s1, s2, ignoreCaseForSorting)); + (s1, s2) => OrganizeImports.compareImportOrExportSpecifiers(s1, s2, comparer)); // 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 @@ -1366,7 +1367,6 @@ function doAddExistingFix( // to do a sorted insertion. const specifierSort = existingSpecifiers?.length && OrganizeImports.detectImportSpecifierSorting(existingSpecifiers, preferences); if (specifierSort && !(ignoreCaseForSorting && specifierSort === SortKind.CaseSensitive)) { - const comparer = OrganizeImports.getOrganizeImportsComparer(preferences, ignoreCaseForSorting); 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 diff --git a/src/services/organizeImports.ts b/src/services/organizeImports.ts index d8d90545e8058..a090e64823240 100644 --- a/src/services/organizeImports.ts +++ b/src/services/organizeImports.ts @@ -96,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 (shouldSort) importGroup = stableSort(importGroup, (s1, s2) => compareImportsOrRequireStatements(s1, s2)); + if (shouldSort) importGroup = stableSort(importGroup, (s1, s2) => compareImportsOrRequireStatements(s1, s2, comparer)); return importGroup; }; @@ -143,7 +143,7 @@ export function organizeImports( ? group(oldImportDecls, importDecl => getExternalModuleName(importDecl.moduleSpecifier)!) : [oldImportDecls]; const sortedImportGroups = shouldSort - ? stableSort(oldImportGroups, (group1, group2) => compareModuleSpecifiers(group1[0].moduleSpecifier, group2[0].moduleSpecifier)) + ? stableSort(oldImportGroups, (group1, group2) => compareModuleSpecifiersWorker(group1[0].moduleSpecifier, group2[0].moduleSpecifier, comparer)) : oldImportGroups; const newImportDecls = flatMap(sortedImportGroups, importGroup => getExternalModuleName(importGroup[0].moduleSpecifier) @@ -308,10 +308,11 @@ function getExternalModuleName(specifier: Expression | undefined) { /** * @param importGroup a list of ImportDeclarations, all with the same module name. * + * @deprecated Only used for testing * @internal */ -export function coalesceImports(importGroup: readonly ImportDeclaration[], ignoreCase?: boolean, sourceFile?: SourceFile): readonly ImportDeclaration[] { - const comparer = getOrganizeImportsOrdinalStringComparer(!!ignoreCase); +export function coalesceImports(importGroup: readonly ImportDeclaration[], ignoreCase: boolean, sourceFile?: SourceFile): readonly ImportDeclaration[] { + const comparer = getOrganizeImportsOrdinalStringComparer(ignoreCase); return coalesceImportsWorker(importGroup, comparer, sourceFile); } @@ -482,6 +483,7 @@ function getCategorizedImports(importGroup: readonly ImportDeclaration[]) { /** * @param exportGroup a list of ExportDeclarations, all with the same module name. * + * @deprecated Only used for testing * @internal */ export function coalesceExports(exportGroup: readonly ExportDeclaration[], ignoreCase: boolean) { @@ -574,31 +576,32 @@ function updateImportDeclarationAndClause( } function sortSpecifiers(specifiers: readonly T[], comparer: Comparer) { - return stableSort(specifiers, (s1, s2) => compareImportOrExportSpecifiersWorker(s1, s2, comparer)); + return stableSort(specifiers, (s1, s2) => compareImportOrExportSpecifiers(s1, s2, comparer)); } /** @internal */ -export function compareImportOrExportSpecifiers(s1: T, s2: T, ignoreCase?: boolean): Comparison { - return compareImportOrExportSpecifiersWorker(s1, s2, getOrganizeImportsOrdinalStringComparer(!!ignoreCase)); -} - -function compareImportOrExportSpecifiersWorker(s1: T, s2: T, comparer: Comparer): Comparison { +export function compareImportOrExportSpecifiers(s1: T, s2: T, comparer: Comparer): Comparison { return compareBooleans(s1.isTypeOnly, s2.isTypeOnly) || comparer(s1.name.text, s2.name.text); } /** * Exported for testing * + * @deprecated Only used for testing * @internal */ export function compareModuleSpecifiers(m1: Expression | undefined, m2: Expression | undefined, ignoreCase?: boolean) { + const comparer = getOrganizeImportsOrdinalStringComparer(!!ignoreCase); + return compareModuleSpecifiersWorker(m1, m2, comparer); +} + +function compareModuleSpecifiersWorker(m1: Expression | undefined, m2: Expression | undefined, comparer: Comparer) { const name1 = m1 === undefined ? undefined : getExternalModuleName(m1); const name2 = m2 === undefined ? undefined : getExternalModuleName(m2); - const compareStrings = ignoreCase ? compareStringsCaseInsensitiveEslintCompatible : compareStringsCaseSensitive; return compareBooleans(name1 === undefined, name2 === undefined) || compareBooleans(isExternalModuleNameRelative(name1!), isExternalModuleNameRelative(name2!)) || // eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion - compareStrings(name1!, name2!); // I don't know why eslint is wrong but this one is necessary + comparer(name1!, name2!); // I don't know why eslint is wrong but this one is necessary } function getModuleSpecifierExpression(declaration: AnyImportOrRequireStatement): Expression | undefined { @@ -620,11 +623,17 @@ export function detectSorting(sourceFile: SourceFile, preferences: UserPreferenc } function detectSortingWorker(importGroups: ImportDeclaration[][], preferences: UserPreferences): SortKind { + const collateCaseSensitive = getOrganizeImportsComparer(preferences, /*ignoreCase*/ false); + const collateCaseInsensitive = getOrganizeImportsComparer(preferences, /*ignoreCase*/ true); let sortState = SortKind.Both; for (const importGroup of importGroups) { // Check module specifiers if (importGroup.length > 1) { - sortState &= detectSortCaseSensitivity(importGroup, i => tryCast(i.moduleSpecifier, isStringLiteral)?.text ?? "", compareStringsCaseSensitive, compareStringsCaseInsensitiveEslintCompatible); + sortState &= detectSortCaseSensitivity( + importGroup, + i => tryCast(i.moduleSpecifier, isStringLiteral)?.text ?? "", + collateCaseSensitive, + collateCaseInsensitive); if (!sortState) { return sortState; } @@ -652,12 +661,14 @@ function detectSortingWorker(importGroups: ImportDeclaration[][], preferences: U } /** @internal */ -export function detectImportDeclarationSorting(imports: readonly AnyImportOrRequireStatement[]): SortKind { +export function detectImportDeclarationSorting(imports: readonly AnyImportOrRequireStatement[], preferences: UserPreferences): SortKind { + const collateCaseSensitive = getOrganizeImportsComparer(preferences, /*ignoreCase*/ false); + const collateCaseInsensitive = getOrganizeImportsComparer(preferences, /*ignoreCase*/ true); return detectSortCaseSensitivity( imports, s => getExternalModuleName(getModuleSpecifierExpression(s)) || "", - compareStringsCaseInsensitiveEslintCompatible, - compareStringsCaseSensitive + collateCaseSensitive, + collateCaseInsensitive ); } @@ -696,20 +707,20 @@ export const detectImportSpecifierSorting = memoizeCached((specifiers: readonly }, new ImportSpecifierSortingCache()); /** @internal */ -export function getImportDeclarationInsertionIndex(sortedImports: readonly AnyImportOrRequireStatement[], newImport: AnyImportOrRequireStatement, ignoreCase?: boolean) { - const index = binarySearch(sortedImports, newImport, identity, (a, b) => compareImportsOrRequireStatements(a, b, ignoreCase)); +export function getImportDeclarationInsertionIndex(sortedImports: readonly AnyImportOrRequireStatement[], newImport: AnyImportOrRequireStatement, comparer: Comparer) { + const index = binarySearch(sortedImports, newImport, identity, (a, b) => compareImportsOrRequireStatements(a, b, comparer)); return index < 0 ? ~index : index; } /** @internal */ export function getImportSpecifierInsertionIndex(sortedImports: readonly ImportSpecifier[], newImport: ImportSpecifier, comparer: Comparer) { - const index = binarySearch(sortedImports, newImport, identity, (s1, s2) => compareImportOrExportSpecifiersWorker(s1, s2, comparer)); + const index = binarySearch(sortedImports, newImport, identity, (s1, s2) => compareImportOrExportSpecifiers(s1, s2, comparer)); return index < 0 ? ~index : index; } /** @internal */ -export function compareImportsOrRequireStatements(s1: AnyImportOrRequireStatement, s2: AnyImportOrRequireStatement, ignoreCase?: boolean) { - return compareModuleSpecifiers(getModuleSpecifierExpression(s1), getModuleSpecifierExpression(s2), ignoreCase) || compareImportKind(s1, s2); +export function compareImportsOrRequireStatements(s1: AnyImportOrRequireStatement, s2: AnyImportOrRequireStatement, comparer: Comparer) { + return compareModuleSpecifiersWorker(getModuleSpecifierExpression(s1), getModuleSpecifierExpression(s2), comparer) || compareImportKind(s1, s2); } function compareImportKind(s1: AnyImportOrRequireStatement, s2: AnyImportOrRequireStatement) { diff --git a/src/services/refactors/moveToNewFile.ts b/src/services/refactors/moveToNewFile.ts index 7950f25f5a6cf..f762111174d91 100644 --- a/src/services/refactors/moveToNewFile.ts +++ b/src/services/refactors/moveToNewFile.ts @@ -282,7 +282,7 @@ function getNewStatementsAndRemoveFromOldFile( const quotePreference = getQuotePreference(oldFile, preferences); const importsFromNewFile = createOldFileImportsFromNewFile(oldFile, usage.oldFileImportsFromNewFile, newFilename, program, host, useEsModuleSyntax, quotePreference); if (importsFromNewFile) { - insertImports(changes, oldFile, importsFromNewFile, /*blankLineBetween*/ true); + insertImports(changes, oldFile, importsFromNewFile, /*blankLineBetween*/ true, preferences); } deleteUnusedOldImports(oldFile, toMove.all, changes, usage.unusedImportsFromOldFile, checker); diff --git a/src/services/utilities.ts b/src/services/utilities.ts index 0eca9e34446c3..b0f5277682b67 100644 --- a/src/services/utilities.ts +++ b/src/services/utilities.ts @@ -305,6 +305,7 @@ import { skipAlias, skipOuterExpressions, some, + SortKind, SourceFile, SourceFileLike, SourceMapper, @@ -2534,17 +2535,20 @@ export function findModifier(node: Node, kind: Modifier["kind"]): Modifier | und } /** @internal */ -export function insertImports(changes: textChanges.ChangeTracker, sourceFile: SourceFile, imports: AnyImportOrRequireStatement | readonly AnyImportOrRequireStatement[], blankLineBetween: boolean): void { +export function insertImports(changes: textChanges.ChangeTracker, sourceFile: SourceFile, imports: AnyImportOrRequireStatement | readonly AnyImportOrRequireStatement[], blankLineBetween: boolean, preferences: UserPreferences): void { const decl = isArray(imports) ? imports[0] : imports; const importKindPredicate: (node: Node) => node is AnyImportOrRequireStatement = decl.kind === SyntaxKind.VariableStatement ? isRequireVariableStatement : isAnyImportSyntax; const existingImportStatements = filter(sourceFile.statements, importKindPredicate); - const sortedNewImports = isArray(imports) ? stableSort(imports, OrganizeImports.compareImportsOrRequireStatements) : [imports]; + let sortKind = isArray(imports) ? OrganizeImports.detectImportDeclarationSorting(imports, preferences) : SortKind.Both; + const comparer = OrganizeImports.getOrganizeImportsComparer(preferences, sortKind == SortKind.CaseInsensitive) + const sortedNewImports = isArray(imports) ? stableSort(imports, (a, b) => OrganizeImports.compareImportsOrRequireStatements(a, b, comparer)) : [imports]; if (!existingImportStatements.length) { changes.insertNodesAtTopOfFile(sourceFile, sortedNewImports, blankLineBetween); } - else if (existingImportStatements && OrganizeImports.detectImportDeclarationSorting(existingImportStatements)) { + else if (existingImportStatements && (sortKind = OrganizeImports.detectImportDeclarationSorting(existingImportStatements, preferences))) { + const comparer = OrganizeImports.getOrganizeImportsComparer(preferences, sortKind == SortKind.CaseInsensitive) for (const newImport of sortedNewImports) { - const insertionIndex = OrganizeImports.getImportDeclarationInsertionIndex(existingImportStatements, newImport); + const insertionIndex = OrganizeImports.getImportDeclarationInsertionIndex(existingImportStatements, newImport, comparer); if (insertionIndex === 0) { // If the first import is top-of-file, insert after the leading comment which is likely the header. const options = existingImportStatements[0] === sourceFile.statements[0] ? diff --git a/tests/cases/fourslash/organizeImportsPathsUnicode1.ts b/tests/cases/fourslash/organizeImportsPathsUnicode1.ts new file mode 100644 index 0000000000000..5495da0d03aa0 --- /dev/null +++ b/tests/cases/fourslash/organizeImportsPathsUnicode1.ts @@ -0,0 +1,30 @@ +/// + +//// import * as Ab from "./Ab"; +//// import * as _aB from "./_aB"; +//// import * as aB from "./aB"; +//// import * as _Ab from "./_Ab"; +//// +//// console.log(_aB, _Ab, aB, Ab); + +verify.organizeImports( +`import * as Ab from "./Ab"; +import * as _Ab from "./_Ab"; +import * as _aB from "./_aB"; +import * as aB from "./aB"; + +console.log(_aB, _Ab, aB, Ab);`, /*mode*/ undefined, { + organizeImportsIgnoreCase: false, + organizeImportsCollation: "ordinal", +}); + +verify.organizeImports( +`import * as _aB from "./_aB"; +import * as _Ab from "./_Ab"; +import * as aB from "./aB"; +import * as Ab from "./Ab"; + +console.log(_aB, _Ab, aB, Ab);`, /*mode*/ undefined, { + organizeImportsIgnoreCase: false, + organizeImportsCollation: "unicode", +}); diff --git a/tests/cases/fourslash/organizeImportsPathsUnicode2.ts b/tests/cases/fourslash/organizeImportsPathsUnicode2.ts new file mode 100644 index 0000000000000..bca471c32590b --- /dev/null +++ b/tests/cases/fourslash/organizeImportsPathsUnicode2.ts @@ -0,0 +1,29 @@ +/// + +//// import * as a2 from "./a2"; +//// import * as a100 from "./a100"; +//// import * as a1 from "./a1"; +//// +//// console.log(a1, a2, a100); + +verify.organizeImports( +`import * as a1 from "./a1"; +import * as a100 from "./a100"; +import * as a2 from "./a2"; + +console.log(a1, a2, a100);`, /*mode*/ undefined, { + organizeImportsIgnoreCase: false, + organizeImportsCollation: "unicode", + organizeImportsNumericCollation: false, +}); + +verify.organizeImports( +`import * as a1 from "./a1"; +import * as a2 from "./a2"; +import * as a100 from "./a100"; + +console.log(a1, a2, a100);`, /*mode*/ undefined, { + organizeImportsIgnoreCase: false, + organizeImportsCollation: "unicode", + organizeImportsNumericCollation: true, +}); diff --git a/tests/cases/fourslash/organizeImportsPathsUnicode3.ts b/tests/cases/fourslash/organizeImportsPathsUnicode3.ts new file mode 100644 index 0000000000000..05e3571bca09a --- /dev/null +++ b/tests/cases/fourslash/organizeImportsPathsUnicode3.ts @@ -0,0 +1,29 @@ +/// + +//// import * as B from "./B"; +//// import * as À from "./À"; +//// import * as A from "./A"; +//// +//// console.log(A, À, B); + +verify.organizeImports( +`import * as À from "./À"; +import * as A from "./A"; +import * as B from "./B"; + +console.log(A, À, B);`, /*mode*/ undefined, { + organizeImportsIgnoreCase: false, + organizeImportsCollation: "unicode", + organizeImportsAccentCollation: false, +}); + +verify.organizeImports( +`import * as A from "./A"; +import * as À from "./À"; +import * as B from "./B"; + +console.log(A, À, B);`, /*mode*/ undefined, { + organizeImportsIgnoreCase: false, + organizeImportsCollation: "unicode", + organizeImportsAccentCollation: true, +}); diff --git a/tests/cases/fourslash/organizeImportsPathsUnicode4.ts b/tests/cases/fourslash/organizeImportsPathsUnicode4.ts new file mode 100644 index 0000000000000..ca7159953515a --- /dev/null +++ b/tests/cases/fourslash/organizeImportsPathsUnicode4.ts @@ -0,0 +1,32 @@ +/// + +//// import * as Ab from "./Ab"; +//// import * as _aB from "./_aB"; +//// import * as aB from "./aB"; +//// import * as _Ab from "./_Ab"; +//// +//// console.log(_aB, _Ab, aB, Ab); + +verify.organizeImports( +`import * as _Ab from "./_Ab"; +import * as _aB from "./_aB"; +import * as Ab from "./Ab"; +import * as aB from "./aB"; + +console.log(_aB, _Ab, aB, Ab);`, /*mode*/ undefined, { + organizeImportsIgnoreCase: false, + organizeImportsCollation: "unicode", + organizeImportsCaseFirst: "upper", +}); + +verify.organizeImports( +`import * as _aB from "./_aB"; +import * as _Ab from "./_Ab"; +import * as aB from "./aB"; +import * as Ab from "./Ab"; + +console.log(_aB, _Ab, aB, Ab);`, /*mode*/ undefined, { + organizeImportsIgnoreCase: false, + organizeImportsCollation: "unicode", + organizeImportsCaseFirst: "lower", +}); From e60cb4957d6382a759bff65c0db959b58f5689b0 Mon Sep 17 00:00:00 2001 From: Ron Buckton Date: Thu, 19 Jan 2023 12:05:47 -0500 Subject: [PATCH 07/10] Fix lint errors --- src/services/utilities.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/services/utilities.ts b/src/services/utilities.ts index 3eae422a5afcc..d49afd59be64e 100644 --- a/src/services/utilities.ts +++ b/src/services/utilities.ts @@ -2540,13 +2540,13 @@ export function insertImports(changes: textChanges.ChangeTracker, sourceFile: So const importKindPredicate: (node: Node) => node is AnyImportOrRequireStatement = decl.kind === SyntaxKind.VariableStatement ? isRequireVariableStatement : isAnyImportSyntax; const existingImportStatements = filter(sourceFile.statements, importKindPredicate); let sortKind = isArray(imports) ? OrganizeImports.detectImportDeclarationSorting(imports, preferences) : SortKind.Both; - const comparer = OrganizeImports.getOrganizeImportsComparer(preferences, sortKind == SortKind.CaseInsensitive) + const comparer = OrganizeImports.getOrganizeImportsComparer(preferences, sortKind === SortKind.CaseInsensitive); const sortedNewImports = isArray(imports) ? stableSort(imports, (a, b) => OrganizeImports.compareImportsOrRequireStatements(a, b, comparer)) : [imports]; if (!existingImportStatements.length) { changes.insertNodesAtTopOfFile(sourceFile, sortedNewImports, blankLineBetween); } else if (existingImportStatements && (sortKind = OrganizeImports.detectImportDeclarationSorting(existingImportStatements, preferences))) { - const comparer = OrganizeImports.getOrganizeImportsComparer(preferences, sortKind == SortKind.CaseInsensitive) + const comparer = OrganizeImports.getOrganizeImportsComparer(preferences, sortKind === SortKind.CaseInsensitive); for (const newImport of sortedNewImports) { const insertionIndex = OrganizeImports.getImportDeclarationInsertionIndex(existingImportStatements, newImport, comparer); if (insertionIndex === 0) { From 12cca65d933a5ea65757cdb682e7dde7a0435206 Mon Sep 17 00:00:00 2001 From: Ron Buckton Date: Thu, 19 Jan 2023 12:14:09 -0500 Subject: [PATCH 08/10] Reintroduce early exit shortcut to detectSortCaseSensitivity --- src/compiler/core.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/compiler/core.ts b/src/compiler/core.ts index af0c175671f3a..6b620569054cd 100644 --- a/src/compiler/core.ts +++ b/src/compiler/core.ts @@ -875,7 +875,7 @@ export function detectSortCaseSensitivity( if (array.length < 2) return kind; let prevElement = getString(array[0]); - for (let i = 1, len = array.length; i < len; i++) { + 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; From d7de7524a9b51431e88c00edc70d2ada703635be Mon Sep 17 00:00:00 2001 From: Ron Buckton Date: Thu, 19 Jan 2023 14:39:24 -0500 Subject: [PATCH 09/10] Fix comments --- src/server/protocol.ts | 12 +++++------- tests/baselines/reference/api/tsserverlibrary.d.ts | 12 +++++------- 2 files changed, 10 insertions(+), 14 deletions(-) diff --git a/src/server/protocol.ts b/src/server/protocol.ts index 5bd17a22f953c..1487502e7c1a7 100644 --- a/src/server/protocol.ts +++ b/src/server/protocol.ts @@ -3527,16 +3527,14 @@ export interface UserPreferences { * [Unicode Collation Algorithm](https://unicode.org/reports/tr10/#Scope)) using rules associated with the locale * specified in {@link organizeImportsCollationLocale}. * - * NOTE: When comparing paths `"ordinal"` collation is always used. - * * Default: `"ordinal"`. */ readonly organizeImportsCollation?: "ordinal" | "unicode"; /** - * Indicates the locale to use for "natural" collation. If not specified, the locale `"en"` is used as an invariant + * Indicates the locale to use for "unicode" collation. If not specified, the locale `"en"` is used as an invariant * for the sake of consistent sorting. Use `"auto"` to use the detected UI locale. * - * This preference is ignored if {@link organizeImportsCollation} is not `"natural"`. + * This preference is ignored if {@link organizeImportsCollation} is not `"unicode"`. * * Default: `"en"` */ @@ -3545,7 +3543,7 @@ export interface UserPreferences { * Indicates whether numeric collation should be used for digit sequences in strings. When `true`, will collate * strings such that `a1z < a2z < a100z`. When `false`, will collate strings such that `a1z < a100z < a2z`. * - * This preference is ignored if {@link organizeImportsCollation} is not `"natural"`. + * This preference is ignored if {@link organizeImportsCollation} is not `"unicode"`. * * Default: `true` */ @@ -3555,7 +3553,7 @@ export interface UserPreferences { * `true`, characters with accents and other diacritics will be collated in the order defined by the locale specified * in {@link organizeImportsCollationLocale}. * - * This preference is ignored if {@link organizeImportsCollation} is not `"natural"`. + * This preference is ignored if {@link organizeImportsCollation} is not `"unicode"`. * * Default: `true` */ @@ -3564,7 +3562,7 @@ export interface UserPreferences { * Indicates whether upper case or lower case should sort first. When `false`, the default order for the locale * specified in {@link organizeImportsCollationLocale} is used. * - * This preference is ignored if {@link organizeImportsCollation} is not `"natural"`. + * This preference is ignored if {@link organizeImportsCollation} is not `"unicode"`. * * Default: `false` */ diff --git a/tests/baselines/reference/api/tsserverlibrary.d.ts b/tests/baselines/reference/api/tsserverlibrary.d.ts index 80782356daac5..c4eb8fb4bb718 100644 --- a/tests/baselines/reference/api/tsserverlibrary.d.ts +++ b/tests/baselines/reference/api/tsserverlibrary.d.ts @@ -2812,16 +2812,14 @@ declare namespace ts { * [Unicode Collation Algorithm](https://unicode.org/reports/tr10/#Scope)) using rules associated with the locale * specified in {@link organizeImportsCollationLocale}. * - * NOTE: When comparing paths `"ordinal"` collation is always used. - * * Default: `"ordinal"`. */ readonly organizeImportsCollation?: "ordinal" | "unicode"; /** - * Indicates the locale to use for "natural" collation. If not specified, the locale `"en"` is used as an invariant + * Indicates the locale to use for "unicode" collation. If not specified, the locale `"en"` is used as an invariant * for the sake of consistent sorting. Use `"auto"` to use the detected UI locale. * - * This preference is ignored if {@link organizeImportsCollation} is not `"natural"`. + * This preference is ignored if {@link organizeImportsCollation} is not `"unicode"`. * * Default: `"en"` */ @@ -2830,7 +2828,7 @@ declare namespace ts { * Indicates whether numeric collation should be used for digit sequences in strings. When `true`, will collate * strings such that `a1z < a2z < a100z`. When `false`, will collate strings such that `a1z < a100z < a2z`. * - * This preference is ignored if {@link organizeImportsCollation} is not `"natural"`. + * This preference is ignored if {@link organizeImportsCollation} is not `"unicode"`. * * Default: `true` */ @@ -2840,7 +2838,7 @@ declare namespace ts { * `true`, characters with accents and other diacritics will be collated in the order defined by the locale specified * in {@link organizeImportsCollationLocale}. * - * This preference is ignored if {@link organizeImportsCollation} is not `"natural"`. + * This preference is ignored if {@link organizeImportsCollation} is not `"unicode"`. * * Default: `true` */ @@ -2849,7 +2847,7 @@ declare namespace ts { * Indicates whether upper case or lower case should sort first. When `false`, the default order for the locale * specified in {@link organizeImportsCollationLocale} is used. * - * This preference is ignored if {@link organizeImportsCollation} is not `"natural"`. + * This preference is ignored if {@link organizeImportsCollation} is not `"unicode"`. * * Default: `false` */ From 1d54f352e4f270fc8b65ff7db885aac865674239 Mon Sep 17 00:00:00 2001 From: Ron Buckton Date: Thu, 19 Jan 2023 16:32:51 -0500 Subject: [PATCH 10/10] Fix comment --- src/server/protocol.ts | 7 +++++-- tests/baselines/reference/api/tsserverlibrary.d.ts | 7 +++++-- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/server/protocol.ts b/src/server/protocol.ts index 1487502e7c1a7..4240ae95c2ddb 100644 --- a/src/server/protocol.ts +++ b/src/server/protocol.ts @@ -3545,7 +3545,7 @@ export interface UserPreferences { * * This preference is ignored if {@link organizeImportsCollation} is not `"unicode"`. * - * Default: `true` + * Default: `false` */ readonly organizeImportsNumericCollation?: boolean; /** @@ -3562,7 +3562,10 @@ export interface UserPreferences { * Indicates whether upper case or lower case should sort first. When `false`, the default order for the locale * specified in {@link organizeImportsCollationLocale} is used. * - * This preference is ignored if {@link organizeImportsCollation} is not `"unicode"`. + * This preference is ignored if {@link organizeImportsCollation} is not `"unicode"`. This preference is also + * ignored if we are using case-insensitive sorting, which occurs when {@link organizeImportsIgnoreCase} is `true`, + * or if {@link organizeImportsIgnoreCase} is `"auto"` and the auto-detected case sensitivity is determined to be + * case-insensitive. * * Default: `false` */ diff --git a/tests/baselines/reference/api/tsserverlibrary.d.ts b/tests/baselines/reference/api/tsserverlibrary.d.ts index c4eb8fb4bb718..db45c13166b5b 100644 --- a/tests/baselines/reference/api/tsserverlibrary.d.ts +++ b/tests/baselines/reference/api/tsserverlibrary.d.ts @@ -2830,7 +2830,7 @@ declare namespace ts { * * This preference is ignored if {@link organizeImportsCollation} is not `"unicode"`. * - * Default: `true` + * Default: `false` */ readonly organizeImportsNumericCollation?: boolean; /** @@ -2847,7 +2847,10 @@ declare namespace ts { * Indicates whether upper case or lower case should sort first. When `false`, the default order for the locale * specified in {@link organizeImportsCollationLocale} is used. * - * This preference is ignored if {@link organizeImportsCollation} is not `"unicode"`. + * This preference is ignored if {@link organizeImportsCollation} is not `"unicode"`. This preference is also + * ignored if we are using case-insensitive sorting, which occurs when {@link organizeImportsIgnoreCase} is `true`, + * or if {@link organizeImportsIgnoreCase} is `"auto"` and the auto-detected case sensitivity is determined to be + * case-insensitive. * * Default: `false` */