Respond to code review comments

microsoft · mhegazy · Oct 24, 2014 · Oct 11, 2014 · Oct 13, 2014 · Oct 13, 2014
commit 4ef68b9fb03c21e55d47230ac97b63ebd43bd42e
diff --git a/src/compiler/parser.ts b/src/compiler/parser.ts
@@ -2561,7 +2561,7 @@ module ts {
 
         // STATEMENTS
         function parseStatementAllowingLetDeclaration() {
-            return parseStatement(/*allowLetDeclarations*/ true);
+            return parseStatement(/*allowLetAndConstDeclarations*/ true);
         }
 
         function parseBlock(ignoreMissingOpenBrace: boolean, checkForStrictMode: boolean): Block {
@@ -2613,8 +2613,8 @@ module ts {
             parseExpected(SyntaxKind.OpenParenToken);
             node.expression = parseExpression();
             parseExpected(SyntaxKind.CloseParenToken);
-            node.thenStatement = parseStatement(/*allowLetDeclarations*/ false);
-            node.elseStatement = parseOptional(SyntaxKind.ElseKeyword) ? parseStatement(/*allowLetDeclarations*/ false) : undefined;
+            node.thenStatement = parseStatement(/*allowLetAndConstDeclarations*/ false);
+            node.elseStatement = parseOptional(SyntaxKind.ElseKeyword) ? parseStatement(/*allowLetAndConstDeclarations*/ false) : undefined;
             return finishNode(node);
         }
 
@@ -2624,7 +2624,7 @@ module ts {
 
             var saveInIterationStatement = inIterationStatement;
             inIterationStatement = ControlBlockContext.Nested;
-            node.statement = parseStatement(/*allowLetDeclarations*/ false);
+            node.statement = parseStatement(/*allowLetAndConstDeclarations*/ false);
             inIterationStatement = saveInIterationStatement;
 
             parseExpected(SyntaxKind.WhileKeyword);
@@ -2649,7 +2649,7 @@ module ts {
 
             var saveInIterationStatement = inIterationStatement;
             inIterationStatement = ControlBlockContext.Nested;
-            node.statement = parseStatement(/*allowLetDeclarations*/ false);
+            node.statement = parseStatement(/*allowLetAndConstDeclarations*/ false);
             inIterationStatement = saveInIterationStatement;
 
             return finishNode(node);
@@ -2722,7 +2722,7 @@ module ts {
 
             var saveInIterationStatement = inIterationStatement;
             inIterationStatement = ControlBlockContext.Nested;
-            forOrForInStatement.statement = parseStatement(/*allowLetDeclarations*/ false);
+            forOrForInStatement.statement = parseStatement(/*allowLetAndConstDeclarations*/ false);
             inIterationStatement = saveInIterationStatement;
 
             return finishNode(forOrForInStatement);
@@ -2833,7 +2833,7 @@ module ts {
             parseExpected(SyntaxKind.OpenParenToken);
             node.expression = parseExpression();
             parseExpected(SyntaxKind.CloseParenToken);
-            node.statement = parseStatement(/*allowLetDeclarations*/ false);
+            node.statement = parseStatement(/*allowLetAndConstDeclarations*/ false);
             node = finishNode(node);
             if (isInStrictMode) {
                 // Strict mode code may not include a WithStatement. The occurrence of a WithStatement in such 
@@ -2963,9 +2963,9 @@ module ts {
             return token === SyntaxKind.WhileKeyword || token === SyntaxKind.DoKeyword || token === SyntaxKind.ForKeyword;
         }
 
-        function parseStatementWithLabelSet(allowLetDeclarations: boolean): Statement {
+        function parseStatementWithLabelSet(allowLetAndConstDeclarations: boolean): Statement {
             labelledStatementInfo.pushCurrentLabelSet(isIterationStatementStart());
-            var statement = parseStatement(allowLetDeclarations);
+            var statement = parseStatement(allowLetAndConstDeclarations);
             labelledStatementInfo.pop();
             return statement;
         }
@@ -2974,7 +2974,7 @@ module ts {
             return isIdentifier() && lookAhead(() => nextToken() === SyntaxKind.ColonToken);
         }
 
-        function parseLabelledStatement(allowLetDeclarations: boolean): LabeledStatement {
+        function parseLabeledStatement(allowLetAndConstDeclarations: boolean): LabeledStatement {
             var node = <LabeledStatement>createNode(SyntaxKind.LabeledStatement);
             node.label = parseIdentifier();
             parseExpected(SyntaxKind.ColonToken);
@@ -2986,7 +2986,7 @@ module ts {
 
             // We only want to call parseStatementWithLabelSet when the label set is complete
             // Therefore, keep parsing labels until we know we're done.
-            node.statement = isLabel() ? parseLabelledStatement(allowLetDeclarations) : parseStatementWithLabelSet(allowLetDeclarations);
+            node.statement = isLabel() ? parseLabeledStatement(allowLetAndConstDeclarations) : parseStatementWithLabelSet(allowLetAndConstDeclarations);
             return finishNode(node);
         }
 
@@ -3052,14 +3052,14 @@ module ts {
             }
         }
 
-        function parseStatement(allowLetDeclarations: boolean): Statement {
+        function parseStatement(allowLetAndConstDeclarations: boolean): Statement {
             switch (token) {
                 case SyntaxKind.OpenBraceToken:
                     return parseBlock(/* ignoreMissingOpenBrace */ false, /*checkForStrictMode*/ false);
                 case SyntaxKind.VarKeyword:
                 case SyntaxKind.LetKeyword:
                 case SyntaxKind.ConstKeyword:
-                    return parseVariableStatement(allowLetDeclarations);
+                    return parseVariableStatement(allowLetAndConstDeclarations);
                 case SyntaxKind.FunctionKeyword:
                     return parseFunctionDeclaration();
                 case SyntaxKind.SemicolonToken:
@@ -3093,7 +3093,7 @@ module ts {
                     return parseDebuggerStatement();
                 default:
                     if (isLabel()) {
-                        return parseLabelledStatement(allowLetDeclarations);
+                        return parseLabeledStatement(allowLetAndConstDeclarations);
                     }
                     return parseExpressionStatement();
             }
@@ -3150,7 +3150,7 @@ module ts {
                 () => parseVariableDeclaration(flags, noIn), /*allowTrailingComma*/ false);
         }
 
-        function parseVariableStatement(allowLetDeclarations: boolean, pos?: number, flags?: NodeFlags): VariableStatement {
+        function parseVariableStatement(allowLetAndConstDeclarations: boolean, pos?: number, flags?: NodeFlags): VariableStatement {
             var node = <VariableStatement>createNode(SyntaxKind.VariableStatement, pos);
             if (flags) node.flags = flags;
             var errorCountBeforeVarStatement = file.syntacticErrors.length;
@@ -3178,7 +3178,7 @@ module ts {
                     grammarErrorOnNode(node, Diagnostics.const_declarations_are_only_available_when_targeting_ECMAScript_6_and_higher);
                 }
             }
-            else if (!allowLetDeclarations) {
+            else if (!allowLetAndConstDeclarations) {
                 if (node.flags & NodeFlags.Let) {
                     grammarErrorOnNode(node, Diagnostics.let_must_be_declared_inside_a_block);
                 }
@@ -3821,7 +3821,7 @@ module ts {
                 case SyntaxKind.VarKeyword:
                 case SyntaxKind.LetKeyword:
                 case SyntaxKind.ConstKeyword:
-                    result = parseVariableStatement(/*allowLetDeclarations*/ true, pos, flags);
+                    result = parseVariableStatement(/*allowLetAndConstDeclarations*/ true, pos, flags);
                     break;
                 case SyntaxKind.FunctionKeyword:
                     result = parseFunctionDeclaration(pos, flags);
@@ -3869,7 +3869,7 @@ module ts {
             var statementStart = scanner.getTokenPos();
             var statementFirstTokenLength = scanner.getTextPos() - statementStart;
             var errorCountBeforeStatement = file.syntacticErrors.length;
-            var statement = parseStatement(/*allowLetDeclarations*/ false);
+            var statement = parseStatement(/*allowLetAndConstDeclarations*/ false);
 
             if (inAmbientContext && file.syntacticErrors.length === errorCountBeforeStatement) {
                 grammarErrorAtPos(statementStart, statementFirstTokenLength, Diagnostics.Statements_are_not_allowed_in_ambient_contexts);

diff --git a/src/compiler/types.ts b/src/compiler/types.ts
@@ -783,7 +783,7 @@ module ts {
         Transient          = 0x04000000,  // Transient symbol (created during type check)
         Prototype          = 0x08000000,  // Prototype property (no source representation)
 
-        BlockScoped        = 0x10000000,
+        BlockScoped        = 0x10000000,  // A block-scoped declaration
 
         Value     = Variable | Property | EnumMember | Function | Class | Enum | ValueModule | Method | GetAccessor | SetAccessor | UnionProperty,
         Type      = Class | Interface | Enum | TypeLiteral | ObjectLiteral | TypeParameter,
@@ -793,8 +793,11 @@ module ts {
         Signature = CallSignature | ConstructSignature | IndexSignature,
 
         ParameterExcludes       = Value,
-        VariableExcludes        = (Value | BlockScoped) & ~Variable,
-        BlockScopedExcludes     = Value,
+        VariableExcludes        = (Value | BlockScoped) & ~Variable,    // Variables can be redeclared, but can not redeclare a block-scoped 
+                                                                        // declaration with the same name, or any other value that is not a
+                                                                        // variable, e.g. ValueModule or Class
+        BlockScopedExcludes     = Value,                                // Block-scoped declarations are not allowed to be re-declared
+                                                                        // they can not merge with anything in the value space
         PropertyExcludes        = Value,
         EnumMemberExcludes      = Value,
         FunctionExcludes        = Value & ~(Function | ValueModule),

diff --git a/src/harness/harness.ts b/src/harness/harness.ts
@@ -686,7 +686,7 @@ module Harness {
                                 } else if (setting.value.toLowerCase() === 'es5') {
                                     options.target = ts.ScriptTarget.ES5;
                                 } else if (setting.value.toLowerCase() === 'es6') {
-                                        options.target = ts.ScriptTarget.ES6;
+                                    options.target = ts.ScriptTarget.ES6;
                                 } else {
                                     throw new Error('Unknown compile target ' + setting.value);
                                 }

diff --git a/src/services/services.ts b/src/services/services.ts
@@ -2722,7 +2722,7 @@ module ts {
                 if (isFirstDeclarationOfSymbolParameter(symbol)) {
                     return ScriptElementKind.parameterElement;
                 }
-                else if(forEach(symbol.declarations, d => d.flags & NodeFlags.Const)) {
+                else if(symbol.valueDeclaration && symbol.valueDeclaration.flags & NodeFlags.Const) {
                     return ScriptElementKind.constantElement;
                 }
                 return isLocalVariableOrFunction(symbol) ? ScriptElementKind.localVariableElement : ScriptElementKind.variableElement;