From c68961741000f773379f1378bd544987f24c01aa Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Thu, 6 Feb 2020 17:36:48 -0800 Subject: [PATCH] Progress --- src/compiler/emitter.ts | 47 ++++++++++++++++++++++++++++-------- src/compiler/utilities.ts | 16 +++++++++--- src/harness/fourslashImpl.ts | 4 ++- 3 files changed, 52 insertions(+), 15 deletions(-) diff --git a/src/compiler/emitter.ts b/src/compiler/emitter.ts index 91985448b93..c7bb8da3843 100644 --- a/src/compiler/emitter.ts +++ b/src/compiler/emitter.ts @@ -4271,8 +4271,9 @@ namespace ts { if (firstChild === undefined) { return rangeIsOnSingleLine(parentNode, currentSourceFile!) ? 0 : 1; } - else if (!positionIsSynthesized(parentNode.pos) && !nodeIsSynthesized(firstChild)) { - return getLinesBetweenRangeEndAndRangeStart(parentNode, firstChild, currentSourceFile!); + else if (!positionIsSynthesized(parentNode.pos) && !nodeIsSynthesized(firstChild) && firstChild.parent === parentNode) { + const lines = getEffectiveLinesBetweenRanges(parentNode, firstChild, getLinesBetweenRangeStartPositions); + return printerOptions.preserveNewlines ? lines : Math.min(lines, 1); } else if (synthesizedNodeStartsOnNewLine(firstChild, format)) { return 1; @@ -4286,8 +4287,9 @@ namespace ts { if (previousNode === undefined || nextNode === undefined) { return 0; } - else if (!nodeIsSynthesized(previousNode) && !nodeIsSynthesized(nextNode)) { - return getLinesBetweenRangeEndAndRangeStart(previousNode, nextNode, currentSourceFile!); + else if (!nodeIsSynthesized(previousNode) && !nodeIsSynthesized(nextNode) && previousNode.parent === nextNode.parent) { + const lines = getEffectiveLinesBetweenRanges(previousNode, nextNode, getLinesBetweenRangeEndAndRangeStart); + return printerOptions.preserveNewlines ? lines : Math.min(lines, 1); } else if (synthesizedNodeStartsOnNewLine(previousNode, format) || synthesizedNodeStartsOnNewLine(nextNode, format)) { return 1; @@ -4309,19 +4311,44 @@ namespace ts { if (lastChild === undefined) { return rangeIsOnSingleLine(parentNode, currentSourceFile!) ? 0 : 1; } - else if (!positionIsSynthesized(parentNode.pos) && !nodeIsSynthesized(lastChild)) { - return getLinesBetweenRangeEndAndRangeStart(parentNode, lastChild, currentSourceFile!); + else if (!positionIsSynthesized(parentNode.pos) && !nodeIsSynthesized(lastChild) && lastChild.parent === parentNode) { + const lines = getLinesBetweenRangeEndPositions(lastChild, parentNode, currentSourceFile!); + return printerOptions.preserveNewlines ? lines : Math.min(lines, 1); } - else { - return Number(synthesizedNodeStartsOnNewLine(lastChild, format)); + else if (synthesizedNodeStartsOnNewLine(lastChild, format)) { + return 1; } } - if (format & ListFormat.MultiLine) { - return (format & ListFormat.NoTrailingNewLine) === 0 ? 1 : 0; + if (format & ListFormat.MultiLine && !(format & ListFormat.NoTrailingNewLine)) { + return 1; } return 0; } + function getEffectiveLinesBetweenRanges( + node1: TextRange, + node2: TextRange, + getLinesBetweenPositions: (range1: TextRange, range2: TextRange, sourceFile: SourceFile, includeComments: boolean) => number + ) { + // We start by measuring the line difference from parentNode's start to node2's comments start, + // so that this is counted as a one line difference, not two: + // + // function node1() { + // // NODE2 COMMENT + // node2; + const lines = getLinesBetweenPositions(node1, node2, currentSourceFile!, /*includeComments*/ true); + if (lines === 0) { + // However, if the line difference considering node2's comments was 0, we might have this: + // + // function node1() { // NODE2 COMMENT + // node2; + // + // in which case we should be ignoring node2's comment. + return getLinesBetweenPositions(node1, node2, currentSourceFile!, /*includeComments*/ false); + } + return lines; + } + 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 d338b39c829..47f656c21be 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -4676,8 +4676,16 @@ namespace ts { return positionsAreOnSameLine(range1.end, getStartPositionOfRange(range2, sourceFile), sourceFile); } - export function getLinesBetweenRangeEndAndRangeStart(range1: TextRange, range2: TextRange, sourceFile: SourceFile) { - return getLineOfLocalPosition(sourceFile, getStartPositionOfRange(range2, sourceFile)) - getLineOfLocalPosition(sourceFile, range1.end); + export function getLinesBetweenRangeEndAndRangeStart(range1: TextRange, range2: TextRange, sourceFile: SourceFile, includeSecondRangeComments: boolean) { + return getLineOfLocalPosition(sourceFile, getStartPositionOfRange(range2, sourceFile, includeSecondRangeComments)) - getLineOfLocalPosition(sourceFile, range1.end); + } + + export function getLinesBetweenRangeStartPositions(range1: TextRange, range2: TextRange, sourceFile: SourceFile, includeSecondRangeComments: boolean) { + return getLineOfLocalPosition(sourceFile, getStartPositionOfRange(range2, sourceFile, includeSecondRangeComments)) - getLineOfLocalPosition(sourceFile, getStartPositionOfRange(range1, sourceFile)); + } + + export function getLinesBetweenRangeEndPositions(range1: TextRange, range2: TextRange, sourceFile: SourceFile) { + return getLineOfLocalPosition(sourceFile, range2.end) - getLineOfLocalPosition(sourceFile, range1.end); } export function isNodeArrayMultiLine(list: NodeArray, sourceFile: SourceFile): boolean { @@ -4689,8 +4697,8 @@ namespace ts { getLineOfLocalPosition(sourceFile, pos1) === getLineOfLocalPosition(sourceFile, pos2); } - export function getStartPositionOfRange(range: TextRange, sourceFile: SourceFile) { - return positionIsSynthesized(range.pos) ? -1 : skipTrivia(sourceFile.text, range.pos); + export function getStartPositionOfRange(range: TextRange, sourceFile: SourceFile, includeComments?: boolean) { + return positionIsSynthesized(range.pos) ? -1 : skipTrivia(sourceFile.text, range.pos, /*stopAfterLineBreak*/ false, includeComments); } /** diff --git a/src/harness/fourslashImpl.ts b/src/harness/fourslashImpl.ts index 6dc821fad6c..44846da7aa7 100644 --- a/src/harness/fourslashImpl.ts +++ b/src/harness/fourslashImpl.ts @@ -2669,7 +2669,9 @@ namespace FourSlash { const oldText = this.tryGetFileContent(change.fileName); ts.Debug.assert(!!change.isNewFile === (oldText === undefined)); const newContent = change.isNewFile ? ts.first(change.textChanges).newText : ts.textChanges.applyChanges(oldText!, change.textChanges); - assert.equal(newContent, expectedNewContent, `String mis-matched in file ${change.fileName}`); + if (newContent !== expectedNewContent) { + assert.fail(undefined, undefined, `String mis-matched in file ${change.fileName}: ${showTextDiff(expectedNewContent, newContent)}`); + } } for (const newFileName in newFileContent) { ts.Debug.assert(changes.some(c => c.fileName === newFileName), "No change in file", () => newFileName);