Do not incorrectly add line separators for non-synthetic nodes when emitting node list (#44070)

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.

Author: devversion

src/compiler/emitter.ts

@@ -4564,16 +4564,26 @@ namespace ts {
  // JsxText will be written with its leading whitespace, so don't add more manually.
  return 0;
  }
- else if (preserveSourceNewlines && siblingNodePositionsAreComparable(previousNode, nextNode)) {
- return getEffectiveLines(
- includeComments => getLinesBetweenRangeEndAndRangeStart(
- previousNode,
- nextNode,
- currentSourceFile!,
- includeComments));
- }
- else if (!preserveSourceNewlines && !nodeIsSynthesized(previousNode) && !nodeIsSynthesized(nextNode)) {
- return rangeEndIsOnSameLineAsRangeStart(previousNode, nextNode, currentSourceFile!) ? 0 : 1;
+ else if (!nodeIsSynthesized(previousNode) && !nodeIsSynthesized(nextNode)) {
+ if (preserveSourceNewlines && siblingNodePositionsAreComparable(previousNode, nextNode)) {
+ return getEffectiveLines(
+ includeComments => getLinesBetweenRangeEndAndRangeStart(
+ previousNode,
+ nextNode,
+ currentSourceFile!,
+ includeComments));
+ }
+ // If `preserveSourceNewlines` is `false` we do not intend to preserve the effective lines between the
+ // previous and next node. Instead we naively check whether nodes are on separate lines within the
+ // same node parent. If so, we intend to preserve a single line terminator. This is less precise and
+ // expensive than checking with `preserveSourceNewlines` as above, but the goal is not to preserve the
+ // effective source lines between two sibling nodes.
+ else if (!preserveSourceNewlines && originalNodesHaveSameParent(previousNode, nextNode)) {
+ return rangeEndIsOnSameLineAsRangeStart(previousNode, nextNode, currentSourceFile!) ? 0 : 1;
+ }
+ // If the two nodes are not comparable, add a line terminator based on the format that can indicate
+ // whether new lines are preferred or not.
+ return format & ListFormat.PreferNewLine ? 1 : 0;
  }
  else if (synthesizedNodeStartsOnNewLine(previousNode, format) || synthesizedNodeStartsOnNewLine(nextNode, format)) {
  return 1;
@@ -5300,11 +5310,14 @@ namespace ts {
 
  }
 
- function siblingNodePositionsAreComparable(previousNode: Node, nextNode: Node) {
- if (nodeIsSynthesized(previousNode) || nodeIsSynthesized(nextNode)) {
- return false;
- }
+ function originalNodesHaveSameParent(nodeA: Node, nodeB: Node) {
+ nodeA = getOriginalNode(nodeA);
+ // For performance, do not call `getOriginalNode` for `nodeB` if `nodeA` doesn't even
+ // have a parent node.
+ return nodeA.parent && nodeA.parent === getOriginalNode(nodeB).parent;
+ }
 
+ function siblingNodePositionsAreComparable(previousNode: Node, nextNode: Node) {
  if (nextNode.pos < previousNode.end) {
  return false;
  }

src/testRunner/unittests/transform.ts

@@ -154,6 +154,26 @@ namespace ts {
  }).outputText;
  });
 
+ testBaseline("issue44068", () => {
+ return transformSourceFile(`
+ const FirstVar = null;
+ const SecondVar = null;
+ `, [
+ context => file => {
+ const firstVarName = (file.statements[0] as VariableStatement)
+ .declarationList.declarations[0].name as Identifier;
+ const secondVarName = (file.statements[0] as VariableStatement)
+ .declarationList.declarations[0].name as Identifier;
+
+ return context.factory.updateSourceFile(file, file.statements.concat([
+ context.factory.createExpressionStatement(
+ context.factory.createArrayLiteralExpression([firstVarName, secondVarName])
+ ),
+ ]));
+ }
+ ]);
+ });
+
  testBaseline("rewrittenNamespace", () => {
  return transpileModule(`namespace Reflect { const x = 1; }`, {
  transformers: {

tests/baselines/reference/decoratorOnClassMethodThisParameter.js

@@ -30,7 +30,8 @@ var C2 = /** @class */ (function () {
  }
  C2.prototype.method = function (allowed) { };
  __decorate([
- __param(0, dec), __param(1, dec)
+ __param(0, dec),
+ __param(1, dec)
  ], C2.prototype, "method", null);
  return C2;
 }());

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

@@ -26,6 +26,5 @@ console.log(
 exports.__esModule = true;
 var jsx_runtime_1 = require("react/jsx-runtime");
 console.log(jsx_runtime_1.jsx("div", { children: jsx_runtime_1.jsx("div", {}, void 0) }, void 0));
-console.log(jsx_runtime_1.jsxs("div", { children: [jsx_runtime_1.jsx("div", {}, void 0),
- jsx_runtime_1.jsx("div", {}, void 0)] }, void 0));
+console.log(jsx_runtime_1.jsxs("div", { children: [jsx_runtime_1.jsx("div", {}, void 0), jsx_runtime_1.jsx("div", {}, void 0)] }, void 0));
 console.log(jsx_runtime_1.jsx("div", { children: [1, 2].map(function (i) { return jsx_runtime_1.jsx("div", { children: i }, i); }) }, void 0));

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

@@ -28,6 +28,5 @@ exports.__esModule = true;
 var jsx_dev_runtime_1 = require("react/jsx-dev-runtime");
 var _jsxFileName = "tests/cases/conformance/jsx/jsxs/jsxJsxsCjsTransformNestedSelfClosingChild.tsx";
 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));
-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));
+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));
 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));

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

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;

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
 };

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": };

tests/baselines/reference/transformApi/transformsCorrectly.issue44068.js

@@ -0,0 +1,3 @@
+const FirstVar = null;
+const SecondVar = null;
+[FirstVar, FirstVar];