Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Automatically create sort groups based on newlines in organize imports #48330

Merged
47 changes: 42 additions & 5 deletions src/services/organizeImports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -88,6 +87,44 @@ namespace ts.OrganizeImports {
}
}

function groupImportsByNewlineContiguous(sourceFile: SourceFile,importDecls: ImportDeclaration[]): ImportDeclaration[][] {
MQuy marked this conversation as resolved.
Show resolved Hide resolved
const groupImports: ImportDeclaration[][] = [];
let groupIndex = 0;
for (const topLevelImportDecl of importDecls) {
if (isStartedNewGroup(sourceFile, topLevelImportDecl)) {
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 isStartedNewGroup(sourceFile: SourceFile, topLevelImportDecl: ImportDeclaration) {
MQuy marked this conversation as resolved.
Show resolved Hide resolved
const startPos = topLevelImportDecl.getFullStart();
const endPos = topLevelImportDecl.getStart();

const scanner = createScanner(sourceFile.languageVersion, /*skipTrivia*/ false, sourceFile.languageVariant, sourceFile.text, /*onError:*/ undefined, startPos);
jakebailey marked this conversation as resolved.
Show resolved Hide resolved

let numberOfNewLines = 0;
while (scanner.getTokenPos() < endPos) {
const tokenKind = scanner.scan();

if (tokenKind === SyntaxKind.NewLineTrivia) {
numberOfNewLines++;
}
}

return numberOfNewLines >= 2;
MQuy marked this conversation as resolved.
Show resolved Hide resolved
}

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) {
Expand Down
17 changes: 0 additions & 17 deletions src/testRunner/unittests/services/organizeImports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
{
Expand Down
17 changes: 0 additions & 17 deletions tests/baselines/reference/organizeImports/SortComments.ts

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ D();

// ==ORGANIZED==

import "lib";
import D from "lib";

declare module "mod" {
Expand All @@ -26,5 +25,6 @@ declare module "mod" {
function F(f1: {} = F1, f2: {} = F2) {}
}

import "lib";
jakebailey marked this conversation as resolved.
Show resolved Hide resolved

D();
10 changes: 8 additions & 2 deletions tests/cases/fourslash/organizeImports6.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;`);
21 changes: 21 additions & 0 deletions tests/cases/fourslash/organizeImportsGroup_CommentInNewline.ts
Original file line number Diff line number Diff line change
@@ -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)`
);
21 changes: 21 additions & 0 deletions tests/cases/fourslash/organizeImportsGroup_MultiNewlines.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
/// <reference path="fourslash.ts" />

////import c from "C";
////
////
jakebailey marked this conversation as resolved.
Show resolved Hide resolved
////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)`
);
Original file line number Diff line number Diff line change
@@ -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)`
);
19 changes: 19 additions & 0 deletions tests/cases/fourslash/organizeImportsGroup_Newline.ts
Original file line number Diff line number Diff line change
@@ -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)`
);