Skip to content

Commit 87cc8c4

Browse files
authored
Evaluate RHS of binding/assignment pattern first (microsoft#34806)
1 parent 00dd1f0 commit 87cc8c4

12 files changed

+62
-43
lines changed

src/compiler/factory.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5389,6 +5389,12 @@ namespace ts {
53895389
* Gets the property name of a BindingOrAssignmentElement
53905390
*/
53915391
export function getPropertyNameOfBindingOrAssignmentElement(bindingElement: BindingOrAssignmentElement): PropertyName | undefined {
5392+
const propertyName = tryGetPropertyNameOfBindingOrAssignmentElement(bindingElement);
5393+
Debug.assert(!!propertyName || isSpreadAssignment(bindingElement), "Invalid property name for binding element.");
5394+
return propertyName;
5395+
}
5396+
5397+
export function tryGetPropertyNameOfBindingOrAssignmentElement(bindingElement: BindingOrAssignmentElement): PropertyName | undefined {
53925398
switch (bindingElement.kind) {
53935399
case SyntaxKind.BindingElement:
53945400
// `a` in `let { a: b } = ...`
@@ -5429,8 +5435,6 @@ namespace ts {
54295435
? target.expression
54305436
: target;
54315437
}
5432-
5433-
Debug.fail("Invalid property name for binding element.");
54345438
}
54355439

54365440
function isStringOrNumericLiteral(node: Node): node is StringLiteral | NumericLiteral {

src/compiler/transformers/destructuring.ts

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,8 @@ namespace ts {
6868
if (value) {
6969
value = visitNode(value, visitor, isExpression);
7070

71-
if (isIdentifier(value) && bindingOrAssignmentElementAssignsToName(node, value.escapedText)) {
71+
if (isIdentifier(value) && bindingOrAssignmentElementAssignsToName(node, value.escapedText) ||
72+
bindingOrAssignmentElementContainsNonLiteralComputedName(node)) {
7273
// If the right-hand value of the assignment is also an assignment target then
7374
// we need to cache the right-hand value.
7475
value = ensureIdentifier(flattenContext, value, /*reuseIdentifierExpressions*/ false, location);
@@ -147,6 +148,19 @@ namespace ts {
147148
return false;
148149
}
149150

151+
function bindingOrAssignmentElementContainsNonLiteralComputedName(element: BindingOrAssignmentElement): boolean {
152+
const propertyName = tryGetPropertyNameOfBindingOrAssignmentElement(element);
153+
if (propertyName && isComputedPropertyName(propertyName) && !isLiteralExpression(propertyName.expression)) {
154+
return true;
155+
}
156+
const target = getTargetOfBindingOrAssignmentElement(element);
157+
return !!target && isBindingOrAssignmentPattern(target) && bindingOrAssignmentPatternContainsNonLiteralComputedName(target);
158+
}
159+
160+
function bindingOrAssignmentPatternContainsNonLiteralComputedName(pattern: BindingOrAssignmentPattern): boolean {
161+
return !!forEach(getElementsOfBindingOrAssignmentPattern(pattern), bindingOrAssignmentElementContainsNonLiteralComputedName);
162+
}
163+
150164
/**
151165
* Flattens a VariableDeclaration or ParameterDeclaration to one or more variable declarations.
152166
*
@@ -184,7 +198,8 @@ namespace ts {
184198

185199
if (isVariableDeclaration(node)) {
186200
let initializer = getInitializerOfBindingOrAssignmentElement(node);
187-
if (initializer && isIdentifier(initializer) && bindingOrAssignmentElementAssignsToName(node, initializer.escapedText)) {
201+
if (initializer && (isIdentifier(initializer) && bindingOrAssignmentElementAssignsToName(node, initializer.escapedText) ||
202+
bindingOrAssignmentElementContainsNonLiteralComputedName(node))) {
188203
// If the right-hand value of the assignment is also an assignment target then
189204
// we need to cache the right-hand value.
190205
initializer = ensureIdentifier(flattenContext, initializer, /*reuseIdentifierExpressions*/ false, initializer);

tests/baselines/reference/blockScopedBindingUsedBeforeDef.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ for (var _i = 0, _a = [{}]; _i < _a.length; _i++) {
1515
continue;
1616
}
1717
// 2:
18-
for (var _c = a, a = {}[_c]; false;)
18+
for (var _c = {}, _d = a, a = _c[_d]; false;)
1919
continue;
2020
// 3:
21-
var _d = b, b = {}[_d];
21+
var _e = {}, _f = b, b = _e[_f];

tests/baselines/reference/computedPropertiesInDestructuring1.js

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -38,15 +38,15 @@ let [{[foo.toExponential()]: bar7}] = [{bar: "bar"}];
3838

3939

4040
//// [computedPropertiesInDestructuring1.js]
41-
var _a, _b, _c, _d, _e, _f;
41+
var _a, _b, _c, _d, _e, _f, _g, _h, _j, _k, _l, _m;
4242
// destructuring in variable declarations
4343
var foo = "bar";
44-
var _g = foo, bar = { bar: "bar" }[_g];
44+
var _o = { bar: "bar" }, _p = foo, bar = _o[_p];
4545
var bar2 = { bar: "bar" }["bar"];
4646
var foo2 = function () { return "bar"; };
47-
var _h = foo2(), bar3 = { bar: "bar" }[_h];
48-
var _j = foo, bar4 = [{ bar: "bar" }][0][_j];
49-
var _k = foo2(), bar5 = [{ bar: "bar" }][0][_k];
47+
var _q = { bar: "bar" }, _r = foo2(), bar3 = _q[_r];
48+
var _s = [{ bar: "bar" }], _t = foo, bar4 = _s[0][_t];
49+
var _u = [{ bar: "bar" }], _v = foo2(), bar5 = _u[0][_v];
5050
function f1(_a) {
5151
var x = _a["bar"];
5252
}
@@ -63,13 +63,13 @@ function f5(_a) {
6363
var _b = foo2(), x = _a[0][_b];
6464
}
6565
// report errors on type errors in computed properties used in destructuring
66-
var _l = foo(), bar6 = [{ bar: "bar" }][0][_l];
67-
var _m = foo.toExponential(), bar7 = [{ bar: "bar" }][0][_m];
66+
var _w = [{ bar: "bar" }], _x = foo(), bar6 = _w[0][_x];
67+
var _y = [{ bar: "bar" }], _z = foo.toExponential(), bar7 = _y[0][_z];
6868
// destructuring assignment
69-
(_a = foo, bar = { bar: "bar" }[_a]);
69+
(_a = { bar: "bar" }, _b = foo, bar = _a[_b]);
7070
(bar2 = { bar: "bar" }["bar"]);
71-
(_b = foo2(), bar3 = { bar: "bar" }[_b]);
72-
_c = foo, bar4 = [{ bar: "bar" }][0][_c];
73-
_d = foo2(), bar5 = [{ bar: "bar" }][0][_d];
74-
_e = foo(), bar4 = [{ bar: "bar" }][0][_e];
75-
_f = (1 + {}), bar4 = [{ bar: "bar" }][0][_f];
71+
(_c = { bar: "bar" }, _d = foo2(), bar3 = _c[_d]);
72+
_e = [{ bar: "bar" }], _f = foo, bar4 = _e[0][_f];
73+
_g = [{ bar: "bar" }], _h = foo2(), bar5 = _g[0][_h];
74+
_j = [{ bar: "bar" }], _k = foo(), bar4 = _j[0][_k];
75+
_l = [{ bar: "bar" }], _m = (1 + {}), bar4 = _l[0][_m];

tests/baselines/reference/computedPropertiesInDestructuring2.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,4 +4,4 @@ let {[foo2()]: bar3} = {};
44

55
//// [computedPropertiesInDestructuring2.js]
66
var foo2 = function () { return "bar"; };
7-
var _a = foo2(), bar3 = {}[_a];
7+
var _a = {}, _b = foo2(), bar3 = _a[_b];

tests/baselines/reference/crashInGetTextOfComputedPropertyName.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,12 +36,12 @@ exports.__esModule = true;
3636
var itemId = 'some-id';
3737
// --- test on first level ---
3838
var items = {};
39-
var _a = itemId, itemOk1 = items[_a];
39+
var _a = items, _b = itemId, itemOk1 = _a[_b];
4040
typeof itemOk1; // pass
4141
var objWithItems = { items: {} };
4242
var itemOk2 = objWithItems.items[itemId];
4343
typeof itemOk2; // pass
44-
var _b = objWithItems.items /*happens when default value is provided*/, _c = itemId, itemWithTSError = (_b === void 0 ? {} /*happens when default value is provided*/ : _b)[_c];
44+
var _c = objWithItems, _d = _c.items /*happens when default value is provided*/, _e = itemId, itemWithTSError = (_d === void 0 ? {} /*happens when default value is provided*/ : _d)[_e];
4545
// in order to re-produce the error, uncomment next line:
4646
typeof itemWithTSError; // :(
4747
// will result in:

tests/baselines/reference/destructureComputedProperty.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ const { p: p3 } = new C();
1313

1414
//// [destructureComputedProperty.js]
1515
var nameN = "n";
16-
var _a = nameN, n = ab[_a];
16+
var _a = ab, _b = nameN, n = _a[_b];
1717
var C = /** @class */ (function () {
1818
function C() {
1919
}
@@ -22,5 +22,5 @@ var C = /** @class */ (function () {
2222
var nameP = "p";
2323
var p0 = new C()["p"];
2424
var p1 = new C()["p"];
25-
var _b = nameP, p2 = new C()[_b];
25+
var _c = new C(), _d = nameP, p2 = _c[_d];
2626
var p3 = new C().p;

tests/baselines/reference/destructuredMaappedTypeIsNotImplicitlyAny.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ function foo<T extends string>(key: T, obj: { [_ in T]: number }) {
99

1010
//// [destructuredMaappedTypeIsNotImplicitlyAny.js]
1111
function foo(key, obj) {
12-
var _a = key, bar = obj[_a]; // Element implicitly has an 'any' type because type '{ [_ in T]: number; }' has no index signature.
12+
var _a = obj, _b = key, bar = _a[_b]; // Element implicitly has an 'any' type because type '{ [_ in T]: number; }' has no index signature.
1313
bar; // bar : any
1414
// Note: this does work:
1515
var lorem = obj[key];

tests/baselines/reference/destructuringAssignment_private.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ const nameO = "o";
1515

1616

1717
//// [destructuringAssignment_private.js]
18-
var _a, _b;
18+
var _a, _b, _c, _d;
1919
var C = /** @class */ (function () {
2020
function C() {
2121
this.x = 0;
@@ -27,6 +27,6 @@ var x;
2727
(x = [{ a: new C() }][0].a.x);
2828
(x = new C().o[0].a);
2929
var nameX = "x";
30-
(_a = nameX, x = [{ a: new C() }][0].a[_a]);
30+
(_a = [{ a: new C() }], _b = nameX, x = _a[0].a[_b]);
3131
var nameO = "o";
32-
(_b = nameO, x = new C()[_b][0].a);
32+
(_c = new C(), _d = nameO, x = _c[_d][0].a);

tests/baselines/reference/destructuringControlFlow.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,11 +69,11 @@ function f3(obj) {
6969
}
7070
}
7171
function f4() {
72-
var _a;
72+
var _a, _b;
7373
var x;
7474
(x = 0..x); // Error
7575
(x = 0["x"]); // Error
76-
(_a = "x" + "", x = 0[_a]); // Errpr
76+
(_a = 0, _b = "x" + "", x = _a[_b]); // Errpr
7777
}
7878
var _a = ["foo"], key = _a[0], value = _a[1];
7979
value.toUpperCase(); // Error

tests/baselines/reference/genericObjectRest.js

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -48,16 +48,16 @@ function f1(obj) {
4848
let { a: a2, b: b2 } = obj, r2 = __rest(obj, ["a", "b"]);
4949
let { 'a': a3 } = obj, r3 = __rest(obj, ['a']);
5050
let { ['a']: a4 } = obj, r4 = __rest(obj, ['a']);
51-
let _a = a, a5 = obj[_a], r5 = __rest(obj, [typeof _a === "symbol" ? _a : _a + ""]);
51+
let _a = obj, _b = a, a5 = _a[_b], r5 = __rest(_a, [typeof _b === "symbol" ? _b : _b + ""]);
5252
}
5353
const sa = Symbol();
5454
const sb = Symbol();
5555
function f2(obj) {
56-
let _a = sa, a1 = obj[_a], _b = sb, b1 = obj[_b], r1 = __rest(obj, [typeof _a === "symbol" ? _a : _a + "", typeof _b === "symbol" ? _b : _b + ""]);
56+
let _a = obj, _b = sa, a1 = _a[_b], _c = sb, b1 = _a[_c], r1 = __rest(_a, [typeof _b === "symbol" ? _b : _b + "", typeof _c === "symbol" ? _c : _c + ""]);
5757
}
5858
function f3(obj, k1, k2) {
59-
let _a = k1, a1 = obj[_a], _b = k2, a2 = obj[_b], r1 = __rest(obj, [typeof _a === "symbol" ? _a : _a + "", typeof _b === "symbol" ? _b : _b + ""]);
59+
let _a = obj, _b = k1, a1 = _a[_b], _c = k2, a2 = _a[_c], r1 = __rest(_a, [typeof _b === "symbol" ? _b : _b + "", typeof _c === "symbol" ? _c : _c + ""]);
6060
}
6161
function f4(obj, k1, k2) {
62-
let _a = k1, a1 = obj[_a], _b = k2, a2 = obj[_b], r1 = __rest(obj, [typeof _a === "symbol" ? _a : _a + "", typeof _b === "symbol" ? _b : _b + ""]);
62+
let _a = obj, _b = k1, a1 = _a[_b], _c = k2, a2 = _a[_c], r1 = __rest(_a, [typeof _b === "symbol" ? _b : _b + "", typeof _c === "symbol" ? _c : _c + ""]);
6363
}

tests/baselines/reference/lateBoundDestructuringImplicitAnyError.js

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -30,26 +30,26 @@ void prop9;
3030

3131
//// [lateBoundDestructuringImplicitAnyError.js]
3232
var named = "foo";
33-
var _a = named, prop = { prop: "foo" }[_a];
33+
var _a = { prop: "foo" }, _b = named, prop = _a[_b];
3434
void prop;
3535
var numIndexed = null;
3636
var strIndexed = null;
3737
var numed = 6;
3838
var symed = Symbol();
3939
var symed2 = Symbol();
40-
var _b = named, prop2 = numIndexed[_b];
40+
var _c = numIndexed, _d = named, prop2 = _c[_d];
4141
void prop2;
42-
var _c = numed, prop3 = numIndexed[_c];
42+
var _e = numIndexed, _f = numed, prop3 = _e[_f];
4343
void prop3;
44-
var _d = named, prop4 = strIndexed[_d];
44+
var _g = strIndexed, _h = named, prop4 = _g[_h];
4545
void prop4;
46-
var _e = numed, prop5 = strIndexed[_e];
46+
var _j = strIndexed, _k = numed, prop5 = _j[_k];
4747
void prop5;
48-
var _f = symed, prop6 = numIndexed[_f];
48+
var _l = numIndexed, _m = symed, prop6 = _l[_m];
4949
void prop6;
50-
var _g = symed, prop7 = strIndexed[_g];
50+
var _o = strIndexed, _p = symed, prop7 = _o[_p];
5151
void prop7;
52-
var _h = symed2, prop8 = numIndexed[_h];
52+
var _q = numIndexed, _r = symed2, prop8 = _q[_r];
5353
void prop8;
54-
var _j = symed2, prop9 = strIndexed[_j];
54+
var _s = strIndexed, _t = symed2, prop9 = _s[_t];
5555
void prop9;

0 commit comments

Comments
 (0)