Merge pull request microsoft#41094 from microsoft/destructuringEvaluationOrder

Fix destructuring evaluation order for initializers

Author: rbuckton

src/compiler/binder.ts

@@ -218,6 +218,9 @@ namespace ts {
         // or if compiler options contain alwaysStrict.
         let inStrictMode: boolean;
 
+        // If we are binding an assignment pattern, we will bind certain expressions differently.
+        let inAssignmentPattern = false;
+
         let symbolCount = 0;
 
         let Symbol: new (flags: SymbolFlags, name: __String) => Symbol;
@@ -275,6 +278,7 @@ namespace ts {
             currentExceptionTarget = undefined;
             activeLabelList = undefined;
             hasExplicitReturn = false;
+            inAssignmentPattern = false;
             emitFlags = NodeFlags.None;
         }
 
@@ -733,9 +737,14 @@ namespace ts {
         }
 
         function bindChildren(node: Node): void {
+            const saveInAssignmentPattern = inAssignmentPattern;
+            // Most nodes aren't valid in an assignment pattern, so we clear the value here
+            // and set it before we descend into nodes that could actually be part of an assignment pattern.
+            inAssignmentPattern = false;
             if (checkUnreachable(node)) {
                 bindEachChild(node);
                 bindJSDoc(node);
+                inAssignmentPattern = saveInAssignmentPattern;
                 return;
             }
             if (node.kind >= SyntaxKind.FirstStatement && node.kind <= SyntaxKind.LastStatement && !options.allowUnreachableCode) {
@@ -791,6 +800,13 @@ namespace ts {
                     bindPostfixUnaryExpressionFlow(<PostfixUnaryExpression>node);
                     break;
                 case SyntaxKind.BinaryExpression:
+                    if (isDestructuringAssignment(node)) {
+                        // Carry over whether we are in an assignment pattern to
+                        // binary expressions that could actually be an initializer
+                        inAssignmentPattern = saveInAssignmentPattern;
+                        bindDestructuringAssignmentFlow(node);
+                        return;
+                    }
                     bindBinaryExpressionFlow(<BinaryExpression>node);
                     break;
                 case SyntaxKind.DeleteExpression:
@@ -827,11 +843,23 @@ namespace ts {
                 case SyntaxKind.ModuleBlock:
                     bindEachFunctionsFirst((node as Block).statements);
                     break;
+                case SyntaxKind.BindingElement:
+                    bindBindingElementFlow(<BindingElement>node);
+                    break;
+                case SyntaxKind.ObjectLiteralExpression:
+                case SyntaxKind.ArrayLiteralExpression:
+                case SyntaxKind.PropertyAssignment:
+                case SyntaxKind.SpreadElement:
+                    // Carry over whether we are in an assignment pattern of Object and Array literals
+                    // as well as their children that are valid assignment targets.
+                    inAssignmentPattern = saveInAssignmentPattern;
+                    // falls through
                 default:
                     bindEachChild(node);
                     break;
             }
             bindJSDoc(node);
+            inAssignmentPattern = saveInAssignmentPattern;
         }
 
         function isNarrowingExpression(expr: Expression): boolean {
@@ -1455,6 +1483,24 @@ namespace ts {
             }
         }
 
+        function bindDestructuringAssignmentFlow(node: DestructuringAssignment) {
+            if (inAssignmentPattern) {
+                inAssignmentPattern = false;
+                bind(node.operatorToken);
+                bind(node.right);
+                inAssignmentPattern = true;
+                bind(node.left);
+            }
+            else {
+                inAssignmentPattern = true;
+                bind(node.left);
+                inAssignmentPattern = false;
+                bind(node.operatorToken);
+                bind(node.right);
+            }
+            bindAssignmentTargetFlow(node.left);
+        }
+
         const enum BindBinaryExpressionFlowState {
             BindThenBindChildren,
             MaybeBindLeft,
@@ -1571,7 +1617,7 @@ namespace ts {
              * If `node` is a BinaryExpression, adds it to the local work stack, otherwise recursively binds it
              */
             function maybeBind(node: Node) {
-                if (node && isBinaryExpression(node)) {
+                if (node && isBinaryExpression(node) && !isDestructuringAssignment(node)) {
                     stackIndex++;
                     workStacks.expr[stackIndex] = node;
                     workStacks.state[stackIndex] = BindBinaryExpressionFlowState.BindThenBindChildren;
@@ -1626,6 +1672,25 @@ namespace ts {
             }
         }
 
+        function bindBindingElementFlow(node: BindingElement) {
+            if (isBindingPattern(node.name)) {
+                // When evaluating a binding pattern, the initializer is evaluated before the binding pattern, per:
+                // - https://tc39.es/ecma262/#sec-destructuring-binding-patterns-runtime-semantics-iteratorbindinginitialization
+                //   - `BindingElement: BindingPattern Initializer?`
+                // - https://tc39.es/ecma262/#sec-runtime-semantics-keyedbindinginitialization
+                //   - `BindingElement: BindingPattern Initializer?`
+                bindEach(node.decorators);
+                bindEach(node.modifiers);
+                bind(node.dotDotDotToken);
+                bind(node.propertyName);
+                bind(node.initializer);
+                bind(node.name);
+            }
+            else {
+                bindEachChild(node);
+            }
+        }
+
         function bindJSDocTypeAlias(node: JSDocTypedefTag | JSDocCallbackTag | JSDocEnumTag) {
             setParent(node.tagName, node);
             if (node.kind !== SyntaxKind.JSDocEnumTag && node.fullName) {

src/compiler/factory/nodeFactory.ts

@@ -2663,12 +2663,14 @@ namespace ts {
                     node.transformFlags |=
                         TransformFlags.ContainsES2015 |
                         TransformFlags.ContainsES2018 |
-                        TransformFlags.ContainsDestructuringAssignment;
+                        TransformFlags.ContainsDestructuringAssignment |
+                        propagateAssignmentPatternFlags(node.left);
                 }
                 else if (isArrayLiteralExpression(node.left)) {
                     node.transformFlags |=
                         TransformFlags.ContainsES2015 |
-                        TransformFlags.ContainsDestructuringAssignment;
+                        TransformFlags.ContainsDestructuringAssignment |
+                        propagateAssignmentPatternFlags(node.left);
                 }
             }
             else if (operatorKind === SyntaxKind.AsteriskAsteriskToken || operatorKind === SyntaxKind.AsteriskAsteriskEqualsToken) {
@@ -2680,6 +2682,27 @@ namespace ts {
             return node;
         }
 
+        function propagateAssignmentPatternFlags(node: AssignmentPattern): TransformFlags {
+            if (node.transformFlags & TransformFlags.ContainsObjectRestOrSpread) return TransformFlags.ContainsObjectRestOrSpread;
+            if (node.transformFlags & TransformFlags.ContainsES2018) {
+                // check for nested spread assignments, otherwise '{ x: { a, ...b } = foo } = c'
+                // will not be correctly interpreted by the ES2018 transformer
+                for (const element of getElementsOfBindingOrAssignmentPattern(node)) {
+                    const target = getTargetOfBindingOrAssignmentElement(element);
+                    if (target && isAssignmentPattern(target)) {
+                        if (target.transformFlags & TransformFlags.ContainsObjectRestOrSpread) {
+                            return TransformFlags.ContainsObjectRestOrSpread;
+                        }
+                        if (target.transformFlags & TransformFlags.ContainsES2018) {
+                            const flags = propagateAssignmentPatternFlags(target);
+                            if (flags) return flags;
+                        }
+                    }
+                }
+            }
+            return TransformFlags.None;
+        }
+
         // @api
         function updateBinaryExpression(node: BinaryExpression, left: Expression, operator: BinaryOperatorToken, right: Expression) {
             return node.left !== left

src/compiler/transformers/destructuring.ts

@@ -5,6 +5,7 @@ namespace ts {
         level: FlattenLevel;
         downlevelIteration: boolean;
         hoistTempVariables: boolean;
+        hasTransformedPriorElement?: boolean; // indicates whether we've transformed a prior declaration
         emitExpression: (value: Expression) => void;
         emitBindingOrAssignment: (target: BindingOrAssignmentElementTarget, value: Expression, location: TextRange, original: Node | undefined) => void;
         createArrayBindingOrAssignmentPattern: (elements: BindingOrAssignmentElement[]) => ArrayBindingOrAssignmentPattern;
@@ -265,18 +266,27 @@ namespace ts {
         value: Expression | undefined,
         location: TextRange,
         skipInitializer?: boolean) {
+        const bindingTarget = getTargetOfBindingOrAssignmentElement(element)!; // TODO: GH#18217
         if (!skipInitializer) {
             const initializer = visitNode(getInitializerOfBindingOrAssignmentElement(element), flattenContext.visitor, isExpression);
             if (initializer) {
                 // Combine value and initializer
-                value = value ? createDefaultValueCheck(flattenContext, value, initializer, location) : initializer;
+                if (value) {
+                    value = createDefaultValueCheck(flattenContext, value, initializer, location);
+                    // If 'value' is not a simple expression, it could contain side-effecting code that should evaluate before an object or array binding pattern.
+                    if (!isSimpleInlineableExpression(initializer) && isBindingOrAssignmentPattern(bindingTarget)) {
+                        value = ensureIdentifier(flattenContext, value, /*reuseIdentifierExpressions*/ true, location);
+                    }
+                }
+                else {
+                    value = initializer;
+                }
             }
             else if (!value) {
                 // Use 'void 0' in absence of value and initializer
                 value = flattenContext.context.factory.createVoidZero();
             }
         }
-        const bindingTarget = getTargetOfBindingOrAssignmentElement(element)!; // TODO: GH#18217
         if (isObjectBindingOrAssignmentPattern(bindingTarget)) {
             flattenObjectBindingOrAssignmentPattern(flattenContext, element, bindingTarget, value!, location);
         }
@@ -393,7 +403,8 @@ namespace ts {
             if (flattenContext.level >= FlattenLevel.ObjectRest) {
                 // If an array pattern contains an ObjectRest, we must cache the result so that we
                 // can perform the ObjectRest destructuring in a different declaration
-                if (element.transformFlags & TransformFlags.ContainsObjectRestOrSpread) {
+                if (element.transformFlags & TransformFlags.ContainsObjectRestOrSpread || flattenContext.hasTransformedPriorElement && !isSimpleBindingOrAssignmentElement(element)) {
+                    flattenContext.hasTransformedPriorElement = true;
                     const temp = flattenContext.context.factory.createTempVariable(/*recordTempVariable*/ undefined);
                     if (flattenContext.hoistTempVariables) {
                         flattenContext.context.hoistVariableDeclaration(temp);
@@ -428,6 +439,17 @@ namespace ts {
         }
     }
 
+    function isSimpleBindingOrAssignmentElement(element: BindingOrAssignmentElement): boolean {
+        const target = getTargetOfBindingOrAssignmentElement(element);
+        if (!target || isOmittedExpression(target)) return true;
+        const propertyName = tryGetPropertyNameOfBindingOrAssignmentElement(element);
+        if (propertyName && !isPropertyNameLiteral(propertyName)) return false;
+        const initializer = getInitializerOfBindingOrAssignmentElement(element);
+        if (initializer && !isSimpleInlineableExpression(initializer)) return false;
+        if (isBindingOrAssignmentPattern(target)) return every(getElementsOfBindingOrAssignmentPattern(target), isSimpleBindingOrAssignmentElement);
+        return isIdentifier(target);
+    }
+
     /**
      * Creates an expression used to provide a default value if a value is `undefined` at runtime.
      *