From b89f697851dff0bdce54f05b8f42ce0ef5c51a47 Mon Sep 17 00:00:00 2001 From: MQuy Date: Sat, 19 Mar 2022 01:19:50 +0100 Subject: [PATCH 1/9] Create sort groups based on newlines --- src/services/organizeImports.ts | 34 ++++++++++++++++++---- tests/cases/fourslash/organizeImports10.ts | 19 ++++++++++++ 2 files changed, 48 insertions(+), 5 deletions(-) create mode 100644 tests/cases/fourslash/organizeImports10.ts diff --git a/src/services/organizeImports.ts b/src/services/organizeImports.ts index d01f2eae5f6d6..8743870c2a016 100644 --- a/src/services/organizeImports.ts +++ b/src/services/organizeImports.ts @@ -15,7 +15,6 @@ namespace ts.OrganizeImports { preferences: UserPreferences, skipDestructiveCodeActions?: boolean ) { - const changeTracker = textChanges.ChangeTracker.fromContext({ host, formatContext, preferences }); const coalesceAndOrganizeImports = (importGroup: readonly ImportDeclaration[]) => stableSort( @@ -23,8 +22,8 @@ namespace ts.OrganizeImports { (s1, s2) => compareImportsOrRequireStatements(s1, s2)); // All of the old ImportDeclarations in the file, in syntactic order. - const topLevelImportDecls = sourceFile.statements.filter(isImportDeclaration); - organizeImportsWorker(topLevelImportDecls, coalesceAndOrganizeImports); + const topLevelImportGroupDecls = groupImportsByNewlineContiguous(sourceFile.statements.filter(isImportDeclaration), host, formatContext); + topLevelImportGroupDecls.forEach(topLevelImportGroupDecl => organizeImportsWorker(topLevelImportGroupDecl, coalesceAndOrganizeImports)); // All of the old ExportDeclarations in the file, in syntactic order. const topLevelExportDecls = sourceFile.statements.filter(isExportDeclaration); @@ -33,8 +32,8 @@ namespace ts.OrganizeImports { for (const ambientModule of sourceFile.statements.filter(isAmbientModule)) { if (!ambientModule.body) continue; - const ambientModuleImportDecls = ambientModule.body.statements.filter(isImportDeclaration); - organizeImportsWorker(ambientModuleImportDecls, coalesceAndOrganizeImports); + const ambientModuleImportGroupDecls = groupImportsByNewlineContiguous(ambientModule.body.statements.filter(isImportDeclaration), host, formatContext); + ambientModuleImportGroupDecls.forEach(ambientModuleImportGroupDecl => organizeImportsWorker(ambientModuleImportGroupDecl, coalesceAndOrganizeImports)); const ambientModuleExportDecls = ambientModule.body.statements.filter(isExportDeclaration); organizeImportsWorker(ambientModuleExportDecls, coalesceExports); @@ -88,6 +87,31 @@ namespace ts.OrganizeImports { } } + function groupImportsByNewlineContiguous(importDecls: ImportDeclaration[], host: LanguageServiceHost, formatContext: formatting.FormatContext): ImportDeclaration[][] { + const groupImports: ImportDeclaration[][] = []; + const newLine = getNewLineOrDefaultFromHost(host, formatContext.options); + const prefixCond = `${newLine}${newLine}`; + + for(let importIndex = 0, groupIndex = 0, length = importDecls.length; importIndex < length ; ++importIndex) { + const topLevelImportDecl = importDecls[importIndex]; + const leadingText = topLevelImportDecl.getFullText().substring( + 0, + topLevelImportDecl.getStart() - topLevelImportDecl.getFullStart()); + + if (startsWith(leadingText, prefixCond)) { + groupIndex++; + } + + if (!groupImports[groupIndex]) { + groupImports[groupIndex] = []; + } + + groupImports[groupIndex].push(topLevelImportDecl); + } + + return groupImports; + } + function removeUnusedImports(oldImports: readonly ImportDeclaration[], sourceFile: SourceFile, program: Program, skipDestructiveCodeActions: boolean | undefined) { // As a precaution, consider unused import detection to be destructive (GH #43051) if (skipDestructiveCodeActions) { diff --git a/tests/cases/fourslash/organizeImports10.ts b/tests/cases/fourslash/organizeImports10.ts new file mode 100644 index 0000000000000..0d74240a9f4ce --- /dev/null +++ b/tests/cases/fourslash/organizeImports10.ts @@ -0,0 +1,19 @@ +/// + +////import c from "C"; +//// +////import d from "D"; +////import a from "A"; +////import b from "B"; +//// +////console.log(a, b, c, d) + +verify.organizeImports( +`import c from "C"; + +import a from "A"; +import b from "B"; +import d from "D"; + +console.log(a, b, c, d)` +); From ef6c7db33f0be2f081b8c5f4d80ab11385b93c33 Mon Sep 17 00:00:00 2001 From: MQuy Date: Sat, 19 Mar 2022 07:57:19 +0100 Subject: [PATCH 2/9] Update ambient module test to match with new sort groups --- .../reference/organizeImports/TopLevelAndAmbientModule.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/baselines/reference/organizeImports/TopLevelAndAmbientModule.ts b/tests/baselines/reference/organizeImports/TopLevelAndAmbientModule.ts index d6f3158efe859..8bbab4f4ff5d8 100644 --- a/tests/baselines/reference/organizeImports/TopLevelAndAmbientModule.ts +++ b/tests/baselines/reference/organizeImports/TopLevelAndAmbientModule.ts @@ -17,7 +17,6 @@ D(); // ==ORGANIZED== -import "lib"; import D from "lib"; declare module "mod" { @@ -26,5 +25,6 @@ declare module "mod" { function F(f1: {} = F1, f2: {} = F2) {} } +import "lib"; D(); From fa83bcee6f450e91807b1910043eabfa99f00c20 Mon Sep 17 00:00:00 2001 From: MQuy Date: Mon, 28 Mar 2022 19:38:33 +0200 Subject: [PATCH 3/9] Use for of to make code shorter and shorten argument --- src/services/organizeImports.ts | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/services/organizeImports.ts b/src/services/organizeImports.ts index 8743870c2a016..d87eef770b3b9 100644 --- a/src/services/organizeImports.ts +++ b/src/services/organizeImports.ts @@ -23,7 +23,7 @@ namespace ts.OrganizeImports { // All of the old ImportDeclarations in the file, in syntactic order. const topLevelImportGroupDecls = groupImportsByNewlineContiguous(sourceFile.statements.filter(isImportDeclaration), host, formatContext); - topLevelImportGroupDecls.forEach(topLevelImportGroupDecl => organizeImportsWorker(topLevelImportGroupDecl, coalesceAndOrganizeImports)); + topLevelImportGroupDecls.forEach(importGroupDecl => organizeImportsWorker(importGroupDecl, coalesceAndOrganizeImports)); // All of the old ExportDeclarations in the file, in syntactic order. const topLevelExportDecls = sourceFile.statements.filter(isExportDeclaration); @@ -33,7 +33,7 @@ namespace ts.OrganizeImports { if (!ambientModule.body) continue; const ambientModuleImportGroupDecls = groupImportsByNewlineContiguous(ambientModule.body.statements.filter(isImportDeclaration), host, formatContext); - ambientModuleImportGroupDecls.forEach(ambientModuleImportGroupDecl => organizeImportsWorker(ambientModuleImportGroupDecl, coalesceAndOrganizeImports)); + ambientModuleImportGroupDecls.forEach(importGroupDecl => organizeImportsWorker(importGroupDecl, coalesceAndOrganizeImports)); const ambientModuleExportDecls = ambientModule.body.statements.filter(isExportDeclaration); organizeImportsWorker(ambientModuleExportDecls, coalesceExports); @@ -92,11 +92,9 @@ namespace ts.OrganizeImports { const newLine = getNewLineOrDefaultFromHost(host, formatContext.options); const prefixCond = `${newLine}${newLine}`; - for(let importIndex = 0, groupIndex = 0, length = importDecls.length; importIndex < length ; ++importIndex) { - const topLevelImportDecl = importDecls[importIndex]; - const leadingText = topLevelImportDecl.getFullText().substring( - 0, - topLevelImportDecl.getStart() - topLevelImportDecl.getFullStart()); + let groupIndex = 0; + for(const topLevelImportDecl of importDecls) { + const leadingText = topLevelImportDecl.getFullText().substring(0, topLevelImportDecl.getStart() - topLevelImportDecl.getFullStart()); if (startsWith(leadingText, prefixCond)) { groupIndex++; From 141f9ee319a7d51f072d268e3359e85271727638 Mon Sep 17 00:00:00 2001 From: MQuy Date: Mon, 28 Mar 2022 21:17:20 +0200 Subject: [PATCH 4/9] Sort import based on newlin eand comments --- src/services/organizeImports.ts | 35 +++++++++++++++++++++++---------- 1 file changed, 25 insertions(+), 10 deletions(-) diff --git a/src/services/organizeImports.ts b/src/services/organizeImports.ts index d87eef770b3b9..d19433157f205 100644 --- a/src/services/organizeImports.ts +++ b/src/services/organizeImports.ts @@ -22,7 +22,7 @@ namespace ts.OrganizeImports { (s1, s2) => compareImportsOrRequireStatements(s1, s2)); // All of the old ImportDeclarations in the file, in syntactic order. - const topLevelImportGroupDecls = groupImportsByNewlineContiguous(sourceFile.statements.filter(isImportDeclaration), host, formatContext); + const topLevelImportGroupDecls = groupImportsByNewlineContiguous(sourceFile, sourceFile.statements.filter(isImportDeclaration)); topLevelImportGroupDecls.forEach(importGroupDecl => organizeImportsWorker(importGroupDecl, coalesceAndOrganizeImports)); // All of the old ExportDeclarations in the file, in syntactic order. @@ -32,7 +32,7 @@ namespace ts.OrganizeImports { for (const ambientModule of sourceFile.statements.filter(isAmbientModule)) { if (!ambientModule.body) continue; - const ambientModuleImportGroupDecls = groupImportsByNewlineContiguous(ambientModule.body.statements.filter(isImportDeclaration), host, formatContext); + const ambientModuleImportGroupDecls = groupImportsByNewlineContiguous(sourceFile, ambientModule.body.statements.filter(isImportDeclaration)); ambientModuleImportGroupDecls.forEach(importGroupDecl => organizeImportsWorker(importGroupDecl, coalesceAndOrganizeImports)); const ambientModuleExportDecls = ambientModule.body.statements.filter(isExportDeclaration); @@ -87,16 +87,11 @@ namespace ts.OrganizeImports { } } - function groupImportsByNewlineContiguous(importDecls: ImportDeclaration[], host: LanguageServiceHost, formatContext: formatting.FormatContext): ImportDeclaration[][] { + function groupImportsByNewlineContiguous(sourceFile: SourceFile,importDecls: ImportDeclaration[]): ImportDeclaration[][] { const groupImports: ImportDeclaration[][] = []; - const newLine = getNewLineOrDefaultFromHost(host, formatContext.options); - const prefixCond = `${newLine}${newLine}`; - let groupIndex = 0; - for(const topLevelImportDecl of importDecls) { - const leadingText = topLevelImportDecl.getFullText().substring(0, topLevelImportDecl.getStart() - topLevelImportDecl.getFullStart()); - - if (startsWith(leadingText, prefixCond)) { + for (const topLevelImportDecl of importDecls) { + if (isStartedNewGroup(sourceFile, topLevelImportDecl)) { groupIndex++; } @@ -110,6 +105,26 @@ namespace ts.OrganizeImports { return groupImports; } + // a new group is created if an import includes at least two new line + // new line from multi-line comment doesn't count + function isStartedNewGroup(sourceFile: SourceFile, topLevelImportDecl: ImportDeclaration) { + const startPos = topLevelImportDecl.getFullStart(); + const endPos = topLevelImportDecl.getStart(); + + const scanner = createScanner(sourceFile.languageVersion, /*skipTrivia*/ false, sourceFile.languageVariant, sourceFile.text, /*onError:*/ undefined, startPos); + + let numberOfNewLines = 0; + while (scanner.getTokenPos() < endPos) { + const tokenKind = scanner.scan(); + + if (tokenKind === SyntaxKind.NewLineTrivia) { + numberOfNewLines++; + } + } + + return numberOfNewLines >= 2; + } + function removeUnusedImports(oldImports: readonly ImportDeclaration[], sourceFile: SourceFile, program: Program, skipDestructiveCodeActions: boolean | undefined) { // As a precaution, consider unused import detection to be destructive (GH #43051) if (skipDestructiveCodeActions) { From c22b6976066f0ddf42b8f9eaa9cf6953f7249444 Mon Sep 17 00:00:00 2001 From: MQuy Date: Mon, 28 Mar 2022 21:18:02 +0200 Subject: [PATCH 5/9] Add import group tests for newline and comments --- .../organizeImportsGroup_CommentInNewline.ts | 21 ++++++++++++++++ ... => organizeImportsGroup_MultiNewlines.ts} | 2 ++ ...eImportsGroup_MultilineCommentInNewline.ts | 25 +++++++++++++++++++ .../fourslash/organizeImportsGroup_Newline.ts | 19 ++++++++++++++ 4 files changed, 67 insertions(+) create mode 100644 tests/cases/fourslash/organizeImportsGroup_CommentInNewline.ts rename tests/cases/fourslash/{organizeImports10.ts => organizeImportsGroup_MultiNewlines.ts} (91%) create mode 100644 tests/cases/fourslash/organizeImportsGroup_MultilineCommentInNewline.ts create mode 100644 tests/cases/fourslash/organizeImportsGroup_Newline.ts diff --git a/tests/cases/fourslash/organizeImportsGroup_CommentInNewline.ts b/tests/cases/fourslash/organizeImportsGroup_CommentInNewline.ts new file mode 100644 index 0000000000000..8a20bf185e97a --- /dev/null +++ b/tests/cases/fourslash/organizeImportsGroup_CommentInNewline.ts @@ -0,0 +1,21 @@ +/// + +////// polyfill +////import c from "C"; +////// not polyfill +////import d from "D"; +////import a from "A"; +////import b from "B"; +//// +////console.log(a, b, c, d) + +verify.organizeImports( +`// polyfill +import c from "C"; +// not polyfill +import a from "A"; +import b from "B"; +import d from "D"; + +console.log(a, b, c, d)` +); diff --git a/tests/cases/fourslash/organizeImports10.ts b/tests/cases/fourslash/organizeImportsGroup_MultiNewlines.ts similarity index 91% rename from tests/cases/fourslash/organizeImports10.ts rename to tests/cases/fourslash/organizeImportsGroup_MultiNewlines.ts index 0d74240a9f4ce..cce8b8f603fa5 100644 --- a/tests/cases/fourslash/organizeImports10.ts +++ b/tests/cases/fourslash/organizeImportsGroup_MultiNewlines.ts @@ -2,6 +2,7 @@ ////import c from "C"; //// +//// ////import d from "D"; ////import a from "A"; ////import b from "B"; @@ -11,6 +12,7 @@ verify.organizeImports( `import c from "C"; + import a from "A"; import b from "B"; import d from "D"; diff --git a/tests/cases/fourslash/organizeImportsGroup_MultilineCommentInNewline.ts b/tests/cases/fourslash/organizeImportsGroup_MultilineCommentInNewline.ts new file mode 100644 index 0000000000000..04fff1675ac30 --- /dev/null +++ b/tests/cases/fourslash/organizeImportsGroup_MultilineCommentInNewline.ts @@ -0,0 +1,25 @@ +/// + +////// polyfill +////import c from "C"; +/////* +////* demo +////*/ +////import d from "D"; +////import a from "A"; +////import b from "B"; +//// +////console.log(a, b, c, d) + +verify.organizeImports( +`// polyfill +import c from "C"; +/* +* demo +*/ +import a from "A"; +import b from "B"; +import d from "D"; + +console.log(a, b, c, d)` +); diff --git a/tests/cases/fourslash/organizeImportsGroup_Newline.ts b/tests/cases/fourslash/organizeImportsGroup_Newline.ts new file mode 100644 index 0000000000000..9f904ef376904 --- /dev/null +++ b/tests/cases/fourslash/organizeImportsGroup_Newline.ts @@ -0,0 +1,19 @@ +/// + +////import c from "C"; +//// +////import d from "D"; +////import a from "A"; // not count +////import b from "B"; +//// +////console.log(a, b, c, d) + +verify.organizeImports( +`import c from "C"; + +import a from "A"; // not count +import b from "B"; +import d from "D"; + +console.log(a, b, c, d)` +); From 2c13fec4c09b45955895dbbc80ee14c16d9ca5f6 Mon Sep 17 00:00:00 2001 From: MQuy Date: Mon, 28 Mar 2022 22:49:40 +0200 Subject: [PATCH 6/9] Remove SortComments test and fix organizeImport6 --- .../unittests/services/organizeImports.ts | 17 ----------------- .../reference/organizeImports/SortComments.ts | 17 ----------------- tests/cases/fourslash/organizeImports6.ts | 10 ++++++++-- 3 files changed, 8 insertions(+), 36 deletions(-) delete mode 100644 tests/baselines/reference/organizeImports/SortComments.ts diff --git a/src/testRunner/unittests/services/organizeImports.ts b/src/testRunner/unittests/services/organizeImports.ts index f3b222bb76791..4ddd1b608d003 100644 --- a/src/testRunner/unittests/services/organizeImports.ts +++ b/src/testRunner/unittests/services/organizeImports.ts @@ -679,23 +679,6 @@ import "lib1"; { path: "/lib1.ts", content: "" }, { path: "/lib2.ts", content: "" }); - testOrganizeImports("SortComments", - /*skipDestructiveCodeActions*/ false, - { - path: "/test.ts", - content: ` -// Header -import "lib3"; -// Comment2 -import "lib2"; -// Comment1 -import "lib1"; -`, - }, - { path: "/lib1.ts", content: "" }, - { path: "/lib2.ts", content: "" }, - { path: "/lib3.ts", content: "" }); - testOrganizeImports("AmbientModule", /*skipDestructiveCodeActions*/ false, { diff --git a/tests/baselines/reference/organizeImports/SortComments.ts b/tests/baselines/reference/organizeImports/SortComments.ts deleted file mode 100644 index 761bca1a2127b..0000000000000 --- a/tests/baselines/reference/organizeImports/SortComments.ts +++ /dev/null @@ -1,17 +0,0 @@ -// ==ORIGINAL== - -// Header -import "lib3"; -// Comment2 -import "lib2"; -// Comment1 -import "lib1"; - -// ==ORGANIZED== - -// Header -// Comment1 -import "lib1"; -// Comment2 -import "lib2"; -import "lib3"; diff --git a/tests/cases/fourslash/organizeImports6.ts b/tests/cases/fourslash/organizeImports6.ts index f5fa3b8269d9d..75e1491c1d19c 100644 --- a/tests/cases/fourslash/organizeImports6.ts +++ b/tests/cases/fourslash/organizeImports6.ts @@ -16,6 +16,12 @@ //// anotherThing; verify.organizeImports( -`import * as anotherThing from "someopath"; /* small comment */ // single line one. +`/* some comment here +* and there +*/ +import * as anotherThing from "someopath"; /* small comment */ // single line one. +/* some comment here +* and there +*/ -anotherThing;`); \ No newline at end of file +anotherThing;`); From bd8ce225bcc65270c93f7c18e93051e831b59055 Mon Sep 17 00:00:00 2001 From: MQuy Date: Wed, 30 Mar 2022 23:50:45 +0200 Subject: [PATCH 7/9] Rename isNewGroup and reuse scanner --- src/services/organizeImports.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/services/organizeImports.ts b/src/services/organizeImports.ts index d19433157f205..636222c7aabcd 100644 --- a/src/services/organizeImports.ts +++ b/src/services/organizeImports.ts @@ -88,10 +88,11 @@ namespace ts.OrganizeImports { } function groupImportsByNewlineContiguous(sourceFile: SourceFile,importDecls: ImportDeclaration[]): ImportDeclaration[][] { + const scanner = createScanner(sourceFile.languageVersion, /*skipTrivia*/ false, sourceFile.languageVariant); const groupImports: ImportDeclaration[][] = []; let groupIndex = 0; for (const topLevelImportDecl of importDecls) { - if (isStartedNewGroup(sourceFile, topLevelImportDecl)) { + if (isNewGroup(sourceFile, topLevelImportDecl, scanner)) { groupIndex++; } @@ -107,11 +108,10 @@ namespace ts.OrganizeImports { // a new group is created if an import includes at least two new line // new line from multi-line comment doesn't count - function isStartedNewGroup(sourceFile: SourceFile, topLevelImportDecl: ImportDeclaration) { + function isNewGroup(sourceFile: SourceFile, topLevelImportDecl: ImportDeclaration, scanner: Scanner) { const startPos = topLevelImportDecl.getFullStart(); const endPos = topLevelImportDecl.getStart(); - - const scanner = createScanner(sourceFile.languageVersion, /*skipTrivia*/ false, sourceFile.languageVariant, sourceFile.text, /*onError:*/ undefined, startPos); + scanner.setText(sourceFile.text, startPos, endPos - startPos); let numberOfNewLines = 0; while (scanner.getTokenPos() < endPos) { From eee44b5e206a90e8eba65663761e721684b4fd03 Mon Sep 17 00:00:00 2001 From: Minh Quy Date: Tue, 5 Apr 2022 20:07:20 +0200 Subject: [PATCH 8/9] Update src/services/organizeImports.ts Co-authored-by: Jake Bailey <5341706+jakebailey@users.noreply.github.com> --- src/services/organizeImports.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/services/organizeImports.ts b/src/services/organizeImports.ts index 636222c7aabcd..dcd9f0567f325 100644 --- a/src/services/organizeImports.ts +++ b/src/services/organizeImports.ts @@ -119,10 +119,13 @@ namespace ts.OrganizeImports { if (tokenKind === SyntaxKind.NewLineTrivia) { numberOfNewLines++; + if (numberOfNewLines >= 2) { + return true + } } } - return numberOfNewLines >= 2; + return false; } function removeUnusedImports(oldImports: readonly ImportDeclaration[], sourceFile: SourceFile, program: Program, skipDestructiveCodeActions: boolean | undefined) { From f61a657136d8a0af776b29dc5bc084c896ad3948 Mon Sep 17 00:00:00 2001 From: MQuy Date: Tue, 5 Apr 2022 20:09:08 +0200 Subject: [PATCH 9/9] Fix code format --- src/services/organizeImports.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/services/organizeImports.ts b/src/services/organizeImports.ts index dcd9f0567f325..7533a54893397 100644 --- a/src/services/organizeImports.ts +++ b/src/services/organizeImports.ts @@ -87,7 +87,7 @@ namespace ts.OrganizeImports { } } - function groupImportsByNewlineContiguous(sourceFile: SourceFile,importDecls: ImportDeclaration[]): ImportDeclaration[][] { + function groupImportsByNewlineContiguous(sourceFile: SourceFile, importDecls: ImportDeclaration[]): ImportDeclaration[][] { const scanner = createScanner(sourceFile.languageVersion, /*skipTrivia*/ false, sourceFile.languageVariant); const groupImports: ImportDeclaration[][] = []; let groupIndex = 0; @@ -119,8 +119,9 @@ namespace ts.OrganizeImports { if (tokenKind === SyntaxKind.NewLineTrivia) { numberOfNewLines++; + if (numberOfNewLines >= 2) { - return true + return true; } } }