Skip to content

Ensure that emitter calls callbacks #18284

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
9 commits merged into from
Sep 7, 2017
Merged

Ensure that emitter calls callbacks #18284

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
9815359
Ensure that emitter calls calbacks
Sep 5, 2017
7d931bb
Move new parameter to end of parameters
Sep 7, 2017
7d56deb
Merge branch 'master' into extract-method-format
Sep 7, 2017
5b44de1
Fix for ConditionalExpression
Sep 7, 2017
aae877d
Make suggested changes to emitter
Sep 7, 2017
8f775dc
Fix parameter ordering
Sep 7, 2017
bca3b09
Respond to minor comments
Sep 7, 2017
15e2663
Remove potentially expensive assertion
Sep 7, 2017
b33dbec
More emitter cleanup
Sep 7, 2017
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
134 changes: 74 additions & 60 deletions src/compiler/emitter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -406,6 +406,14 @@ namespace ts {
setWriter(/*output*/ undefined);
}

// TODO: Should this just be `emit`?
// See https://github.com/Microsoft/TypeScript/pull/18284#discussion_r137611034
function emitIfPresent(node: Node | undefined) {
if (node) {
pipelineEmitWithNotification(EmitHint.Unspecified, node);
}
}

function emit(node: Node) {
pipelineEmitWithNotification(EmitHint.Unspecified, node);
}
Expand Down Expand Up @@ -451,6 +459,7 @@ namespace ts {
case EmitHint.SourceFile: return pipelineEmitSourceFile(node);
case EmitHint.IdentifierName: return pipelineEmitIdentifierName(node);
case EmitHint.Expression: return pipelineEmitExpression(node);
case EmitHint.MappedTypeParameter: return pipelineEmitMappedTypeParameter(cast(node, isTypeParameterDeclaration));
case EmitHint.Unspecified: return pipelineEmitUnspecified(node);
}
}
Expand All @@ -465,6 +474,12 @@ namespace ts {
emitIdentifier(<Identifier>node);
}

function pipelineEmitMappedTypeParameter(node: TypeParameterDeclaration): void {
Copy link
Contributor

@rbuckton rbuckton Sep 7, 2017

Choose a reason for hiding this comment

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

Just call this emitMappedTypeParameter.

emit(node.name);
write(" in ");
emit(node.constraint);
}

function pipelineEmitUnspecified(node: Node): void {
const kind = node.kind;

Expand Down Expand Up @@ -898,9 +913,9 @@ namespace ts {
function emitParameter(node: ParameterDeclaration) {
emitDecorators(node, node.decorators);
emitModifiers(node, node.modifiers);
writeIfPresent(node.dotDotDotToken, "...");
emitIfPresent(node.dotDotDotToken);
emit(node.name);
writeIfPresent(node.questionToken, "?");
emitIfPresent(node.questionToken);
emitWithPrefix(": ", node.type);
emitExpressionWithPrefix(" = ", node.initializer);
}
Expand All @@ -918,7 +933,7 @@ namespace ts {
emitDecorators(node, node.decorators);
emitModifiers(node, node.modifiers);
emit(node.name);
writeIfPresent(node.questionToken, "?");
emitIfPresent(node.questionToken);
emitWithPrefix(": ", node.type);
write(";");
}
Expand All @@ -927,7 +942,7 @@ namespace ts {
emitDecorators(node, node.decorators);
emitModifiers(node, node.modifiers);
emit(node.name);
writeIfPresent(node.questionToken, "?");
emitIfPresent(node.questionToken);
emitWithPrefix(": ", node.type);
emitExpressionWithPrefix(" = ", node.initializer);
write(";");
Expand All @@ -937,7 +952,7 @@ namespace ts {
emitDecorators(node, node.decorators);
emitModifiers(node, node.modifiers);
emit(node.name);
writeIfPresent(node.questionToken, "?");
emitIfPresent(node.questionToken);
emitTypeParameters(node, node.typeParameters);
emitParameters(node, node.parameters);
emitWithPrefix(": ", node.type);
Expand All @@ -947,9 +962,9 @@ namespace ts {
function emitMethodDeclaration(node: MethodDeclaration) {
emitDecorators(node, node.decorators);
emitModifiers(node, node.modifiers);
writeIfPresent(node.asteriskToken, "*");
emitIfPresent(node.asteriskToken);
emit(node.name);
writeIfPresent(node.questionToken, "?");
emitIfPresent(node.questionToken);
emitSignatureAndBody(node, emitSignatureHead);
}

Expand Down Expand Up @@ -1035,10 +1050,8 @@ namespace ts {

function emitTypeLiteral(node: TypeLiteralNode) {
write("{");
// If the literal is empty, do not add spaces between braces.
if (node.members.length > 0) {
emitList(node, node.members, getEmitFlags(node) & EmitFlags.SingleLine ? ListFormat.SingleLineTypeLiteralMembers : ListFormat.MultiLineTypeLiteralMembers);
}
const flags = getEmitFlags(node) & EmitFlags.SingleLine ? ListFormat.SingleLineTypeLiteralMembers : ListFormat.MultiLineTypeLiteralMembers;
emitList(node, node.members, flags | ListFormat.NoSpaceIfEmpty);
write("}");
}

Expand Down Expand Up @@ -1094,13 +1107,16 @@ namespace ts {
writeLine();
increaseIndent();
}
writeIfPresent(node.readonlyToken, "readonly ");
if (node.readonlyToken) {
emit(node.readonlyToken);
write(" ");
}

write("[");
emit(node.typeParameter.name);
write(" in ");
emit(node.typeParameter.constraint);
pipelineEmitWithNotification(EmitHint.MappedTypeParameter, node.typeParameter);
write("]");
writeIfPresent(node.questionToken, "?");

emitIfPresent(node.questionToken);
write(": ");
emit(node.type);
write(";");
Expand Down Expand Up @@ -1148,7 +1164,7 @@ namespace ts {

function emitBindingElement(node: BindingElement) {
emitWithSuffix(node.propertyName, ": ");
writeIfPresent(node.dotDotDotToken, "...");
emitIfPresent(node.dotDotDotToken);
emit(node.name);
emitExpressionWithPrefix(" = ", node.initializer);
}
Expand All @@ -1159,33 +1175,22 @@ namespace ts {

function emitArrayLiteralExpression(node: ArrayLiteralExpression) {
const elements = node.elements;
if (elements.length === 0) {
write("[]");
}
else {
const preferNewLine = node.multiLine ? ListFormat.PreferNewLine : ListFormat.None;
emitExpressionList(node, elements, ListFormat.ArrayLiteralExpressionElements | preferNewLine);
}
const preferNewLine = node.multiLine ? ListFormat.PreferNewLine : ListFormat.None;
emitExpressionList(node, elements, ListFormat.ArrayLiteralExpressionElements | preferNewLine);
}

function emitObjectLiteralExpression(node: ObjectLiteralExpression) {
const properties = node.properties;
if (properties.length === 0) {
write("{}");
const indentedFlag = getEmitFlags(node) & EmitFlags.Indented;
if (indentedFlag) {
increaseIndent();
}
else {
const indentedFlag = getEmitFlags(node) & EmitFlags.Indented;
if (indentedFlag) {
increaseIndent();
}

const preferNewLine = node.multiLine ? ListFormat.PreferNewLine : ListFormat.None;
const allowTrailingComma = currentSourceFile.languageVersion >= ScriptTarget.ES5 ? ListFormat.AllowTrailingComma : ListFormat.None;
emitList(node, properties, ListFormat.ObjectLiteralExpressionProperties | allowTrailingComma | preferNewLine);
const preferNewLine = node.multiLine ? ListFormat.PreferNewLine : ListFormat.None;
const allowTrailingComma = currentSourceFile.languageVersion >= ScriptTarget.ES5 ? ListFormat.AllowTrailingComma : ListFormat.None;
emitList(node, node.properties, ListFormat.ObjectLiteralExpressionProperties | allowTrailingComma | preferNewLine);

if (indentedFlag) {
decreaseIndent();
}
if (indentedFlag) {
decreaseIndent();
}
}

Expand Down Expand Up @@ -1286,7 +1291,8 @@ namespace ts {
emitTypeParameters(node, node.typeParameters);
emitParametersForArrow(node, node.parameters);
emitWithPrefix(": ", node.type);
write(" =>");
write(" ");
emit(node.equalsGreaterThanToken);
}

function emitDeleteExpression(node: DeleteExpression) {
Expand Down Expand Up @@ -1364,13 +1370,13 @@ namespace ts {

emitExpression(node.condition);
increaseIndentIf(indentBeforeQuestion, " ");
write("?");
emit(node.questionToken);
increaseIndentIf(indentAfterQuestion, " ");
emitExpression(node.whenTrue);
decreaseIndentIf(indentBeforeQuestion, indentAfterQuestion);

increaseIndentIf(indentBeforeColon, " ");
write(":");
emit(node.colonToken);
increaseIndentIf(indentAfterColon, " ");
emitExpression(node.whenFalse);
decreaseIndentIf(indentBeforeColon, indentAfterColon);
Expand All @@ -1382,7 +1388,8 @@ namespace ts {
}

function emitYieldExpression(node: YieldExpression) {
write(node.asteriskToken ? "yield*" : "yield");
write("yield");
emit(node.asteriskToken);
emitExpressionWithPrefix(" ", node.expression);
}

Expand Down Expand Up @@ -1662,7 +1669,9 @@ namespace ts {
function emitFunctionDeclarationOrExpression(node: FunctionDeclaration | FunctionExpression) {
emitDecorators(node, node.decorators);
emitModifiers(node, node.modifiers);
write(node.asteriskToken ? "function* " : "function ");
write("function");
Copy link
Contributor

@rbuckton rbuckton Sep 6, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

No need to guard against undefined as that is already handled in emit

Edit: I was incorrect. Apparently emit does not have this guard.

Copy link
Author

@ghost ghost Sep 7, 2017 •
edited by ghost
Loading

Choose a reason for hiding this comment

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

I get a test failure in the test PrinterAPI printFile removeComments if I try to emit the asterisk unconditionally. emit doesn't seem to check for an undefined node.

emitIfPresent(node.asteriskToken);
write(" ");
emitIdentifierName(node.name);
emitSignatureAndBody(node, emitSignatureHead);
}
Expand Down Expand Up @@ -2068,9 +2077,7 @@ namespace ts {
function emitJsxExpression(node: JsxExpression) {
if (node.expression) {
write("{");
if (node.dotDotDotToken) {
write("...");
}
emitIfPresent(node.dotDotDotToken);
emitExpression(node.expression);
write("}");
}
Expand Down Expand Up @@ -2128,13 +2135,12 @@ namespace ts {
emitTrailingCommentsOfPosition(statements.pos);
}

let format = ListFormat.CaseOrDefaultClauseStatements;
if (emitAsSingleStatement) {
write(" ");
emit(statements[0]);
}
else {
emitList(parentNode, statements, ListFormat.CaseOrDefaultClauseStatements);
format &= ~(ListFormat.MultiLine | ListFormat.Indented);
}
emitList(parentNode, statements, format);
}

function emitHeritageClause(node: HeritageClause) {
Expand Down Expand Up @@ -2384,7 +2390,7 @@ namespace ts {

function emitParametersForArrow(parentNode: FunctionTypeNode | ArrowFunction, parameters: NodeArray<ParameterDeclaration>) {
if (canEmitSimpleArrowHead(parentNode, parameters)) {
emit(parameters[0]);
emitSingleElementList(parameters);
}
else {
emitParameters(parentNode, parameters);
Expand All @@ -2395,6 +2401,17 @@ namespace ts {
emitList(parentNode, parameters, ListFormat.IndexSignatureParameters);
}

function emitSingleElementList(list: NodeArray<Node>) {
Copy link
Contributor

@rbuckton rbuckton Sep 6, 2017

Choose a reason for hiding this comment

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

Can you craft suitable ListFormat bitmasks to use instead and just call emitList where applicable instead of creating a special cased list emit function?

Debug.assert(list.length === 1);
if (onBeforeEmitNodeArray) {
onBeforeEmitNodeArray(list);
}
emit(list[0]);
if (onAfterEmitNodeArray) {
onAfterEmitNodeArray(list);
}
}

function emitList(parentNode: Node, children: NodeArray<Node>, format: ListFormat, start?: number, count?: number) {
emitNodeList(emit, parentNode, children, format, start, count);
}
Expand Down Expand Up @@ -2427,7 +2444,7 @@ namespace ts {
if (format & ListFormat.MultiLine) {
writeLine();
}
else if (format & ListFormat.SpaceBetweenBraces) {
else if (format & ListFormat.SpaceBetweenBraces && !(format & ListFormat.NoSpaceIfEmpty)) {
write(" ");
}
}
Expand Down Expand Up @@ -2568,12 +2585,6 @@ namespace ts {
}
}

function writeIfPresent(node: Node, text: string) {
if (node) {
write(text);
}
}

function writeToken(token: SyntaxKind, pos: number, contextNode?: Node) {
return onEmitSourceMapOfToken
? onEmitSourceMapOfToken(contextNode, token, pos, writeTokenText)
Expand All @@ -2584,7 +2595,7 @@ namespace ts {
if (onBeforeEmitToken) {
onBeforeEmitToken(node);
}
writeTokenText(node.kind);
write(tokenToString(node.kind));
if (onAfterEmitToken) {
onAfterEmitToken(node);
}
Expand Down Expand Up @@ -3107,6 +3118,9 @@ namespace ts {
NoTrailingNewLine = 1 << 16, // Do not emit a trailing NewLine for a MultiLine list.
NoInterveningComments = 1 << 17, // Do not emit comments between each node

NoSpaceIfEmpty = 1 << 18, // If the literal is empty, do not add spaces between braces.
SingleElement = 1 << 19,

// Precomputed Formats
Modifiers = SingleLine | SpaceBetweenSiblings | NoInterveningComments,
HeritageClauses = SingleLine | SpaceBetweenSiblings,
Expand All @@ -3118,7 +3132,7 @@ namespace ts {
IntersectionTypeConstituents = AmpersandDelimited | SpaceBetweenSiblings | SingleLine,
ObjectBindingPatternElements = SingleLine | AllowTrailingComma | SpaceBetweenBraces | CommaDelimited | SpaceBetweenSiblings,
ArrayBindingPatternElements = SingleLine | AllowTrailingComma | CommaDelimited | SpaceBetweenSiblings,
ObjectLiteralExpressionProperties = PreserveLines | CommaDelimited | SpaceBetweenSiblings | SpaceBetweenBraces | Indented | Braces,
ObjectLiteralExpressionProperties = PreserveLines | CommaDelimited | SpaceBetweenSiblings | SpaceBetweenBraces | Indented | Braces | NoSpaceIfEmpty,
ArrayLiteralExpressionElements = PreserveLines | CommaDelimited | SpaceBetweenSiblings | AllowTrailingComma | Indented | SquareBrackets,
CommaListElements = CommaDelimited | SpaceBetweenSiblings | SingleLine,
CallExpressionArguments = CommaDelimited | SpaceBetweenSiblings | SingleLine | Parenthesis,
Expand Down
Loading