Skip to content

Fix array spread with sideeffects #41523

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jan 6, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24695,7 +24695,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);
Expand Down Expand Up @@ -24723,7 +24723,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)) {
Expand Down Expand Up @@ -39048,8 +39048,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";
Expand Down
62 changes: 21 additions & 41 deletions src/compiler/factory/emitHelpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -58,8 +57,7 @@ namespace ts {
// ES2015 Helpers
createExtendsHelper,
createTemplateObjectHelper,
createSpreadHelper,
createSpreadArraysHelper,
createSpreadArrayHelper,
// ES2015 Destructuring Helpers
createValuesHelper,
createReadHelper,
Expand Down Expand Up @@ -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]
);
}

Expand Down Expand Up @@ -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;
};`
};

Expand Down Expand Up @@ -886,8 +860,7 @@ namespace ts {
awaiterHelper,
extendsHelper,
templateObjectHelper,
spreadHelper,
spreadArraysHelper,
spreadArrayHelper,
valuesHelper,
readHelper,
generatorHelper,
Expand Down Expand Up @@ -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;
}
}
82 changes: 48 additions & 34 deletions src/compiler/transformers/es2015.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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) {
Expand Down
28 changes: 13 additions & 15 deletions src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,

Expand All @@ -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 {
Expand Down
11 changes: 11 additions & 0 deletions src/compiler/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down
3 changes: 2 additions & 1 deletion src/testRunner/unittests/tsbuild/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,`);
Expand Down
12 changes: 5 additions & 7 deletions tests/baselines/reference/argumentExpressionContextualTyping.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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)[]
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Loading