[blocked by discussion in Microsoft/TypeScript repo] Fix property initializers with block scoped bindings

src/compiler/checker.ts

@@ -16200,6 +16200,11 @@ namespace ts {
                             if (container.kind === SyntaxKind.PropertyDeclaration && hasModifier(container, ModifierFlags.Static)) {
                                 getNodeLinks(declaration).flags |= NodeCheckFlags.ClassWithConstructorReference;
                                 getNodeLinks(node).flags |= NodeCheckFlags.ConstructorReferenceInClass;
+                                // If the class expression is in a loop and the name of the class is used,
+                                // the temporary variable which stores the evaluated class expression must be block scoped.
+                                if (getEnclosingIterationStatement(declaration)) {
+                                    getNodeLinks(declaration).flags |= NodeCheckFlags.BlockScopedBindingInLoop;
+                                }
                             }
                             break;
                         }
@@ -16301,8 +16306,20 @@ namespace ts {
             return assignmentKind ? getBaseTypeOfLiteralType(flowType) : flowType;
         }
 
-        function isInsideFunction(node: Node, threshold: Node): boolean {
-            return !!findAncestor(node, n => n === threshold ? "quit" : isFunctionLike(n));
+        function isInsideFunctionOrInstancePropertyInitializer(node: Node, threshold: Node): boolean {
+            return !!findAncestor(node, n => {
+                if (n === threshold) {
+                    return "quit";
+                }
+                if (isFunctionLike(n)) {
+                    return true;
+                }
+                return (n.parent && isPropertyDeclaration(n.parent) && !hasStaticModifier(n.parent) && n.parent.initializer === n);
+            });
+        }
+
+        function getEnclosingIterationStatement(node: Node): Node | undefined {
+            return findAncestor(node, n => (!n || nodeStartsNewLexicalEnvironment(n)) ? "quit" : isIterationStatement(n, /* lookInLabeledStatements */ false));
         }
 
         function getPartOfForStatementContainingNode(node: Node, container: ForStatement) {
@@ -16322,20 +16339,10 @@ namespace ts {
             // if there is an iteration statement in between declaration and boundary (is binding/class declared inside iteration statement)
 
             const container = getEnclosingBlockScopeContainer(symbol.valueDeclaration);
-            const usedInFunction = isInsideFunction(node.parent, container);
-            let current = container;
-
-            let containedInIterationStatement = false;
-            while (current && !nodeStartsNewLexicalEnvironment(current)) {
-                if (isIterationStatement(current, /*lookInLabeledStatements*/ false)) {
-                    containedInIterationStatement = true;
-                    break;
-                }
-                current = current.parent;
-            }
-
-            if (containedInIterationStatement) {
-                if (usedInFunction) {
+            const usedInFunctionOrInstanceProperty = isInsideFunctionOrInstancePropertyInitializer(node, container);
+            const enclosingIterationStatement = getEnclosingIterationStatement(container);
+            if (enclosingIterationStatement) {
+                if (usedInFunctionOrInstanceProperty) {
                     // mark iteration statement as containing block-scoped binding captured in some function
                     let capturesBlockScopeBindingInLoopBody = true;
                     if (isForStatement(container) &&
@@ -16354,7 +16361,7 @@ namespace ts {
                         }
                     }
                     if (capturesBlockScopeBindingInLoopBody) {
-                        getNodeLinks(current).flags |= NodeCheckFlags.LoopWithCapturedBlockScopedBinding;
+                        getNodeLinks(enclosingIterationStatement).flags |= NodeCheckFlags.LoopWithCapturedBlockScopedBinding;
                     }
                 }
 
@@ -16370,7 +16377,7 @@ namespace ts {
                 getNodeLinks(symbol.valueDeclaration).flags |= NodeCheckFlags.BlockScopedBindingInLoop;
             }
 
-            if (usedInFunction) {
+            if (usedInFunctionOrInstanceProperty) {
                 getNodeLinks(symbol.valueDeclaration).flags |= NodeCheckFlags.CapturedBlockScopedBinding;
             }
         }
@@ -17834,6 +17841,20 @@ namespace ts {
             const links = getNodeLinks(node.expression);
             if (!links.resolvedType) {
                 links.resolvedType = checkExpression(node.expression);
+
+                if (isPropertyDeclaration(node.parent) && isClassExpression(node.parent.parent)) {
+                    const container = getEnclosingBlockScopeContainer(node);
+                    const enclosingIterationStatement = getEnclosingIterationStatement(container);
+                    // A computed property of a class expression inside a loop must be block scoped because
+                    // the property name should be bound at class evaluation time.
+                    if (enclosingIterationStatement) {
+                        getNodeLinks(enclosingIterationStatement).flags |= NodeCheckFlags.LoopWithCapturedBlockScopedBinding;
+                        // The hoisted variable which stores the evaluated property name should be block scoped.
+                        getNodeLinks(node).flags |= NodeCheckFlags.BlockScopedBindingInLoop;
+                        // The temporary name of the class expression should be block scoped.
+                        getNodeLinks(node.parent.parent).flags |= NodeCheckFlags.BlockScopedBindingInLoop;
+                    }
+                }
                 // This will allow types number, string, symbol or any. It will also allow enums, the unknown
                 // type, and any union of these types (like string | number).
                 if (links.resolvedType.flags & TypeFlags.Nullable ||

src/compiler/factory.ts

@@ -3125,6 +3125,8 @@ namespace ts {
         enableEmitNotification: noop,
         enableSubstitution: noop,
         endLexicalEnvironment: () => undefined,
+        endBlockScope: () => undefined,
+        startBlockScope: noop,
         getCompilerOptions: notImplemented,
         getEmitHost: notImplemented,
         getEmitResolver: notImplemented,

src/compiler/transformer.ts

@@ -94,6 +94,9 @@ namespace ts {
         let lexicalEnvironmentFunctionDeclarationsStack: FunctionDeclaration[][] = [];
         let lexicalEnvironmentStackOffset = 0;
         let lexicalEnvironmentSuspended = false;
+        let blockScopedVariableDeclarationsStack: Identifier[][] = [];
+        let blockScopeStackOffset = 0;
+        let blockScopedVariableDeclarations: Identifier[];
         let emitHelpers: EmitHelper[] | undefined;
         let onSubstituteNode: TransformationContext["onSubstituteNode"] = noEmitSubstitution;
         let onEmitNode: TransformationContext["onEmitNode"] = noEmitNotification;
@@ -112,6 +115,8 @@ namespace ts {
             endLexicalEnvironment,
             hoistVariableDeclaration,
             hoistFunctionDeclaration,
+            startBlockScope,
+            endBlockScope,
             requestEmitHelper,
             readEmitHelpers,
             enableSubstitution,
@@ -239,6 +244,11 @@ namespace ts {
         function hoistVariableDeclaration(name: Identifier): void {
             Debug.assert(state > TransformationState.Uninitialized, "Cannot modify the lexical environment during initialization.");
             Debug.assert(state < TransformationState.Completed, "Cannot modify the lexical environment after transformation has completed.");
+            // If the checker determined that this is a block scoped binding in a loop, we must emit a block-level variable declaration.
+            if (resolver && resolver.getNodeCheckFlags(name) & NodeCheckFlags.BlockScopedBindingInLoop) {
+                (blockScopedVariableDeclarations || (blockScopedVariableDeclarations = [])).push(name);
+                return;
+            }
             const decl = setEmitFlags(createVariableDeclaration(name), EmitFlags.NoNestedSourceMaps);
             if (!lexicalEnvironmentVariableDeclarations) {
                 lexicalEnvironmentVariableDeclarations = [decl];
@@ -262,6 +272,41 @@ namespace ts {
             }
         }
 
+        /**
+         * Starts a block scope. Any existing block hoisted variables are pushed onto the stack and the related storage variables are reset.
+         */
+        function startBlockScope() {
+            Debug.assert(state > TransformationState.Uninitialized, "Cannot start a block scope during initialization.");
+            Debug.assert(state < TransformationState.Completed, "Cannot start a block scope after transformation has completed.");
+            blockScopedVariableDeclarationsStack[blockScopeStackOffset] = blockScopedVariableDeclarations;
+            blockScopeStackOffset++;
+            blockScopedVariableDeclarations = undefined!;
+        }
+
+        /**
+         * Ends a block scope. The previous set of block hoisted variables are restored. Any hoisted declarations are returned.
+         */
+        function endBlockScope() {
+            Debug.assert(state > TransformationState.Uninitialized, "Cannot end a block scope during initialization.");
+            Debug.assert(state < TransformationState.Completed, "Cannot end a block scope after transformation has completed.");
+            const statements: Statement[] | undefined = some(blockScopedVariableDeclarations) ?
+                [
+                    createVariableStatement(
+                        /*modifiers*/ undefined,
+                        createVariableDeclarationList(
+                            blockScopedVariableDeclarations.map(identifier => createVariableDeclaration(identifier)),
+                            NodeFlags.Let
+                        )
+                    )
+                ] : undefined;
+            blockScopeStackOffset--;
+            blockScopedVariableDeclarations = blockScopedVariableDeclarationsStack[blockScopeStackOffset];
+            if (blockScopeStackOffset === 0) {
+                blockScopedVariableDeclarationsStack = [];
+            }
+            return statements;
+        }
+
         /**
          * Starts a new lexical environment. Any existing hoisted variable or function declarations
          * are pushed onto a stack, and the related storage variables are reset.

src/compiler/transformers/ts.ts

@@ -37,6 +37,8 @@ namespace ts {
             resumeLexicalEnvironment,
             endLexicalEnvironment,
             hoistVariableDeclaration,
+            startBlockScope,
+            endBlockScope
         } = context;
 
         const resolver = context.getEmitResolver();
@@ -208,7 +210,10 @@ namespace ts {
          * @param node The node to visit.
          */
         function visitorWorker(node: Node): VisitResult<Node> {
-            if (node.transformFlags & TransformFlags.TypeScript) {
+            if (node.kind === SyntaxKind.Block && node.transformFlags & TransformFlags.AssertTypeScript) {
+                return visitBlock(node as Block);
+            }
+            else if (node.transformFlags & TransformFlags.TypeScript) {
                 // This node is explicitly marked as TypeScript, so we should transform the node.
                 return visitTypeScript(node);
             }
@@ -560,6 +565,19 @@ namespace ts {
             }
         }
 
+        function visitBlock(node: Block): Block {
+            startBlockScope();
+            node = visitEachChild(node, visitor, context);
+            const declarations = endBlockScope();
+            if (some(declarations)) {
+                return updateBlock(
+                    node,
+                    mergeLexicalEnvironment(node.statements, declarations)
+                );
+            }
+            return node;
+        }
+
         function visitSourceFile(node: SourceFile) {
             const alwaysStrict = getStrictOptionValue(compilerOptions, "alwaysStrict") &&
                 !(isExternalModule(node) && moduleKind >= ModuleKind.ES2015) &&
@@ -921,7 +939,13 @@ namespace ts {
             if (some(staticProperties) || some(pendingExpressions)) {
                 const expressions: Expression[] = [];
                 const isClassWithConstructorReference = resolver.getNodeCheckFlags(node) & NodeCheckFlags.ClassWithConstructorReference;
-                const temp = createTempVariable(hoistVariableDeclaration, !!isClassWithConstructorReference);
+                const temp = createTempVariable(
+                    name => {
+                        setOriginalNode(name, node);
+                        hoistVariableDeclaration(name);
+                    },
+                    !!isClassWithConstructorReference
+                );
                 if (isClassWithConstructorReference) {
                     // record an alias as the class name is not in scope for statics.
                     enableSubstitutionForClassAliases();
@@ -2191,6 +2215,7 @@ namespace ts {
                 const inlinable = isSimpleInlineableExpression(innerExpression);
                 if (!inlinable && shouldHoist) {
                     const generatedName = getGeneratedNameForNode(name);
+                    setOriginalNode(generatedName, name);
                     hoistVariableDeclaration(generatedName);
                     return createAssignment(generatedName, expression);
                 }

src/compiler/types.ts

@@ -5266,6 +5266,12 @@ namespace ts {
         /** Hoists a variable declaration to the containing scope. */
         hoistVariableDeclaration(node: Identifier): void;
 
+        /* @internal */
+        startBlockScope(): void;
+
+        /* @internal */
+        endBlockScope(): Statement[] | undefined;
+
         /** Records a request for a non-scoped emit helper in the current context. */
         requestEmitHelper(helper: EmitHelper): void;
 

tests/baselines/reference/classExpressionWithStaticProperties3.js

@@ -10,9 +10,9 @@ for (let i = 0; i < 3; i++) {
 arr.forEach(C => console.log(C.y()));
 
 //// [classExpressionWithStaticProperties3.js]
-var _a;
 var arr = [];
 var _loop_1 = function (i) {
+    var _a = void 0;
     arr.push((_a = /** @class */ (function () {
             function C() {
             }

tests/baselines/reference/classExpressionWithStaticPropertiesES63.js

@@ -10,9 +10,9 @@ for (let i = 0; i < 3; i++) {
 arr.forEach(C => console.log(C.y()));
 
 //// [classExpressionWithStaticPropertiesES63.js]
-var _a;
 const arr = [];
 for (let i = 0; i < 3; i++) {
+    let _a;
     arr.push((_a = class C {
         },
         _a.x = i,

tests/baselines/reference/computedPropertyNames52_ES5.errors.txt

@@ -0,0 +1,17 @@
+tests/cases/conformance/es6/computedProperties/computedPropertyNames52_ES5.ts(5,13): error TS1166: A computed property name in a class property declaration must refer to an expression whose type is a literal type or a 'unique symbol' type.
+
+
+==== tests/cases/conformance/es6/computedProperties/computedPropertyNames52_ES5.ts (1 errors) ====
+    const classes = [];
+    for (let i = 0; i <= 10; ++i) {
+        classes.push(
+            class A {
+                [i] = "my property";
+                ~~~
+!!! error TS1166: A computed property name in a class property declaration must refer to an expression whose type is a literal type or a 'unique symbol' type.
+            }
+        );
+    }
+    for (const clazz of classes) {
+        console.log(Object.getOwnPropertyNames(new clazz()));
+    }

tests/baselines/reference/computedPropertyNames52_ES5.js

@@ -0,0 +1,33 @@
+//// [computedPropertyNames52_ES5.ts]
+const classes = [];
+for (let i = 0; i <= 10; ++i) {
+    classes.push(
+        class A {
+            [i] = "my property";
+        }
+    );
+}
+for (const clazz of classes) {
+    console.log(Object.getOwnPropertyNames(new clazz()));
+}
+
+//// [computedPropertyNames52_ES5.js]
+var classes = [];
+var _loop_1 = function (i) {
+    var _a = void 0, _b = void 0;
+    classes.push((_b = /** @class */ (function () {
+            function A() {
+                this[_a] = "my property";
+            }
+            return A;
+        }()),
+        _a = i,
+        _b));
+};
+for (var i = 0; i <= 10; ++i) {
+    _loop_1(i);
+}
+for (var _i = 0, classes_1 = classes; _i < classes_1.length; _i++) {
+    var clazz = classes_1[_i];
+    console.log(Object.getOwnPropertyNames(new clazz()));
+}

tests/baselines/reference/computedPropertyNames52_ES5.symbols

@@ -0,0 +1,36 @@
+=== tests/cases/conformance/es6/computedProperties/computedPropertyNames52_ES5.ts ===
+const classes = [];
+>classes : Symbol(classes, Decl(computedPropertyNames52_ES5.ts, 0, 5))
+
+for (let i = 0; i <= 10; ++i) {
+>i : Symbol(i, Decl(computedPropertyNames52_ES5.ts, 1, 8))
+>i : Symbol(i, Decl(computedPropertyNames52_ES5.ts, 1, 8))
+>i : Symbol(i, Decl(computedPropertyNames52_ES5.ts, 1, 8))
+
+    classes.push(
+>classes.push : Symbol(Array.push, Decl(lib.es5.d.ts, --, --))
+>classes : Symbol(classes, Decl(computedPropertyNames52_ES5.ts, 0, 5))
+>push : Symbol(Array.push, Decl(lib.es5.d.ts, --, --))
+
+        class A {
+>A : Symbol(A, Decl(computedPropertyNames52_ES5.ts, 2, 17))
+
+            [i] = "my property";
+>[i] : Symbol(A[i], Decl(computedPropertyNames52_ES5.ts, 3, 17))
+>i : Symbol(i, Decl(computedPropertyNames52_ES5.ts, 1, 8))
+        }
+    );
+}
+for (const clazz of classes) {
+>clazz : Symbol(clazz, Decl(computedPropertyNames52_ES5.ts, 8, 10))
+>classes : Symbol(classes, Decl(computedPropertyNames52_ES5.ts, 0, 5))
+
+    console.log(Object.getOwnPropertyNames(new clazz()));
+>console.log : Symbol(Console.log, Decl(lib.dom.d.ts, --, --))
+>console : Symbol(console, Decl(lib.dom.d.ts, --, --))
+>log : Symbol(Console.log, Decl(lib.dom.d.ts, --, --))
+>Object.getOwnPropertyNames : Symbol(ObjectConstructor.getOwnPropertyNames, Decl(lib.es5.d.ts, --, --))
+>Object : Symbol(Object, Decl(lib.es5.d.ts, --, --), Decl(lib.es5.d.ts, --, --))
+>getOwnPropertyNames : Symbol(ObjectConstructor.getOwnPropertyNames, Decl(lib.es5.d.ts, --, --))
+>clazz : Symbol(clazz, Decl(computedPropertyNames52_ES5.ts, 8, 10))
+}