Merge pull request #2309 from Microsoft/recursiveLetConst

disallow recursive references for block-scoped bindings

Author: vladima

src/compiler/checker.ts

@@ -435,18 +435,67 @@ module ts {
                     return undefined;
                 }
                 if (result.flags & SymbolFlags.BlockScopedVariable) {
-                    // Block-scoped variables cannot be used before their definition
-                    var declaration = forEach(result.declarations, d => isBlockOrCatchScoped(d) ? d : undefined);
-
-                    Debug.assert(declaration !== undefined, "Block-scoped variable declaration is undefined");
-                    if (!isDefinedBefore(declaration, errorLocation)) {
-                        error(errorLocation, Diagnostics.Block_scoped_variable_0_used_before_its_declaration, declarationNameToString(declaration.name));
-                    }
+                    checkResolvedBlockScopedVariable(result, errorLocation);
                 }
             }
             return result;
         }
 
+        function checkResolvedBlockScopedVariable(result: Symbol, errorLocation: Node): void {
+            Debug.assert((result.flags & SymbolFlags.BlockScopedVariable) !== 0)
+            // Block-scoped variables cannot be used before their definition
+            var declaration = forEach(result.declarations, d => isBlockOrCatchScoped(d) ? d : undefined);
+
+            Debug.assert(declaration !== undefined, "Block-scoped variable declaration is undefined");
+                    
+            // first check if usage is lexically located after the declaration
+            var isUsedBeforeDeclaration = !isDefinedBefore(declaration, errorLocation);
+            if (!isUsedBeforeDeclaration) {
+                // lexical check succeeded however code still can be illegal.
+                // - block scoped variables cannot be used in its initializers
+                //   let x = x; // illegal but usage is lexically after definition
+                // - in ForIn/ForOf statements variable cannot be contained in expression part
+                //   for (let x in x)
+                //   for (let x of x)
+
+                // climb up to the variable declaration skipping binding patterns
+                var variableDeclaration = <VariableDeclaration>getAncestor(declaration, SyntaxKind.VariableDeclaration);
+                var container = getEnclosingBlockScopeContainer(variableDeclaration);
+
+                if (variableDeclaration.parent.parent.kind === SyntaxKind.VariableStatement ||
+                    variableDeclaration.parent.parent.kind === SyntaxKind.ForStatement) {
+                    // variable statement/for statement case, 
+                    // use site should not be inside variable declaration (initializer of declaration or binding element)
+                    isUsedBeforeDeclaration = isSameScopeDescendentOf(errorLocation, variableDeclaration, container);
+                }
+                else if (variableDeclaration.parent.parent.kind === SyntaxKind.ForOfStatement ||
+                    variableDeclaration.parent.parent.kind === SyntaxKind.ForInStatement) {
+                    // ForIn/ForOf case - use site should not be used in expression part
+                    var expression = (<ForInStatement | ForOfStatement>variableDeclaration.parent.parent).expression;
+                    isUsedBeforeDeclaration = isSameScopeDescendentOf(errorLocation, expression, container);
+                }
+            }
+            if (isUsedBeforeDeclaration) {
+                error(errorLocation, Diagnostics.Block_scoped_variable_0_used_before_its_declaration, declarationNameToString(declaration.name));
+            }
+        }
+
+        /* Starting from 'initial' node walk up the parent chain until 'stopAt' node is reached.
+         * If at any point current node is equal to 'parent' node - return true.
+         * Return false if 'stopAt' node is reached or isFunctionLike(current) === true.
+         */
+        function isSameScopeDescendentOf(initial: Node, parent: Node, stopAt: Node): boolean {
+            if (!parent) {
+                return false;
+            }
+            for (var current = initial; current && current !== stopAt && !isFunctionLike(current); current = current.parent) {
+                if (current === parent) {
+                    return true;
+                }
+            }
+            return false;
+        }
+
         // An alias symbol is created by one of the following declarations:
         // import <symbol> = ...
         // import <symbol> from ...

src/compiler/emitter.ts

@@ -4116,34 +4116,6 @@ module ts {
                 }
             }
 
-            function getEnclosingBlockScopeContainer(node: Node): Node {
-                var current = node;
-                while (current) {
-                    if (isFunctionLike(current)) {
-                        return current;
-                    }
-                    switch (current.kind) {
-                        case SyntaxKind.SourceFile:
-                        case SyntaxKind.CaseBlock:
-                        case SyntaxKind.CatchClause:
-                        case SyntaxKind.ModuleDeclaration:
-                        case SyntaxKind.ForStatement:
-                        case SyntaxKind.ForInStatement:
-                        case SyntaxKind.ForOfStatement:
-                            return current;
-                        case SyntaxKind.Block:
-                            // function block is not considered block-scope container
-                            // see comment in binder.ts: bind(...), case for SyntaxKind.Block
-                            if (!isFunctionLike(current.parent)) {
-                                return current;
-                            }
-                    }
-
-                    current = current.parent;
-                }
-            }
-            
-
             function getCombinedFlagsForIdentifier(node: Identifier): NodeFlags {
                 if (!node.parent || (node.parent.kind !== SyntaxKind.VariableDeclaration && node.parent.kind !== SyntaxKind.BindingElement)) {
                     return 0;

src/compiler/utilities.ts

@@ -203,6 +203,33 @@ module ts {
             isCatchClauseVariableDeclaration(declaration);
     }
 
+    export function getEnclosingBlockScopeContainer(node: Node): Node {
+        var current = node;
+        while (current) {
+            if (isFunctionLike(current)) {
+                return current;
+            }
+            switch (current.kind) {
+                case SyntaxKind.SourceFile:
+                case SyntaxKind.CaseBlock:
+                case SyntaxKind.CatchClause:
+                case SyntaxKind.ModuleDeclaration:
+                case SyntaxKind.ForStatement:
+                case SyntaxKind.ForInStatement:
+                case SyntaxKind.ForOfStatement:
+                    return current;
+                case SyntaxKind.Block:
+                    // function block is not considered block-scope container
+                    // see comment in binder.ts: bind(...), case for SyntaxKind.Block
+                    if (!isFunctionLike(current.parent)) {
+                        return current;
+                    }
+            }
+
+            current = current.parent;
+        }
+    }
+
     export function isCatchClauseVariableDeclaration(declaration: Declaration) {
         return declaration &&
             declaration.kind === SyntaxKind.VariableDeclaration &&

tests/baselines/reference/for-of55.errors.txt

@@ -0,0 +1,10 @@
+tests/cases/conformance/es6/for-ofStatements/for-of55.ts(2,15): error TS2448: Block-scoped variable 'v' used before its declaration.
+
+
+==== tests/cases/conformance/es6/for-ofStatements/for-of55.ts (1 errors) ====
+    let v = [1];
+    for (let v of v) {
+                  ~
+!!! error TS2448: Block-scoped variable 'v' used before its declaration.
+        v;
+    }

tests/baselines/reference/for-of55.types

@@ -0,0 +1,47 @@
+tests/cases/compiler/recursiveLetConst.ts(2,9): error TS2448: Block-scoped variable 'x' used before its declaration.
+tests/cases/compiler/recursiveLetConst.ts(3,12): error TS2448: Block-scoped variable 'x1' used before its declaration.
+tests/cases/compiler/recursiveLetConst.ts(4,11): error TS2448: Block-scoped variable 'y' used before its declaration.
+tests/cases/compiler/recursiveLetConst.ts(5,14): error TS2448: Block-scoped variable 'y1' used before its declaration.
+tests/cases/compiler/recursiveLetConst.ts(6,14): error TS2448: Block-scoped variable 'v' used before its declaration.
+tests/cases/compiler/recursiveLetConst.ts(7,16): error TS2448: Block-scoped variable 'v' used before its declaration.
+tests/cases/compiler/recursiveLetConst.ts(8,15): error TS2448: Block-scoped variable 'v' used before its declaration.
+tests/cases/compiler/recursiveLetConst.ts(9,15): error TS2448: Block-scoped variable 'v' used before its declaration.
+tests/cases/compiler/recursiveLetConst.ts(10,17): error TS2448: Block-scoped variable 'v' used before its declaration.
+tests/cases/compiler/recursiveLetConst.ts(11,11): error TS2448: Block-scoped variable 'x2' used before its declaration.
+
+
+==== tests/cases/compiler/recursiveLetConst.ts (10 errors) ====
+    'use strict'
+    let x = x + 1;
+            ~
+!!! error TS2448: Block-scoped variable 'x' used before its declaration.
+    let [x1] = x1 + 1;
+               ~~
+!!! error TS2448: Block-scoped variable 'x1' used before its declaration.
+    const y = y + 2;
+              ~
+!!! error TS2448: Block-scoped variable 'y' used before its declaration.
+    const [y1] = y1 + 1;
+                 ~~
+!!! error TS2448: Block-scoped variable 'y1' used before its declaration.
+    for (let v = v; ; ) { }
+                 ~
+!!! error TS2448: Block-scoped variable 'v' used before its declaration.
+    for (let [v] = v; ;) { }
+                   ~
+!!! error TS2448: Block-scoped variable 'v' used before its declaration.
+    for (let v in v) { }
+                  ~
+!!! error TS2448: Block-scoped variable 'v' used before its declaration.
+    for (let v of v) { }
+                  ~
+!!! error TS2448: Block-scoped variable 'v' used before its declaration.
+    for (let [v] of v) { }
+                    ~
+!!! error TS2448: Block-scoped variable 'v' used before its declaration.
+    let [x2 = x2] = []
+              ~~
+!!! error TS2448: Block-scoped variable 'x2' used before its declaration.
+    let z0 = () => z0;
+    let z1 = function () { return z1; }
+    let z2 = { f() { return z2;}}

tests/baselines/reference/recursiveLetConst.errors.txt

@@ -0,0 +1,42 @@
+//// [recursiveLetConst.ts]
+'use strict'
+let x = x + 1;
+let [x1] = x1 + 1;
+const y = y + 2;
+const [y1] = y1 + 1;
+for (let v = v; ; ) { }
+for (let [v] = v; ;) { }
+for (let v in v) { }
+for (let v of v) { }
+for (let [v] of v) { }
+let [x2 = x2] = []
+let z0 = () => z0;
+let z1 = function () { return z1; }
+let z2 = { f() { return z2;}}
+
+//// [recursiveLetConst.js]
+'use strict';
+let x = x + 1;
+let [x1] = x1 + 1;
+const y = y + 2;
+const [y1] = y1 + 1;
+for (let v = v;;) {
+}
+for (let [v] = v;;) {
+}
+for (let v in v) {
+}
+for (let v of v) {
+}
+for (let [v] of v) {
+}
+let [x2 = x2] = [];
+let z0 = () => z0;
+let z1 = function () {
+    return z1;
+};
+let z2 = {
+    f() {
+        return z2;
+    }
+};

tests/baselines/reference/recursiveLetConst.js

@@ -0,0 +1,15 @@
+// @target:es6
+'use strict'
+let x = x + 1;
+let [x1] = x1 + 1;
+const y = y + 2;
+const [y1] = y1 + 1;
+for (let v = v; ; ) { }
+for (let [v] = v; ;) { }
+for (let v in v) { }
+for (let v of v) { }
+for (let [v] of v) { }
+let [x2 = x2] = []
+let z0 = () => z0;
+let z1 = function () { return z1; }
+let z2 = { f() { return z2;}}