Skip to content

Commit 237ea52

Browse files
andrewbranchandrewbranch
authored
Preserve newlines from original source when printing nodes from TextChanges (#36688)
* Allow emitter to write multiple newlines in node lists * Progress * Progress * Fix recomputeIndentation * Add tests, fix leading line terminator count * Do a bit less work when `preserveNewlines` is off * Fix accidental find/replace rename * Restore some monomorphism * Fix single line writer * Fix other writers * Revert "Fix other writers" This reverts commit 21b0cb8. * Revert "Fix single line writer" This reverts commit e535e27. * Revert "Restore some monomorphism" This reverts commit e3ef427. * Add equal position optimization to getLinesBetweenRangeEndAndRangeStart * Add one more test * Actually save the test file * Rename preserveNewlines to preserveSourceNewlines * Make ignoreSourceNewlines internal * Optimize lines-between functions * Add comment; * Fix trailing line terminator count bug for function parameters * Preserve newlines around parenthesized expressions * Back to speculative microoptimizations, yay * Don’t call getEffectiveLines during tsc emit at all
1 parent 667f3b4 commit 237ea52

24 files changed

+477
-137
lines changed

‎src/compiler/emitter.ts

Lines changed: 158 additions & 85 deletions
Large diffs are not rendered by default.

‎src/compiler/factoryPublic.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3567,6 +3567,12 @@ namespace ts {
35673567
return node;
35683568
}
35693569

3570+
/** @internal */
3571+
export function ignoreSourceNewlines<T extends Node>(node: T): T {
3572+
getOrCreateEmitNode(node).flags |= EmitFlags.IgnoreSourceNewlines;
3573+
return node;
3574+
}
3575+
35703576
/**
35713577
* Gets the constant value to emit for an expression.
35723578
*/

‎src/compiler/scanner.ts

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -409,11 +409,20 @@ namespace ts {
409409
}
410410

411411
/* @internal */
412+
export function computeLineAndCharacterOfPosition(lineStarts: readonly number[], position: number): LineAndCharacter {
413+
const lineNumber = computeLineOfPosition(lineStarts, position);
414+
return {
415+
line: lineNumber,
416+
character: position - lineStarts[lineNumber]
417+
};
418+
}
419+
412420
/**
421+
* @internal
413422
* We assume the first line starts at position 0 and 'position' is non-negative.
414423
*/
415-
export function computeLineAndCharacterOfPosition(lineStarts: readonly number[], position: number): LineAndCharacter {
416-
let lineNumber = binarySearch(lineStarts, position, identity, compareValues);
424+
export function computeLineOfPosition(lineStarts: readonly number[], position: number, lowerBound?: number) {
425+
let lineNumber = binarySearch(lineStarts, position, identity, compareValues, lowerBound);
417426
if (lineNumber < 0) {
418427
// If the actual position was not found,
419428
// the binary search returns the 2's-complement of the next line start
@@ -425,10 +434,19 @@ namespace ts {
425434
lineNumber = ~lineNumber - 1;
426435
Debug.assert(lineNumber !== -1, "position cannot precede the beginning of the file");
427436
}
428-
return {
429-
line: lineNumber,
430-
character: position - lineStarts[lineNumber]
431-
};
437+
return lineNumber;
438+
}
439+
440+
/** @internal */
441+
export function getLinesBetweenPositions(sourceFile: SourceFileLike, pos1: number, pos2: number) {
442+
if (pos1 === pos2) return 0;
443+
const lineStarts = getLineStarts(sourceFile);
444+
const lower = Math.min(pos1, pos2);
445+
const isNegative = lower === pos2;
446+
const upper = isNegative ? pos1 : pos2;
447+
const lowerLine = computeLineOfPosition(lineStarts, lower);
448+
const upperLine = computeLineOfPosition(lineStarts, upper, lowerLine);
449+
return isNegative ? lowerLine - upperLine : upperLine - lowerLine;
432450
}
433451

434452
export function getLineAndCharacterOfPosition(sourceFile: SourceFileLike, position: number): LineAndCharacter {

‎src/compiler/types.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3784,7 +3784,7 @@ namespace ts {
37843784
writeParameter(text: string): void;
37853785
writeProperty(text: string): void;
37863786
writeSymbol(text: string, symbol: Symbol): void;
3787-
writeLine(): void;
3787+
writeLine(force?: boolean): void;
37883788
increaseIndent(): void;
37893789
decreaseIndent(): void;
37903790
clear(): void;
@@ -5860,6 +5860,7 @@ namespace ts {
58605860
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.
58615861
/*@internal*/ TypeScriptClassWrapper = 1 << 25, // The node is an IIFE class wrapper created by the ts transform.
58625862
/*@internal*/ NeverApplyImportHelper = 1 << 26, // Indicates the node should never be wrapped with an import star helper (because, for example, it imports tslib itself)
5863+
/*@internal*/ IgnoreSourceNewlines = 1 << 27, // Overrides `printerOptions.preserveSourceNewlines` to print this node (and all descendants) with default whitespace.
58635864
}
58645865

58655866
export interface EmitHelper {
@@ -6324,6 +6325,7 @@ namespace ts {
63246325
/*@internal*/ writeBundleFileInfo?: boolean;
63256326
/*@internal*/ recordInternalSection?: boolean;
63266327
/*@internal*/ stripInternal?: boolean;
6328+
/*@internal*/ preserveSourceNewlines?: boolean;
63276329
/*@internal*/ relativeToBuildInfo?: (path: string) => string;
63286330
}
63296331

‎src/compiler/utilities.ts

Lines changed: 43 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3633,8 +3633,8 @@ namespace ts {
36333633
}
36343634
}
36353635

3636-
function writeLine() {
3637-
if (!lineStart) {
3636+
function writeLine(force?: boolean) {
3637+
if (!lineStart || force) {
36383638
output += newLine;
36393639
lineCount++;
36403640
linePos = output.length;
@@ -3913,12 +3913,13 @@ namespace ts {
39133913
}
39143914
}
39153915

3916-
export function getLineOfLocalPosition(currentSourceFile: SourceFile, pos: number) {
3917-
return getLineAndCharacterOfPosition(currentSourceFile, pos).line;
3916+
export function getLineOfLocalPosition(sourceFile: SourceFile, pos: number) {
3917+
const lineStarts = getLineStarts(sourceFile);
3918+
return computeLineOfPosition(lineStarts, pos);
39183919
}
39193920

39203921
export function getLineOfLocalPositionFromLineMap(lineMap: readonly number[], pos: number) {
3921-
return computeLineAndCharacterOfPosition(lineMap, pos).line;
3922+
return computeLineOfPosition(lineMap, pos);
39223923
}
39233924

39243925
export function getFirstConstructorWithBody(node: ClassLikeDeclaration): ConstructorDeclaration & { body: FunctionBody } | undefined {
@@ -4743,32 +4744,62 @@ namespace ts {
47434744
}
47444745

47454746
export function rangeStartPositionsAreOnSameLine(range1: TextRange, range2: TextRange, sourceFile: SourceFile) {
4746-
return positionsAreOnSameLine(getStartPositionOfRange(range1, sourceFile), getStartPositionOfRange(range2, sourceFile), sourceFile);
4747+
return positionsAreOnSameLine(
4748+
getStartPositionOfRange(range1, sourceFile, /*includeComments*/ false),
4749+
getStartPositionOfRange(range2, sourceFile, /*includeComments*/ false),
4750+
sourceFile);
47474751
}
47484752

47494753
export function rangeEndPositionsAreOnSameLine(range1: TextRange, range2: TextRange, sourceFile: SourceFile) {
47504754
return positionsAreOnSameLine(range1.end, range2.end, sourceFile);
47514755
}
47524756

47534757
export function rangeStartIsOnSameLineAsRangeEnd(range1: TextRange, range2: TextRange, sourceFile: SourceFile) {
4754-
return positionsAreOnSameLine(getStartPositionOfRange(range1, sourceFile), range2.end, sourceFile);
4758+
return positionsAreOnSameLine(getStartPositionOfRange(range1, sourceFile, /*includeComments*/ false), range2.end, sourceFile);
47554759
}
47564760

47574761
export function rangeEndIsOnSameLineAsRangeStart(range1: TextRange, range2: TextRange, sourceFile: SourceFile) {
4758-
return positionsAreOnSameLine(range1.end, getStartPositionOfRange(range2, sourceFile), sourceFile);
4762+
return positionsAreOnSameLine(range1.end, getStartPositionOfRange(range2, sourceFile, /*includeComments*/ false), sourceFile);
4763+
}
4764+
4765+
export function getLinesBetweenRangeEndAndRangeStart(range1: TextRange, range2: TextRange, sourceFile: SourceFile, includeSecondRangeComments: boolean) {
4766+
const range2Start = getStartPositionOfRange(range2, sourceFile, includeSecondRangeComments);
4767+
return getLinesBetweenPositions(sourceFile, range1.end, range2Start);
4768+
}
4769+
4770+
export function getLinesBetweenRangeEndPositions(range1: TextRange, range2: TextRange, sourceFile: SourceFile) {
4771+
return getLinesBetweenPositions(sourceFile, range1.end, range2.end);
47594772
}
47604773

47614774
export function isNodeArrayMultiLine(list: NodeArray<Node>, sourceFile: SourceFile): boolean {
47624775
return !positionsAreOnSameLine(list.pos, list.end, sourceFile);
47634776
}
47644777

47654778
export function positionsAreOnSameLine(pos1: number, pos2: number, sourceFile: SourceFile) {
4766-
return pos1 === pos2 ||
4767-
getLineOfLocalPosition(sourceFile, pos1) === getLineOfLocalPosition(sourceFile, pos2);
4779+
return getLinesBetweenPositions(sourceFile, pos1, pos2) === 0;
4780+
}
4781+
4782+
export function getStartPositionOfRange(range: TextRange, sourceFile: SourceFile, includeComments: boolean) {
4783+
return positionIsSynthesized(range.pos) ? -1 : skipTrivia(sourceFile.text, range.pos, /*stopAfterLineBreak*/ false, includeComments);
47684784
}
47694785

4770-
export function getStartPositionOfRange(range: TextRange, sourceFile: SourceFile) {
4771-
return positionIsSynthesized(range.pos) ? -1 : skipTrivia(sourceFile.text, range.pos);
4786+
export function getLinesBetweenPositionAndPrecedingNonWhitespaceCharacter(pos: number, sourceFile: SourceFile, includeComments?: boolean) {
4787+
const startPos = skipTrivia(sourceFile.text, pos, /*stopAfterLineBreak*/ false, includeComments);
4788+
const prevPos = getPreviousNonWhitespacePosition(startPos, sourceFile);
4789+
return getLinesBetweenPositions(sourceFile, prevPos || 0, startPos);
4790+
}
4791+
4792+
export function getLinesBetweenPositionAndNextNonWhitespaceCharacter(pos: number, sourceFile: SourceFile, includeComments?: boolean) {
4793+
const nextPos = skipTrivia(sourceFile.text, pos, /*stopAfterLineBreak*/ false, includeComments);
4794+
return getLinesBetweenPositions(sourceFile, pos, nextPos);
4795+
}
4796+
4797+
function getPreviousNonWhitespacePosition(pos: number, sourceFile: SourceFile) {
4798+
while (pos-- > 0) {
4799+
if (!isWhiteSpaceLike(sourceFile.text.charCodeAt(pos))) {
4800+
return pos;
4801+
}
4802+
}
47724803
}
47734804

47744805
/**

‎src/harness/fourslashImpl.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3306,7 +3306,7 @@ namespace FourSlash {
33063306
}
33073307

33083308
public moveToNewFile(options: FourSlashInterface.MoveToNewFileOptions): void {
3309-
assert(this.getRanges().length === 1);
3309+
assert(this.getRanges().length === 1, "Must have exactly one fourslash range (source enclosed between '[|' and '|]' delimiters) in the source file");
33103310
const range = this.getRanges()[0];
33113311
const refactor = ts.find(this.getApplicableRefactors(range, { allowTextChangesInNewFiles: true }), r => r.name === "Move to a new file")!;
33123312
assert(refactor.actions.length === 1);

‎src/services/formatting/formatting.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ namespace ts.formatting {
7070
* Formatter calls this function when rule adds or deletes new lines from the text
7171
* so indentation scope can adjust values of indentation and delta.
7272
*/
73-
recomputeIndentation(lineAddedByFormatting: boolean): void;
73+
recomputeIndentation(lineAddedByFormatting: boolean, parent: Node): void;
7474
}
7575

7676
export function formatOnEnter(position: number, sourceFile: SourceFile, formatContext: FormatContext): TextChange[] {
@@ -567,8 +567,8 @@ namespace ts.formatting {
567567
!suppressDelta && shouldAddDelta(line, kind, container) ? indentation + getDelta(container) : indentation,
568568
getIndentation: () => indentation,
569569
getDelta,
570-
recomputeIndentation: lineAdded => {
571-
if (node.parent && SmartIndenter.shouldIndentChildNode(options, node.parent, node, sourceFile)) {
570+
recomputeIndentation: (lineAdded, parent) => {
571+
if (SmartIndenter.shouldIndentChildNode(options, parent, node, sourceFile)) {
572572
indentation += lineAdded ? options.indentSize! : -options.indentSize!;
573573
delta = SmartIndenter.shouldIndentChildNode(options, node) ? options.indentSize! : 0;
574574
}
@@ -996,15 +996,15 @@ namespace ts.formatting {
996996
// Handle the case where the next line is moved to be the end of this line.
997997
// In this case we don't indent the next line in the next pass.
998998
if (currentParent.getStart(sourceFile) === currentItem.pos) {
999-
dynamicIndentation.recomputeIndentation(/*lineAddedByFormatting*/ false);
999+
dynamicIndentation.recomputeIndentation(/*lineAddedByFormatting*/ false, contextNode);
10001000
}
10011001
break;
10021002
case LineAction.LineAdded:
10031003
// Handle the case where token2 is moved to the new line.
10041004
// In this case we indent token2 in the next pass but we set
10051005
// sameLineIndent flag to notify the indenter that the indentation is within the line.
10061006
if (currentParent.getStart(sourceFile) === currentItem.pos) {
1007-
dynamicIndentation.recomputeIndentation(/*lineAddedByFormatting*/ true);
1007+
dynamicIndentation.recomputeIndentation(/*lineAddedByFormatting*/ true, contextNode);
10081008
}
10091009
break;
10101010
default:

‎src/services/refactors/extractType.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,7 @@ namespace ts.refactor {
159159
typeParameters.map(id => updateTypeParameterDeclaration(id, id.name, id.constraint, /* defaultType */ undefined)),
160160
selection
161161
);
162-
changes.insertNodeBefore(file, firstStatement, newTypeNode, /* blankLineBetween */ true);
162+
changes.insertNodeBefore(file, firstStatement, ignoreSourceNewlines(newTypeNode), /* blankLineBetween */ true);
163163
changes.replaceNode(file, selection, createTypeReferenceNode(name, typeParameters.map(id => createTypeReferenceNode(id.name, /* typeArguments */ undefined))));
164164
}
165165

@@ -174,7 +174,7 @@ namespace ts.refactor {
174174
/* heritageClauses */ undefined,
175175
typeElements
176176
);
177-
changes.insertNodeBefore(file, firstStatement, newTypeNode, /* blankLineBetween */ true);
177+
changes.insertNodeBefore(file, firstStatement, ignoreSourceNewlines(newTypeNode), /* blankLineBetween */ true);
178178
changes.replaceNode(file, selection, createTypeReferenceNode(name, typeParameters.map(id => createTypeReferenceNode(id.name, /* typeArguments */ undefined))));
179179
}
180180

‎src/services/textChanges.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -935,7 +935,7 @@ namespace ts.textChanges {
935935
export function getNonformattedText(node: Node, sourceFile: SourceFile | undefined, newLineCharacter: string): { text: string, node: Node } {
936936
const writer = createWriter(newLineCharacter);
937937
const newLine = newLineCharacter === "\n" ? NewLineKind.LineFeed : NewLineKind.CarriageReturnLineFeed;
938-
createPrinter({ newLine, neverAsciiEscape: true }, writer).writeNode(EmitHint.Unspecified, node, sourceFile, writer);
938+
createPrinter({ newLine, neverAsciiEscape: true, preserveSourceNewlines: true }, writer).writeNode(EmitHint.Unspecified, node, sourceFile, writer);
939939
return { text: writer.getText(), node: assignPositionsToNode(node) };
940940
}
941941
}
@@ -1064,8 +1064,8 @@ namespace ts.textChanges {
10641064
writer.writeSymbol(s, sym);
10651065
setLastNonTriviaPosition(s, /*force*/ false);
10661066
}
1067-
function writeLine(): void {
1068-
writer.writeLine();
1067+
function writeLine(force?: boolean): void {
1068+
writer.writeLine(force);
10691069
}
10701070
function increaseIndent(): void {
10711071
writer.increaseIndent();

‎tests/baselines/reference/objectLiteralShorthandPropertiesErrorFromNotUsingIdentifier.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,8 @@ var y = {
3535
"typeof":
3636
};
3737
var x = (_a = {
38-
a: a, : .b,
38+
a: a,
39+
: .b,
3940
a: a
4041
},
4142
_a["ss"] = ,

‎tests/baselines/reference/objectLiteralShorthandPropertiesErrorWithModule.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,8 @@ var n;
2525
(function (n) {
2626
var z = 10000;
2727
n.y = {
28-
m: m, : .x // error
28+
m: m,
29+
: .x // error
2930
};
3031
})(n || (n = {}));
3132
m.y.x;

‎tests/baselines/reference/objectTypesWithOptionalProperties2.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,5 +42,6 @@ var C2 = /** @class */ (function () {
4242
return C2;
4343
}());
4444
var b = {
45-
x: function () { }, 1: // error
45+
x: function () { },
46+
1: // error
4647
};

‎tests/baselines/reference/parserErrorRecovery_ObjectLiteral2.js

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,5 +3,4 @@ var v = { a
33
return;
44

55
//// [parserErrorRecovery_ObjectLiteral2.js]
6-
var v = { a: a,
7-
"return": };
6+
var v = { a: a, "return": };

‎tests/cases/fourslash/extract-const-callback-function-this3.ts

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,6 @@ edit.applyRefactor({
1010
actionDescription: "Extract to constant in enclosing scope",
1111
newContent:
1212
`declare function fWithThis(fn: (this: { a: string }, a: string) => string): void;
13-
const newLocal = function(this: {
14-
a: string;
15-
}, a: string): string { return this.a; };
13+
const newLocal = function(this: { a: string; }, a: string): string { return this.a; };
1614
fWithThis(/*RENAME*/newLocal);`
1715
});

‎tests/cases/fourslash/moveToNewFile_declarationKinds.ts

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -24,16 +24,11 @@ type U = T; type V = I;`,
2424
"/x.ts":
2525
`export const x = 0;
2626
export function f() { }
27-
export class C {
28-
}
29-
export enum E {
30-
}
31-
export namespace N {
32-
export const x = 0;
33-
}
27+
export class C { }
28+
export enum E { }
29+
export namespace N { export const x = 0; }
3430
export type T = number;
35-
export interface I {
36-
}
31+
export interface I { }
3732
`,
3833
},
3934
});

‎tests/cases/fourslash/refactorConvertToEs6Module_export_object.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,5 @@ verify.codeFix({
2222
export function f() { }
2323
export function g() { }
2424
export function h() { }
25-
export class C {
26-
}`,
25+
export class C { }`,
2726
});

‎tests/cases/fourslash/refactorConvertToEs6Module_expressionToDeclaration.ts

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,6 @@ verify.codeFix({
1515
`var C = {};
1616
console.log(C);
1717
export async function* f(p) { p; }
18-
const _C = class C extends D {
19-
m() { }
20-
};
18+
const _C = class C extends D { m() { } };
2119
export { _C as C };`,
2220
});

‎tests/cases/fourslash/textChangesPreserveNewlines1.ts

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
/// <reference path="fourslash.ts" />
2+
3+
//// /*1*/console.log(1);
4+
////
5+
//// console.log(2);
6+
////
7+
//// console.log(3);/*2*/
8+
9+
goTo.select("1", "2");
10+
edit.applyRefactor({
11+
refactorName: "Extract Symbol",
12+
actionName: "function_scope_0",
13+
actionDescription: "Extract to function in global scope",
14+
newContent:
15+
`/*RENAME*/newFunction();
16+
17+
function newFunction() {
18+
console.log(1);
19+
20+
console.log(2);
21+
22+
console.log(3);
23+
}
24+
`
25+
});

0 commit comments

Comments
 (0)