Skip to content

Fix emit issue regarding null/undefined in type annotations #11653

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

Merged
merged 5 commits into from
Oct 16, 2016
Merged

Fix emit issue regarding null/undefined in type annotations #11653

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
980a894
Fix emit issue regarding null/undefined in type annotations
rbuckton Oct 15, 2016
40e99e7
Updated comment
rbuckton Oct 15, 2016
7f6faaa
Merge branch 'master' into fix11295
rbuckton Oct 15, 2016
a4a7c23
Fix issues from merge
rbuckton Oct 15, 2016
db76a9c
More tests
rbuckton Oct 16, 2016
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
124 changes: 84 additions & 40 deletions src/compiler/binder.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2351,6 +2351,9 @@ namespace ts {
case SyntaxKind.CallExpression:
return computeCallExpression(<CallExpression>node, subtreeFlags);

case SyntaxKind.NewExpression:
return computeNewExpression(<NewExpression>node, subtreeFlags);

case SyntaxKind.ModuleDeclaration:
return computeModuleDeclaration(<ModuleDeclaration>node, subtreeFlags);

Expand Down Expand Up @@ -2428,6 +2431,10 @@ namespace ts {
const expression = node.expression;
const expressionKind = expression.kind;

if (node.typeArguments) {
transformFlags |= TransformFlags.AssertTypeScript;
}

if (subtreeFlags & TransformFlags.ContainsSpreadElementExpression
|| isSuperOrSuperProperty(expression, expressionKind)) {
// If the this node contains a SpreadElementExpression, or is a super call, then it is an ES6
Expand All @@ -2454,6 +2461,21 @@ namespace ts {
return false;
}

function computeNewExpression(node: NewExpression, subtreeFlags: TransformFlags) {
let transformFlags = subtreeFlags;
if (node.typeArguments) {
transformFlags |= TransformFlags.AssertTypeScript;
}
if (subtreeFlags & TransformFlags.ContainsSpreadElementExpression) {
// If the this node contains a SpreadElementExpression then it is an ES6
// node.
transformFlags |= TransformFlags.AssertES2015;
}
node.transformFlags = transformFlags | TransformFlags.HasComputedFlags;
return transformFlags & ~TransformFlags.ArrayLiteralOrCallOrNewExcludes;
}


function computeBinaryExpression(node: BinaryExpression, subtreeFlags: TransformFlags) {
let transformFlags = subtreeFlags;
const operatorTokenKind = node.operatorToken.kind;
Expand Down Expand Up @@ -2482,13 +2504,12 @@ namespace ts {
const initializer = node.initializer;
const dotDotDotToken = node.dotDotDotToken;

// If the parameter has a question token, then it is TypeScript syntax.
if (node.questionToken) {
transformFlags |= TransformFlags.AssertTypeScript;
}

// If the parameter's name is 'this', then it is TypeScript syntax.
if (subtreeFlags & TransformFlags.ContainsDecorators || isThisIdentifier(name)) {
// The '?' token, type annotations, decorators, and 'this' parameters are TypeSCript
// syntax.
if (node.questionToken
|| node.type
|| subtreeFlags & TransformFlags.ContainsDecorators
|| isThisIdentifier(name)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

can you add a test to check this condition?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The only new functionality is the test for node.type, which is covered by transformsElideNullUndefinedType

transformFlags |= TransformFlags.AssertTypeScript;
}

Expand Down Expand Up @@ -2547,7 +2568,8 @@ namespace ts {
// TypeScript syntax.
// An exported declaration may be TypeScript syntax.
if ((subtreeFlags & TransformFlags.TypeScriptClassSyntaxMask)
|| (modifierFlags & ModifierFlags.Export)) {
|| (modifierFlags & ModifierFlags.Export)
|| node.typeParameters) {
transformFlags |= TransformFlags.AssertTypeScript;
}

Expand All @@ -2568,7 +2590,8 @@ namespace ts {

// A class with a parameter property assignment, property initializer, or decorator is
// TypeScript syntax.
if (subtreeFlags & TransformFlags.TypeScriptClassSyntaxMask) {
if (subtreeFlags & TransformFlags.TypeScriptClassSyntaxMask
|| node.typeParameters) {
transformFlags |= TransformFlags.AssertTypeScript;
}

Expand Down Expand Up @@ -2622,10 +2645,10 @@ namespace ts {

function computeConstructor(node: ConstructorDeclaration, subtreeFlags: TransformFlags) {
let transformFlags = subtreeFlags;
const body = node.body;

if (body === undefined) {
// An overload constructor is TypeScript syntax.
// TypeScript-specific modifiers and overloads are TypeScript syntax
if (hasModifier(node, ModifierFlags.TypeScriptModifier)
|| !node.body) {
transformFlags |= TransformFlags.AssertTypeScript;
}

Expand All @@ -2636,27 +2659,24 @@ namespace ts {
function computeMethod(node: MethodDeclaration, subtreeFlags: TransformFlags) {
// A MethodDeclaration is ES6 syntax.
let transformFlags = subtreeFlags | TransformFlags.AssertES2015;
const modifierFlags = getModifierFlags(node);
const body = node.body;
const typeParameters = node.typeParameters;
const asteriskToken = node.asteriskToken;

// A MethodDeclaration is TypeScript syntax if it is either abstract, overloaded,
// generic, or has a decorator.
if (!body
|| typeParameters
|| (modifierFlags & ModifierFlags.Abstract)
|| (subtreeFlags & TransformFlags.ContainsDecorators)) {

// Decorators, TypeScript-specific modifiers, type parameters, type annotations, and
// overloads are TypeScript syntax.
if (node.decorators
|| hasModifier(node, ModifierFlags.TypeScriptModifier)
|| node.typeParameters
|| node.type
|| !node.body) {
transformFlags |= TransformFlags.AssertTypeScript;
}

// An async method declaration is ES2017 syntax.
if (modifierFlags & ModifierFlags.Async) {
if (hasModifier(node, ModifierFlags.Async)) {
transformFlags |= TransformFlags.AssertES2017;
}

// Currently, we only support generators that were originally async function bodies.
if (asteriskToken && getEmitFlags(node) & EmitFlags.AsyncFunctionBody) {
if (node.asteriskToken && getEmitFlags(node) & EmitFlags.AsyncFunctionBody) {
transformFlags |= TransformFlags.AssertGenerator;
}

Expand All @@ -2666,14 +2686,13 @@ namespace ts {

function computeAccessor(node: AccessorDeclaration, subtreeFlags: TransformFlags) {
let transformFlags = subtreeFlags;
const modifierFlags = getModifierFlags(node);
const body = node.body;

// An accessor is TypeScript syntax if it is either abstract, overloaded,
// generic, or has a decorator.
if (!body
|| (modifierFlags & ModifierFlags.Abstract)
|| (subtreeFlags & TransformFlags.ContainsDecorators)) {
// Decorators, TypeScript-specific modifiers, type annotations, and overloads are
// TypeScript syntax.
if (node.decorators
|| hasModifier(node, ModifierFlags.TypeScriptModifier)
|| node.type
|| !node.body) {
transformFlags |= TransformFlags.AssertTypeScript;
}

Expand All @@ -2699,7 +2718,6 @@ namespace ts {
let transformFlags: TransformFlags;
const modifierFlags = getModifierFlags(node);
const body = node.body;
const asteriskToken = node.asteriskToken;

if (!body || (modifierFlags & ModifierFlags.Ambient)) {
// An ambient declaration is TypeScript syntax.
Expand All @@ -2714,6 +2732,14 @@ namespace ts {
transformFlags |= TransformFlags.AssertTypeScript | TransformFlags.AssertES2015;
}

// TypeScript-specific modifiers, type parameters, and type annotations are TypeScript
// syntax.
if (modifierFlags & ModifierFlags.TypeScriptModifier
|| node.typeParameters
|| node.type) {
transformFlags |= TransformFlags.AssertTypeScript;
}

// An async function declaration is ES2017 syntax.
if (modifierFlags & ModifierFlags.Async) {
transformFlags |= TransformFlags.AssertES2017;
Expand All @@ -2731,7 +2757,7 @@ namespace ts {
// down-level generator.
// Currently we do not support transforming any other generator fucntions
// down level.
if (asteriskToken && getEmitFlags(node) & EmitFlags.AsyncFunctionBody) {
if (node.asteriskToken && getEmitFlags(node) & EmitFlags.AsyncFunctionBody) {
transformFlags |= TransformFlags.AssertGenerator;
}
}
Expand All @@ -2742,11 +2768,17 @@ namespace ts {

function computeFunctionExpression(node: FunctionExpression, subtreeFlags: TransformFlags) {
let transformFlags = subtreeFlags;
const modifierFlags = getModifierFlags(node);
const asteriskToken = node.asteriskToken;

// TypeScript-specific modifiers, type parameters, and type annotations are TypeScript
// syntax.
if (hasModifier(node, ModifierFlags.TypeScriptModifier)
|| node.typeParameters
|| node.type) {
transformFlags |= TransformFlags.AssertTypeScript;
}

// An async function expression is ES2017 syntax.
if (modifierFlags & ModifierFlags.Async) {
if (hasModifier(node, ModifierFlags.Async)) {
transformFlags |= TransformFlags.AssertES2017;
}

Expand All @@ -2762,7 +2794,7 @@ namespace ts {
// down-level generator.
// Currently we do not support transforming any other generator fucntions
// down level.
if (asteriskToken && getEmitFlags(node) & EmitFlags.AsyncFunctionBody) {
if (node.asteriskToken && getEmitFlags(node) & EmitFlags.AsyncFunctionBody) {
transformFlags |= TransformFlags.AssertGenerator;
}

Expand All @@ -2773,10 +2805,17 @@ namespace ts {
function computeArrowFunction(node: ArrowFunction, subtreeFlags: TransformFlags) {
// An ArrowFunction is ES6 syntax, and excludes markers that should not escape the scope of an ArrowFunction.
let transformFlags = subtreeFlags | TransformFlags.AssertES2015;
const modifierFlags = getModifierFlags(node);

// TypeScript-specific modifiers, type parameters, and type annotations are TypeScript
// syntax.
if (hasModifier(node, ModifierFlags.TypeScriptModifier)
|| node.typeParameters
|| node.type) {
transformFlags |= TransformFlags.AssertTypeScript;
}

// An async arrow function is ES2017 syntax.
if (modifierFlags & ModifierFlags.Async) {
if (hasModifier(node, ModifierFlags.Async)) {
transformFlags |= TransformFlags.AssertES2017;
}

Expand Down Expand Up @@ -2813,6 +2852,11 @@ namespace ts {
transformFlags |= TransformFlags.AssertES2015 | TransformFlags.ContainsBindingPattern;
}

// Type annotations are TypeScript syntax.
if (node.type) {
transformFlags |= TransformFlags.AssertTypeScript;
}

node.transformFlags = transformFlags | TransformFlags.HasComputedFlags;
return transformFlags & ~TransformFlags.NodeExcludes;
}
Expand Down
19 changes: 18 additions & 1 deletion src/compiler/emitter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -504,15 +504,29 @@ const _super = (function (geti, seti) {

// Contextual keywords
case SyntaxKind.AbstractKeyword:
case SyntaxKind.AsKeyword:
case SyntaxKind.AnyKeyword:
case SyntaxKind.AsyncKeyword:
case SyntaxKind.AwaitKeyword:
case SyntaxKind.BooleanKeyword:
case SyntaxKind.ConstructorKeyword:
case SyntaxKind.DeclareKeyword:
case SyntaxKind.NumberKeyword:
case SyntaxKind.GetKeyword:
case SyntaxKind.IsKeyword:
case SyntaxKind.ModuleKeyword:
case SyntaxKind.NamespaceKeyword:
case SyntaxKind.NeverKeyword:
case SyntaxKind.ReadonlyKeyword:
case SyntaxKind.RequireKeyword:
case SyntaxKind.NumberKeyword:
case SyntaxKind.SetKeyword:
case SyntaxKind.StringKeyword:
case SyntaxKind.SymbolKeyword:
case SyntaxKind.TypeKeyword:
case SyntaxKind.UndefinedKeyword:
case SyntaxKind.FromKeyword:
case SyntaxKind.GlobalKeyword:
case SyntaxKind.OfKeyword:
writeTokenText(kind);
return;

Expand Down Expand Up @@ -1198,12 +1212,14 @@ const _super = (function (geti, seti) {

function emitCallExpression(node: CallExpression) {
emitExpression(node.expression);
emitTypeArguments(node, node.typeArguments);
emitExpressionList(node, node.arguments, ListFormat.CallExpressionArguments);
}

function emitNewExpression(node: NewExpression) {
write("new ");
emitExpression(node.expression);
emitTypeArguments(node, node.typeArguments);
Copy link
Contributor

Choose a reason for hiding this comment

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

I can see why this is necessary, but do we have cases today when we need to emit type arguments or type annotations - asking because It will be nice to test this functionality

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Primarily it helped me catch other possible future errors.

emitExpressionList(node, node.arguments, ListFormat.NewExpressionArguments);
}

Expand Down Expand Up @@ -1575,6 +1591,7 @@ const _super = (function (geti, seti) {

function emitVariableDeclaration(node: VariableDeclaration) {
emit(node.name);
emitWithPrefix(": ", node.type);
emitExpressionWithPrefix(" = ", node.initializer);
}

Expand Down
2 changes: 2 additions & 0 deletions src/compiler/transformers/es2015.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1417,6 +1417,7 @@ namespace ts {
if (getAccessor) {
const getterFunction = transformFunctionLikeToExpression(getAccessor, /*location*/ undefined, /*name*/ undefined);
setSourceMapRange(getterFunction, getSourceMapRange(getAccessor));
setEmitFlags(getterFunction, EmitFlags.NoLeadingComments);
const getter = createPropertyAssignment("get", getterFunction);
setCommentRange(getter, getCommentRange(getAccessor));
properties.push(getter);
Expand All @@ -1425,6 +1426,7 @@ namespace ts {
if (setAccessor) {
const setterFunction = transformFunctionLikeToExpression(setAccessor, /*location*/ undefined, /*name*/ undefined);
setSourceMapRange(setterFunction, getSourceMapRange(setAccessor));
setEmitFlags(setterFunction, EmitFlags.NoLeadingComments);
const setter = createPropertyAssignment("set", setterFunction);
setCommentRange(setter, getCommentRange(setAccessor));
properties.push(setter);
Expand Down
Loading