From eef3da5b6bf9372ea2899e0cd4fadcb33e0948cd Mon Sep 17 00:00:00 2001 From: Gabriela Araujo Britto Date: Fri, 15 Feb 2019 15:38:10 -0800 Subject: [PATCH] create new ConfigurableStart and ConfigurableEnd options and rename them --- src/services/organizeImports.ts | 4 +- .../refactors/convertToNamedParameters.ts | 8 ++- src/services/textChanges.ts | 69 ++++++++++++------- .../unittests/services/textChanges.ts | 26 +++---- 4 files changed, 65 insertions(+), 42 deletions(-) diff --git a/src/services/organizeImports.ts b/src/services/organizeImports.ts index 157ba98e262..8096ffa4ad2 100644 --- a/src/services/organizeImports.ts +++ b/src/services/organizeImports.ts @@ -68,8 +68,8 @@ namespace ts.OrganizeImports { else { // Note: Delete the surrounding trivia because it will have been retained in newImportDecls. changeTracker.replaceNodeWithNodes(sourceFile, oldImportDecls[0], newImportDecls, { - useNonAdjustedStartPosition: true, // Leave header comment in place - useNonAdjustedEndPosition: false, + startPosition: textChanges.LeadingTriviaOption.Exclude, // Leave header comment in place + endPosition: textChanges.TrailingTriviaOption.Include, suffix: getNewLineOrDefaultFromHost(host, formatContext.options), }); } diff --git a/src/services/refactors/convertToNamedParameters.ts b/src/services/refactors/convertToNamedParameters.ts index 6e5881a31f6..cf348bd9e73 100644 --- a/src/services/refactors/convertToNamedParameters.ts +++ b/src/services/refactors/convertToNamedParameters.ts @@ -47,7 +47,13 @@ namespace ts.refactor.convertToNamedParameters { first(functionDeclaration.parameters), last(functionDeclaration.parameters), newParamDeclaration, - { joiner: ", ", indentation: 0 }); // indentation is set to 0 because otherwise the object parameter will be indented if there is a `this` parameter + { joiner: ", ", + // indentation is set to 0 because otherwise the object parameter will be indented if there is a `this` parameter + indentation: 0, + startPosition: textChanges.LeadingTriviaOption.IncludeAll, + endPosition: textChanges.TrailingTriviaOption.Include + }); + const functionCalls = groupedReferences.calls; forEach(functionCalls, call => { diff --git a/src/services/textChanges.ts b/src/services/textChanges.ts index 4689e18508c..546fddaca14 100644 --- a/src/services/textChanges.ts +++ b/src/services/textChanges.ts @@ -28,17 +28,32 @@ namespace ts.textChanges { } export interface ConfigurableStart { - /** True to use getStart() (NB, not getFullStart()) without adjustment. */ - useNonAdjustedStartPosition?: boolean; + startPosition?: LeadingTriviaOption; } export interface ConfigurableEnd { /** True to use getEnd() without adjustment. */ - useNonAdjustedEndPosition?: boolean; + endPosition?: TrailingTriviaOption; } - export enum Position { - FullStart, - Start + export enum LeadingTriviaOption { + /** Exclude all leading trivia (use getStart()) */ + Exclude, + /** Include leading trivia (default behavior) */ + Include, + /** Include leading trivia and, + * if there are no line breaks between the node and the previous token, + * include all trivia between the node and the previous token + */ + IncludeAll, + } + + export enum TrailingTriviaOption { + /** Exclude all leading trivia (use getEnd()) */ + Exclude, + /** TODO (default behavior) */ + IncludeIfLineBreak, + /** Include trailing trivia */ + Include, } function skipWhitespacesAndLineBreaks(text: string, start: number) { @@ -73,8 +88,8 @@ namespace ts.textChanges { export interface ConfigurableStartEnd extends ConfigurableStart, ConfigurableEnd {} export const useNonAdjustedPositions: ConfigurableStartEnd = { - useNonAdjustedStartPosition: true, - useNonAdjustedEndPosition: true, + startPosition: LeadingTriviaOption.Exclude, + endPosition: TrailingTriviaOption.Exclude, }; export interface InsertNodeOptions { @@ -143,11 +158,12 @@ namespace ts.textChanges { } function getAdjustedRange(sourceFile: SourceFile, startNode: Node, endNode: Node, options: ConfigurableStartEnd): TextRange { - return { pos: getAdjustedStartPosition(sourceFile, startNode, options, Position.Start), end: getAdjustedEndPosition(sourceFile, endNode, options) }; + return { pos: getAdjustedStartPosition(sourceFile, startNode, options), end: getAdjustedEndPosition(sourceFile, endNode, options) }; } - function getAdjustedStartPosition(sourceFile: SourceFile, node: Node, options: ConfigurableStart, position: Position) { - if (options.useNonAdjustedStartPosition) { + function getAdjustedStartPosition(sourceFile: SourceFile, node: Node, options: ConfigurableStart) { + const { startPosition } = options; + if (startPosition === LeadingTriviaOption.Exclude) { return node.getStart(sourceFile); } const fullStart = node.getFullStart(); @@ -165,7 +181,7 @@ namespace ts.textChanges { // fullstart // when b is replaced - we usually want to keep the leading trvia // when b is deleted - we delete it - return position === Position.Start ? start : fullStart; + return startPosition === LeadingTriviaOption.IncludeAll ? fullStart : start; } // get start position of the line following the line that contains fullstart position // (but only if the fullstart isn't the very beginning of the file) @@ -178,11 +194,12 @@ namespace ts.textChanges { function getAdjustedEndPosition(sourceFile: SourceFile, node: Node, options: ConfigurableEnd) { const { end } = node; - if (options.useNonAdjustedEndPosition || isExpression(node)) { + const { endPosition } = options; + if (endPosition === TrailingTriviaOption.Exclude || isExpression(node)) { return end; } const newEnd = skipTrivia(sourceFile.text, end, /*stopAfterLineBreak*/ true); - return newEnd !== end && isLineBreak(sourceFile.text.charCodeAt(newEnd - 1)) + return newEnd !== end && (endPosition === TrailingTriviaOption.Include || isLineBreak(sourceFile.text.charCodeAt(newEnd - 1))) ? newEnd : end; } @@ -240,15 +257,15 @@ namespace ts.textChanges { this.deleteRange(sourceFile, { pos: modifier.getStart(sourceFile), end: skipTrivia(sourceFile.text, modifier.end, /*stopAfterLineBreak*/ true) }); } - public deleteNodeRange(sourceFile: SourceFile, startNode: Node, endNode: Node, options: ConfigurableStartEnd = {}): void { - const startPosition = getAdjustedStartPosition(sourceFile, startNode, options, Position.FullStart); + public deleteNodeRange(sourceFile: SourceFile, startNode: Node, endNode: Node, options: ConfigurableStartEnd = { startPosition: LeadingTriviaOption.IncludeAll }): void { + const startPosition = getAdjustedStartPosition(sourceFile, startNode, options); const endPosition = getAdjustedEndPosition(sourceFile, endNode, options); this.deleteRange(sourceFile, { pos: startPosition, end: endPosition }); } - public deleteNodeRangeExcludingEnd(sourceFile: SourceFile, startNode: Node, afterEndNode: Node | undefined, options: ConfigurableStartEnd = {}): void { - const startPosition = getAdjustedStartPosition(sourceFile, startNode, options, Position.FullStart); - const endPosition = afterEndNode === undefined ? sourceFile.text.length : getAdjustedStartPosition(sourceFile, afterEndNode, options, Position.FullStart); + public deleteNodeRangeExcludingEnd(sourceFile: SourceFile, startNode: Node, afterEndNode: Node | undefined, options: ConfigurableStartEnd = { startPosition: LeadingTriviaOption.IncludeAll }): void { + const startPosition = getAdjustedStartPosition(sourceFile, startNode, options); + const endPosition = afterEndNode === undefined ? sourceFile.text.length : getAdjustedStartPosition(sourceFile, afterEndNode, options); this.deleteRange(sourceFile, { pos: startPosition, end: endPosition }); } @@ -307,7 +324,7 @@ namespace ts.textChanges { } public insertNodeBefore(sourceFile: SourceFile, before: Node, newNode: Node, blankLineBetween = false): void { - this.insertNodeAt(sourceFile, getAdjustedStartPosition(sourceFile, before, {}, Position.Start), newNode, this.getOptionsForInsertNodeBefore(before, blankLineBetween)); + this.insertNodeAt(sourceFile, getAdjustedStartPosition(sourceFile, before, {}), newNode, this.getOptionsForInsertNodeBefore(before, blankLineBetween)); } public insertModifierBefore(sourceFile: SourceFile, modifier: SyntaxKind, before: Node): void { @@ -427,7 +444,7 @@ namespace ts.textChanges { } public insertNodeAtEndOfScope(sourceFile: SourceFile, scope: Node, newNode: Node): void { - const pos = getAdjustedStartPosition(sourceFile, scope.getLastToken()!, {}, Position.Start); + const pos = getAdjustedStartPosition(sourceFile, scope.getLastToken()!, {}); this.insertNodeAt(sourceFile, pos, newNode, { prefix: isLineBreak(sourceFile.text.charCodeAt(scope.getLastToken()!.pos)) ? this.newLineCharacter : this.newLineCharacter + this.newLineCharacter, suffix: this.newLineCharacter @@ -736,7 +753,7 @@ namespace ts.textChanges { // find first non-whitespace position in the leading trivia of the node function startPositionToDeleteNodeInList(sourceFile: SourceFile, node: Node): number { - return skipTrivia(sourceFile.text, getAdjustedStartPosition(sourceFile, node, {}, Position.FullStart), /*stopAfterLineBreak*/ false, /*stopAtComments*/ true); + return skipTrivia(sourceFile.text, getAdjustedStartPosition(sourceFile, node, { startPosition: LeadingTriviaOption.IncludeAll }), /*stopAfterLineBreak*/ false, /*stopAtComments*/ true); } function getClassOrObjectBraceEnds(cls: ClassLikeDeclaration | InterfaceDeclaration | ObjectLiteralExpression, sourceFile: SourceFile): [number, number] { @@ -1090,7 +1107,7 @@ namespace ts.textChanges { case SyntaxKind.ImportDeclaration: deleteNode(changes, sourceFile, node, // For first import, leave header comment in place - node === sourceFile.imports[0].parent ? { useNonAdjustedStartPosition: true, useNonAdjustedEndPosition: false } : undefined); + node === sourceFile.imports[0].parent ? { startPosition: LeadingTriviaOption.Exclude, endPosition: TrailingTriviaOption.IncludeIfLineBreak } : undefined); break; case SyntaxKind.BindingElement: @@ -1134,7 +1151,7 @@ namespace ts.textChanges { deleteNodeInList(changes, deletedNodesInLists, sourceFile, node); } else { - deleteNode(changes, sourceFile, node, node.kind === SyntaxKind.SemicolonToken ? { useNonAdjustedEndPosition: true } : undefined); + deleteNode(changes, sourceFile, node, node.kind === SyntaxKind.SemicolonToken ? { endPosition: TrailingTriviaOption.Exclude } : undefined); } } } @@ -1213,8 +1230,8 @@ namespace ts.textChanges { /** Warning: This deletes comments too. See `copyComments` in `convertFunctionToEs6Class`. */ // Exported for tests only! (TODO: improve tests to not need this) - export function deleteNode(changes: ChangeTracker, sourceFile: SourceFile, node: Node, options: ConfigurableStartEnd = {}): void { - const startPosition = getAdjustedStartPosition(sourceFile, node, options, Position.FullStart); + export function deleteNode(changes: ChangeTracker, sourceFile: SourceFile, node: Node, options: ConfigurableStartEnd = { startPosition: LeadingTriviaOption.IncludeAll }): void { + const startPosition = getAdjustedStartPosition(sourceFile, node, options); const endPosition = getAdjustedEndPosition(sourceFile, node, options); changes.deleteRange(sourceFile, { pos: startPosition, end: endPosition }); } diff --git a/src/testRunner/unittests/services/textChanges.ts b/src/testRunner/unittests/services/textChanges.ts index de92efafbeb..f9e03a578de 100644 --- a/src/testRunner/unittests/services/textChanges.ts +++ b/src/testRunner/unittests/services/textChanges.ts @@ -140,13 +140,13 @@ var z = 3; // comment 4 deleteNode(changeTracker, sourceFile, findVariableStatementContaining("y", sourceFile)); }); runSingleFileTest("deleteNode2", /*placeOpenBraceOnNewLineForFunctions*/ false, text, /*validateNodes*/ false, (sourceFile, changeTracker) => { - deleteNode(changeTracker, sourceFile, findVariableStatementContaining("y", sourceFile), { useNonAdjustedStartPosition: true }); + deleteNode(changeTracker, sourceFile, findVariableStatementContaining("y", sourceFile), { startPosition: textChanges.LeadingTriviaOption.Exclude }); }); runSingleFileTest("deleteNode3", /*placeOpenBraceOnNewLineForFunctions*/ false, text, /*validateNodes*/ false, (sourceFile, changeTracker) => { - deleteNode(changeTracker, sourceFile, findVariableStatementContaining("y", sourceFile), { useNonAdjustedEndPosition: true }); + deleteNode(changeTracker, sourceFile, findVariableStatementContaining("y", sourceFile), { endPosition: textChanges.TrailingTriviaOption.Exclude }); }); runSingleFileTest("deleteNode4", /*placeOpenBraceOnNewLineForFunctions*/ false, text, /*validateNodes*/ false, (sourceFile, changeTracker) => { - deleteNode(changeTracker, sourceFile, findVariableStatementContaining("y", sourceFile), { useNonAdjustedStartPosition: true, useNonAdjustedEndPosition: true }); + deleteNode(changeTracker, sourceFile, findVariableStatementContaining("y", sourceFile), { startPosition: textChanges.LeadingTriviaOption.Exclude, endPosition: textChanges.TrailingTriviaOption.Exclude }); }); runSingleFileTest("deleteNode5", /*placeOpenBraceOnNewLineForFunctions*/ false, text, /*validateNodes*/ false, (sourceFile, changeTracker) => { deleteNode(changeTracker, sourceFile, findVariableStatementContaining("x", sourceFile)); @@ -167,15 +167,15 @@ var a = 4; // comment 7 }); runSingleFileTest("deleteNodeRange2", /*placeOpenBraceOnNewLineForFunctions*/ false, text, /*validateNodes*/ false, (sourceFile, changeTracker) => { changeTracker.deleteNodeRange(sourceFile, findVariableStatementContaining("y", sourceFile), findVariableStatementContaining("z", sourceFile), - { useNonAdjustedStartPosition: true }); + { startPosition: textChanges.LeadingTriviaOption.Exclude }); }); runSingleFileTest("deleteNodeRange3", /*placeOpenBraceOnNewLineForFunctions*/ false, text, /*validateNodes*/ false, (sourceFile, changeTracker) => { changeTracker.deleteNodeRange(sourceFile, findVariableStatementContaining("y", sourceFile), findVariableStatementContaining("z", sourceFile), - { useNonAdjustedEndPosition: true }); + { endPosition: textChanges.TrailingTriviaOption.Exclude }); }); runSingleFileTest("deleteNodeRange4", /*placeOpenBraceOnNewLineForFunctions*/ false, text, /*validateNodes*/ false, (sourceFile, changeTracker) => { changeTracker.deleteNodeRange(sourceFile, findVariableStatementContaining("y", sourceFile), findVariableStatementContaining("z", sourceFile), - { useNonAdjustedStartPosition: true, useNonAdjustedEndPosition: true }); + { startPosition: textChanges.LeadingTriviaOption.Exclude, endPosition: textChanges.TrailingTriviaOption.Exclude }); }); } function createTestVariableDeclaration(name: string) { @@ -254,16 +254,16 @@ var a = 4; // comment 7`; changeTracker.replaceNode(sourceFile, findVariableStatementContaining("y", sourceFile), createTestClass(), { suffix: newLineCharacter }); }); runSingleFileTest("replaceNode2", /*placeOpenBraceOnNewLineForFunctions*/ true, text, /*validateNodes*/ true, (sourceFile, changeTracker) => { - changeTracker.replaceNode(sourceFile, findVariableStatementContaining("y", sourceFile), createTestClass(), { useNonAdjustedStartPosition: true, suffix: newLineCharacter, prefix: newLineCharacter }); + changeTracker.replaceNode(sourceFile, findVariableStatementContaining("y", sourceFile), createTestClass(), { startPosition: textChanges.LeadingTriviaOption.Exclude, suffix: newLineCharacter, prefix: newLineCharacter }); }); runSingleFileTest("replaceNode3", /*placeOpenBraceOnNewLineForFunctions*/ true, text, /*validateNodes*/ true, (sourceFile, changeTracker) => { - changeTracker.replaceNode(sourceFile, findVariableStatementContaining("y", sourceFile), createTestClass(), { useNonAdjustedEndPosition: true, suffix: newLineCharacter }); + changeTracker.replaceNode(sourceFile, findVariableStatementContaining("y", sourceFile), createTestClass(), { endPosition: textChanges.TrailingTriviaOption.Exclude, suffix: newLineCharacter }); }); runSingleFileTest("replaceNode4", /*placeOpenBraceOnNewLineForFunctions*/ true, text, /*validateNodes*/ true, (sourceFile, changeTracker) => { - changeTracker.replaceNode(sourceFile, findVariableStatementContaining("y", sourceFile), createTestClass(), { useNonAdjustedStartPosition: true, useNonAdjustedEndPosition: true }); + changeTracker.replaceNode(sourceFile, findVariableStatementContaining("y", sourceFile), createTestClass(), { startPosition: textChanges.LeadingTriviaOption.Exclude, endPosition: textChanges.TrailingTriviaOption.Exclude }); }); runSingleFileTest("replaceNode5", /*placeOpenBraceOnNewLineForFunctions*/ true, text, /*validateNodes*/ true, (sourceFile, changeTracker) => { - changeTracker.replaceNode(sourceFile, findVariableStatementContaining("x", sourceFile), createTestClass(), { useNonAdjustedStartPosition: true, useNonAdjustedEndPosition: true }); + changeTracker.replaceNode(sourceFile, findVariableStatementContaining("x", sourceFile), createTestClass(), { startPosition: textChanges.LeadingTriviaOption.Exclude, endPosition: textChanges.TrailingTriviaOption.Exclude }); }); } { @@ -279,13 +279,13 @@ var a = 4; // comment 7`; changeTracker.replaceNodeRange(sourceFile, findVariableStatementContaining("y", sourceFile), findVariableStatementContaining("z", sourceFile), createTestClass(), { suffix: newLineCharacter }); }); runSingleFileTest("replaceNodeRange2", /*placeOpenBraceOnNewLineForFunctions*/ true, text, /*validateNodes*/ true, (sourceFile, changeTracker) => { - changeTracker.replaceNodeRange(sourceFile, findVariableStatementContaining("y", sourceFile), findVariableStatementContaining("z", sourceFile), createTestClass(), { useNonAdjustedStartPosition: true, suffix: newLineCharacter, prefix: newLineCharacter }); + changeTracker.replaceNodeRange(sourceFile, findVariableStatementContaining("y", sourceFile), findVariableStatementContaining("z", sourceFile), createTestClass(), { startPosition: textChanges.LeadingTriviaOption.Exclude, suffix: newLineCharacter, prefix: newLineCharacter }); }); runSingleFileTest("replaceNodeRange3", /*placeOpenBraceOnNewLineForFunctions*/ true, text, /*validateNodes*/ true, (sourceFile, changeTracker) => { - changeTracker.replaceNodeRange(sourceFile, findVariableStatementContaining("y", sourceFile), findVariableStatementContaining("z", sourceFile), createTestClass(), { useNonAdjustedEndPosition: true, suffix: newLineCharacter }); + changeTracker.replaceNodeRange(sourceFile, findVariableStatementContaining("y", sourceFile), findVariableStatementContaining("z", sourceFile), createTestClass(), { endPosition: textChanges.TrailingTriviaOption.Exclude, suffix: newLineCharacter }); }); runSingleFileTest("replaceNodeRange4", /*placeOpenBraceOnNewLineForFunctions*/ true, text, /*validateNodes*/ true, (sourceFile, changeTracker) => { - changeTracker.replaceNodeRange(sourceFile, findVariableStatementContaining("y", sourceFile), findVariableStatementContaining("z", sourceFile), createTestClass(), { useNonAdjustedStartPosition: true, useNonAdjustedEndPosition: true }); + changeTracker.replaceNodeRange(sourceFile, findVariableStatementContaining("y", sourceFile), findVariableStatementContaining("z", sourceFile), createTestClass(), { startPosition: textChanges.LeadingTriviaOption.Exclude, endPosition: textChanges.TrailingTriviaOption.Exclude }); }); } {