Skip to content

Added literal kind properties for each node.#11342

Merged
rbuckton merged 5 commits into
masterfrom
syntaxKindLiterals
Oct 6, 2016
Merged

Added literal kind properties for each node.#11342
rbuckton merged 5 commits into
masterfrom
syntaxKindLiterals

Conversation

@rbuckton
Copy link
Copy Markdown
Contributor

@rbuckton rbuckton commented Oct 4, 2016

This PR adds literal kind properties on each Node subtype in the compiler. While this is not a full migration to ADTs, it has caught a few errors in the compiler not presently caught by existing unit tests.

This also relaxes the typeOperatorSpacingRule for tslint so that it allows union and intersection types to span multiple lines.

Comment thread src/compiler/binder.ts
else {
forEachChild(node, bind);
if (node.operator === SyntaxKind.PlusEqualsToken || node.operator === SyntaxKind.MinusMinusToken) {
if (node.operator === SyntaxKind.PlusPlusToken || node.operator === SyntaxKind.MinusMinusToken) {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This was discovered due to the use of the more specific kind property.

Comment thread src/compiler/checker.ts
const enumType = <EnumType>getDeclaredTypeOfEnum(getParentOfSymbol(symbol));
links.declaredType = enumType.flags & TypeFlags.Union ?
enumType.memberTypes[getEnumMemberValue(<EnumDeclaration>symbol.valueDeclaration)] :
enumType.memberTypes[getEnumMemberValue(<EnumMember>symbol.valueDeclaration)] :
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This was discovered due to the use of the more specific kind property.

Comment thread src/compiler/checker.ts
}

if (produceDiagnostics && node.kind !== SyntaxKind.MethodDeclaration && node.kind !== SyntaxKind.MethodSignature) {
if (produceDiagnostics && node.kind !== SyntaxKind.MethodDeclaration) {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This was discovered due to the use of the more specific kind property. This was an unnecessary condition since MethodSignature would not be a legal element.

Comment thread src/compiler/checker.ts
}
else {
Debug.fail("Unexpected syntax kind:" + prop.kind);
Debug.fail("Unexpected syntax kind:" + (<Node>prop).kind);
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This branch is possibly unnecessary since prop is typed as never here.

@rbuckton
Copy link
Copy Markdown
Contributor Author

rbuckton commented Oct 4, 2016

// CC: @mhegazy, @ahejlsberg, @DanielRosenwasser

Comment thread src/compiler/core.ts
getIdentifierConstructor(): new (kind: SyntaxKind, pos?: number, end?: number) => Token;
getSourceFileConstructor(): new (kind: SyntaxKind, pos?: number, end?: number) => SourceFile;
getTokenConstructor(): new <TKind extends SyntaxKind>(kind: TKind, pos?: number, end?: number) => Token<TKind>;
getIdentifierConstructor(): new (kind: SyntaxKind.Identifier, pos?: number, end?: number) => Identifier;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Kind of funny that the Identifier constructor needs to take a SyntaxKind

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Who knows, we could eventually have different kind's of Identifier nodes.

Comment thread src/compiler/checker.ts
checkCollisionWithCapturedThisVariable(node, node.name);
checkCollisionWithRequireExportsInGeneratedCode(node, node.name);
checkCollisionWithGlobalPromiseInGeneratedCode(node, node.name);
if (isIdentifier(node.name)) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Was this a bug, or are we just trying to assert here that this is an Identifier?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We were calling each checkCollisionWith function with the node's name which may have been a StringLiteral. It may have been a bug that we never saw since StringLiterals cannot collide with any identifiers.

@rbuckton rbuckton merged commit 4a5f56f into master Oct 6, 2016
@rbuckton rbuckton deleted the syntaxKindLiterals branch October 6, 2016 21:00
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants