Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 16 additions & 1 deletion src/compiler/emitter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2130,7 +2130,22 @@ namespace ts {

function emitImportDeclaration(node: ImportDeclaration) {
emitModifiers(node, node.modifiers);
emitTokenWithComment(SyntaxKind.ImportKeyword, node.modifiers ? node.modifiers.end : node.pos, writeKeyword, node);
const pos = node.modifiers ? node.modifiers.end : node.pos;
if (node.emitNode && node.emitNode.flags && (node.emitNode.flags & EmitFlags.NoLeadingComments)) {
// HACK: If we called emitTokenWithComment, it would emit the leading trivia, regardless
// of the EmitFlags on node. However, in order to be able to organize imports without
// disrupting header comments, we need to be able to control this. For now, inline the
// parts of emitTokenWithComment that do what we want.
// TODO (https://github.com/Microsoft/TypeScript/issues/22831): Remove this hack.
const keywordPos = skipTrivia(currentSourceFile.text, pos);
const trailingCommentPos = writeTokenText(SyntaxKind.ImportKeyword, writeKeyword, keywordPos);
if (emitTrailingCommentsOfPosition) {
emitTrailingCommentsOfPosition(trailingCommentPos, /*prefixSpace*/ true);
}
}
else {
emitTokenWithComment(SyntaxKind.ImportKeyword, pos, writeKeyword, node);
}
writeSpace();
if (node.importClause) {
emit(node.importClause);
Expand Down
24 changes: 24 additions & 0 deletions src/harness/unittests/organizeImports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,30 @@ F1();
},
libFile);

testOrganizeImports("UnusedHeaderComment",
{
path: "/test.ts",
content: `
// Header

import { F1 } from "lib";
`,
},
libFile);

testOrganizeImports("SortHeaderComment",
{
path: "/test.ts",
content: `
// Header

import "lib2";
import "lib1";
`,
},
{ path: "/lib1.ts", content: "" },
{ path: "/lib2.ts", content: "" });

testOrganizeImports("AmbientModule",
{
path: "/test.ts",
Expand Down
14 changes: 12 additions & 2 deletions src/services/organizeImports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,13 @@ namespace ts.OrganizeImports {
return;
}

// Special case: normally, we'd expect leading and trailing trivia to follow each import
// around as it's sorted. However, we do not want this to happen for leading trivia
// on the first import because it is probably the header comment for the file.
// Consider: we could do a more careful check that this trivia is actually a header,
// but the consequences of being wrong are very minor.
suppressLeadingTrivia(oldImportDecls[0]);

const oldImportGroups = group(oldImportDecls, importDecl => getExternalModuleName(importDecl.moduleSpecifier));
const sortedImportGroups = stableSort(oldImportGroups, (group1, group2) => compareModuleSpecifiers(group1[0].moduleSpecifier, group2[0].moduleSpecifier));
const newImportDecls = flatMap(sortedImportGroups, importGroup =>
Expand All @@ -43,12 +50,15 @@ namespace ts.OrganizeImports {

// Delete or replace the first import.
if (newImportDecls.length === 0) {
changeTracker.deleteNode(sourceFile, oldImportDecls[0]);
changeTracker.deleteNode(sourceFile, oldImportDecls[0], {
useNonAdjustedStartPosition: true, // Leave header comment in place
useNonAdjustedEndPosition: false,
});
}
else {
// Note: Delete the surrounding trivia because it will have been retained in newImportDecls.
changeTracker.replaceNodeWithNodes(sourceFile, oldImportDecls[0], newImportDecls, {
useNonAdjustedStartPosition: false,
useNonAdjustedStartPosition: true, // Leave header comment in place
useNonAdjustedEndPosition: false,
suffix: getNewLineOrDefaultFromHost(host, formatContext.options),
});
Expand Down
32 changes: 25 additions & 7 deletions src/services/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1486,14 +1486,32 @@ namespace ts {
*/
/* @internal */
export function suppressLeadingAndTrailingTrivia(node: Node) {
suppressLeadingTrivia(node);
suppressTrailingTrivia(node);
}

/**
* Sets EmitFlags to suppress leading trivia on the node.
*/
/* @internal */
export function suppressLeadingTrivia(node: Node) {
Debug.assertDefined(node);
suppress(node, EmitFlags.NoLeadingComments, getFirstChild);
suppress(node, EmitFlags.NoTrailingComments, getLastChild);
function suppress(node: Node, flag: EmitFlags, getChild: (n: Node) => Node) {
addEmitFlags(node, flag);
const child = getChild(node);
if (child) suppress(child, flag, getChild);
}
addEmitFlagsRecursively(node, EmitFlags.NoLeadingComments, getFirstChild);
}

/**
* Sets EmitFlags to suppress trailing trivia on the node.
*/
/* @internal */
export function suppressTrailingTrivia(node: Node) {
Debug.assertDefined(node);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems unnecessary as getOrCreateEmitNode will crash on undefined input anyway, and as of #22088 the type system should help with this.

addEmitFlagsRecursively(node, EmitFlags.NoTrailingComments, getLastChild);
}

function addEmitFlagsRecursively(node: Node, flag: EmitFlags, getChild: (n: Node) => Node) {
addEmitFlags(node, flag);
const child = getChild(node);
if (child) addEmitFlagsRecursively(child, flag, getChild);
}

function getFirstChild(node: Node): Node | undefined {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ F2();

// ==ORGANIZED==

/*A*/ import /*B*/ { /*L*/ F1 /*M*/, /*C*/ F2 /*D*/ } /*E*/ from /*F*/ "lib" /*G*/; /*H*/ //I
/*A*/import /*B*/ { /*L*/ F1 /*M*/, /*C*/ F2 /*D*/ } /*E*/ from /*F*/ "lib" /*G*/; /*H*/ //I

F1();
F2();
13 changes: 13 additions & 0 deletions tests/baselines/reference/organizeImports/SortHeaderComment.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// ==ORIGINAL==

// Header

import "lib2";
import "lib1";

// ==ORGANIZED==

// Header

import "lib1";
import "lib2";
4 changes: 2 additions & 2 deletions tests/baselines/reference/organizeImports/SortTrivia.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,5 @@

// ==ORGANIZED==

/*F*/ import /*G*/ "lib1" /*H*/; /*I*/ //J
/*A*/ import /*B*/ "lib2" /*C*/; /*D*/ //E
/*A*//*F*/ import /*G*/ "lib1" /*H*/; /*I*/ //J
import /*B*/ "lib2" /*C*/; /*D*/ //E
10 changes: 10 additions & 0 deletions tests/baselines/reference/organizeImports/UnusedHeaderComment.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// ==ORIGINAL==

// Header

import { F1 } from "lib";

// ==ORGANIZED==

// Header

1 change: 1 addition & 0 deletions tests/baselines/reference/organizeImports/UnusedTrivia1.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,4 @@

// ==ORGANIZED==

/*A*/
2 changes: 1 addition & 1 deletion tests/baselines/reference/organizeImports/UnusedTrivia2.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,6 @@ F1();

// ==ORGANIZED==

/*A*/ import /*B*/ { /*C*/ F1 /*D*/ } /*G*/ from /*H*/ "lib" /*I*/; /*J*/ //K
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure why we lost the space, but it doesn't seem very problematic since this commenting pattern is very unusual.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like the space wasn't present in the input, so this is a good change.

/*A*/import /*B*/ { /*C*/ F1 /*D*/ } /*G*/ from /*H*/ "lib" /*I*/; /*J*/ //K

F1();