[Feature] - Automatically create sort groups based on newlines (#48330)

Minh Quy · jakebailey · web-flow · commit 6e0447fdf165 · 2022-04-05T11:53:19.000-07:00
Co-authored-by: Jake Bailey &lt;5341706+jakebailey@users.noreply.github.com&gt;
diff --git a/src/services/organizeImports.ts b/src/services/organizeImports.ts
@@ -15,16 +15,15 @@ namespace ts.OrganizeImports {
         preferences: UserPreferences,
         skipDestructiveCodeActions?: boolean
     ) {
-
         const changeTracker = textChanges.ChangeTracker.fromContext({ host, formatContext, preferences });
 
         const coalesceAndOrganizeImports = (importGroup: readonly ImportDeclaration[]) => stableSort(
             coalesceImports(removeUnusedImports(importGroup, sourceFile, program, skipDestructiveCodeActions)),
             (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, sourceFile.statements.filter(isImportDeclaration));
+        topLevelImportGroupDecls.forEach(importGroupDecl => organizeImportsWorker(importGroupDecl, 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(sourceFile, ambientModule.body.statements.filter(isImportDeclaration));
+            ambientModuleImportGroupDecls.forEach(importGroupDecl => organizeImportsWorker(importGroupDecl, coalesceAndOrganizeImports));
 
             const ambientModuleExportDecls = ambientModule.body.statements.filter(isExportDeclaration);
             organizeImportsWorker(ambientModuleExportDecls, coalesceExports);
@@ -88,6 +87,48 @@ 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 (isNewGroup(sourceFile, topLevelImportDecl, scanner)) {
+                groupIndex++;
+            }
+
+            if (!groupImports[groupIndex]) {
+                groupImports[groupIndex] = [];
+            }
+
+            groupImports[groupIndex].push(topLevelImportDecl);
+        }
+
+        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 isNewGroup(sourceFile: SourceFile, topLevelImportDecl: ImportDeclaration, scanner: Scanner) {
+        const startPos = topLevelImportDecl.getFullStart();
+        const endPos = topLevelImportDecl.getStart();
+        scanner.setText(sourceFile.text, startPos, endPos - startPos);
+
+        let numberOfNewLines = 0;
+        while (scanner.getTokenPos() < endPos) {
+            const tokenKind = scanner.scan();
+
+            if (tokenKind === SyntaxKind.NewLineTrivia) {
+                numberOfNewLines++;
+
+                if (numberOfNewLines >= 2) {
+                    return true;
+                }
+            }
+        }
+
+        return false;
+    }
+
     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/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
diff --git 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();
diff --git 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;`);
+anotherThing;`);
diff --git a/tests/cases/fourslash/organizeImportsGroup_CommentInNewline.ts b/tests/cases/fourslash/organizeImportsGroup_CommentInNewline.ts
@@ -0,0 +1,21 @@
+/// <reference path="fourslash.ts" />
+
+////// 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/organizeImportsGroup_MultiNewlines.ts b/tests/cases/fourslash/organizeImportsGroup_MultiNewlines.ts
@@ -0,0 +1,21 @@
+/// <reference path="fourslash.ts" />
+
+////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)`
+);
diff --git a/tests/cases/fourslash/organizeImportsGroup_MultilineCommentInNewline.ts b/tests/cases/fourslash/organizeImportsGroup_MultilineCommentInNewline.ts
@@ -0,0 +1,25 @@
+/// <reference path="fourslash.ts" />
+
+////// 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
@@ -0,0 +1,19 @@
+/// <reference path="fourslash.ts" />
+
+////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)`
+);

Original file line number	Diff line number	Diff line change
`@@ -17,7 +17,6 @@ D();`
`17`	`17`
`18`	`18`	`// ==ORGANIZED==`
`19`	`19`
`20`		`-import "lib";`
`21`	`20`	`import D from "lib";`
`22`	`21`
`23`	`22`	`declare module "mod" {`
`@@ -26,5 +25,6 @@ declare module "mod" {`
`26`	`25`	`function F(f1: {} = F1, f2: {} = F2) {}`
`27`	`26`	`}`
`28`	`27`
	`28`	`+import "lib";`
`29`	`29`
`30`	`30`	`D();`