From 2b9e4aabf229ec351015489380e33a4f063d4d21 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Thu, 6 Feb 2020 09:06:56 -0800 Subject: [PATCH 01/24] Allow emitter to write multiple newlines in node lists --- src/compiler/emitter.ts | 95 ++++++++++++++++++------------------- src/compiler/types.ts | 3 +- src/compiler/utilities.ts | 8 +++- src/services/textChanges.ts | 6 +-- 4 files changed, 56 insertions(+), 56 deletions(-) diff --git a/src/compiler/emitter.ts b/src/compiler/emitter.ts index a5eb13647e92f..91985448b9351 100644 --- a/src/compiler/emitter.ts +++ b/src/compiler/emitter.ts @@ -2924,14 +2924,14 @@ namespace ts { return false; } - if (shouldWriteLeadingLineTerminator(body, body.statements, ListFormat.PreserveLines) - || shouldWriteClosingLineTerminator(body, body.statements, ListFormat.PreserveLines)) { + if (getLeadingLineTerminatorCount(body, body.statements, ListFormat.PreserveLines) + || getClosingLineTerminatorCount(body, body.statements, ListFormat.PreserveLines)) { return false; } let previousStatement: Statement | undefined; for (const statement of body.statements) { - if (shouldWriteSeparatingLineTerminator(previousStatement, statement, ListFormat.PreserveLines)) { + if (getSeparatingLineTerminatorCount(previousStatement, statement, ListFormat.PreserveLines) > 0) { return false; } @@ -4002,8 +4002,9 @@ namespace ts { // Write the opening line terminator or leading whitespace. const mayEmitInterveningComments = (format & ListFormat.NoInterveningComments) === 0; let shouldEmitInterveningComments = mayEmitInterveningComments; - if (shouldWriteLeadingLineTerminator(parentNode, children!, format)) { // TODO: GH#18217 - writeLine(); + const leadingLineTerminatorCount = getLeadingLineTerminatorCount(parentNode, children!, format); // TODO: GH#18217 + if (leadingLineTerminatorCount) { + writeLine(leadingLineTerminatorCount); shouldEmitInterveningComments = false; } else if (format & ListFormat.SpaceBetweenBraces) { @@ -4042,7 +4043,8 @@ namespace ts { recordBundleFileInternalSectionEnd(previousSourceFileTextKind); // Write either a line terminator or whitespace to separate the elements. - if (shouldWriteSeparatingLineTerminator(previousSibling, child, format)) { + const separatingLineTerminatorCount = getSeparatingLineTerminatorCount(previousSibling, child, format); + if (separatingLineTerminatorCount > 0) { // If a synthesized node in a single-line list starts on a new // line, we should increase the indent. if ((format & (ListFormat.LinesMask | ListFormat.Indented)) === ListFormat.SingleLine) { @@ -4050,7 +4052,7 @@ namespace ts { shouldDecreaseIndentAfterEmit = true; } - writeLine(); + writeLine(separatingLineTerminatorCount); shouldEmitInterveningComments = false; } else if (previousSibling && format & ListFormat.SpaceBetweenSiblings) { @@ -4105,8 +4107,9 @@ namespace ts { recordBundleFileInternalSectionEnd(previousSourceFileTextKind); // Write the closing line terminator or closing whitespace. - if (shouldWriteClosingLineTerminator(parentNode, children!, format)) { - writeLine(); + const closingLineTerminatorCount = getClosingLineTerminatorCount(parentNode, children!, format); + if (closingLineTerminatorCount) { + writeLine(closingLineTerminatorCount); } else if (format & ListFormat.SpaceBetweenBraces) { writeSpace(); @@ -4176,8 +4179,10 @@ namespace ts { writer.writeProperty(s); } - function writeLine() { - writer.writeLine(); + function writeLine(count = 1) { + for (let i = 0; i < count; i++) { + writer.writeLine(i > 0); + } } function increaseIndent() { @@ -4256,75 +4261,65 @@ namespace ts { } } - function shouldWriteLeadingLineTerminator(parentNode: TextRange, children: NodeArray, format: ListFormat) { - if (format & ListFormat.MultiLine) { - return true; - } - - if (format & ListFormat.PreserveLines) { + function getLeadingLineTerminatorCount(parentNode: TextRange, children: NodeArray, format: ListFormat): number { + if (format & ListFormat.PreserveLines || printerOptions.preserveNewlines) { if (format & ListFormat.PreferNewLine) { - return true; + return 1; } const firstChild = children[0]; if (firstChild === undefined) { - return !rangeIsOnSingleLine(parentNode, currentSourceFile!); + return rangeIsOnSingleLine(parentNode, currentSourceFile!) ? 0 : 1; } - else if (positionIsSynthesized(parentNode.pos) || nodeIsSynthesized(firstChild)) { - return synthesizedNodeStartsOnNewLine(firstChild, format); + else if (!positionIsSynthesized(parentNode.pos) && !nodeIsSynthesized(firstChild)) { + return getLinesBetweenRangeEndAndRangeStart(parentNode, firstChild, currentSourceFile!); } - else { - return !rangeStartPositionsAreOnSameLine(parentNode, firstChild, currentSourceFile!); + else if (synthesizedNodeStartsOnNewLine(firstChild, format)) { + return 1; } } - else { - return false; - } + return format & ListFormat.MultiLine ? 1 : 0; } - function shouldWriteSeparatingLineTerminator(previousNode: Node | undefined, nextNode: Node, format: ListFormat) { - if (format & ListFormat.MultiLine) { - return true; - } - else if (format & ListFormat.PreserveLines) { + function getSeparatingLineTerminatorCount(previousNode: Node | undefined, nextNode: Node, format: ListFormat): number { + if (format & ListFormat.PreserveLines || printerOptions.preserveNewlines) { if (previousNode === undefined || nextNode === undefined) { - return false; + return 0; } - else if (nodeIsSynthesized(previousNode) || nodeIsSynthesized(nextNode)) { - return synthesizedNodeStartsOnNewLine(previousNode, format) || synthesizedNodeStartsOnNewLine(nextNode, format); + else if (!nodeIsSynthesized(previousNode) && !nodeIsSynthesized(nextNode)) { + return getLinesBetweenRangeEndAndRangeStart(previousNode, nextNode, currentSourceFile!); } - else { - return !rangeEndIsOnSameLineAsRangeStart(previousNode, nextNode, currentSourceFile!); + else if (synthesizedNodeStartsOnNewLine(previousNode, format) || synthesizedNodeStartsOnNewLine(nextNode, format)) { + return 1; } } - else { - return getStartsOnNewLine(nextNode); + else if (getStartsOnNewLine(nextNode)) { + return 1; } + return format & ListFormat.MultiLine ? 1 : 0; } - function shouldWriteClosingLineTerminator(parentNode: TextRange, children: NodeArray, format: ListFormat) { - if (format & ListFormat.MultiLine) { - return (format & ListFormat.NoTrailingNewLine) === 0; - } - else if (format & ListFormat.PreserveLines) { + function getClosingLineTerminatorCount(parentNode: TextRange, children: NodeArray, format: ListFormat): number { + if (format & ListFormat.PreserveLines || printerOptions.preserveNewlines) { if (format & ListFormat.PreferNewLine) { - return true; + return 1; } const lastChild = lastOrUndefined(children); if (lastChild === undefined) { - return !rangeIsOnSingleLine(parentNode, currentSourceFile!); + return rangeIsOnSingleLine(parentNode, currentSourceFile!) ? 0 : 1; } - else if (positionIsSynthesized(parentNode.pos) || nodeIsSynthesized(lastChild)) { - return synthesizedNodeStartsOnNewLine(lastChild, format); + else if (!positionIsSynthesized(parentNode.pos) && !nodeIsSynthesized(lastChild)) { + return getLinesBetweenRangeEndAndRangeStart(parentNode, lastChild, currentSourceFile!); } else { - return !rangeEndPositionsAreOnSameLine(parentNode, lastChild, currentSourceFile!); + return Number(synthesizedNodeStartsOnNewLine(lastChild, format)); } } - else { - return false; + if (format & ListFormat.MultiLine) { + return (format & ListFormat.NoTrailingNewLine) === 0 ? 1 : 0; } + return 0; } function synthesizedNodeStartsOnNewLine(node: Node, format: ListFormat) { diff --git a/src/compiler/types.ts b/src/compiler/types.ts index 2c67997f2e017..3743b89dc9975 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -3753,7 +3753,7 @@ namespace ts { writeParameter(text: string): void; writeProperty(text: string): void; writeSymbol(text: string, symbol: Symbol): void; - writeLine(): void; + writeLine(force?: boolean): void; increaseIndent(): void; decreaseIndent(): void; clear(): void; @@ -6242,6 +6242,7 @@ namespace ts { /*@internal*/ writeBundleFileInfo?: boolean; /*@internal*/ recordInternalSection?: boolean; /*@internal*/ stripInternal?: boolean; + /*@internal*/ preserveNewlines?: boolean; /*@internal*/ relativeToBuildInfo?: (path: string) => string; } diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index a0a57176b0fb3..d338b39c82950 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -3569,8 +3569,8 @@ namespace ts { } } - function writeLine() { - if (!lineStart) { + function writeLine(force?: boolean) { + if (!lineStart || force) { output += newLine; lineCount++; linePos = output.length; @@ -4676,6 +4676,10 @@ 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 isNodeArrayMultiLine(list: NodeArray, sourceFile: SourceFile): boolean { return !positionsAreOnSameLine(list.pos, list.end, sourceFile); } diff --git a/src/services/textChanges.ts b/src/services/textChanges.ts index be978b5a3342b..a6301a452259c 100644 --- a/src/services/textChanges.ts +++ b/src/services/textChanges.ts @@ -923,7 +923,7 @@ namespace ts.textChanges { export function getNonformattedText(node: Node, sourceFile: SourceFile | undefined, newLineCharacter: string): { text: string, node: Node } { const writer = createWriter(newLineCharacter); const newLine = newLineCharacter === "\n" ? NewLineKind.LineFeed : NewLineKind.CarriageReturnLineFeed; - createPrinter({ newLine, neverAsciiEscape: true }, writer).writeNode(EmitHint.Unspecified, node, sourceFile, writer); + createPrinter({ newLine, neverAsciiEscape: true, preserveNewlines: true }, writer).writeNode(EmitHint.Unspecified, node, sourceFile, writer); return { text: writer.getText(), node: assignPositionsToNode(node) }; } } @@ -1053,8 +1053,8 @@ namespace ts.textChanges { writer.writeSymbol(s, sym); setLastNonTriviaPosition(s, /*force*/ false); } - function writeLine(): void { - writer.writeLine(); + function writeLine(force?: boolean): void { + writer.writeLine(force); } function increaseIndent(): void { writer.increaseIndent(); From c68961741000f773379f1378bd544987f24c01aa Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Thu, 6 Feb 2020 17:36:48 -0800 Subject: [PATCH 02/24] 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 91985448b9351..c7bb8da3843a9 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 d338b39c82950..47f656c21be5d 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 6dc821fad6c08..44846da7aa72d 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); From 6e272f3939fa0eda39a9aef9441b1732d1ff9d26 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Fri, 7 Feb 2020 11:41:49 -0800 Subject: [PATCH 03/24] Progress --- src/compiler/emitter.ts | 14 ++++++++++---- src/compiler/factoryPublic.ts | 5 +++++ src/compiler/types.ts | 1 + src/services/formatting/formatting.ts | 11 ++++++----- src/services/refactors/extractType.ts | 4 ++-- tests/baselines/reference/api/tsserverlibrary.d.ts | 1 + tests/baselines/reference/api/typescript.d.ts | 1 + ...orthandPropertiesErrorFromNotUsingIdentifier.js | 3 ++- ...ectLiteralShorthandPropertiesErrorWithModule.js | 3 ++- .../objectTypesWithOptionalProperties2.js | 3 ++- .../parserErrorRecovery_ObjectLiteral2.js | 3 +-- .../extract-const-callback-function-this3.ts | 4 +--- .../fourslash/moveToNewFile_declarationKinds.ts | 13 ++++--------- .../refactorConvertToEs6Module_export_object.ts | 3 +-- ...orConvertToEs6Module_expressionToDeclaration.ts | 4 +--- 15 files changed, 40 insertions(+), 33 deletions(-) diff --git a/src/compiler/emitter.ts b/src/compiler/emitter.ts index c7bb8da3843a9..dd5fa8574419d 100644 --- a/src/compiler/emitter.ts +++ b/src/compiler/emitter.ts @@ -842,6 +842,7 @@ namespace ts { let tempFlags: TempFlags; // TempFlags for the current name generation scope. let reservedNamesStack: Map[]; // Stack of TempFlags reserved in enclosing name generation scopes. let reservedNames: Map; // TempFlags to reserve in nested name generation scopes. + let preserveNewlines = printerOptions.preserveNewlines; // Can be overridden inside nodes with the `IgnoreSourceNewlines` emit flag. let writer: EmitTextWriter; let ownWriter: EmitTextWriter; // Reusable `EmitTextWriter` for basic printing. @@ -1164,8 +1165,12 @@ namespace ts { function pipelineEmit(emitHint: EmitHint, node: Node) { const savedLastNode = lastNode; const savedLastSubstitution = lastSubstitution; + const savedPreserveNewlines = preserveNewlines; lastNode = node; lastSubstitution = undefined; + if (preserveNewlines && !!(getEmitFlags(node) & EmitFlags.IgnoreSourceNewlines)) { + preserveNewlines = false; + } const pipelinePhase = getPipelinePhase(PipelinePhase.Notification, emitHint, node); pipelinePhase(emitHint, node); @@ -1175,6 +1180,7 @@ namespace ts { const substitute = lastSubstitution; lastNode = savedLastNode; lastSubstitution = savedLastSubstitution; + preserveNewlines = savedPreserveNewlines; return substitute || node; } @@ -3991,7 +3997,7 @@ namespace ts { if (isEmpty) { // Write a line terminator if the parent node was multi-line - if (format & ListFormat.MultiLine) { + if (format & ListFormat.MultiLine && !(preserveNewlines && rangeIsOnSingleLine(parentNode, currentSourceFile!))) { writeLine(); } else if (format & ListFormat.SpaceBetweenBraces && !(format & ListFormat.NoSpaceIfEmpty)) { @@ -4262,7 +4268,7 @@ namespace ts { } function getLeadingLineTerminatorCount(parentNode: TextRange, children: NodeArray, format: ListFormat): number { - if (format & ListFormat.PreserveLines || printerOptions.preserveNewlines) { + if (format & ListFormat.PreserveLines || preserveNewlines) { if (format & ListFormat.PreferNewLine) { return 1; } @@ -4283,7 +4289,7 @@ namespace ts { } function getSeparatingLineTerminatorCount(previousNode: Node | undefined, nextNode: Node, format: ListFormat): number { - if (format & ListFormat.PreserveLines || printerOptions.preserveNewlines) { + if (format & ListFormat.PreserveLines || preserveNewlines) { if (previousNode === undefined || nextNode === undefined) { return 0; } @@ -4302,7 +4308,7 @@ namespace ts { } function getClosingLineTerminatorCount(parentNode: TextRange, children: NodeArray, format: ListFormat): number { - if (format & ListFormat.PreserveLines || printerOptions.preserveNewlines) { + if (format & ListFormat.PreserveLines || preserveNewlines) { if (format & ListFormat.PreferNewLine) { return 1; } diff --git a/src/compiler/factoryPublic.ts b/src/compiler/factoryPublic.ts index 4479e6c2e711e..90ff58e0a1a05 100644 --- a/src/compiler/factoryPublic.ts +++ b/src/compiler/factoryPublic.ts @@ -3559,6 +3559,11 @@ namespace ts { return node; } + export function ignoreSourceNewlines(node: T): T { + getOrCreateEmitNode(node).flags |= EmitFlags.IgnoreSourceNewlines; + return node; + } + /** * Gets the constant value to emit for an expression. */ diff --git a/src/compiler/types.ts b/src/compiler/types.ts index 3743b89dc9975..a6a16eebeacb1 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -5780,6 +5780,7 @@ namespace ts { NoAsciiEscaping = 1 << 24, // When synthesizing nodes that lack an original node or textSourceNode, we want to write the text on the node with ASCII escaping substitutions. /*@internal*/ TypeScriptClassWrapper = 1 << 25, // The node is an IIFE class wrapper created by the ts transform. /*@internal*/ NeverApplyImportHelper = 1 << 26, // Indicates the node should never be wrapped with an import star helper (because, for example, it imports tslib itself) + /*@internal*/ IgnoreSourceNewlines = 1 << 27, // Overrides `printerOptions.preserveNewlines` to print this node (and all descendants) with default whitespace. } export interface EmitHelper { diff --git a/src/services/formatting/formatting.ts b/src/services/formatting/formatting.ts index d7d8d1bcb7757..572596cde15b6 100644 --- a/src/services/formatting/formatting.ts +++ b/src/services/formatting/formatting.ts @@ -70,7 +70,7 @@ namespace ts.formatting { * Formatter calls this function when rule adds or deletes new lines from the text * so indentation scope can adjust values of indentation and delta. */ - recomputeIndentation(lineAddedByFormatting: boolean): void; + recomputeIndentation(lineAddedByFormatting: boolean, parent: Node): void; } export function formatOnEnter(position: number, sourceFile: SourceFile, formatContext: FormatContext): TextChange[] { @@ -565,8 +565,9 @@ namespace ts.formatting { !suppressDelta && shouldAddDelta(line, kind, container) ? indentation + getDelta(container) : indentation, getIndentation: () => indentation, getDelta, - recomputeIndentation: lineAdded => { - if (node.parent && SmartIndenter.shouldIndentChildNode(options, node.parent, node, sourceFile)) { + recomputeIndentation: (lineAdded, parentIn) => { + const parent = node.parent || parentIn; + if (node !== parent && SmartIndenter.shouldIndentChildNode(options, parent, node, sourceFile)) { indentation += lineAdded ? options.indentSize! : -options.indentSize!; delta = SmartIndenter.shouldIndentChildNode(options, node) ? options.indentSize! : 0; } @@ -991,7 +992,7 @@ namespace ts.formatting { // Handle the case where the next line is moved to be the end of this line. // In this case we don't indent the next line in the next pass. if (currentParent.getStart(sourceFile) === currentItem.pos) { - dynamicIndentation.recomputeIndentation(/*lineAddedByFormatting*/ false); + dynamicIndentation.recomputeIndentation(/*lineAddedByFormatting*/ false, currentParent); } break; case LineAction.LineAdded: @@ -999,7 +1000,7 @@ namespace ts.formatting { // In this case we indent token2 in the next pass but we set // sameLineIndent flag to notify the indenter that the indentation is within the line. if (currentParent.getStart(sourceFile) === currentItem.pos) { - dynamicIndentation.recomputeIndentation(/*lineAddedByFormatting*/ true); + dynamicIndentation.recomputeIndentation(/*lineAddedByFormatting*/ true, currentParent); } break; default: diff --git a/src/services/refactors/extractType.ts b/src/services/refactors/extractType.ts index 70934ef7a7ce1..c6886dba28d5f 100644 --- a/src/services/refactors/extractType.ts +++ b/src/services/refactors/extractType.ts @@ -159,7 +159,7 @@ namespace ts.refactor { typeParameters.map(id => updateTypeParameterDeclaration(id, id.name, id.constraint, /* defaultType */ undefined)), selection ); - changes.insertNodeBefore(file, firstStatement, newTypeNode, /* blankLineBetween */ true); + changes.insertNodeBefore(file, firstStatement, ignoreSourceNewlines(newTypeNode), /* blankLineBetween */ true); changes.replaceNode(file, selection, createTypeReferenceNode(name, typeParameters.map(id => createTypeReferenceNode(id.name, /* typeArguments */ undefined)))); } @@ -174,7 +174,7 @@ namespace ts.refactor { /* heritageClauses */ undefined, typeElements ); - changes.insertNodeBefore(file, firstStatement, newTypeNode, /* blankLineBetween */ true); + changes.insertNodeBefore(file, firstStatement, ignoreSourceNewlines(newTypeNode), /* blankLineBetween */ true); changes.replaceNode(file, selection, createTypeReferenceNode(name, typeParameters.map(id => createTypeReferenceNode(id.name, /* typeArguments */ undefined)))); } diff --git a/tests/baselines/reference/api/tsserverlibrary.d.ts b/tests/baselines/reference/api/tsserverlibrary.d.ts index dbc336fccce23..d3ea5796bfae7 100644 --- a/tests/baselines/reference/api/tsserverlibrary.d.ts +++ b/tests/baselines/reference/api/tsserverlibrary.d.ts @@ -4359,6 +4359,7 @@ declare namespace ts { function setSyntheticTrailingComments(node: T, comments: SynthesizedComment[] | undefined): T; function addSyntheticTrailingComment(node: T, kind: SyntaxKind.SingleLineCommentTrivia | SyntaxKind.MultiLineCommentTrivia, text: string, hasTrailingNewLine?: boolean): T; function moveSyntheticComments(node: T, original: Node): T; + function ignoreSourceNewlines(node: T): T; /** * Gets the constant value to emit for an expression. */ diff --git a/tests/baselines/reference/api/typescript.d.ts b/tests/baselines/reference/api/typescript.d.ts index 8b600f90fdd41..44e11abdb59a5 100644 --- a/tests/baselines/reference/api/typescript.d.ts +++ b/tests/baselines/reference/api/typescript.d.ts @@ -4359,6 +4359,7 @@ declare namespace ts { function setSyntheticTrailingComments(node: T, comments: SynthesizedComment[] | undefined): T; function addSyntheticTrailingComment(node: T, kind: SyntaxKind.SingleLineCommentTrivia | SyntaxKind.MultiLineCommentTrivia, text: string, hasTrailingNewLine?: boolean): T; function moveSyntheticComments(node: T, original: Node): T; + function ignoreSourceNewlines(node: T): T; /** * Gets the constant value to emit for an expression. */ diff --git a/tests/baselines/reference/objectLiteralShorthandPropertiesErrorFromNotUsingIdentifier.js b/tests/baselines/reference/objectLiteralShorthandPropertiesErrorFromNotUsingIdentifier.js index e0aecb5f18814..33ae1f6c8f0ee 100644 --- a/tests/baselines/reference/objectLiteralShorthandPropertiesErrorFromNotUsingIdentifier.js +++ b/tests/baselines/reference/objectLiteralShorthandPropertiesErrorFromNotUsingIdentifier.js @@ -35,7 +35,8 @@ var y = { "typeof": }; var x = (_a = { - a: a, : .b, + a: a, + : .b, a: a }, _a["ss"] = , diff --git a/tests/baselines/reference/objectLiteralShorthandPropertiesErrorWithModule.js b/tests/baselines/reference/objectLiteralShorthandPropertiesErrorWithModule.js index ee46d4741dd15..f48a6c3a4e5d6 100644 --- a/tests/baselines/reference/objectLiteralShorthandPropertiesErrorWithModule.js +++ b/tests/baselines/reference/objectLiteralShorthandPropertiesErrorWithModule.js @@ -25,7 +25,8 @@ var n; (function (n) { var z = 10000; n.y = { - m: m, : .x // error + m: m, + : .x // error }; })(n || (n = {})); m.y.x; diff --git a/tests/baselines/reference/objectTypesWithOptionalProperties2.js b/tests/baselines/reference/objectTypesWithOptionalProperties2.js index 03d2b162847ea..284ecdea92ca7 100644 --- a/tests/baselines/reference/objectTypesWithOptionalProperties2.js +++ b/tests/baselines/reference/objectTypesWithOptionalProperties2.js @@ -42,5 +42,6 @@ var C2 = /** @class */ (function () { return C2; }()); var b = { - x: function () { }, 1: // error + x: function () { }, + 1: // error }; diff --git a/tests/baselines/reference/parserErrorRecovery_ObjectLiteral2.js b/tests/baselines/reference/parserErrorRecovery_ObjectLiteral2.js index 1cdf73ddadcb3..6d723a00d4a9d 100644 --- a/tests/baselines/reference/parserErrorRecovery_ObjectLiteral2.js +++ b/tests/baselines/reference/parserErrorRecovery_ObjectLiteral2.js @@ -3,5 +3,4 @@ var v = { a return; //// [parserErrorRecovery_ObjectLiteral2.js] -var v = { a: a, - "return": }; +var v = { a: a, "return": }; diff --git a/tests/cases/fourslash/extract-const-callback-function-this3.ts b/tests/cases/fourslash/extract-const-callback-function-this3.ts index 1d9d8ba59e767..099229a11ba60 100644 --- a/tests/cases/fourslash/extract-const-callback-function-this3.ts +++ b/tests/cases/fourslash/extract-const-callback-function-this3.ts @@ -10,8 +10,6 @@ edit.applyRefactor({ actionDescription: "Extract to constant in enclosing scope", newContent: `declare function fWithThis(fn: (this: { a: string }, a: string) => string): void; -const newLocal = function(this: { - a: string; -}, a: string): string { return this.a; }; +const newLocal = function(this: { a: string; }, a: string): string { return this.a; }; fWithThis(/*RENAME*/newLocal);` }); diff --git a/tests/cases/fourslash/moveToNewFile_declarationKinds.ts b/tests/cases/fourslash/moveToNewFile_declarationKinds.ts index 90c5eb6973a6a..4e9a051108665 100644 --- a/tests/cases/fourslash/moveToNewFile_declarationKinds.ts +++ b/tests/cases/fourslash/moveToNewFile_declarationKinds.ts @@ -24,16 +24,11 @@ type U = T; type V = I;`, "/x.ts": `export const x = 0; export function f() { } -export class C { -} -export enum E { -} -export namespace N { - export const x = 0; -} +export class C { } +export enum E { } +export namespace N { export const x = 0; } export type T = number; -export interface I { -} +export interface I { } `, }, }); diff --git a/tests/cases/fourslash/refactorConvertToEs6Module_export_object.ts b/tests/cases/fourslash/refactorConvertToEs6Module_export_object.ts index 97056ceefdad4..664796b117275 100644 --- a/tests/cases/fourslash/refactorConvertToEs6Module_export_object.ts +++ b/tests/cases/fourslash/refactorConvertToEs6Module_export_object.ts @@ -22,6 +22,5 @@ verify.codeFix({ export function f() { } export function g() { } export function h() { } -export class C { -}`, +export class C { }`, }); diff --git a/tests/cases/fourslash/refactorConvertToEs6Module_expressionToDeclaration.ts b/tests/cases/fourslash/refactorConvertToEs6Module_expressionToDeclaration.ts index c91c726dbfd76..73dcff282854a 100644 --- a/tests/cases/fourslash/refactorConvertToEs6Module_expressionToDeclaration.ts +++ b/tests/cases/fourslash/refactorConvertToEs6Module_expressionToDeclaration.ts @@ -15,8 +15,6 @@ verify.codeFix({ `var C = {}; console.log(C); export async function* f(p) { p; } -const _C = class C extends D { - m() { } -}; +const _C = class C extends D { m() { } }; export { _C as C };`, }); From 839fb8e8c6657b9a22067d38ead2f16d2d320bff Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Fri, 7 Feb 2020 12:50:45 -0800 Subject: [PATCH 04/24] Fix recomputeIndentation --- src/services/formatting/formatting.ts | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/services/formatting/formatting.ts b/src/services/formatting/formatting.ts index 572596cde15b6..8942c27ee529d 100644 --- a/src/services/formatting/formatting.ts +++ b/src/services/formatting/formatting.ts @@ -565,9 +565,8 @@ namespace ts.formatting { !suppressDelta && shouldAddDelta(line, kind, container) ? indentation + getDelta(container) : indentation, getIndentation: () => indentation, getDelta, - recomputeIndentation: (lineAdded, parentIn) => { - const parent = node.parent || parentIn; - if (node !== parent && SmartIndenter.shouldIndentChildNode(options, parent, node, sourceFile)) { + recomputeIndentation: (lineAdded, parent) => { + if (SmartIndenter.shouldIndentChildNode(options, parent, node, sourceFile)) { indentation += lineAdded ? options.indentSize! : -options.indentSize!; delta = SmartIndenter.shouldIndentChildNode(options, node) ? options.indentSize! : 0; } @@ -992,7 +991,7 @@ namespace ts.formatting { // Handle the case where the next line is moved to be the end of this line. // In this case we don't indent the next line in the next pass. if (currentParent.getStart(sourceFile) === currentItem.pos) { - dynamicIndentation.recomputeIndentation(/*lineAddedByFormatting*/ false, currentParent); + dynamicIndentation.recomputeIndentation(/*lineAddedByFormatting*/ false, contextNode); } break; case LineAction.LineAdded: @@ -1000,7 +999,7 @@ namespace ts.formatting { // In this case we indent token2 in the next pass but we set // sameLineIndent flag to notify the indenter that the indentation is within the line. if (currentParent.getStart(sourceFile) === currentItem.pos) { - dynamicIndentation.recomputeIndentation(/*lineAddedByFormatting*/ true, currentParent); + dynamicIndentation.recomputeIndentation(/*lineAddedByFormatting*/ true, contextNode); } break; default: From 0ea8d79fbf8b1769c156eb74cb9a4b0fd2ae2922 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Fri, 7 Feb 2020 14:43:54 -0800 Subject: [PATCH 05/24] Add tests, fix leading line terminator count --- src/compiler/emitter.ts | 91 ++++++++++--------- src/compiler/utilities.ts | 14 +++ .../fourslash/textChangesPreserveNewlines1.ts | 25 +++++ .../fourslash/textChangesPreserveNewlines2.ts | 29 ++++++ .../fourslash/textChangesPreserveNewlines3.ts | 35 +++++++ .../fourslash/textChangesPreserveNewlines4.ts | 24 +++++ .../fourslash/textChangesPreserveNewlines5.ts | 31 +++++++ 7 files changed, 206 insertions(+), 43 deletions(-) create mode 100644 tests/cases/fourslash/textChangesPreserveNewlines1.ts create mode 100644 tests/cases/fourslash/textChangesPreserveNewlines2.ts create mode 100644 tests/cases/fourslash/textChangesPreserveNewlines3.ts create mode 100644 tests/cases/fourslash/textChangesPreserveNewlines4.ts create mode 100644 tests/cases/fourslash/textChangesPreserveNewlines5.ts diff --git a/src/compiler/emitter.ts b/src/compiler/emitter.ts index dd5fa8574419d..7256d179505e6 100644 --- a/src/compiler/emitter.ts +++ b/src/compiler/emitter.ts @@ -2279,10 +2279,10 @@ namespace ts { function emitPropertyAccessExpression(node: PropertyAccessExpression) { const expression = cast(emitExpression(node.expression), isExpression); const token = getDotOrQuestionDotToken(node); - const indentBeforeDot = needsIndentation(node, node.expression, token); - const indentAfterDot = needsIndentation(node, token, node.name); + const linesBeforeDot = getLinesBetweenNodes(node, node.expression, token); + const linesAfterDot = getLinesBetweenNodes(node, token, node.name); - increaseIndentIf(indentBeforeDot, /*writeSpaceIfNotIndenting*/ false); + writeLinesAndIndent(linesBeforeDot, /*writeSpaceIfNotIndenting*/ false); const shouldEmitDotDot = token.kind !== SyntaxKind.QuestionDotToken && @@ -2295,9 +2295,9 @@ namespace ts { } emitTokenWithComment(token.kind, node.expression.end, writePunctuation, node); - increaseIndentIf(indentAfterDot, /*writeSpaceIfNotIndenting*/ false); + writeLinesAndIndent(linesAfterDot, /*writeSpaceIfNotIndenting*/ false); emit(node.name); - decreaseIndentIf(indentBeforeDot, indentAfterDot); + decreaseIndentIf(linesBeforeDot, linesAfterDot); } // 1..toString is a valid property access, emit a dot after the literal @@ -2462,20 +2462,20 @@ namespace ts { } case EmitBinaryExpressionState.EmitRight: { const isCommaOperator = node.operatorToken.kind !== SyntaxKind.CommaToken; - const indentBeforeOperator = needsIndentation(node, node.left, node.operatorToken); - const indentAfterOperator = needsIndentation(node, node.operatorToken, node.right); - increaseIndentIf(indentBeforeOperator, isCommaOperator); + const linesBeforeOperator = getLinesBetweenNodes(node, node.left, node.operatorToken); + const linesAfterOperator = getLinesBetweenNodes(node, node.operatorToken, node.right); + writeLinesAndIndent(linesBeforeOperator, isCommaOperator); emitLeadingCommentsOfPosition(node.operatorToken.pos); writeTokenNode(node.operatorToken, node.operatorToken.kind === SyntaxKind.InKeyword ? writeKeyword : writeOperator); emitTrailingCommentsOfPosition(node.operatorToken.end, /*prefixSpace*/ true); // Binary operators should have a space before the comment starts - increaseIndentIf(indentAfterOperator, /*writeSpaceIfNotIndenting*/ true); + writeLinesAndIndent(linesAfterOperator, /*writeSpaceIfNotIndenting*/ true); maybePipelineEmitExpression(node.right); break; } case EmitBinaryExpressionState.FinishEmit: { - const indentBeforeOperator = needsIndentation(node, node.left, node.operatorToken); - const indentAfterOperator = needsIndentation(node, node.operatorToken, node.right); - decreaseIndentIf(indentBeforeOperator, indentAfterOperator); + const linesBeforeOperator = getLinesBetweenNodes(node, node.left, node.operatorToken); + const linesAfterOperator = getLinesBetweenNodes(node, node.operatorToken, node.right); + decreaseIndentIf(linesBeforeOperator, linesAfterOperator); stackIndex--; break; } @@ -2519,23 +2519,23 @@ namespace ts { } function emitConditionalExpression(node: ConditionalExpression) { - const indentBeforeQuestion = needsIndentation(node, node.condition, node.questionToken); - const indentAfterQuestion = needsIndentation(node, node.questionToken, node.whenTrue); - const indentBeforeColon = needsIndentation(node, node.whenTrue, node.colonToken); - const indentAfterColon = needsIndentation(node, node.colonToken, node.whenFalse); + const linesBeforeQuestion = getLinesBetweenNodes(node, node.condition, node.questionToken); + const linesAfterQuestion = getLinesBetweenNodes(node, node.questionToken, node.whenTrue); + const linesBeforeColon = getLinesBetweenNodes(node, node.whenTrue, node.colonToken); + const linesAfterColon = getLinesBetweenNodes(node, node.colonToken, node.whenFalse); emitExpression(node.condition); - increaseIndentIf(indentBeforeQuestion, /*writeSpaceIfNotIndenting*/ true); + writeLinesAndIndent(linesBeforeQuestion, /*writeSpaceIfNotIndenting*/ true); emit(node.questionToken); - increaseIndentIf(indentAfterQuestion, /*writeSpaceIfNotIndenting*/ true); + writeLinesAndIndent(linesAfterQuestion, /*writeSpaceIfNotIndenting*/ true); emitExpression(node.whenTrue); - decreaseIndentIf(indentBeforeQuestion, indentAfterQuestion); + decreaseIndentIf(linesBeforeQuestion, linesAfterQuestion); - increaseIndentIf(indentBeforeColon, /*writeSpaceIfNotIndenting*/ true); + writeLinesAndIndent(linesBeforeColon, /*writeSpaceIfNotIndenting*/ true); emit(node.colonToken); - increaseIndentIf(indentAfterColon, /*writeSpaceIfNotIndenting*/ true); + writeLinesAndIndent(linesAfterColon, /*writeSpaceIfNotIndenting*/ true); emitExpression(node.whenFalse); - decreaseIndentIf(indentBeforeColon, indentAfterColon); + decreaseIndentIf(linesBeforeColon, linesAfterColon); } function emitTemplateExpression(node: TemplateExpression) { @@ -4025,7 +4025,7 @@ namespace ts { // Emit each child. let previousSibling: Node | undefined; let previousSourceFileTextKind: ReturnType; - let shouldDecreaseIndentAfterEmit = false; + let shouldDecreaselinesAfterEmit = false; for (let i = 0; i < count; i++) { const child = children![start + i]; @@ -4055,7 +4055,7 @@ namespace ts { // line, we should increase the indent. if ((format & (ListFormat.LinesMask | ListFormat.Indented)) === ListFormat.SingleLine) { increaseIndent(); - shouldDecreaseIndentAfterEmit = true; + shouldDecreaselinesAfterEmit = true; } writeLine(separatingLineTerminatorCount); @@ -4080,9 +4080,9 @@ namespace ts { emit(child); - if (shouldDecreaseIndentAfterEmit) { + if (shouldDecreaselinesAfterEmit) { decreaseIndent(); - shouldDecreaseIndentAfterEmit = false; + shouldDecreaselinesAfterEmit = false; } previousSibling = child; @@ -4244,10 +4244,10 @@ namespace ts { } } - function increaseIndentIf(value: boolean, writeSpaceIfNotIndenting: boolean) { - if (value) { + function writeLinesAndIndent(lineCount: number, writeSpaceIfNotIndenting: boolean) { + if (lineCount) { increaseIndent(); - writeLine(); + writeLine(lineCount); } else if (writeSpaceIfNotIndenting) { writeSpace(); @@ -4258,7 +4258,7 @@ namespace ts { // previous indent values to be considered at a time. This also allows caller to just // call this once, passing in all their appropriate indent values, instead of needing // to call this helper function multiple times. - function decreaseIndentIf(value1: boolean, value2: boolean) { + function decreaseIndentIf(value1: boolean | number, value2: boolean | number) { if (value1) { decreaseIndent(); } @@ -4278,7 +4278,10 @@ namespace ts { return rangeIsOnSingleLine(parentNode, currentSourceFile!) ? 0 : 1; } else if (!positionIsSynthesized(parentNode.pos) && !nodeIsSynthesized(firstChild) && firstChild.parent === parentNode) { - const lines = getEffectiveLinesBetweenRanges(parentNode, firstChild, getLinesBetweenRangeStartPositions); + let lines = getLinesBetweenPositionAndPrecedingNonWhitespaceCharacter(firstChild.pos, currentSourceFile!, /*includeComments*/ true); + if (lines === 0) { + lines = getLinesBetweenPositionAndPrecedingNonWhitespaceCharacter(firstChild.pos, currentSourceFile!, /*includeComments*/ false); + } return printerOptions.preserveNewlines ? lines : Math.min(lines, 1); } else if (synthesizedNodeStartsOnNewLine(firstChild, format)) { @@ -4339,15 +4342,15 @@ namespace ts { // 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; + // 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; + // node1; // NODE2 COMMENT + // node2; // // in which case we should be ignoring node2's comment. return getLinesBetweenPositions(node1, node2, currentSourceFile!, /*includeComments*/ false); @@ -4368,9 +4371,9 @@ namespace ts { return (format & ListFormat.PreferNewLine) !== 0; } - function needsIndentation(parent: Node, node1: Node, node2: Node): boolean { + function getLinesBetweenNodes(parent: Node, node1: Node, node2: Node): number { if (getEmitFlags(parent) & EmitFlags.NoIndentation) { - return false; + return 0; } parent = skipSynthesizedParentheses(parent); @@ -4379,13 +4382,15 @@ namespace ts { // Always use a newline for synthesized code if the synthesizer desires it. if (getStartsOnNewLine(node2)) { - return true; + return 1; + } + + if (!nodeIsSynthesized(parent) && !nodeIsSynthesized(node1) && !nodeIsSynthesized(node2)) { + const lines = getEffectiveLinesBetweenRanges(node1, node2, getLinesBetweenRangeEndAndRangeStart); + return preserveNewlines ? lines : Math.min(lines, 1); } - return !nodeIsSynthesized(parent) - && !nodeIsSynthesized(node1) - && !nodeIsSynthesized(node2) - && !rangeEndIsOnSameLineAsRangeStart(node1, node2, currentSourceFile!); + return 0; } function isEmptyBlock(block: BlockLike) { diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index 47f656c21be5d..085670d31b84c 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -4701,6 +4701,20 @@ namespace ts { return positionIsSynthesized(range.pos) ? -1 : skipTrivia(sourceFile.text, range.pos, /*stopAfterLineBreak*/ false, includeComments); } + export function getLinesBetweenPositionAndPrecedingNonWhitespaceCharacter(pos: number, sourceFile: SourceFile, includeComments?: boolean) { + const startPos = skipTrivia(sourceFile.text, pos, /*stopAfterLineBreak*/ false, includeComments); + const prevPos = getPreviousNonWhitespacePosition(pos, sourceFile); + return getLineOfLocalPosition(sourceFile, startPos) - getLineOfLocalPosition(sourceFile, prevPos || 0); + } + + function getPreviousNonWhitespacePosition(pos: number, sourceFile: SourceFile) { + while (pos-- > 0) { + if (!isWhiteSpaceLike(sourceFile.text.charCodeAt(pos))) { + return pos; + } + } + } + /** * Determines whether a name was originally the declaration name of an enum or namespace * declaration. diff --git a/tests/cases/fourslash/textChangesPreserveNewlines1.ts b/tests/cases/fourslash/textChangesPreserveNewlines1.ts new file mode 100644 index 0000000000000..7e9a11a738d23 --- /dev/null +++ b/tests/cases/fourslash/textChangesPreserveNewlines1.ts @@ -0,0 +1,25 @@ +/// + +//// /*1*/console.log(1); +//// +//// console.log(2); +//// +//// console.log(3);/*2*/ + +goTo.select("1", "2"); +edit.applyRefactor({ + refactorName: "Extract Symbol", + actionName: "function_scope_0", + actionDescription: "Extract to function in global scope", + newContent: +`/*RENAME*/newFunction(); + +function newFunction() { + console.log(1); + + console.log(2); + + console.log(3); +} +` +}); diff --git a/tests/cases/fourslash/textChangesPreserveNewlines2.ts b/tests/cases/fourslash/textChangesPreserveNewlines2.ts new file mode 100644 index 0000000000000..b5d343b037689 --- /dev/null +++ b/tests/cases/fourslash/textChangesPreserveNewlines2.ts @@ -0,0 +1,29 @@ +/// + +//// /*1*/1 + +//// 2 + +//// +//// 3 + +//// +//// +//// 4;/*2*/ + +goTo.select("1", "2"); +edit.applyRefactor({ + refactorName: "Extract Symbol", + actionName: "function_scope_0", + actionDescription: "Extract to function in global scope", + newContent: +`/*RENAME*/newFunction(); + +function newFunction() { + 1 + + 2 + + + 3 + + + + 4; +} +` +}); diff --git a/tests/cases/fourslash/textChangesPreserveNewlines3.ts b/tests/cases/fourslash/textChangesPreserveNewlines3.ts new file mode 100644 index 0000000000000..d01a8c083b985 --- /dev/null +++ b/tests/cases/fourslash/textChangesPreserveNewlines3.ts @@ -0,0 +1,35 @@ +/// + +//// /*1*/app +//// .use(foo) +//// +//// .use(bar) +//// +//// +//// .use( +//// baz, +//// +//// blob);/*2*/ + +goTo.select("1", "2"); +edit.applyRefactor({ + refactorName: "Extract Symbol", + actionName: "function_scope_0", + actionDescription: "Extract to function in global scope", + newContent: +`/*RENAME*/newFunction(); + +function newFunction() { + app + .use(foo) + + .use(bar) + + + .use( + baz, + + blob); +} +` +}); diff --git a/tests/cases/fourslash/textChangesPreserveNewlines4.ts b/tests/cases/fourslash/textChangesPreserveNewlines4.ts new file mode 100644 index 0000000000000..77892b893c7a0 --- /dev/null +++ b/tests/cases/fourslash/textChangesPreserveNewlines4.ts @@ -0,0 +1,24 @@ +/// + +//// const x = /*1*/function f() +//// { +//// +//// console.log(); +//// }/*2*/; + +goTo.select("1", "2"); +edit.applyRefactor({ + refactorName: "Extract Symbol", + actionName: "function_scope_0", + actionDescription: "Extract to function in global scope", + newContent: +`const x = /*RENAME*/newFunction(); + +function newFunction() { + return function f() { + + console.log(); + }; +} +` +}); diff --git a/tests/cases/fourslash/textChangesPreserveNewlines5.ts b/tests/cases/fourslash/textChangesPreserveNewlines5.ts new file mode 100644 index 0000000000000..d630fb47e0f48 --- /dev/null +++ b/tests/cases/fourslash/textChangesPreserveNewlines5.ts @@ -0,0 +1,31 @@ +/// + +//// /*1*/f // call expression +//// (arg)( +//// /** @type {number} */ +//// blah, +//// +//// blah +//// +//// );/*2*/ + +goTo.select("1", "2"); +edit.applyRefactor({ + refactorName: "Extract Symbol", + actionName: "function_scope_0", + actionDescription: "Extract to function in global scope", + newContent: +`/*RENAME*/newFunction(); + +function newFunction() { + f // call expression + (arg)( + /** @type {number} */ + blah, + + blah + + ); +} +` +}); From 6ff9c2bbbb7f5b4ae64534e851b3c7351e5d6bb6 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Mon, 10 Feb 2020 09:57:38 -0800 Subject: [PATCH 06/24] Do a bit less work when `preserveNewlines` is off --- src/compiler/emitter.ts | 40 +++++++++++++++++++++------------------ src/compiler/utilities.ts | 2 +- 2 files changed, 23 insertions(+), 19 deletions(-) diff --git a/src/compiler/emitter.ts b/src/compiler/emitter.ts index 7256d179505e6..82a8ab1f1283e 100644 --- a/src/compiler/emitter.ts +++ b/src/compiler/emitter.ts @@ -4278,11 +4278,13 @@ namespace ts { return rangeIsOnSingleLine(parentNode, currentSourceFile!) ? 0 : 1; } else if (!positionIsSynthesized(parentNode.pos) && !nodeIsSynthesized(firstChild) && firstChild.parent === parentNode) { - let lines = getLinesBetweenPositionAndPrecedingNonWhitespaceCharacter(firstChild.pos, currentSourceFile!, /*includeComments*/ true); - if (lines === 0) { - lines = getLinesBetweenPositionAndPrecedingNonWhitespaceCharacter(firstChild.pos, currentSourceFile!, /*includeComments*/ false); + if (preserveNewlines) { + const lines = getLinesBetweenPositionAndPrecedingNonWhitespaceCharacter(firstChild.pos, currentSourceFile!, /*includeComments*/ true); + return lines === 0 + ? getLinesBetweenPositionAndPrecedingNonWhitespaceCharacter(firstChild.pos, currentSourceFile!, /*includeComments*/ false) + : lines; } - return printerOptions.preserveNewlines ? lines : Math.min(lines, 1); + return rangeStartPositionsAreOnSameLine(parentNode, firstChild, currentSourceFile!) ? 0 : 1; } else if (synthesizedNodeStartsOnNewLine(firstChild, format)) { return 1; @@ -4297,8 +4299,8 @@ namespace ts { return 0; } else if (!nodeIsSynthesized(previousNode) && !nodeIsSynthesized(nextNode) && previousNode.parent === nextNode.parent) { - const lines = getEffectiveLinesBetweenRanges(previousNode, nextNode, getLinesBetweenRangeEndAndRangeStart); - return printerOptions.preserveNewlines ? lines : Math.min(lines, 1); + const lines = getEffectiveLinesBetweenRanges(previousNode, nextNode); + return preserveNewlines ? lines : Math.min(lines, 1); } else if (synthesizedNodeStartsOnNewLine(previousNode, format) || synthesizedNodeStartsOnNewLine(nextNode, format)) { return 1; @@ -4322,7 +4324,7 @@ namespace ts { } else if (!positionIsSynthesized(parentNode.pos) && !nodeIsSynthesized(lastChild) && lastChild.parent === parentNode) { const lines = getLinesBetweenRangeEndPositions(lastChild, parentNode, currentSourceFile!); - return printerOptions.preserveNewlines ? lines : Math.min(lines, 1); + return preserveNewlines ? lines : Math.min(lines, 1); } else if (synthesizedNodeStartsOnNewLine(lastChild, format)) { return 1; @@ -4334,26 +4336,28 @@ namespace ts { 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 getEffectiveLinesBetweenRanges(node1: TextRange, node2: TextRange) { + // If 'preserveNewlines' is disabled, skip the more accurate check that might require + // querying for source position twice. + if (!preserveNewlines) { + return getLinesBetweenRangeEndAndRangeStart(node1, node2, currentSourceFile!, /*includeComments*/ false); + } + // We start by measuring the line difference from node1's end to node2's comments start, + // so that this is counted as a one-line difference, not two: // // node1; // // NODE2 COMMENT // node2; - const lines = getLinesBetweenPositions(node1, node2, currentSourceFile!, /*includeComments*/ true); + const lines = getLinesBetweenRangeEndAndRangeStart(node1, node2, currentSourceFile!, /*includeComments*/ true); if (lines === 0) { // However, if the line difference considering node2's comments was 0, we might have this: // // node1; // NODE2 COMMENT // node2; // - // in which case we should be ignoring node2's comment. - return getLinesBetweenPositions(node1, node2, currentSourceFile!, /*includeComments*/ false); + // in which case we should be ignoring node2's comment, so this too is counted as + // a one-line difference, not zero. + return getLinesBetweenRangeEndAndRangeStart(node1, node2, currentSourceFile!, /*includeComments*/ false); } return lines; } @@ -4386,7 +4390,7 @@ namespace ts { } if (!nodeIsSynthesized(parent) && !nodeIsSynthesized(node1) && !nodeIsSynthesized(node2)) { - const lines = getEffectiveLinesBetweenRanges(node1, node2, getLinesBetweenRangeEndAndRangeStart); + const lines = getEffectiveLinesBetweenRanges(node1, node2); return preserveNewlines ? lines : Math.min(lines, 1); } diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index 085670d31b84c..dc9dc0db52499 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -4703,7 +4703,7 @@ namespace ts { export function getLinesBetweenPositionAndPrecedingNonWhitespaceCharacter(pos: number, sourceFile: SourceFile, includeComments?: boolean) { const startPos = skipTrivia(sourceFile.text, pos, /*stopAfterLineBreak*/ false, includeComments); - const prevPos = getPreviousNonWhitespacePosition(pos, sourceFile); + const prevPos = getPreviousNonWhitespacePosition(startPos, sourceFile); return getLineOfLocalPosition(sourceFile, startPos) - getLineOfLocalPosition(sourceFile, prevPos || 0); } From c91efd4a85cf451ebdda5de624e41abf3588c84c Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Tue, 11 Feb 2020 10:29:08 -0800 Subject: [PATCH 07/24] Fix accidental find/replace rename --- src/compiler/emitter.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/compiler/emitter.ts b/src/compiler/emitter.ts index 82a8ab1f1283e..ae4a3796ddf9e 100644 --- a/src/compiler/emitter.ts +++ b/src/compiler/emitter.ts @@ -4025,7 +4025,7 @@ namespace ts { // Emit each child. let previousSibling: Node | undefined; let previousSourceFileTextKind: ReturnType; - let shouldDecreaselinesAfterEmit = false; + let shouldDecreaseIndentAfterEmit = false; for (let i = 0; i < count; i++) { const child = children![start + i]; @@ -4055,7 +4055,7 @@ namespace ts { // line, we should increase the indent. if ((format & (ListFormat.LinesMask | ListFormat.Indented)) === ListFormat.SingleLine) { increaseIndent(); - shouldDecreaselinesAfterEmit = true; + shouldDecreaseIndentAfterEmit = true; } writeLine(separatingLineTerminatorCount); @@ -4080,9 +4080,9 @@ namespace ts { emit(child); - if (shouldDecreaselinesAfterEmit) { + if (shouldDecreaseIndentAfterEmit) { decreaseIndent(); - shouldDecreaselinesAfterEmit = false; + shouldDecreaseIndentAfterEmit = false; } previousSibling = child; From e3ef42743a99333a15d3cb357125c67aa13f2d03 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Wed, 19 Feb 2020 09:20:51 -0800 Subject: [PATCH 08/24] Restore some monomorphism --- src/compiler/emitter.ts | 35 ++++++++++++++++++++++------------- src/compiler/types.ts | 3 ++- src/compiler/utilities.ts | 19 ++++++++++++------- 3 files changed, 36 insertions(+), 21 deletions(-) diff --git a/src/compiler/emitter.ts b/src/compiler/emitter.ts index ae4a3796ddf9e..df146d9dbbb51 100644 --- a/src/compiler/emitter.ts +++ b/src/compiler/emitter.ts @@ -2297,7 +2297,7 @@ namespace ts { emitTokenWithComment(token.kind, node.expression.end, writePunctuation, node); writeLinesAndIndent(linesAfterDot, /*writeSpaceIfNotIndenting*/ false); emit(node.name); - decreaseIndentIf(linesBeforeDot, linesAfterDot); + decreaseIndentIf(linesBeforeDot > 0, linesAfterDot > 0); } // 1..toString is a valid property access, emit a dot after the literal @@ -2475,7 +2475,7 @@ namespace ts { case EmitBinaryExpressionState.FinishEmit: { const linesBeforeOperator = getLinesBetweenNodes(node, node.left, node.operatorToken); const linesAfterOperator = getLinesBetweenNodes(node, node.operatorToken, node.right); - decreaseIndentIf(linesBeforeOperator, linesAfterOperator); + decreaseIndentIf(linesBeforeOperator > 0, linesAfterOperator > 0); stackIndex--; break; } @@ -2529,13 +2529,13 @@ namespace ts { emit(node.questionToken); writeLinesAndIndent(linesAfterQuestion, /*writeSpaceIfNotIndenting*/ true); emitExpression(node.whenTrue); - decreaseIndentIf(linesBeforeQuestion, linesAfterQuestion); + decreaseIndentIf(linesBeforeQuestion > 0, linesAfterQuestion > 0); writeLinesAndIndent(linesBeforeColon, /*writeSpaceIfNotIndenting*/ true); emit(node.colonToken); writeLinesAndIndent(linesAfterColon, /*writeSpaceIfNotIndenting*/ true); emitExpression(node.whenFalse); - decreaseIndentIf(linesBeforeColon, linesAfterColon); + decreaseIndentIf(linesBeforeColon > 0, linesAfterColon > 0); } function emitTemplateExpression(node: TemplateExpression) { @@ -4010,7 +4010,7 @@ namespace ts { let shouldEmitInterveningComments = mayEmitInterveningComments; const leadingLineTerminatorCount = getLeadingLineTerminatorCount(parentNode, children!, format); // TODO: GH#18217 if (leadingLineTerminatorCount) { - writeLine(leadingLineTerminatorCount); + writeBlankLines(leadingLineTerminatorCount); shouldEmitInterveningComments = false; } else if (format & ListFormat.SpaceBetweenBraces) { @@ -4058,7 +4058,7 @@ namespace ts { shouldDecreaseIndentAfterEmit = true; } - writeLine(separatingLineTerminatorCount); + writeBlankLines(separatingLineTerminatorCount); shouldEmitInterveningComments = false; } else if (previousSibling && format & ListFormat.SpaceBetweenSiblings) { @@ -4115,7 +4115,7 @@ namespace ts { // Write the closing line terminator or closing whitespace. const closingLineTerminatorCount = getClosingLineTerminatorCount(parentNode, children!, format); if (closingLineTerminatorCount) { - writeLine(closingLineTerminatorCount); + writeBlankLines(closingLineTerminatorCount); } else if (format & ListFormat.SpaceBetweenBraces) { writeSpace(); @@ -4185,10 +4185,8 @@ namespace ts { writer.writeProperty(s); } - function writeLine(count = 1) { - for (let i = 0; i < count; i++) { - writer.writeLine(i > 0); - } + function writeLine() { + writer.writeLine(); } function increaseIndent() { @@ -4244,10 +4242,21 @@ namespace ts { } } + function writeBlankLines(lineCount: number) { + for (let i = 0; i < lineCount; i++) { + if (i === 0) { + writer.writeLine(); + } + else { + writer.forceWriteLine(); + } + } + } + function writeLinesAndIndent(lineCount: number, writeSpaceIfNotIndenting: boolean) { if (lineCount) { increaseIndent(); - writeLine(lineCount); + writeBlankLines(lineCount); } else if (writeSpaceIfNotIndenting) { writeSpace(); @@ -4258,7 +4267,7 @@ namespace ts { // previous indent values to be considered at a time. This also allows caller to just // call this once, passing in all their appropriate indent values, instead of needing // to call this helper function multiple times. - function decreaseIndentIf(value1: boolean | number, value2: boolean | number) { + function decreaseIndentIf(value1: boolean, value2: boolean) { if (value1) { decreaseIndent(); } diff --git a/src/compiler/types.ts b/src/compiler/types.ts index a6a16eebeacb1..d988b77ced0ec 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -3753,7 +3753,7 @@ namespace ts { writeParameter(text: string): void; writeProperty(text: string): void; writeSymbol(text: string, symbol: Symbol): void; - writeLine(force?: boolean): void; + writeLine(): void; increaseIndent(): void; decreaseIndent(): void; clear(): void; @@ -6328,6 +6328,7 @@ namespace ts { getText(): string; rawWrite(s: string): void; writeLiteral(s: string): void; + forceWriteLine(): void; getTextPos(): number; getLine(): number; getColumn(): number; diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index dc9dc0db52499..5242711efea43 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -3569,13 +3569,17 @@ namespace ts { } } - function writeLine(force?: boolean) { - if (!lineStart || force) { - output += newLine; - lineCount++; - linePos = output.length; - lineStart = true; - hasTrailingComment = false; + function forceWriteLine() { + output += newLine; + lineCount++; + linePos = output.length; + lineStart = true; + hasTrailingComment = false; + } + + function writeLine() { + if (!lineStart) { + forceWriteLine(); } } @@ -3590,6 +3594,7 @@ namespace ts { rawWrite, writeLiteral, writeLine, + forceWriteLine, increaseIndent: () => { indent++; }, decreaseIndent: () => { indent--; }, getIndent: () => indent, From e535e279f99a423aab44dca8eca46f46669d045f Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Wed, 19 Feb 2020 10:02:24 -0800 Subject: [PATCH 09/24] Fix single line writer --- src/compiler/utilities.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index 5242711efea43..4b700fb211d25 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -75,6 +75,7 @@ namespace ts { // Completely ignore indentation for string writers. And map newlines to // a single space. writeLine: () => str += " ", + forceWriteLine: () => str += " ", increaseIndent: noop, decreaseIndent: noop, clear: () => str = "", From 21b0cb8f3beb6915eb969e4924ce0ad4b1645876 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Wed, 19 Feb 2020 10:27:15 -0800 Subject: [PATCH 10/24] Fix other writers --- src/services/textChanges.ts | 8 ++++++-- src/services/utilities.ts | 1 + 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/services/textChanges.ts b/src/services/textChanges.ts index a6301a452259c..6b2fe04035a2a 100644 --- a/src/services/textChanges.ts +++ b/src/services/textChanges.ts @@ -1053,8 +1053,11 @@ namespace ts.textChanges { writer.writeSymbol(s, sym); setLastNonTriviaPosition(s, /*force*/ false); } - function writeLine(force?: boolean): void { - writer.writeLine(force); + function writeLine(): void { + writer.writeLine(); + } + function forceWriteLine() { + writer.forceWriteLine(); } function increaseIndent(): void { writer.increaseIndent(); @@ -1111,6 +1114,7 @@ namespace ts.textChanges { writeStringLiteral, writeSymbol, writeLine, + forceWriteLine, increaseIndent, decreaseIndent, getText, diff --git a/src/services/utilities.ts b/src/services/utilities.ts index 655d6d890bad5..562c343a1f1b1 100644 --- a/src/services/utilities.ts +++ b/src/services/utilities.ts @@ -1912,6 +1912,7 @@ namespace ts { writeLiteral: text => writeKind(text, SymbolDisplayPartKind.stringLiteral), writeSymbol, writeLine, + forceWriteLine: writeLine, write: unknownWrite, writeComment: unknownWrite, getText: () => "", From 0c536c505a96fea3a3cfcdeff771ac30c67e2858 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Wed, 19 Feb 2020 11:19:39 -0800 Subject: [PATCH 11/24] Revert "Fix other writers" This reverts commit 21b0cb8f3beb6915eb969e4924ce0ad4b1645876. --- src/services/textChanges.ts | 8 ++------ src/services/utilities.ts | 1 - 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/src/services/textChanges.ts b/src/services/textChanges.ts index 2d7770e49931d..615eeda5feb7c 100644 --- a/src/services/textChanges.ts +++ b/src/services/textChanges.ts @@ -1053,11 +1053,8 @@ namespace ts.textChanges { writer.writeSymbol(s, sym); setLastNonTriviaPosition(s, /*force*/ false); } - function writeLine(): void { - writer.writeLine(); - } - function forceWriteLine() { - writer.forceWriteLine(); + function writeLine(force?: boolean): void { + writer.writeLine(force); } function increaseIndent(): void { writer.increaseIndent(); @@ -1114,7 +1111,6 @@ namespace ts.textChanges { writeStringLiteral, writeSymbol, writeLine, - forceWriteLine, increaseIndent, decreaseIndent, getText, diff --git a/src/services/utilities.ts b/src/services/utilities.ts index 3cc23e5729c78..d94106486446d 100644 --- a/src/services/utilities.ts +++ b/src/services/utilities.ts @@ -1912,7 +1912,6 @@ namespace ts { writeLiteral: text => writeKind(text, SymbolDisplayPartKind.stringLiteral), writeSymbol, writeLine, - forceWriteLine: writeLine, write: unknownWrite, writeComment: unknownWrite, getText: () => "", From 91eaf800355d48519e7472e9d25f5397913893a8 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Wed, 19 Feb 2020 11:20:18 -0800 Subject: [PATCH 12/24] Revert "Fix single line writer" This reverts commit e535e279f99a423aab44dca8eca46f46669d045f. --- src/compiler/utilities.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index b6e2b256b1e23..510820f5be6c1 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -75,7 +75,6 @@ namespace ts { // Completely ignore indentation for string writers. And map newlines to // a single space. writeLine: () => str += " ", - forceWriteLine: () => str += " ", increaseIndent: noop, decreaseIndent: noop, clear: () => str = "", From 3be2c866a28567b67b205d3c17ab29b067513962 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Wed, 19 Feb 2020 11:20:34 -0800 Subject: [PATCH 13/24] Revert "Restore some monomorphism" This reverts commit e3ef42743a99333a15d3cb357125c67aa13f2d03. --- src/compiler/emitter.ts | 35 +++++++++++++---------------------- src/compiler/types.ts | 3 +-- src/compiler/utilities.ts | 19 +++++++------------ 3 files changed, 21 insertions(+), 36 deletions(-) diff --git a/src/compiler/emitter.ts b/src/compiler/emitter.ts index df146d9dbbb51..ae4a3796ddf9e 100644 --- a/src/compiler/emitter.ts +++ b/src/compiler/emitter.ts @@ -2297,7 +2297,7 @@ namespace ts { emitTokenWithComment(token.kind, node.expression.end, writePunctuation, node); writeLinesAndIndent(linesAfterDot, /*writeSpaceIfNotIndenting*/ false); emit(node.name); - decreaseIndentIf(linesBeforeDot > 0, linesAfterDot > 0); + decreaseIndentIf(linesBeforeDot, linesAfterDot); } // 1..toString is a valid property access, emit a dot after the literal @@ -2475,7 +2475,7 @@ namespace ts { case EmitBinaryExpressionState.FinishEmit: { const linesBeforeOperator = getLinesBetweenNodes(node, node.left, node.operatorToken); const linesAfterOperator = getLinesBetweenNodes(node, node.operatorToken, node.right); - decreaseIndentIf(linesBeforeOperator > 0, linesAfterOperator > 0); + decreaseIndentIf(linesBeforeOperator, linesAfterOperator); stackIndex--; break; } @@ -2529,13 +2529,13 @@ namespace ts { emit(node.questionToken); writeLinesAndIndent(linesAfterQuestion, /*writeSpaceIfNotIndenting*/ true); emitExpression(node.whenTrue); - decreaseIndentIf(linesBeforeQuestion > 0, linesAfterQuestion > 0); + decreaseIndentIf(linesBeforeQuestion, linesAfterQuestion); writeLinesAndIndent(linesBeforeColon, /*writeSpaceIfNotIndenting*/ true); emit(node.colonToken); writeLinesAndIndent(linesAfterColon, /*writeSpaceIfNotIndenting*/ true); emitExpression(node.whenFalse); - decreaseIndentIf(linesBeforeColon > 0, linesAfterColon > 0); + decreaseIndentIf(linesBeforeColon, linesAfterColon); } function emitTemplateExpression(node: TemplateExpression) { @@ -4010,7 +4010,7 @@ namespace ts { let shouldEmitInterveningComments = mayEmitInterveningComments; const leadingLineTerminatorCount = getLeadingLineTerminatorCount(parentNode, children!, format); // TODO: GH#18217 if (leadingLineTerminatorCount) { - writeBlankLines(leadingLineTerminatorCount); + writeLine(leadingLineTerminatorCount); shouldEmitInterveningComments = false; } else if (format & ListFormat.SpaceBetweenBraces) { @@ -4058,7 +4058,7 @@ namespace ts { shouldDecreaseIndentAfterEmit = true; } - writeBlankLines(separatingLineTerminatorCount); + writeLine(separatingLineTerminatorCount); shouldEmitInterveningComments = false; } else if (previousSibling && format & ListFormat.SpaceBetweenSiblings) { @@ -4115,7 +4115,7 @@ namespace ts { // Write the closing line terminator or closing whitespace. const closingLineTerminatorCount = getClosingLineTerminatorCount(parentNode, children!, format); if (closingLineTerminatorCount) { - writeBlankLines(closingLineTerminatorCount); + writeLine(closingLineTerminatorCount); } else if (format & ListFormat.SpaceBetweenBraces) { writeSpace(); @@ -4185,8 +4185,10 @@ namespace ts { writer.writeProperty(s); } - function writeLine() { - writer.writeLine(); + function writeLine(count = 1) { + for (let i = 0; i < count; i++) { + writer.writeLine(i > 0); + } } function increaseIndent() { @@ -4242,21 +4244,10 @@ namespace ts { } } - function writeBlankLines(lineCount: number) { - for (let i = 0; i < lineCount; i++) { - if (i === 0) { - writer.writeLine(); - } - else { - writer.forceWriteLine(); - } - } - } - function writeLinesAndIndent(lineCount: number, writeSpaceIfNotIndenting: boolean) { if (lineCount) { increaseIndent(); - writeBlankLines(lineCount); + writeLine(lineCount); } else if (writeSpaceIfNotIndenting) { writeSpace(); @@ -4267,7 +4258,7 @@ namespace ts { // previous indent values to be considered at a time. This also allows caller to just // call this once, passing in all their appropriate indent values, instead of needing // to call this helper function multiple times. - function decreaseIndentIf(value1: boolean, value2: boolean) { + function decreaseIndentIf(value1: boolean | number, value2: boolean | number) { if (value1) { decreaseIndent(); } diff --git a/src/compiler/types.ts b/src/compiler/types.ts index 6a81fd6af4f46..c2a6e0f8be8b1 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -3753,7 +3753,7 @@ namespace ts { writeParameter(text: string): void; writeProperty(text: string): void; writeSymbol(text: string, symbol: Symbol): void; - writeLine(): void; + writeLine(force?: boolean): void; increaseIndent(): void; decreaseIndent(): void; clear(): void; @@ -6335,7 +6335,6 @@ namespace ts { getText(): string; rawWrite(s: string): void; writeLiteral(s: string): void; - forceWriteLine(): void; getTextPos(): number; getLine(): number; getColumn(): number; diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index 510820f5be6c1..917abc6c636a9 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -3569,17 +3569,13 @@ namespace ts { } } - function forceWriteLine() { - output += newLine; - lineCount++; - linePos = output.length; - lineStart = true; - hasTrailingComment = false; - } - - function writeLine() { - if (!lineStart) { - forceWriteLine(); + function writeLine(force?: boolean) { + if (!lineStart || force) { + output += newLine; + lineCount++; + linePos = output.length; + lineStart = true; + hasTrailingComment = false; } } @@ -3594,7 +3590,6 @@ namespace ts { rawWrite, writeLiteral, writeLine, - forceWriteLine, increaseIndent: () => { indent++; }, decreaseIndent: () => { indent--; }, getIndent: () => indent, From 8ed98bc84533cd2c502a5467d2e51c2611e1f04c Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Wed, 19 Feb 2020 11:23:13 -0800 Subject: [PATCH 14/24] Add equal position optimization to getLinesBetweenRangeEndAndRangeStart --- src/compiler/utilities.ts | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index 917abc6c636a9..29e0e73de78a3 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -4664,7 +4664,10 @@ namespace ts { } export function rangeStartPositionsAreOnSameLine(range1: TextRange, range2: TextRange, sourceFile: SourceFile) { - return positionsAreOnSameLine(getStartPositionOfRange(range1, sourceFile), getStartPositionOfRange(range2, sourceFile), sourceFile); + return positionsAreOnSameLine( + getStartPositionOfRange(range1, sourceFile, /*includeComments*/ false), + getStartPositionOfRange(range2, sourceFile, /*includeComments*/ false), + sourceFile); } export function rangeEndPositionsAreOnSameLine(range1: TextRange, range2: TextRange, sourceFile: SourceFile) { @@ -4672,23 +4675,20 @@ namespace ts { } export function rangeStartIsOnSameLineAsRangeEnd(range1: TextRange, range2: TextRange, sourceFile: SourceFile) { - return positionsAreOnSameLine(getStartPositionOfRange(range1, sourceFile), range2.end, sourceFile); + return positionsAreOnSameLine(getStartPositionOfRange(range1, sourceFile, /*includeComments*/ false), range2.end, sourceFile); } export function rangeEndIsOnSameLineAsRangeStart(range1: TextRange, range2: TextRange, sourceFile: SourceFile) { - return positionsAreOnSameLine(range1.end, getStartPositionOfRange(range2, sourceFile), sourceFile); + return positionsAreOnSameLine(range1.end, getStartPositionOfRange(range2, sourceFile, /*includeComments*/ false), sourceFile); } 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)); + const range2Start = getStartPositionOfRange(range2, sourceFile, includeSecondRangeComments); + return range2Start === range1.end ? 0 : getLineOfLocalPosition(sourceFile, range2Start) - getLineOfLocalPosition(sourceFile, range1.end); } export function getLinesBetweenRangeEndPositions(range1: TextRange, range2: TextRange, sourceFile: SourceFile) { - return getLineOfLocalPosition(sourceFile, range2.end) - getLineOfLocalPosition(sourceFile, range1.end); + return range1.end === range2.end ? 0 : getLineOfLocalPosition(sourceFile, range2.end) - getLineOfLocalPosition(sourceFile, range1.end); } export function isNodeArrayMultiLine(list: NodeArray, sourceFile: SourceFile): boolean { @@ -4700,7 +4700,7 @@ namespace ts { getLineOfLocalPosition(sourceFile, pos1) === getLineOfLocalPosition(sourceFile, pos2); } - export function getStartPositionOfRange(range: TextRange, sourceFile: SourceFile, includeComments?: boolean) { + export function getStartPositionOfRange(range: TextRange, sourceFile: SourceFile, includeComments: boolean) { return positionIsSynthesized(range.pos) ? -1 : skipTrivia(sourceFile.text, range.pos, /*stopAfterLineBreak*/ false, includeComments); } From d9c80fd7c68d142d17e49b0bcdf2ff630719e6af Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Wed, 19 Feb 2020 12:21:30 -0800 Subject: [PATCH 15/24] Add one more test --- .../fourslash/textChangesPreserveNewlines6.ts | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 tests/cases/fourslash/textChangesPreserveNewlines6.ts diff --git a/tests/cases/fourslash/textChangesPreserveNewlines6.ts b/tests/cases/fourslash/textChangesPreserveNewlines6.ts new file mode 100644 index 0000000000000..d630fb47e0f48 --- /dev/null +++ b/tests/cases/fourslash/textChangesPreserveNewlines6.ts @@ -0,0 +1,31 @@ +/// + +//// /*1*/f // call expression +//// (arg)( +//// /** @type {number} */ +//// blah, +//// +//// blah +//// +//// );/*2*/ + +goTo.select("1", "2"); +edit.applyRefactor({ + refactorName: "Extract Symbol", + actionName: "function_scope_0", + actionDescription: "Extract to function in global scope", + newContent: +`/*RENAME*/newFunction(); + +function newFunction() { + f // call expression + (arg)( + /** @type {number} */ + blah, + + blah + + ); +} +` +}); From af188d411e6fece39d0d6783997bdb3015997d31 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Wed, 19 Feb 2020 13:35:34 -0800 Subject: [PATCH 16/24] Actually save the test file --- tests/cases/fourslash/textChangesPreserveNewlines6.ts | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/tests/cases/fourslash/textChangesPreserveNewlines6.ts b/tests/cases/fourslash/textChangesPreserveNewlines6.ts index d630fb47e0f48..d3f5032047292 100644 --- a/tests/cases/fourslash/textChangesPreserveNewlines6.ts +++ b/tests/cases/fourslash/textChangesPreserveNewlines6.ts @@ -3,13 +3,14 @@ //// /*1*/f // call expression //// (arg)( //// /** @type {number} */ -//// blah, -//// -//// blah +//// blah, /* another param */ blah // TODO: name variable not 'blah' //// //// );/*2*/ goTo.select("1", "2"); + +// Note: the loss of `// TODO: name variable not 'blah'` +// is not desirable, but not related to this test. edit.applyRefactor({ refactorName: "Extract Symbol", actionName: "function_scope_0", @@ -21,9 +22,7 @@ function newFunction() { f // call expression (arg)( /** @type {number} */ - blah, - - blah + blah, /* another param */ blah ); } From 19a9728837ea4a56031057ce96402b1ad4bd58d6 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Fri, 28 Feb 2020 15:26:32 -0800 Subject: [PATCH 17/24] Rename preserveNewlines to preserveSourceNewlines --- src/compiler/emitter.ts | 30 +++++++++++++++--------------- src/compiler/types.ts | 4 ++-- src/services/textChanges.ts | 2 +- 3 files changed, 18 insertions(+), 18 deletions(-) diff --git a/src/compiler/emitter.ts b/src/compiler/emitter.ts index ae4a3796ddf9e..17f7dc349e843 100644 --- a/src/compiler/emitter.ts +++ b/src/compiler/emitter.ts @@ -842,7 +842,7 @@ namespace ts { let tempFlags: TempFlags; // TempFlags for the current name generation scope. let reservedNamesStack: Map[]; // Stack of TempFlags reserved in enclosing name generation scopes. let reservedNames: Map; // TempFlags to reserve in nested name generation scopes. - let preserveNewlines = printerOptions.preserveNewlines; // Can be overridden inside nodes with the `IgnoreSourceNewlines` emit flag. + let preserveSourceNewlines = printerOptions.preserveSourceNewlines; // Can be overridden inside nodes with the `IgnoreSourceNewlines` emit flag. let writer: EmitTextWriter; let ownWriter: EmitTextWriter; // Reusable `EmitTextWriter` for basic printing. @@ -1165,11 +1165,11 @@ namespace ts { function pipelineEmit(emitHint: EmitHint, node: Node) { const savedLastNode = lastNode; const savedLastSubstitution = lastSubstitution; - const savedPreserveNewlines = preserveNewlines; + const savedPreserveSourceNewlines = preserveSourceNewlines; lastNode = node; lastSubstitution = undefined; - if (preserveNewlines && !!(getEmitFlags(node) & EmitFlags.IgnoreSourceNewlines)) { - preserveNewlines = false; + if (preserveSourceNewlines && !!(getEmitFlags(node) & EmitFlags.IgnoreSourceNewlines)) { + preserveSourceNewlines = false; } const pipelinePhase = getPipelinePhase(PipelinePhase.Notification, emitHint, node); @@ -1180,7 +1180,7 @@ namespace ts { const substitute = lastSubstitution; lastNode = savedLastNode; lastSubstitution = savedLastSubstitution; - preserveNewlines = savedPreserveNewlines; + preserveSourceNewlines = savedPreserveSourceNewlines; return substitute || node; } @@ -3997,7 +3997,7 @@ namespace ts { if (isEmpty) { // Write a line terminator if the parent node was multi-line - if (format & ListFormat.MultiLine && !(preserveNewlines && rangeIsOnSingleLine(parentNode, currentSourceFile!))) { + if (format & ListFormat.MultiLine && !(preserveSourceNewlines && rangeIsOnSingleLine(parentNode, currentSourceFile!))) { writeLine(); } else if (format & ListFormat.SpaceBetweenBraces && !(format & ListFormat.NoSpaceIfEmpty)) { @@ -4268,7 +4268,7 @@ namespace ts { } function getLeadingLineTerminatorCount(parentNode: TextRange, children: NodeArray, format: ListFormat): number { - if (format & ListFormat.PreserveLines || preserveNewlines) { + if (format & ListFormat.PreserveLines || preserveSourceNewlines) { if (format & ListFormat.PreferNewLine) { return 1; } @@ -4278,7 +4278,7 @@ namespace ts { return rangeIsOnSingleLine(parentNode, currentSourceFile!) ? 0 : 1; } else if (!positionIsSynthesized(parentNode.pos) && !nodeIsSynthesized(firstChild) && firstChild.parent === parentNode) { - if (preserveNewlines) { + if (preserveSourceNewlines) { const lines = getLinesBetweenPositionAndPrecedingNonWhitespaceCharacter(firstChild.pos, currentSourceFile!, /*includeComments*/ true); return lines === 0 ? getLinesBetweenPositionAndPrecedingNonWhitespaceCharacter(firstChild.pos, currentSourceFile!, /*includeComments*/ false) @@ -4294,13 +4294,13 @@ namespace ts { } function getSeparatingLineTerminatorCount(previousNode: Node | undefined, nextNode: Node, format: ListFormat): number { - if (format & ListFormat.PreserveLines || preserveNewlines) { + if (format & ListFormat.PreserveLines || preserveSourceNewlines) { if (previousNode === undefined || nextNode === undefined) { return 0; } else if (!nodeIsSynthesized(previousNode) && !nodeIsSynthesized(nextNode) && previousNode.parent === nextNode.parent) { const lines = getEffectiveLinesBetweenRanges(previousNode, nextNode); - return preserveNewlines ? lines : Math.min(lines, 1); + return preserveSourceNewlines ? lines : Math.min(lines, 1); } else if (synthesizedNodeStartsOnNewLine(previousNode, format) || synthesizedNodeStartsOnNewLine(nextNode, format)) { return 1; @@ -4313,7 +4313,7 @@ namespace ts { } function getClosingLineTerminatorCount(parentNode: TextRange, children: NodeArray, format: ListFormat): number { - if (format & ListFormat.PreserveLines || preserveNewlines) { + if (format & ListFormat.PreserveLines || preserveSourceNewlines) { if (format & ListFormat.PreferNewLine) { return 1; } @@ -4324,7 +4324,7 @@ namespace ts { } else if (!positionIsSynthesized(parentNode.pos) && !nodeIsSynthesized(lastChild) && lastChild.parent === parentNode) { const lines = getLinesBetweenRangeEndPositions(lastChild, parentNode, currentSourceFile!); - return preserveNewlines ? lines : Math.min(lines, 1); + return preserveSourceNewlines ? lines : Math.min(lines, 1); } else if (synthesizedNodeStartsOnNewLine(lastChild, format)) { return 1; @@ -4337,9 +4337,9 @@ namespace ts { } function getEffectiveLinesBetweenRanges(node1: TextRange, node2: TextRange) { - // If 'preserveNewlines' is disabled, skip the more accurate check that might require + // If 'preserveSourceNewlines' is disabled, skip the more accurate check that might require // querying for source position twice. - if (!preserveNewlines) { + if (!preserveSourceNewlines) { return getLinesBetweenRangeEndAndRangeStart(node1, node2, currentSourceFile!, /*includeComments*/ false); } // We start by measuring the line difference from node1's end to node2's comments start, @@ -4391,7 +4391,7 @@ namespace ts { if (!nodeIsSynthesized(parent) && !nodeIsSynthesized(node1) && !nodeIsSynthesized(node2)) { const lines = getEffectiveLinesBetweenRanges(node1, node2); - return preserveNewlines ? lines : Math.min(lines, 1); + return preserveSourceNewlines ? lines : Math.min(lines, 1); } return 0; diff --git a/src/compiler/types.ts b/src/compiler/types.ts index c2a6e0f8be8b1..a7a6b9ab7538c 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -5787,7 +5787,7 @@ namespace ts { NoAsciiEscaping = 1 << 24, // When synthesizing nodes that lack an original node or textSourceNode, we want to write the text on the node with ASCII escaping substitutions. /*@internal*/ TypeScriptClassWrapper = 1 << 25, // The node is an IIFE class wrapper created by the ts transform. /*@internal*/ NeverApplyImportHelper = 1 << 26, // Indicates the node should never be wrapped with an import star helper (because, for example, it imports tslib itself) - /*@internal*/ IgnoreSourceNewlines = 1 << 27, // Overrides `printerOptions.preserveNewlines` to print this node (and all descendants) with default whitespace. + /*@internal*/ IgnoreSourceNewlines = 1 << 27, // Overrides `printerOptions.preserveSourceNewlines` to print this node (and all descendants) with default whitespace. } export interface EmitHelper { @@ -6250,7 +6250,7 @@ namespace ts { /*@internal*/ writeBundleFileInfo?: boolean; /*@internal*/ recordInternalSection?: boolean; /*@internal*/ stripInternal?: boolean; - /*@internal*/ preserveNewlines?: boolean; + /*@internal*/ preserveSourceNewlines?: boolean; /*@internal*/ relativeToBuildInfo?: (path: string) => string; } diff --git a/src/services/textChanges.ts b/src/services/textChanges.ts index 615eeda5feb7c..4cc9a27d1a235 100644 --- a/src/services/textChanges.ts +++ b/src/services/textChanges.ts @@ -923,7 +923,7 @@ namespace ts.textChanges { export function getNonformattedText(node: Node, sourceFile: SourceFile | undefined, newLineCharacter: string): { text: string, node: Node } { const writer = createWriter(newLineCharacter); const newLine = newLineCharacter === "\n" ? NewLineKind.LineFeed : NewLineKind.CarriageReturnLineFeed; - createPrinter({ newLine, neverAsciiEscape: true, preserveNewlines: true }, writer).writeNode(EmitHint.Unspecified, node, sourceFile, writer); + createPrinter({ newLine, neverAsciiEscape: true, preserveSourceNewlines: true }, writer).writeNode(EmitHint.Unspecified, node, sourceFile, writer); return { text: writer.getText(), node: assignPositionsToNode(node) }; } } From 131f2bb54e65b08a64be430e1313407d272cfe0d Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Fri, 28 Feb 2020 15:29:03 -0800 Subject: [PATCH 18/24] Make ignoreSourceNewlines internal --- src/compiler/factoryPublic.ts | 1 + tests/baselines/reference/api/tsserverlibrary.d.ts | 1 - tests/baselines/reference/api/typescript.d.ts | 1 - 3 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/compiler/factoryPublic.ts b/src/compiler/factoryPublic.ts index 90ff58e0a1a05..8a1b61df1a503 100644 --- a/src/compiler/factoryPublic.ts +++ b/src/compiler/factoryPublic.ts @@ -3559,6 +3559,7 @@ namespace ts { return node; } + /** @internal */ export function ignoreSourceNewlines(node: T): T { getOrCreateEmitNode(node).flags |= EmitFlags.IgnoreSourceNewlines; return node; diff --git a/tests/baselines/reference/api/tsserverlibrary.d.ts b/tests/baselines/reference/api/tsserverlibrary.d.ts index 41b2a896b699f..2d120de005258 100644 --- a/tests/baselines/reference/api/tsserverlibrary.d.ts +++ b/tests/baselines/reference/api/tsserverlibrary.d.ts @@ -4359,7 +4359,6 @@ declare namespace ts { function setSyntheticTrailingComments(node: T, comments: SynthesizedComment[] | undefined): T; function addSyntheticTrailingComment(node: T, kind: SyntaxKind.SingleLineCommentTrivia | SyntaxKind.MultiLineCommentTrivia, text: string, hasTrailingNewLine?: boolean): T; function moveSyntheticComments(node: T, original: Node): T; - function ignoreSourceNewlines(node: T): T; /** * Gets the constant value to emit for an expression. */ diff --git a/tests/baselines/reference/api/typescript.d.ts b/tests/baselines/reference/api/typescript.d.ts index e745d2eb8d419..8484fbf9edf54 100644 --- a/tests/baselines/reference/api/typescript.d.ts +++ b/tests/baselines/reference/api/typescript.d.ts @@ -4359,7 +4359,6 @@ declare namespace ts { function setSyntheticTrailingComments(node: T, comments: SynthesizedComment[] | undefined): T; function addSyntheticTrailingComment(node: T, kind: SyntaxKind.SingleLineCommentTrivia | SyntaxKind.MultiLineCommentTrivia, text: string, hasTrailingNewLine?: boolean): T; function moveSyntheticComments(node: T, original: Node): T; - function ignoreSourceNewlines(node: T): T; /** * Gets the constant value to emit for an expression. */ From 34147a57936256af1acdcc779926e28222a886bf Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Mon, 2 Mar 2020 11:05:03 -0800 Subject: [PATCH 19/24] Optimize lines-between functions --- src/compiler/scanner.ts | 30 ++++++++++++++++++++++++------ src/compiler/utilities.ts | 16 ++++++++-------- 2 files changed, 32 insertions(+), 14 deletions(-) diff --git a/src/compiler/scanner.ts b/src/compiler/scanner.ts index bf76051b6adb3..38c280b3d4008 100644 --- a/src/compiler/scanner.ts +++ b/src/compiler/scanner.ts @@ -399,11 +399,20 @@ namespace ts { } /* @internal */ + export function computeLineAndCharacterOfPosition(lineStarts: readonly number[], position: number): LineAndCharacter { + const lineNumber = computeLineOfPosition(lineStarts, position); + return { + line: lineNumber, + character: position - lineStarts[lineNumber] + }; + } + /** + * @internal * We assume the first line starts at position 0 and 'position' is non-negative. */ - export function computeLineAndCharacterOfPosition(lineStarts: readonly number[], position: number): LineAndCharacter { - let lineNumber = binarySearch(lineStarts, position, identity, compareValues); + export function computeLineOfPosition(lineStarts: readonly number[], position: number, lowerBound?: number) { + let lineNumber = binarySearch(lineStarts, position, identity, compareValues, lowerBound); if (lineNumber < 0) { // If the actual position was not found, // the binary search returns the 2's-complement of the next line start @@ -415,10 +424,19 @@ namespace ts { lineNumber = ~lineNumber - 1; Debug.assert(lineNumber !== -1, "position cannot precede the beginning of the file"); } - return { - line: lineNumber, - character: position - lineStarts[lineNumber] - }; + return lineNumber; + } + + /** @internal */ + export function getLinesBetweenPositions(sourceFile: SourceFileLike, pos1: number, pos2: number) { + if (pos1 === pos2) return 0; + const lineStarts = getLineStarts(sourceFile); + const lower = Math.min(pos1, pos2); + const isNegative = lower === pos2; + const upper = isNegative ? pos1 : pos2; + const lowerLine = computeLineOfPosition(lineStarts, lower); + const upperLine = computeLineOfPosition(lineStarts, upper, lowerLine); + return isNegative ? lowerLine - upperLine : upperLine - lowerLine; } export function getLineAndCharacterOfPosition(sourceFile: SourceFileLike, position: number): LineAndCharacter { diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index 29e0e73de78a3..23de1f992cd03 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -3841,12 +3841,13 @@ namespace ts { } } - export function getLineOfLocalPosition(currentSourceFile: SourceFile, pos: number) { - return getLineAndCharacterOfPosition(currentSourceFile, pos).line; + export function getLineOfLocalPosition(sourceFile: SourceFile, pos: number) { + const lineStarts = getLineStarts(sourceFile); + return computeLineOfPosition(lineStarts, pos); } export function getLineOfLocalPositionFromLineMap(lineMap: readonly number[], pos: number) { - return computeLineAndCharacterOfPosition(lineMap, pos).line; + return computeLineOfPosition(lineMap, pos); } export function getFirstConstructorWithBody(node: ClassLikeDeclaration): ConstructorDeclaration & { body: FunctionBody } | undefined { @@ -4684,11 +4685,11 @@ namespace ts { export function getLinesBetweenRangeEndAndRangeStart(range1: TextRange, range2: TextRange, sourceFile: SourceFile, includeSecondRangeComments: boolean) { const range2Start = getStartPositionOfRange(range2, sourceFile, includeSecondRangeComments); - return range2Start === range1.end ? 0 : getLineOfLocalPosition(sourceFile, range2Start) - getLineOfLocalPosition(sourceFile, range1.end); + return getLinesBetweenPositions(sourceFile, range1.end, range2Start); } export function getLinesBetweenRangeEndPositions(range1: TextRange, range2: TextRange, sourceFile: SourceFile) { - return range1.end === range2.end ? 0 : getLineOfLocalPosition(sourceFile, range2.end) - getLineOfLocalPosition(sourceFile, range1.end); + return getLinesBetweenPositions(sourceFile, range1.end, range2.end); } export function isNodeArrayMultiLine(list: NodeArray, sourceFile: SourceFile): boolean { @@ -4696,8 +4697,7 @@ namespace ts { } export function positionsAreOnSameLine(pos1: number, pos2: number, sourceFile: SourceFile) { - return pos1 === pos2 || - getLineOfLocalPosition(sourceFile, pos1) === getLineOfLocalPosition(sourceFile, pos2); + return getLinesBetweenPositions(sourceFile, pos1, pos2) === 0; } export function getStartPositionOfRange(range: TextRange, sourceFile: SourceFile, includeComments: boolean) { @@ -4707,7 +4707,7 @@ namespace ts { export function getLinesBetweenPositionAndPrecedingNonWhitespaceCharacter(pos: number, sourceFile: SourceFile, includeComments?: boolean) { const startPos = skipTrivia(sourceFile.text, pos, /*stopAfterLineBreak*/ false, includeComments); const prevPos = getPreviousNonWhitespacePosition(startPos, sourceFile); - return getLineOfLocalPosition(sourceFile, startPos) - getLineOfLocalPosition(sourceFile, prevPos || 0); + return getLinesBetweenPositions(sourceFile, prevPos || 0, startPos); } function getPreviousNonWhitespacePosition(pos: number, sourceFile: SourceFile) { From 075c7829c83a1e7a719a6d539a85bb267637a899 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Mon, 2 Mar 2020 13:04:49 -0800 Subject: [PATCH 20/24] Add comment; --- src/compiler/emitter.ts | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/compiler/emitter.ts b/src/compiler/emitter.ts index 80911494cd46a..4d38e14e93ead 100644 --- a/src/compiler/emitter.ts +++ b/src/compiler/emitter.ts @@ -4280,6 +4280,19 @@ namespace ts { } else if (!positionIsSynthesized(parentNode.pos) && !nodeIsSynthesized(firstChild) && firstChild.parent === parentNode) { if (preserveSourceNewlines) { + // To determine how many line breaks to write before `firstChild`, we first walk backwards from its comments, + // so that this is measured as a one-line difference, not two: + // + // function parentNode() { + // // FIRSTCHILD COMMENT + // firstChild; + // + // However, if that returns zero, we might have this: + // + // function parentNode() { // FIRSTCHILD COMMENT + // firstChild; + // + // in which case we should be ignoring the comment, so this too is counted as a one-line difference, not zero. const lines = getLinesBetweenPositionAndPrecedingNonWhitespaceCharacter(firstChild.pos, currentSourceFile!, /*includeComments*/ true); return lines === 0 ? getLinesBetweenPositionAndPrecedingNonWhitespaceCharacter(firstChild.pos, currentSourceFile!, /*includeComments*/ false) From 563d2230f43008632e06ab169707363caafaba37 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Fri, 13 Mar 2020 15:23:42 -0700 Subject: [PATCH 21/24] Fix trailing line terminator count bug for function parameters --- src/compiler/emitter.ts | 62 ++++++++++--------- src/compiler/utilities.ts | 5 ++ src/harness/fourslashImpl.ts | 2 +- .../fourslash/textChangesPreserveNewlines7.ts | 43 +++++++++++++ 4 files changed, 81 insertions(+), 31 deletions(-) create mode 100644 tests/cases/fourslash/textChangesPreserveNewlines7.ts diff --git a/src/compiler/emitter.ts b/src/compiler/emitter.ts index 4d38e14e93ead..e1ad5b63d2ec4 100644 --- a/src/compiler/emitter.ts +++ b/src/compiler/emitter.ts @@ -4280,23 +4280,11 @@ namespace ts { } else if (!positionIsSynthesized(parentNode.pos) && !nodeIsSynthesized(firstChild) && firstChild.parent === parentNode) { if (preserveSourceNewlines) { - // To determine how many line breaks to write before `firstChild`, we first walk backwards from its comments, - // so that this is measured as a one-line difference, not two: - // - // function parentNode() { - // // FIRSTCHILD COMMENT - // firstChild; - // - // However, if that returns zero, we might have this: - // - // function parentNode() { // FIRSTCHILD COMMENT - // firstChild; - // - // in which case we should be ignoring the comment, so this too is counted as a one-line difference, not zero. - const lines = getLinesBetweenPositionAndPrecedingNonWhitespaceCharacter(firstChild.pos, currentSourceFile!, /*includeComments*/ true); - return lines === 0 - ? getLinesBetweenPositionAndPrecedingNonWhitespaceCharacter(firstChild.pos, currentSourceFile!, /*includeComments*/ false) - : lines; + return getEffectiveLines( + includeComments => getLinesBetweenPositionAndPrecedingNonWhitespaceCharacter( + firstChild.pos, + currentSourceFile!, + includeComments)); } return rangeStartPositionsAreOnSameLine(parentNode, firstChild, currentSourceFile!) ? 0 : 1; } @@ -4313,8 +4301,12 @@ namespace ts { return 0; } else if (!nodeIsSynthesized(previousNode) && !nodeIsSynthesized(nextNode) && previousNode.parent === nextNode.parent) { - const lines = getEffectiveLinesBetweenRanges(previousNode, nextNode); - return preserveSourceNewlines ? lines : Math.min(lines, 1); + return getEffectiveLines( + includeComments => getLinesBetweenRangeEndAndRangeStart( + previousNode, + nextNode, + currentSourceFile!, + includeComments)); } else if (synthesizedNodeStartsOnNewLine(previousNode, format) || synthesizedNodeStartsOnNewLine(nextNode, format)) { return 1; @@ -4337,8 +4329,14 @@ namespace ts { return rangeIsOnSingleLine(parentNode, currentSourceFile!) ? 0 : 1; } else if (!positionIsSynthesized(parentNode.pos) && !nodeIsSynthesized(lastChild) && lastChild.parent === parentNode) { - const lines = getLinesBetweenRangeEndPositions(lastChild, parentNode, currentSourceFile!); - return preserveSourceNewlines ? lines : Math.min(lines, 1); + if (preserveSourceNewlines) { + return getEffectiveLines( + includeComments => getLinesBetweenPositionAndNextNonWhitespaceCharacter( + lastChild.end, + currentSourceFile!, + includeComments)); + } + return rangeEndPositionsAreOnSameLine(parentNode, lastChild, currentSourceFile!) ? 0 : 1; } else if (synthesizedNodeStartsOnNewLine(lastChild, format)) { return 1; @@ -4350,28 +4348,28 @@ namespace ts { return 0; } - function getEffectiveLinesBetweenRanges(node1: TextRange, node2: TextRange) { + function getEffectiveLines(getLineDifference: (includeComments: boolean) => number) { // If 'preserveSourceNewlines' is disabled, skip the more accurate check that might require - // querying for source position twice. + // querying for source positions twice. if (!preserveSourceNewlines) { - return getLinesBetweenRangeEndAndRangeStart(node1, node2, currentSourceFile!, /*includeComments*/ false); + return Math.min(1, getLineDifference(/*includeComments*/ false)); } - // We start by measuring the line difference from node1's end to node2's comments start, + // We start by measuring the line difference from a position to its adjacent comments, // so that this is counted as a one-line difference, not two: // // node1; // // NODE2 COMMENT // node2; - const lines = getLinesBetweenRangeEndAndRangeStart(node1, node2, currentSourceFile!, /*includeComments*/ true); + const lines = getLineDifference(/*includeComments*/ true); if (lines === 0) { - // However, if the line difference considering node2's comments was 0, we might have this: + // However, if the line difference considering comments was 0, we might have this: // // node1; // NODE2 COMMENT // node2; // // in which case we should be ignoring node2's comment, so this too is counted as // a one-line difference, not zero. - return getLinesBetweenRangeEndAndRangeStart(node1, node2, currentSourceFile!, /*includeComments*/ false); + return getLineDifference(/*includeComments*/ false); } return lines; } @@ -4404,8 +4402,12 @@ namespace ts { } if (!nodeIsSynthesized(parent) && !nodeIsSynthesized(node1) && !nodeIsSynthesized(node2)) { - const lines = getEffectiveLinesBetweenRanges(node1, node2); - return preserveSourceNewlines ? lines : Math.min(lines, 1); + return getEffectiveLines( + includeComments => getLinesBetweenRangeEndAndRangeStart( + node1, + node2, + currentSourceFile!, + includeComments)); } return 0; diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index 9d18167e0a274..8691bb3daf304 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -4735,6 +4735,11 @@ namespace ts { return getLinesBetweenPositions(sourceFile, prevPos || 0, startPos); } + export function getLinesBetweenPositionAndNextNonWhitespaceCharacter(pos: number, sourceFile: SourceFile, includeComments?: boolean) { + const nextPos = skipTrivia(sourceFile.text, pos, /*stopAfterLineBreak*/ false, includeComments); + return getLinesBetweenPositions(sourceFile, pos, nextPos); + } + function getPreviousNonWhitespacePosition(pos: number, sourceFile: SourceFile) { while (pos-- > 0) { if (!isWhiteSpaceLike(sourceFile.text.charCodeAt(pos))) { diff --git a/src/harness/fourslashImpl.ts b/src/harness/fourslashImpl.ts index 6d59fe66e68f4..4daf0e12f27af 100644 --- a/src/harness/fourslashImpl.ts +++ b/src/harness/fourslashImpl.ts @@ -3300,7 +3300,7 @@ namespace FourSlash { } public moveToNewFile(options: FourSlashInterface.MoveToNewFileOptions): void { - assert(this.getRanges().length === 1); + assert(this.getRanges().length === 1, "Must have exactly one fourslash range (source enclosed between '[|' and '|]' delimiters) in the source file"); const range = this.getRanges()[0]; const refactor = ts.find(this.getApplicableRefactors(range, { allowTextChangesInNewFiles: true }), r => r.name === "Move to a new file")!; assert(refactor.actions.length === 1); diff --git a/tests/cases/fourslash/textChangesPreserveNewlines7.ts b/tests/cases/fourslash/textChangesPreserveNewlines7.ts new file mode 100644 index 0000000000000..a70fe9ec58778 --- /dev/null +++ b/tests/cases/fourslash/textChangesPreserveNewlines7.ts @@ -0,0 +1,43 @@ +/// + +// @Filename: /index.tsx + +////[|function Foo({ label }: { label: string }) { +//// return ( +////
+////
+////
{label}
+////
+////
+//// ); +////}|] + +verify.moveToNewFile({ + newFileContents: { + "/index.tsx": "", + "/Foo.tsx": +`function Foo({ label }: { label: string; }) { + return ( +
+
+
{label}
+
+
+ ); +}` + } +}); From 8b61d665178149d25d81adbe018e2a084a22741c Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Fri, 13 Mar 2020 17:01:53 -0700 Subject: [PATCH 22/24] Preserve newlines around parenthesized expressions --- src/compiler/emitter.ts | 31 ++++++++++++++----- .../fourslash/textChangesPreserveNewlines7.ts | 3 +- 2 files changed, 26 insertions(+), 8 deletions(-) diff --git a/src/compiler/emitter.ts b/src/compiler/emitter.ts index e1ad5b63d2ec4..d777c1fc20b8c 100644 --- a/src/compiler/emitter.ts +++ b/src/compiler/emitter.ts @@ -2360,7 +2360,16 @@ namespace ts { function emitParenthesizedExpression(node: ParenthesizedExpression) { const openParenPos = emitTokenWithComment(SyntaxKind.OpenParenToken, node.pos, writePunctuation, node); + const leadingNewlines = getLeadingLineTerminatorCount(node, node.expression, ListFormat.None); + if (leadingNewlines) { + writeLinesAndIndent(leadingNewlines, /*writeLinesIfNotIndenting*/ false); + } emitExpression(node.expression); + const trailingNewlines = getClosingLineTerminatorCount(node, node.expression, ListFormat.None); + if (trailingNewlines) { + writeLine(trailingNewlines); + } + decreaseIndentIf(leadingNewlines); emitTokenWithComment(SyntaxKind.CloseParenToken, node.expression ? node.expression.end : openParenPos, writePunctuation, node); } @@ -4259,7 +4268,7 @@ namespace ts { // previous indent values to be considered at a time. This also allows caller to just // call this once, passing in all their appropriate indent values, instead of needing // to call this helper function multiple times. - function decreaseIndentIf(value1: boolean | number, value2: boolean | number) { + function decreaseIndentIf(value1: boolean | number | undefined, value2?: boolean | number) { if (value1) { decreaseIndent(); } @@ -4268,17 +4277,21 @@ namespace ts { } } - function getLeadingLineTerminatorCount(parentNode: TextRange, children: NodeArray, format: ListFormat): number { + function getLeadingLineTerminatorCount(parentNode: TextRange, children: NodeArray | Node, format: ListFormat): number { if (format & ListFormat.PreserveLines || preserveSourceNewlines) { if (format & ListFormat.PreferNewLine) { return 1; } - const firstChild = children[0]; + const firstChild = isArray(children) ? children[0] : children; if (firstChild === undefined) { return rangeIsOnSingleLine(parentNode, currentSourceFile!) ? 0 : 1; } - else if (!positionIsSynthesized(parentNode.pos) && !nodeIsSynthesized(firstChild) && firstChild.parent === parentNode) { + if (firstChild.kind === SyntaxKind.JsxText) { + // JsxText will be written with its leading whitespace, so don't add more manually. + return 0; + } + if (!positionIsSynthesized(parentNode.pos) && !nodeIsSynthesized(firstChild) && firstChild.parent === parentNode) { if (preserveSourceNewlines) { return getEffectiveLines( includeComments => getLinesBetweenPositionAndPrecedingNonWhitespaceCharacter( @@ -4288,7 +4301,7 @@ namespace ts { } return rangeStartPositionsAreOnSameLine(parentNode, firstChild, currentSourceFile!) ? 0 : 1; } - else if (synthesizedNodeStartsOnNewLine(firstChild, format)) { + if (synthesizedNodeStartsOnNewLine(firstChild, format)) { return 1; } } @@ -4300,6 +4313,10 @@ namespace ts { if (previousNode === undefined || nextNode === undefined) { return 0; } + if (nextNode.kind === SyntaxKind.JsxText) { + // JsxText will be written with its leading whitespace, so don't add more manually. + return 0; + } else if (!nodeIsSynthesized(previousNode) && !nodeIsSynthesized(nextNode) && previousNode.parent === nextNode.parent) { return getEffectiveLines( includeComments => getLinesBetweenRangeEndAndRangeStart( @@ -4318,13 +4335,13 @@ namespace ts { return format & ListFormat.MultiLine ? 1 : 0; } - function getClosingLineTerminatorCount(parentNode: TextRange, children: NodeArray, format: ListFormat): number { + function getClosingLineTerminatorCount(parentNode: TextRange, children: NodeArray | Node, format: ListFormat): number { if (format & ListFormat.PreserveLines || preserveSourceNewlines) { if (format & ListFormat.PreferNewLine) { return 1; } - const lastChild = lastOrUndefined(children); + const lastChild = isArray(children) ? lastOrUndefined(children) : children; if (lastChild === undefined) { return rangeIsOnSingleLine(parentNode, currentSourceFile!) ? 0 : 1; } diff --git a/tests/cases/fourslash/textChangesPreserveNewlines7.ts b/tests/cases/fourslash/textChangesPreserveNewlines7.ts index a70fe9ec58778..9c04d29968ed5 100644 --- a/tests/cases/fourslash/textChangesPreserveNewlines7.ts +++ b/tests/cases/fourslash/textChangesPreserveNewlines7.ts @@ -38,6 +38,7 @@ verify.moveToNewFile({ ); -}` +} +` } }); From e99d833d5a9054ed8758f32f9c89c56a9798ad39 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Mon, 16 Mar 2020 10:44:36 -0700 Subject: [PATCH 23/24] Back to speculative microoptimizations, yay --- src/compiler/emitter.ts | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/compiler/emitter.ts b/src/compiler/emitter.ts index 658fff1b09234..456cbf3163692 100644 --- a/src/compiler/emitter.ts +++ b/src/compiler/emitter.ts @@ -2365,12 +2365,12 @@ namespace ts { function emitParenthesizedExpression(node: ParenthesizedExpression) { const openParenPos = emitTokenWithComment(SyntaxKind.OpenParenToken, node.pos, writePunctuation, node); - const leadingNewlines = getLeadingLineTerminatorCount(node, node.expression, ListFormat.None); + const leadingNewlines = preserveSourceNewlines && getLeadingLineTerminatorCount(node, [node.expression], ListFormat.None); if (leadingNewlines) { writeLinesAndIndent(leadingNewlines, /*writeLinesIfNotIndenting*/ false); } emitExpression(node.expression); - const trailingNewlines = getClosingLineTerminatorCount(node, node.expression, ListFormat.None); + const trailingNewlines = preserveSourceNewlines && getClosingLineTerminatorCount(node, [node.expression], ListFormat.None); if (trailingNewlines) { writeLine(trailingNewlines); } @@ -4282,13 +4282,13 @@ namespace ts { } } - function getLeadingLineTerminatorCount(parentNode: TextRange, children: NodeArray | Node, format: ListFormat): number { + function getLeadingLineTerminatorCount(parentNode: TextRange, children: readonly Node[], format: ListFormat): number { if (format & ListFormat.PreserveLines || preserveSourceNewlines) { if (format & ListFormat.PreferNewLine) { return 1; } - const firstChild = isArray(children) ? children[0] : children; + const firstChild = children[0]; if (firstChild === undefined) { return rangeIsOnSingleLine(parentNode, currentSourceFile!) ? 0 : 1; } @@ -4340,17 +4340,17 @@ namespace ts { return format & ListFormat.MultiLine ? 1 : 0; } - function getClosingLineTerminatorCount(parentNode: TextRange, children: NodeArray | Node, format: ListFormat): number { + function getClosingLineTerminatorCount(parentNode: TextRange, children: readonly Node[], format: ListFormat): number { if (format & ListFormat.PreserveLines || preserveSourceNewlines) { if (format & ListFormat.PreferNewLine) { return 1; } - const lastChild = isArray(children) ? lastOrUndefined(children) : children; + const lastChild = lastOrUndefined(children); if (lastChild === undefined) { return rangeIsOnSingleLine(parentNode, currentSourceFile!) ? 0 : 1; } - else if (!positionIsSynthesized(parentNode.pos) && !nodeIsSynthesized(lastChild) && lastChild.parent === parentNode) { + if (!positionIsSynthesized(parentNode.pos) && !nodeIsSynthesized(lastChild) && lastChild.parent === parentNode) { if (preserveSourceNewlines) { return getEffectiveLines( includeComments => getLinesBetweenPositionAndNextNonWhitespaceCharacter( @@ -4360,7 +4360,7 @@ namespace ts { } return rangeEndPositionsAreOnSameLine(parentNode, lastChild, currentSourceFile!) ? 0 : 1; } - else if (synthesizedNodeStartsOnNewLine(lastChild, format)) { + if (synthesizedNodeStartsOnNewLine(lastChild, format)) { return 1; } } From ac768d030290290ade1a951624fb6f7d76521666 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Mon, 16 Mar 2020 11:51:57 -0700 Subject: [PATCH 24/24] =?UTF-8?q?Don=E2=80=99t=20call=20getEffectiveLines?= =?UTF-8?q?=20during=20tsc=20emit=20at=20all?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/compiler/emitter.ts | 38 +++++++++++++++++++++----------------- 1 file changed, 21 insertions(+), 17 deletions(-) diff --git a/src/compiler/emitter.ts b/src/compiler/emitter.ts index 456cbf3163692..80b32deebb501 100644 --- a/src/compiler/emitter.ts +++ b/src/compiler/emitter.ts @@ -4323,12 +4323,15 @@ namespace ts { return 0; } else if (!nodeIsSynthesized(previousNode) && !nodeIsSynthesized(nextNode) && previousNode.parent === nextNode.parent) { - return getEffectiveLines( - includeComments => getLinesBetweenRangeEndAndRangeStart( - previousNode, - nextNode, - currentSourceFile!, - includeComments)); + if (preserveSourceNewlines) { + return getEffectiveLines( + includeComments => getLinesBetweenRangeEndAndRangeStart( + previousNode, + nextNode, + currentSourceFile!, + includeComments)); + } + return rangeEndIsOnSameLineAsRangeStart(previousNode, nextNode, currentSourceFile!) ? 0 : 1; } else if (synthesizedNodeStartsOnNewLine(previousNode, format) || synthesizedNodeStartsOnNewLine(nextNode, format)) { return 1; @@ -4371,11 +4374,9 @@ namespace ts { } function getEffectiveLines(getLineDifference: (includeComments: boolean) => number) { - // If 'preserveSourceNewlines' is disabled, skip the more accurate check that might require - // querying for source positions twice. - if (!preserveSourceNewlines) { - return Math.min(1, getLineDifference(/*includeComments*/ false)); - } + // If 'preserveSourceNewlines' is disabled, we should never call this function + // because it could be more expensive than alternative approximations. + Debug.assert(!!preserveSourceNewlines); // We start by measuring the line difference from a position to its adjacent comments, // so that this is counted as a one-line difference, not two: // @@ -4424,12 +4425,15 @@ namespace ts { } if (!nodeIsSynthesized(parent) && !nodeIsSynthesized(node1) && !nodeIsSynthesized(node2)) { - return getEffectiveLines( - includeComments => getLinesBetweenRangeEndAndRangeStart( - node1, - node2, - currentSourceFile!, - includeComments)); + if (preserveSourceNewlines) { + return getEffectiveLines( + includeComments => getLinesBetweenRangeEndAndRangeStart( + node1, + node2, + currentSourceFile!, + includeComments)); + } + return rangeEndIsOnSameLineAsRangeStart(node1, node2, currentSourceFile!) ? 0 : 1; } return 0;