From 14afb68399ac2a035de3b4cc3c93bed5cb4cd3e5 Mon Sep 17 00:00:00 2001 From: Isabel Duan Date: Thu, 3 Aug 2023 12:19:59 -0700 Subject: [PATCH 01/14] add organizeImports option to sort types first, last, or inline --- src/compiler/types.ts | 1 + src/server/protocol.ts | 7 ++++ src/services/organizeImports.ts | 38 +++++++++++-------- tests/cases/fourslash/fourslash.ts | 1 + tests/cases/fourslash/organizeImportsType1.ts | 20 ++++++++++ tests/cases/fourslash/organizeImportsType2.ts | 22 +++++++++++ tests/cases/fourslash/organizeImportsType3.ts | 22 +++++++++++ tests/cases/fourslash/organizeImportsType4.ts | 19 ++++++++++ tests/cases/fourslash/organizeImportsType5.ts | 21 ++++++++++ tests/cases/fourslash/organizeImportsType6.ts | 21 ++++++++++ 10 files changed, 156 insertions(+), 16 deletions(-) create mode 100644 tests/cases/fourslash/organizeImportsType1.ts create mode 100644 tests/cases/fourslash/organizeImportsType2.ts create mode 100644 tests/cases/fourslash/organizeImportsType3.ts create mode 100644 tests/cases/fourslash/organizeImportsType4.ts create mode 100644 tests/cases/fourslash/organizeImportsType5.ts create mode 100644 tests/cases/fourslash/organizeImportsType6.ts diff --git a/src/compiler/types.ts b/src/compiler/types.ts index 008d4d710bea5..f58922ea464ec 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -9979,6 +9979,7 @@ export interface UserPreferences { readonly organizeImportsNumericCollation?: boolean; readonly organizeImportsAccentCollation?: boolean; readonly organizeImportsCaseFirst?: "upper" | "lower" | false; + readonly organizeImportsTypeOrder?: "first" | "last" | false; } /** Represents a bigint literal value without requiring bigint support */ diff --git a/src/server/protocol.ts b/src/server/protocol.ts index c0be0b0b93167..464942b714945 100644 --- a/src/server/protocol.ts +++ b/src/server/protocol.ts @@ -3591,6 +3591,13 @@ export interface UserPreferences { * Default: `false` */ readonly organizeImportsCaseFirst?: "upper" | "lower" | false; + /** + * Indicates where named type-only imports should sort. The default order (`false`) is to ignore whether an import is + * type-only, and sort them inline. + * + * Default: `false` + */ + readonly organizeImportsTypeOrder?: "first" | "last" | false; /** * Indicates whether {@link ReferencesResponseItem.lineText} is supported. diff --git a/src/services/organizeImports.ts b/src/services/organizeImports.ts index 573f908abf974..2d0036908b7a1 100644 --- a/src/services/organizeImports.ts +++ b/src/services/organizeImports.ts @@ -94,7 +94,7 @@ export function organizeImports( const processImportsOfSameModuleSpecifier = (importGroup: readonly ImportDeclaration[]) => { if (shouldRemove) importGroup = removeUnusedImports(importGroup, sourceFile, program); - if (shouldCombine) importGroup = coalesceImportsWorker(importGroup, comparer, sourceFile); + if (shouldCombine) importGroup = coalesceImportsWorker(importGroup, comparer, sourceFile, preferences); if (shouldSort) importGroup = stableSort(importGroup, (s1, s2) => compareImportsOrRequireStatements(s1, s2, comparer)); return importGroup; }; @@ -105,7 +105,7 @@ export function organizeImports( if (mode !== OrganizeImportsMode.RemoveUnused) { // All of the old ExportDeclarations in the file, in syntactic order. getTopLevelExportGroups(sourceFile).forEach(exportGroupDecl => - organizeImportsWorker(exportGroupDecl, group => coalesceExportsWorker(group, comparer))); + organizeImportsWorker(exportGroupDecl, group => coalesceExportsWorker(group, comparer, preferences))); } for (const ambientModule of sourceFile.statements.filter(isAmbientModule)) { @@ -117,7 +117,7 @@ export function organizeImports( // Exports are always used if (mode !== OrganizeImportsMode.RemoveUnused) { const ambientModuleExportDecls = ambientModule.body.statements.filter(isExportDeclaration); - organizeImportsWorker(ambientModuleExportDecls, group => coalesceExportsWorker(group, comparer)); + organizeImportsWorker(ambientModuleExportDecls, group => coalesceExportsWorker(group, comparer, preferences)); } } @@ -310,12 +310,12 @@ function getExternalModuleName(specifier: Expression | undefined) { * @deprecated Only used for testing * @internal */ -export function coalesceImports(importGroup: readonly ImportDeclaration[], ignoreCase: boolean, sourceFile?: SourceFile): readonly ImportDeclaration[] { +export function coalesceImports(importGroup: readonly ImportDeclaration[], ignoreCase: boolean, sourceFile?: SourceFile, preferences?: UserPreferences): readonly ImportDeclaration[] { const comparer = getOrganizeImportsOrdinalStringComparer(ignoreCase); - return coalesceImportsWorker(importGroup, comparer, sourceFile); + return coalesceImportsWorker(importGroup, comparer, sourceFile, preferences); } -function coalesceImportsWorker(importGroup: readonly ImportDeclaration[], comparer: Comparer, sourceFile?: SourceFile): readonly ImportDeclaration[] { +function coalesceImportsWorker(importGroup: readonly ImportDeclaration[], comparer: Comparer, sourceFile?: SourceFile, preferences?: UserPreferences): readonly ImportDeclaration[] { if (importGroup.length === 0) { return importGroup; } @@ -372,7 +372,7 @@ function coalesceImportsWorker(importGroup: readonly ImportDeclaration[], compar newImportSpecifiers.push(...getNewImportSpecifiers(namedImports)); const sortedImportSpecifiers = factory.createNodeArray( - sortSpecifiers(newImportSpecifiers, comparer), + sortSpecifiers(newImportSpecifiers, comparer, preferences), firstNamedImport?.importClause.namedBindings.elements.hasTrailingComma ); @@ -485,18 +485,17 @@ function getCategorizedImports(importGroup: readonly ImportDeclaration[]) { * @deprecated Only used for testing * @internal */ -export function coalesceExports(exportGroup: readonly ExportDeclaration[], ignoreCase: boolean) { +export function coalesceExports(exportGroup: readonly ExportDeclaration[], ignoreCase: boolean, preferences?: UserPreferences) { const comparer = getOrganizeImportsOrdinalStringComparer(ignoreCase); - return coalesceExportsWorker(exportGroup, comparer); + return coalesceExportsWorker(exportGroup, comparer, preferences); } -function coalesceExportsWorker(exportGroup: readonly ExportDeclaration[], comparer: Comparer) { +function coalesceExportsWorker(exportGroup: readonly ExportDeclaration[], comparer: Comparer, preferences?: UserPreferences) { if (exportGroup.length === 0) { return exportGroup; } const { exportWithoutClause, namedExports, typeOnlyExports } = getCategorizedExports(exportGroup); - const coalescedExports: ExportDeclaration[] = []; if (exportWithoutClause) { @@ -510,7 +509,7 @@ function coalesceExportsWorker(exportGroup: readonly ExportDeclaration[], compar const newExportSpecifiers: ExportSpecifier[] = []; newExportSpecifiers.push(...flatMap(exportGroup, i => i.exportClause && isNamedExports(i.exportClause) ? i.exportClause.elements : emptyArray)); - const sortedExportSpecifiers = sortSpecifiers(newExportSpecifiers, comparer); + const sortedExportSpecifiers = sortSpecifiers(newExportSpecifiers, comparer, preferences); const exportDecl = exportGroup[0]; coalescedExports.push( @@ -574,13 +573,20 @@ function updateImportDeclarationAndClause( importDeclaration.assertClause); } -function sortSpecifiers(specifiers: readonly T[], comparer: Comparer) { - return stableSort(specifiers, (s1, s2) => compareImportOrExportSpecifiers(s1, s2, comparer)); +function sortSpecifiers(specifiers: readonly T[], comparer: Comparer, preferences?: UserPreferences): readonly T[] { + return stableSort(specifiers, (s1, s2) => compareImportOrExportSpecifiers(s1, s2, comparer, preferences)); } /** @internal */ -export function compareImportOrExportSpecifiers(s1: T, s2: T, comparer: Comparer): Comparison { - return compareBooleans(s1.isTypeOnly, s2.isTypeOnly) || comparer(s1.name.text, s2.name.text); +export function compareImportOrExportSpecifiers(s1: T, s2: T, comparer: Comparer, preferences?: UserPreferences): Comparison { + switch (preferences?.organizeImportsTypeOrder){ + case "first": + return compareBooleans(s2.isTypeOnly, s1.isTypeOnly) || comparer(s1.name.text, s2.name.text); + case "last": + return compareBooleans(s1.isTypeOnly, s2.isTypeOnly) || comparer(s1.name.text, s2.name.text); + default: + return comparer(s1.name.text, s2.name.text); + } } /** diff --git a/tests/cases/fourslash/fourslash.ts b/tests/cases/fourslash/fourslash.ts index 82321aef548a9..4ee7ee6931e4c 100644 --- a/tests/cases/fourslash/fourslash.ts +++ b/tests/cases/fourslash/fourslash.ts @@ -673,6 +673,7 @@ declare namespace FourSlashInterface { readonly organizeImportsNumericCollation?: boolean; readonly organizeImportsAccentCollation?: boolean; readonly organizeImportsCaseFirst?: "upper" | "lower" | false; + readonly organizeImportsTypeOrder?: "first" | "last" | false; } interface InlayHintsOptions extends UserPreferences { readonly includeInlayParameterNameHints?: "none" | "literals" | "all"; diff --git a/tests/cases/fourslash/organizeImportsType1.ts b/tests/cases/fourslash/organizeImportsType1.ts new file mode 100644 index 0000000000000..c656780447eff --- /dev/null +++ b/tests/cases/fourslash/organizeImportsType1.ts @@ -0,0 +1,20 @@ +/// + +// @allowSyntheticDefaultImports: true +// @moduleResolution: node +// @noUnusedLocals: true +// @target: es2018 + +//// import { A } from "foo"; +//// import { type B } from "foo"; +//// import { C } from "foo"; +//// import { type E } from "foo"; +//// import { D } from "foo"; +//// +//// console.log(A, B, C, D, E); + +verify.organizeImports( +`import { A, type B, C, D, type E } from "foo"; + +console.log(A, B, C, D, E);` +); diff --git a/tests/cases/fourslash/organizeImportsType2.ts b/tests/cases/fourslash/organizeImportsType2.ts new file mode 100644 index 0000000000000..1d28edc04ff47 --- /dev/null +++ b/tests/cases/fourslash/organizeImportsType2.ts @@ -0,0 +1,22 @@ +/// + +// @allowSyntheticDefaultImports: true +// @moduleResolution: node +// @noUnusedLocals: true +// @target: es2018 + +//// import { A } from "foo"; +//// import { type B } from "foo"; +//// import { C } from "foo"; +//// import { type E } from "foo"; +//// import { D } from "foo"; +//// +//// console.log(A, B, C, D, E); + +verify.organizeImports( +`import { type B, type E, A, C, D } from "foo"; + +console.log(A, B, C, D, E);`, + undefined, + { organizeImportsTypeOrder : "first" } +); \ No newline at end of file diff --git a/tests/cases/fourslash/organizeImportsType3.ts b/tests/cases/fourslash/organizeImportsType3.ts new file mode 100644 index 0000000000000..85e6d909b033c --- /dev/null +++ b/tests/cases/fourslash/organizeImportsType3.ts @@ -0,0 +1,22 @@ +/// + +// @allowSyntheticDefaultImports: true +// @moduleResolution: node +// @noUnusedLocals: true +// @target: es2018 + +//// import { A } from "foo"; +//// import { type B } from "foo"; +//// import { C } from "foo"; +//// import { type E } from "foo"; +//// import { D } from "foo"; +//// +//// console.log(A, B, C, D, E); + +verify.organizeImports( +`import { A, C, D, type B, type E } from "foo"; + +console.log(A, B, C, D, E);`, + undefined, + { organizeImportsTypeOrder : "last" } +); \ No newline at end of file diff --git a/tests/cases/fourslash/organizeImportsType4.ts b/tests/cases/fourslash/organizeImportsType4.ts new file mode 100644 index 0000000000000..7df135fefb643 --- /dev/null +++ b/tests/cases/fourslash/organizeImportsType4.ts @@ -0,0 +1,19 @@ +/// + +// @allowSyntheticDefaultImports: true +// @moduleResolution: node +// @noUnusedLocals: true +// @target: es2018 + +//// type A = string; +//// type B = string; +//// const C = "hello"; +//// export { A, type B, C }; + +verify.organizeImports( +`type A = string; +type B = string; +const C = "hello"; +export { A, type B, C }; +` +); \ No newline at end of file diff --git a/tests/cases/fourslash/organizeImportsType5.ts b/tests/cases/fourslash/organizeImportsType5.ts new file mode 100644 index 0000000000000..5a312b40ead6e --- /dev/null +++ b/tests/cases/fourslash/organizeImportsType5.ts @@ -0,0 +1,21 @@ +/// + +// @allowSyntheticDefaultImports: true +// @moduleResolution: node +// @noUnusedLocals: true +// @target: es2018 + +//// type A = string; +//// type B = string; +//// const C = "hello"; +//// export { A, type B, C }; + +verify.organizeImports( +`type A = string; +type B = string; +const C = "hello"; +export { type B, A, C }; +`, + undefined, + { organizeImportsTypeOrder : "first" } +); \ No newline at end of file diff --git a/tests/cases/fourslash/organizeImportsType6.ts b/tests/cases/fourslash/organizeImportsType6.ts new file mode 100644 index 0000000000000..579c1755c874b --- /dev/null +++ b/tests/cases/fourslash/organizeImportsType6.ts @@ -0,0 +1,21 @@ +/// + +// @allowSyntheticDefaultImports: true +// @moduleResolution: node +// @noUnusedLocals: true +// @target: es2018 + +//// type A = string; +//// type B = string; +//// const C = "hello"; +//// export { A, type B, C }; + +verify.organizeImports( +`type A = string; +type B = string; +const C = "hello"; +export { A, C, type B }; +`, + undefined, + { organizeImportsTypeOrder : "last" } +); \ No newline at end of file From e72037789fff2a3c96fa5b5d073558b4cfa370e9 Mon Sep 17 00:00:00 2001 From: Isabel Duan Date: Thu, 3 Aug 2023 21:56:55 -0700 Subject: [PATCH 02/14] update tests --- src/compiler/types.ts | 2 +- src/server/protocol.ts | 8 ++-- src/services/codefixes/importFixes.ts | 7 ++- .../unittests/services/organizeImports.ts | 2 +- .../reference/api/tsserverlibrary.d.ts | 8 ++++ tests/baselines/reference/api/typescript.d.ts | 1 + tests/cases/fourslash/fourslash.ts | 2 +- .../importNameCodeFix_importType7.ts | 2 +- tests/cases/fourslash/organizeImportsType1.ts | 25 +++++++++++ tests/cases/fourslash/organizeImportsType2.ts | 45 +++++++++++++++---- tests/cases/fourslash/organizeImportsType3.ts | 22 --------- tests/cases/fourslash/organizeImportsType4.ts | 19 -------- tests/cases/fourslash/organizeImportsType5.ts | 21 --------- tests/cases/fourslash/organizeImportsType6.ts | 21 --------- 14 files changed, 84 insertions(+), 101 deletions(-) delete mode 100644 tests/cases/fourslash/organizeImportsType3.ts delete mode 100644 tests/cases/fourslash/organizeImportsType4.ts delete mode 100644 tests/cases/fourslash/organizeImportsType5.ts delete mode 100644 tests/cases/fourslash/organizeImportsType6.ts diff --git a/src/compiler/types.ts b/src/compiler/types.ts index 01c4027a6db7b..bf944077a15ef 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -9995,7 +9995,7 @@ export interface UserPreferences { readonly organizeImportsNumericCollation?: boolean; readonly organizeImportsAccentCollation?: boolean; readonly organizeImportsCaseFirst?: "upper" | "lower" | false; - readonly organizeImportsTypeOrder?: "first" | "last" | false; + readonly organizeImportsTypeOrder?: "first" | "last" | "inline"; } /** Represents a bigint literal value without requiring bigint support */ diff --git a/src/server/protocol.ts b/src/server/protocol.ts index 56b80952a91cf..2a055b62759d8 100644 --- a/src/server/protocol.ts +++ b/src/server/protocol.ts @@ -3599,12 +3599,12 @@ export interface UserPreferences { */ readonly organizeImportsCaseFirst?: "upper" | "lower" | false; /** - * Indicates where named type-only imports should sort. The default order (`false`) is to ignore whether an import is - * type-only, and sort them inline. + * Indicates where named type-only imports should sort. The default order ("inline") sorts imports regardless of if the imports are + * type-only. * - * Default: `false` + * Default: `inline` */ - readonly organizeImportsTypeOrder?: "first" | "last" | false; + readonly organizeImportsTypeOrder?: "first" | "last" | "inline"; /** * Indicates whether {@link ReferencesResponseItem.lineText} is supported. diff --git a/src/services/codefixes/importFixes.ts b/src/services/codefixes/importFixes.ts index e6f0eb91405e3..fde1683b12e26 100644 --- a/src/services/codefixes/importFixes.ts +++ b/src/services/codefixes/importFixes.ts @@ -53,6 +53,7 @@ import { getSourceFileOfNode, getSymbolId, getTokenAtPosition, + getTokenPosOfNode, getTypeKeywordOfTypeOnlyImport, getUniqueSymbolId, hostGetCanonicalFileName, @@ -1329,10 +1330,14 @@ function promoteFromTypeOnly(changes: textChanges.ChangeTracker, aliasDeclaratio if (aliasDeclaration.isTypeOnly) { 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 comparer = OrganizeImports.getOrganizeImportsComparer(preferences, sortKind === SortKind.CaseInsensitive); const insertionIndex = OrganizeImports.getImportSpecifierInsertionIndex(aliasDeclaration.parent.elements, newSpecifier, comparer); + if (insertionIndex === aliasDeclaration.parent.elements.indexOf(aliasDeclaration)) { + changes.deleteRange(sourceFile, { pos: getTokenPosOfNode(aliasDeclaration.getFirstToken()!), end: getTokenPosOfNode(aliasDeclaration.propertyName ?? aliasDeclaration.name) }); + return aliasDeclaration; + } + changes.delete(sourceFile, aliasDeclaration); changes.insertImportSpecifierAtIndex(sourceFile, newSpecifier, aliasDeclaration.parent, insertionIndex); } else { diff --git a/src/testRunner/unittests/services/organizeImports.ts b/src/testRunner/unittests/services/organizeImports.ts index 9794bd1d68746..9e79a64ca8ea2 100644 --- a/src/testRunner/unittests/services/organizeImports.ts +++ b/src/testRunner/unittests/services/organizeImports.ts @@ -217,7 +217,7 @@ describe("unittests:: services:: organizeImports", () => { it("Sort specifiers - type-only", () => { const sortedImports = parseImports(`import { type z, y, type x, c, type b, a } from "lib";`); const actualCoalescedImports = ts.OrganizeImports.coalesceImports(sortedImports, /*ignoreCase*/ true); - const expectedCoalescedImports = parseImports(`import { a, c, y, type b, type x, type z } from "lib";`); + const expectedCoalescedImports = parseImports(`import { a, type b, c, type x, y, type z } from "lib";`); assertListEqual(actualCoalescedImports, expectedCoalescedImports); }); diff --git a/tests/baselines/reference/api/tsserverlibrary.d.ts b/tests/baselines/reference/api/tsserverlibrary.d.ts index 4571351e5a9cc..eb047cf0fa386 100644 --- a/tests/baselines/reference/api/tsserverlibrary.d.ts +++ b/tests/baselines/reference/api/tsserverlibrary.d.ts @@ -2891,6 +2891,13 @@ declare namespace ts { * Default: `false` */ readonly organizeImportsCaseFirst?: "upper" | "lower" | false; + /** + * Indicates where named type-only imports should sort. The default order ("inline") sorts imports regardless of if the imports are + * type-only. + * + * Default: `inline` + */ + readonly organizeImportsTypeOrder?: "first" | "last" | "inline"; /** * Indicates whether {@link ReferencesResponseItem.lineText} is supported. */ @@ -8414,6 +8421,7 @@ declare namespace ts { readonly organizeImportsNumericCollation?: boolean; readonly organizeImportsAccentCollation?: boolean; readonly organizeImportsCaseFirst?: "upper" | "lower" | false; + readonly organizeImportsTypeOrder?: "first" | "last" | "inline"; } /** 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 09abb27b97058..670640bb15373 100644 --- a/tests/baselines/reference/api/typescript.d.ts +++ b/tests/baselines/reference/api/typescript.d.ts @@ -4356,6 +4356,7 @@ declare namespace ts { readonly organizeImportsNumericCollation?: boolean; readonly organizeImportsAccentCollation?: boolean; readonly organizeImportsCaseFirst?: "upper" | "lower" | false; + readonly organizeImportsTypeOrder?: "first" | "last" | "inline"; } /** Represents a bigint literal value without requiring bigint support */ interface PseudoBigInt { diff --git a/tests/cases/fourslash/fourslash.ts b/tests/cases/fourslash/fourslash.ts index 66e6226102241..017eab3987bf6 100644 --- a/tests/cases/fourslash/fourslash.ts +++ b/tests/cases/fourslash/fourslash.ts @@ -670,7 +670,7 @@ declare namespace FourSlashInterface { readonly organizeImportsNumericCollation?: boolean; readonly organizeImportsAccentCollation?: boolean; readonly organizeImportsCaseFirst?: "upper" | "lower" | false; - readonly organizeImportsTypeOrder?: "first" | "last" | false; + readonly organizeImportsTypeOrder?: "first" | "last" | "inline"; } interface InlayHintsOptions extends UserPreferences { readonly includeInlayParameterNameHints?: "none" | "literals" | "all"; diff --git a/tests/cases/fourslash/importNameCodeFix_importType7.ts b/tests/cases/fourslash/importNameCodeFix_importType7.ts index 7d28f3019654b..f5d6157f10a54 100644 --- a/tests/cases/fourslash/importNameCodeFix_importType7.ts +++ b/tests/cases/fourslash/importNameCodeFix_importType7.ts @@ -18,7 +18,7 @@ goTo.marker(""); verify.importFixAtPosition([ `import { - SomePig, type SomeInterface, + SomePig, } from "./exports.js"; new SomePig`]); diff --git a/tests/cases/fourslash/organizeImportsType1.ts b/tests/cases/fourslash/organizeImportsType1.ts index c656780447eff..75dde2ec7839a 100644 --- a/tests/cases/fourslash/organizeImportsType1.ts +++ b/tests/cases/fourslash/organizeImportsType1.ts @@ -18,3 +18,28 @@ verify.organizeImports( console.log(A, B, C, D, E);` ); + +verify.organizeImports( +`import { A, type B, C, D, type E } from "foo"; + +console.log(A, B, C, D, E);`, + undefined, + { organizeImportsTypeOrder : "inline" } +); + + +verify.organizeImports( +`import { type B, type E, A, C, D } from "foo"; + +console.log(A, B, C, D, E);`, + undefined, + { organizeImportsTypeOrder : "first" } +); + +verify.organizeImports( +`import { A, C, D, type B, type E } from "foo"; + +console.log(A, B, C, D, E);`, + undefined, + { organizeImportsTypeOrder : "last" } +); \ No newline at end of file diff --git a/tests/cases/fourslash/organizeImportsType2.ts b/tests/cases/fourslash/organizeImportsType2.ts index 1d28edc04ff47..781b5f667e200 100644 --- a/tests/cases/fourslash/organizeImportsType2.ts +++ b/tests/cases/fourslash/organizeImportsType2.ts @@ -5,18 +5,45 @@ // @noUnusedLocals: true // @target: es2018 -//// import { A } from "foo"; -//// import { type B } from "foo"; -//// import { C } from "foo"; -//// import { type E } from "foo"; -//// import { D } from "foo"; -//// -//// console.log(A, B, C, D, E); +//// type A = string; +//// type B = string; +//// const C = "hello"; +//// export { A, type B, C }; verify.organizeImports( -`import { type B, type E, A, C, D } from "foo"; +`type A = string; +type B = string; +const C = "hello"; +export { A, type B, C }; +` +); -console.log(A, B, C, D, E);`, +verify.organizeImports( +`type A = string; +type B = string; +const C = "hello"; +export { A, type B, C }; +`, + undefined, + { organizeImportsTypeOrder : "inline" } +); + +verify.organizeImports( +`type A = string; +type B = string; +const C = "hello"; +export { type B, A, C }; +`, undefined, { organizeImportsTypeOrder : "first" } +); + +verify.organizeImports( +`type A = string; +type B = string; +const C = "hello"; +export { A, C, type B }; +`, + undefined, + { organizeImportsTypeOrder : "last" } ); \ No newline at end of file diff --git a/tests/cases/fourslash/organizeImportsType3.ts b/tests/cases/fourslash/organizeImportsType3.ts deleted file mode 100644 index 85e6d909b033c..0000000000000 --- a/tests/cases/fourslash/organizeImportsType3.ts +++ /dev/null @@ -1,22 +0,0 @@ -/// - -// @allowSyntheticDefaultImports: true -// @moduleResolution: node -// @noUnusedLocals: true -// @target: es2018 - -//// import { A } from "foo"; -//// import { type B } from "foo"; -//// import { C } from "foo"; -//// import { type E } from "foo"; -//// import { D } from "foo"; -//// -//// console.log(A, B, C, D, E); - -verify.organizeImports( -`import { A, C, D, type B, type E } from "foo"; - -console.log(A, B, C, D, E);`, - undefined, - { organizeImportsTypeOrder : "last" } -); \ No newline at end of file diff --git a/tests/cases/fourslash/organizeImportsType4.ts b/tests/cases/fourslash/organizeImportsType4.ts deleted file mode 100644 index 7df135fefb643..0000000000000 --- a/tests/cases/fourslash/organizeImportsType4.ts +++ /dev/null @@ -1,19 +0,0 @@ -/// - -// @allowSyntheticDefaultImports: true -// @moduleResolution: node -// @noUnusedLocals: true -// @target: es2018 - -//// type A = string; -//// type B = string; -//// const C = "hello"; -//// export { A, type B, C }; - -verify.organizeImports( -`type A = string; -type B = string; -const C = "hello"; -export { A, type B, C }; -` -); \ No newline at end of file diff --git a/tests/cases/fourslash/organizeImportsType5.ts b/tests/cases/fourslash/organizeImportsType5.ts deleted file mode 100644 index 5a312b40ead6e..0000000000000 --- a/tests/cases/fourslash/organizeImportsType5.ts +++ /dev/null @@ -1,21 +0,0 @@ -/// - -// @allowSyntheticDefaultImports: true -// @moduleResolution: node -// @noUnusedLocals: true -// @target: es2018 - -//// type A = string; -//// type B = string; -//// const C = "hello"; -//// export { A, type B, C }; - -verify.organizeImports( -`type A = string; -type B = string; -const C = "hello"; -export { type B, A, C }; -`, - undefined, - { organizeImportsTypeOrder : "first" } -); \ No newline at end of file diff --git a/tests/cases/fourslash/organizeImportsType6.ts b/tests/cases/fourslash/organizeImportsType6.ts deleted file mode 100644 index 579c1755c874b..0000000000000 --- a/tests/cases/fourslash/organizeImportsType6.ts +++ /dev/null @@ -1,21 +0,0 @@ -/// - -// @allowSyntheticDefaultImports: true -// @moduleResolution: node -// @noUnusedLocals: true -// @target: es2018 - -//// type A = string; -//// type B = string; -//// const C = "hello"; -//// export { A, type B, C }; - -verify.organizeImports( -`type A = string; -type B = string; -const C = "hello"; -export { A, C, type B }; -`, - undefined, - { organizeImportsTypeOrder : "last" } -); \ No newline at end of file From 3652bce42293a7160a9885421821b93f2e1a2706 Mon Sep 17 00:00:00 2001 From: Isabel Duan Date: Fri, 4 Aug 2023 09:46:19 -0700 Subject: [PATCH 03/14] change detection to account for type ordering --- src/server/protocol.ts | 2 +- src/services/codefixes/importFixes.ts | 11 ++--- src/services/organizeImports.ts | 6 ++- .../reference/api/tsserverlibrary.d.ts | 2 +- tests/cases/fourslash/organizeImportsType3.ts | 48 +++++++++++++++++++ tests/cases/fourslash/organizeImportsType4.ts | 31 ++++++++++++ tests/cases/fourslash/organizeImportsType5.ts | 31 ++++++++++++ tests/cases/fourslash/organizeImportsType6.ts | 40 ++++++++++++++++ tests/cases/fourslash/organizeImportsType7.ts | 40 ++++++++++++++++ 9 files changed, 201 insertions(+), 10 deletions(-) create mode 100644 tests/cases/fourslash/organizeImportsType3.ts create mode 100644 tests/cases/fourslash/organizeImportsType4.ts create mode 100644 tests/cases/fourslash/organizeImportsType5.ts create mode 100644 tests/cases/fourslash/organizeImportsType6.ts create mode 100644 tests/cases/fourslash/organizeImportsType7.ts diff --git a/src/server/protocol.ts b/src/server/protocol.ts index 2a055b62759d8..99de9c2a9514d 100644 --- a/src/server/protocol.ts +++ b/src/server/protocol.ts @@ -3599,7 +3599,7 @@ export interface UserPreferences { */ readonly organizeImportsCaseFirst?: "upper" | "lower" | false; /** - * Indicates where named type-only imports should sort. The default order ("inline") sorts imports regardless of if the imports are + * Indicates where named type-only imports should sort. The default order ("inline") sorts imports regardless of if imports are * type-only. * * Default: `inline` diff --git a/src/services/codefixes/importFixes.ts b/src/services/codefixes/importFixes.ts index fde1683b12e26..dee619c272698 100644 --- a/src/services/codefixes/importFixes.ts +++ b/src/services/codefixes/importFixes.ts @@ -1333,16 +1333,13 @@ function promoteFromTypeOnly(changes: textChanges.ChangeTracker, aliasDeclaratio const newSpecifier = factory.updateImportSpecifier(aliasDeclaration, /*isTypeOnly*/ false, aliasDeclaration.propertyName, aliasDeclaration.name); const comparer = OrganizeImports.getOrganizeImportsComparer(preferences, sortKind === SortKind.CaseInsensitive); const insertionIndex = OrganizeImports.getImportSpecifierInsertionIndex(aliasDeclaration.parent.elements, newSpecifier, comparer); - if (insertionIndex === aliasDeclaration.parent.elements.indexOf(aliasDeclaration)) { - changes.deleteRange(sourceFile, { pos: getTokenPosOfNode(aliasDeclaration.getFirstToken()!), end: getTokenPosOfNode(aliasDeclaration.propertyName ?? aliasDeclaration.name) }); + if (insertionIndex !== aliasDeclaration.parent.elements.indexOf(aliasDeclaration)) { + changes.delete(sourceFile, aliasDeclaration); + changes.insertImportSpecifierAtIndex(sourceFile, newSpecifier, aliasDeclaration.parent, insertionIndex); return aliasDeclaration; } - changes.delete(sourceFile, aliasDeclaration); - changes.insertImportSpecifierAtIndex(sourceFile, newSpecifier, aliasDeclaration.parent, insertionIndex); - } - else { - changes.deleteRange(sourceFile, aliasDeclaration.getFirstToken()!); } + changes.deleteRange(sourceFile, { pos: getTokenPosOfNode(aliasDeclaration.getFirstToken()!), end: getTokenPosOfNode(aliasDeclaration.propertyName ?? aliasDeclaration.name) }); return aliasDeclaration; } else { diff --git a/src/services/organizeImports.ts b/src/services/organizeImports.ts index 2d0036908b7a1..0ae1ecea862af 100644 --- a/src/services/organizeImports.ts +++ b/src/services/organizeImports.ts @@ -715,9 +715,13 @@ class ImportSpecifierSortingCache implements MemoizeCache<[readonly ImportSpecif /** @internal */ export const detectImportSpecifierSorting = memoizeCached((specifiers: readonly ImportSpecifier[], preferences: UserPreferences): SortKind => { - if (!arrayIsSorted(specifiers, (s1, s2) => compareBooleans(s1.isTypeOnly, s2.isTypeOnly))) { + // If types are not sorted as specified, then imports are assumed to be unsorted. + // If there is no type sorting specification, then we move on to case sensitivity detection. + if (preferences.organizeImportsTypeOrder === "last" && !arrayIsSorted(specifiers, (s1, s2) => compareBooleans(s1.isTypeOnly, s2.isTypeOnly)) + || preferences.organizeImportsTypeOrder === "first" && !arrayIsSorted(specifiers, (s1, s2) => compareBooleans(s2.isTypeOnly, s1.isTypeOnly))) { return SortKind.None; } + const collateCaseSensitive = getOrganizeImportsComparer(preferences, /*ignoreCase*/ false); const collateCaseInsensitive = getOrganizeImportsComparer(preferences, /*ignoreCase*/ true); return detectSortCaseSensitivity(specifiers, specifier => specifier.name.text, collateCaseSensitive, collateCaseInsensitive); diff --git a/tests/baselines/reference/api/tsserverlibrary.d.ts b/tests/baselines/reference/api/tsserverlibrary.d.ts index eb047cf0fa386..3e34ddf43a0b2 100644 --- a/tests/baselines/reference/api/tsserverlibrary.d.ts +++ b/tests/baselines/reference/api/tsserverlibrary.d.ts @@ -2892,7 +2892,7 @@ declare namespace ts { */ readonly organizeImportsCaseFirst?: "upper" | "lower" | false; /** - * Indicates where named type-only imports should sort. The default order ("inline") sorts imports regardless of if the imports are + * Indicates where named type-only imports should sort. The default order ("inline") sorts imports regardless of if imports are * type-only. * * Default: `inline` diff --git a/tests/cases/fourslash/organizeImportsType3.ts b/tests/cases/fourslash/organizeImportsType3.ts new file mode 100644 index 0000000000000..1e0e4c845b607 --- /dev/null +++ b/tests/cases/fourslash/organizeImportsType3.ts @@ -0,0 +1,48 @@ +/// + +// Regression test for bug #41417 + +//// import { +//// d, +//// type d as D, +//// type c, +//// c as C, +//// b, +//// b as B, +//// type A, +//// a +//// } from './foo'; +//// console.log(A, a, B, b, c, C, d, D); + +verify.organizeImports( +`import { + type A, + b as B, + c as C, + type d as D, + a, + b, + type c, + d +} from './foo'; +console.log(A, a, B, b, c, C, d, D);`, + /*mode*/ undefined, + { organizeImportsIgnoreCase: false } +); + +verify.organizeImports( +`import { + type A, + a, + b as B, + b, + c as C, + type c, + type d as D, + d +} from './foo'; +console.log(A, a, B, b, c, C, d, D);`, + /*mode*/ undefined, + { organizeImportsIgnoreCase: true } +); + diff --git a/tests/cases/fourslash/organizeImportsType4.ts b/tests/cases/fourslash/organizeImportsType4.ts new file mode 100644 index 0000000000000..cfa29e0c1113b --- /dev/null +++ b/tests/cases/fourslash/organizeImportsType4.ts @@ -0,0 +1,31 @@ +/// + +//// import { type a, A, b } from "foo"; +//// interface Use extends A {} +//// console.log(a, b); + +verify.organizeImports( +`import { type a, A, b } from "foo"; +interface Use extends A {} +console.log(a, b);`); + +verify.organizeImports( +`import { type a, A, b } from "foo"; +interface Use extends A {} +console.log(a, b);`, + /*mode*/ undefined, + { organizeImportsIgnoreCase: "auto" }); + +verify.organizeImports( +`import { type a, A, b } from "foo"; +interface Use extends A {} +console.log(a, b);`, + /*mode*/ undefined, + { organizeImportsIgnoreCase: true }); + +verify.organizeImports( +`import { A, type a, b } from "foo"; +interface Use extends A {} +console.log(a, b);`, + /*mode*/ undefined, + { organizeImportsIgnoreCase: false }); \ No newline at end of file diff --git a/tests/cases/fourslash/organizeImportsType5.ts b/tests/cases/fourslash/organizeImportsType5.ts new file mode 100644 index 0000000000000..35011189c1940 --- /dev/null +++ b/tests/cases/fourslash/organizeImportsType5.ts @@ -0,0 +1,31 @@ +/// + +//// import { a, type A, b } from "foo"; +//// interface Use extends A {} +//// console.log(a, b); + +verify.organizeImports( +`import { a, type A, b } from "foo"; +interface Use extends A {} +console.log(a, b);`); + +verify.organizeImports( +`import { a, type A, b } from "foo"; +interface Use extends A {} +console.log(a, b);`, + /*mode*/ undefined, + { organizeImportsIgnoreCase: "auto" }); + +verify.organizeImports( +`import { a, type A, b } from "foo"; +interface Use extends A {} +console.log(a, b);`, + /*mode*/ undefined, + { organizeImportsIgnoreCase: true }); + +verify.organizeImports( +`import { type A, a, b } from "foo"; +interface Use extends A {} +console.log(a, b);`, + /*mode*/ undefined, + { organizeImportsIgnoreCase: false }); \ No newline at end of file diff --git a/tests/cases/fourslash/organizeImportsType6.ts b/tests/cases/fourslash/organizeImportsType6.ts new file mode 100644 index 0000000000000..73bf5b3fd45db --- /dev/null +++ b/tests/cases/fourslash/organizeImportsType6.ts @@ -0,0 +1,40 @@ +/// + +//// import { type A, type a, b, B } from "foo"; +//// console.log(a, b, A, B); + +verify.organizeImports( +`import { type A, type a, b, B } from "foo"; +console.log(a, b, A, B);`); + +verify.organizeImports( +`import { type A, type a, b, B } from "foo"; +console.log(a, b, A, B);`, + /*mode*/ undefined, + { organizeImportsIgnoreCase: "auto" }); + +verify.organizeImports( +`import { type A, type a, b, B } from "foo"; +console.log(a, b, A, B);`, + /*mode*/ undefined, + { organizeImportsIgnoreCase: "auto", organizeImportsTypeOrder: "first"} +); + +verify.organizeImports( +`import { B, b, type A, type a } from "foo"; +console.log(a, b, A, B);`, + /*mode*/ undefined, + { organizeImportsIgnoreCase: "auto", organizeImportsTypeOrder: "last" } +); + +// verify.organizeImports( +// `import { type A, type a, B, b } from "foo"; +// console.log(a, b, A, B);`, +// /*mode*/ undefined, +// { organizeImportsIgnoreCase: true }); + +verify.organizeImports( +`import { type A, B, type a, b } from "foo"; +console.log(a, b, A, B);`, + /*mode*/ undefined, + { organizeImportsIgnoreCase: false }); \ No newline at end of file diff --git a/tests/cases/fourslash/organizeImportsType7.ts b/tests/cases/fourslash/organizeImportsType7.ts new file mode 100644 index 0000000000000..ecfe356401871 --- /dev/null +++ b/tests/cases/fourslash/organizeImportsType7.ts @@ -0,0 +1,40 @@ +/// + +//// import { type a, type A, b, B } from "foo"; +//// console.log(a, b, A, B); + +verify.organizeImports( +`import { type a, type A, b, B } from "foo"; +console.log(a, b, A, B);`); + +verify.organizeImports( +`import { type a, type A, b, B } from "foo"; +console.log(a, b, A, B);`, + /*mode*/ undefined, + { organizeImportsIgnoreCase: "auto" }); + +verify.organizeImports( +`import { type a, type A, b, B } from "foo"; +console.log(a, b, A, B);`, + /*mode*/ undefined, + { organizeImportsIgnoreCase: "auto", organizeImportsTypeOrder: "first"} +); + +verify.organizeImports( +`import { B, b, type A, type a } from "foo"; +console.log(a, b, A, B);`, + /*mode*/ undefined, + { organizeImportsIgnoreCase: "auto", organizeImportsTypeOrder: "last" } +); + +verify.organizeImports( +`import { type A, type a, B, b } from "foo"; +console.log(a, b, A, B);`, + /*mode*/ undefined, + { organizeImportsIgnoreCase: true }); + +verify.organizeImports( +`import { type A, B, type a, b } from "foo"; +console.log(a, b, A, B);`, + /*mode*/ undefined, + { organizeImportsIgnoreCase: false }); \ No newline at end of file From ee32e438531d742e60c226065a53bca95ffdf583 Mon Sep 17 00:00:00 2001 From: Isabel Duan Date: Fri, 4 Aug 2023 15:13:24 -0700 Subject: [PATCH 04/14] add tests for autoimports --- .../cases/fourslash/autoImportTypeImport1.ts | 41 +++++++++++++++++++ .../cases/fourslash/autoImportTypeImport2.ts | 41 +++++++++++++++++++ 2 files changed, 82 insertions(+) create mode 100644 tests/cases/fourslash/autoImportTypeImport1.ts create mode 100644 tests/cases/fourslash/autoImportTypeImport2.ts diff --git a/tests/cases/fourslash/autoImportTypeImport1.ts b/tests/cases/fourslash/autoImportTypeImport1.ts new file mode 100644 index 0000000000000..727a35d37590d --- /dev/null +++ b/tests/cases/fourslash/autoImportTypeImport1.ts @@ -0,0 +1,41 @@ +/// + +// @verbatimModuleSyntax: true +// @target: esnext + +// @Filename: /foo.ts +//// export const A = 1; +//// export type B = { x: number }; +//// export type C = 1; +//// export class D = { y: string }; + +// @Filename: /test.ts +//// import { A, D, type C } from './foo'; +//// const b: B/**/ | C; +//// console.log(A, D); + +goTo.marker(""); + +verify.importFixAtPosition([ +`import { A, D, type C, type B } from './foo'; +const b: B | C; +console.log(A, D);`], + /*errorCode*/ undefined, + { organizeImportsTypeOrder: "inline" } +); + +verify.importFixAtPosition([ +`import { A, D, type C, type B } from './foo'; +const b: B | C; +console.log(A, D);`], + /*errorCode*/ undefined, + { organizeImportsTypeOrder: "last" } +); + +verify.importFixAtPosition([ +`import { A, D, type C, type B } from './foo'; +const b: B | C; +console.log(A, D);`], + /*errorCode*/ undefined, + { organizeImportsTypeOrder: "first" } +); \ No newline at end of file diff --git a/tests/cases/fourslash/autoImportTypeImport2.ts b/tests/cases/fourslash/autoImportTypeImport2.ts new file mode 100644 index 0000000000000..f98f5bef31f9f --- /dev/null +++ b/tests/cases/fourslash/autoImportTypeImport2.ts @@ -0,0 +1,41 @@ +/// + +// @verbatimModuleSyntax: true +// @target: esnext + +// @Filename: /foo.ts +//// export const A = 1; +//// export type B = { x: number }; +//// export type C = 1; +//// export class D = { y: string }; + +// @Filename: /test.ts +//// import { A, type C, D } from './foo'; +//// const b: B/**/ | C; +//// console.log(A, D); + +goTo.marker(""); + +verify.importFixAtPosition([ +`import { A, type B, type C, D } from './foo'; +const b: B | C; +console.log(A, D);`], + /*errorCode*/ undefined, + { organizeImportsTypeOrder: "inline" } +); + +verify.importFixAtPosition([ +`import { A, type C, D, type B } from './foo'; +const b: B | C; +console.log(A, D);`], + /*errorCode*/ undefined, + { organizeImportsTypeOrder: "last" } +); + +verify.importFixAtPosition([ +`import { A, type C, D, type B } from './foo'; +const b: B | C; +console.log(A, D);`], + /*errorCode*/ undefined, + { organizeImportsTypeOrder: "first" } +); \ No newline at end of file From bb47232b26f9cae4b3d240b38ae322625990364d Mon Sep 17 00:00:00 2001 From: Isabel Duan Date: Mon, 11 Sep 2023 16:43:06 -0700 Subject: [PATCH 05/14] update baselines --- tests/baselines/reference/api/typescript.d.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/baselines/reference/api/typescript.d.ts b/tests/baselines/reference/api/typescript.d.ts index ed74331805bad..5c49fa19b0c3f 100644 --- a/tests/baselines/reference/api/typescript.d.ts +++ b/tests/baselines/reference/api/typescript.d.ts @@ -2893,6 +2893,13 @@ declare namespace ts { * Default: `false` */ readonly organizeImportsCaseFirst?: "upper" | "lower" | false; + /** + * Indicates where named type-only imports should sort. The default order ("inline") sorts imports regardless of if imports are + * type-only. + * + * Default: `inline` + */ + readonly organizeImportsTypeOrder?: "first" | "last" | "inline"; /** * Indicates whether {@link ReferencesResponseItem.lineText} is supported. */ From 44b07dd9a2c1c0db2699e03f4ca6f85aa658d423 Mon Sep 17 00:00:00 2001 From: Isabel Duan Date: Thu, 14 Sep 2023 12:37:59 -0700 Subject: [PATCH 06/14] Merge auto-import-type --- src/services/codefixes/importFixes.ts | 4 +- src/services/organizeImports.ts | 38 +++++- .../cases/fourslash/autoImportTypeImport1.ts | 2 +- .../cases/fourslash/autoImportTypeImport3.ts | 38 ++++++ .../cases/fourslash/autoImportTypeImport4.ts | 111 ++++++++++++++++++ .../cases/fourslash/autoImportTypeImport5.ts | 86 ++++++++++++++ 6 files changed, 273 insertions(+), 6 deletions(-) create mode 100644 tests/cases/fourslash/autoImportTypeImport3.ts create mode 100644 tests/cases/fourslash/autoImportTypeImport4.ts create mode 100644 tests/cases/fourslash/autoImportTypeImport5.ts diff --git a/src/services/codefixes/importFixes.ts b/src/services/codefixes/importFixes.ts index 52143885bc4af..b2c495380dba1 100644 --- a/src/services/codefixes/importFixes.ts +++ b/src/services/codefixes/importFixes.ts @@ -1365,7 +1365,7 @@ function promoteFromTypeOnly(changes: textChanges.ChangeTracker, aliasDeclaratio if (aliasDeclaration.parent.elements.length > 1 && sortKind) { const newSpecifier = factory.updateImportSpecifier(aliasDeclaration, /*isTypeOnly*/ false, aliasDeclaration.propertyName, aliasDeclaration.name); const comparer = OrganizeImports.getOrganizeImportsComparer(preferences, sortKind === SortKind.CaseInsensitive); - const insertionIndex = OrganizeImports.getImportSpecifierInsertionIndex(aliasDeclaration.parent.elements, newSpecifier, comparer); + const insertionIndex = OrganizeImports.getImportSpecifierInsertionIndex(aliasDeclaration.parent.elements, newSpecifier, comparer, preferences); if (insertionIndex !== aliasDeclaration.parent.elements.indexOf(aliasDeclaration)) { changes.delete(sourceFile, aliasDeclaration); changes.insertImportSpecifierAtIndex(sourceFile, newSpecifier, aliasDeclaration.parent, insertionIndex); @@ -1497,7 +1497,7 @@ function doAddExistingFix( // type-only, there's no need to ask for the insertion index - it's 0. const insertionIndex = promoteFromTypeOnly && !spec.isTypeOnly ? 0 - : OrganizeImports.getImportSpecifierInsertionIndex(existingSpecifiers, spec, comparer); + : OrganizeImports.getImportSpecifierInsertionIndex(existingSpecifiers, spec, comparer, preferences); changes.insertImportSpecifierAtIndex(sourceFile, spec, clause.namedBindings as NamedImports, insertionIndex); } } diff --git a/src/services/organizeImports.ts b/src/services/organizeImports.ts index e7150a982a29a..64060181ac4de 100644 --- a/src/services/organizeImports.ts +++ b/src/services/organizeImports.ts @@ -67,6 +67,9 @@ import { UserPreferences, } from "./_namespaces/ts"; +// todo: change to "last", keeping inline for testing purposes +// const DEFAULT_TYPE_ORDER_PREFERENCE = undefined; + /** * Organize imports by: * 1) Removing unused imports @@ -734,12 +737,41 @@ export const detectImportSpecifierSorting = memoizeCached((specifiers: readonly || preferences.organizeImportsTypeOrder === "first" && !arrayIsSorted(specifiers, (s1, s2) => compareBooleans(s2.isTypeOnly, s1.isTypeOnly))) { return SortKind.None; } - const collateCaseSensitive = getOrganizeImportsComparer(preferences, /*ignoreCase*/ false); const collateCaseInsensitive = getOrganizeImportsComparer(preferences, /*ignoreCase*/ true); + + if (preferences.organizeImportsTypeOrder === "first" || preferences.organizeImportsTypeOrder === "last") { + const { regularImports, typeImports } = getCategorizedSpecifiers(specifiers); + const regularCaseSensitivity = regularImports.length + ? detectSortCaseSensitivity(regularImports, specifier => specifier.name.text, collateCaseSensitive, collateCaseInsensitive) + : undefined; + const typeCaseSensitivity = typeImports.length + ? detectSortCaseSensitivity(typeImports, specifier => specifier.name.text ?? "", collateCaseSensitive, collateCaseInsensitive) + : undefined; + if (regularCaseSensitivity === undefined) { + return typeCaseSensitivity ?? SortKind.None; + } + if (typeCaseSensitivity === undefined) { + return regularCaseSensitivity; + } + if (regularCaseSensitivity === SortKind.None || typeCaseSensitivity === SortKind.None) { + return SortKind.None; + } + return typeCaseSensitivity & regularCaseSensitivity; + } + return detectSortCaseSensitivity(specifiers, specifier => specifier.name.text, collateCaseSensitive, collateCaseInsensitive); }, new ImportSpecifierSortingCache()); +function getCategorizedSpecifiers(specifiers: readonly ImportSpecifier[]) { + // assumes type imports are at the end + const firstType = specifiers.findIndex(s => s.isTypeOnly); + return { + regularImports: specifiers.slice(0, firstType), + typeImports: specifiers.slice(firstType), + } +} + /** @internal */ export function getImportDeclarationInsertionIndex(sortedImports: readonly AnyImportOrRequireStatement[], newImport: AnyImportOrRequireStatement, comparer: Comparer) { const index = binarySearch(sortedImports, newImport, identity, (a, b) => compareImportsOrRequireStatements(a, b, comparer)); @@ -747,8 +779,8 @@ export function getImportDeclarationInsertionIndex(sortedImports: readonly AnyIm } /** @internal */ -export function getImportSpecifierInsertionIndex(sortedImports: readonly ImportSpecifier[], newImport: ImportSpecifier, comparer: Comparer) { - const index = binarySearch(sortedImports, newImport, identity, (s1, s2) => compareImportOrExportSpecifiers(s1, s2, comparer)); +export function getImportSpecifierInsertionIndex(sortedImports: readonly ImportSpecifier[], newImport: ImportSpecifier, comparer: Comparer, preferences: UserPreferences) { + const index = binarySearch(sortedImports, newImport, identity, (s1, s2) => compareImportOrExportSpecifiers(s1, s2, comparer, preferences)); return index < 0 ? ~index : index; } diff --git a/tests/cases/fourslash/autoImportTypeImport1.ts b/tests/cases/fourslash/autoImportTypeImport1.ts index 727a35d37590d..1ed34dd5d829e 100644 --- a/tests/cases/fourslash/autoImportTypeImport1.ts +++ b/tests/cases/fourslash/autoImportTypeImport1.ts @@ -25,7 +25,7 @@ console.log(A, D);`], ); verify.importFixAtPosition([ -`import { A, D, type C, type B } from './foo'; +`import { A, D, type B, type C } from './foo'; const b: B | C; console.log(A, D);`], /*errorCode*/ undefined, diff --git a/tests/cases/fourslash/autoImportTypeImport3.ts b/tests/cases/fourslash/autoImportTypeImport3.ts new file mode 100644 index 0000000000000..bb507a2088c2e --- /dev/null +++ b/tests/cases/fourslash/autoImportTypeImport3.ts @@ -0,0 +1,38 @@ +/// + +// @verbatimModuleSyntax: true +// @target: esnext + +// @Filename: /foo.ts +//// export const A = 1; +//// export type B = { x: number }; +//// export type C = 1; +//// export class D = { y: string }; + +// @Filename: /test.ts +//// import { A, type B, type C } from './foo'; +//// const b: B | C; +//// console.log(A, D/**/); + +goTo.marker(""); + +verify.importFixAtPosition([ +`import { A, D, type B, type C } from './foo'; +const b: B | C; +console.log(A, D);`], + /*errorCode*/ undefined, + { organizeImportsIgnoreCase: true, organizeImportsTypeOrder : "last" }); + +// verify.importFixAtPosition([ +// `import { A, type B, type C, D } from './foo'; +// const b: B | C; +// console.log(A, D);`], +// /*errorCode*/ undefined, +// { organizeImportsIgnoreCase: true, organizeImportsTypeOrder : "inline" }); + +// verify.importFixAtPosition([ +// `import { A, type B, type C, D } from './foo'; +// const b: B | C; +// console.log(A, D);`], +// /*errorCode*/ undefined, +// { organizeImportsIgnoreCase: true, organizeImportsTypeOrder : "first" }); \ No newline at end of file diff --git a/tests/cases/fourslash/autoImportTypeImport4.ts b/tests/cases/fourslash/autoImportTypeImport4.ts new file mode 100644 index 0000000000000..7bdea0c97b0e9 --- /dev/null +++ b/tests/cases/fourslash/autoImportTypeImport4.ts @@ -0,0 +1,111 @@ +/// + +// @verbatimModuleSyntax: true +// @target: esnext + +// @Filename: /exports1.ts +//// export const a = 0; +//// export const A = 1; +//// export const b = 2; +//// export const B = 3; +//// export const c = 4; +//// export const C = 5; +//// export type x = 6; +//// export const X = 7; +//// export const Y = 8; +//// export const Z = 9; + +// @Filename: /exports2.ts +//// export const d = 0; +//// export const D = 1; +//// export const e = 2; +//// export const E = 3; + +// @Filename: /index0.ts +//// import { A, B, C } from "./exports1"; +//// a/*0*/; +//// b; + +// @Filename: /index2.ts +//// import { A, B, C, type Y, type Z } from "./exports1"; +//// a/*3*/; +//// b; + +// @Filename: /index1.ts +//// import { A, a, B, b, type Y, type Z } from "./exports1"; +//// import { E } from "./exports2"; +//// d/*1*/ + +// addition of correctly sorted type imports should not affect behavior as shown in autoImportSortCaseSensitivity1.ts +goTo.marker("0"); +verify.importFixAtPosition([ + `import { A, B, C, a } from "./exports1";\na;\nb;`, + `import { A, B, C, b } from "./exports1";\na;\nb;`, +], + /*errorCode*/ undefined, + { organizeImportsTypeOrder : "last" }); +verify.importFixAtPosition([ + `import { a, A, B, C } from "./exports1";\na;\nb;`, + `import { A, b, B, C } from "./exports1";\na;\nb;` +], + /*errorCode*/ undefined, + { organizeImportsIgnoreCase: true, organizeImportsTypeOrder : "last" }); + +goTo.marker("1"); +verify.importFixAtPosition([ + `import { A, B, C, a, type Y, type Z } from "./exports1";\na;\nb;`, + `import { A, B, C, b, type Y, type Z } from "./exports1";\na;\nb;`, +], + /*errorCode*/ undefined, + { organizeImportsTypeOrder : "last" }); +verify.importFixAtPosition([ + `import { a, A, B, C, type Y, type Z } from "./exports1";\na;\nb;`, + `import { A, b, B, C, type Y, type Z } from "./exports1";\na;\nb;` +], + /*errorCode*/ undefined, + { organizeImportsIgnoreCase: true, organizeImportsTypeOrder : "last" }); + +// if we sort inline and sensitive, then all upper case imports should be sorted before any lower case imports +verify.importFixAtPosition([ + `import { A, B, C, type Y, type Z, a } from "./exports1";\na;\nb;`, + `import { A, B, C, type Y, type Z, b } from "./exports1";\na;\nb;`, +], + /*errorCode*/ undefined, + { organizeImportsTypeOrder : "inline" }); +verify.importFixAtPosition([ + `import { a, A, B, C, type Y, type Z } from "./exports1";\na;\nb;`, + `import { A, b, B, C, type Y, type Z } from "./exports1";\na;\nb;` +], + /*errorCode*/ undefined, + { organizeImportsIgnoreCase: true, organizeImportsTypeOrder : "inline" }); + +goTo.marker("2"); +verify.importFixAtPosition([ +`import { A, a, B, b, type Y, type Z } from "./exports1"; +import { d, E } from "./exports2"; +d`, +], + /*errorCode*/ undefined, + { organizeImportsTypeOrder : "last" }); +verify.importFixAtPosition([ +`import { A, a, B, b, type Y, type Z } from "./exports1"; +import { E, d } from "./exports2"; +d` +], + /*errorCode*/ undefined, + { organizeImportsIgnoreCase: false, organizeImportsTypeOrder : "last" }); + +verify.importFixAtPosition([ +`import { A, a, B, b, type Y, type Z } from "./exports1"; +import { d, E } from "./exports2"; +d`, +], + /*errorCode*/ undefined, + { organizeImportsTypeOrder : "last" }); +verify.importFixAtPosition([ +`import { A, a, B, b, type Y, type Z } from "./exports1"; +import { E, d } from "./exports2"; +d` +], + /*errorCode*/ undefined, + { organizeImportsIgnoreCase: false, organizeImportsTypeOrder : "last" }); \ No newline at end of file diff --git a/tests/cases/fourslash/autoImportTypeImport5.ts b/tests/cases/fourslash/autoImportTypeImport5.ts new file mode 100644 index 0000000000000..30e141d3710e7 --- /dev/null +++ b/tests/cases/fourslash/autoImportTypeImport5.ts @@ -0,0 +1,86 @@ +/// + +// @verbatimModuleSyntax: true +// @target: esnext + +// @Filename: /exports1.ts +//// export const a = 0; +//// export const A = 1; +//// export const b = 2; +//// export const B = 3; +//// export const c = 4; +//// export const C = 5; +//// export type x = 6; +//// export const X = 7; +//// export type y = 8 +//// export const Y = 9; +//// export const Z = 10; + +// @Filename: /exports2.ts +//// export const d = 0; +//// export const D = 1; +//// export const e = 2; +//// export const E = 3; + +// @Filename: /index0.ts +//// import { type X, type Y, type Z } from "./exports1"; +//// const foo: x/*0*/; +//// const bar: y; + +// @Filename: /index1.ts +//// import { A, B, type X, type Y, type Z } from "./exports1"; +//// const foo: x/*3*/; +//// const bar: y; + +// addition of correctly sorted regular imports should not affect correctly sorted type imports +goTo.marker("0"); +verify.importFixAtPosition([ + `import { type X, type Y, type Z, type x } from "./exports1";\nconst foo: x;\nconst bar: y;`, + `import { type X, type Y, type Z, type y } from "./exports1";\nconst foo: x;\nconst bar: y;`, +], + /*errorCode*/ undefined, + { organizeImportsTypeOrder : "last" }); +verify.importFixAtPosition([ + `import { type x, type X, type Y, type Z } from "./exports1";\nconst foo: x;\nconst bar: y;`, + `import { type X, type y, type Y, type Z } from "./exports1";\nconst foo: x;\nconst bar: y;`, +], + /*errorCode*/ undefined, + { organizeImportsIgnoreCase: true, organizeImportsTypeOrder : "last" }); +verify.importFixAtPosition([ + `import { type X, type Y, type Z, type x } from "./exports1";\nconst foo: x;\nconst bar: y;`, + `import { type X, type Y, type Z, type y } from "./exports1";\nconst foo: x;\nconst bar: y;`, +], + /*errorCode*/ undefined, + { organizeImportsTypeOrder : "inline" }); +verify.importFixAtPosition([ + `import { type x, type X, type Y, type Z } from "./exports1";\nconst foo: x;\nconst bar: y;`, + `import { type X, type y, type Y, type Z } from "./exports1";\nconst foo: x;\nconst bar: y;`, +], + /*errorCode*/ undefined, + { organizeImportsIgnoreCase: true, organizeImportsTypeOrder : "inline" }); +verify.importFixAtPosition([ + `import { type X, type Y, type Z, type x } from "./exports1";\nconst foo: x;\nconst bar: y;`, + `import { type X, type Y, type Z, type y } from "./exports1";\nconst foo: x;\nconst bar: y;`, +], + /*errorCode*/ undefined, + { organizeImportsTypeOrder : "first" }); +verify.importFixAtPosition([ + `import { type x, type X, type Y, type Z } from "./exports1";\nconst foo: x;\nconst bar: y;`, + `import { type X, type y, type Y, type Z } from "./exports1";\nconst foo: x;\nconst bar: y;`, +], + /*errorCode*/ undefined, + { organizeImportsIgnoreCase: true, organizeImportsTypeOrder : "first" }); + +goTo.marker("1"); +verify.importFixAtPosition([ + `import { A, B, type X, type Y, type Z, type x } from "./exports1";\nconst foo: x;\nconst bar: y;`, + `import { A, B, type X, type Y, type Z, type y } from "./exports1";\nconst foo: x;\nconst bar: y;`, +], + /*errorCode*/ undefined, + { organizeImportsTypeOrder : "last" }); +verify.importFixAtPosition([ + `import { A, B, type x, type X, type Y, type Z } from "./exports1";\nconst foo: x;\nconst bar: y;`, + `import { A, B, type X, type y, type Y, type Z } from "./exports1";\nconst foo: x;\nconst bar: y;`, +], + /*errorCode*/ undefined, + { organizeImportsIgnoreCase: true, organizeImportsTypeOrder : "last" }); \ No newline at end of file From 4a83f8c1851652a601333d84cc250d502f01efeb Mon Sep 17 00:00:00 2001 From: Isabel Duan Date: Tue, 3 Oct 2023 14:27:00 -0700 Subject: [PATCH 07/14] change default order to "last" --- src/server/protocol.ts | 6 +-- src/services/organizeImports.ts | 30 +++++++++------ tests/baselines/reference/api/typescript.d.ts | 6 +-- .../cases/fourslash/autoImportTypeImport4.ts | 8 ++-- .../cases/fourslash/autoImportTypeImport5.ts | 2 +- tests/cases/fourslash/organizeImportsType1.ts | 3 +- tests/cases/fourslash/organizeImportsType2.ts | 3 +- tests/cases/fourslash/organizeImportsType3.ts | 4 +- tests/cases/fourslash/organizeImportsType4.ts | 10 +++-- tests/cases/fourslash/organizeImportsType5.ts | 10 +++-- tests/cases/fourslash/organizeImportsType6.ts | 37 +++++++++++++------ tests/cases/fourslash/organizeImportsType7.ts | 29 ++++++++------- 12 files changed, 87 insertions(+), 61 deletions(-) diff --git a/src/server/protocol.ts b/src/server/protocol.ts index 18b41648d3902..dec7f7ffa60e3 100644 --- a/src/server/protocol.ts +++ b/src/server/protocol.ts @@ -3595,12 +3595,12 @@ export interface UserPreferences { */ readonly organizeImportsCaseFirst?: "upper" | "lower" | false; /** - * Indicates where named type-only imports should sort. The default order ("inline") sorts imports regardless of if imports are + * Indicates where named type-only imports should sort. "inline" sorts imports without regard to if the import is * type-only. * - * Default: `inline` + * Default: `last` */ - readonly organizeImportsTypeOrder?: "first" | "last" | "inline"; + readonly organizeImportsTypeOrder?: "last" | "first" | "inline"; /** * Indicates whether {@link ReferencesResponseItem.lineText} is supported. diff --git a/src/services/organizeImports.ts b/src/services/organizeImports.ts index 64060181ac4de..3419d46664131 100644 --- a/src/services/organizeImports.ts +++ b/src/services/organizeImports.ts @@ -595,10 +595,10 @@ export function compareImportOrExportSpecifiers { // If types are not sorted as specified, then imports are assumed to be unsorted. - // If there is no type sorting specification, then we move on to case sensitivity detection. - if (preferences.organizeImportsTypeOrder === "last" && !arrayIsSorted(specifiers, (s1, s2) => compareBooleans(s1.isTypeOnly, s2.isTypeOnly)) - || preferences.organizeImportsTypeOrder === "first" && !arrayIsSorted(specifiers, (s1, s2) => compareBooleans(s2.isTypeOnly, s1.isTypeOnly))) { + // If there is no type sorting specification, we default to "last" and move on to case sensitivity detection. + if (preferences.organizeImportsTypeOrder === "first" && !arrayIsSorted(specifiers, (s1, s2) => compareBooleans(s2.isTypeOnly, s1.isTypeOnly)) + || preferences.organizeImportsTypeOrder !== "inline" && !arrayIsSorted(specifiers, (s1, s2) => compareBooleans(s1.isTypeOnly, s2.isTypeOnly))) { return SortKind.None; } const collateCaseSensitive = getOrganizeImportsComparer(preferences, /*ignoreCase*/ false); const collateCaseInsensitive = getOrganizeImportsComparer(preferences, /*ignoreCase*/ true); - if (preferences.organizeImportsTypeOrder === "first" || preferences.organizeImportsTypeOrder === "last") { + if (preferences.organizeImportsTypeOrder !== "inline") { const { regularImports, typeImports } = getCategorizedSpecifiers(specifiers); const regularCaseSensitivity = regularImports.length ? detectSortCaseSensitivity(regularImports, specifier => specifier.name.text, collateCaseSensitive, collateCaseInsensitive) @@ -760,16 +760,22 @@ export const detectImportSpecifierSorting = memoizeCached((specifiers: readonly return typeCaseSensitivity & regularCaseSensitivity; } + // else inline return detectSortCaseSensitivity(specifiers, specifier => specifier.name.text, collateCaseSensitive, collateCaseInsensitive); }, new ImportSpecifierSortingCache()); function getCategorizedSpecifiers(specifiers: readonly ImportSpecifier[]) { - // assumes type imports are at the end - const firstType = specifiers.findIndex(s => s.isTypeOnly); - return { - regularImports: specifiers.slice(0, firstType), - typeImports: specifiers.slice(firstType), + const regularImports: ImportSpecifier[] = []; + const typeImports: ImportSpecifier[] = []; + for (const s of specifiers) { + if (s.isTypeOnly) { + typeImports.push(s); + } + else { + regularImports.push(s); + } } + return { regularImports, typeImports }; } /** @internal */ diff --git a/tests/baselines/reference/api/typescript.d.ts b/tests/baselines/reference/api/typescript.d.ts index 5c49fa19b0c3f..694da0b5e358d 100644 --- a/tests/baselines/reference/api/typescript.d.ts +++ b/tests/baselines/reference/api/typescript.d.ts @@ -2894,12 +2894,12 @@ declare namespace ts { */ readonly organizeImportsCaseFirst?: "upper" | "lower" | false; /** - * Indicates where named type-only imports should sort. The default order ("inline") sorts imports regardless of if imports are + * Indicates where named type-only imports should sort. "inline" sorts imports without regard to if the import is * type-only. * - * Default: `inline` + * Default: `last` */ - readonly organizeImportsTypeOrder?: "first" | "last" | "inline"; + readonly organizeImportsTypeOrder?: "last" | "first" | "inline"; /** * Indicates whether {@link ReferencesResponseItem.lineText} is supported. */ diff --git a/tests/cases/fourslash/autoImportTypeImport4.ts b/tests/cases/fourslash/autoImportTypeImport4.ts index 7bdea0c97b0e9..82dff821438f7 100644 --- a/tests/cases/fourslash/autoImportTypeImport4.ts +++ b/tests/cases/fourslash/autoImportTypeImport4.ts @@ -26,15 +26,15 @@ //// a/*0*/; //// b; -// @Filename: /index2.ts +// @Filename: /index1.ts //// import { A, B, C, type Y, type Z } from "./exports1"; -//// a/*3*/; +//// a/*1*/; //// b; -// @Filename: /index1.ts +// @Filename: /index2.ts //// import { A, a, B, b, type Y, type Z } from "./exports1"; //// import { E } from "./exports2"; -//// d/*1*/ +//// d/*2*/ // addition of correctly sorted type imports should not affect behavior as shown in autoImportSortCaseSensitivity1.ts goTo.marker("0"); diff --git a/tests/cases/fourslash/autoImportTypeImport5.ts b/tests/cases/fourslash/autoImportTypeImport5.ts index 30e141d3710e7..a0800d3f236f0 100644 --- a/tests/cases/fourslash/autoImportTypeImport5.ts +++ b/tests/cases/fourslash/autoImportTypeImport5.ts @@ -29,7 +29,7 @@ // @Filename: /index1.ts //// import { A, B, type X, type Y, type Z } from "./exports1"; -//// const foo: x/*3*/; +//// const foo: x/*1*/; //// const bar: y; // addition of correctly sorted regular imports should not affect correctly sorted type imports diff --git a/tests/cases/fourslash/organizeImportsType1.ts b/tests/cases/fourslash/organizeImportsType1.ts index 75dde2ec7839a..998cb2f041123 100644 --- a/tests/cases/fourslash/organizeImportsType1.ts +++ b/tests/cases/fourslash/organizeImportsType1.ts @@ -13,8 +13,9 @@ //// //// console.log(A, B, C, D, E); +// default behavior is "last" verify.organizeImports( -`import { A, type B, C, D, type E } from "foo"; +`import { A, C, D, type B, type E } from "foo"; console.log(A, B, C, D, E);` ); diff --git a/tests/cases/fourslash/organizeImportsType2.ts b/tests/cases/fourslash/organizeImportsType2.ts index 781b5f667e200..57b2b4db26950 100644 --- a/tests/cases/fourslash/organizeImportsType2.ts +++ b/tests/cases/fourslash/organizeImportsType2.ts @@ -10,11 +10,12 @@ //// const C = "hello"; //// export { A, type B, C }; +// default behavior is equivalent to "last" verify.organizeImports( `type A = string; type B = string; const C = "hello"; -export { A, type B, C }; +export { A, C, type B }; ` ); diff --git a/tests/cases/fourslash/organizeImportsType3.ts b/tests/cases/fourslash/organizeImportsType3.ts index 1e0e4c845b607..4a2ad7ca4fc4e 100644 --- a/tests/cases/fourslash/organizeImportsType3.ts +++ b/tests/cases/fourslash/organizeImportsType3.ts @@ -27,7 +27,7 @@ verify.organizeImports( } from './foo'; console.log(A, a, B, b, c, C, d, D);`, /*mode*/ undefined, - { organizeImportsIgnoreCase: false } + { organizeImportsIgnoreCase: false, organizeImportsTypeOrder: "inline" } ); verify.organizeImports( @@ -43,6 +43,6 @@ verify.organizeImports( } from './foo'; console.log(A, a, B, b, c, C, d, D);`, /*mode*/ undefined, - { organizeImportsIgnoreCase: true } + { organizeImportsIgnoreCase: true, organizeImportsTypeOrder: "inline" } ); diff --git a/tests/cases/fourslash/organizeImportsType4.ts b/tests/cases/fourslash/organizeImportsType4.ts index cfa29e0c1113b..7e287c65dee54 100644 --- a/tests/cases/fourslash/organizeImportsType4.ts +++ b/tests/cases/fourslash/organizeImportsType4.ts @@ -7,25 +7,27 @@ verify.organizeImports( `import { type a, A, b } from "foo"; interface Use extends A {} -console.log(a, b);`); +console.log(a, b);`, + /*mode*/ undefined, + { organizeImportsTypeOrder: "inline" }); verify.organizeImports( `import { type a, A, b } from "foo"; interface Use extends A {} console.log(a, b);`, /*mode*/ undefined, - { organizeImportsIgnoreCase: "auto" }); + { organizeImportsIgnoreCase: "auto", organizeImportsTypeOrder: "inline" }); verify.organizeImports( `import { type a, A, b } from "foo"; interface Use extends A {} console.log(a, b);`, /*mode*/ undefined, - { organizeImportsIgnoreCase: true }); + { organizeImportsIgnoreCase: true, organizeImportsTypeOrder: "inline" }); verify.organizeImports( `import { A, type a, b } from "foo"; interface Use extends A {} console.log(a, b);`, /*mode*/ undefined, - { organizeImportsIgnoreCase: false }); \ No newline at end of file + { organizeImportsIgnoreCase: false, organizeImportsTypeOrder: "inline" }); \ No newline at end of file diff --git a/tests/cases/fourslash/organizeImportsType5.ts b/tests/cases/fourslash/organizeImportsType5.ts index 35011189c1940..ca594cbac743c 100644 --- a/tests/cases/fourslash/organizeImportsType5.ts +++ b/tests/cases/fourslash/organizeImportsType5.ts @@ -7,25 +7,27 @@ verify.organizeImports( `import { a, type A, b } from "foo"; interface Use extends A {} -console.log(a, b);`); +console.log(a, b);`, + /*mode*/ undefined, + { organizeImportsTypeOrder: "inline" }); verify.organizeImports( `import { a, type A, b } from "foo"; interface Use extends A {} console.log(a, b);`, /*mode*/ undefined, - { organizeImportsIgnoreCase: "auto" }); + { organizeImportsIgnoreCase: "auto", organizeImportsTypeOrder: "inline" }); verify.organizeImports( `import { a, type A, b } from "foo"; interface Use extends A {} console.log(a, b);`, /*mode*/ undefined, - { organizeImportsIgnoreCase: true }); + { organizeImportsIgnoreCase: true, organizeImportsTypeOrder: "inline"}); verify.organizeImports( `import { type A, a, b } from "foo"; interface Use extends A {} console.log(a, b);`, /*mode*/ undefined, - { organizeImportsIgnoreCase: false }); \ No newline at end of file + { organizeImportsIgnoreCase: false, organizeImportsTypeOrder: "inline" }); \ No newline at end of file diff --git a/tests/cases/fourslash/organizeImportsType6.ts b/tests/cases/fourslash/organizeImportsType6.ts index 73bf5b3fd45db..a6cb93ef704a1 100644 --- a/tests/cases/fourslash/organizeImportsType6.ts +++ b/tests/cases/fourslash/organizeImportsType6.ts @@ -3,21 +3,25 @@ //// import { type A, type a, b, B } from "foo"; //// console.log(a, b, A, B); -verify.organizeImports( -`import { type A, type a, b, B } from "foo"; -console.log(a, b, A, B);`); +// verify.organizeImports( +// `import { B, b, type A, type a } from "foo"; +// console.log(a, b, A, B);`, +// /*mode*/ undefined, +// { organizeImportsIgnoreCase: "auto", organizeImportsTypeOrder: "last" } +// ); verify.organizeImports( `import { type A, type a, b, B } from "foo"; console.log(a, b, A, B);`, /*mode*/ undefined, - { organizeImportsIgnoreCase: "auto" }); + { organizeImportsIgnoreCase: "auto", organizeImportsTypeOrder: "inline" } +); verify.organizeImports( `import { type A, type a, b, B } from "foo"; console.log(a, b, A, B);`, /*mode*/ undefined, - { organizeImportsIgnoreCase: "auto", organizeImportsTypeOrder: "first"} + { organizeImportsIgnoreCase: "auto", organizeImportsTypeOrder: "first" } ); verify.organizeImports( @@ -27,14 +31,23 @@ console.log(a, b, A, B);`, { organizeImportsIgnoreCase: "auto", organizeImportsTypeOrder: "last" } ); -// verify.organizeImports( -// `import { type A, type a, B, b } from "foo"; -// console.log(a, b, A, B);`, -// /*mode*/ undefined, -// { organizeImportsIgnoreCase: true }); +verify.organizeImports( +`import { B, b, type A, type a } from "foo"; +console.log(a, b, A, B);`, + /*mode*/ undefined, + { organizeImportsIgnoreCase: "auto" } +); + +verify.organizeImports( +`import { B, b, type A, type a } from "foo"; +console.log(a, b, A, B);`, + /*mode*/ undefined, + { organizeImportsIgnoreCase: true } +); verify.organizeImports( -`import { type A, B, type a, b } from "foo"; +`import { B, b, type A, type a } from "foo"; console.log(a, b, A, B);`, /*mode*/ undefined, - { organizeImportsIgnoreCase: false }); \ No newline at end of file + { organizeImportsIgnoreCase: false } +); \ No newline at end of file diff --git a/tests/cases/fourslash/organizeImportsType7.ts b/tests/cases/fourslash/organizeImportsType7.ts index ecfe356401871..f80799fcbe3c4 100644 --- a/tests/cases/fourslash/organizeImportsType7.ts +++ b/tests/cases/fourslash/organizeImportsType7.ts @@ -3,15 +3,16 @@ //// import { type a, type A, b, B } from "foo"; //// console.log(a, b, A, B); -verify.organizeImports( -`import { type a, type A, b, B } from "foo"; -console.log(a, b, A, B);`); +// verify.organizeImports( +// `import { b, B, type a, type A } from "foo"; +// console.log(a, b, A, B);` +// ); verify.organizeImports( `import { type a, type A, b, B } from "foo"; console.log(a, b, A, B);`, /*mode*/ undefined, - { organizeImportsIgnoreCase: "auto" }); + { organizeImportsIgnoreCase: "auto", organizeImportsTypeOrder: "inline" }); verify.organizeImports( `import { type a, type A, b, B } from "foo"; @@ -27,14 +28,14 @@ console.log(a, b, A, B);`, { organizeImportsIgnoreCase: "auto", organizeImportsTypeOrder: "last" } ); -verify.organizeImports( -`import { type A, type a, B, b } from "foo"; -console.log(a, b, A, B);`, - /*mode*/ undefined, - { organizeImportsIgnoreCase: true }); +// verify.organizeImports( +// `import { b, B, type a, type A } from "foo"; +// console.log(a, b, A, B);`, +// /*mode*/ undefined, +// { organizeImportsIgnoreCase: true }); -verify.organizeImports( -`import { type A, B, type a, b } from "foo"; -console.log(a, b, A, B);`, - /*mode*/ undefined, - { organizeImportsIgnoreCase: false }); \ No newline at end of file +// verify.organizeImports( +// `import { B, b, type A, type a } from "foo"; +// console.log(a, b, A, B);`, +// /*mode*/ undefined, +// { organizeImportsIgnoreCase: false }); \ No newline at end of file From 489e92aa9bce29dc1c9fffd810411882076feea2 Mon Sep 17 00:00:00 2001 From: Isabel Duan Date: Mon, 8 Jan 2024 10:27:21 -0800 Subject: [PATCH 08/14] add types to detection, adjust tests accordingly --- src/services/organizeImports.ts | 32 +++++--- .../unittests/services/organizeImports.ts | 4 +- .../importNameCodeFix_importType10.ts | 27 +++++++ .../importNameCodeFix_importType7.ts | 2 +- tests/cases/fourslash/organizeImportsType3.ts | 19 ----- tests/cases/fourslash/organizeImportsType4.ts | 52 ++++++------- tests/cases/fourslash/organizeImportsType5.ts | 62 +++++++-------- tests/cases/fourslash/organizeImportsType6.ts | 61 ++++++--------- tests/cases/fourslash/organizeImportsType7.ts | 77 +++++++++---------- tests/cases/fourslash/organizeImportsType8.ts | 53 +++++++++++++ tests/cases/fourslash/organizeImportsType9.ts | 48 ++++++++++++ 11 files changed, 264 insertions(+), 173 deletions(-) create mode 100644 tests/cases/fourslash/importNameCodeFix_importType10.ts create mode 100644 tests/cases/fourslash/organizeImportsType8.ts create mode 100644 tests/cases/fourslash/organizeImportsType9.ts diff --git a/src/services/organizeImports.ts b/src/services/organizeImports.ts index 3419d46664131..acf8211981165 100644 --- a/src/services/organizeImports.ts +++ b/src/services/organizeImports.ts @@ -23,6 +23,7 @@ import { flatMap, formatting, getNewLineOrDefaultFromHost, + getStringComparer, getUILocale, group, Identifier, @@ -67,9 +68,6 @@ import { UserPreferences, } from "./_namespaces/ts"; -// todo: change to "last", keeping inline for testing purposes -// const DEFAULT_TYPE_ORDER_PREFERENCE = undefined; - /** * Organize imports by: * 1) Removing unused imports @@ -107,8 +105,7 @@ export function organizeImports( // Exports are always used if (mode !== OrganizeImportsMode.RemoveUnused) { // All of the old ExportDeclarations in the file, in syntactic order. - getTopLevelExportGroups(sourceFile).forEach(exportGroupDecl => - organizeImportsWorker(exportGroupDecl, group => coalesceExportsWorker(group, comparer, preferences))); + getTopLevelExportGroups(sourceFile).forEach(exportGroupDecl => organizeImportsWorker(exportGroupDecl, group => coalesceExportsWorker(group, comparer, preferences))); } for (const ambientModule of sourceFile.statements.filter(isAmbientModule)) { @@ -379,7 +376,7 @@ function coalesceImportsWorker(importGroup: readonly ImportDeclaration[], compar const sortedImportSpecifiers = factory.createNodeArray( sortSpecifiers(newImportSpecifiers, comparer, preferences), - firstNamedImport?.importClause.namedBindings.elements.hasTrailingComma + firstNamedImport?.importClause.namedBindings.elements.hasTrailingComma, ); const newNamedImports = sortedImportSpecifiers.length === 0 @@ -592,7 +589,7 @@ function sortSpecifiers(specifiers: readonly /** @internal */ export function compareImportOrExportSpecifiers(s1: T, s2: T, comparer: Comparer, preferences?: UserPreferences): Comparison { - switch (preferences?.organizeImportsTypeOrder){ + switch (preferences?.organizeImportsTypeOrder) { case "first": return compareBooleans(s2.isTypeOnly, s1.isTypeOnly) || comparer(s1.name.text, s2.name.text); case "inline": @@ -733,10 +730,25 @@ class ImportSpecifierSortingCache implements MemoizeCache<[readonly ImportSpecif export const detectImportSpecifierSorting = memoizeCached((specifiers: readonly ImportSpecifier[], preferences: UserPreferences): SortKind => { // If types are not sorted as specified, then imports are assumed to be unsorted. // If there is no type sorting specification, we default to "last" and move on to case sensitivity detection. - if (preferences.organizeImportsTypeOrder === "first" && !arrayIsSorted(specifiers, (s1, s2) => compareBooleans(s2.isTypeOnly, s1.isTypeOnly)) - || preferences.organizeImportsTypeOrder !== "inline" && !arrayIsSorted(specifiers, (s1, s2) => compareBooleans(s1.isTypeOnly, s2.isTypeOnly))) { - return SortKind.None; + switch (preferences.organizeImportsTypeOrder) { + case "first": + if (!arrayIsSorted(specifiers, (s1, s2) => compareBooleans(s2.isTypeOnly, s1.isTypeOnly))) return SortKind.None; + break; + case "inline": + if ( + !arrayIsSorted(specifiers, (s1, s2) => { + const comparer = getStringComparer(/*ignoreCase*/ true); + return comparer(s1.name.text, s2.name.text); + }) + ) { + return SortKind.None; + } + break; + default: + if (!arrayIsSorted(specifiers, (s1, s2) => compareBooleans(s1.isTypeOnly, s2.isTypeOnly))) return SortKind.None; + break; } + const collateCaseSensitive = getOrganizeImportsComparer(preferences, /*ignoreCase*/ false); const collateCaseInsensitive = getOrganizeImportsComparer(preferences, /*ignoreCase*/ true); diff --git a/src/testRunner/unittests/services/organizeImports.ts b/src/testRunner/unittests/services/organizeImports.ts index c81587a69981f..7c363d7f5dcc4 100644 --- a/src/testRunner/unittests/services/organizeImports.ts +++ b/src/testRunner/unittests/services/organizeImports.ts @@ -241,9 +241,9 @@ describe("unittests:: services:: organizeImports", () => { assertListEqual(actualCoalescedExports, expectedCoalescedExports); }); - it("Sort specifiers - type-only", () => { + it("Sort specifiers - type-only-inline", () => { const sortedImports = parseImports(`import { type z, y, type x, c, type b, a } from "lib";`); - const actualCoalescedImports = ts.OrganizeImports.coalesceImports(sortedImports, /*ignoreCase*/ true); + const actualCoalescedImports = ts.OrganizeImports.coalesceImports(sortedImports, /*ignoreCase*/ true, ts.getSourceFileOfNode(sortedImports[0]), { organizeImportsTypeOrder: "inline" }); const expectedCoalescedImports = parseImports(`import { a, type b, c, type x, y, type z } from "lib";`); assertListEqual(actualCoalescedImports, expectedCoalescedImports); }); diff --git a/tests/cases/fourslash/importNameCodeFix_importType10.ts b/tests/cases/fourslash/importNameCodeFix_importType10.ts new file mode 100644 index 0000000000000..2c41364645105 --- /dev/null +++ b/tests/cases/fourslash/importNameCodeFix_importType10.ts @@ -0,0 +1,27 @@ +/// + +// @module: es2015 + +// sorting and multiline imports and trailing commas, oh my + +// @Filename: /exports.ts +//// export interface SomeInterface {} +//// export class SomePig {} + +// @Filename: /a.ts +//// import { +//// type SomeInterface, +//// type SomePig, +//// } from "./exports.js"; +//// new SomePig/**/ + +goTo.marker(""); +verify.importFixAtPosition([ +`import { + type SomeInterface, + SomePig, +} from "./exports.js"; +new SomePig`], + /*errorCode*/ undefined, + { organizeImportsTypeOrder: "inline" } +); diff --git a/tests/cases/fourslash/importNameCodeFix_importType7.ts b/tests/cases/fourslash/importNameCodeFix_importType7.ts index f5d6157f10a54..7d28f3019654b 100644 --- a/tests/cases/fourslash/importNameCodeFix_importType7.ts +++ b/tests/cases/fourslash/importNameCodeFix_importType7.ts @@ -18,7 +18,7 @@ goTo.marker(""); verify.importFixAtPosition([ `import { - type SomeInterface, SomePig, + type SomeInterface, } from "./exports.js"; new SomePig`]); diff --git a/tests/cases/fourslash/organizeImportsType3.ts b/tests/cases/fourslash/organizeImportsType3.ts index 4a2ad7ca4fc4e..4ed9a24f04cb5 100644 --- a/tests/cases/fourslash/organizeImportsType3.ts +++ b/tests/cases/fourslash/organizeImportsType3.ts @@ -1,7 +1,5 @@ /// -// Regression test for bug #41417 - //// import { //// d, //// type d as D, @@ -29,20 +27,3 @@ console.log(A, a, B, b, c, C, d, D);`, /*mode*/ undefined, { organizeImportsIgnoreCase: false, organizeImportsTypeOrder: "inline" } ); - -verify.organizeImports( -`import { - type A, - a, - b as B, - b, - c as C, - type c, - type d as D, - d -} from './foo'; -console.log(A, a, B, b, c, C, d, D);`, - /*mode*/ undefined, - { organizeImportsIgnoreCase: true, organizeImportsTypeOrder: "inline" } -); - diff --git a/tests/cases/fourslash/organizeImportsType4.ts b/tests/cases/fourslash/organizeImportsType4.ts index 7e287c65dee54..da76b5f17bbea 100644 --- a/tests/cases/fourslash/organizeImportsType4.ts +++ b/tests/cases/fourslash/organizeImportsType4.ts @@ -1,33 +1,29 @@ /// -//// import { type a, A, b } from "foo"; -//// interface Use extends A {} -//// console.log(a, b); +//// import { +//// d, +//// type d as D, +//// type c, +//// c as C, +//// b, +//// b as B, +//// type A, +//// a +//// } from './foo'; +//// console.log(A, a, B, b, c, C, d, D); verify.organizeImports( -`import { type a, A, b } from "foo"; -interface Use extends A {} -console.log(a, b);`, +`import { + type A, + a, + b, + b as B, + type c, + c as C, + d, + type d as D +} from './foo'; +console.log(A, a, B, b, c, C, d, D);`, /*mode*/ undefined, - { organizeImportsTypeOrder: "inline" }); - -verify.organizeImports( -`import { type a, A, b } from "foo"; -interface Use extends A {} -console.log(a, b);`, - /*mode*/ undefined, - { organizeImportsIgnoreCase: "auto", organizeImportsTypeOrder: "inline" }); - -verify.organizeImports( -`import { type a, A, b } from "foo"; -interface Use extends A {} -console.log(a, b);`, - /*mode*/ undefined, - { organizeImportsIgnoreCase: true, organizeImportsTypeOrder: "inline" }); - -verify.organizeImports( -`import { A, type a, b } from "foo"; -interface Use extends A {} -console.log(a, b);`, - /*mode*/ undefined, - { organizeImportsIgnoreCase: false, organizeImportsTypeOrder: "inline" }); \ No newline at end of file + { organizeImportsIgnoreCase: true, organizeImportsTypeOrder: "inline" } +); diff --git a/tests/cases/fourslash/organizeImportsType5.ts b/tests/cases/fourslash/organizeImportsType5.ts index ca594cbac743c..1f8bb8dd602a7 100644 --- a/tests/cases/fourslash/organizeImportsType5.ts +++ b/tests/cases/fourslash/organizeImportsType5.ts @@ -1,33 +1,29 @@ -/// - -//// import { a, type A, b } from "foo"; -//// interface Use extends A {} -//// console.log(a, b); - -verify.organizeImports( -`import { a, type A, b } from "foo"; -interface Use extends A {} -console.log(a, b);`, - /*mode*/ undefined, - { organizeImportsTypeOrder: "inline" }); - -verify.organizeImports( -`import { a, type A, b } from "foo"; -interface Use extends A {} -console.log(a, b);`, - /*mode*/ undefined, - { organizeImportsIgnoreCase: "auto", organizeImportsTypeOrder: "inline" }); - -verify.organizeImports( -`import { a, type A, b } from "foo"; -interface Use extends A {} -console.log(a, b);`, - /*mode*/ undefined, - { organizeImportsIgnoreCase: true, organizeImportsTypeOrder: "inline"}); - -verify.organizeImports( -`import { type A, a, b } from "foo"; -interface Use extends A {} -console.log(a, b);`, - /*mode*/ undefined, - { organizeImportsIgnoreCase: false, organizeImportsTypeOrder: "inline" }); \ No newline at end of file +/// + +//// import { +//// d, +//// type d as D, +//// type c, +//// c as C, +//// b, +//// b as B, +//// type A, +//// a +//// } from './foo'; +//// console.log(A, a, B, b, c, C, d, D); + +verify.organizeImports( +`import { + type A, + b as B, + c as C, + type d as D, + a, + b, + type c, + d +} from './foo'; +console.log(A, a, B, b, c, C, d, D);`, + /*mode*/ undefined, + { organizeImportsIgnoreCase: "auto", organizeImportsTypeOrder: "inline" } +); diff --git a/tests/cases/fourslash/organizeImportsType6.ts b/tests/cases/fourslash/organizeImportsType6.ts index a6cb93ef704a1..c38fc504e07b1 100644 --- a/tests/cases/fourslash/organizeImportsType6.ts +++ b/tests/cases/fourslash/organizeImportsType6.ts @@ -1,53 +1,36 @@ /// -//// import { type A, type a, b, B } from "foo"; -//// console.log(a, b, A, B); - -// verify.organizeImports( -// `import { B, b, type A, type a } from "foo"; -// console.log(a, b, A, B);`, -// /*mode*/ undefined, -// { organizeImportsIgnoreCase: "auto", organizeImportsTypeOrder: "last" } -// ); - -verify.organizeImports( -`import { type A, type a, b, B } from "foo"; -console.log(a, b, A, B);`, - /*mode*/ undefined, - { organizeImportsIgnoreCase: "auto", organizeImportsTypeOrder: "inline" } -); - -verify.organizeImports( -`import { type A, type a, b, B } from "foo"; -console.log(a, b, A, B);`, - /*mode*/ undefined, - { organizeImportsIgnoreCase: "auto", organizeImportsTypeOrder: "first" } -); +//// import { type a, A, b } from "foo"; +//// interface Use extends A {} +//// console.log(a, b); verify.organizeImports( -`import { B, b, type A, type a } from "foo"; -console.log(a, b, A, B);`, +`import { type a, A, b } from "foo"; +interface Use extends A {} +console.log(a, b);`, /*mode*/ undefined, - { organizeImportsIgnoreCase: "auto", organizeImportsTypeOrder: "last" } -); + { organizeImportsTypeOrder: "inline" }); +edit.replaceLine(0, 'import { type a, A, b } from "foo1";'); verify.organizeImports( -`import { B, b, type A, type a } from "foo"; -console.log(a, b, A, B);`, +`import { type a, A, b } from "foo1"; +interface Use extends A {} +console.log(a, b);`, /*mode*/ undefined, - { organizeImportsIgnoreCase: "auto" } -); + { organizeImportsIgnoreCase: "auto", organizeImportsTypeOrder: "inline" }); +edit.replaceLine(0, 'import { type a, A, b } from "foo2";'); verify.organizeImports( -`import { B, b, type A, type a } from "foo"; -console.log(a, b, A, B);`, +`import { type a, A, b } from "foo2"; +interface Use extends A {} +console.log(a, b);`, /*mode*/ undefined, - { organizeImportsIgnoreCase: true } -); + { organizeImportsIgnoreCase: true, organizeImportsTypeOrder: "inline" }); +edit.replaceLine(0, 'import { type a, A, b } from "foo3";'); verify.organizeImports( -`import { B, b, type A, type a } from "foo"; -console.log(a, b, A, B);`, +`import { A, type a, b } from "foo3"; +interface Use extends A {} +console.log(a, b);`, /*mode*/ undefined, - { organizeImportsIgnoreCase: false } -); \ No newline at end of file + { organizeImportsIgnoreCase: false, organizeImportsTypeOrder: "inline" }); \ No newline at end of file diff --git a/tests/cases/fourslash/organizeImportsType7.ts b/tests/cases/fourslash/organizeImportsType7.ts index f80799fcbe3c4..72fd2af92c9db 100644 --- a/tests/cases/fourslash/organizeImportsType7.ts +++ b/tests/cases/fourslash/organizeImportsType7.ts @@ -1,41 +1,36 @@ -/// - -//// import { type a, type A, b, B } from "foo"; -//// console.log(a, b, A, B); - -// verify.organizeImports( -// `import { b, B, type a, type A } from "foo"; -// console.log(a, b, A, B);` -// ); - -verify.organizeImports( -`import { type a, type A, b, B } from "foo"; -console.log(a, b, A, B);`, - /*mode*/ undefined, - { organizeImportsIgnoreCase: "auto", organizeImportsTypeOrder: "inline" }); - -verify.organizeImports( -`import { type a, type A, b, B } from "foo"; -console.log(a, b, A, B);`, - /*mode*/ undefined, - { organizeImportsIgnoreCase: "auto", organizeImportsTypeOrder: "first"} -); - -verify.organizeImports( -`import { B, b, type A, type a } from "foo"; -console.log(a, b, A, B);`, - /*mode*/ undefined, - { organizeImportsIgnoreCase: "auto", organizeImportsTypeOrder: "last" } -); - -// verify.organizeImports( -// `import { b, B, type a, type A } from "foo"; -// console.log(a, b, A, B);`, -// /*mode*/ undefined, -// { organizeImportsIgnoreCase: true }); - -// verify.organizeImports( -// `import { B, b, type A, type a } from "foo"; -// console.log(a, b, A, B);`, -// /*mode*/ undefined, -// { organizeImportsIgnoreCase: false }); \ No newline at end of file +/// + +//// import { a, type A, b } from "foo"; +//// interface Use extends A {} +//// console.log(a, b); + +verify.organizeImports( +`import { a, type A, b } from "foo"; +interface Use extends A {} +console.log(a, b);`, + /*mode*/ undefined, + { organizeImportsTypeOrder: "inline" }); + +edit.replaceLine(0, 'import { a, type A, b } from "foo1";'); +verify.organizeImports( +`import { a, type A, b } from "foo1"; +interface Use extends A {} +console.log(a, b);`, + /*mode*/ undefined, + { organizeImportsIgnoreCase: "auto", organizeImportsTypeOrder: "inline" }); + +edit.replaceLine(0, 'import { a, type A, b } from "foo2";'); +verify.organizeImports( +`import { a, type A, b } from "foo2"; +interface Use extends A {} +console.log(a, b);`, + /*mode*/ undefined, + { organizeImportsIgnoreCase: true, organizeImportsTypeOrder: "inline"}); + +edit.replaceLine(0, 'import { a, type A, b } from "foo3";'); +verify.organizeImports( +`import { type A, a, b } from "foo3"; +interface Use extends A {} +console.log(a, b);`, + /*mode*/ undefined, + { organizeImportsIgnoreCase: false, organizeImportsTypeOrder: "inline" }); \ No newline at end of file diff --git a/tests/cases/fourslash/organizeImportsType8.ts b/tests/cases/fourslash/organizeImportsType8.ts new file mode 100644 index 0000000000000..da43f893431df --- /dev/null +++ b/tests/cases/fourslash/organizeImportsType8.ts @@ -0,0 +1,53 @@ +/// + +//// import { type A, type a, b, B } from "foo"; +//// console.log(a, b, A, B); + +verify.organizeImports( +`import { type A, type a, b, B } from "foo"; +console.log(a, b, A, B);`, + /*mode*/ undefined, + { organizeImportsIgnoreCase: "auto", organizeImportsTypeOrder: "inline" } +); + +edit.replaceLine(0, 'import { type A, type a, b, B } from "foo1";'); +verify.organizeImports( +`import { type A, type a, b, B } from "foo1"; +console.log(a, b, A, B);`, + /*mode*/ undefined, + { organizeImportsIgnoreCase: "auto", organizeImportsTypeOrder: "first" } +); + +edit.replaceLine(0, 'import { type A, type a, b, B } from "foo2";'); +verify.organizeImports( +`import { B, b, type A, type a } from "foo2"; +console.log(a, b, A, B);`, + /*mode*/ undefined, + { organizeImportsIgnoreCase: "auto", organizeImportsTypeOrder: "last" } +); + +// default behavior is { organizeImportsTypeOrder: "last" } + +edit.replaceLine(0, 'import { type A, type a, b, B } from "foo3";'); +verify.organizeImports( +`import { B, b, type A, type a } from "foo3"; +console.log(a, b, A, B);`, + /*mode*/ undefined, + { organizeImportsIgnoreCase: "auto" } +); + +edit.replaceLine(0, 'import { type A, type a, b, B } from "foo4";'); +verify.organizeImports( +`import { b, B, type A, type a } from "foo4"; +console.log(a, b, A, B);`, + /*mode*/ undefined, + { organizeImportsIgnoreCase: true } +); + +edit.replaceLine(0, 'import { type A, type a, b, B } from "foo5";'); +verify.organizeImports( +`import { B, b, type A, type a } from "foo5"; +console.log(a, b, A, B);`, + /*mode*/ undefined, + { organizeImportsIgnoreCase: false } +); \ No newline at end of file diff --git a/tests/cases/fourslash/organizeImportsType9.ts b/tests/cases/fourslash/organizeImportsType9.ts new file mode 100644 index 0000000000000..efe6107e020b6 --- /dev/null +++ b/tests/cases/fourslash/organizeImportsType9.ts @@ -0,0 +1,48 @@ +/// + +//// import { type a, type A, b, B } from "foo"; +//// console.log(a, b, A, B); + +verify.organizeImports( +`import { type a, type A, b, B } from "foo"; +console.log(a, b, A, B);`, + /*mode*/ undefined, + { organizeImportsIgnoreCase: "auto", organizeImportsTypeOrder: "inline" }); + +edit.replaceLine(0, 'import { type a, type A, b, B } from "foo1";'); +verify.organizeImports( +`import { type a, type A, b, B } from "foo1"; +console.log(a, b, A, B);`, + /*mode*/ undefined, + { organizeImportsIgnoreCase: "auto", organizeImportsTypeOrder: "first"} +); + +edit.replaceLine(0, 'import { type a, type A, b, B } from "foo2";'); +verify.organizeImports( +`import { B, b, type A, type a } from "foo2"; +console.log(a, b, A, B);`, + /*mode*/ undefined, + { organizeImportsIgnoreCase: "auto", organizeImportsTypeOrder: "last" } +); + +edit.replaceLine(0, 'import { type a, type A, b, B } from "foo3";'); +verify.organizeImports( +`import { B, b, type A, type a } from "foo3"; +console.log(a, b, A, B);`, + /*mode*/ undefined, + { organizeImportsIgnoreCase: "auto" } +); + +edit.replaceLine(0, 'import { type a, type A, b, B } from "foo4";'); +verify.organizeImports( +`import { b, B, type a, type A } from "foo4"; +console.log(a, b, A, B);`, + /*mode*/ undefined, + { organizeImportsIgnoreCase: true }); + +edit.replaceLine(0, 'import { type a, type A, b, B } from "foo5";'); +verify.organizeImports( +`import { B, b, type A, type a } from "foo5"; +console.log(a, b, A, B);`, + /*mode*/ undefined, + { organizeImportsIgnoreCase: false }); \ No newline at end of file From 11929fefa94b02b3e83f6056321f348a52261750 Mon Sep 17 00:00:00 2001 From: Isabel Duan Date: Mon, 8 Jan 2024 11:07:47 -0800 Subject: [PATCH 09/14] fix potential organizeImports chaining in test calls --- tests/cases/fourslash/organizeImports1.ts | 38 +++++++++++----------- tests/cases/fourslash/organizeImports16.ts | 9 +++-- 2 files changed, 25 insertions(+), 22 deletions(-) diff --git a/tests/cases/fourslash/organizeImports1.ts b/tests/cases/fourslash/organizeImports1.ts index 58306c8161caa..53ee2c25b1fe1 100644 --- a/tests/cases/fourslash/organizeImports1.ts +++ b/tests/cases/fourslash/organizeImports1.ts @@ -18,26 +18,26 @@ //// console.log(a, B, b, c, C, d, D); //// console.log(e, f, F, g, G, H, h); -// verify.organizeImports( -// `import { -// a, -// b, -// b as B, -// c, -// c as C, -// d, d as D, -// e, -// f, -// f as F, -// g, -// g as G, -// h, h as H -// } from './foo'; +verify.organizeImports( +`import { + a, + b, + b as B, + c, + c as C, + d, d as D, + e, + f, + f as F, + g, + g as G, + h, h as H +} from './foo'; -// console.log(a, B, b, c, C, d, D); -// console.log(e, f, F, g, G, H, h);`, -// /*mode*/ undefined, -// { organizeImportsIgnoreCase: true }); +console.log(a, B, b, c, C, d, D); +console.log(e, f, F, g, G, H, h);`, +/*mode*/ undefined, +{ organizeImportsIgnoreCase: true }); verify.organizeImports( `import { diff --git a/tests/cases/fourslash/organizeImports16.ts b/tests/cases/fourslash/organizeImports16.ts index 93aa5a81f2cb8..be7150c047e0f 100644 --- a/tests/cases/fourslash/organizeImports16.ts +++ b/tests/cases/fourslash/organizeImports16.ts @@ -9,22 +9,25 @@ verify.organizeImports( interface Use extends A {} console.log(a, b);`); +edit.replaceLine(0, 'import { a, A, b } from "foo1";'); verify.organizeImports( -`import { a, A, b } from "foo"; +`import { a, A, b } from "foo1"; interface Use extends A {} console.log(a, b);`, /*mode*/ undefined, { organizeImportsIgnoreCase: "auto" }); +edit.replaceLine(0, 'import { a, A, b } from "foo2";'); verify.organizeImports( -`import { a, A, b } from "foo"; +`import { a, A, b } from "foo2"; interface Use extends A {} console.log(a, b);`, /*mode*/ undefined, { organizeImportsIgnoreCase: true }); +edit.replaceLine(0, 'import { a, A, b } from "foo3";'); verify.organizeImports( -`import { A, a, b } from "foo"; +`import { A, a, b } from "foo3"; interface Use extends A {} console.log(a, b);`, /*mode*/ undefined, From 4560a6b08ea1a48d187570342669c45198af225d Mon Sep 17 00:00:00 2001 From: Isabel Duan Date: Mon, 8 Jan 2024 12:15:16 -0800 Subject: [PATCH 10/14] post-merge bug fix --- src/services/codefixes/importFixes.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/services/codefixes/importFixes.ts b/src/services/codefixes/importFixes.ts index 2b317fbdea0ce..22cd9693ab98f 100644 --- a/src/services/codefixes/importFixes.ts +++ b/src/services/codefixes/importFixes.ts @@ -1414,7 +1414,7 @@ function promoteFromTypeOnly( return aliasDeclaration; } } - changes.deleteRange(sourceFile, aliasDeclaration.getFirstToken()!); + changes.deleteRange(sourceFile, { pos: getTokenPosOfNode(aliasDeclaration.getFirstToken()!), end: getTokenPosOfNode(aliasDeclaration.propertyName ?? aliasDeclaration.name) }); return aliasDeclaration; } else { From 42033fca6219d8b2b270cc8abfbbeac0e07fcecf Mon Sep 17 00:00:00 2001 From: Isabel Duan Date: Tue, 9 Jan 2024 10:37:06 -0800 Subject: [PATCH 11/14] fixed tests and verify.importFixes --- src/harness/fourslashImpl.ts | 10 ++++--- src/services/organizeImports.ts | 21 +++----------- .../cases/fourslash/autoImportTypeImport3.ts | 24 ++++++++-------- .../importNameCodeFix_importType10.ts | 27 ------------------ .../importNameCodeFix_importType7.ts | 28 +++++++++++++++++++ 5 files changed, 50 insertions(+), 60 deletions(-) delete mode 100644 tests/cases/fourslash/importNameCodeFix_importType10.ts diff --git a/src/harness/fourslashImpl.ts b/src/harness/fourslashImpl.ts index 5763b24c6bde6..8e9a78dc2b49e 100644 --- a/src/harness/fourslashImpl.ts +++ b/src/harness/fourslashImpl.ts @@ -3521,10 +3521,12 @@ export class TestState { actualTextArray.push(text); // Undo changes to perform next fix - const span = change.textChanges[0].span; - const deletedText = originalContent.substr(span.start, change.textChanges[0].span.length); - const insertedText = change.textChanges[0].newText; - this.editScriptAndUpdateMarkers(fileName, span.start, span.start + insertedText.length, deletedText); + for (const textChange of change.textChanges) { + const span = textChange.span; + const deletedText = originalContent.slice(span.start, textChange.span.length); + const insertedText = textChange.newText; + this.editScriptAndUpdateMarkers(fileName, span.start, span.start + insertedText.length, deletedText); + } } if (expectedTextArray.length !== actualTextArray.length) { this.raiseError(`Expected ${expectedTextArray.length} import fixes, got ${actualTextArray.length}:\n\n${actualTextArray.join("\n\n" + "-".repeat(20) + "\n\n")}`); diff --git a/src/services/organizeImports.ts b/src/services/organizeImports.ts index c6502cfcbf74f..b9c2f7e89096f 100644 --- a/src/services/organizeImports.ts +++ b/src/services/organizeImports.ts @@ -26,6 +26,7 @@ import { getStringComparer, getUILocale, group, + groupBy, Identifier, identity, ImportClause, @@ -753,11 +754,11 @@ export const detectImportSpecifierSorting = memoizeCached((specifiers: readonly const collateCaseInsensitive = getOrganizeImportsComparer(preferences, /*ignoreCase*/ true); if (preferences.organizeImportsTypeOrder !== "inline") { - const { regularImports, typeImports } = getCategorizedSpecifiers(specifiers); - const regularCaseSensitivity = regularImports.length + const { type: regularImports, regular: typeImports } = groupBy(specifiers, s => s.isTypeOnly ? "type" : "regular"); + const regularCaseSensitivity = regularImports?.length ? detectSortCaseSensitivity(regularImports, specifier => specifier.name.text, collateCaseSensitive, collateCaseInsensitive) : undefined; - const typeCaseSensitivity = typeImports.length + const typeCaseSensitivity = typeImports?.length ? detectSortCaseSensitivity(typeImports, specifier => specifier.name.text ?? "", collateCaseSensitive, collateCaseInsensitive) : undefined; if (regularCaseSensitivity === undefined) { @@ -776,20 +777,6 @@ export const detectImportSpecifierSorting = memoizeCached((specifiers: readonly return detectSortCaseSensitivity(specifiers, specifier => specifier.name.text, collateCaseSensitive, collateCaseInsensitive); }, new ImportSpecifierSortingCache()); -function getCategorizedSpecifiers(specifiers: readonly ImportSpecifier[]) { - const regularImports: ImportSpecifier[] = []; - const typeImports: ImportSpecifier[] = []; - for (const s of specifiers) { - if (s.isTypeOnly) { - typeImports.push(s); - } - else { - regularImports.push(s); - } - } - return { regularImports, typeImports }; -} - /** @internal */ export function getImportDeclarationInsertionIndex(sortedImports: readonly AnyImportOrRequireStatement[], newImport: AnyImportOrRequireStatement, comparer: Comparer) { const index = binarySearch(sortedImports, newImport, identity, (a, b) => compareImportsOrRequireStatements(a, b, comparer)); diff --git a/tests/cases/fourslash/autoImportTypeImport3.ts b/tests/cases/fourslash/autoImportTypeImport3.ts index bb507a2088c2e..a554a85cbac9a 100644 --- a/tests/cases/fourslash/autoImportTypeImport3.ts +++ b/tests/cases/fourslash/autoImportTypeImport3.ts @@ -23,16 +23,16 @@ console.log(A, D);`], /*errorCode*/ undefined, { organizeImportsIgnoreCase: true, organizeImportsTypeOrder : "last" }); -// verify.importFixAtPosition([ -// `import { A, type B, type C, D } from './foo'; -// const b: B | C; -// console.log(A, D);`], -// /*errorCode*/ undefined, -// { organizeImportsIgnoreCase: true, organizeImportsTypeOrder : "inline" }); +verify.importFixAtPosition([ +`import { A, type B, type C, D } from './foo'; +const b: B | C; +console.log(A, D);`], + /*errorCode*/ undefined, + { organizeImportsIgnoreCase: true, organizeImportsTypeOrder : "inline" }); -// verify.importFixAtPosition([ -// `import { A, type B, type C, D } from './foo'; -// const b: B | C; -// console.log(A, D);`], -// /*errorCode*/ undefined, -// { organizeImportsIgnoreCase: true, organizeImportsTypeOrder : "first" }); \ No newline at end of file +verify.importFixAtPosition([ +`import { A, type B, type C, D } from './foo'; +const b: B | C; +console.log(A, D);`], + /*errorCode*/ undefined, + { organizeImportsIgnoreCase: true, organizeImportsTypeOrder : "first" }); \ No newline at end of file diff --git a/tests/cases/fourslash/importNameCodeFix_importType10.ts b/tests/cases/fourslash/importNameCodeFix_importType10.ts deleted file mode 100644 index 2c41364645105..0000000000000 --- a/tests/cases/fourslash/importNameCodeFix_importType10.ts +++ /dev/null @@ -1,27 +0,0 @@ -/// - -// @module: es2015 - -// sorting and multiline imports and trailing commas, oh my - -// @Filename: /exports.ts -//// export interface SomeInterface {} -//// export class SomePig {} - -// @Filename: /a.ts -//// import { -//// type SomeInterface, -//// type SomePig, -//// } from "./exports.js"; -//// new SomePig/**/ - -goTo.marker(""); -verify.importFixAtPosition([ -`import { - type SomeInterface, - SomePig, -} from "./exports.js"; -new SomePig`], - /*errorCode*/ undefined, - { organizeImportsTypeOrder: "inline" } -); diff --git a/tests/cases/fourslash/importNameCodeFix_importType7.ts b/tests/cases/fourslash/importNameCodeFix_importType7.ts index 7d28f3019654b..c74a3c2c22837 100644 --- a/tests/cases/fourslash/importNameCodeFix_importType7.ts +++ b/tests/cases/fourslash/importNameCodeFix_importType7.ts @@ -22,3 +22,31 @@ verify.importFixAtPosition([ type SomeInterface, } from "./exports.js"; new SomePig`]); + +verify.importFixAtPosition([ +`import { + SomePig, + type SomeInterface, +} from "./exports.js"; +new SomePig`], +/*errorCode*/ undefined, +{ organizeImportsTypeOrder: "last" }); + +verify.importFixAtPosition([ +`import { + type SomeInterface, + SomePig, +} from "./exports.js"; +new SomePig`], + /*errorCode*/ undefined, + { organizeImportsTypeOrder: "inline" } +); + +verify.importFixAtPosition([ +`import { + type SomeInterface, + SomePig, +} from "./exports.js"; +new SomePig`], +/*errorCode*/ undefined, +{ organizeImportsTypeOrder: "first" }); From dca9ba624c15a2b69ddc8f20f0a8f7f51b7b4d38 Mon Sep 17 00:00:00 2001 From: Isabel Duan Date: Tue, 9 Jan 2024 11:00:10 -0800 Subject: [PATCH 12/14] fix verifyImportFixAtPosition --- src/harness/fourslashImpl.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/harness/fourslashImpl.ts b/src/harness/fourslashImpl.ts index 8e9a78dc2b49e..2842fd6896a5f 100644 --- a/src/harness/fourslashImpl.ts +++ b/src/harness/fourslashImpl.ts @@ -3523,7 +3523,7 @@ export class TestState { // Undo changes to perform next fix for (const textChange of change.textChanges) { const span = textChange.span; - const deletedText = originalContent.slice(span.start, textChange.span.length); + const deletedText = originalContent.slice(span.start, span.start + textChange.span.length); const insertedText = textChange.newText; this.editScriptAndUpdateMarkers(fileName, span.start, span.start + insertedText.length, deletedText); } From d408f5a1cc106205784d789f924bc2ab51f3a7ba Mon Sep 17 00:00:00 2001 From: Isabel Duan Date: Tue, 9 Jan 2024 15:18:14 -0800 Subject: [PATCH 13/14] add comments --- src/server/protocol.ts | 2 +- tests/cases/fourslash/autoImportTypeImport1.ts | 5 +++++ tests/cases/fourslash/autoImportTypeImport2.ts | 2 ++ tests/cases/fourslash/autoImportTypeImport3.ts | 12 +++++++++--- 4 files changed, 17 insertions(+), 4 deletions(-) diff --git a/src/server/protocol.ts b/src/server/protocol.ts index 2353575601d89..e0964cea6d9d6 100644 --- a/src/server/protocol.ts +++ b/src/server/protocol.ts @@ -3644,7 +3644,7 @@ export interface UserPreferences { */ readonly organizeImportsCaseFirst?: "upper" | "lower" | false; /** - * Indicates where named type-only imports should sort. "inline" sorts imports without regard to if the import is + * Indicates where named type-only imports should sort. "inline" sorts named imports without regard to if the import is * type-only. * * Default: `last` diff --git a/tests/cases/fourslash/autoImportTypeImport1.ts b/tests/cases/fourslash/autoImportTypeImport1.ts index 1ed34dd5d829e..40422df03d2e7 100644 --- a/tests/cases/fourslash/autoImportTypeImport1.ts +++ b/tests/cases/fourslash/autoImportTypeImport1.ts @@ -16,12 +16,15 @@ goTo.marker(""); +// importFixes should only place the import in sorted position if the existing imports are sorted as specified, +// otherwise the import should be placed at the end verify.importFixAtPosition([ `import { A, D, type C, type B } from './foo'; const b: B | C; console.log(A, D);`], /*errorCode*/ undefined, { organizeImportsTypeOrder: "inline" } + // `type B` is added to the end since the existing imports are not sorted as specified ); verify.importFixAtPosition([ @@ -30,6 +33,7 @@ const b: B | C; console.log(A, D);`], /*errorCode*/ undefined, { organizeImportsTypeOrder: "last" } + // `type B` is added to the sorted position since the existing imports *are* sorted as specified ); verify.importFixAtPosition([ @@ -38,4 +42,5 @@ const b: B | C; console.log(A, D);`], /*errorCode*/ undefined, { organizeImportsTypeOrder: "first" } + // `type B` is added to the end (default behavior) since the existing imports are not sorted as specified ); \ No newline at end of file diff --git a/tests/cases/fourslash/autoImportTypeImport2.ts b/tests/cases/fourslash/autoImportTypeImport2.ts index f98f5bef31f9f..195a12980c2f8 100644 --- a/tests/cases/fourslash/autoImportTypeImport2.ts +++ b/tests/cases/fourslash/autoImportTypeImport2.ts @@ -16,6 +16,8 @@ goTo.marker(""); +// importFixes should only place the import in sorted position if the existing imports are sorted as specified, +// otherwise the import should be placed at the end verify.importFixAtPosition([ `import { A, type B, type C, D } from './foo'; const b: B | C; diff --git a/tests/cases/fourslash/autoImportTypeImport3.ts b/tests/cases/fourslash/autoImportTypeImport3.ts index a554a85cbac9a..50594e625817c 100644 --- a/tests/cases/fourslash/autoImportTypeImport3.ts +++ b/tests/cases/fourslash/autoImportTypeImport3.ts @@ -16,23 +16,29 @@ goTo.marker(""); +// importFixes should only place the import in sorted position if the existing imports are sorted as specified, +// otherwise the import should be placed at the end (regardless of if it's a regular or type-only import) verify.importFixAtPosition([ `import { A, D, type B, type C } from './foo'; const b: B | C; console.log(A, D);`], /*errorCode*/ undefined, - { organizeImportsIgnoreCase: true, organizeImportsTypeOrder : "last" }); + { organizeImportsIgnoreCase: true, organizeImportsTypeOrder : "last" } +); verify.importFixAtPosition([ `import { A, type B, type C, D } from './foo'; const b: B | C; console.log(A, D);`], /*errorCode*/ undefined, - { organizeImportsIgnoreCase: true, organizeImportsTypeOrder : "inline" }); + { organizeImportsIgnoreCase: true, organizeImportsTypeOrder : "inline" } +); verify.importFixAtPosition([ `import { A, type B, type C, D } from './foo'; const b: B | C; console.log(A, D);`], /*errorCode*/ undefined, - { organizeImportsIgnoreCase: true, organizeImportsTypeOrder : "first" }); \ No newline at end of file + { organizeImportsIgnoreCase: true, organizeImportsTypeOrder : "first" } + // `D` is added to the end since `A, type B, type C` is not sorted to "first" +); \ No newline at end of file From 7902488c2fa43cd05f2396411c3429cacd8e987d Mon Sep 17 00:00:00 2001 From: Isabel Duan Date: Tue, 9 Jan 2024 16:09:20 -0800 Subject: [PATCH 14/14] update baselines --- tests/baselines/reference/api/typescript.d.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/baselines/reference/api/typescript.d.ts b/tests/baselines/reference/api/typescript.d.ts index 0e5be0f2a6f69..fe1a414ef6f51 100644 --- a/tests/baselines/reference/api/typescript.d.ts +++ b/tests/baselines/reference/api/typescript.d.ts @@ -2934,7 +2934,7 @@ declare namespace ts { */ readonly organizeImportsCaseFirst?: "upper" | "lower" | false; /** - * Indicates where named type-only imports should sort. "inline" sorts imports without regard to if the import is + * Indicates where named type-only imports should sort. "inline" sorts named imports without regard to if the import is * type-only. * * Default: `last`