Skip to content

Commit

Permalink
Checker updates for decorators
Browse files Browse the repository at this point in the history
  • Loading branch information
@rbuckton
rbuckton committed Mar 17, 2015
1 parent 8282a0b commit 39001f5
Show file tree
Hide file tree
Showing 3 changed files with 187 additions and 25 deletions.
178 changes: 162 additions & 16 deletions src/compiler/checker.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,11 +100,18 @@ module ts {
let globalIterableType: ObjectType;

let anyArrayType: Type;
let globalTypedPropertyDescriptorType: ObjectType;
let globalClassDecoratorType: ObjectType;
let globalClassAnnotationType: ObjectType;
let globalParameterAnnotationType: ObjectType;
let globalPropertyAnnotationType: ObjectType;
let globalPropertyDecoratorType: ObjectType;

let tupleTypes: Map<TupleType> = {};
let unionTypes: Map<UnionType> = {};
let stringLiteralTypes: Map<StringLiteralType> = {};
let emitExtends = false;
let emitDecorate = false;

let mergedSymbols: Symbol[] = [];
let symbolLinks: SymbolLinks[] = [];
Expand Down Expand Up @@ -311,6 +318,7 @@ module ts {
let lastLocation: Node;
let propertyWithInvalidInitializer: Node;
let errorLocation = location;
let grandparent: Node;

loop: while (location) {
// Locals of a source file are not in scope (because they get merged into the global symbol table)
Expand Down Expand Up @@ -376,7 +384,7 @@ module ts {
// }
//
case SyntaxKind.ComputedPropertyName:
let grandparent = location.parent.parent;
grandparent = location.parent.parent;
if (grandparent.kind === SyntaxKind.ClassDeclaration || grandparent.kind === SyntaxKind.InterfaceDeclaration) {
// A reference to this grandparent's type parameters would be an error
if (result = getSymbol(getSymbolOfNode(grandparent).members, name, meaning & SymbolFlags.Type)) {
Expand Down Expand Up @@ -408,6 +416,26 @@ module ts {
break loop;
}
break;
case SyntaxKind.Decorator:
if (location.parent) {
lastLocation = location;
grandparent = location.parent.parent;
if (location.parent.kind === SyntaxKind.Parameter) {
// Parameter decorators are resolved in the context of their great-grandparent
if (grandparent) {
location = grandparent.parent;

This comment has been minimized.

Sign in to view

Copy link
@JsonFreeman

JsonFreeman Mar 20, 2015

Contributor

Seems like this is only correct for method and constructor parameters, but what about function parameters?
}
else {
break loop;

This comment has been minimized.

Sign in to view

Copy link
@JsonFreeman

JsonFreeman Mar 20, 2015

Contributor

Please give an example of where this could happen
}
}
else {
// all other decorators are resolved in the context of their grandparent
location = grandparent;
}
continue;
}
break;
}
lastLocation = location;
location = location.parent;
Expand Down Expand Up @@ -7852,7 +7880,7 @@ module ts {
// strict mode FunctionLikeDeclaration or FunctionExpression(13.1)

// Grammar checking
checkGrammarModifiers(node) || checkGrammarEvalOrArgumentsInStrictMode(node, <Identifier>node.name);
checkGrammarDecorators(node) || checkGrammarModifiers(node) || checkGrammarEvalOrArgumentsInStrictMode(node, <Identifier>node.name);

checkVariableLikeDeclaration(node);
let func = getContainingFunction(node);
Expand Down Expand Up @@ -7954,7 +7982,7 @@ module ts {

function checkPropertyDeclaration(node: PropertyDeclaration) {
// Grammar checking
checkGrammarModifiers(node) || checkGrammarProperty(node) || checkGrammarComputedPropertyName(node.name);
checkGrammarDecorators(node) || checkGrammarModifiers(node) || checkGrammarProperty(node) || checkGrammarComputedPropertyName(node.name);

checkVariableLikeDeclaration(node);
}
Expand Down Expand Up @@ -8093,6 +8121,10 @@ module ts {
checkFunctionLikeDeclaration(node);
}

function checkIncompleteDeclaration(node: Declaration) {
error(node, Diagnostics.Declaration_expected);

This comment has been minimized.

Sign in to view

Copy link
@JsonFreeman

JsonFreeman Mar 20, 2015

Contributor

I think I prefer a parse error here
}

function checkTypeReference(node: TypeReferenceNode) {
// Grammar checking
checkGrammarTypeArguments(node, node.typeArguments);
Expand Down Expand Up @@ -8484,6 +8516,70 @@ module ts {
}
}

function checkDecoratorSignature(node: Decorator, exprType: Type, expectedAnnotationType: Type, parentType?: Type, expectedDecoratorType?: Type, message?: DiagnosticMessage) {

This comment has been minimized.

Sign in to view

Copy link
@JsonFreeman

JsonFreeman Mar 20, 2015

Contributor

Wrap this line

This comment has been minimized.

Sign in to view

Copy link
@JsonFreeman

JsonFreeman Mar 20, 2015

Contributor

Also, I don't understand the difference between annotations and decorators. I would prefer to conflate them into one concept.

This comment has been minimized.

Sign in to view

Copy link
@JsonFreeman

JsonFreeman Mar 20, 2015

Contributor

Actually, besides node, I'm not sure what any of these parameters are.
if (checkTypeAssignableTo(exprType, expectedAnnotationType, node) && expectedDecoratorType) {
let signature = getSingleCallSignature(expectedDecoratorType);
if (!signature) {
// if we couldn't get the signature of the decorator function type, it is likely because we are using an out-of-date lib.d.ts
// and we have already reported an error in initializeTypeChecker.
return;
}

let instantiatedSignature = getSignatureInstantiation(signature, [parentType]);
let instantiatedSignatureType = getOrCreateTypeFromSignature(instantiatedSignature);
checkTypeAssignableTo(exprType, instantiatedSignatureType, node, message);

This comment has been minimized.

Sign in to view

Copy link
@JsonFreeman

JsonFreeman Mar 20, 2015

Contributor

I guess this makes sense, I'm wondering how to do it without having both the decorator type and the annotation type.

This comment has been minimized.

Sign in to view

Copy link
@JsonFreeman

JsonFreeman Mar 20, 2015

Contributor

Maybe the best way to do it is to just instantiate the global decorator type with its target right off the bat, and then check the compatibility between that and the decorator's type. For the class case, where you have a constraint, you don't actually need to check that constraint, because you have a class, which is compatible with Function by definition.

This comment has been minimized.

Sign in to view

Copy link
@rbuckton

rbuckton Mar 23, 2015

Author Member

I'm renaming the parameters and variables to be a little less confusing.

Here are the goals of this function:

  1. Verify that the signature is correct based on the declaration target.
  2. Verify that if the signature has a return type, that it is assignable to the type of the decorated declaration (e.g. the type is the same or a subtype).

The approach I've taken is to define function types with generic type parameters to enforce this restriction, e.g.:

type ClassDecorator = <TFunc extends Function>(target: TFunc) => TFunc | void;

I then satisfy [1] above by instantiating the signature of ClassDecorator using Function and checking assignability of the decorator expression to (target: Function) => Function | void.
Then, to satisfy [2] above, I instantiate the signature of ClassDecorator with the type of the class declaration, to verify the return type is either void or a subtype constructor.

Another approach to solve [2] would be to just check the signature like I do currently to solve [1], then find the compatible overload for [1] and check its return type. I feel that my current solution to solve [2] is cleaner and reuses existing logic rather than introducing something different for the same purpose.
}
}

/** Check a decorator */
function checkDecorator(node: Decorator): void {
let expression: Expression = node.expression;
let exprType = checkExpression(expression);

switch (node.parent.kind) {
case SyntaxKind.ClassDeclaration:
let classSymbol = getSymbolOfNode(node.parent);
let classType = getTypeOfSymbol(classSymbol);

This comment has been minimized.

Sign in to view

Copy link
@JsonFreeman

JsonFreeman Mar 20, 2015

Contributor

classConstructorType
checkDecoratorSignature(node, exprType, globalClassAnnotationType, classType, globalClassDecoratorType, Diagnostics.Decorators_may_not_change_the_type_of_a_class);
break;

case SyntaxKind.PropertyDeclaration:
case SyntaxKind.MethodDeclaration:
case SyntaxKind.GetAccessor:
case SyntaxKind.SetAccessor:
let propertyType = getTypeOfNode(node.parent);
checkDecoratorSignature(node, exprType, globalPropertyAnnotationType, propertyType, globalPropertyDecoratorType, Diagnostics.Decorators_may_not_change_the_type_of_a_member);
break;

case SyntaxKind.Parameter:
checkDecoratorSignature(node, exprType, globalParameterAnnotationType);
break;
}
}

/** Check the decorators of a node */
function checkDecorators(node: Node): void {
if (!node.decorators) {
return;
}

switch (node.kind) {
case SyntaxKind.ClassDeclaration:
case SyntaxKind.MethodDeclaration:
case SyntaxKind.GetAccessor:
case SyntaxKind.SetAccessor:
case SyntaxKind.PropertyDeclaration:
case SyntaxKind.Parameter:
emitDecorate = true;
break;

default:
return;
}

forEach(node.decorators, checkDecorator);
}

function checkFunctionDeclaration(node: FunctionDeclaration): void {
if (produceDiagnostics) {
checkFunctionLikeDeclaration(node) ||
Expand All @@ -8498,6 +8594,7 @@ module ts {
}

function checkFunctionLikeDeclaration(node: FunctionLikeDeclaration): void {
checkDecorators(node);
checkSignatureDeclaration(node);

// Do not use hasDynamicName here, because that returns false for well known symbols.
Expand Down Expand Up @@ -8780,6 +8877,7 @@ module ts {

// Check variable, parameter, or property declaration
function checkVariableLikeDeclaration(node: VariableLikeDeclaration) {
checkDecorators(node);
checkSourceElement(node.type);
// For a computed property, just check the initializer and exit
// Do not use hasDynamicName here, because that returns false for well known symbols.
Expand Down Expand Up @@ -8852,7 +8950,7 @@ module ts {

function checkVariableStatement(node: VariableStatement) {
// Grammar checking
checkGrammarDisallowedModifiersInBlockOrObjectLiteralExpression(node) || checkGrammarModifiers(node) || checkGrammarVariableDeclarationList(node.declarationList) || checkGrammarForDisallowedLetOrConstStatement(node);
checkGrammarDecorators(node) || checkGrammarDisallowedModifiersInBlockOrObjectLiteralExpression(node) || checkGrammarModifiers(node) || checkGrammarVariableDeclarationList(node.declarationList) || checkGrammarForDisallowedLetOrConstStatement(node);

forEach(node.declarationList.declarations, checkSourceElement);
}
Expand Down Expand Up @@ -9483,7 +9581,7 @@ module ts {
function checkClassDeclaration(node: ClassDeclaration) {
// Grammar checking
checkGrammarClassDeclarationHeritageClauses(node);

checkDecorators(node);
if (node.name) {
checkTypeNameIsReserved(node.name, Diagnostics.Class_name_cannot_be_0);
checkCollisionWithCapturedThisVariable(node, node.name);
Expand Down Expand Up @@ -9688,7 +9786,7 @@ module ts {

function checkInterfaceDeclaration(node: InterfaceDeclaration) {
// Grammar checking
checkGrammarModifiers(node) || checkGrammarInterfaceDeclaration(node);
checkGrammarDecorators(node) || checkGrammarModifiers(node) || checkGrammarInterfaceDeclaration(node);

checkTypeParameters(node.typeParameters);
if (produceDiagnostics) {
Expand Down Expand Up @@ -9725,7 +9823,7 @@ module ts {

function checkTypeAliasDeclaration(node: TypeAliasDeclaration) {
// Grammar checking
checkGrammarModifiers(node);
checkGrammarDecorators(node) || checkGrammarModifiers(node);

checkTypeNameIsReserved(node.name, Diagnostics.Type_alias_name_cannot_be_0);
checkSourceElement(node.type);
Expand Down Expand Up @@ -9894,7 +9992,7 @@ module ts {
}

// Grammar checking
checkGrammarModifiers(node) || checkGrammarEnumDeclaration(node);
checkGrammarDecorators(node) || checkGrammarModifiers(node) || checkGrammarEnumDeclaration(node);

checkTypeNameIsReserved(node.name, Diagnostics.Enum_name_cannot_be_0);
checkCollisionWithCapturedThisVariable(node, node.name);
Expand Down Expand Up @@ -9960,7 +10058,7 @@ module ts {
function checkModuleDeclaration(node: ModuleDeclaration) {
if (produceDiagnostics) {
// Grammar checking
if (!checkGrammarModifiers(node)) {
if (!checkGrammarDecorators(node) && !checkGrammarModifiers(node)) {
if (!isInAmbientContext(node) && node.name.kind === SyntaxKind.StringLiteral) {
grammarErrorOnNode(node.name, Diagnostics.Only_ambient_modules_can_use_quoted_names);
}
Expand Down Expand Up @@ -10055,7 +10153,7 @@ module ts {
}

function checkImportDeclaration(node: ImportDeclaration) {
if (!checkGrammarModifiers(node) && (node.flags & NodeFlags.Modifier)) {
if (!checkGrammarDecorators(node) && !checkGrammarModifiers(node) && (node.flags & NodeFlags.Modifier)) {
grammarErrorOnFirstToken(node, Diagnostics.An_import_declaration_cannot_have_modifiers);
}
if (checkExternalImportOrExportDeclaration(node)) {
Expand All @@ -10077,7 +10175,7 @@ module ts {
}

function checkImportEqualsDeclaration(node: ImportEqualsDeclaration) {
checkGrammarModifiers(node);
checkGrammarDecorators(node) || checkGrammarModifiers(node);
if (isInternalModuleImportEqualsDeclaration(node) || checkExternalImportOrExportDeclaration(node)) {
checkImportBinding(node);
if (node.flags & NodeFlags.Export) {
Expand Down Expand Up @@ -10108,7 +10206,7 @@ module ts {
}

function checkExportDeclaration(node: ExportDeclaration) {
if (!checkGrammarModifiers(node) && (node.flags & NodeFlags.Modifier)) {
if (!checkGrammarDecorators(node) && !checkGrammarModifiers(node) && (node.flags & NodeFlags.Modifier)) {
grammarErrorOnFirstToken(node, Diagnostics.An_export_declaration_cannot_have_modifiers);
}
if (!node.moduleSpecifier || checkExternalImportOrExportDeclaration(node)) {
Expand All @@ -10132,7 +10230,7 @@ module ts {
return;
}
// Grammar checking
if (!checkGrammarModifiers(node) && (node.flags & NodeFlags.Modifier)) {
if (!checkGrammarDecorators(node) && !checkGrammarModifiers(node) && (node.flags & NodeFlags.Modifier)) {
grammarErrorOnFirstToken(node, Diagnostics.An_export_assignment_cannot_have_modifiers);
}
if (node.expression) {
Expand Down Expand Up @@ -10232,6 +10330,8 @@ module ts {
case SyntaxKind.GetAccessor:
case SyntaxKind.SetAccessor:
return checkAccessorDeclaration(<AccessorDeclaration>node);
case SyntaxKind.IncompleteDeclaration:
return checkIncompleteDeclaration(<Declaration>node);
case SyntaxKind.TypeReference:
return checkTypeReference(<TypeReferenceNode>node);
case SyntaxKind.TypeQuery:
Expand Down Expand Up @@ -10436,6 +10536,10 @@ module ts {
links.flags |= NodeCheckFlags.EmitExtends;
}

if (emitDecorate) {
links.flags |= NodeCheckFlags.EmitDecorate;
}

links.flags |= NodeCheckFlags.TypeChecked;
}
}
Expand Down Expand Up @@ -11235,6 +11339,12 @@ module ts {
globalNumberType = getGlobalType("Number");
globalBooleanType = getGlobalType("Boolean");
globalRegExpType = getGlobalType("RegExp");
globalTypedPropertyDescriptorType = getTypeOfGlobalSymbol(getGlobalTypeSymbol("TypedPropertyDescriptor"), 1);
globalClassDecoratorType = getGlobalType("ClassDecorator");
globalPropertyDecoratorType = getGlobalType("PropertyDecorator");
globalClassAnnotationType = getGlobalType("ClassAnnotation");
globalPropertyAnnotationType = getGlobalType("PropertyAnnotation");
globalParameterAnnotationType = getGlobalType("ParameterAnnotation");

// If we're in ES6 mode, load the TemplateStringsArray.
// Otherwise, default to 'unknown' for the purposes of type checking in LS scenarios.
Expand All @@ -11259,6 +11369,42 @@ module ts {


// GRAMMAR CHECKING
function checkGrammarDecorators(node: Node): boolean {
if (!node.decorators) {
return false;
}

let target = node;
while (target) {
switch (target.kind) {
case SyntaxKind.ClassDeclaration:
return false;

case SyntaxKind.Constructor:
if (node.kind !== SyntaxKind.Parameter) {
break;
}

case SyntaxKind.PropertyDeclaration:
case SyntaxKind.Parameter:
target = target.parent;
continue;


case SyntaxKind.MethodDeclaration:
case SyntaxKind.GetAccessor:
case SyntaxKind.SetAccessor:
if ((<FunctionLikeDeclaration>target).body) {
target = target.parent;
continue;
}
break;
}
break;
}

This comment has been minimized.

Sign in to view

Copy link
@JsonFreeman

JsonFreeman Mar 20, 2015

Contributor

Would it be possible to simplify the control flow here? I find it kind of hard to follow.
return grammarErrorOnNode(node, Diagnostics.Decorators_are_not_valid_on_this_declaration_type);
}

function checkGrammarModifiers(node: Node): boolean {
switch (node.kind) {
case SyntaxKind.GetAccessor:
Expand Down Expand Up @@ -11455,7 +11601,7 @@ module ts {
function checkGrammarFunctionLikeDeclaration(node: FunctionLikeDeclaration): boolean {
// Prevent cascading error by short-circuit
let file = getSourceFileOfNode(node);
return checkGrammarModifiers(node) || checkGrammarTypeParameterList(node, node.typeParameters, file) ||
return checkGrammarDecorators(node) || checkGrammarModifiers(node) || checkGrammarTypeParameterList(node, node.typeParameters, file) ||
checkGrammarParameterList(node.parameters) || checkGrammarArrowFunction(node, file);
}

Expand Down Expand Up @@ -11512,7 +11658,7 @@ module ts {

function checkGrammarIndexSignature(node: SignatureDeclaration) {
// Prevent cascading error by short-circuit
checkGrammarModifiers(node) || checkGrammarIndexSignatureParameters(node) || checkGrammarForIndexSignatureModifier(node);
return checkGrammarDecorators(node) || checkGrammarModifiers(node) || checkGrammarIndexSignatureParameters(node) || checkGrammarForIndexSignatureModifier(node);
}

function checkGrammarForAtLeastOneTypeArgument(node: Node, typeArguments: NodeArray<TypeNode>): boolean {
Expand Down Expand Up @@ -11561,7 +11707,7 @@ module ts {
let seenExtendsClause = false;
let seenImplementsClause = false;

if (!checkGrammarModifiers(node) && node.heritageClauses) {
if (!checkGrammarDecorators(node) && !checkGrammarModifiers(node) && node.heritageClauses) {
for (let heritageClause of node.heritageClauses) {
if (heritageClause.token === SyntaxKind.ExtendsKeyword) {
if (seenExtendsClause) {
Expand Down
19 changes: 10 additions & 9 deletions src/compiler/types.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1346,17 +1346,18 @@ module ts {
}

export const enum NodeCheckFlags {
TypeChecked = 0x00000001, // Node has been type checked
LexicalThis = 0x00000002, // Lexical 'this' reference
CaptureThis = 0x00000004, // Lexical 'this' used in body
EmitExtends = 0x00000008, // Emit __extends
SuperInstance = 0x00000010, // Instance 'super' reference
SuperStatic = 0x00000020, // Static 'super' reference
ContextChecked = 0x00000040, // Contextual types have been assigned
TypeChecked = 0x00000001, // Node has been type checked
LexicalThis = 0x00000002, // Lexical 'this' reference
CaptureThis = 0x00000004, // Lexical 'this' used in body
EmitExtends = 0x00000008, // Emit __extends
SuperInstance = 0x00000010, // Instance 'super' reference
SuperStatic = 0x00000020, // Static 'super' reference
ContextChecked = 0x00000040, // Contextual types have been assigned

// Values for enum members have been computed, and any errors have been reported for them.
EnumValuesComputed = 0x00000080,
BlockScopedBindingInLoop = 0x00000100,
EnumValuesComputed = 0x00000080,
BlockScopedBindingInLoop = 0x00000100,
EmitDecorate = 0x00000200, // Emit __decorate
}

export interface NodeLinks {
Expand Down
Loading

0 comments on commit 39001f5

Please sign in to comment.