Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify some places in the compiler where we have esoteric declarations. #2162

Merged
merged 7 commits into from
Feb 27, 2015
Merged

Simplify some places in the compiler where we have esoteric declarations. #2162

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
81b6588
Make isDeclaration return 'true' on FunctionExpressions.
CyrusNajmabadi Feb 27, 2015
7d6c0f0
Add ArrowFunction and sort cases.
CyrusNajmabadi Feb 27, 2015
11772e7
Simplify how catch clauses are represented in our AST.
CyrusNajmabadi Feb 27, 2015
9125aa5
Rename method.
CyrusNajmabadi Feb 27, 2015
e2d9ea5
Add test for catch variables with initializers.
CyrusNajmabadi Feb 27, 2015
27e8882
CR feedback.
CyrusNajmabadi Feb 27, 2015
7b7f497
Adding test.
CyrusNajmabadi Feb 27, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 2 additions & 9 deletions src/compiler/binder.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -342,14 +342,7 @@ module ts {
}

function bindCatchVariableDeclaration(node: CatchClause) {
var symbol = createSymbol(SymbolFlags.FunctionScopedVariable, node.name.text || "__missing");
addDeclarationToSymbol(symbol, node, SymbolFlags.FunctionScopedVariable);
var saveParent = parent;
var savedBlockScopeContainer = blockScopeContainer;
parent = blockScopeContainer = node;
forEachChild(node, bind);
parent = saveParent;
blockScopeContainer = savedBlockScopeContainer;
bindChildren(node, /*symbolKind:*/ 0, /*isBlockScopeContainer:*/ true);
}

function bindBlockScopedVariableDeclaration(node: Declaration) {
Expand Down Expand Up @@ -390,7 +383,7 @@ module ts {
if (isBindingPattern((<Declaration>node).name)) {
bindChildren(node, 0, /*isBlockScopeContainer*/ false);
}
else if (getCombinedNodeFlags(node) & NodeFlags.BlockScoped) {
else if (isBlockOrCatchScoped(<Declaration>node)) {
bindBlockScopedVariableDeclaration(<Declaration>node);
}
else {
Expand Down
61 changes: 32 additions & 29 deletions src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -416,13 +416,6 @@ module ts {
break loop;
}
break;
case SyntaxKind.CatchClause:
var id = (<CatchClause>location).name;
if (name === id.text) {
result = location.symbol;
break loop;
}
break;
}
lastLocation = location;
location = location.parent;
Expand Down Expand Up @@ -451,7 +444,8 @@ module ts {
}
if (result.flags & SymbolFlags.BlockScopedVariable) {
// Block-scoped variables cannot be used before their definition
var declaration = forEach(result.declarations, d => getCombinedNodeFlags(d) & NodeFlags.BlockScoped ? d : undefined);
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));
Expand Down Expand Up @@ -1994,7 +1988,7 @@ module ts {
}
// Handle catch clause variables
var declaration = symbol.valueDeclaration;
if (declaration.kind === SyntaxKind.CatchClause) {
if (declaration.parent.kind === SyntaxKind.CatchClause) {
return links.type = anyType;
}
// Handle variable, parameter or property
Expand Down Expand Up @@ -8926,18 +8920,29 @@ module ts {
var catchClause = node.catchClause;
if (catchClause) {
// Grammar checking
if (catchClause.type) {
var sourceFile = getSourceFileOfNode(node);
var colonStart = skipTrivia(sourceFile.text, catchClause.name.end);
grammarErrorAtPos(sourceFile, colonStart, ":".length, Diagnostics.Catch_clause_parameter_cannot_have_a_type_annotation);
if (catchClause.variableDeclaration) {
if (catchClause.variableDeclaration.name.kind !== SyntaxKind.Identifier) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add test

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

grammarErrorOnFirstToken(catchClause.variableDeclaration.name, Diagnostics.Catch_clause_variable_name_must_be_an_identifier);
}
else if (catchClause.variableDeclaration.type) {
grammarErrorOnFirstToken(catchClause.variableDeclaration.type, Diagnostics.Catch_clause_variable_cannot_have_a_type_annotation);
}
else if (catchClause.variableDeclaration.initializer) {
grammarErrorOnFirstToken(catchClause.variableDeclaration.initializer, Diagnostics.Catch_clause_variable_cannot_have_an_initializer);
}
else {
// It is a SyntaxError if a TryStatement with a Catch occurs within strict code and the Identifier of the
// Catch production is eval or arguments
checkGrammarEvalOrArgumentsInStrictMode(node, <Identifier>catchClause.variableDeclaration.name);
}
}
// It is a SyntaxError if a TryStatement with a Catch occurs within strict code and the Identifier of the
// Catch production is eval or arguments
checkGrammarEvalOrArgumentsInStrictMode(node, catchClause.name);

checkBlock(catchClause.block);
}
if (node.finallyBlock) checkBlock(node.finallyBlock);

if (node.finallyBlock) {
checkBlock(node.finallyBlock);
}
}

function checkIndexConstraints(type: Type) {
Expand Down Expand Up @@ -10049,11 +10054,6 @@ module ts {
copySymbol(location.symbol, meaning);
}
break;
case SyntaxKind.CatchClause:
if ((<CatchClause>location).name.text) {
copySymbol(location.symbol, meaning);
}
break;
}
memberFlags = location.flags;
location = location.parent;
Expand Down Expand Up @@ -10190,7 +10190,7 @@ module ts {
}

function getSymbolOfEntityNameOrPropertyAccessExpression(entityName: EntityName | PropertyAccessExpression): Symbol {
if (isDeclarationOrFunctionExpressionOrCatchVariableName(entityName)) {
if (isDeclarationName(entityName)) {
return getSymbolOfNode(entityName.parent);
}

Expand Down Expand Up @@ -10255,7 +10255,7 @@ module ts {
return undefined;
}

if (isDeclarationOrFunctionExpressionOrCatchVariableName(node)) {
if (isDeclarationName(node)) {
// This is a declaration, call getSymbolOfNode
return getSymbolOfNode(node.parent);
}
Expand Down Expand Up @@ -10351,7 +10351,7 @@ module ts {
return getTypeOfSymbol(symbol);
}

if (isDeclarationOrFunctionExpressionOrCatchVariableName(node)) {
if (isDeclarationName(node)) {
var symbol = getSymbolInfo(node);
return symbol && getTypeOfSymbol(symbol);
}
Expand Down Expand Up @@ -11616,10 +11616,13 @@ module ts {
}
}

function checkGrammarEvalOrArgumentsInStrictMode(contextNode: Node, identifier: Identifier): boolean {
if (contextNode && (contextNode.parserContextFlags & ParserContextFlags.StrictMode) && isEvalOrArgumentsIdentifier(identifier)) {
var name = declarationNameToString(identifier);
return grammarErrorOnNode(identifier, Diagnostics.Invalid_use_of_0_in_strict_mode, name);
function checkGrammarEvalOrArgumentsInStrictMode(contextNode: Node, name: Node): boolean {
if (name && name.kind === SyntaxKind.Identifier) {
var identifier = <Identifier>name;
if (contextNode && (contextNode.parserContextFlags & ParserContextFlags.StrictMode) && isEvalOrArgumentsIdentifier(identifier)) {
var nameText = declarationNameToString(identifier);
return grammarErrorOnNode(identifier, Diagnostics.Invalid_use_of_0_in_strict_mode, nameText);
}
}
}

Expand Down
4 changes: 3 additions & 1 deletion src/compiler/diagnosticInformationMap.generated.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ module ts {
Trailing_comma_not_allowed: { code: 1009, category: DiagnosticCategory.Error, key: "Trailing comma not allowed." },
Asterisk_Slash_expected: { code: 1010, category: DiagnosticCategory.Error, key: "'*/' expected." },
Unexpected_token: { code: 1012, category: DiagnosticCategory.Error, key: "Unexpected token." },
Catch_clause_parameter_cannot_have_a_type_annotation: { code: 1013, category: DiagnosticCategory.Error, key: "Catch clause parameter cannot have a type annotation." },
A_rest_parameter_must_be_last_in_a_parameter_list: { code: 1014, category: DiagnosticCategory.Error, key: "A rest parameter must be last in a parameter list." },
Parameter_cannot_have_question_mark_and_initializer: { code: 1015, category: DiagnosticCategory.Error, key: "Parameter cannot have question mark and initializer." },
A_required_parameter_cannot_follow_an_optional_parameter: { code: 1016, category: DiagnosticCategory.Error, key: "A required parameter cannot follow an optional parameter." },
Expand Down Expand Up @@ -153,6 +152,9 @@ module ts {
External_module_0_has_no_default_export_or_export_assignment: { code: 1192, category: DiagnosticCategory.Error, key: "External module '{0}' has no default export or export assignment." },
An_export_declaration_cannot_have_modifiers: { code: 1193, category: DiagnosticCategory.Error, key: "An export declaration cannot have modifiers." },
Export_declarations_are_not_permitted_in_an_internal_module: { code: 1194, category: DiagnosticCategory.Error, key: "Export declarations are not permitted in an internal module." },
Catch_clause_variable_name_must_be_an_identifier: { code: 1195, category: DiagnosticCategory.Error, key: "Catch clause variable name must be an identifier." },
Catch_clause_variable_cannot_have_a_type_annotation: { code: 1196, category: DiagnosticCategory.Error, key: "Catch clause variable cannot have a type annotation." },
Catch_clause_variable_cannot_have_an_initializer: { code: 1197, category: DiagnosticCategory.Error, key: "Catch clause variable cannot have an initializer." },
Duplicate_identifier_0: { code: 2300, category: DiagnosticCategory.Error, key: "Duplicate identifier '{0}'." },
Initializer_of_instance_member_variable_0_cannot_reference_identifier_1_declared_in_the_constructor: { code: 2301, category: DiagnosticCategory.Error, key: "Initializer of instance member variable '{0}' cannot reference identifier '{1}' declared in the constructor." },
Static_members_cannot_reference_class_type_parameters: { code: 2302, category: DiagnosticCategory.Error, key: "Static members cannot reference class type parameters." },
Expand Down
18 changes: 13 additions & 5 deletions src/compiler/diagnosticMessages.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,6 @@
"category": "Error",
"code": 1012
},
"Catch clause parameter cannot have a type annotation.": {
"category": "Error",
"code": 1013
},
"A rest parameter must be last in a parameter list.": {
"category": "Error",
"code": 1014
Expand Down Expand Up @@ -603,6 +599,18 @@
"category": "Error",
"code": 1194
},
"Catch clause variable name must be an identifier.": {
"category": "Error",
"code": 1195
},
"Catch clause variable cannot have a type annotation.": {
"category": "Error",
"code": 1196
},
"Catch clause variable cannot have an initializer.": {
"category": "Error",
"code": 1197
},

"Duplicate identifier '{0}'.": {
"category": "Error",
Expand Down Expand Up @@ -1572,7 +1580,7 @@
"Exported type alias '{0}' has or is using private name '{1}'.": {
"category": "Error",
"code": 4081
},
},
"The current host does not support the '{0}' option.": {
"category": "Error",
"code": 5001
Expand Down
6 changes: 2 additions & 4 deletions src/compiler/emitter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2461,8 +2461,6 @@ module ts {
return false;
case SyntaxKind.LabeledStatement:
return (<LabeledStatement>node.parent).label === node;
case SyntaxKind.CatchClause:
return (<CatchClause>node.parent).name === node;
}
}

Expand Down Expand Up @@ -3428,8 +3426,8 @@ module ts {
var endPos = emitToken(SyntaxKind.CatchKeyword, node.pos);
write(" ");
emitToken(SyntaxKind.OpenParenToken, endPos);
emit(node.name);
emitToken(SyntaxKind.CloseParenToken, node.name.end);
emit(node.variableDeclaration);
emitToken(SyntaxKind.CloseParenToken, node.variableDeclaration ? node.variableDeclaration.end : endPos);
write(" ");
emitBlock(node.block);
}
Expand Down
10 changes: 5 additions & 5 deletions src/compiler/parser.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -222,8 +222,7 @@ module ts {
visitNode(cbNode, (<TryStatement>node).catchClause) ||
visitNode(cbNode, (<TryStatement>node).finallyBlock);
case SyntaxKind.CatchClause:
return visitNode(cbNode, (<CatchClause>node).name) ||
visitNode(cbNode, (<CatchClause>node).type) ||
return visitNode(cbNode, (<CatchClause>node).variableDeclaration) ||
visitNode(cbNode, (<CatchClause>node).block);
case SyntaxKind.ClassDeclaration:
return visitNodes(cbNodes, node.modifiers) ||
Expand Down Expand Up @@ -3973,9 +3972,10 @@ module ts {
function parseCatchClause(): CatchClause {
var result = <CatchClause>createNode(SyntaxKind.CatchClause);
parseExpected(SyntaxKind.CatchKeyword);
parseExpected(SyntaxKind.OpenParenToken);
result.name = parseIdentifier();
result.type = parseTypeAnnotation();
if (parseExpected(SyntaxKind.OpenParenToken)) {
result.variableDeclaration = parseVariableDeclaration();
}

parseExpected(SyntaxKind.CloseParenToken);
result.block = parseBlock(/*ignoreMissingOpenBrace:*/ false, /*checkForStrictMode:*/ false);
return finishNode(result);
Expand Down
5 changes: 2 additions & 3 deletions src/compiler/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -814,9 +814,8 @@ module ts {
finallyBlock?: Block;
}

export interface CatchClause extends Declaration {
name: Identifier;
type?: TypeNode;
export interface CatchClause extends Node {
variableDeclaration: VariableDeclaration;
block: Block;
}

Expand Down
58 changes: 34 additions & 24 deletions src/compiler/utilities.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,18 @@ module ts {
return getBaseFileName(moduleName).replace(/\W/g, "_");
}

export function isBlockOrCatchScoped(declaration: Declaration) {
return (getCombinedNodeFlags(declaration) & NodeFlags.BlockScoped) !== 0 ||
isCatchClauseVariableDeclaration(declaration);
}

export function isCatchClauseVariableDeclaration(declaration: Declaration) {
return declaration &&
declaration.kind === SyntaxKind.VariableDeclaration &&
declaration.parent &&
declaration.parent.kind === SyntaxKind.CatchClause;
}

// Return display name of an identifier
// Computed property names will just be emitted as "[<expr>]", where <expr> is the source
// text of the expression in the computed property.
Expand Down Expand Up @@ -681,31 +693,33 @@ module ts {

export function isDeclaration(node: Node): boolean {
switch (node.kind) {
case SyntaxKind.TypeParameter:
case SyntaxKind.Parameter:
case SyntaxKind.VariableDeclaration:
case SyntaxKind.ArrowFunction:
case SyntaxKind.BindingElement:
case SyntaxKind.PropertyDeclaration:
case SyntaxKind.PropertySignature:
case SyntaxKind.PropertyAssignment:
case SyntaxKind.ShorthandPropertyAssignment:
case SyntaxKind.ClassDeclaration:
case SyntaxKind.Constructor:
case SyntaxKind.EnumDeclaration:
case SyntaxKind.EnumMember:
case SyntaxKind.MethodDeclaration:
case SyntaxKind.MethodSignature:
case SyntaxKind.ExportSpecifier:
case SyntaxKind.FunctionDeclaration:
case SyntaxKind.FunctionExpression:
case SyntaxKind.GetAccessor:
case SyntaxKind.SetAccessor:
case SyntaxKind.Constructor:
case SyntaxKind.ClassDeclaration:
case SyntaxKind.InterfaceDeclaration:
case SyntaxKind.TypeAliasDeclaration:
case SyntaxKind.EnumDeclaration:
case SyntaxKind.ModuleDeclaration:
case SyntaxKind.ImportEqualsDeclaration:
case SyntaxKind.ImportClause:
case SyntaxKind.ImportEqualsDeclaration:
case SyntaxKind.ImportSpecifier:
case SyntaxKind.InterfaceDeclaration:
case SyntaxKind.MethodDeclaration:
case SyntaxKind.MethodSignature:
case SyntaxKind.ModuleDeclaration:
case SyntaxKind.NamespaceImport:
case SyntaxKind.ExportSpecifier:
case SyntaxKind.Parameter:
case SyntaxKind.PropertyAssignment:
case SyntaxKind.PropertyDeclaration:
case SyntaxKind.PropertySignature:
case SyntaxKind.SetAccessor:
case SyntaxKind.ShorthandPropertyAssignment:
case SyntaxKind.TypeAliasDeclaration:
case SyntaxKind.TypeParameter:
case SyntaxKind.VariableDeclaration:
return true;
}
return false;
Expand Down Expand Up @@ -739,7 +753,7 @@ module ts {
}

// True if the given identifier, string literal, or number literal is the name of a declaration node
export function isDeclarationOrFunctionExpressionOrCatchVariableName(name: Node): boolean {
export function isDeclarationName(name: Node): boolean {
if (name.kind !== SyntaxKind.Identifier && name.kind !== SyntaxKind.StringLiteral && name.kind !== SyntaxKind.NumericLiteral) {
return false;
}
Expand All @@ -751,14 +765,10 @@ module ts {
}
}

if (isDeclaration(parent) || parent.kind === SyntaxKind.FunctionExpression) {
if (isDeclaration(parent)) {
return (<Declaration>parent).name === name;
}

if (parent.kind === SyntaxKind.CatchClause) {
return (<CatchClause>parent).name === name;
}

return false;
}

Expand Down
4 changes: 2 additions & 2 deletions src/services/services.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4756,7 +4756,7 @@ module ts {

/** A node is considered a writeAccess iff it is a name of a declaration or a target of an assignment */
function isWriteAccess(node: Node): boolean {
if (node.kind === SyntaxKind.Identifier && isDeclarationOrFunctionExpressionOrCatchVariableName(node)) {
if (node.kind === SyntaxKind.Identifier && isDeclarationName(node)) {
return true;
}

Expand Down Expand Up @@ -4918,7 +4918,7 @@ module ts {
else if (isInRightSideOfImport(node)) {
return getMeaningFromRightHandSideOfImportEquals(node);
}
else if (isDeclarationOrFunctionExpressionOrCatchVariableName(node)) {
else if (isDeclarationName(node)) {
return getMeaningFromDeclaration(node.parent);
}
else if (isTypeReference(node)) {
Expand Down
5 changes: 2 additions & 3 deletions tests/baselines/reference/APISample_compile.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -673,9 +673,8 @@ declare module "typescript" {
catchClause?: CatchClause;
finallyBlock?: Block;
}
interface CatchClause extends Declaration {
name: Identifier;
type?: TypeNode;
interface CatchClause extends Node {
variableDeclaration: VariableDeclaration;
block: Block;
}
interface ModuleElement extends Node {
Expand Down
Loading