Alternate but more general token comment emit fix + organizeImports fix (#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.
This commit is contained in:
Wesley Wigham
2018-03-23 14:13:40 -07:00
committed by GitHub
parent 5daffbbad1
commit 0736554db7
10 changed files with 86 additions and 18 deletions

View File

@@ -1768,14 +1768,14 @@ namespace ts {
writeSemicolon();
}
function emitTokenWithComment(token: SyntaxKind, pos: number, writer: (s: string) => void, contextNode?: Node, indentLeading?: boolean) {
const node = contextNode && getParseTreeNode(contextNode);
function emitTokenWithComment(token: SyntaxKind, pos: number, writer: (s: string) => void, contextNode: Node, indentLeading?: boolean) {
const node = getParseTreeNode(contextNode);
const isSimilarNode = node && node.kind === contextNode.kind;
const startPos = pos;
if (isSimilarNode) {
pos = skipTrivia(currentSourceFile.text, pos);
}
if (emitLeadingCommentsOfPosition && isSimilarNode) {
if (emitLeadingCommentsOfPosition && isSimilarNode && contextNode.pos !== startPos) {
const needsIndent = indentLeading && !positionsAreOnSameLine(startPos, pos, currentSourceFile);
if (needsIndent) {
increaseIndent();
@@ -1786,7 +1786,7 @@ namespace ts {
}
}
pos = writeTokenText(token, writer, pos);
if (emitTrailingCommentsOfPosition && isSimilarNode) {
if (emitTrailingCommentsOfPosition && isSimilarNode && contextNode.end !== pos) {
emitTrailingCommentsOfPosition(pos, /*prefixSpace*/ true);
}
return pos;

View File

@@ -339,6 +339,28 @@ 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",

View File

@@ -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 =>
@@ -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),
});

View File

@@ -1486,14 +1486,30 @@ namespace ts {
*/
/* @internal */
export function suppressLeadingAndTrailingTrivia(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);
}
suppressLeadingTrivia(node);
suppressTrailingTrivia(node);
}
/**
* Sets EmitFlags to suppress leading trivia on the node.
*/
/* @internal */
export function suppressLeadingTrivia(node: Node) {
addEmitFlagsRecursively(node, EmitFlags.NoLeadingComments, getFirstChild);
}
/**
* Sets EmitFlags to suppress trailing trivia on the node.
*/
/* @internal */
export function suppressTrailingTrivia(node: Node) {
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 {

View File

@@ -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();

View File

@@ -0,0 +1,11 @@
// ==ORIGINAL==
// Header
import "lib2";
import "lib1";
// ==ORGANIZED==
// Header
import "lib1";
import "lib2";

View File

@@ -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

View File

@@ -0,0 +1,8 @@
// ==ORIGINAL==
// Header
import { F1 } from "lib";
// ==ORGANIZED==
// Header

View File

@@ -4,3 +4,4 @@
// ==ORGANIZED==
/*A*/

View File

@@ -6,6 +6,6 @@ F1();
// ==ORGANIZED==
/*A*/ import /*B*/ { /*C*/ F1 /*D*/ } /*G*/ from /*H*/ "lib" /*I*/; /*J*/ //K
/*A*/import /*B*/ { /*C*/ F1 /*D*/ } /*G*/ from /*H*/ "lib" /*I*/; /*J*/ //K
F1();