Skip to content

Commit 0736554

Browse files
weswighamweswigham
authored
Alternate but more general token comment emit fix + organizeImports fix (microsoft#22836)
@amcasey: Preserve leading trivia when organizing imports When organizing imports, we used to move the leading and trailing trivia of each individual import with that import as it was repositioned or deleted. This had the unfortunate effect of repositioning or deleting the header comment of the file. Our new approach is to leave the leading trivia of the first import ahead of the resulting block of imports (i.e. it no longer follows the first import as it is repositioned or deleted). Trailing trivia on the first import and trivia on subsequent imports are treated as before.
1 parent 5daffbb commit 0736554

File tree

10 files changed

+86
-18
lines changed

10 files changed

+86
-18
lines changed

‎src/compiler/emitter.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1768,14 +1768,14 @@ namespace ts {
17681768
writeSemicolon();
17691769
}
17701770

1771-
function emitTokenWithComment(token: SyntaxKind, pos: number, writer: (s: string) => void, contextNode?: Node, indentLeading?: boolean) {
1772-
const node = contextNode && getParseTreeNode(contextNode);
1771+
function emitTokenWithComment(token: SyntaxKind, pos: number, writer: (s: string) => void, contextNode: Node, indentLeading?: boolean) {
1772+
const node = getParseTreeNode(contextNode);
17731773
const isSimilarNode = node && node.kind === contextNode.kind;
17741774
const startPos = pos;
17751775
if (isSimilarNode) {
17761776
pos = skipTrivia(currentSourceFile.text, pos);
17771777
}
1778-
if (emitLeadingCommentsOfPosition && isSimilarNode) {
1778+
if (emitLeadingCommentsOfPosition && isSimilarNode && contextNode.pos !== startPos) {
17791779
const needsIndent = indentLeading && !positionsAreOnSameLine(startPos, pos, currentSourceFile);
17801780
if (needsIndent) {
17811781
increaseIndent();
@@ -1786,7 +1786,7 @@ namespace ts {
17861786
}
17871787
}
17881788
pos = writeTokenText(token, writer, pos);
1789-
if (emitTrailingCommentsOfPosition && isSimilarNode) {
1789+
if (emitTrailingCommentsOfPosition && isSimilarNode && contextNode.end !== pos) {
17901790
emitTrailingCommentsOfPosition(pos, /*prefixSpace*/ true);
17911791
}
17921792
return pos;

‎src/harness/unittests/organizeImports.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -339,6 +339,28 @@ F1();
339339
},
340340
libFile);
341341

342+
testOrganizeImports("UnusedHeaderComment",
343+
{
344+
path: "/test.ts",
345+
content: `
346+
// Header
347+
import { F1 } from "lib";
348+
`,
349+
},
350+
libFile);
351+
352+
testOrganizeImports("SortHeaderComment",
353+
{
354+
path: "/test.ts",
355+
content: `
356+
// Header
357+
import "lib2";
358+
import "lib1";
359+
`,
360+
},
361+
{ path: "/lib1.ts", content: "" },
362+
{ path: "/lib2.ts", content: "" });
363+
342364
testOrganizeImports("AmbientModule",
343365
{
344366
path: "/test.ts",

‎src/services/organizeImports.ts

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,13 @@ namespace ts.OrganizeImports {
3434
return;
3535
}
3636

37+
// Special case: normally, we'd expect leading and trailing trivia to follow each import
38+
// around as it's sorted. However, we do not want this to happen for leading trivia
39+
// on the first import because it is probably the header comment for the file.
40+
// Consider: we could do a more careful check that this trivia is actually a header,
41+
// but the consequences of being wrong are very minor.
42+
suppressLeadingTrivia(oldImportDecls[0]);
43+
3744
const oldImportGroups = group(oldImportDecls, importDecl => getExternalModuleName(importDecl.moduleSpecifier));
3845
const sortedImportGroups = stableSort(oldImportGroups, (group1, group2) => compareModuleSpecifiers(group1[0].moduleSpecifier, group2[0].moduleSpecifier));
3946
const newImportDecls = flatMap(sortedImportGroups, importGroup =>
@@ -43,12 +50,15 @@ namespace ts.OrganizeImports {
4350

4451
// Delete or replace the first import.
4552
if (newImportDecls.length === 0) {
46-
changeTracker.deleteNode(sourceFile, oldImportDecls[0]);
53+
changeTracker.deleteNode(sourceFile, oldImportDecls[0], {
54+
useNonAdjustedStartPosition: true, // Leave header comment in place
55+
useNonAdjustedEndPosition: false,
56+
});
4757
}
4858
else {
4959
// Note: Delete the surrounding trivia because it will have been retained in newImportDecls.
5060
changeTracker.replaceNodeWithNodes(sourceFile, oldImportDecls[0], newImportDecls, {
51-
useNonAdjustedStartPosition: false,
61+
useNonAdjustedStartPosition: true, // Leave header comment in place
5262
useNonAdjustedEndPosition: false,
5363
suffix: getNewLineOrDefaultFromHost(host, formatContext.options),
5464
});

‎src/services/utilities.ts

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1486,14 +1486,30 @@ namespace ts {
14861486
*/
14871487
/* @internal */
14881488
export function suppressLeadingAndTrailingTrivia(node: Node) {
1489-
Debug.assertDefined(node);
1490-
suppress(node, EmitFlags.NoLeadingComments, getFirstChild);
1491-
suppress(node, EmitFlags.NoTrailingComments, getLastChild);
1492-
function suppress(node: Node, flag: EmitFlags, getChild: (n: Node) => Node) {
1493-
addEmitFlags(node, flag);
1494-
const child = getChild(node);
1495-
if (child) suppress(child, flag, getChild);
1496-
}
1489+
suppressLeadingTrivia(node);
1490+
suppressTrailingTrivia(node);
1491+
}
1492+
1493+
/**
1494+
* Sets EmitFlags to suppress leading trivia on the node.
1495+
*/
1496+
/* @internal */
1497+
export function suppressLeadingTrivia(node: Node) {
1498+
addEmitFlagsRecursively(node, EmitFlags.NoLeadingComments, getFirstChild);
1499+
}
1500+
1501+
/**
1502+
* Sets EmitFlags to suppress trailing trivia on the node.
1503+
*/
1504+
/* @internal */
1505+
export function suppressTrailingTrivia(node: Node) {
1506+
addEmitFlagsRecursively(node, EmitFlags.NoTrailingComments, getLastChild);
1507+
}
1508+
1509+
function addEmitFlagsRecursively(node: Node, flag: EmitFlags, getChild: (n: Node) => Node) {
1510+
addEmitFlags(node, flag);
1511+
const child = getChild(node);
1512+
if (child) addEmitFlagsRecursively(child, flag, getChild);
14971513
}
14981514

14991515
function getFirstChild(node: Node): Node | undefined {

‎tests/baselines/reference/organizeImports/CoalesceTrivia.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ F2();
88

99
// ==ORGANIZED==
1010

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

1313
F1();
1414
F2();

‎tests/baselines/reference/organizeImports/SortHeaderComment.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
// ==ORIGINAL==
2+
3+
// Header
4+
import "lib2";
5+
import "lib1";
6+
7+
// ==ORGANIZED==
8+
9+
// Header
10+
import "lib1";
11+
import "lib2";

‎tests/baselines/reference/organizeImports/SortTrivia.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,5 +5,5 @@
55

66
// ==ORGANIZED==
77

8-
/*F*/ import /*G*/ "lib1" /*H*/; /*I*/ //J
9-
/*A*/ import /*B*/ "lib2" /*C*/; /*D*/ //E
8+
/*A*//*F*/ import /*G*/ "lib1" /*H*/; /*I*/ //J
9+
import /*B*/ "lib2" /*C*/; /*D*/ //E

‎tests/baselines/reference/organizeImports/UnusedHeaderComment.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
// ==ORIGINAL==
2+
3+
// Header
4+
import { F1 } from "lib";
5+
6+
// ==ORGANIZED==
7+
8+
// Header

‎tests/baselines/reference/organizeImports/UnusedTrivia1.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,3 +4,4 @@
44

55
// ==ORGANIZED==
66

7+
/*A*/

‎tests/baselines/reference/organizeImports/UnusedTrivia2.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,6 @@ F1();
66

77
// ==ORGANIZED==
88

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

1111
F1();

0 commit comments

Comments
 (0)