Skip to content

Commit 44ec8dd

Browse files
rbucktonrbuckton
authored
Fix array spread with sideeffects (microsoft#41523)
* Fix array spread with sideeffects * Minor cleanup, ensure multiple emit helpers for outfile tests
1 parent 7a5aadc commit 44ec8dd

File tree

55 files changed

+5253
-4282
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

55 files changed

+5253
-4282
lines changed

‎src/compiler/checker.ts

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24695,7 +24695,7 @@ namespace ts {
2469524695

2469624696
function checkSpreadExpression(node: SpreadElement, checkMode?: CheckMode): Type {
2469724697
if (languageVersion < ScriptTarget.ES2015) {
24698-
checkExternalEmitHelpers(node, compilerOptions.downlevelIteration ? ExternalEmitHelpers.SpreadIncludes : ExternalEmitHelpers.SpreadArrays);
24698+
checkExternalEmitHelpers(node, compilerOptions.downlevelIteration ? ExternalEmitHelpers.SpreadIncludes : ExternalEmitHelpers.SpreadArray);
2469924699
}
2470024700

2470124701
const arrayOrIterableType = checkExpression(node.expression, checkMode);
@@ -24723,7 +24723,7 @@ namespace ts {
2472324723
const e = elements[i];
2472424724
if (e.kind === SyntaxKind.SpreadElement) {
2472524725
if (languageVersion < ScriptTarget.ES2015) {
24726-
checkExternalEmitHelpers(e, compilerOptions.downlevelIteration ? ExternalEmitHelpers.SpreadIncludes : ExternalEmitHelpers.SpreadArrays);
24726+
checkExternalEmitHelpers(e, compilerOptions.downlevelIteration ? ExternalEmitHelpers.SpreadIncludes : ExternalEmitHelpers.SpreadArray);
2472724727
}
2472824728
const spreadType = checkExpression((<SpreadElement>e).expression, checkMode, forceTuple);
2472924729
if (isArrayLikeType(spreadType)) {
@@ -39048,8 +39048,7 @@ namespace ts {
3904839048
case ExternalEmitHelpers.Generator: return "__generator";
3904939049
case ExternalEmitHelpers.Values: return "__values";
3905039050
case ExternalEmitHelpers.Read: return "__read";
39051-
case ExternalEmitHelpers.Spread: return "__spread";
39052-
case ExternalEmitHelpers.SpreadArrays: return "__spreadArrays";
39051+
case ExternalEmitHelpers.SpreadArray: return "__spreadArray";
3905339052
case ExternalEmitHelpers.Await: return "__await";
3905439053
case ExternalEmitHelpers.AsyncGenerator: return "__asyncGenerator";
3905539054
case ExternalEmitHelpers.AsyncDelegator: return "__asyncDelegator";

‎src/compiler/factory/emitHelpers.ts

Lines changed: 21 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,7 @@ namespace ts {
1919
// ES2015 Helpers
2020
createExtendsHelper(name: Identifier): Expression;
2121
createTemplateObjectHelper(cooked: ArrayLiteralExpression, raw: ArrayLiteralExpression): Expression;
22-
createSpreadHelper(argumentList: readonly Expression[]): Expression;
23-
createSpreadArraysHelper(argumentList: readonly Expression[]): Expression;
22+
createSpreadArrayHelper(to: Expression, from: Expression): Expression;
2423
// ES2015 Destructuring Helpers
2524
createValuesHelper(expression: Expression): Expression;
2625
createReadHelper(iteratorRecord: Expression, count: number | undefined): Expression;
@@ -58,8 +57,7 @@ namespace ts {
5857
// ES2015 Helpers
5958
createExtendsHelper,
6059
createTemplateObjectHelper,
61-
createSpreadHelper,
62-
createSpreadArraysHelper,
60+
createSpreadArrayHelper,
6361
// ES2015 Destructuring Helpers
6462
createValuesHelper,
6563
createReadHelper,
@@ -284,22 +282,12 @@ namespace ts {
284282
);
285283
}
286284

287-
function createSpreadHelper(argumentList: readonly Expression[]) {
288-
context.requestEmitHelper(readHelper);
289-
context.requestEmitHelper(spreadHelper);
285+
function createSpreadArrayHelper(to: Expression, from: Expression) {
286+
context.requestEmitHelper(spreadArrayHelper);
290287
return factory.createCallExpression(
291-
getUnscopedHelperName("__spread"),
288+
getUnscopedHelperName("__spreadArray"),
292289
/*typeArguments*/ undefined,
293-
argumentList
294-
);
295-
}
296-
297-
function createSpreadArraysHelper(argumentList: readonly Expression[]) {
298-
context.requestEmitHelper(spreadArraysHelper);
299-
return factory.createCallExpression(
300-
getUnscopedHelperName("__spreadArrays"),
301-
/*typeArguments*/ undefined,
302-
argumentList
290+
[to, from]
303291
);
304292
}
305293

@@ -629,29 +617,15 @@ namespace ts {
629617
};`
630618
};
631619

632-
export const spreadHelper: UnscopedEmitHelper = {
633-
name: "typescript:spread",
634-
importName: "__spread",
635-
scoped: false,
636-
dependencies: [readHelper],
637-
text: `
638-
var __spread = (this && this.__spread) || function () {
639-
for (var ar = [], i = 0; i < arguments.length; i++) ar = ar.concat(__read(arguments[i]));
640-
return ar;
641-
};`
642-
};
643-
644-
export const spreadArraysHelper: UnscopedEmitHelper = {
645-
name: "typescript:spreadArrays",
646-
importName: "__spreadArrays",
620+
export const spreadArrayHelper: UnscopedEmitHelper = {
621+
name: "typescript:spreadArray",
622+
importName: "__spreadArray",
647623
scoped: false,
648624
text: `
649-
var __spreadArrays = (this && this.__spreadArrays) || function () {
650-
for (var s = 0, i = 0, il = arguments.length; i < il; i++) s += arguments[i].length;
651-
for (var r = Array(s), k = 0, i = 0; i < il; i++)
652-
for (var a = arguments[i], j = 0, jl = a.length; j < jl; j++, k++)
653-
r[k] = a[j];
654-
return r;
625+
var __spreadArray = (this && this.__spreadArray) || function (to, from) {
626+
for (var i = 0, il = from.length, j = to.length; i < il; i++, j++)
627+
to[j] = from[i];
628+
return to;
655629
};`
656630
};
657631

@@ -886,8 +860,7 @@ namespace ts {
886860
awaiterHelper,
887861
extendsHelper,
888862
templateObjectHelper,
889-
spreadHelper,
890-
spreadArraysHelper,
863+
spreadArrayHelper,
891864
valuesHelper,
892865
readHelper,
893866
generatorHelper,
@@ -917,4 +890,11 @@ namespace ts {
917890
return name => cache[name] || (cache[name] = { get value() { return geti(name); }, set value(v) { seti(name, v); } });
918891
})(name => super[name], (name, value) => super[name] = value);`
919892
};
893+
894+
export function isCallToHelper(firstSegment: Expression, helperName: __String) {
895+
return isCallExpression(firstSegment)
896+
&& isIdentifier(firstSegment.expression)
897+
&& (getEmitFlags(firstSegment.expression) & EmitFlags.HelperName)
898+
&& firstSegment.expression.escapedText === helperName;
899+
}
920900
}

‎src/compiler/transformers/es2015.ts

Lines changed: 48 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -3892,17 +3892,41 @@ namespace ts {
38923892
*
38933893
* @param elements The array of Expression nodes.
38943894
* @param needsUniqueCopy A value indicating whether to ensure that the result is a fresh array.
3895+
* This should be `true` when spreading into an `ArrayLiteral`, and `false` when spreading into an
3896+
* argument list.
38953897
* @param multiLine A value indicating whether the result should be emitted on multiple lines.
38963898
*/
38973899
function transformAndSpreadElements(elements: NodeArray<Expression>, needsUniqueCopy: boolean, multiLine: boolean, hasTrailingComma: boolean): Expression {
3900+
// When there is no leading SpreadElement:
3901+
//
38983902
// [source]
38993903
// [a, ...b, c]
39003904
//
39013905
// [output (downlevelIteration)]
3902-
// __spread([a], b, [c])
3906+
// __spreadArray(__spreadArray([a], __read(b)), [c])
39033907
//
39043908
// [output]
3905-
// __spreadArrays([a], b, [c])
3909+
// __spreadArray(__spreadArray([a], b), [c])
3910+
//
3911+
// When there *is* a leading SpreadElement:
3912+
//
3913+
// [source]
3914+
// [...a, b]
3915+
//
3916+
// [output (downlevelIteration)]
3917+
// __spreadArray(__spreadArray([], __read(a)), [b])
3918+
//
3919+
// [output]
3920+
// __spreadArray(__spreadArray([], a), [b])
3921+
//
3922+
// NOTE: We use `isPackedArrayLiteral` below rather than just `isArrayLiteral`
3923+
// because ES2015 spread will replace _missing_ array elements with `undefined`,
3924+
// so we cannot just use an array as is. For example:
3925+
//
3926+
// `[1, ...[2, , 3]]` becomes `[1, 2, undefined, 3]`
3927+
//
3928+
// However, for packed array literals (i.e., an array literal with no OmittedExpression
3929+
// elements), we can use the array as-is.
39063930

39073931
// Map spans of spread expressions into their expressions and spans of other
39083932
// expressions into an array literal.
@@ -3913,43 +3937,33 @@ namespace ts {
39133937
)
39143938
);
39153939

3916-
if (compilerOptions.downlevelIteration) {
3917-
if (segments.length === 1) {
3918-
const firstSegment = segments[0];
3919-
if (isCallToHelper(firstSegment, "___spread" as __String)) {
3920-
return segments[0];
3921-
}
3940+
if (segments.length === 1) {
3941+
const firstSegment = segments[0];
3942+
// If we don't need a unique copy, then we are spreading into an argument list for
3943+
// a CallExpression or NewExpression. When using `--downlevelIteration`, we need
3944+
// to coerce this into an array for use with `apply`, so we will use the code path
3945+
// that follows instead.
3946+
if (!needsUniqueCopy && !compilerOptions.downlevelIteration
3947+
|| isPackedArrayLiteral(firstSegment) // see NOTE (above)
3948+
|| isCallToHelper(firstSegment, "___spreadArray" as __String)) {
3949+
return segments[0];
39223950
}
3923-
3924-
return emitHelpers().createSpreadHelper(segments);
39253951
}
3926-
else {
3927-
if (segments.length === 1) {
3928-
const firstSegment = segments[0];
3929-
if (!needsUniqueCopy
3930-
|| isPackedArrayLiteral(firstSegment)
3931-
|| isCallToHelper(firstSegment, "___spreadArrays" as __String)) {
3932-
return segments[0];
3933-
}
3934-
}
39353952

3936-
return emitHelpers().createSpreadArraysHelper(segments);
3953+
const helpers = emitHelpers();
3954+
const startsWithSpread = isSpreadElement(elements[0]);
3955+
let expression: Expression =
3956+
startsWithSpread ? factory.createArrayLiteralExpression() :
3957+
segments[0];
3958+
for (let i = startsWithSpread ? 0 : 1; i < segments.length; i++) {
3959+
expression = helpers.createSpreadArrayHelper(
3960+
expression,
3961+
compilerOptions.downlevelIteration && !isPackedArrayLiteral(segments[i]) ? // see NOTE (above)
3962+
helpers.createReadHelper(segments[i], /*count*/ undefined) :
3963+
segments[i]);
39373964
}
3938-
}
39393965

3940-
function isPackedElement(node: Expression) {
3941-
return !isOmittedExpression(node);
3942-
}
3943-
3944-
function isPackedArrayLiteral(node: Expression) {
3945-
return isArrayLiteralExpression(node) && every(node.elements, isPackedElement);
3946-
}
3947-
3948-
function isCallToHelper(firstSegment: Expression, helperName: __String) {
3949-
return isCallExpression(firstSegment)
3950-
&& isIdentifier(firstSegment.expression)
3951-
&& (getEmitFlags(firstSegment.expression) & EmitFlags.HelperName)
3952-
&& firstSegment.expression.escapedText === helperName;
3966+
return expression;
39533967
}
39543968

39553969
function partitionSpread(node: Expression) {

‎src/compiler/types.ts

Lines changed: 13 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -6605,19 +6605,18 @@ namespace ts {
66056605
Generator = 1 << 7, // __generator (used by ES2015 generator transformation)
66066606
Values = 1 << 8, // __values (used by ES2015 for..of and yield* transformations)
66076607
Read = 1 << 9, // __read (used by ES2015 iterator destructuring transformation)
6608-
Spread = 1 << 10, // __spread (used by ES2015 array spread and argument list spread transformations)
6609-
SpreadArrays = 1 << 11, // __spreadArrays (used by ES2015 array spread and argument list spread transformations)
6610-
Await = 1 << 12, // __await (used by ES2017 async generator transformation)
6611-
AsyncGenerator = 1 << 13, // __asyncGenerator (used by ES2017 async generator transformation)
6612-
AsyncDelegator = 1 << 14, // __asyncDelegator (used by ES2017 async generator yield* transformation)
6613-
AsyncValues = 1 << 15, // __asyncValues (used by ES2017 for..await..of transformation)
6614-
ExportStar = 1 << 16, // __exportStar (used by CommonJS/AMD/UMD module transformation)
6615-
ImportStar = 1 << 17, // __importStar (used by CommonJS/AMD/UMD module transformation)
6616-
ImportDefault = 1 << 18, // __importStar (used by CommonJS/AMD/UMD module transformation)
6617-
MakeTemplateObject = 1 << 19, // __makeTemplateObject (used for constructing template string array objects)
6618-
ClassPrivateFieldGet = 1 << 20, // __classPrivateFieldGet (used by the class private field transformation)
6619-
ClassPrivateFieldSet = 1 << 21, // __classPrivateFieldSet (used by the class private field transformation)
6620-
CreateBinding = 1 << 22, // __createBinding (use by the module transform for (re)exports and namespace imports)
6608+
SpreadArray = 1 << 10, // __spreadArray (used by ES2015 array spread and argument list spread transformations)
6609+
Await = 1 << 11, // __await (used by ES2017 async generator transformation)
6610+
AsyncGenerator = 1 << 12, // __asyncGenerator (used by ES2017 async generator transformation)
6611+
AsyncDelegator = 1 << 13, // __asyncDelegator (used by ES2017 async generator yield* transformation)
6612+
AsyncValues = 1 << 14, // __asyncValues (used by ES2017 for..await..of transformation)
6613+
ExportStar = 1 << 15, // __exportStar (used by CommonJS/AMD/UMD module transformation)
6614+
ImportStar = 1 << 16, // __importStar (used by CommonJS/AMD/UMD module transformation)
6615+
ImportDefault = 1 << 17, // __importStar (used by CommonJS/AMD/UMD module transformation)
6616+
MakeTemplateObject = 1 << 18, // __makeTemplateObject (used for constructing template string array objects)
6617+
ClassPrivateFieldGet = 1 << 19, // __classPrivateFieldGet (used by the class private field transformation)
6618+
ClassPrivateFieldSet = 1 << 20, // __classPrivateFieldSet (used by the class private field transformation)
6619+
CreateBinding = 1 << 21, // __createBinding (use by the module transform for (re)exports and namespace imports)
66216620
FirstEmitHelper = Extends,
66226621
LastEmitHelper = CreateBinding,
66236622

@@ -6634,8 +6633,7 @@ namespace ts {
66346633
AsyncDelegatorIncludes = Await | AsyncDelegator | AsyncValues,
66356634

66366635
// Helpers included by ES2015 spread
6637-
SpreadIncludes = Read | Spread,
6638-
6636+
SpreadIncludes = Read | SpreadArray,
66396637
}
66406638

66416639
export const enum EmitHint {

‎src/compiler/utilities.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7033,6 +7033,17 @@ namespace ts {
70337033
}
70347034
}
70357035

7036+
function isPackedElement(node: Expression) {
7037+
return !isOmittedExpression(node);
7038+
}
7039+
7040+
/**
7041+
* Determines whether the provided node is an ArrayLiteralExpression that contains no missing elements.
7042+
*/
7043+
export function isPackedArrayLiteral(node: Expression) {
7044+
return isArrayLiteralExpression(node) && every(node.elements, isPackedElement);
7045+
}
7046+
70367047
/**
70377048
* Indicates whether the result of an `Expression` will be unused.
70387049
*

‎src/testRunner/unittests/tsbuild/helpers.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -615,7 +615,8 @@ const { b, ...rest } = { a: 10, b: 30, yy: 30 };
615615
const content = fs.readFileSync(path, "utf8");
616616
fs.writeFileSync(path, `${content}
617617
function ${project}${file}Spread(...b: number[]) { }
618-
${project}${file}Spread(...[10, 20, 30]);`);
618+
const ${project}${file}_ar = [20, 30];
619+
${project}${file}Spread(10, ...${project}${file}_ar);`);
619620

620621
replaceText(fs, `src/${project}/tsconfig.json`, `"strict": false,`, `"strict": false,
621622
"downlevelIteration": true,`);

‎tests/baselines/reference/argumentExpressionContextualTyping.js

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,10 @@ baz(["string", 1, true, ...array]); // Error
1919
foo(o); // Error because x has an array type namely (string|number)[]
2020

2121
//// [argumentExpressionContextualTyping.js]
22-
var __spreadArrays = (this && this.__spreadArrays) || function () {
23-
for (var s = 0, i = 0, il = arguments.length; i < il; i++) s += arguments[i].length;
24-
for (var r = Array(s), k = 0, i = 0; i < il; i++)
25-
for (var a = arguments[i], j = 0, jl = a.length; j < jl; j++, k++)
26-
r[k] = a[j];
27-
return r;
22+
var __spreadArray = (this && this.__spreadArray) || function (to, from) {
23+
for (var i = 0, il = from.length, j = to.length; i < il; i++, j++)
24+
to[j] = from[i];
25+
return to;
2826
};
2927
// In a typed function call, argument expressions are contextually typed by their corresponding parameter types.
3028
function foo(_a) {
@@ -43,5 +41,5 @@ var tuple = ["string", 1, true];
4341
baz(tuple);
4442
baz(["string", 1, true]);
4543
baz(array); // Error
46-
baz(__spreadArrays(["string", 1, true], array)); // Error
44+
baz(__spreadArray(["string", 1, true], array)); // Error
4745
foo(o); // Error because x has an array type namely (string|number)[]

‎tests/baselines/reference/arrayLiteralExpressionContextualTyping.js

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,10 @@ var spr2:[number, number, number] = [1, 2, 3, ...tup]; // Error
1616

1717

1818
//// [arrayLiteralExpressionContextualTyping.js]
19-
var __spreadArrays = (this && this.__spreadArrays) || function () {
20-
for (var s = 0, i = 0, il = arguments.length; i < il; i++) s += arguments[i].length;
21-
for (var r = Array(s), k = 0, i = 0; i < il; i++)
22-
for (var a = arguments[i], j = 0, jl = a.length; j < jl; j++, k++)
23-
r[k] = a[j];
24-
return r;
19+
var __spreadArray = (this && this.__spreadArray) || function (to, from) {
20+
for (var i = 0, il = from.length, j = to.length; i < il; i++, j++)
21+
to[j] = from[i];
22+
return to;
2523
};
2624
// In a contextually typed array literal expression containing no spread elements, an element expression at index N is contextually typed by
2725
// 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"];
3331
var tup2 = [1, 2, 3, "string"]; // Error
3432
// In a contextually typed array literal expression containing one or more spread elements,
3533
// an element expression at index N is contextually typed by the numeric index type of the contextual type, if any.
36-
var spr = __spreadArrays([1, 2, 3], array);
37-
var spr1 = __spreadArrays([1, 2, 3], tup);
38-
var spr2 = __spreadArrays([1, 2, 3], tup); // Error
34+
var spr = __spreadArray([1, 2, 3], array);
35+
var spr1 = __spreadArray([1, 2, 3], tup);
36+
var spr2 = __spreadArray([1, 2, 3], tup); // Error

0 commit comments

Comments
 (0)