Code review feedback

This commit is contained in:
Sheetal Nandi 2014-11-12 15:40:45 -08:00
parent 0fdb0fa2d4
commit 5e94f4a5c2
6 changed files with 36 additions and 29 deletions

View file

@ -18,7 +18,7 @@ module ts {
return ModuleInstanceState.NonInstantiated;
}
// 2. const enum declarations don't make module instantiated
else if (node.kind === SyntaxKind.EnumDeclaration && isConstEnumDeclaration(<EnumDeclaration>node)) {
else if (isConstEnumDeclaration(node)) {
return ModuleInstanceState.ConstEnumOnly;
}
// 3. non - exported import declarations
@ -402,7 +402,7 @@ module ts {
bindDeclaration(<Declaration>node, SymbolFlags.TypeAlias, SymbolFlags.TypeAliasExcludes, /*isBlockScopeContainer*/ false);
break;
case SyntaxKind.EnumDeclaration:
if (isConstEnumDeclaration(<EnumDeclaration>node)) {
if (isConst(node)) {
bindDeclaration(<Declaration>node, SymbolFlags.ConstEnum, SymbolFlags.ConstEnumExcludes, /*isBlockScopeContainer*/ false);
}
else {

View file

@ -7940,7 +7940,7 @@ module ts {
var enumType = getDeclaredTypeOfSymbol(enumSymbol);
var autoValue = 0;
var ambient = isInAmbientContext(node);
var enumIsConst = isConstEnumDeclaration(node);
var enumIsConst = isConst(node);
forEach(node.members, member => {
// TODO(jfreeman): Check that it is not a computed name
@ -8111,10 +8111,10 @@ module ts {
var firstDeclaration = getDeclarationOfKind(enumSymbol, node.kind);
if (node === firstDeclaration) {
if (enumSymbol.declarations.length > 1) {
var enumIsConst = isConstEnumDeclaration(node);
var enumIsConst = isConst(node);
// check that const is placed\omitted on all enum declarations
forEach(enumSymbol.declarations, decl => {
if (isConstEnumDeclaration(<EnumDeclaration>decl) !== enumIsConst) {
if (isConstEnumDeclaration(decl) !== enumIsConst) {
error(decl.name, Diagnostics.Enum_declarations_must_all_be_const_or_non_const);
}
});

View file

@ -1271,10 +1271,10 @@ module ts {
write(" ");
endPos = emitToken(SyntaxKind.OpenParenToken, endPos);
if (node.declarations) {
if (node.declarations[0] && node.declarations[0].flags & NodeFlags.Let) {
if (node.declarations[0] && isLet(node.declarations[0])) {
emitToken(SyntaxKind.LetKeyword, endPos);
}
else if (node.declarations[0] && node.declarations[0].flags & NodeFlags.Const) {
else if (node.declarations[0] && isConst(node.declarations[0])) {
emitToken(SyntaxKind.ConstKeyword, endPos);
}
else {
@ -1299,7 +1299,7 @@ module ts {
write(" ");
endPos = emitToken(SyntaxKind.OpenParenToken, endPos);
if (node.declaration) {
if (node.declaration.flags & NodeFlags.Let) {
if (isLet(node.declaration)) {
emitToken(SyntaxKind.LetKeyword, endPos);
}
else {
@ -1445,10 +1445,10 @@ module ts {
function emitVariableStatement(node: VariableStatement) {
emitLeadingComments(node);
if (!(node.flags & NodeFlags.Export)) {
if (node.flags & NodeFlags.Let) {
if (isLet(node)) {
write("let ");
}
else if (node.flags & NodeFlags.Const) {
else if (isConst(node)) {
write("const ");
}
else {
@ -1901,7 +1901,7 @@ module ts {
function emitEnumDeclaration(node: EnumDeclaration) {
// const enums are completely erased during compilation.
var isConstEnum = isConstEnumDeclaration(node);
var isConstEnum = isConst(node);
if (isConstEnum && !compilerOptions.preserveConstEnums) {
return;
}
@ -2754,7 +2754,7 @@ module ts {
if (resolver.isDeclarationVisible(node)) {
emitJsDocComments(node);
emitDeclarationFlags(node);
if (isConstEnumDeclaration(node)) {
if (isConst(node)) {
write("const ")
}
write("enum ");
@ -3041,10 +3041,10 @@ module ts {
if (hasDeclarationWithEmit) {
emitJsDocComments(node);
emitDeclarationFlags(node);
if (node.flags & NodeFlags.Let) {
if (isLet(node)) {
write("let ");
}
else if (node.flags & NodeFlags.Const) {
else if (isConst(node)) {
write("const ");
}
else {

View file

@ -116,8 +116,16 @@ module ts {
return (file.flags & NodeFlags.DeclarationFile) !== 0;
}
export function isConstEnumDeclaration(node: EnumDeclaration): boolean {
return (node.flags & NodeFlags.Const) !== 0;
export function isConstEnumDeclaration(node: Declaration): boolean {
return node.kind === SyntaxKind.EnumDeclaration && !!(node.flags & NodeFlags.Const);
}
export function isConst(node: Declaration): boolean {
return !!(node.flags & NodeFlags.Const);
}
export function isLet(node: Declaration): boolean {
return !!(node.flags & NodeFlags.Let);
}
export function isPrologueDirective(node: Node): boolean {
@ -3480,18 +3488,18 @@ module ts {
grammarErrorOnNode(node, Diagnostics.Variable_declaration_list_cannot_be_empty);
}
if (languageVersion < ScriptTarget.ES6) {
if (node.flags & NodeFlags.Let) {
if (isLet(node)) {
grammarErrorOnNode(node, Diagnostics.let_declarations_are_only_available_when_targeting_ECMAScript_6_and_higher);
}
else if (node.flags & NodeFlags.Const) {
else if (isConst(node)) {
grammarErrorOnNode(node, Diagnostics.const_declarations_are_only_available_when_targeting_ECMAScript_6_and_higher);
}
}
else if (!allowLetAndConstDeclarations) {
if (node.flags & NodeFlags.Let) {
if (isLet(node)) {
grammarErrorOnNode(node, Diagnostics.let_declarations_can_only_be_declared_inside_a_block);
}
else if (node.flags & NodeFlags.Const) {
else if (isConst(node)) {
grammarErrorOnNode(node, Diagnostics.const_declarations_can_only_be_declared_inside_a_block);
}
}

View file

@ -235,7 +235,7 @@ module ts.NavigationBar {
return createItem(node, getTextOfNode((<FunctionLikeDeclaration>node).name), ts.ScriptElementKind.functionElement);
case SyntaxKind.VariableDeclaration:
if (node.flags & NodeFlags.Const) {
if (isConst(node)) {
return createItem(node, getTextOfNode((<VariableDeclaration>node).name), ts.ScriptElementKind.constantElement);
}
else if (node.flags & NodeFlags.Let) {

View file

@ -2859,7 +2859,7 @@ module ts {
if (isFirstDeclarationOfSymbolParameter(symbol)) {
return ScriptElementKind.parameterElement;
}
else if (symbol.valueDeclaration && symbol.valueDeclaration.flags & NodeFlags.Const) {
else if (symbol.valueDeclaration && isConst(symbol.valueDeclaration)) {
return ScriptElementKind.constantElement;
}
else if (forEach(symbol.declarations, declaration => declaration.flags & NodeFlags.Let)) {
@ -2920,11 +2920,11 @@ module ts {
case SyntaxKind.InterfaceDeclaration: return ScriptElementKind.interfaceElement;
case SyntaxKind.TypeAliasDeclaration: return ScriptElementKind.typeElement;
case SyntaxKind.EnumDeclaration: return ScriptElementKind.enumElement;
case SyntaxKind.VariableDeclaration: return node.flags & NodeFlags.Const ?
ScriptElementKind.constantElement :
node.flags & NodeFlags.Let ?
ScriptElementKind.letElement :
ScriptElementKind.variableElement;
case SyntaxKind.VariableDeclaration: return isConst(node)
? ScriptElementKind.constantElement
: node.flags & NodeFlags.Let
? ScriptElementKind.letElement
: ScriptElementKind.variableElement;
case SyntaxKind.FunctionDeclaration: return ScriptElementKind.functionElement;
case SyntaxKind.GetAccessor: return ScriptElementKind.memberGetAccessorElement;
case SyntaxKind.SetAccessor: return ScriptElementKind.memberSetAccessorElement;
@ -3099,8 +3099,7 @@ module ts {
}
if (symbolFlags & SymbolFlags.Enum) {
addNewLineIfDisplayPartsExist();
if (forEach(symbol.declarations, declaration =>
declaration.kind === SyntaxKind.EnumDeclaration && isConstEnumDeclaration(<EnumDeclaration>declaration))) {
if (forEach(symbol.declarations, declaration => isConstEnumDeclaration(declaration))) {
displayParts.push(keywordPart(SyntaxKind.ConstKeyword));
displayParts.push(spacePart());
}