Fix array spread with sideeffects (microsoft#41523)

* Fix array spread with sideeffects

* Minor cleanup, ensure multiple emit helpers for outfile tests

Author: rbuckton

src/compiler/checker.ts

@@ -24700,7 +24700,7 @@ namespace ts {
 
         function checkSpreadExpression(node: SpreadElement, checkMode?: CheckMode): Type {
             if (languageVersion < ScriptTarget.ES2015) {
-                checkExternalEmitHelpers(node, compilerOptions.downlevelIteration ? ExternalEmitHelpers.SpreadIncludes : ExternalEmitHelpers.SpreadArrays);
+                checkExternalEmitHelpers(node, compilerOptions.downlevelIteration ? ExternalEmitHelpers.SpreadIncludes : ExternalEmitHelpers.SpreadArray);
             }
 
             const arrayOrIterableType = checkExpression(node.expression, checkMode);
@@ -24728,7 +24728,7 @@ namespace ts {
                 const e = elements[i];
                 if (e.kind === SyntaxKind.SpreadElement) {
                     if (languageVersion < ScriptTarget.ES2015) {
-                        checkExternalEmitHelpers(e, compilerOptions.downlevelIteration ? ExternalEmitHelpers.SpreadIncludes : ExternalEmitHelpers.SpreadArrays);
+                        checkExternalEmitHelpers(e, compilerOptions.downlevelIteration ? ExternalEmitHelpers.SpreadIncludes : ExternalEmitHelpers.SpreadArray);
                     }
                     const spreadType = checkExpression((<SpreadElement>e).expression, checkMode, forceTuple);
                     if (isArrayLikeType(spreadType)) {
@@ -39053,8 +39053,7 @@ namespace ts {
                 case ExternalEmitHelpers.Generator: return "__generator";
                 case ExternalEmitHelpers.Values: return "__values";
                 case ExternalEmitHelpers.Read: return "__read";
-                case ExternalEmitHelpers.Spread: return "__spread";
-                case ExternalEmitHelpers.SpreadArrays: return "__spreadArrays";
+                case ExternalEmitHelpers.SpreadArray: return "__spreadArray";
                 case ExternalEmitHelpers.Await: return "__await";
                 case ExternalEmitHelpers.AsyncGenerator: return "__asyncGenerator";
                 case ExternalEmitHelpers.AsyncDelegator: return "__asyncDelegator";

src/compiler/factory/emitHelpers.ts

@@ -19,8 +19,7 @@ namespace ts {
         // ES2015 Helpers
         createExtendsHelper(name: Identifier): Expression;
         createTemplateObjectHelper(cooked: ArrayLiteralExpression, raw: ArrayLiteralExpression): Expression;
-        createSpreadHelper(argumentList: readonly Expression[]): Expression;
-        createSpreadArraysHelper(argumentList: readonly Expression[]): Expression;
+        createSpreadArrayHelper(to: Expression, from: Expression): Expression;
         // ES2015 Destructuring Helpers
         createValuesHelper(expression: Expression): Expression;
         createReadHelper(iteratorRecord: Expression, count: number | undefined): Expression;
@@ -58,8 +57,7 @@ namespace ts {
             // ES2015 Helpers
             createExtendsHelper,
             createTemplateObjectHelper,
-            createSpreadHelper,
-            createSpreadArraysHelper,
+            createSpreadArrayHelper,
             // ES2015 Destructuring Helpers
             createValuesHelper,
             createReadHelper,
@@ -284,22 +282,12 @@ namespace ts {
             );
         }
 
-        function createSpreadHelper(argumentList: readonly Expression[]) {
-            context.requestEmitHelper(readHelper);
-            context.requestEmitHelper(spreadHelper);
+        function createSpreadArrayHelper(to: Expression, from: Expression) {
+            context.requestEmitHelper(spreadArrayHelper);
             return factory.createCallExpression(
-                getUnscopedHelperName("__spread"),
+                getUnscopedHelperName("__spreadArray"),
                 /*typeArguments*/ undefined,
-                argumentList
-            );
-        }
-
-        function createSpreadArraysHelper(argumentList: readonly Expression[]) {
-            context.requestEmitHelper(spreadArraysHelper);
-            return factory.createCallExpression(
-                getUnscopedHelperName("__spreadArrays"),
-                /*typeArguments*/ undefined,
-                argumentList
+                [to, from]
             );
         }
 
@@ -629,29 +617,15 @@ namespace ts {
             };`
     };
 
-    export const spreadHelper: UnscopedEmitHelper = {
-        name: "typescript:spread",
-        importName: "__spread",
-        scoped: false,
-        dependencies: [readHelper],
-        text: `
-            var __spread = (this && this.__spread) || function () {
-                for (var ar = [], i = 0; i < arguments.length; i++) ar = ar.concat(__read(arguments[i]));
-                return ar;
-            };`
-    };
-
-    export const spreadArraysHelper: UnscopedEmitHelper = {
-        name: "typescript:spreadArrays",
-        importName: "__spreadArrays",
+    export const spreadArrayHelper: UnscopedEmitHelper = {
+        name: "typescript:spreadArray",
+        importName: "__spreadArray",
         scoped: false,
         text: `
-            var __spreadArrays = (this && this.__spreadArrays) || function () {
-                for (var s = 0, i = 0, il = arguments.length; i < il; i++) s += arguments[i].length;
-                for (var r = Array(s), k = 0, i = 0; i < il; i++)
-                    for (var a = arguments[i], j = 0, jl = a.length; j < jl; j++, k++)
-                        r[k] = a[j];
-                return r;
+            var __spreadArray = (this && this.__spreadArray) || function (to, from) {
+                for (var i = 0, il = from.length, j = to.length; i < il; i++, j++)
+                    to[j] = from[i];
+                return to;
             };`
     };
 
@@ -886,8 +860,7 @@ namespace ts {
             awaiterHelper,
             extendsHelper,
             templateObjectHelper,
-            spreadHelper,
-            spreadArraysHelper,
+            spreadArrayHelper,
             valuesHelper,
             readHelper,
             generatorHelper,
@@ -917,4 +890,11 @@ namespace ts {
                 return name => cache[name] || (cache[name] = { get value() { return geti(name); }, set value(v) { seti(name, v); } });
             })(name => super[name], (name, value) => super[name] = value);`
     };
+
+    export function isCallToHelper(firstSegment: Expression, helperName: __String) {
+        return isCallExpression(firstSegment)
+            && isIdentifier(firstSegment.expression)
+            && (getEmitFlags(firstSegment.expression) & EmitFlags.HelperName)
+            && firstSegment.expression.escapedText === helperName;
+    }
 }

src/compiler/transformers/es2015.ts

@@ -3892,17 +3892,41 @@ namespace ts {
          *
          * @param elements The array of Expression nodes.
          * @param needsUniqueCopy A value indicating whether to ensure that the result is a fresh array.
+         * This should be `true` when spreading into an `ArrayLiteral`, and `false` when spreading into an
+         * argument list.
          * @param multiLine A value indicating whether the result should be emitted on multiple lines.
          */
         function transformAndSpreadElements(elements: NodeArray<Expression>, needsUniqueCopy: boolean, multiLine: boolean, hasTrailingComma: boolean): Expression {
+            // When there is no leading SpreadElement:
+            //
             // [source]
             //      [a, ...b, c]
             //
             // [output (downlevelIteration)]
-            //      __spread([a], b, [c])
+            //      __spreadArray(__spreadArray([a], __read(b)), [c])
             //
             // [output]
-            //      __spreadArrays([a], b, [c])
+            //      __spreadArray(__spreadArray([a], b), [c])
+            //
+            // When there *is* a leading SpreadElement:
+            //
+            // [source]
+            //      [...a, b]
+            //
+            // [output (downlevelIteration)]
+            //      __spreadArray(__spreadArray([], __read(a)), [b])
+            //
+            // [output]
+            //      __spreadArray(__spreadArray([], a), [b])
+            //
+            // NOTE: We use `isPackedArrayLiteral` below rather than just `isArrayLiteral`
+            // because ES2015 spread will replace _missing_ array elements with `undefined`,
+            // so we cannot just use an array as is. For example:
+            //
+            // `[1, ...[2, , 3]]` becomes `[1, 2, undefined, 3]`
+            //
+            // However, for packed array literals (i.e., an array literal with no OmittedExpression
+            // elements), we can use the array as-is.
 
             // Map spans of spread expressions into their expressions and spans of other
             // expressions into an array literal.
@@ -3913,43 +3937,33 @@ namespace ts {
                 )
             );
 
-            if (compilerOptions.downlevelIteration) {
-                if (segments.length === 1) {
-                    const firstSegment = segments[0];
-                    if (isCallToHelper(firstSegment, "___spread" as __String)) {
-                        return segments[0];
-                    }
+            if (segments.length === 1) {
+                const firstSegment = segments[0];
+                // If we don't need a unique copy, then we are spreading into an argument list for
+                // a CallExpression or NewExpression. When using `--downlevelIteration`, we need
+                // to coerce this into an array for use with `apply`, so we will use the code path
+                // that follows instead.
+                if (!needsUniqueCopy && !compilerOptions.downlevelIteration
+                    || isPackedArrayLiteral(firstSegment) // see NOTE (above)
+                    || isCallToHelper(firstSegment, "___spreadArray" as __String)) {
+                    return segments[0];
                 }
-
-                return emitHelpers().createSpreadHelper(segments);
             }
-            else {
-                if (segments.length === 1) {
-                    const firstSegment = segments[0];
-                    if (!needsUniqueCopy
-                        || isPackedArrayLiteral(firstSegment)
-                        || isCallToHelper(firstSegment, "___spreadArrays" as __String)) {
-                        return segments[0];
-                    }
-                }
 
-                return emitHelpers().createSpreadArraysHelper(segments);
+            const helpers = emitHelpers();
+            const startsWithSpread = isSpreadElement(elements[0]);
+            let expression: Expression =
+                startsWithSpread ? factory.createArrayLiteralExpression() :
+                segments[0];
+            for (let i = startsWithSpread ? 0 : 1; i < segments.length; i++) {
+                expression = helpers.createSpreadArrayHelper(
+                    expression,
+                    compilerOptions.downlevelIteration && !isPackedArrayLiteral(segments[i]) ? // see NOTE (above)
+                        helpers.createReadHelper(segments[i], /*count*/ undefined) :
+                        segments[i]);
             }
-        }
 
-        function isPackedElement(node: Expression) {
-            return !isOmittedExpression(node);
-        }
-
-        function isPackedArrayLiteral(node: Expression) {
-            return isArrayLiteralExpression(node) && every(node.elements, isPackedElement);
-        }
-
-        function isCallToHelper(firstSegment: Expression, helperName: __String) {
-            return isCallExpression(firstSegment)
-                && isIdentifier(firstSegment.expression)
-                && (getEmitFlags(firstSegment.expression) & EmitFlags.HelperName)
-                && firstSegment.expression.escapedText === helperName;
+            return expression;
         }
 
         function partitionSpread(node: Expression) {

src/compiler/types.ts

@@ -6605,19 +6605,18 @@ namespace ts {
         Generator = 1 << 7,             // __generator (used by ES2015 generator transformation)
         Values = 1 << 8,                // __values (used by ES2015 for..of and yield* transformations)
         Read = 1 << 9,                  // __read (used by ES2015 iterator destructuring transformation)
-        Spread = 1 << 10,               // __spread (used by ES2015 array spread and argument list spread transformations)
-        SpreadArrays = 1 << 11,         // __spreadArrays (used by ES2015 array spread and argument list spread transformations)
-        Await = 1 << 12,                // __await (used by ES2017 async generator transformation)
-        AsyncGenerator = 1 << 13,       // __asyncGenerator (used by ES2017 async generator transformation)
-        AsyncDelegator = 1 << 14,       // __asyncDelegator (used by ES2017 async generator yield* transformation)
-        AsyncValues = 1 << 15,          // __asyncValues (used by ES2017 for..await..of transformation)
-        ExportStar = 1 << 16,           // __exportStar (used by CommonJS/AMD/UMD module transformation)
-        ImportStar = 1 << 17,           // __importStar (used by CommonJS/AMD/UMD module transformation)
-        ImportDefault = 1 << 18,        // __importStar (used by CommonJS/AMD/UMD module transformation)
-        MakeTemplateObject = 1 << 19,   // __makeTemplateObject (used for constructing template string array objects)
-        ClassPrivateFieldGet = 1 << 20, // __classPrivateFieldGet (used by the class private field transformation)
-        ClassPrivateFieldSet = 1 << 21, // __classPrivateFieldSet (used by the class private field transformation)
-        CreateBinding = 1 << 22,        // __createBinding (use by the module transform for (re)exports and namespace imports)
+        SpreadArray = 1 << 10,          // __spreadArray (used by ES2015 array spread and argument list spread transformations)
+        Await = 1 << 11,                // __await (used by ES2017 async generator transformation)
+        AsyncGenerator = 1 << 12,       // __asyncGenerator (used by ES2017 async generator transformation)
+        AsyncDelegator = 1 << 13,       // __asyncDelegator (used by ES2017 async generator yield* transformation)
+        AsyncValues = 1 << 14,          // __asyncValues (used by ES2017 for..await..of transformation)
+        ExportStar = 1 << 15,           // __exportStar (used by CommonJS/AMD/UMD module transformation)
+        ImportStar = 1 << 16,           // __importStar (used by CommonJS/AMD/UMD module transformation)
+        ImportDefault = 1 << 17,        // __importStar (used by CommonJS/AMD/UMD module transformation)
+        MakeTemplateObject = 1 << 18,   // __makeTemplateObject (used for constructing template string array objects)
+        ClassPrivateFieldGet = 1 << 19, // __classPrivateFieldGet (used by the class private field transformation)
+        ClassPrivateFieldSet = 1 << 20, // __classPrivateFieldSet (used by the class private field transformation)
+        CreateBinding = 1 << 21,        // __createBinding (use by the module transform for (re)exports and namespace imports)
         FirstEmitHelper = Extends,
         LastEmitHelper = CreateBinding,
 
@@ -6634,8 +6633,7 @@ namespace ts {
         AsyncDelegatorIncludes = Await | AsyncDelegator | AsyncValues,
 
         // Helpers included by ES2015 spread
-        SpreadIncludes = Read | Spread,
-
+        SpreadIncludes = Read | SpreadArray,
     }
 
     export const enum EmitHint {

src/compiler/utilities.ts

@@ -7033,6 +7033,17 @@ namespace ts {
         }
     }
 
+    function isPackedElement(node: Expression) {
+        return !isOmittedExpression(node);
+    }
+
+    /**
+     * Determines whether the provided node is an ArrayLiteralExpression that contains no missing elements.
+     */
+    export function isPackedArrayLiteral(node: Expression) {
+        return isArrayLiteralExpression(node) && every(node.elements, isPackedElement);
+    }
+
     /**
      * Indicates whether the result of an `Expression` will be unused.
      *

src/testRunner/unittests/tsbuild/helpers.ts

@@ -615,7 +615,8 @@ const { b, ...rest } = { a: 10, b: 30, yy: 30 };
         const content = fs.readFileSync(path, "utf8");
         fs.writeFileSync(path, `${content}
 function ${project}${file}Spread(...b: number[]) { }
-${project}${file}Spread(...[10, 20, 30]);`);
+const ${project}${file}_ar = [20, 30];
+${project}${file}Spread(10, ...${project}${file}_ar);`);
 
         replaceText(fs, `src/${project}/tsconfig.json`, `"strict": false,`, `"strict": false,
     "downlevelIteration": true,`);

tests/baselines/reference/argumentExpressionContextualTyping.js

@@ -19,12 +19,10 @@ baz(["string", 1, true, ...array]);  // Error
 foo(o);                              // Error because x has an array type namely (string|number)[]
 
 //// [argumentExpressionContextualTyping.js]
-var __spreadArrays = (this && this.__spreadArrays) || function () {
-    for (var s = 0, i = 0, il = arguments.length; i < il; i++) s += arguments[i].length;
-    for (var r = Array(s), k = 0, i = 0; i < il; i++)
-        for (var a = arguments[i], j = 0, jl = a.length; j < jl; j++, k++)
-            r[k] = a[j];
-    return r;
+var __spreadArray = (this && this.__spreadArray) || function (to, from) {
+    for (var i = 0, il = from.length, j = to.length; i < il; i++, j++)
+        to[j] = from[i];
+    return to;
 };
 // In a typed function call, argument expressions are contextually typed by their corresponding parameter types.
 function foo(_a) {
@@ -43,5 +41,5 @@ var tuple = ["string", 1, true];
 baz(tuple);
 baz(["string", 1, true]);
 baz(array); // Error
-baz(__spreadArrays(["string", 1, true], array)); // Error
+baz(__spreadArray(["string", 1, true], array)); // Error
 foo(o); // Error because x has an array type namely (string|number)[]

tests/baselines/reference/arrayLiteralExpressionContextualTyping.js

@@ -16,12 +16,10 @@ var spr2:[number, number, number] = [1, 2, 3, ...tup];  // Error
 
 
 //// [arrayLiteralExpressionContextualTyping.js]
-var __spreadArrays = (this && this.__spreadArrays) || function () {
-    for (var s = 0, i = 0, il = arguments.length; i < il; i++) s += arguments[i].length;
-    for (var r = Array(s), k = 0, i = 0; i < il; i++)
-        for (var a = arguments[i], j = 0, jl = a.length; j < jl; j++, k++)
-            r[k] = a[j];
-    return r;
+var __spreadArray = (this && this.__spreadArray) || function (to, from) {
+    for (var i = 0, il = from.length, j = to.length; i < il; i++, j++)
+        to[j] = from[i];
+    return to;
 };
 // In a contextually typed array literal expression containing no spread elements, an element expression at index N is contextually typed by
 //      the type of the property with the numeric name N in the contextual type, if any, or otherwise
@@ -33,6 +31,6 @@ var tup1 = [1, 2, 3, "string"];
 var tup2 = [1, 2, 3, "string"]; // Error
 // In a contextually typed array literal expression containing one or more spread elements,
 // an element expression at index N is contextually typed by the numeric index type of the contextual type, if any.
-var spr = __spreadArrays([1, 2, 3], array);
-var spr1 = __spreadArrays([1, 2, 3], tup);
-var spr2 = __spreadArrays([1, 2, 3], tup); // Error
+var spr = __spreadArray([1, 2, 3], array);
+var spr1 = __spreadArray([1, 2, 3], tup);
+var spr2 = __spreadArray([1, 2, 3], tup); // Error