Skip to content

Commit

Permalink
Types for decorators
Browse files Browse the repository at this point in the history
  • Loading branch information
rbuckton committed Mar 17, 2015
1 parent 42a0c34 commit 5db3b0d
Showing 1 changed file with 27 additions and 11 deletions.
38 changes: 27 additions & 11 deletions src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ module ts {
BarBarToken,
QuestionToken,
ColonToken,
AtToken,
// Assignments
EqualsToken,
PlusEqualsToken,
Expand Down Expand Up @@ -154,6 +155,7 @@ module ts {
// Signature elements
TypeParameter,
Parameter,
Decorator,
// TypeMember
PropertySignature,
PropertyDeclaration,
Expand Down Expand Up @@ -244,6 +246,7 @@ module ts {
ExportDeclaration,
NamedExports,
ExportSpecifier,
IncompleteDeclaration,

This comment has been minimized.

Copy link
@JsonFreeman

JsonFreeman Mar 18, 2015

Contributor

What's this?

This comment has been minimized.

Copy link
@rbuckton

rbuckton Mar 18, 2015

Author Member

This is due to the fact I don't want infinite lookahead when parsing a decorator, just to determine if what follows is a valid declaration. Without it, isDeclarationStart would see an @ token and assume a declaration follows and when the token after the decorator isn't a valid declaration we would fail in a number of places. Instead, if the thing that follows the decorator isn't a valid declaration, I attach the decorators to an IncompleteDeclaration token, which I then report as a grammar error during type check.

This comment has been minimized.

Copy link
@JsonFreeman

JsonFreeman Mar 18, 2015

Contributor

I see. I would have thought that isDeclarationStart would return true if it is an decorator or a declaration without a decorator. You could call it twice, once before parsing the decorator and once after. And if you don't have a valid declaration afterward, you can just parse that as a missing node.


// Module references
ExternalModuleReference,
Expand Down Expand Up @@ -327,22 +330,25 @@ module ts {
// If this node was parsed in the parameters of a generator.
GeneratorParameter = 1 << 3,

// If this node was parsed as part of a decorator
Decorator = 1 << 4,

This comment has been minimized.

Copy link
@JsonFreeman

JsonFreeman Mar 18, 2015

Contributor

What is the grammar production that cares about this?

This comment has been minimized.

Copy link
@rbuckton

rbuckton Mar 18, 2015

Author Member

I use this in parseCallExpressionRest to opt into a small lookahead when parsing the covering grammar for decorators preceding a ComputedPropertyName, instead of having to have an infinite lookahead when parsing a decorator. Consider:

class C {
  @dec ["test"](p: number) {  }
}

Normally, parseCallExpressionRest would greedily parse the decorator expression as dec["test"](p and report an error when it encounters :. Instead, when [Decorator] is set, I do a much smaller lookahead to try to parse a signature and avoid a parse error.

This comment has been minimized.

Copy link
@JsonFreeman

JsonFreeman Mar 18, 2015

Contributor

I think I'd need to see the grammar productions. Specifically, where/how do you decide if you will parse a computed property or an element access? Is it at the open bracket?


// If the parser encountered an error when parsing the code that created this node. Note
// the parser only sets this directly on the node it creates right after encountering the
// error.
ThisNodeHasError = 1 << 4,
ThisNodeHasError = 1 << 5,

// Context flags set directly by the parser.
ParserGeneratedFlags = StrictMode | DisallowIn | Yield | GeneratorParameter | ThisNodeHasError,
ParserGeneratedFlags = StrictMode | DisallowIn | Yield | GeneratorParameter | Decorator | ThisNodeHasError,

// Context flags computed by aggregating child flags upwards.

// Used during incremental parsing to determine if this node or any of its children had an
// error. Computed only once and then cached.
ThisNodeOrAnySubNodesHasError = 1 << 5,
ThisNodeOrAnySubNodesHasError = 1 << 6,

// Used to know if we've computed data from children and cached it in this node.
HasAggregatedChildData = 1 << 6
HasAggregatedChildData = 1 << 7
}

export const enum RelationComparisonResult {
Expand All @@ -357,13 +363,14 @@ module ts {
// Specific context the parser was in when this node was created. Normally undefined.
// Only set when the parser was in some interesting context (like async/yield).
parserContextFlags?: ParserContextFlags;
modifiers?: ModifiersArray; // Array of modifiers
id?: number; // Unique id (used to look up NodeLinks)
parent?: Node; // Parent node (initialized by binding)
symbol?: Symbol; // Symbol declared by node (initialized by binding)
locals?: SymbolTable; // Locals associated with node (initialized by binding)
nextContainer?: Node; // Next container in declaration order (initialized by binding)
localSymbol?: Symbol; // Local symbol declared by node (initialized by binding only for exported nodes)
decorators?: NodeArray<Decorator>; // Array of decorators

This comment has been minimized.

Copy link
@JsonFreeman

JsonFreeman Mar 18, 2015

Contributor

In order from top to bottom, right? I would mention that in the comment here

This comment has been minimized.

Copy link
@JsonFreeman

JsonFreeman Mar 18, 2015

Contributor

Also, should this be on Node or Declaration?

This comment has been minimized.

Copy link
@rbuckton

rbuckton Mar 18, 2015

Author Member

I added it to Node as modifiers is also on Node.

This comment has been minimized.

Copy link
@JsonFreeman

JsonFreeman Mar 18, 2015

Contributor

Okay

modifiers?: ModifiersArray; // Array of modifiers
id?: number; // Unique id (used to look up NodeLinks)
parent?: Node; // Parent node (initialized by binding)
symbol?: Symbol; // Symbol declared by node (initialized by binding)
locals?: SymbolTable; // Locals associated with node (initialized by binding)
nextContainer?: Node; // Next container in declaration order (initialized by binding)
localSymbol?: Symbol; // Local symbol declared by node (initialized by binding only for exported nodes)
}

export interface NodeArray<T> extends Array<T>, TextRange {
Expand Down Expand Up @@ -397,6 +404,11 @@ module ts {
expression: Expression;
}

export interface Decorator extends Node {
atToken: Node;

This comment has been minimized.

Copy link
@JsonFreeman

JsonFreeman Mar 18, 2015

Contributor

Right now, the parser does not make nodes for tokens like this. Is there a particular reason this is necessary?

This comment has been minimized.

Copy link
@rbuckton

rbuckton Mar 18, 2015

Author Member

I added these tokens so that I can refine the covering grammar for ElementAccess/CallExpression and ComputedPropertyName. I need the actual offsets for these tokens when I refine an ElementAccess into a ComputedPropertyName and a CallExpression into a Signature. See extractDeclarationNameFromDecoratorTail in parser.ts

This comment has been minimized.

Copy link
@JsonFreeman

JsonFreeman Mar 18, 2015

Contributor

The parser doesn't usually do refinement, it does lookahead instead.

This comment has been minimized.

Copy link
@mhegazy

mhegazy Mar 18, 2015

Contributor

if we loose the refinement we do not need this.

This comment has been minimized.

Copy link
@rbuckton

rbuckton Mar 23, 2015

Author Member

I've dropped the covering grammar and refinement for now in favor of disallowing ElementAccessExpression when in the [[Decorator]] context.

This comment has been minimized.

Copy link
@JsonFreeman

JsonFreeman Mar 23, 2015

Contributor

🎉

expression: LeftHandSideExpression;
}

export interface TypeParameterDeclaration extends Declaration {
name: Identifier;
constraint?: TypeNode;
Expand Down Expand Up @@ -687,7 +699,9 @@ module ts {
}

export interface ArrayLiteralExpression extends PrimaryExpression {
openBracketToken: Node;
elements: NodeArray<Expression>;
closeBracketToken: Node;

This comment has been minimized.

Copy link
@JsonFreeman

JsonFreeman Mar 18, 2015

Contributor

Why?

This comment has been minimized.

Copy link
@rbuckton
}

export interface SpreadElementExpression extends Expression {
Expand All @@ -707,7 +721,9 @@ module ts {

export interface ElementAccessExpression extends MemberExpression {
expression: LeftHandSideExpression;
openBracketToken: Node;
argumentExpression?: Expression;
closeBracketToken: Node;

This comment has been minimized.

Copy link
@JsonFreeman

JsonFreeman Mar 18, 2015

Contributor

Why?

This comment has been minimized.

Copy link
@rbuckton
}

export interface CallExpression extends LeftHandSideExpression {
Expand Down

0 comments on commit 5db3b0d

Please sign in to comment.