From 8231519b853b0fe594cb70b8b89fb8e5c15c2918 Mon Sep 17 00:00:00 2001 From: Eli Barzilay Date: Mon, 1 Jun 2020 11:58:44 -0400 Subject: [PATCH] Simplify `visitObjectLiteralExpression` I ran into it and the comment at the top tripped me, then I proceeded to simplify the code. Patched a bit more of the function to make sure that the indentation doesn't change, and added tests. --- src/compiler/transformers/es2015.ts | 82 ++++--- .../es5-asyncFunctionObjectLiterals.js | 8 +- .../es5-yieldFunctionObjectLiterals.js | 200 ++++++++++++++++++ .../es5-yieldFunctionObjectLiterals.symbols | 135 ++++++++++++ .../es5-yieldFunctionObjectLiterals.types | 178 ++++++++++++++++ .../es5-yieldFunctionObjectLiterals.ts | 66 ++++++ 6 files changed, 620 insertions(+), 49 deletions(-) create mode 100644 tests/baselines/reference/es5-yieldFunctionObjectLiterals.js create mode 100644 tests/baselines/reference/es5-yieldFunctionObjectLiterals.symbols create mode 100644 tests/baselines/reference/es5-yieldFunctionObjectLiterals.types create mode 100644 tests/cases/compiler/es5-yieldFunctionObjectLiterals.ts diff --git a/src/compiler/transformers/es2015.ts b/src/compiler/transformers/es2015.ts index a23b701cf07..07c0e278271 100644 --- a/src/compiler/transformers/es2015.ts +++ b/src/compiler/transformers/es2015.ts @@ -2566,62 +2566,54 @@ namespace ts { * @param node An ObjectLiteralExpression node. */ function visitObjectLiteralExpression(node: ObjectLiteralExpression): Expression { - // We are here because a ComputedPropertyName was used somewhere in the expression. const properties = node.properties; - const numProperties = properties.length; // Find the first computed property. // Everything until that point can be emitted as part of the initial object literal. - let numInitialProperties = numProperties; - let numInitialPropertiesWithoutYield = numProperties; - for (let i = 0; i < numProperties; i++) { + let numInitialProperties = -1, hasComputed = false; + for (let i = 0; i < properties.length; i++) { const property = properties[i]; - if ((property.transformFlags & TransformFlags.ContainsYield && hierarchyFacts & HierarchyFacts.AsyncFunctionBody) - && i < numInitialPropertiesWithoutYield) { - numInitialPropertiesWithoutYield = i; - } - if (Debug.checkDefined(property.name).kind === SyntaxKind.ComputedPropertyName) { + if ((property.transformFlags & TransformFlags.ContainsYield && + hierarchyFacts & HierarchyFacts.AsyncFunctionBody) + || (hasComputed = Debug.checkDefined(property.name).kind === SyntaxKind.ComputedPropertyName)) { numInitialProperties = i; break; } } - if (numInitialProperties !== numProperties) { - if (numInitialPropertiesWithoutYield < numInitialProperties) { - numInitialProperties = numInitialPropertiesWithoutYield; - } - - // For computed properties, we need to create a unique handle to the object - // literal so we can modify it without risking internal assignments tainting the object. - const temp = createTempVariable(hoistVariableDeclaration); - - // Write out the first non-computed properties, then emit the rest through indexing on the temp variable. - const expressions: Expression[] = []; - const assignment = createAssignment( - temp, - setEmitFlags( - createObjectLiteral( - visitNodes(properties, visitor, isObjectLiteralElementLike, 0, numInitialProperties), - node.multiLine - ), - EmitFlags.Indented - ) - ); - - if (node.multiLine) { - startOnNewLine(assignment); - } - - expressions.push(assignment); - - addObjectLiteralMembers(expressions, node, temp, numInitialProperties); - - // We need to clone the temporary identifier so that we can write it on a - // new line - expressions.push(node.multiLine ? startOnNewLine(getMutableClone(temp)) : temp); - return inlineExpressions(expressions); + if (numInitialProperties < 0) { + return visitEachChild(node, visitor, context); } - return visitEachChild(node, visitor, context); + + // For computed properties, we need to create a unique handle to the object + // literal so we can modify it without risking internal assignments tainting the object. + const temp = createTempVariable(hoistVariableDeclaration); + + // Write out the first non-computed properties, then emit the rest through indexing on the temp variable. + const expressions: Expression[] = []; + const assignment = createAssignment( + temp, + setEmitFlags( + createObjectLiteral( + visitNodes(properties, visitor, isObjectLiteralElementLike, 0, numInitialProperties), + node.multiLine + ), + hasComputed ? EmitFlags.Indented : 0 + ) + ); + + if (node.multiLine) { + startOnNewLine(assignment); + } + + expressions.push(assignment); + + addObjectLiteralMembers(expressions, node, temp, numInitialProperties); + + // We need to clone the temporary identifier so that we can write it on a + // new line + expressions.push(node.multiLine ? startOnNewLine(getMutableClone(temp)) : temp); + return inlineExpressions(expressions); } interface ForStatementWithConvertibleInitializer extends ForStatement { diff --git a/tests/baselines/reference/es5-asyncFunctionObjectLiterals.js b/tests/baselines/reference/es5-asyncFunctionObjectLiterals.js index e6031d256af..a419bc4ffae 100644 --- a/tests/baselines/reference/es5-asyncFunctionObjectLiterals.js +++ b/tests/baselines/reference/es5-asyncFunctionObjectLiterals.js @@ -146,8 +146,8 @@ function objectLiteral5() { switch (_b.label) { case 0: _a = { - a: y - }; + a: y + }; return [4 /*yield*/, b]; case 1: x = (_a[_b.sent()] = z, @@ -165,8 +165,8 @@ function objectLiteral6() { switch (_c.label) { case 0: _b = { - a: y - }; + a: y + }; _a = b; return [4 /*yield*/, z]; case 1: diff --git a/tests/baselines/reference/es5-yieldFunctionObjectLiterals.js b/tests/baselines/reference/es5-yieldFunctionObjectLiterals.js new file mode 100644 index 00000000000..a445ecc9349 --- /dev/null +++ b/tests/baselines/reference/es5-yieldFunctionObjectLiterals.js @@ -0,0 +1,200 @@ +//// [es5-yieldFunctionObjectLiterals.ts] +// mainly to verify indentation of emitted code + +function g() { return "g"; } + +function* objectLiteral1() { + const x = { + a: 1, + b: yield 2, + c: 3, + } +} + +function* objectLiteral2() { + const x = { + a: 1, + [g()]: yield 2, + c: 3, + } +} + +function* objectLiteral3() { + const x = { + a: 1, + b: yield 2, + [g()]: 3, + c: 4, + } +} + +function* objectLiteral4() { + const x = { + a: 1, + [g()]: 2, + b: yield 3, + c: 4, + } +} + +function* objectLiteral5() { + const x = { + a: 1, + [g()]: yield 2, + c: 4, + } +} + +function* objectLiteral6() { + const x = { + a: 1, + [yield]: 2, + c: 4, + } +} + +function* objectLiteral7() { + const x = { + a: 1, + [yield]: yield 2, + c: 4, + } +} + + +//// [es5-yieldFunctionObjectLiterals.js] +// mainly to verify indentation of emitted code +function g() { return "g"; } +function objectLiteral1() { + var x, _a; + return __generator(this, function (_b) { + switch (_b.label) { + case 0: + _a = { + a: 1 + }; + return [4 /*yield*/, 2]; + case 1: + x = (_a.b = _b.sent(), + _a.c = 3, + _a); + return [2 /*return*/]; + } + }); +} +function objectLiteral2() { + var x, _a; + var _b; + return __generator(this, function (_c) { + switch (_c.label) { + case 0: + _b = { + a: 1 + }; + _a = g(); + return [4 /*yield*/, 2]; + case 1: + x = (_b[_a] = _c.sent(), + _b.c = 3, + _b); + return [2 /*return*/]; + } + }); +} +function objectLiteral3() { + var x, _a; + var _b; + return __generator(this, function (_c) { + switch (_c.label) { + case 0: + _a = { + a: 1 + }; + return [4 /*yield*/, 2]; + case 1: + x = (_b = (_a.b = _c.sent(), + _a), + _b[g()] = 3, + _b.c = 4, + _b); + return [2 /*return*/]; + } + }); +} +function objectLiteral4() { + var x; + var _a; + return __generator(this, function (_b) { + switch (_b.label) { + case 0: + _a = { + a: 1 + }, + _a[g()] = 2; + return [4 /*yield*/, 3]; + case 1: + x = (_a.b = _b.sent(), + _a.c = 4, + _a); + return [2 /*return*/]; + } + }); +} +function objectLiteral5() { + var x, _a; + var _b; + return __generator(this, function (_c) { + switch (_c.label) { + case 0: + _b = { + a: 1 + }; + _a = g(); + return [4 /*yield*/, 2]; + case 1: + x = (_b[_a] = _c.sent(), + _b.c = 4, + _b); + return [2 /*return*/]; + } + }); +} +function objectLiteral6() { + var x; + var _a; + return __generator(this, function (_b) { + switch (_b.label) { + case 0: + _a = { + a: 1 + }; + return [4 /*yield*/]; + case 1: + x = (_a[_b.sent()] = 2, + _a.c = 4, + _a); + return [2 /*return*/]; + } + }); +} +function objectLiteral7() { + var x, _a; + var _b; + return __generator(this, function (_c) { + switch (_c.label) { + case 0: + _b = { + a: 1 + }; + return [4 /*yield*/]; + case 1: + _a = _c.sent(); + return [4 /*yield*/, 2]; + case 2: + x = (_b[_a] = _c.sent(), + _b.c = 4, + _b); + return [2 /*return*/]; + } + }); +} diff --git a/tests/baselines/reference/es5-yieldFunctionObjectLiterals.symbols b/tests/baselines/reference/es5-yieldFunctionObjectLiterals.symbols new file mode 100644 index 00000000000..7d6ae6b28c4 --- /dev/null +++ b/tests/baselines/reference/es5-yieldFunctionObjectLiterals.symbols @@ -0,0 +1,135 @@ +=== tests/cases/compiler/es5-yieldFunctionObjectLiterals.ts === +// mainly to verify indentation of emitted code + +function g() { return "g"; } +>g : Symbol(g, Decl(es5-yieldFunctionObjectLiterals.ts, 0, 0)) + +function* objectLiteral1() { +>objectLiteral1 : Symbol(objectLiteral1, Decl(es5-yieldFunctionObjectLiterals.ts, 2, 28)) + + const x = { +>x : Symbol(x, Decl(es5-yieldFunctionObjectLiterals.ts, 5, 9)) + + a: 1, +>a : Symbol(a, Decl(es5-yieldFunctionObjectLiterals.ts, 5, 15)) + + b: yield 2, +>b : Symbol(b, Decl(es5-yieldFunctionObjectLiterals.ts, 6, 13)) + + c: 3, +>c : Symbol(c, Decl(es5-yieldFunctionObjectLiterals.ts, 7, 19)) + } +} + +function* objectLiteral2() { +>objectLiteral2 : Symbol(objectLiteral2, Decl(es5-yieldFunctionObjectLiterals.ts, 10, 1)) + + const x = { +>x : Symbol(x, Decl(es5-yieldFunctionObjectLiterals.ts, 13, 9)) + + a: 1, +>a : Symbol(a, Decl(es5-yieldFunctionObjectLiterals.ts, 13, 15)) + + [g()]: yield 2, +>[g()] : Symbol([g()], Decl(es5-yieldFunctionObjectLiterals.ts, 14, 13)) +>g : Symbol(g, Decl(es5-yieldFunctionObjectLiterals.ts, 0, 0)) + + c: 3, +>c : Symbol(c, Decl(es5-yieldFunctionObjectLiterals.ts, 15, 23)) + } +} + +function* objectLiteral3() { +>objectLiteral3 : Symbol(objectLiteral3, Decl(es5-yieldFunctionObjectLiterals.ts, 18, 1)) + + const x = { +>x : Symbol(x, Decl(es5-yieldFunctionObjectLiterals.ts, 21, 9)) + + a: 1, +>a : Symbol(a, Decl(es5-yieldFunctionObjectLiterals.ts, 21, 15)) + + b: yield 2, +>b : Symbol(b, Decl(es5-yieldFunctionObjectLiterals.ts, 22, 13)) + + [g()]: 3, +>[g()] : Symbol([g()], Decl(es5-yieldFunctionObjectLiterals.ts, 23, 19)) +>g : Symbol(g, Decl(es5-yieldFunctionObjectLiterals.ts, 0, 0)) + + c: 4, +>c : Symbol(c, Decl(es5-yieldFunctionObjectLiterals.ts, 24, 17)) + } +} + +function* objectLiteral4() { +>objectLiteral4 : Symbol(objectLiteral4, Decl(es5-yieldFunctionObjectLiterals.ts, 27, 1)) + + const x = { +>x : Symbol(x, Decl(es5-yieldFunctionObjectLiterals.ts, 30, 9)) + + a: 1, +>a : Symbol(a, Decl(es5-yieldFunctionObjectLiterals.ts, 30, 15)) + + [g()]: 2, +>[g()] : Symbol([g()], Decl(es5-yieldFunctionObjectLiterals.ts, 31, 13)) +>g : Symbol(g, Decl(es5-yieldFunctionObjectLiterals.ts, 0, 0)) + + b: yield 3, +>b : Symbol(b, Decl(es5-yieldFunctionObjectLiterals.ts, 32, 17)) + + c: 4, +>c : Symbol(c, Decl(es5-yieldFunctionObjectLiterals.ts, 33, 19)) + } +} + +function* objectLiteral5() { +>objectLiteral5 : Symbol(objectLiteral5, Decl(es5-yieldFunctionObjectLiterals.ts, 36, 1)) + + const x = { +>x : Symbol(x, Decl(es5-yieldFunctionObjectLiterals.ts, 39, 9)) + + a: 1, +>a : Symbol(a, Decl(es5-yieldFunctionObjectLiterals.ts, 39, 15)) + + [g()]: yield 2, +>[g()] : Symbol([g()], Decl(es5-yieldFunctionObjectLiterals.ts, 40, 13)) +>g : Symbol(g, Decl(es5-yieldFunctionObjectLiterals.ts, 0, 0)) + + c: 4, +>c : Symbol(c, Decl(es5-yieldFunctionObjectLiterals.ts, 41, 23)) + } +} + +function* objectLiteral6() { +>objectLiteral6 : Symbol(objectLiteral6, Decl(es5-yieldFunctionObjectLiterals.ts, 44, 1)) + + const x = { +>x : Symbol(x, Decl(es5-yieldFunctionObjectLiterals.ts, 47, 9)) + + a: 1, +>a : Symbol(a, Decl(es5-yieldFunctionObjectLiterals.ts, 47, 15)) + + [yield]: 2, +>[yield] : Symbol([yield], Decl(es5-yieldFunctionObjectLiterals.ts, 48, 13)) + + c: 4, +>c : Symbol(c, Decl(es5-yieldFunctionObjectLiterals.ts, 49, 19)) + } +} + +function* objectLiteral7() { +>objectLiteral7 : Symbol(objectLiteral7, Decl(es5-yieldFunctionObjectLiterals.ts, 52, 1)) + + const x = { +>x : Symbol(x, Decl(es5-yieldFunctionObjectLiterals.ts, 55, 9)) + + a: 1, +>a : Symbol(a, Decl(es5-yieldFunctionObjectLiterals.ts, 55, 15)) + + [yield]: yield 2, +>[yield] : Symbol([yield], Decl(es5-yieldFunctionObjectLiterals.ts, 56, 13)) + + c: 4, +>c : Symbol(c, Decl(es5-yieldFunctionObjectLiterals.ts, 57, 25)) + } +} + diff --git a/tests/baselines/reference/es5-yieldFunctionObjectLiterals.types b/tests/baselines/reference/es5-yieldFunctionObjectLiterals.types new file mode 100644 index 00000000000..c32ec8fd5c3 --- /dev/null +++ b/tests/baselines/reference/es5-yieldFunctionObjectLiterals.types @@ -0,0 +1,178 @@ +=== tests/cases/compiler/es5-yieldFunctionObjectLiterals.ts === +// mainly to verify indentation of emitted code + +function g() { return "g"; } +>g : () => string +>"g" : "g" + +function* objectLiteral1() { +>objectLiteral1 : () => Generator + + const x = { +>x : { a: number; b: any; c: number; } +>{ a: 1, b: yield 2, c: 3, } : { a: number; b: any; c: number; } + + a: 1, +>a : number +>1 : 1 + + b: yield 2, +>b : any +>yield 2 : any +>2 : 2 + + c: 3, +>c : number +>3 : 3 + } +} + +function* objectLiteral2() { +>objectLiteral2 : () => Generator + + const x = { +>x : { [x: string]: any; a: number; c: number; } +>{ a: 1, [g()]: yield 2, c: 3, } : { [x: string]: any; a: number; c: number; } + + a: 1, +>a : number +>1 : 1 + + [g()]: yield 2, +>[g()] : any +>g() : string +>g : () => string +>yield 2 : any +>2 : 2 + + c: 3, +>c : number +>3 : 3 + } +} + +function* objectLiteral3() { +>objectLiteral3 : () => Generator + + const x = { +>x : { [x: string]: any; a: number; b: any; c: number; } +>{ a: 1, b: yield 2, [g()]: 3, c: 4, } : { [x: string]: any; a: number; b: any; c: number; } + + a: 1, +>a : number +>1 : 1 + + b: yield 2, +>b : any +>yield 2 : any +>2 : 2 + + [g()]: 3, +>[g()] : number +>g() : string +>g : () => string +>3 : 3 + + c: 4, +>c : number +>4 : 4 + } +} + +function* objectLiteral4() { +>objectLiteral4 : () => Generator + + const x = { +>x : { [x: string]: any; a: number; b: any; c: number; } +>{ a: 1, [g()]: 2, b: yield 3, c: 4, } : { [x: string]: any; a: number; b: any; c: number; } + + a: 1, +>a : number +>1 : 1 + + [g()]: 2, +>[g()] : number +>g() : string +>g : () => string +>2 : 2 + + b: yield 3, +>b : any +>yield 3 : any +>3 : 3 + + c: 4, +>c : number +>4 : 4 + } +} + +function* objectLiteral5() { +>objectLiteral5 : () => Generator + + const x = { +>x : { [x: string]: any; a: number; c: number; } +>{ a: 1, [g()]: yield 2, c: 4, } : { [x: string]: any; a: number; c: number; } + + a: 1, +>a : number +>1 : 1 + + [g()]: yield 2, +>[g()] : any +>g() : string +>g : () => string +>yield 2 : any +>2 : 2 + + c: 4, +>c : number +>4 : 4 + } +} + +function* objectLiteral6() { +>objectLiteral6 : () => Generator + + const x = { +>x : { [x: number]: number; a: number; c: number; } +>{ a: 1, [yield]: 2, c: 4, } : { [x: number]: number; a: number; c: number; } + + a: 1, +>a : number +>1 : 1 + + [yield]: 2, +>[yield] : number +>yield : any +>2 : 2 + + c: 4, +>c : number +>4 : 4 + } +} + +function* objectLiteral7() { +>objectLiteral7 : () => Generator + + const x = { +>x : { [x: number]: any; a: number; c: number; } +>{ a: 1, [yield]: yield 2, c: 4, } : { [x: number]: any; a: number; c: number; } + + a: 1, +>a : number +>1 : 1 + + [yield]: yield 2, +>[yield] : any +>yield : any +>yield 2 : any +>2 : 2 + + c: 4, +>c : number +>4 : 4 + } +} + diff --git a/tests/cases/compiler/es5-yieldFunctionObjectLiterals.ts b/tests/cases/compiler/es5-yieldFunctionObjectLiterals.ts new file mode 100644 index 00000000000..f6a00a3f956 --- /dev/null +++ b/tests/cases/compiler/es5-yieldFunctionObjectLiterals.ts @@ -0,0 +1,66 @@ +// @lib: es5,es2015.promise +// @noEmitHelpers: true +// @target: ES5 +// @lib: es2015 + +// mainly to verify indentation of emitted code + +function g() { return "g"; } + +function* objectLiteral1() { + const x = { + a: 1, + b: yield 2, + c: 3, + } +} + +function* objectLiteral2() { + const x = { + a: 1, + [g()]: yield 2, + c: 3, + } +} + +function* objectLiteral3() { + const x = { + a: 1, + b: yield 2, + [g()]: 3, + c: 4, + } +} + +function* objectLiteral4() { + const x = { + a: 1, + [g()]: 2, + b: yield 3, + c: 4, + } +} + +function* objectLiteral5() { + const x = { + a: 1, + [g()]: yield 2, + c: 4, + } +} + +function* objectLiteral6() { + const x = { + a: 1, + [yield]: 2, + c: 4, + } +} + +function* objectLiteral7() { + const x = { + a: 1, + [yield]: yield 2, + c: 4, + } +}