From 6f81426c5d625db9b12ddd1c0983eeee034c5eac Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Thu, 20 May 2021 21:20:25 +0000 Subject: [PATCH] Cherry-pick PR #44070 into release-4.3 Component commits: d1920805b0 Do not incorrectly add line separators for non-synthetic nodes when emitting node list As of 3c32f6e154ead6749b76ec9c19cbfdd2acad97d6, 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. --- src/compiler/emitter.ts | 41 ++++++++++++------- src/testRunner/unittests/transform.ts | 20 +++++++++ .../decoratorOnClassMethodThisParameter.js | 3 +- ...rmNestedSelfClosingChild(jsx=react-jsx).js | 3 +- ...estedSelfClosingChild(jsx=react-jsxdev).js | 3 +- ...ndPropertiesErrorFromNotUsingIdentifier.js | 3 +- ...teralShorthandPropertiesErrorWithModule.js | 3 +- .../objectTypesWithOptionalProperties2.js | 3 +- .../parserErrorRecovery_ObjectLiteral2.js | 3 +- .../transformsCorrectly.issue44068.js | 3 ++ 10 files changed, 61 insertions(+), 24 deletions(-) create mode 100644 tests/baselines/reference/transformApi/transformsCorrectly.issue44068.js diff --git a/src/compiler/emitter.ts b/src/compiler/emitter.ts index a3007559f0dd9..11ef21f1be751 100644 --- a/src/compiler/emitter.ts +++ b/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; } diff --git a/src/testRunner/unittests/transform.ts b/src/testRunner/unittests/transform.ts index 470366d39098d..13de003fd31ca 100644 --- a/src/testRunner/unittests/transform.ts +++ b/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: { diff --git a/tests/baselines/reference/decoratorOnClassMethodThisParameter.js b/tests/baselines/reference/decoratorOnClassMethodThisParameter.js index 5feeb0ef90e36..0938ccbc4438b 100644 --- a/tests/baselines/reference/decoratorOnClassMethodThisParameter.js +++ b/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; }()); diff --git a/tests/baselines/reference/jsxJsxsCjsTransformNestedSelfClosingChild(jsx=react-jsx).js b/tests/baselines/reference/jsxJsxsCjsTransformNestedSelfClosingChild(jsx=react-jsx).js index b7d0e0d6a9493..db697c30671d8 100644 --- a/tests/baselines/reference/jsxJsxsCjsTransformNestedSelfClosingChild(jsx=react-jsx).js +++ b/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)); diff --git a/tests/baselines/reference/jsxJsxsCjsTransformNestedSelfClosingChild(jsx=react-jsxdev).js b/tests/baselines/reference/jsxJsxsCjsTransformNestedSelfClosingChild(jsx=react-jsxdev).js index 6ddbbb35817ab..670a68b264336 100644 --- a/tests/baselines/reference/jsxJsxsCjsTransformNestedSelfClosingChild(jsx=react-jsxdev).js +++ b/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)); 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/baselines/reference/transformApi/transformsCorrectly.issue44068.js b/tests/baselines/reference/transformApi/transformsCorrectly.issue44068.js new file mode 100644 index 0000000000000..493a92a6c8c40 --- /dev/null +++ b/tests/baselines/reference/transformApi/transformsCorrectly.issue44068.js @@ -0,0 +1,3 @@ +const FirstVar = null; +const SecondVar = null; +[FirstVar, FirstVar];