Skip to content

Commit 419f1e7

Browse files
TypeScript Botdevversion
TypeScript Bot
andauthored
Cherry-pick PR #44070 into release-4.3 (#44187)
Component commits: d192080 Do not incorrectly add line separators for non-synthetic nodes when emitting node list As of 3c32f6e, a line separator is added between nodes if the nodes are not synthetic and on separate lines. This it push s wrong and previously only happened if the non-synthetic nodes were on different lines but had the same parent. Fixes #44068. Co-authored-by: Paul Gschwendtner <[email protected]>
1 parent 60f6d7b commit 419f1e7

10 files changed

+61
-24
lines changed

Diff for: src/compiler/emitter.ts

+27-14
Original file line numberDiff line numberDiff line change
@@ -4564,16 +4564,26 @@ namespace ts {
45644564
// JsxText will be written with its leading whitespace, so don't add more manually.
45654565
return 0;
45664566
}
4567-
else if (preserveSourceNewlines && siblingNodePositionsAreComparable(previousNode, nextNode)) {
4568-
return getEffectiveLines(
4569-
includeComments => getLinesBetweenRangeEndAndRangeStart(
4570-
previousNode,
4571-
nextNode,
4572-
currentSourceFile!,
4573-
includeComments));
4574-
}
4575-
else if (!preserveSourceNewlines && !nodeIsSynthesized(previousNode) && !nodeIsSynthesized(nextNode)) {
4576-
return rangeEndIsOnSameLineAsRangeStart(previousNode, nextNode, currentSourceFile!) ? 0 : 1;
4567+
else if (!nodeIsSynthesized(previousNode) && !nodeIsSynthesized(nextNode)) {
4568+
if (preserveSourceNewlines && siblingNodePositionsAreComparable(previousNode, nextNode)) {
4569+
return getEffectiveLines(
4570+
includeComments => getLinesBetweenRangeEndAndRangeStart(
4571+
previousNode,
4572+
nextNode,
4573+
currentSourceFile!,
4574+
includeComments));
4575+
}
4576+
// If `preserveSourceNewlines` is `false` we do not intend to preserve the effective lines between the
4577+
// previous and next node. Instead we naively check whether nodes are on separate lines within the
4578+
// same node parent. If so, we intend to preserve a single line terminator. This is less precise and
4579+
// expensive than checking with `preserveSourceNewlines` as above, but the goal is not to preserve the
4580+
// effective source lines between two sibling nodes.
4581+
else if (!preserveSourceNewlines && originalNodesHaveSameParent(previousNode, nextNode)) {
4582+
return rangeEndIsOnSameLineAsRangeStart(previousNode, nextNode, currentSourceFile!) ? 0 : 1;
4583+
}
4584+
// If the two nodes are not comparable, add a line terminator based on the format that can indicate
4585+
// whether new lines are preferred or not.
4586+
return format & ListFormat.PreferNewLine ? 1 : 0;
45774587
}
45784588
else if (synthesizedNodeStartsOnNewLine(previousNode, format) || synthesizedNodeStartsOnNewLine(nextNode, format)) {
45794589
return 1;
@@ -5300,11 +5310,14 @@ namespace ts {
53005310

53015311
}
53025312

5303-
function siblingNodePositionsAreComparable(previousNode: Node, nextNode: Node) {
5304-
if (nodeIsSynthesized(previousNode) || nodeIsSynthesized(nextNode)) {
5305-
return false;
5306-
}
5313+
function originalNodesHaveSameParent(nodeA: Node, nodeB: Node) {
5314+
nodeA = getOriginalNode(nodeA);
5315+
// For performance, do not call `getOriginalNode` for `nodeB` if `nodeA` doesn't even
5316+
// have a parent node.
5317+
return nodeA.parent && nodeA.parent === getOriginalNode(nodeB).parent;
5318+
}
53075319

5320+
function siblingNodePositionsAreComparable(previousNode: Node, nextNode: Node) {
53085321
if (nextNode.pos < previousNode.end) {
53095322
return false;
53105323
}

Diff for: src/testRunner/unittests/transform.ts

+20
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,26 @@ namespace ts {
154154
}).outputText;
155155
});
156156

157+
testBaseline("issue44068", () => {
158+
return transformSourceFile(`
159+
const FirstVar = null;
160+
const SecondVar = null;
161+
`, [
162+
context => file => {
163+
const firstVarName = (file.statements[0] as VariableStatement)
164+
.declarationList.declarations[0].name as Identifier;
165+
const secondVarName = (file.statements[0] as VariableStatement)
166+
.declarationList.declarations[0].name as Identifier;
167+
168+
return context.factory.updateSourceFile(file, file.statements.concat([
169+
context.factory.createExpressionStatement(
170+
context.factory.createArrayLiteralExpression([firstVarName, secondVarName])
171+
),
172+
]));
173+
}
174+
]);
175+
});
176+
157177
testBaseline("rewrittenNamespace", () => {
158178
return transpileModule(`namespace Reflect { const x = 1; }`, {
159179
transformers: {

Diff for: tests/baselines/reference/decoratorOnClassMethodThisParameter.js

+2-1
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,8 @@ var C2 = /** @class */ (function () {
3030
}
3131
C2.prototype.method = function (allowed) { };
3232
__decorate([
33-
__param(0, dec), __param(1, dec)
33+
__param(0, dec),
34+
__param(1, dec)
3435
], C2.prototype, "method", null);
3536
return C2;
3637
}());

Diff for: tests/baselines/reference/jsxJsxsCjsTransformNestedSelfClosingChild(jsx=react-jsx).js

+1-2
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,5 @@ console.log(
2626
exports.__esModule = true;
2727
var jsx_runtime_1 = require("react/jsx-runtime");
2828
console.log(jsx_runtime_1.jsx("div", { children: jsx_runtime_1.jsx("div", {}, void 0) }, void 0));
29-
console.log(jsx_runtime_1.jsxs("div", { children: [jsx_runtime_1.jsx("div", {}, void 0),
30-
jsx_runtime_1.jsx("div", {}, void 0)] }, void 0));
29+
console.log(jsx_runtime_1.jsxs("div", { children: [jsx_runtime_1.jsx("div", {}, void 0), jsx_runtime_1.jsx("div", {}, void 0)] }, void 0));
3130
console.log(jsx_runtime_1.jsx("div", { children: [1, 2].map(function (i) { return jsx_runtime_1.jsx("div", { children: i }, i); }) }, void 0));

Diff for: tests/baselines/reference/jsxJsxsCjsTransformNestedSelfClosingChild(jsx=react-jsxdev).js

+1-2
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,5 @@ exports.__esModule = true;
2828
var jsx_dev_runtime_1 = require("react/jsx-dev-runtime");
2929
var _jsxFileName = "tests/cases/conformance/jsx/jsxs/jsxJsxsCjsTransformNestedSelfClosingChild.tsx";
3030
console.log(jsx_dev_runtime_1.jsxDEV("div", { children: jsx_dev_runtime_1.jsxDEV("div", {}, void 0, false, { fileName: _jsxFileName, lineNumber: 6, columnNumber: 5 }, this) }, void 0, false, { fileName: _jsxFileName, lineNumber: 4, columnNumber: 13 }, this));
31-
console.log(jsx_dev_runtime_1.jsxDEV("div", { children: [jsx_dev_runtime_1.jsxDEV("div", {}, void 0, false, { fileName: _jsxFileName, lineNumber: 12, columnNumber: 5 }, this),
32-
jsx_dev_runtime_1.jsxDEV("div", {}, void 0, false, { fileName: _jsxFileName, lineNumber: 13, columnNumber: 5 }, this)] }, void 0, true, { fileName: _jsxFileName, lineNumber: 10, columnNumber: 13 }, this));
31+
console.log(jsx_dev_runtime_1.jsxDEV("div", { children: [jsx_dev_runtime_1.jsxDEV("div", {}, void 0, false, { fileName: _jsxFileName, lineNumber: 12, columnNumber: 5 }, this), jsx_dev_runtime_1.jsxDEV("div", {}, void 0, false, { fileName: _jsxFileName, lineNumber: 13, columnNumber: 5 }, this)] }, void 0, true, { fileName: _jsxFileName, lineNumber: 10, columnNumber: 13 }, this));
3332
console.log(jsx_dev_runtime_1.jsxDEV("div", { children: [1, 2].map(function (i) { return jsx_dev_runtime_1.jsxDEV("div", { children: i }, i, false, { fileName: _jsxFileName, lineNumber: 19, columnNumber: 21 }, _this); }) }, void 0, false, { fileName: _jsxFileName, lineNumber: 17, columnNumber: 13 }, this));

Diff for: tests/baselines/reference/objectLiteralShorthandPropertiesErrorFromNotUsingIdentifier.js

+2-1
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"] = ,

Diff for: tests/baselines/reference/objectLiteralShorthandPropertiesErrorWithModule.js

+2-1
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;

Diff for: tests/baselines/reference/objectTypesWithOptionalProperties2.js

+2-1
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
};

Diff for: tests/baselines/reference/parserErrorRecovery_ObjectLiteral2.js

+1-2
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": };
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
const FirstVar = null;
2+
const SecondVar = null;
3+
[FirstVar, FirstVar];

0 commit comments

Comments
 (0)