Skip to content

Commit

Permalink
Addressing CR feedback:
Browse files Browse the repository at this point in the history 
New SyntaxKind.BindingElement
Introduced new VariableLikeDeclaration and BindingElement types
Cleaned up VariableDeclaration, ParameterDeclaration, PropertyDeclaration types
Node kind of binding element is always SyntaxKind.BindingElement
Changed CheckVariableDeclaration to CheckVariableLikeDeclaration
Reorganized CheckVariableLikeDeclaration
  • Loading branch information
@ahejlsberg
ahejlsberg committed Dec 6, 2014
1 parent 4118ffc commit 05c9966
Show file tree
Hide file tree
Showing 7 changed files with 231 additions and 156 deletions.
8 changes: 2 additions & 6 deletions src/compiler/binder.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -389,18 +389,14 @@ module ts {
break;
case SyntaxKind.Parameter:
if (isBindingPattern((<Declaration>node).name)) {
if (isBindingPattern(parent)) {
bindChildren(node, 0, /*isBlockScopeContainer*/ false);
}
else {
bindAnonymousDeclaration(<Declaration>node, SymbolFlags.FunctionScopedVariable, getDestructuringParameterName(<Declaration>node), /*isBlockScopeContainer*/ false);
}
bindAnonymousDeclaration(<Declaration>node, SymbolFlags.FunctionScopedVariable, getDestructuringParameterName(<Declaration>node), /*isBlockScopeContainer*/ false);
}
else {
bindDeclaration(<Declaration>node, SymbolFlags.FunctionScopedVariable, SymbolFlags.ParameterExcludes, /*isBlockScopeContainer*/ false);
}
break;
case SyntaxKind.VariableDeclaration:
case SyntaxKind.BindingElement:
if (isBindingPattern((<Declaration>node).name)) {
bindChildren(node, 0, /*isBlockScopeContainer*/ false);
}
Expand Down
228 changes: 133 additions & 95 deletions src/compiler/checker.ts
View file

Large diffs are not rendered by default.

13 changes: 7 additions & 6 deletions src/compiler/emitter.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -440,7 +440,7 @@ module ts {
handleSymbolAccessibilityError(resolver.isSymbolAccessible(symbol, enclosingDeclaration, meaning));
}

function writeTypeOfDeclaration(declaration: AccessorDeclaration | VariableOrParameterDeclaration, type: TypeNode | StringLiteralExpression, getSymbolAccessibilityDiagnostic: GetSymbolAccessibilityDiagnostic) {
function writeTypeOfDeclaration(declaration: AccessorDeclaration | VariableLikeDeclaration, type: TypeNode | StringLiteralExpression, getSymbolAccessibilityDiagnostic: GetSymbolAccessibilityDiagnostic) {
writer.getSymbolAccessibilityDiagnostic = getSymbolAccessibilityDiagnostic;
write(": ");
if (type) {
Expand Down Expand Up @@ -996,7 +996,7 @@ module ts {
}
}

function emitTypeOfVariableDeclarationFromTypeLiteral(node: VariableOrParameterDeclaration) {
function emitTypeOfVariableDeclarationFromTypeLiteral(node: VariableLikeDeclaration) {
// if this is property of type literal,
// or is parameter of method/call/construct/index signature of type literal
// emit only if type is specified
Expand Down Expand Up @@ -2149,6 +2149,7 @@ module ts {
switch (parent.kind) {
case SyntaxKind.Parameter:
case SyntaxKind.VariableDeclaration:
case SyntaxKind.BindingElement:
case SyntaxKind.Property:
case SyntaxKind.PropertyAssignment:
case SyntaxKind.ShorthandPropertyAssignment:
Expand Down Expand Up @@ -2761,10 +2762,10 @@ module ts {
emitEnd(node.name);
}

function emitDestructuring(root: BinaryExpression | BindingElement, value?: Expression) {
function emitDestructuring(root: BinaryExpression | VariableDeclaration | ParameterDeclaration, value?: Expression) {
var emitCount = 0;
// An exported declaration is actually emitted as an assignment (to a property on the module object), so
// temporary variables in an exported declaration need to have real declarations elsewhere.
// temporary variables in an exported declaration need to have real declarations elsewhere
var isDeclaration = (root.kind === SyntaxKind.VariableDeclaration && !(root.flags & NodeFlags.Export)) || root.kind === SyntaxKind.Parameter;
if (root.kind === SyntaxKind.BinaryExpression) {
emitAssignmentExpression(<BinaryExpression>root);
Expand All @@ -2777,8 +2778,8 @@ module ts {
if (emitCount++) {
write(", ");
}
if (name.parent && name.parent.kind === SyntaxKind.VariableDeclaration) {
emitModuleMemberName(<VariableDeclaration>name.parent);
if (name.parent && (name.parent.kind === SyntaxKind.VariableDeclaration || name.parent.kind === SyntaxKind.BindingElement)) {
emitModuleMemberName(<Declaration>name.parent);
}
else {
emit(name);
Expand Down
70 changes: 38 additions & 32 deletions src/compiler/parser.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ module ts {
// This list is a work in progress. Add missing node kinds to improve their error
// spans.
case SyntaxKind.VariableDeclaration:
case SyntaxKind.BindingElement:
case SyntaxKind.ClassDeclaration:
case SyntaxKind.InterfaceDeclaration:
case SyntaxKind.ModuleDeclaration:
Expand Down Expand Up @@ -216,13 +217,14 @@ module ts {
case SyntaxKind.PropertyAssignment:
case SyntaxKind.ShorthandPropertyAssignment:
case SyntaxKind.VariableDeclaration:
case SyntaxKind.BindingElement:
return children(node.modifiers) ||
child((<VariableDeclaration>node).propertyName) ||
child((<VariableDeclaration>node).dotDotDotToken) ||
child((<VariableDeclaration>node).name) ||
child((<VariableDeclaration>node).questionToken) ||
child((<VariableDeclaration>node).type) ||
child((<VariableDeclaration>node).initializer);
child((<VariableLikeDeclaration>node).propertyName) ||
child((<VariableLikeDeclaration>node).dotDotDotToken) ||
child((<VariableLikeDeclaration>node).name) ||
child((<VariableLikeDeclaration>node).questionToken) ||
child((<VariableLikeDeclaration>node).type) ||
child((<VariableLikeDeclaration>node).initializer);
case SyntaxKind.FunctionType:
case SyntaxKind.ConstructorType:
case SyntaxKind.CallSignature:
Expand Down Expand Up @@ -580,7 +582,8 @@ module ts {
case SyntaxKind.Property:
case SyntaxKind.EnumMember:
case SyntaxKind.PropertyAssignment:
return (<VariableDeclaration>parent).initializer === node;
case SyntaxKind.BindingElement:
return (<VariableLikeDeclaration>parent).initializer === node;
case SyntaxKind.ExpressionStatement:
case SyntaxKind.IfStatement:
case SyntaxKind.DoStatement:
Expand Down Expand Up @@ -679,6 +682,7 @@ module ts {
case SyntaxKind.TypeParameter:
case SyntaxKind.Parameter:
case SyntaxKind.VariableDeclaration:
case SyntaxKind.BindingElement:
case SyntaxKind.Property:
case SyntaxKind.PropertyAssignment:
case SyntaxKind.ShorthandPropertyAssignment:
Expand Down Expand Up @@ -1892,12 +1896,7 @@ module ts {
// [+GeneratorParameter]BindingIdentifier[Yield]Initializer[In]opt
// [~GeneratorParameter]BindingIdentifier[?Yield]Initializer[In, ?Yield]opt

var savedYieldContext = inYieldContext();
if (inGeneratorParameterContext()) {
setYieldContext(true);
}
node.name = parseIdentifierOrPattern(SyntaxKind.Parameter);
setYieldContext(savedYieldContext);
node.name = inGeneratorParameterContext() ? doInYieldContext(parseIdentifierOrPattern) : parseIdentifierOrPattern();

if (getFullWidth(node.name) === 0 && node.flags === 0 && isModifier(token)) {
// in cases like
Expand All @@ -1913,9 +1912,7 @@ module ts {

node.questionToken = parseOptionalToken(SyntaxKind.QuestionToken);
node.type = parseParameterType();
node.initializer = inGeneratorParameterContext()
? doOutsideOfYieldContext(parseParameterInitializer)
: parseParameterInitializer();
node.initializer = inGeneratorParameterContext() ? doOutsideOfYieldContext(parseParameterInitializer) : parseParameterInitializer();

// Do not check for initializers in an ambient context for parameters. This is not
// a grammar error because the grammar allows arbitrary call signatures in
Expand Down Expand Up @@ -3691,11 +3688,11 @@ module ts {

// DECLARATIONS

function parseBindingElement(kind: SyntaxKind, context: ParsingContext): BindingElement {
function parseBindingElement(context: ParsingContext): BindingElement {
if (context === ParsingContext.ArrayBindingElements && token === SyntaxKind.CommaToken) {
return <BindingElement>createNode(SyntaxKind.OmittedExpression);
}
var node = <BindingElement>createNode(kind);
var node = <BindingElement>createNode(SyntaxKind.BindingElement);
if (context === ParsingContext.ObjectBindingElements) {
// TODO(andersh): Handle computed properties
var id = parsePropertyName();
Expand All @@ -3705,32 +3702,32 @@ module ts {
else {
parseExpected(SyntaxKind.ColonToken);
node.propertyName = <Identifier>id;
node.name = parseIdentifierOrPattern(kind);
node.name = parseIdentifierOrPattern();
}
}
else {
node.name = parseIdentifierOrPattern(kind);
node.name = parseIdentifierOrPattern();
}
node.initializer = parseInitializer(/*inParameter*/ false);
return finishNode(node);
}

function parseBindingList(kind: SyntaxKind, context: ParsingContext): NodeArray<BindingElement> {
return parseDelimitedList(context, () => parseBindingElement(kind, context));
function parseBindingList(context: ParsingContext): NodeArray<BindingElement> {
return parseDelimitedList(context, () => parseBindingElement(context));
}

function parseObjectBindingPattern(kind: SyntaxKind): BindingPattern {
function parseObjectBindingPattern(): BindingPattern {
var node = <BindingPattern>createNode(SyntaxKind.ObjectBindingPattern);
parseExpected(SyntaxKind.OpenBraceToken);
node.elements = parseBindingList(kind, ParsingContext.ObjectBindingElements);
node.elements = parseBindingList(ParsingContext.ObjectBindingElements);
parseExpected(SyntaxKind.CloseBraceToken);
return finishNode(node);
}

function parseArrayBindingPattern(kind: SyntaxKind): BindingPattern {
function parseArrayBindingPattern(): BindingPattern {
var node = <BindingPattern>createNode(SyntaxKind.ArrayBindingPattern);
parseExpected(SyntaxKind.OpenBracketToken);
node.elements = parseBindingList(kind, ParsingContext.ArrayBindingElements);
node.elements = parseBindingList(ParsingContext.ArrayBindingElements);
parseExpected(SyntaxKind.CloseBracketToken);
return finishNode(node);
}
Expand All @@ -3739,19 +3736,19 @@ module ts {
return token === SyntaxKind.OpenBraceToken || token === SyntaxKind.OpenBracketToken || isIdentifier();
}

function parseIdentifierOrPattern(kind: SyntaxKind): Identifier | BindingPattern {
function parseIdentifierOrPattern(): Identifier | BindingPattern {
if (token === SyntaxKind.OpenBracketToken) {
return parseArrayBindingPattern(kind);
return parseArrayBindingPattern();
}
if (token === SyntaxKind.OpenBraceToken) {
return parseObjectBindingPattern(kind);
return parseObjectBindingPattern();
}
return parseIdentifier();
}

function parseVariableDeclaration(): VariableDeclaration {
var node = <VariableDeclaration>createNode(SyntaxKind.VariableDeclaration);
node.name = parseIdentifierOrPattern(SyntaxKind.VariableDeclaration);
node.name = parseIdentifierOrPattern();
node.type = parseTypeAnnotation();
node.initializer = parseInitializer(/*inParameter*/ false);
return finishNode(node);
Expand Down Expand Up @@ -4492,6 +4489,7 @@ module ts {

case SyntaxKind.EnumDeclaration: return checkEnumDeclaration(<EnumDeclaration>node);
case SyntaxKind.BinaryExpression: return checkBinaryExpression(<BinaryExpression>node);
case SyntaxKind.BindingElement: return checkBindingElement(<BindingElement>node);
case SyntaxKind.CatchClause: return checkCatchClause(<CatchClause>node);
case SyntaxKind.ClassDeclaration: return checkClassDeclaration(<ClassDeclaration>node);
case SyntaxKind.ComputedPropertyName: return checkComputedPropertyName(<ComputedPropertyName>node);
Expand Down Expand Up @@ -5594,14 +5592,22 @@ module ts {
return checkTypeArguments(node.typeArguments);
}

function checkBindingElement(node: BindingElement) {
if (node.parserContextFlags & ParserContextFlags.StrictMode && isEvalOrArgumentsIdentifier(node.name)) {
// It is a SyntaxError if a VariableDeclaration or VariableDeclarationNoIn occurs within strict code
// and its Identifier is eval or arguments
return reportInvalidUseInStrictMode(<Identifier>node.name);
}
}

function checkVariableDeclaration(node: VariableDeclaration) {
if (inAmbientContext) {
if (isBindingPattern(node.name)) {
return grammarErrorOnNode(node, Diagnostics.Destructuring_declarations_are_not_allowed_in_ambient_contexts);
}
if (node.initializer) {
var equalsPos = node.type ? skipTrivia(sourceText, node.type.end) : skipTrivia(sourceText, node.name.end);
return grammarErrorAtPos(equalsPos, "=".length, Diagnostics.Initializers_are_not_allowed_in_ambient_contexts);
// Error on equals token which immediate precedes the initializer
return grammarErrorAtPos(node.initializer.pos - 1, 1, Diagnostics.Initializers_are_not_allowed_in_ambient_contexts);
}
}
else {
Expand Down
Loading

1 comment on commit 05c9966

@CyrusNajmabadi
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks pretty good overall. Thanks for making these changes!

Please sign in to comment.