Skip to content

Commit 7d4fc73

Browse files
authored
Fix preserveNewlines printer option when a list child has the same start or end as its parent (microsoft#37846)
* Fix preserveNewlines printer option when a list child has the same start or end as its parent * Fix leading line separator calculation and JSX bug
1 parent d571a09 commit 7d4fc73

File tree

3 files changed

+59
-16
lines changed

3 files changed

+59
-16
lines changed

src/compiler/emitter.ts

Lines changed: 23 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2366,16 +2366,10 @@ namespace ts {
23662366

23672367
function emitParenthesizedExpression(node: ParenthesizedExpression) {
23682368
const openParenPos = emitTokenWithComment(SyntaxKind.OpenParenToken, node.pos, writePunctuation, node);
2369-
const leadingNewlines = preserveSourceNewlines && getLeadingLineTerminatorCount(node, [node.expression], ListFormat.None);
2370-
if (leadingNewlines) {
2371-
writeLinesAndIndent(leadingNewlines, /*writeLinesIfNotIndenting*/ false);
2372-
}
2369+
const indented = writeLineSeparatorsAndIndentBefore(node.expression, node);
23732370
emitExpression(node.expression);
2374-
const trailingNewlines = preserveSourceNewlines && getClosingLineTerminatorCount(node, [node.expression], ListFormat.None);
2375-
if (trailingNewlines) {
2376-
writeLine(trailingNewlines);
2377-
}
2378-
decreaseIndentIf(leadingNewlines);
2371+
writeLineSeparatorsAfter(node.expression, node);
2372+
decreaseIndentIf(indented);
23792373
emitTokenWithComment(SyntaxKind.CloseParenToken, node.expression ? node.expression.end : openParenPos, writePunctuation, node);
23802374
}
23812375

@@ -3293,12 +3287,15 @@ namespace ts {
32933287
writePunctuation("<");
32943288

32953289
if (isJsxOpeningElement(node)) {
3290+
const indented = writeLineSeparatorsAndIndentBefore(node.tagName, node);
32963291
emitJsxTagName(node.tagName);
32973292
emitTypeArguments(node, node.typeArguments);
32983293
if (node.attributes.properties && node.attributes.properties.length > 0) {
32993294
writeSpace();
33003295
}
33013296
emit(node.attributes);
3297+
writeLineSeparatorsAfter(node.attributes, node);
3298+
decreaseIndentIf(indented);
33023299
}
33033300

33043301
writePunctuation(">");
@@ -4302,6 +4299,7 @@ namespace ts {
43024299
return getEffectiveLines(
43034300
includeComments => getLinesBetweenPositionAndPrecedingNonWhitespaceCharacter(
43044301
firstChild.pos,
4302+
parentNode.pos,
43054303
currentSourceFile!,
43064304
includeComments));
43074305
}
@@ -4359,6 +4357,7 @@ namespace ts {
43594357
return getEffectiveLines(
43604358
includeComments => getLinesBetweenPositionAndNextNonWhitespaceCharacter(
43614359
lastChild.end,
4360+
parentNode.end,
43624361
currentSourceFile!,
43634362
includeComments));
43644363
}
@@ -4398,6 +4397,21 @@ namespace ts {
43984397
return lines;
43994398
}
44004399

4400+
function writeLineSeparatorsAndIndentBefore(node: Node, parent: Node): boolean {
4401+
const leadingNewlines = preserveSourceNewlines && getLeadingLineTerminatorCount(parent, [node], ListFormat.None);
4402+
if (leadingNewlines) {
4403+
writeLinesAndIndent(leadingNewlines, /*writeLinesIfNotIndenting*/ false);
4404+
}
4405+
return !!leadingNewlines;
4406+
}
4407+
4408+
function writeLineSeparatorsAfter(node: Node, parent: Node) {
4409+
const trailingNewlines = preserveSourceNewlines && getClosingLineTerminatorCount(parent, [node], ListFormat.None);
4410+
if (trailingNewlines) {
4411+
writeLine(trailingNewlines);
4412+
}
4413+
}
4414+
44014415
function synthesizedNodeStartsOnNewLine(node: Node, format: ListFormat) {
44024416
if (nodeIsSynthesized(node)) {
44034417
const startsOnNewLine = getStartsOnNewLine(node);

src/compiler/utilities.ts

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4780,19 +4780,19 @@ namespace ts {
47804780
return positionIsSynthesized(range.pos) ? -1 : skipTrivia(sourceFile.text, range.pos, /*stopAfterLineBreak*/ false, includeComments);
47814781
}
47824782

4783-
export function getLinesBetweenPositionAndPrecedingNonWhitespaceCharacter(pos: number, sourceFile: SourceFile, includeComments?: boolean) {
4783+
export function getLinesBetweenPositionAndPrecedingNonWhitespaceCharacter(pos: number, stopPos: number, sourceFile: SourceFile, includeComments?: boolean) {
47844784
const startPos = skipTrivia(sourceFile.text, pos, /*stopAfterLineBreak*/ false, includeComments);
4785-
const prevPos = getPreviousNonWhitespacePosition(startPos, sourceFile);
4786-
return getLinesBetweenPositions(sourceFile, prevPos || 0, startPos);
4785+
const prevPos = getPreviousNonWhitespacePosition(startPos, stopPos, sourceFile);
4786+
return getLinesBetweenPositions(sourceFile, prevPos ?? stopPos, startPos);
47874787
}
47884788

4789-
export function getLinesBetweenPositionAndNextNonWhitespaceCharacter(pos: number, sourceFile: SourceFile, includeComments?: boolean) {
4789+
export function getLinesBetweenPositionAndNextNonWhitespaceCharacter(pos: number, stopPos: number, sourceFile: SourceFile, includeComments?: boolean) {
47904790
const nextPos = skipTrivia(sourceFile.text, pos, /*stopAfterLineBreak*/ false, includeComments);
4791-
return getLinesBetweenPositions(sourceFile, pos, nextPos);
4791+
return getLinesBetweenPositions(sourceFile, pos, Math.min(stopPos, nextPos));
47924792
}
47934793

4794-
function getPreviousNonWhitespacePosition(pos: number, sourceFile: SourceFile) {
4795-
while (pos-- > 0) {
4794+
function getPreviousNonWhitespacePosition(pos: number, stopPos = 0, sourceFile: SourceFile) {
4795+
while (pos-- > stopPos) {
47964796
if (!isWhiteSpaceLike(sourceFile.text.charCodeAt(pos))) {
47974797
return pos;
47984798
}
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
// #37813
2+
3+
/// <reference path="fourslash.ts" />
4+
5+
////function foo() {
6+
//// /*1*/var x: number
7+
////
8+
//// x = 10;
9+
//// return x;/*2*/
10+
////}
11+
12+
goTo.select("1", "2");
13+
edit.applyRefactor({
14+
refactorName: "Extract Symbol",
15+
actionName: "function_scope_1",
16+
actionDescription: "Extract to function in global scope",
17+
newContent:
18+
`function foo() {
19+
return /*RENAME*/newFunction();
20+
}
21+
22+
function newFunction() {
23+
var x: number;
24+
25+
x = 10;
26+
return x;
27+
}
28+
`
29+
});

0 commit comments

Comments
 (0)