From a2e9e6919d3c9302a205a82d470f55ee4173fad3 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Wed, 8 Apr 2020 14:01:18 -0700 Subject: [PATCH 1/2] Fix preserveNewlines printer option when a list child has the same start or end as its parent --- src/compiler/emitter.ts | 2 ++ src/compiler/utilities.ts | 12 ++++---- .../fourslash/textChangesPreserveNewlines8.ts | 29 +++++++++++++++++++ 3 files changed, 37 insertions(+), 6 deletions(-) create mode 100644 tests/cases/fourslash/textChangesPreserveNewlines8.ts diff --git a/src/compiler/emitter.ts b/src/compiler/emitter.ts index 3f52f4e9ee9e5..974c168c83b7c 100644 --- a/src/compiler/emitter.ts +++ b/src/compiler/emitter.ts @@ -4301,6 +4301,7 @@ namespace ts { return getEffectiveLines( includeComments => getLinesBetweenPositionAndPrecedingNonWhitespaceCharacter( firstChild.pos, + parentNode.pos, currentSourceFile!, includeComments)); } @@ -4358,6 +4359,7 @@ namespace ts { return getEffectiveLines( includeComments => getLinesBetweenPositionAndNextNonWhitespaceCharacter( lastChild.end, + parentNode.end, currentSourceFile!, includeComments)); } diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index a5235eeae2308..9dbfaf19c56f0 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -4776,19 +4776,19 @@ namespace ts { return positionIsSynthesized(range.pos) ? -1 : skipTrivia(sourceFile.text, range.pos, /*stopAfterLineBreak*/ false, includeComments); } - export function getLinesBetweenPositionAndPrecedingNonWhitespaceCharacter(pos: number, sourceFile: SourceFile, includeComments?: boolean) { + export function getLinesBetweenPositionAndPrecedingNonWhitespaceCharacter(pos: number, stopPos: number, sourceFile: SourceFile, includeComments?: boolean) { const startPos = skipTrivia(sourceFile.text, pos, /*stopAfterLineBreak*/ false, includeComments); - const prevPos = getPreviousNonWhitespacePosition(startPos, sourceFile); + const prevPos = getPreviousNonWhitespacePosition(startPos, stopPos, sourceFile); return getLinesBetweenPositions(sourceFile, prevPos || 0, startPos); } - export function getLinesBetweenPositionAndNextNonWhitespaceCharacter(pos: number, sourceFile: SourceFile, includeComments?: boolean) { + export function getLinesBetweenPositionAndNextNonWhitespaceCharacter(pos: number, stopPos: number, sourceFile: SourceFile, includeComments?: boolean) { const nextPos = skipTrivia(sourceFile.text, pos, /*stopAfterLineBreak*/ false, includeComments); - return getLinesBetweenPositions(sourceFile, pos, nextPos); + return getLinesBetweenPositions(sourceFile, pos, Math.min(stopPos, nextPos)); } - function getPreviousNonWhitespacePosition(pos: number, sourceFile: SourceFile) { - while (pos-- > 0) { + function getPreviousNonWhitespacePosition(pos: number, stopPos = 0, sourceFile: SourceFile) { + while (pos-- > stopPos) { if (!isWhiteSpaceLike(sourceFile.text.charCodeAt(pos))) { return pos; } diff --git a/tests/cases/fourslash/textChangesPreserveNewlines8.ts b/tests/cases/fourslash/textChangesPreserveNewlines8.ts new file mode 100644 index 0000000000000..e74aef6c9d00f --- /dev/null +++ b/tests/cases/fourslash/textChangesPreserveNewlines8.ts @@ -0,0 +1,29 @@ +// #37813 + +/// + +////function foo() { +//// /*1*/var x: number +//// +//// x = 10; +//// return x;/*2*/ +////} + +goTo.select("1", "2"); +edit.applyRefactor({ + refactorName: "Extract Symbol", + actionName: "function_scope_1", + actionDescription: "Extract to function in global scope", + newContent: +`function foo() { + return /*RENAME*/newFunction(); +} + +function newFunction() { + var x: number; + + x = 10; + return x; +} +` +}); From a15ea78a12d9e81d554afcff59250747da75a2b1 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Wed, 8 Apr 2020 14:28:53 -0700 Subject: [PATCH 2/2] Fix leading line separator calculation and JSX bug --- src/compiler/emitter.ts | 30 +++++++++++++++++++++--------- src/compiler/utilities.ts | 2 +- 2 files changed, 22 insertions(+), 10 deletions(-) diff --git a/src/compiler/emitter.ts b/src/compiler/emitter.ts index 974c168c83b7c..31dbd50c048c1 100644 --- a/src/compiler/emitter.ts +++ b/src/compiler/emitter.ts @@ -2365,16 +2365,10 @@ namespace ts { function emitParenthesizedExpression(node: ParenthesizedExpression) { const openParenPos = emitTokenWithComment(SyntaxKind.OpenParenToken, node.pos, writePunctuation, node); - const leadingNewlines = preserveSourceNewlines && getLeadingLineTerminatorCount(node, [node.expression], ListFormat.None); - if (leadingNewlines) { - writeLinesAndIndent(leadingNewlines, /*writeLinesIfNotIndenting*/ false); - } + const indented = writeLineSeparatorsAndIndentBefore(node.expression, node); emitExpression(node.expression); - const trailingNewlines = preserveSourceNewlines && getClosingLineTerminatorCount(node, [node.expression], ListFormat.None); - if (trailingNewlines) { - writeLine(trailingNewlines); - } - decreaseIndentIf(leadingNewlines); + writeLineSeparatorsAfter(node.expression, node); + decreaseIndentIf(indented); emitTokenWithComment(SyntaxKind.CloseParenToken, node.expression ? node.expression.end : openParenPos, writePunctuation, node); } @@ -3292,12 +3286,15 @@ namespace ts { writePunctuation("<"); if (isJsxOpeningElement(node)) { + const indented = writeLineSeparatorsAndIndentBefore(node.tagName, node); emitJsxTagName(node.tagName); emitTypeArguments(node, node.typeArguments); if (node.attributes.properties && node.attributes.properties.length > 0) { writeSpace(); } emit(node.attributes); + writeLineSeparatorsAfter(node.attributes, node); + decreaseIndentIf(indented); } writePunctuation(">"); @@ -4399,6 +4396,21 @@ namespace ts { return lines; } + function writeLineSeparatorsAndIndentBefore(node: Node, parent: Node): boolean { + const leadingNewlines = preserveSourceNewlines && getLeadingLineTerminatorCount(parent, [node], ListFormat.None); + if (leadingNewlines) { + writeLinesAndIndent(leadingNewlines, /*writeLinesIfNotIndenting*/ false); + } + return !!leadingNewlines; + } + + function writeLineSeparatorsAfter(node: Node, parent: Node) { + const trailingNewlines = preserveSourceNewlines && getClosingLineTerminatorCount(parent, [node], ListFormat.None); + if (trailingNewlines) { + writeLine(trailingNewlines); + } + } + function synthesizedNodeStartsOnNewLine(node: Node, format: ListFormat) { if (nodeIsSynthesized(node)) { const startsOnNewLine = getStartsOnNewLine(node); diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index 9dbfaf19c56f0..493accb8d619c 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -4779,7 +4779,7 @@ namespace ts { export function getLinesBetweenPositionAndPrecedingNonWhitespaceCharacter(pos: number, stopPos: number, sourceFile: SourceFile, includeComments?: boolean) { const startPos = skipTrivia(sourceFile.text, pos, /*stopAfterLineBreak*/ false, includeComments); const prevPos = getPreviousNonWhitespacePosition(startPos, stopPos, sourceFile); - return getLinesBetweenPositions(sourceFile, prevPos || 0, startPos); + return getLinesBetweenPositions(sourceFile, prevPos ?? stopPos, startPos); } export function getLinesBetweenPositionAndNextNonWhitespaceCharacter(pos: number, stopPos: number, sourceFile: SourceFile, includeComments?: boolean) {