Evaluate RHS of binding/assignment pattern first (microsoft#34806)

Author: rbuckton

src/compiler/factory.ts

@@ -5389,6 +5389,12 @@ namespace ts {
      * Gets the property name of a BindingOrAssignmentElement
      */
     export function getPropertyNameOfBindingOrAssignmentElement(bindingElement: BindingOrAssignmentElement): PropertyName | undefined {
+        const propertyName = tryGetPropertyNameOfBindingOrAssignmentElement(bindingElement);
+        Debug.assert(!!propertyName || isSpreadAssignment(bindingElement), "Invalid property name for binding element.");
+        return propertyName;
+    }
+
+    export function tryGetPropertyNameOfBindingOrAssignmentElement(bindingElement: BindingOrAssignmentElement): PropertyName | undefined {
         switch (bindingElement.kind) {
             case SyntaxKind.BindingElement:
                 // `a` in `let { a: b } = ...`
@@ -5429,8 +5435,6 @@ namespace ts {
                 ? target.expression
                 : target;
         }
-
-        Debug.fail("Invalid property name for binding element.");
     }
 
     function isStringOrNumericLiteral(node: Node): node is StringLiteral | NumericLiteral {

src/compiler/transformers/destructuring.ts

@@ -68,7 +68,8 @@ namespace ts {
         if (value) {
             value = visitNode(value, visitor, isExpression);
 
-            if (isIdentifier(value) && bindingOrAssignmentElementAssignsToName(node, value.escapedText)) {
+            if (isIdentifier(value) && bindingOrAssignmentElementAssignsToName(node, value.escapedText) ||
+                bindingOrAssignmentElementContainsNonLiteralComputedName(node)) {
                 // If the right-hand value of the assignment is also an assignment target then
                 // we need to cache the right-hand value.
                 value = ensureIdentifier(flattenContext, value, /*reuseIdentifierExpressions*/ false, location);
@@ -147,6 +148,19 @@ namespace ts {
         return false;
     }
 
+    function bindingOrAssignmentElementContainsNonLiteralComputedName(element: BindingOrAssignmentElement): boolean {
+        const propertyName = tryGetPropertyNameOfBindingOrAssignmentElement(element);
+        if (propertyName && isComputedPropertyName(propertyName) && !isLiteralExpression(propertyName.expression)) {
+            return true;
+        }
+        const target = getTargetOfBindingOrAssignmentElement(element);
+        return !!target && isBindingOrAssignmentPattern(target) && bindingOrAssignmentPatternContainsNonLiteralComputedName(target);
+    }
+
+    function bindingOrAssignmentPatternContainsNonLiteralComputedName(pattern: BindingOrAssignmentPattern): boolean {
+        return !!forEach(getElementsOfBindingOrAssignmentPattern(pattern), bindingOrAssignmentElementContainsNonLiteralComputedName);
+    }
+
     /**
      * Flattens a VariableDeclaration or ParameterDeclaration to one or more variable declarations.
      *
@@ -184,7 +198,8 @@ namespace ts {
 
         if (isVariableDeclaration(node)) {
             let initializer = getInitializerOfBindingOrAssignmentElement(node);
-            if (initializer && isIdentifier(initializer) && bindingOrAssignmentElementAssignsToName(node, initializer.escapedText)) {
+            if (initializer && (isIdentifier(initializer) && bindingOrAssignmentElementAssignsToName(node, initializer.escapedText) ||
+                bindingOrAssignmentElementContainsNonLiteralComputedName(node))) {
                 // If the right-hand value of the assignment is also an assignment target then
                 // we need to cache the right-hand value.
                 initializer = ensureIdentifier(flattenContext, initializer, /*reuseIdentifierExpressions*/ false, initializer);

tests/baselines/reference/blockScopedBindingUsedBeforeDef.js

@@ -15,7 +15,7 @@ for (var _i = 0, _a = [{}]; _i < _a.length; _i++) {
     continue;
 }
 // 2:
-for (var _c = a, a = {}[_c]; false;)
+for (var _c = {}, _d = a, a = _c[_d]; false;)
     continue;
 // 3:
-var _d = b, b = {}[_d];
+var _e = {}, _f = b, b = _e[_f];

tests/baselines/reference/computedPropertiesInDestructuring1.js

@@ -38,15 +38,15 @@ let [{[foo.toExponential()]: bar7}] = [{bar: "bar"}];
 
 
 //// [computedPropertiesInDestructuring1.js]
-var _a, _b, _c, _d, _e, _f;
+var _a, _b, _c, _d, _e, _f, _g, _h, _j, _k, _l, _m;
 // destructuring in variable declarations
 var foo = "bar";
-var _g = foo, bar = { bar: "bar" }[_g];
+var _o = { bar: "bar" }, _p = foo, bar = _o[_p];
 var bar2 = { bar: "bar" }["bar"];
 var foo2 = function () { return "bar"; };
-var _h = foo2(), bar3 = { bar: "bar" }[_h];
-var _j = foo, bar4 = [{ bar: "bar" }][0][_j];
-var _k = foo2(), bar5 = [{ bar: "bar" }][0][_k];
+var _q = { bar: "bar" }, _r = foo2(), bar3 = _q[_r];
+var _s = [{ bar: "bar" }], _t = foo, bar4 = _s[0][_t];
+var _u = [{ bar: "bar" }], _v = foo2(), bar5 = _u[0][_v];
 function f1(_a) {
     var x = _a["bar"];
 }
@@ -63,13 +63,13 @@ function f5(_a) {
     var _b = foo2(), x = _a[0][_b];
 }
 // report errors on type errors in computed properties used in destructuring
-var _l = foo(), bar6 = [{ bar: "bar" }][0][_l];
-var _m = foo.toExponential(), bar7 = [{ bar: "bar" }][0][_m];
+var _w = [{ bar: "bar" }], _x = foo(), bar6 = _w[0][_x];
+var _y = [{ bar: "bar" }], _z = foo.toExponential(), bar7 = _y[0][_z];
 // destructuring assignment
-(_a = foo, bar = { bar: "bar" }[_a]);
+(_a = { bar: "bar" }, _b = foo, bar = _a[_b]);
 (bar2 = { bar: "bar" }["bar"]);
-(_b = foo2(), bar3 = { bar: "bar" }[_b]);
-_c = foo, bar4 = [{ bar: "bar" }][0][_c];
-_d = foo2(), bar5 = [{ bar: "bar" }][0][_d];
-_e = foo(), bar4 = [{ bar: "bar" }][0][_e];
-_f = (1 + {}), bar4 = [{ bar: "bar" }][0][_f];
+(_c = { bar: "bar" }, _d = foo2(), bar3 = _c[_d]);
+_e = [{ bar: "bar" }], _f = foo, bar4 = _e[0][_f];
+_g = [{ bar: "bar" }], _h = foo2(), bar5 = _g[0][_h];
+_j = [{ bar: "bar" }], _k = foo(), bar4 = _j[0][_k];
+_l = [{ bar: "bar" }], _m = (1 + {}), bar4 = _l[0][_m];

tests/baselines/reference/computedPropertiesInDestructuring2.js

@@ -4,4 +4,4 @@ let {[foo2()]: bar3} = {};
 
 //// [computedPropertiesInDestructuring2.js]
 var foo2 = function () { return "bar"; };
-var _a = foo2(), bar3 = {}[_a];
+var _a = {}, _b = foo2(), bar3 = _a[_b];

tests/baselines/reference/crashInGetTextOfComputedPropertyName.js

@@ -36,12 +36,12 @@ exports.__esModule = true;
 var itemId = 'some-id';
 // --- test on first level ---
 var items = {};
-var _a = itemId, itemOk1 = items[_a];
+var _a = items, _b = itemId, itemOk1 = _a[_b];
 typeof itemOk1; // pass
 var objWithItems = { items: {} };
 var itemOk2 = objWithItems.items[itemId];
 typeof itemOk2; // pass
-var _b = objWithItems.items /*happens when default value is provided*/, _c = itemId, itemWithTSError = (_b === void 0 ? {} /*happens when default value is provided*/ : _b)[_c];
+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];
 // in order to re-produce the error, uncomment next line:
 typeof itemWithTSError; // :(
 // will result in:

tests/baselines/reference/destructureComputedProperty.js

@@ -13,7 +13,7 @@ const { p: p3 } = new C();
 
 //// [destructureComputedProperty.js]
 var nameN = "n";
-var _a = nameN, n = ab[_a];
+var _a = ab, _b = nameN, n = _a[_b];
 var C = /** @class */ (function () {
     function C() {
     }
@@ -22,5 +22,5 @@ var C = /** @class */ (function () {
 var nameP = "p";
 var p0 = new C()["p"];
 var p1 = new C()["p"];
-var _b = nameP, p2 = new C()[_b];
+var _c = new C(), _d = nameP, p2 = _c[_d];
 var p3 = new C().p;

tests/baselines/reference/destructuredMaappedTypeIsNotImplicitlyAny.js

@@ -9,7 +9,7 @@ function foo<T extends string>(key: T, obj: { [_ in T]: number }) {
 
 //// [destructuredMaappedTypeIsNotImplicitlyAny.js]
 function foo(key, obj) {
-    var _a = key, bar = obj[_a]; // Element implicitly has an 'any' type because type '{ [_ in T]: number; }' has no index signature.
+    var _a = obj, _b = key, bar = _a[_b]; // Element implicitly has an 'any' type because type '{ [_ in T]: number; }' has no index signature.
     bar; // bar : any
     // Note: this does work:
     var lorem = obj[key];

tests/baselines/reference/destructuringAssignment_private.js

@@ -15,7 +15,7 @@ const nameO = "o";
 
 
 //// [destructuringAssignment_private.js]
-var _a, _b;
+var _a, _b, _c, _d;
 var C = /** @class */ (function () {
     function C() {
         this.x = 0;
@@ -27,6 +27,6 @@ var x;
 (x = [{ a: new C() }][0].a.x);
 (x = new C().o[0].a);
 var nameX = "x";
-(_a = nameX, x = [{ a: new C() }][0].a[_a]);
+(_a = [{ a: new C() }], _b = nameX, x = _a[0].a[_b]);
 var nameO = "o";
-(_b = nameO, x = new C()[_b][0].a);
+(_c = new C(), _d = nameO, x = _c[_d][0].a);

tests/baselines/reference/destructuringControlFlow.js

@@ -69,11 +69,11 @@ function f3(obj) {
     }
 }
 function f4() {
-    var _a;
+    var _a, _b;
     var x;
     (x = 0..x); // Error
     (x = 0["x"]); // Error
-    (_a = "x" + "", x = 0[_a]); // Errpr
+    (_a = 0, _b = "x" + "", x = _a[_b]); // Errpr
 }
 var _a = ["foo"], key = _a[0], value = _a[1];
 value.toUpperCase(); // Error