From d795a001375a7d06592450d6f57a4f05d4d3e8e9 Mon Sep 17 00:00:00 2001 From: Pete Bacon Darwin Date: Fri, 11 Sep 2020 16:43:23 +0100 Subject: [PATCH] refactor(compiler): replace Comment nodes with leadingComments property (#38811) Common AST formats such as TS and Babel do not use a separate node for comments, but instead attach comments to other AST nodes. Previously this was worked around in TS by creating a `NotEmittedStatement` AST node to attach the comment to. But Babel does not have this facility, so it will not be a viable approach for the linker. This commit refactors the output AST, to remove the `CommentStmt` and `JSDocCommentStmt` nodes. Instead statements have a collection of `leadingComments` that are rendered/attached to the final AST nodes when being translated or printed. PR Close #38811 --- packages/compiler-cli/BUILD.bazel | 1 + .../ngcc/src/rendering/renderer.ts | 28 +-- .../ngcc/test/integration/ngcc_spec.ts | 4 +- .../ngtsc/modulewithproviders/src/scanner.ts | 2 +- .../src/ngtsc/translator/index.ts | 2 +- .../src/ngtsc/translator/src/translator.ts | 111 +++++---- .../src/ngtsc/typecheck/src/environment.ts | 3 +- .../src/transformers/node_emitter.ts | 131 +++++------ .../transformers/node_emitter_transform.ts | 19 +- .../compiler-cli/src/transformers/util.ts | 9 + packages/compiler-cli/test/ngc_spec.ts | 12 +- .../test/transformers/node_emitter_spec.ts | 65 ++++-- packages/compiler/src/compiler.ts | 2 +- .../compiler/src/output/abstract_emitter.ts | 37 +-- packages/compiler/src/output/output_ast.ts | 212 +++++++++--------- .../compiler/src/output/output_interpreter.ts | 6 - packages/compiler/src/render3/util.ts | 14 +- .../src/render3/view/i18n/get_msg_utils.ts | 16 +- .../compiler/src/render3/view/i18n/meta.ts | 4 +- .../compiler/src/render3/view/i18n/util.ts | 2 +- .../compiler/src/render3/view/template.ts | 2 +- .../compiler/test/output/js_emitter_spec.ts | 45 +++- .../compiler/test/output/output_ast_spec.ts | 17 -- .../compiler/test/output/ts_emitter_spec.ts | 46 ++-- 24 files changed, 427 insertions(+), 363 deletions(-) diff --git a/packages/compiler-cli/BUILD.bazel b/packages/compiler-cli/BUILD.bazel index 27885e70af..62f6ce86ec 100644 --- a/packages/compiler-cli/BUILD.bazel +++ b/packages/compiler-cli/BUILD.bazel @@ -32,6 +32,7 @@ ts_library( "//packages/compiler-cli/src/ngtsc/perf", "//packages/compiler-cli/src/ngtsc/reflection", "//packages/compiler-cli/src/ngtsc/shims", + "//packages/compiler-cli/src/ngtsc/translator", "//packages/compiler-cli/src/ngtsc/typecheck", "@npm//@bazel/typescript", "@npm//@types/node", diff --git a/packages/compiler-cli/ngcc/src/rendering/renderer.ts b/packages/compiler-cli/ngcc/src/rendering/renderer.ts index cffe8b0639..8e02accfda 100644 --- a/packages/compiler-cli/ngcc/src/rendering/renderer.ts +++ b/packages/compiler-cli/ngcc/src/rendering/renderer.ts @@ -5,7 +5,7 @@ * Use of this source code is governed by an MIT-style license that can be * found in the LICENSE file at https://angular.io/license */ -import {CommentStmt, ConstantPool, Expression, Statement, WrappedNodeExpr, WritePropExpr} from '@angular/compiler'; +import {ConstantPool, Expression, jsDocComment, LeadingComment, Statement, WrappedNodeExpr, WritePropExpr} from '@angular/compiler'; import MagicString from 'magic-string'; import * as ts from 'typescript'; @@ -166,11 +166,11 @@ export class Renderer { sourceFile: ts.SourceFile, compiledClass: CompiledClass, imports: ImportManager, annotateForClosureCompiler: boolean): string { const name = this.host.getInternalNameOfClass(compiledClass.declaration); - const statements: Statement[][] = compiledClass.compilation.map(c => { - return createAssignmentStatements( - name, c.name, c.initializer, annotateForClosureCompiler ? '* @nocollapse ' : undefined); - }); - return this.renderStatements(sourceFile, Array.prototype.concat.apply([], statements), imports); + const leadingComment = + annotateForClosureCompiler ? jsDocComment([{tagName: 'nocollapse'}]) : undefined; + const statements: Statement[] = compiledClass.compilation.map( + c => createAssignmentStatement(name, c.name, c.initializer, leadingComment)); + return this.renderStatements(sourceFile, statements, imports); } /** @@ -213,16 +213,16 @@ export function renderConstantPool( * compiled decorator to be applied to the class. * @param analyzedClass The info about the class whose statement we want to create. */ -function createAssignmentStatements( +function createAssignmentStatement( receiverName: ts.DeclarationName, propName: string, initializer: Expression, - leadingComment?: string): Statement[] { + leadingComment?: LeadingComment): Statement { const receiver = new WrappedNodeExpr(receiverName); - const statements = - [new WritePropExpr( - receiver, propName, initializer, /* type */ undefined, /* sourceSpan */ undefined) - .toStmt()]; + const statement = + new WritePropExpr( + receiver, propName, initializer, /* type */ undefined, /* sourceSpan */ undefined) + .toStmt(); if (leadingComment !== undefined) { - statements.unshift(new CommentStmt(leadingComment, true)); + statement.addLeadingComment(leadingComment); } - return statements; + return statement; } diff --git a/packages/compiler-cli/ngcc/test/integration/ngcc_spec.ts b/packages/compiler-cli/ngcc/test/integration/ngcc_spec.ts index edbc732621..16c7dc8935 100644 --- a/packages/compiler-cli/ngcc/test/integration/ngcc_spec.ts +++ b/packages/compiler-cli/ngcc/test/integration/ngcc_spec.ts @@ -1663,12 +1663,12 @@ runInEachFileSystem(() => { JSON.stringify({angularCompilerOptions: {annotateForClosureCompiler: true}})); mainNgcc({basePath: '/dist', propertiesToConsider: ['es2015']}); const jsContents = fs.readFile(_(`/dist/local-package/index.js`)); - expect(jsContents).toContain('/** @nocollapse */ \nAppComponent.ɵcmp ='); + expect(jsContents).toContain('/** @nocollapse */\nAppComponent.ɵcmp ='); }); it('should default to not give closure annotated output', () => { mainNgcc({basePath: '/dist', propertiesToConsider: ['es2015']}); const jsContents = fs.readFile(_(`/dist/local-package/index.js`)); - expect(jsContents).not.toContain('/** @nocollapse */'); + expect(jsContents).not.toContain('@nocollapse'); }); }); diff --git a/packages/compiler-cli/src/ngtsc/modulewithproviders/src/scanner.ts b/packages/compiler-cli/src/ngtsc/modulewithproviders/src/scanner.ts index 0ccb7eba16..f5635550e3 100644 --- a/packages/compiler-cli/src/ngtsc/modulewithproviders/src/scanner.ts +++ b/packages/compiler-cli/src/ngtsc/modulewithproviders/src/scanner.ts @@ -103,7 +103,7 @@ export class ModuleWithProvidersScanner { this.emitter.emit(ngModule, decl.getSourceFile(), ImportFlags.ForceNewImport); const ngModuleType = new ExpressionType(ngModuleExpr); const mwpNgType = new ExpressionType( - new ExternalExpr(Identifiers.ModuleWithProviders), /* modifiers */ null, [ngModuleType]); + new ExternalExpr(Identifiers.ModuleWithProviders), [/* modifiers */], [ngModuleType]); dts.addTypeReplacement(decl, mwpNgType); } diff --git a/packages/compiler-cli/src/ngtsc/translator/index.ts b/packages/compiler-cli/src/ngtsc/translator/index.ts index 0e2dab70c5..64338c5488 100644 --- a/packages/compiler-cli/src/ngtsc/translator/index.ts +++ b/packages/compiler-cli/src/ngtsc/translator/index.ts @@ -6,4 +6,4 @@ * found in the LICENSE file at https://angular.io/license */ -export {Import, ImportManager, NamedImport, translateExpression, translateStatement, translateType} from './src/translator'; +export {attachComments, Import, ImportManager, NamedImport, translateExpression, translateStatement, translateType} from './src/translator'; diff --git a/packages/compiler-cli/src/ngtsc/translator/src/translator.ts b/packages/compiler-cli/src/ngtsc/translator/src/translator.ts index 6a24078d3b..84c1d926f1 100644 --- a/packages/compiler-cli/src/ngtsc/translator/src/translator.ts +++ b/packages/compiler-cli/src/ngtsc/translator/src/translator.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {ArrayType, AssertNotNull, BinaryOperator, BinaryOperatorExpr, BuiltinType, BuiltinTypeName, CastExpr, ClassStmt, CommaExpr, CommentStmt, ConditionalExpr, DeclareFunctionStmt, DeclareVarStmt, Expression, ExpressionStatement, ExpressionType, ExpressionVisitor, ExternalExpr, FunctionExpr, IfStmt, InstantiateExpr, InvokeFunctionExpr, InvokeMethodExpr, JSDocCommentStmt, LiteralArrayExpr, LiteralExpr, LiteralMapExpr, MapType, NotExpr, ParseSourceSpan, ReadKeyExpr, ReadPropExpr, ReadVarExpr, ReturnStatement, Statement, StatementVisitor, StmtModifier, ThrowStmt, TryCatchStmt, Type, TypeofExpr, TypeVisitor, WrappedNodeExpr, WriteKeyExpr, WritePropExpr, WriteVarExpr} from '@angular/compiler'; +import {ArrayType, AssertNotNull, BinaryOperator, BinaryOperatorExpr, BuiltinType, BuiltinTypeName, CastExpr, ClassStmt, CommaExpr, ConditionalExpr, DeclareFunctionStmt, DeclareVarStmt, Expression, ExpressionStatement, ExpressionType, ExpressionVisitor, ExternalExpr, FunctionExpr, IfStmt, InstantiateExpr, InvokeFunctionExpr, InvokeMethodExpr, LeadingComment, LiteralArrayExpr, LiteralExpr, LiteralMapExpr, MapType, NotExpr, ParseSourceSpan, ReadKeyExpr, ReadPropExpr, ReadVarExpr, ReturnStatement, Statement, StatementVisitor, StmtModifier, ThrowStmt, TryCatchStmt, Type, TypeofExpr, TypeVisitor, WrappedNodeExpr, WriteKeyExpr, WritePropExpr, WriteVarExpr} from '@angular/compiler'; import {LocalizedString, UnaryOperator, UnaryOperatorExpr} from '@angular/compiler/src/output/output_ast'; import * as ts from 'typescript'; @@ -134,34 +134,45 @@ class ExpressionTranslatorVisitor implements ExpressionVisitor, StatementVisitor private scriptTarget: Exclude) {} visitDeclareVarStmt(stmt: DeclareVarStmt, context: Context): ts.VariableStatement { - const nodeFlags = - ((this.scriptTarget >= ts.ScriptTarget.ES2015) && stmt.hasModifier(StmtModifier.Final)) ? - ts.NodeFlags.Const : - ts.NodeFlags.None; - return ts.createVariableStatement( - undefined, - ts.createVariableDeclarationList( - [ts.createVariableDeclaration( - stmt.name, undefined, - stmt.value && stmt.value.visitExpression(this, context.withExpressionMode))], - nodeFlags)); + const isConst = + this.scriptTarget >= ts.ScriptTarget.ES2015 && stmt.hasModifier(StmtModifier.Final); + const varDeclaration = ts.createVariableDeclaration( + /* name */ stmt.name, + /* type */ undefined, + /* initializer */ stmt.value?.visitExpression(this, context.withExpressionMode)); + const declarationList = ts.createVariableDeclarationList( + /* declarations */[varDeclaration], + /* flags */ isConst ? ts.NodeFlags.Const : ts.NodeFlags.None); + const varStatement = ts.createVariableStatement(undefined, declarationList); + return attachComments(varStatement, stmt.leadingComments); } visitDeclareFunctionStmt(stmt: DeclareFunctionStmt, context: Context): ts.FunctionDeclaration { - return ts.createFunctionDeclaration( - undefined, undefined, undefined, stmt.name, undefined, + const fnDeclaration = ts.createFunctionDeclaration( + /* decorators */ undefined, + /* modifiers */ undefined, + /* asterisk */ undefined, + /* name */ stmt.name, + /* typeParameters */ undefined, + /* parameters */ stmt.params.map(param => ts.createParameter(undefined, undefined, undefined, param.name)), - undefined, + /* type */ undefined, + /* body */ ts.createBlock( stmt.statements.map(child => child.visitStatement(this, context.withStatementMode)))); + return attachComments(fnDeclaration, stmt.leadingComments); } visitExpressionStmt(stmt: ExpressionStatement, context: Context): ts.ExpressionStatement { - return ts.createStatement(stmt.expr.visitExpression(this, context.withStatementMode)); + return attachComments( + ts.createStatement(stmt.expr.visitExpression(this, context.withStatementMode)), + stmt.leadingComments); } visitReturnStmt(stmt: ReturnStatement, context: Context): ts.ReturnStatement { - return ts.createReturn(stmt.value.visitExpression(this, context.withExpressionMode)); + return attachComments( + ts.createReturn(stmt.value.visitExpression(this, context.withExpressionMode)), + stmt.leadingComments); } visitDeclareClassStmt(stmt: ClassStmt, context: Context) { @@ -174,14 +185,15 @@ class ExpressionTranslatorVisitor implements ExpressionVisitor, StatementVisitor } visitIfStmt(stmt: IfStmt, context: Context): ts.IfStatement { - return ts.createIf( - stmt.condition.visitExpression(this, context), + const thenBlock = ts.createBlock( + stmt.trueCase.map(child => child.visitStatement(this, context.withStatementMode))); + const elseBlock = stmt.falseCase.length > 0 ? ts.createBlock( - stmt.trueCase.map(child => child.visitStatement(this, context.withStatementMode))), - stmt.falseCase.length > 0 ? - ts.createBlock(stmt.falseCase.map( - child => child.visitStatement(this, context.withStatementMode))) : - undefined); + stmt.falseCase.map(child => child.visitStatement(this, context.withStatementMode))) : + undefined; + const ifStatement = + ts.createIf(stmt.condition.visitExpression(this, context), thenBlock, elseBlock); + return attachComments(ifStatement, stmt.leadingComments); } visitTryCatchStmt(stmt: TryCatchStmt, context: Context) { @@ -189,25 +201,9 @@ class ExpressionTranslatorVisitor implements ExpressionVisitor, StatementVisitor } visitThrowStmt(stmt: ThrowStmt, context: Context): ts.ThrowStatement { - return ts.createThrow(stmt.error.visitExpression(this, context.withExpressionMode)); - } - - visitCommentStmt(stmt: CommentStmt, context: Context): ts.NotEmittedStatement { - const commentStmt = ts.createNotEmittedStatement(ts.createLiteral('')); - ts.addSyntheticLeadingComment( - commentStmt, - stmt.multiline ? ts.SyntaxKind.MultiLineCommentTrivia : - ts.SyntaxKind.SingleLineCommentTrivia, - stmt.comment, /** hasTrailingNewLine */ false); - return commentStmt; - } - - visitJSDocCommentStmt(stmt: JSDocCommentStmt, context: Context): ts.NotEmittedStatement { - const commentStmt = ts.createNotEmittedStatement(ts.createLiteral('')); - const text = stmt.toString(); - const kind = ts.SyntaxKind.MultiLineCommentTrivia; - ts.setSyntheticLeadingComments(commentStmt, [{kind, text, pos: -1, end: -1}]); - return commentStmt; + return attachComments( + ts.createThrow(stmt.error.visitExpression(this, context.withExpressionMode)), + stmt.leadingComments); } visitReadVarExpr(ast: ReadVarExpr, context: Context): ts.Identifier { @@ -784,4 +780,31 @@ function createTemplateTail(cooked: string, raw: string): ts.TemplateTail { const node: ts.TemplateLiteralLikeNode = ts.createTemplateHead(cooked, raw); (node.kind as ts.SyntaxKind) = ts.SyntaxKind.TemplateTail; return node as ts.TemplateTail; -} \ No newline at end of file +} + +/** + * Attach the given `leadingComments` to the `statement` node. + * + * @param statement The statement that will have comments attached. + * @param leadingComments The comments to attach to the statement. + */ +export function attachComments( + statement: T, leadingComments?: LeadingComment[]): T { + if (leadingComments === undefined) { + return statement; + } + + for (const comment of leadingComments) { + const commentKind = comment.multiline ? ts.SyntaxKind.MultiLineCommentTrivia : + ts.SyntaxKind.SingleLineCommentTrivia; + if (comment.multiline) { + ts.addSyntheticLeadingComment( + statement, commentKind, comment.toString(), comment.trailingNewline); + } else { + for (const line of comment.text.split('\n')) { + ts.addSyntheticLeadingComment(statement, commentKind, line, comment.trailingNewline); + } + } + } + return statement; +} diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/environment.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/environment.ts index bfa79ad8c4..48b1bca849 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/environment.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/environment.ts @@ -245,7 +245,8 @@ export class Environment { */ referenceExternalType(moduleName: string, name: string, typeParams?: Type[]): ts.TypeNode { const external = new ExternalExpr({moduleName, name}); - return translateType(new ExpressionType(external, null, typeParams), this.importManager); + return translateType( + new ExpressionType(external, [/* modifiers */], typeParams), this.importManager); } getPreludeStatements(): ts.Statement[] { diff --git a/packages/compiler-cli/src/transformers/node_emitter.ts b/packages/compiler-cli/src/transformers/node_emitter.ts index 5ad9e0b2e3..333c324d50 100644 --- a/packages/compiler-cli/src/transformers/node_emitter.ts +++ b/packages/compiler-cli/src/transformers/node_emitter.ts @@ -6,10 +6,10 @@ * found in the LICENSE file at https://angular.io/license */ -import {AssertNotNull, BinaryOperator, BinaryOperatorExpr, BuiltinMethod, BuiltinVar, CastExpr, ClassStmt, CommaExpr, CommentStmt, ConditionalExpr, DeclareFunctionStmt, DeclareVarStmt, ExpressionStatement, ExpressionVisitor, ExternalExpr, ExternalReference, FunctionExpr, IfStmt, InstantiateExpr, InvokeFunctionExpr, InvokeMethodExpr, JSDocCommentStmt, LiteralArrayExpr, LiteralExpr, LiteralMapExpr, NotExpr, ParseSourceFile, ParseSourceSpan, PartialModule, ReadKeyExpr, ReadPropExpr, ReadVarExpr, ReturnStatement, Statement, StatementVisitor, StmtModifier, ThrowStmt, TryCatchStmt, TypeofExpr, WrappedNodeExpr, WriteKeyExpr, WritePropExpr, WriteVarExpr} from '@angular/compiler'; -import {LocalizedString, UnaryOperator, UnaryOperatorExpr} from '@angular/compiler/src/output/output_ast'; +import {AssertNotNull, BinaryOperator, BinaryOperatorExpr, BuiltinMethod, BuiltinVar, CastExpr, ClassStmt, CommaExpr, ConditionalExpr, DeclareFunctionStmt, DeclareVarStmt, ExpressionStatement, ExpressionVisitor, ExternalExpr, ExternalReference, FunctionExpr, IfStmt, InstantiateExpr, InvokeFunctionExpr, InvokeMethodExpr, LeadingComment, leadingComment, LiteralArrayExpr, LiteralExpr, LiteralMapExpr, LocalizedString, NotExpr, ParseSourceFile, ParseSourceSpan, PartialModule, ReadKeyExpr, ReadPropExpr, ReadVarExpr, ReturnStatement, Statement, StatementVisitor, StmtModifier, ThrowStmt, TryCatchStmt, TypeofExpr, UnaryOperator, UnaryOperatorExpr, WrappedNodeExpr, WriteKeyExpr, WritePropExpr, WriteVarExpr} from '@angular/compiler'; import * as ts from 'typescript'; +import {attachComments} from '../ngtsc/translator'; import {error} from './util'; export interface Node { @@ -31,30 +31,25 @@ export class TypeScriptNodeEmitter { // stmts. const statements: any[] = [].concat( ...stmts.map(stmt => stmt.visitStatement(converter, null)).filter(stmt => stmt != null)); - const preambleStmts: ts.Statement[] = []; - if (preamble) { - const commentStmt = this.createCommentStatement(sourceFile, preamble); - preambleStmts.push(commentStmt); - } const sourceStatements = - [...preambleStmts, ...converter.getReexports(), ...converter.getImports(), ...statements]; + [...converter.getReexports(), ...converter.getImports(), ...statements]; + if (preamble) { + // We always attach the preamble comment to a `NotEmittedStatement` node, because tsickle uses + // this node type as a marker of the preamble to ensure that it adds its own new nodes after + // the preamble. + const preambleCommentHolder = ts.createNotEmittedStatement(sourceFile); + // Preamble comments are passed through as-is, which means that they must already contain a + // leading `*` if they should be a JSDOC comment. + ts.addSyntheticLeadingComment( + preambleCommentHolder, ts.SyntaxKind.MultiLineCommentTrivia, preamble, + /* hasTrailingNewline */ true); + sourceStatements.unshift(preambleCommentHolder); + } + converter.updateSourceMap(sourceStatements); const newSourceFile = ts.updateSourceFileNode(sourceFile, sourceStatements); return [newSourceFile, converter.getNodeMap()]; } - - /** Creates a not emitted statement containing the given comment. */ - createCommentStatement(sourceFile: ts.SourceFile, comment: string): ts.Statement { - if (comment.startsWith('/*') && comment.endsWith('*/')) { - comment = comment.substr(2, comment.length - 4); - } - const commentStmt = ts.createNotEmittedStatement(sourceFile); - ts.setSyntheticLeadingComments( - commentStmt, - [{kind: ts.SyntaxKind.MultiLineCommentTrivia, text: comment, pos: -1, end: -1}]); - ts.setEmitFlags(commentStmt, ts.EmitFlags.CustomPrologue); - return commentStmt; - } } /** @@ -288,10 +283,13 @@ export class NodeEmitterVisitor implements StatementVisitor, ExpressionVisitor { recordLastSourceRange(); } - private record(ngNode: Node, tsNode: T|null): RecordedNode { + private postProcess(ngNode: Node, tsNode: T|null): RecordedNode { if (tsNode && !this._nodeMap.has(tsNode)) { this._nodeMap.set(tsNode, ngNode); } + if (tsNode !== null && ngNode instanceof Statement) { + attachComments(tsNode as unknown as ts.Statement, ngNode.leadingComments); + } return tsNode as RecordedNode; } @@ -347,19 +345,19 @@ export class NodeEmitterVisitor implements StatementVisitor, ExpressionVisitor { // Note: We need to add an explicit variable and export declaration so that // the variable can be referred in the same file as well. const tsVarStmt = - this.record(stmt, ts.createVariableStatement(/* modifiers */[], varDeclList)); - const exportStmt = this.record( + this.postProcess(stmt, ts.createVariableStatement(/* modifiers */[], varDeclList)); + const exportStmt = this.postProcess( stmt, ts.createExportDeclaration( /*decorators*/ undefined, /*modifiers*/ undefined, ts.createNamedExports([ts.createExportSpecifier(stmt.name, stmt.name)]))); return [tsVarStmt, exportStmt]; } - return this.record(stmt, ts.createVariableStatement(this.getModifiers(stmt), varDeclList)); + return this.postProcess(stmt, ts.createVariableStatement(this.getModifiers(stmt), varDeclList)); } visitDeclareFunctionStmt(stmt: DeclareFunctionStmt) { - return this.record( + return this.postProcess( stmt, ts.createFunctionDeclaration( /* decorators */ undefined, this.getModifiers(stmt), @@ -372,11 +370,11 @@ export class NodeEmitterVisitor implements StatementVisitor, ExpressionVisitor { } visitExpressionStmt(stmt: ExpressionStatement) { - return this.record(stmt, ts.createStatement(stmt.expr.visitExpression(this, null))); + return this.postProcess(stmt, ts.createStatement(stmt.expr.visitExpression(this, null))); } visitReturnStmt(stmt: ReturnStatement) { - return this.record( + return this.postProcess( stmt, ts.createReturn(stmt.value ? stmt.value.visitExpression(this, null) : undefined)); } @@ -434,7 +432,7 @@ export class NodeEmitterVisitor implements StatementVisitor, ExpressionVisitor { /* decorators */ undefined, /* modifiers */ undefined, /* dotDotDotToken */ undefined, p.name)), /* type */ undefined, this._visitStatements(method.body))); - return this.record( + return this.postProcess( stmt, ts.createClassDeclaration( /* decorators */ undefined, modifiers, stmt.name, /* typeParameters*/ undefined, @@ -446,7 +444,7 @@ export class NodeEmitterVisitor implements StatementVisitor, ExpressionVisitor { } visitIfStmt(stmt: IfStmt) { - return this.record( + return this.postProcess( stmt, ts.createIf( stmt.condition.visitExpression(this, null), this._visitStatements(stmt.trueCase), @@ -455,7 +453,7 @@ export class NodeEmitterVisitor implements StatementVisitor, ExpressionVisitor { } visitTryCatchStmt(stmt: TryCatchStmt): RecordedNode { - return this.record( + return this.postProcess( stmt, ts.createTry( this._visitStatements(stmt.bodyStmts), @@ -474,64 +472,46 @@ export class NodeEmitterVisitor implements StatementVisitor, ExpressionVisitor { } visitThrowStmt(stmt: ThrowStmt) { - return this.record(stmt, ts.createThrow(stmt.error.visitExpression(this, null))); - } - - visitCommentStmt(stmt: CommentStmt, sourceFile: ts.SourceFile) { - const text = stmt.multiline ? ` ${stmt.comment} ` : ` ${stmt.comment}`; - return this.createCommentStmt(text, stmt.multiline, sourceFile); - } - - visitJSDocCommentStmt(stmt: JSDocCommentStmt, sourceFile: ts.SourceFile) { - return this.createCommentStmt(stmt.toString(), true, sourceFile); - } - - private createCommentStmt(text: string, multiline: boolean, sourceFile: ts.SourceFile): - ts.NotEmittedStatement { - const commentStmt = ts.createNotEmittedStatement(sourceFile); - const kind = - multiline ? ts.SyntaxKind.MultiLineCommentTrivia : ts.SyntaxKind.SingleLineCommentTrivia; - ts.setSyntheticLeadingComments(commentStmt, [{kind, text, pos: -1, end: -1}]); - return commentStmt; + return this.postProcess(stmt, ts.createThrow(stmt.error.visitExpression(this, null))); } // ExpressionVisitor visitWrappedNodeExpr(expr: WrappedNodeExpr) { - return this.record(expr, expr.node); + return this.postProcess(expr, expr.node); } visitTypeofExpr(expr: TypeofExpr) { const typeOf = ts.createTypeOf(expr.expr.visitExpression(this, null)); - return this.record(expr, typeOf); + return this.postProcess(expr, typeOf); } // ExpressionVisitor visitReadVarExpr(expr: ReadVarExpr) { switch (expr.builtin) { case BuiltinVar.This: - return this.record(expr, ts.createIdentifier(METHOD_THIS_NAME)); + return this.postProcess(expr, ts.createIdentifier(METHOD_THIS_NAME)); case BuiltinVar.CatchError: - return this.record(expr, ts.createIdentifier(CATCH_ERROR_NAME)); + return this.postProcess(expr, ts.createIdentifier(CATCH_ERROR_NAME)); case BuiltinVar.CatchStack: - return this.record(expr, ts.createIdentifier(CATCH_STACK_NAME)); + return this.postProcess(expr, ts.createIdentifier(CATCH_STACK_NAME)); case BuiltinVar.Super: - return this.record(expr, ts.createSuper()); + return this.postProcess(expr, ts.createSuper()); } if (expr.name) { - return this.record(expr, ts.createIdentifier(expr.name)); + return this.postProcess(expr, ts.createIdentifier(expr.name)); } throw Error(`Unexpected ReadVarExpr form`); } visitWriteVarExpr(expr: WriteVarExpr): RecordedNode { - return this.record( + return this.postProcess( expr, ts.createAssignment( ts.createIdentifier(expr.name), expr.value.visitExpression(this, null))); } visitWriteKeyExpr(expr: WriteKeyExpr): RecordedNode { - return this.record( + return this.postProcess( expr, ts.createAssignment( ts.createElementAccess( @@ -540,7 +520,7 @@ export class NodeEmitterVisitor implements StatementVisitor, ExpressionVisitor { } visitWritePropExpr(expr: WritePropExpr): RecordedNode { - return this.record( + return this.postProcess( expr, ts.createAssignment( ts.createPropertyAccess(expr.receiver.visitExpression(this, null), expr.name), @@ -549,7 +529,7 @@ export class NodeEmitterVisitor implements StatementVisitor, ExpressionVisitor { visitInvokeMethodExpr(expr: InvokeMethodExpr): RecordedNode { const methodName = getMethodName(expr); - return this.record( + return this.postProcess( expr, ts.createCall( ts.createPropertyAccess(expr.receiver.visitExpression(this, null), methodName), @@ -557,7 +537,7 @@ export class NodeEmitterVisitor implements StatementVisitor, ExpressionVisitor { } visitInvokeFunctionExpr(expr: InvokeFunctionExpr): RecordedNode { - return this.record( + return this.postProcess( expr, ts.createCall( expr.fn.visitExpression(this, null), /* typeArguments */ undefined, @@ -565,7 +545,7 @@ export class NodeEmitterVisitor implements StatementVisitor, ExpressionVisitor { } visitInstantiateExpr(expr: InstantiateExpr): RecordedNode { - return this.record( + return this.postProcess( expr, ts.createNew( expr.classExpr.visitExpression(this, null), /* typeArguments */ undefined, @@ -573,7 +553,7 @@ export class NodeEmitterVisitor implements StatementVisitor, ExpressionVisitor { } visitLiteralExpr(expr: LiteralExpr) { - return this.record(expr, createLiteral(expr.value)); + return this.postProcess(expr, createLiteral(expr.value)); } visitLocalizedString(expr: LocalizedString, context: any) { @@ -581,12 +561,12 @@ export class NodeEmitterVisitor implements StatementVisitor, ExpressionVisitor { } visitExternalExpr(expr: ExternalExpr) { - return this.record(expr, this._visitIdentifier(expr.value)); + return this.postProcess(expr, this._visitIdentifier(expr.value)); } visitConditionalExpr(expr: ConditionalExpr): RecordedNode { // TODO {chuckj}: Review use of ! on falseCase. Should it be non-nullable? - return this.record( + return this.postProcess( expr, ts.createParen(ts.createConditional( expr.condition.visitExpression(this, null), expr.trueCase.visitExpression(this, null), @@ -594,7 +574,7 @@ export class NodeEmitterVisitor implements StatementVisitor, ExpressionVisitor { } visitNotExpr(expr: NotExpr): RecordedNode { - return this.record( + return this.postProcess( expr, ts.createPrefix( ts.SyntaxKind.ExclamationToken, expr.condition.visitExpression(this, null))); @@ -609,7 +589,7 @@ export class NodeEmitterVisitor implements StatementVisitor, ExpressionVisitor { } visitFunctionExpr(expr: FunctionExpr) { - return this.record( + return this.postProcess( expr, ts.createFunctionExpression( /* modifiers */ undefined, /* astriskToken */ undefined, @@ -636,7 +616,7 @@ export class NodeEmitterVisitor implements StatementVisitor, ExpressionVisitor { throw new Error(`Unknown operator: ${expr.operator}`); } const binary = ts.createPrefix(unaryOperator, expr.expr.visitExpression(this, null)); - return this.record(expr, expr.parens ? ts.createParen(binary) : binary); + return this.postProcess(expr, expr.parens ? ts.createParen(binary) : binary); } visitBinaryOperatorExpr(expr: BinaryOperatorExpr): @@ -696,28 +676,28 @@ export class NodeEmitterVisitor implements StatementVisitor, ExpressionVisitor { } const binary = ts.createBinary( expr.lhs.visitExpression(this, null), binaryOperator, expr.rhs.visitExpression(this, null)); - return this.record(expr, expr.parens ? ts.createParen(binary) : binary); + return this.postProcess(expr, expr.parens ? ts.createParen(binary) : binary); } visitReadPropExpr(expr: ReadPropExpr): RecordedNode { - return this.record( + return this.postProcess( expr, ts.createPropertyAccess(expr.receiver.visitExpression(this, null), expr.name)); } visitReadKeyExpr(expr: ReadKeyExpr): RecordedNode { - return this.record( + return this.postProcess( expr, ts.createElementAccess( expr.receiver.visitExpression(this, null), expr.index.visitExpression(this, null))); } visitLiteralArrayExpr(expr: LiteralArrayExpr): RecordedNode { - return this.record( + return this.postProcess( expr, ts.createArrayLiteral(expr.entries.map(entry => entry.visitExpression(this, null)))); } visitLiteralMapExpr(expr: LiteralMapExpr): RecordedNode { - return this.record( + return this.postProcess( expr, ts.createObjectLiteral(expr.entries.map( entry => ts.createPropertyAssignment( @@ -728,7 +708,7 @@ export class NodeEmitterVisitor implements StatementVisitor, ExpressionVisitor { } visitCommaExpr(expr: CommaExpr): RecordedNode { - return this.record( + return this.postProcess( expr, expr.parts.map(e => e.visitExpression(this, null)) .reduce( @@ -773,7 +753,6 @@ export class NodeEmitterVisitor implements StatementVisitor, ExpressionVisitor { } } - function getMethodName(methodRef: {name: string|null; builtin: BuiltinMethod | null}): string { if (methodRef.name) { return methodRef.name; diff --git a/packages/compiler-cli/src/transformers/node_emitter_transform.ts b/packages/compiler-cli/src/transformers/node_emitter_transform.ts index 0e3a499746..043b6bcbec 100644 --- a/packages/compiler-cli/src/transformers/node_emitter_transform.ts +++ b/packages/compiler-cli/src/transformers/node_emitter_transform.ts @@ -10,15 +10,15 @@ import {GeneratedFile} from '@angular/compiler'; import * as ts from 'typescript'; import {TypeScriptNodeEmitter} from './node_emitter'; -import {GENERATED_FILES} from './util'; +import {GENERATED_FILES, stripComment} from './util'; function getPreamble(original: string) { - return `/** + return `* * @fileoverview This file was generated by the Angular template compiler. Do not edit. * ${original} * @suppress {suspiciousCode,uselessCode,missingProperties,missingOverride,checkTypes,extraRequire} * tslint:disable - */`; + `; } /** @@ -41,9 +41,6 @@ export function getAngularEmitterTransformFactory( if (orig) originalComment = getFileoverviewComment(orig); const preamble = getPreamble(originalComment); if (g && g.stmts) { - const orig = program.getSourceFile(g.srcFileUrl); - let originalComment = ''; - if (orig) originalComment = getFileoverviewComment(orig); const [newSourceFile] = emitter.updateSourceFile(sourceFile, g.stmts, preamble); return newSourceFile; } else if (GENERATED_FILES.test(sourceFile.fileName)) { @@ -51,8 +48,11 @@ export function getAngularEmitterTransformFactory( // and various minutiae. // Clear out the source file entirely, only including the preamble comment, so that // ngc produces an empty .js file. - return ts.updateSourceFileNode( - sourceFile, [emitter.createCommentStatement(sourceFile, preamble)]); + const commentStmt = ts.createNotEmittedStatement(sourceFile); + ts.addSyntheticLeadingComment( + commentStmt, ts.SyntaxKind.MultiLineCommentTrivia, preamble, + /* hasTrailingNewline */ true); + return ts.updateSourceFileNode(sourceFile, [commentStmt]); } return sourceFile; }; @@ -75,5 +75,6 @@ function getFileoverviewComment(sourceFile: ts.SourceFile): string { const commentText = sourceFile.getFullText().substring(comment.pos, comment.end); // Closure Compiler ignores @suppress and similar if the comment contains @license. if (commentText.indexOf('@license') !== -1) return ''; - return commentText.replace(/^\/\*\*/, '').replace(/ ?\*\/$/, ''); + // Also remove any leading `* ` from the first line in case it was a JSDOC comment + return stripComment(commentText).replace(/^\*\s+/, ''); } diff --git a/packages/compiler-cli/src/transformers/util.ts b/packages/compiler-cli/src/transformers/util.ts index 6b139cb7b7..47fdb14145 100644 --- a/packages/compiler-cli/src/transformers/util.ts +++ b/packages/compiler-cli/src/transformers/util.ts @@ -94,3 +94,12 @@ export function ngToTsDiagnostic(ng: Diagnostic): ts.Diagnostic { length, }; } + +/** + * Strip multiline comment start and end markers from the `commentText` string. + * + * This will also strip the JSDOC comment start marker (`/**`). + */ +export function stripComment(commentText: string): string { + return commentText.replace(/^\/\*\*?/, '').replace(/\*\/$/, '').trim(); +} diff --git a/packages/compiler-cli/test/ngc_spec.ts b/packages/compiler-cli/test/ngc_spec.ts index b310c5adf7..0e35c3ee8c 100644 --- a/packages/compiler-cli/test/ngc_spec.ts +++ b/packages/compiler-cli/test/ngc_spec.ts @@ -141,7 +141,7 @@ describe('ngc transformer command-line', () => { write('mymodule.ts', ` import {NgModule} from '@angular/core'; import {AClass} from './aclass'; - + @NgModule({declarations: []}) export class MyModule { constructor(importedClass: AClass) {} @@ -382,13 +382,13 @@ describe('ngc transformer command-line', () => { }) export class MyModule {} `); - expect(contents).toContain('@fileoverview'); - expect(contents).toContain('generated by the Angular template compiler'); - expect(contents).toContain('@suppress {suspiciousCode'); + expect(contents).toContain( + '/**\n * @fileoverview This file was generated by the Angular template compiler. Do not edit.'); + expect(contents).toContain('\n * @suppress {suspiciousCode'); }); it('should be merged with existing fileoverview comments', () => { - const contents = compileAndRead(`/** Hello world. */ + const contents = compileAndRead(`/**\n * @fileoverview Hello world.\n */ import {CommonModule} from '@angular/common'; import {NgModule} from '@angular/core'; @@ -398,7 +398,7 @@ describe('ngc transformer command-line', () => { }) export class MyModule {} `); - expect(contents).toContain('Hello world.'); + expect(contents).toContain('\n * @fileoverview Hello world.\n'); }); it('should only pick file comments', () => { diff --git a/packages/compiler-cli/test/transformers/node_emitter_spec.ts b/packages/compiler-cli/test/transformers/node_emitter_spec.ts index 35638fb4f0..f4915d7704 100644 --- a/packages/compiler-cli/test/transformers/node_emitter_spec.ts +++ b/packages/compiler-cli/test/transformers/node_emitter_spec.ts @@ -267,48 +267,69 @@ describe('TypeScriptNodeEmitter', () => { }); describe('comments', () => { - it('should support a preamble', () => { - expect(emitStmt(o.variable('a').toStmt(), Format.Flat, '/* SomePreamble */')) - .toBe('/* SomePreamble */ a;'); - }); + it('should support a preamble, which is wrapped as a multi-line comment with no trimming or padding', + () => { + expect(emitStmt(o.variable('a').toStmt(), Format.Raw, '*\n * SomePreamble\n ')) + .toBe('/**\n * SomePreamble\n */\na;'); + }); it('should support singleline comments', () => { - expect(emitStmt(new o.CommentStmt('Simple comment'))).toBe('// Simple comment'); + expect(emitStmt( + new o.ReturnStatement(o.literal(1), null, [o.leadingComment(' a\n b', false)]), + Format.Raw)) + .toBe('// a\n// b\nreturn 1;'); }); it('should support multiline comments', () => { - expect(emitStmt(new o.CommentStmt('Multiline comment', true))) - .toBe('/* Multiline comment */'); - expect(emitStmt(new o.CommentStmt(`Multiline\ncomment`, true), Format.Raw)) - .toBe(`/* Multiline\ncomment */`); + expect(emitStmt( + new o.ReturnStatement( + o.literal(1), null, [o.leadingComment('Multiline comment', true)]), + Format.Raw)) + .toBe('/* Multiline comment */\nreturn 1;'); + expect(emitStmt( + new o.ReturnStatement( + o.literal(1), null, [o.leadingComment(`Multiline\ncomment`, true)]), + Format.Raw)) + .toBe(`/* Multiline\ncomment */\nreturn 1;`); }); describe('JSDoc comments', () => { it('should be supported', () => { - expect(emitStmt(new o.JSDocCommentStmt([{text: 'Intro comment'}]), Format.Raw)) - .toBe(`/**\n * Intro comment\n */`); expect(emitStmt( - new o.JSDocCommentStmt([{tagName: o.JSDocTagName.Desc, text: 'description'}]), + new o.ReturnStatement( + o.literal(1), null, [o.jsDocComment([{text: 'Intro comment'}])]), Format.Raw)) - .toBe(`/**\n * @desc description\n */`); + .toBe(`/**\n * Intro comment\n */\nreturn 1;`); expect(emitStmt( - new o.JSDocCommentStmt([ - {text: 'Intro comment'}, - {tagName: o.JSDocTagName.Desc, text: 'description'}, - {tagName: o.JSDocTagName.Id, text: '{number} identifier 123'}, - ]), + new o.ReturnStatement( + o.literal(1), null, + [o.jsDocComment([{tagName: o.JSDocTagName.Desc, text: 'description'}])]), + Format.Raw)) + .toBe(`/**\n * @desc description\n */\nreturn 1;`); + expect(emitStmt( + new o.ReturnStatement( + o.literal(1), null, [o.jsDocComment([ + {text: 'Intro comment'}, + {tagName: o.JSDocTagName.Desc, text: 'description'}, + {tagName: o.JSDocTagName.Id, text: '{number} identifier 123'}, + ])]), Format.Raw)) .toBe( - `/**\n * Intro comment\n * @desc description\n * @id {number} identifier 123\n */`); + `/**\n * Intro comment\n * @desc description\n * @id {number} identifier 123\n */\nreturn 1;`); }); it('should escape @ in the text', () => { - expect(emitStmt(new o.JSDocCommentStmt([{text: 'email@google.com'}]), Format.Raw)) - .toBe(`/**\n * email\\@google.com\n */`); + expect(emitStmt( + new o.ReturnStatement( + o.literal(1), null, [o.jsDocComment([{text: 'email@google.com'}])]), + Format.Raw)) + .toBe(`/**\n * email\\@google.com\n */\nreturn 1;`); }); it('should not allow /* and */ in the text', () => { - expect(() => emitStmt(new o.JSDocCommentStmt([{text: 'some text /* */'}]), Format.Raw)) + expect( + () => emitStmt(new o.ReturnStatement( + o.literal(1), null, [o.jsDocComment([{text: 'some text /* */'}])]))) .toThrowError(`JSDoc text cannot contain "/*" and "*/"`); }); }); diff --git a/packages/compiler/src/compiler.ts b/packages/compiler/src/compiler.ts index 06405307ad..c734932b88 100644 --- a/packages/compiler/src/compiler.ts +++ b/packages/compiler/src/compiler.ts @@ -78,7 +78,7 @@ export * from './ml_parser/tags'; export {LexerRange} from './ml_parser/lexer'; export * from './ml_parser/xml_parser'; export {NgModuleCompiler} from './ng_module_compiler'; -export {ArrayType, AssertNotNull, DYNAMIC_TYPE, BinaryOperator, BinaryOperatorExpr, BuiltinMethod, BuiltinType, BuiltinTypeName, BuiltinVar, CastExpr, ClassField, ClassMethod, ClassStmt, CommaExpr, CommentStmt, ConditionalExpr, DeclareFunctionStmt, DeclareVarStmt, Expression, ExpressionStatement, ExpressionType, ExpressionVisitor, ExternalExpr, ExternalReference, literalMap, FunctionExpr, IfStmt, InstantiateExpr, InvokeFunctionExpr, InvokeMethodExpr, JSDocCommentStmt, LiteralArrayExpr, LiteralExpr, LiteralMapExpr, MapType, NotExpr, NONE_TYPE, ReadKeyExpr, ReadPropExpr, ReadVarExpr, ReturnStatement, StatementVisitor, ThrowStmt, TryCatchStmt, Type, TypeVisitor, WrappedNodeExpr, WriteKeyExpr, WritePropExpr, WriteVarExpr, StmtModifier, Statement, STRING_TYPE, TypeofExpr, collectExternalReferences} from './output/output_ast'; +export {ArrayType, AssertNotNull, DYNAMIC_TYPE, BinaryOperator, BinaryOperatorExpr, BuiltinMethod, BuiltinType, BuiltinTypeName, BuiltinVar, CastExpr, ClassField, ClassMethod, ClassStmt, CommaExpr, ConditionalExpr, DeclareFunctionStmt, DeclareVarStmt, Expression, ExpressionStatement, ExpressionType, ExpressionVisitor, ExternalExpr, ExternalReference, literalMap, FunctionExpr, IfStmt, InstantiateExpr, InvokeFunctionExpr, InvokeMethodExpr, LiteralArrayExpr, LiteralExpr, LiteralMapExpr, MapType, NotExpr, NONE_TYPE, ReadKeyExpr, ReadPropExpr, ReadVarExpr, ReturnStatement, StatementVisitor, ThrowStmt, TryCatchStmt, Type, TypeVisitor, WrappedNodeExpr, WriteKeyExpr, WritePropExpr, WriteVarExpr, StmtModifier, Statement, STRING_TYPE, TypeofExpr, collectExternalReferences, jsDocComment, leadingComment, LeadingComment, JSDocComment, UnaryOperator, UnaryOperatorExpr, LocalizedString} from './output/output_ast'; export {EmitterVisitorContext} from './output/abstract_emitter'; export {JitEvaluator} from './output/output_jit'; export * from './output/ts_emitter'; diff --git a/packages/compiler/src/output/abstract_emitter.ts b/packages/compiler/src/output/abstract_emitter.ts index 486e075358..bbb561bacc 100644 --- a/packages/compiler/src/output/abstract_emitter.ts +++ b/packages/compiler/src/output/abstract_emitter.ts @@ -202,13 +202,34 @@ export class EmitterVisitorContext { export abstract class AbstractEmitterVisitor implements o.StatementVisitor, o.ExpressionVisitor { constructor(private _escapeDollarInStrings: boolean) {} + protected printLeadingComments(stmt: o.Statement, ctx: EmitterVisitorContext): void { + if (stmt.leadingComments === undefined) { + return; + } + for (const comment of stmt.leadingComments) { + if (comment instanceof o.JSDocComment) { + ctx.print(stmt, `/*${comment.toString()}*/`, comment.trailingNewline); + } else { + if (comment.multiline) { + ctx.print(stmt, `/* ${comment.text} */`, comment.trailingNewline); + } else { + comment.text.split('\n').forEach((line) => { + ctx.println(stmt, `// ${line}`); + }); + } + } + } + } + visitExpressionStmt(stmt: o.ExpressionStatement, ctx: EmitterVisitorContext): any { + this.printLeadingComments(stmt, ctx); stmt.expr.visitExpression(this, ctx); ctx.println(stmt, ';'); return null; } visitReturnStmt(stmt: o.ReturnStatement, ctx: EmitterVisitorContext): any { + this.printLeadingComments(stmt, ctx); ctx.print(stmt, `return `); stmt.value.visitExpression(this, ctx); ctx.println(stmt, ';'); @@ -220,6 +241,7 @@ export abstract class AbstractEmitterVisitor implements o.StatementVisitor, o.Ex abstract visitDeclareClassStmt(stmt: o.ClassStmt, ctx: EmitterVisitorContext): any; visitIfStmt(stmt: o.IfStmt, ctx: EmitterVisitorContext): any { + this.printLeadingComments(stmt, ctx); ctx.print(stmt, `if (`); stmt.condition.visitExpression(this, ctx); ctx.print(stmt, `) {`); @@ -248,25 +270,12 @@ export abstract class AbstractEmitterVisitor implements o.StatementVisitor, o.Ex abstract visitTryCatchStmt(stmt: o.TryCatchStmt, ctx: EmitterVisitorContext): any; visitThrowStmt(stmt: o.ThrowStmt, ctx: EmitterVisitorContext): any { + this.printLeadingComments(stmt, ctx); ctx.print(stmt, `throw `); stmt.error.visitExpression(this, ctx); ctx.println(stmt, `;`); return null; } - visitCommentStmt(stmt: o.CommentStmt, ctx: EmitterVisitorContext): any { - if (stmt.multiline) { - ctx.println(stmt, `/* ${stmt.comment} */`); - } else { - stmt.comment.split('\n').forEach((line) => { - ctx.println(stmt, `// ${line}`); - }); - } - return null; - } - visitJSDocCommentStmt(stmt: o.JSDocCommentStmt, ctx: EmitterVisitorContext) { - ctx.println(stmt, `/*${stmt.toString()}*/`); - return null; - } abstract visitDeclareVarStmt(stmt: o.DeclareVarStmt, ctx: EmitterVisitorContext): any; diff --git a/packages/compiler/src/output/output_ast.ts b/packages/compiler/src/output/output_ast.ts index c0e06637f0..c7332111c2 100644 --- a/packages/compiler/src/output/output_ast.ts +++ b/packages/compiler/src/output/output_ast.ts @@ -16,15 +16,11 @@ export enum TypeModifier { } export abstract class Type { - constructor(public modifiers: TypeModifier[]|null = null) { - if (!modifiers) { - this.modifiers = []; - } - } + constructor(public modifiers: TypeModifier[] = []) {} abstract visitType(visitor: TypeVisitor, context: any): any; hasModifier(modifier: TypeModifier): boolean { - return this.modifiers!.indexOf(modifier) !== -1; + return this.modifiers.indexOf(modifier) !== -1; } } @@ -40,7 +36,7 @@ export enum BuiltinTypeName { } export class BuiltinType extends Type { - constructor(public name: BuiltinTypeName, modifiers: TypeModifier[]|null = null) { + constructor(public name: BuiltinTypeName, modifiers?: TypeModifier[]) { super(modifiers); } visitType(visitor: TypeVisitor, context: any): any { @@ -50,8 +46,7 @@ export class BuiltinType extends Type { export class ExpressionType extends Type { constructor( - public value: Expression, modifiers: TypeModifier[]|null = null, - public typeParams: Type[]|null = null) { + public value: Expression, modifiers?: TypeModifier[], public typeParams: Type[]|null = null) { super(modifiers); } visitType(visitor: TypeVisitor, context: any): any { @@ -61,7 +56,7 @@ export class ExpressionType extends Type { export class ArrayType extends Type { - constructor(public of: Type, modifiers: TypeModifier[]|null = null) { + constructor(public of: Type, modifiers?: TypeModifier[]) { super(modifiers); } visitType(visitor: TypeVisitor, context: any): any { @@ -72,7 +67,7 @@ export class ArrayType extends Type { export class MapType extends Type { public valueType: Type|null; - constructor(valueType: Type|null|undefined, modifiers: TypeModifier[]|null = null) { + constructor(valueType: Type|null|undefined, modifiers?: TypeModifier[]) { super(modifiers); this.valueType = valueType || null; } @@ -357,7 +352,7 @@ export class WriteVarExpr extends Expression { return visitor.visitWriteVarExpr(this, context); } - toDeclStmt(type?: Type|null, modifiers?: StmtModifier[]|null): DeclareVarStmt { + toDeclStmt(type?: Type|null, modifiers?: StmtModifier[]): DeclareVarStmt { return new DeclareVarStmt(this.name, this.value, type, modifiers, this.sourceSpan); } @@ -764,7 +759,7 @@ export class FunctionExpr extends Expression { return visitor.visitFunctionExpr(this, context); } - toDeclStmt(name: string, modifiers: StmtModifier[]|null = null): DeclareFunctionStmt { + toDeclStmt(name: string, modifiers?: StmtModifier[]): DeclareFunctionStmt { return new DeclareFunctionStmt( name, this.params, this.statements, this.type, modifiers, this.sourceSpan); } @@ -978,13 +973,25 @@ export enum StmtModifier { Static, } -export abstract class Statement { - public modifiers: StmtModifier[]; - public sourceSpan: ParseSourceSpan|null; - constructor(modifiers?: StmtModifier[]|null, sourceSpan?: ParseSourceSpan|null) { - this.modifiers = modifiers || []; - this.sourceSpan = sourceSpan || null; +export class LeadingComment { + constructor(public text: string, public multiline: boolean, public trailingNewline: boolean) {} + toString() { + return this.multiline ? ` ${this.text} ` : this.text; } +} +export class JSDocComment extends LeadingComment { + constructor(public tags: JSDocTag[]) { + super('', /* multiline */ true, /* trailingNewline */ true); + } + toString(): string { + return serializeTags(this.tags); + } +} + +export abstract class Statement { + constructor( + public modifiers: StmtModifier[] = [], public sourceSpan: ParseSourceSpan|null = null, + public leadingComments?: LeadingComment[]) {} /** * Calculates whether this statement produces the same value as the given statement. * Note: We don't check Types nor ParseSourceSpans nor function arguments. @@ -994,7 +1001,12 @@ export abstract class Statement { abstract visitStatement(visitor: StatementVisitor, context: any): any; hasModifier(modifier: StmtModifier): boolean { - return this.modifiers!.indexOf(modifier) !== -1; + return this.modifiers.indexOf(modifier) !== -1; + } + + addLeadingComment(leadingComment: LeadingComment): void { + this.leadingComments = this.leadingComments ?? []; + this.leadingComments.push(leadingComment); } } @@ -1002,9 +1014,9 @@ export abstract class Statement { export class DeclareVarStmt extends Statement { public type: Type|null; constructor( - public name: string, public value?: Expression, type?: Type|null, - modifiers: StmtModifier[]|null = null, sourceSpan?: ParseSourceSpan|null) { - super(modifiers, sourceSpan); + public name: string, public value?: Expression, type?: Type|null, modifiers?: StmtModifier[], + sourceSpan?: ParseSourceSpan|null, leadingComments?: LeadingComment[]) { + super(modifiers, sourceSpan, leadingComments); this.type = type || (value && value.type) || null; } isEquivalent(stmt: Statement): boolean { @@ -1020,28 +1032,29 @@ export class DeclareFunctionStmt extends Statement { public type: Type|null; constructor( public name: string, public params: FnParam[], public statements: Statement[], - type?: Type|null, modifiers: StmtModifier[]|null = null, sourceSpan?: ParseSourceSpan|null) { - super(modifiers, sourceSpan); + type?: Type|null, modifiers?: StmtModifier[], sourceSpan?: ParseSourceSpan|null, + leadingComments?: LeadingComment[]) { + super(modifiers, sourceSpan, leadingComments); this.type = type || null; } isEquivalent(stmt: Statement): boolean { return stmt instanceof DeclareFunctionStmt && areAllEquivalent(this.params, stmt.params) && areAllEquivalent(this.statements, stmt.statements); } - visitStatement(visitor: StatementVisitor, context: any): any { return visitor.visitDeclareFunctionStmt(this, context); } } export class ExpressionStatement extends Statement { - constructor(public expr: Expression, sourceSpan?: ParseSourceSpan|null) { - super(null, sourceSpan); + constructor( + public expr: Expression, sourceSpan?: ParseSourceSpan|null, + leadingComments?: LeadingComment[]) { + super([], sourceSpan, leadingComments); } isEquivalent(stmt: Statement): boolean { return stmt instanceof ExpressionStatement && this.expr.isEquivalent(stmt.expr); } - visitStatement(visitor: StatementVisitor, context: any): any { return visitor.visitExpressionStmt(this, context); } @@ -1049,8 +1062,10 @@ export class ExpressionStatement extends Statement { export class ReturnStatement extends Statement { - constructor(public value: Expression, sourceSpan?: ParseSourceSpan|null) { - super(null, sourceSpan); + constructor( + public value: Expression, sourceSpan: ParseSourceSpan|null = null, + leadingComments?: LeadingComment[]) { + super([], sourceSpan, leadingComments); } isEquivalent(stmt: Statement): boolean { return stmt instanceof ReturnStatement && this.value.isEquivalent(stmt.value); @@ -1061,21 +1076,15 @@ export class ReturnStatement extends Statement { } export class AbstractClassPart { - public type: Type|null; - constructor(type: Type|null|undefined, public modifiers: StmtModifier[]|null) { - if (!modifiers) { - this.modifiers = []; - } - this.type = type || null; - } + constructor(public type: Type|null = null, public modifiers: StmtModifier[] = []) {} hasModifier(modifier: StmtModifier): boolean { - return this.modifiers!.indexOf(modifier) !== -1; + return this.modifiers.indexOf(modifier) !== -1; } } export class ClassField extends AbstractClassPart { constructor( - public name: string, type?: Type|null, modifiers: StmtModifier[]|null = null, + public name: string, type?: Type|null, modifiers?: StmtModifier[], public initializer?: Expression) { super(type, modifiers); } @@ -1088,7 +1097,7 @@ export class ClassField extends AbstractClassPart { export class ClassMethod extends AbstractClassPart { constructor( public name: string|null, public params: FnParam[], public body: Statement[], - type?: Type|null, modifiers: StmtModifier[]|null = null) { + type?: Type|null, modifiers?: StmtModifier[]) { super(type, modifiers); } isEquivalent(m: ClassMethod) { @@ -1099,8 +1108,7 @@ export class ClassMethod extends AbstractClassPart { export class ClassGetter extends AbstractClassPart { constructor( - public name: string, public body: Statement[], type?: Type|null, - modifiers: StmtModifier[]|null = null) { + public name: string, public body: Statement[], type?: Type|null, modifiers?: StmtModifier[]) { super(type, modifiers); } isEquivalent(m: ClassGetter) { @@ -1113,9 +1121,9 @@ export class ClassStmt extends Statement { constructor( public name: string, public parent: Expression|null, public fields: ClassField[], public getters: ClassGetter[], public constructorMethod: ClassMethod, - public methods: ClassMethod[], modifiers: StmtModifier[]|null = null, - sourceSpan?: ParseSourceSpan|null) { - super(modifiers, sourceSpan); + public methods: ClassMethod[], modifiers?: StmtModifier[], sourceSpan?: ParseSourceSpan|null, + leadingComments?: LeadingComment[]) { + super(modifiers, sourceSpan, leadingComments); } isEquivalent(stmt: Statement): boolean { return stmt instanceof ClassStmt && this.name === stmt.name && @@ -1134,8 +1142,9 @@ export class ClassStmt extends Statement { export class IfStmt extends Statement { constructor( public condition: Expression, public trueCase: Statement[], - public falseCase: Statement[] = [], sourceSpan?: ParseSourceSpan|null) { - super(null, sourceSpan); + public falseCase: Statement[] = [], sourceSpan?: ParseSourceSpan|null, + leadingComments?: LeadingComment[]) { + super([], sourceSpan, leadingComments); } isEquivalent(stmt: Statement): boolean { return stmt instanceof IfStmt && this.condition.isEquivalent(stmt.condition) && @@ -1147,38 +1156,11 @@ export class IfStmt extends Statement { } } -export class CommentStmt extends Statement { - constructor(public comment: string, public multiline = false, sourceSpan?: ParseSourceSpan|null) { - super(null, sourceSpan); - } - isEquivalent(stmt: Statement): boolean { - return stmt instanceof CommentStmt; - } - visitStatement(visitor: StatementVisitor, context: any): any { - return visitor.visitCommentStmt(this, context); - } -} - -export class JSDocCommentStmt extends Statement { - constructor(public tags: JSDocTag[] = [], sourceSpan?: ParseSourceSpan|null) { - super(null, sourceSpan); - } - isEquivalent(stmt: Statement): boolean { - return stmt instanceof JSDocCommentStmt && this.toString() === stmt.toString(); - } - visitStatement(visitor: StatementVisitor, context: any): any { - return visitor.visitJSDocCommentStmt(this, context); - } - toString(): string { - return serializeTags(this.tags); - } -} - export class TryCatchStmt extends Statement { constructor( public bodyStmts: Statement[], public catchStmts: Statement[], - sourceSpan?: ParseSourceSpan|null) { - super(null, sourceSpan); + sourceSpan: ParseSourceSpan|null = null, leadingComments?: LeadingComment[]) { + super([], sourceSpan, leadingComments); } isEquivalent(stmt: Statement): boolean { return stmt instanceof TryCatchStmt && areAllEquivalent(this.bodyStmts, stmt.bodyStmts) && @@ -1191,8 +1173,10 @@ export class TryCatchStmt extends Statement { export class ThrowStmt extends Statement { - constructor(public error: Expression, sourceSpan?: ParseSourceSpan|null) { - super(null, sourceSpan); + constructor( + public error: Expression, sourceSpan: ParseSourceSpan|null = null, + leadingComments?: LeadingComment[]) { + super([], sourceSpan, leadingComments); } isEquivalent(stmt: ThrowStmt): boolean { return stmt instanceof TryCatchStmt && this.error.isEquivalent(stmt.error); @@ -1211,8 +1195,6 @@ export interface StatementVisitor { visitIfStmt(stmt: IfStmt, context: any): any; visitTryCatchStmt(stmt: TryCatchStmt, context: any): any; visitThrowStmt(stmt: ThrowStmt, context: any): any; - visitCommentStmt(stmt: CommentStmt, context: any): any; - visitJSDocCommentStmt(stmt: JSDocCommentStmt, context: any): any; } export class AstTransformer implements StatementVisitor, ExpressionVisitor { @@ -1374,7 +1356,7 @@ export class AstTransformer implements StatementVisitor, ExpressionVisitor { const entries = ast.entries.map( (entry): LiteralMapEntry => new LiteralMapEntry( entry.key, entry.value.visitExpression(this, context), entry.quoted)); - const mapType = new MapType(ast.valueType, null); + const mapType = new MapType(ast.valueType); return this.transformExpr(new LiteralMapExpr(entries, mapType, ast.sourceSpan), context); } visitCommaExpr(ast: CommaExpr, context: any): any { @@ -1388,25 +1370,30 @@ export class AstTransformer implements StatementVisitor, ExpressionVisitor { visitDeclareVarStmt(stmt: DeclareVarStmt, context: any): any { const value = stmt.value && stmt.value.visitExpression(this, context); return this.transformStmt( - new DeclareVarStmt(stmt.name, value, stmt.type, stmt.modifiers, stmt.sourceSpan), context); + new DeclareVarStmt( + stmt.name, value, stmt.type, stmt.modifiers, stmt.sourceSpan, stmt.leadingComments), + context); } visitDeclareFunctionStmt(stmt: DeclareFunctionStmt, context: any): any { return this.transformStmt( new DeclareFunctionStmt( stmt.name, stmt.params, this.visitAllStatements(stmt.statements, context), stmt.type, - stmt.modifiers, stmt.sourceSpan), + stmt.modifiers, stmt.sourceSpan, stmt.leadingComments), context); } visitExpressionStmt(stmt: ExpressionStatement, context: any): any { return this.transformStmt( - new ExpressionStatement(stmt.expr.visitExpression(this, context), stmt.sourceSpan), + new ExpressionStatement( + stmt.expr.visitExpression(this, context), stmt.sourceSpan, stmt.leadingComments), context); } visitReturnStmt(stmt: ReturnStatement, context: any): any { return this.transformStmt( - new ReturnStatement(stmt.value.visitExpression(this, context), stmt.sourceSpan), context); + new ReturnStatement( + stmt.value.visitExpression(this, context), stmt.sourceSpan, stmt.leadingComments), + context); } visitDeclareClassStmt(stmt: ClassStmt, context: any): any { @@ -1435,7 +1422,8 @@ export class AstTransformer implements StatementVisitor, ExpressionVisitor { new IfStmt( stmt.condition.visitExpression(this, context), this.visitAllStatements(stmt.trueCase, context), - this.visitAllStatements(stmt.falseCase, context), stmt.sourceSpan), + this.visitAllStatements(stmt.falseCase, context), stmt.sourceSpan, + stmt.leadingComments), context); } @@ -1443,21 +1431,16 @@ export class AstTransformer implements StatementVisitor, ExpressionVisitor { return this.transformStmt( new TryCatchStmt( this.visitAllStatements(stmt.bodyStmts, context), - this.visitAllStatements(stmt.catchStmts, context), stmt.sourceSpan), + this.visitAllStatements(stmt.catchStmts, context), stmt.sourceSpan, + stmt.leadingComments), context); } visitThrowStmt(stmt: ThrowStmt, context: any): any { return this.transformStmt( - new ThrowStmt(stmt.error.visitExpression(this, context), stmt.sourceSpan), context); - } - - visitCommentStmt(stmt: CommentStmt, context: any): any { - return this.transformStmt(stmt, context); - } - - visitJSDocCommentStmt(stmt: JSDocCommentStmt, context: any): any { - return this.transformStmt(stmt, context); + new ThrowStmt( + stmt.error.visitExpression(this, context), stmt.sourceSpan, stmt.leadingComments), + context); } visitAllStatements(stmts: Statement[], context: any): Statement[] { @@ -1647,12 +1630,6 @@ export class RecursiveAstVisitor implements StatementVisitor, ExpressionVisitor stmt.error.visitExpression(this, context); return stmt; } - visitCommentStmt(stmt: CommentStmt, context: any): any { - return stmt; - } - visitJSDocCommentStmt(stmt: JSDocCommentStmt, context: any): any { - return stmt; - } visitAllStatements(stmts: Statement[], context: any): void { stmts.forEach(stmt => stmt.visitStatement(this, context)); } @@ -1743,6 +1720,15 @@ class _ApplySourceSpanTransformer extends AstTransformer { } } +export function leadingComment( + text: string, multiline: boolean = false, trailingNewline: boolean = true): LeadingComment { + return new LeadingComment(text, multiline, trailingNewline); +} + +export function jsDocComment(tags: JSDocTag[] = []): JSDocComment { + return new JSDocComment(tags); +} + export function variable( name: string, type?: Type|null, sourceSpan?: ParseSourceSpan|null): ReadVarExpr { return new ReadVarExpr(name, type, sourceSpan); @@ -1755,14 +1741,13 @@ export function importExpr( } export function importType( - id: ExternalReference, typeParams: Type[]|null = null, - typeModifiers: TypeModifier[]|null = null): ExpressionType|null { + id: ExternalReference, typeParams?: Type[]|null, + typeModifiers?: TypeModifier[]): ExpressionType|null { return id != null ? expressionType(importExpr(id, typeParams, null), typeModifiers) : null; } export function expressionType( - expr: Expression, typeModifiers: TypeModifier[]|null = null, - typeParams: Type[]|null = null): ExpressionType { + expr: Expression, typeModifiers?: TypeModifier[], typeParams?: Type[]|null): ExpressionType { return new ExpressionType(expr, typeModifiers, typeParams); } @@ -1802,8 +1787,10 @@ export function fn( return new FunctionExpr(params, body, type, sourceSpan, name); } -export function ifStmt(condition: Expression, thenClause: Statement[], elseClause?: Statement[]) { - return new IfStmt(condition, thenClause, elseClause); +export function ifStmt( + condition: Expression, thenClause: Statement[], elseClause?: Statement[], + sourceSpan?: ParseSourceSpan, leadingComments?: LeadingComment[]) { + return new IfStmt(condition, thenClause, elseClause, sourceSpan, leadingComments); } export function literal( @@ -1865,10 +1852,15 @@ function tagToString(tag: JSDocTag): string { function serializeTags(tags: JSDocTag[]): string { if (tags.length === 0) return ''; + if (tags.length === 1 && tags[0].tagName && !tags[0].text) { + // The JSDOC comment is a single simple tag: e.g `/** @tagname */`. + return `*${tagToString(tags[0])} `; + } + let out = '*\n'; for (const tag of tags) { out += ' *'; - // If the tagToString is multi-line, insert " * " prefixes on subsequent lines. + // If the tagToString is multi-line, insert " * " prefixes on lines. out += tagToString(tag).replace(/\n/g, '\n * '); out += '\n'; } diff --git a/packages/compiler/src/output/output_interpreter.ts b/packages/compiler/src/output/output_interpreter.ts index b45444469d..72ae1871ca 100644 --- a/packages/compiler/src/output/output_interpreter.ts +++ b/packages/compiler/src/output/output_interpreter.ts @@ -233,12 +233,6 @@ class StatementInterpreter implements o.StatementVisitor, o.ExpressionVisitor { visitThrowStmt(stmt: o.ThrowStmt, ctx: _ExecutionContext): any { throw stmt.error.visitExpression(this, ctx); } - visitCommentStmt(stmt: o.CommentStmt, context?: any): any { - return null; - } - visitJSDocCommentStmt(stmt: o.JSDocCommentStmt, context?: any): any { - return null; - } visitInstantiateExpr(ast: o.InstantiateExpr, ctx: _ExecutionContext): any { const args = this.visitAllExpressions(ast.args, ctx); const clazz = ast.classExpr.visitExpression(this, ctx); diff --git a/packages/compiler/src/render3/util.ts b/packages/compiler/src/render3/util.ts index 50a6141438..5c1d7bb40a 100644 --- a/packages/compiler/src/render3/util.ts +++ b/packages/compiler/src/render3/util.ts @@ -45,14 +45,14 @@ export function convertMetaToOutput(meta: any, ctx: OutputContext): o.Expression } export function typeWithParameters(type: o.Expression, numParams: number): o.ExpressionType { - let params: o.Type[]|null = null; - if (numParams > 0) { - params = []; - for (let i = 0; i < numParams; i++) { - params.push(o.DYNAMIC_TYPE); - } + if (numParams === 0) { + return o.expressionType(type); } - return o.expressionType(type, null, params); + const params: o.Type[] = []; + for (let i = 0; i < numParams; i++) { + params.push(o.DYNAMIC_TYPE); + } + return o.expressionType(type, undefined, params); } export interface R3Reference { diff --git a/packages/compiler/src/render3/view/i18n/get_msg_utils.ts b/packages/compiler/src/render3/view/i18n/get_msg_utils.ts index c3987f028b..1ead29cb4e 100644 --- a/packages/compiler/src/render3/view/i18n/get_msg_utils.ts +++ b/packages/compiler/src/render3/view/i18n/get_msg_utils.ts @@ -10,7 +10,7 @@ import {mapLiteral} from '../../../output/map_util'; import * as o from '../../../output/output_ast'; import {serializeIcuNode} from './icu_serializer'; -import {i18nMetaToDocStmt} from './meta'; +import {i18nMetaToJSDoc} from './meta'; import {formatI18nPlaceholderName} from './util'; /** Closure uses `goog.getMsg(message)` to lookup translations */ @@ -31,15 +31,13 @@ export function createGoogleGetMsgStatements( // */ // const MSG_... = goog.getMsg(..); // I18N_X = MSG_...; - const statements = []; - const jsdocComment = i18nMetaToDocStmt(message); - if (jsdocComment !== null) { - statements.push(jsdocComment); + const googGetMsgStmt = closureVar.set(o.variable(GOOG_GET_MSG).callFn(args)).toConstDecl(); + const metaComment = i18nMetaToJSDoc(message); + if (metaComment !== null) { + googGetMsgStmt.addLeadingComment(metaComment); } - statements.push(closureVar.set(o.variable(GOOG_GET_MSG).callFn(args)).toConstDecl()); - statements.push(new o.ExpressionStatement(variable.set(closureVar))); - - return statements; + const i18nAssignmentStmt = new o.ExpressionStatement(variable.set(closureVar)); + return [googGetMsgStmt, i18nAssignmentStmt]; } /** diff --git a/packages/compiler/src/render3/view/i18n/meta.ts b/packages/compiler/src/render3/view/i18n/meta.ts index baa7e24e01..31e9e2bc1b 100644 --- a/packages/compiler/src/render3/view/i18n/meta.ts +++ b/packages/compiler/src/render3/view/i18n/meta.ts @@ -231,7 +231,7 @@ export function parseI18nMeta(meta: string = ''): I18nMeta { // Converts i18n meta information for a message (id, description, meaning) // to a JsDoc statement formatted as expected by the Closure compiler. -export function i18nMetaToDocStmt(meta: I18nMeta): o.JSDocCommentStmt|null { +export function i18nMetaToJSDoc(meta: I18nMeta): o.JSDocComment|null { const tags: o.JSDocTag[] = []; if (meta.description) { tags.push({tagName: o.JSDocTagName.Desc, text: meta.description}); @@ -239,5 +239,5 @@ export function i18nMetaToDocStmt(meta: I18nMeta): o.JSDocCommentStmt|null { if (meta.meaning) { tags.push({tagName: o.JSDocTagName.Meaning, text: meta.meaning}); } - return tags.length == 0 ? null : new o.JSDocCommentStmt(tags); + return tags.length == 0 ? null : o.jsDocComment(tags); } diff --git a/packages/compiler/src/render3/view/i18n/util.ts b/packages/compiler/src/render3/view/i18n/util.ts index da71db4b67..3c0360cffe 100644 --- a/packages/compiler/src/render3/view/i18n/util.ts +++ b/packages/compiler/src/render3/view/i18n/util.ts @@ -179,5 +179,5 @@ export function getTranslationConstPrefix(extra: string): string { */ export function declareI18nVariable(variable: o.ReadVarExpr): o.Statement { return new o.DeclareVarStmt( - variable.name!, undefined, o.INFERRED_TYPE, null, variable.sourceSpan); + variable.name!, undefined, o.INFERRED_TYPE, undefined, variable.sourceSpan); } diff --git a/packages/compiler/src/render3/view/template.ts b/packages/compiler/src/render3/view/template.ts index 3dea5078b0..eb92a1590c 100644 --- a/packages/compiler/src/render3/view/template.ts +++ b/packages/compiler/src/render3/view/template.ts @@ -893,7 +893,7 @@ export class TemplateDefinitionBuilder implements t.Visitor, LocalResolver const templateFunctionExpr = templateVisitor.buildTemplateFunction( template.children, template.variables, this._ngContentReservedSlots.length + this._ngContentSelectorsOffset, template.i18n); - this.constantPool.statements.push(templateFunctionExpr.toDeclStmt(templateName, null)); + this.constantPool.statements.push(templateFunctionExpr.toDeclStmt(templateName)); if (templateVisitor._ngContentReservedSlots.length) { this._ngContentReservedSlots.push(...templateVisitor._ngContentReservedSlots); } diff --git a/packages/compiler/test/output/js_emitter_spec.ts b/packages/compiler/test/output/js_emitter_spec.ts index b6ff7deab5..e55c8e1339 100644 --- a/packages/compiler/test/output/js_emitter_spec.ts +++ b/packages/compiler/test/output/js_emitter_spec.ts @@ -189,8 +189,49 @@ const externalModuleIdentifier = new o.ExternalReference(anotherModuleUrl, 'some ].join('\n')); }); - it('should support comments', () => { - expect(emitStmt(new o.CommentStmt('a\nb'))).toEqual(['// a', '// b'].join('\n')); + describe('comments', () => { + it('should support a preamble', () => { + expect(emitStmt(o.variable('a').toStmt(), '/* SomePreamble */')).toBe([ + '/* SomePreamble */', 'a;' + ].join('\n')); + }); + + it('should support singleline comments', () => { + expect(emitStmt(new o.ReturnStatement(o.literal(1), null, [o.leadingComment('a\nb')]))) + .toBe('// a\n// b\nreturn 1;'); + }); + + it('should support multiline comments', () => { + expect(emitStmt(new o.ReturnStatement(o.literal(1), null, [ + o.leadingComment('Multiline comment', true) + ]))).toBe('/* Multiline comment */\nreturn 1;'); + expect(emitStmt(new o.ReturnStatement(o.literal(1), null, [ + o.leadingComment(`Multiline\ncomment`, true) + ]))).toBe(`/* Multiline\ncomment */\nreturn 1;`); + }); + + it('should support inline multiline comments', () => { + expect(emitStmt(new o.ReturnStatement(o.literal(1), null, [ + o.leadingComment('inline comment', true, false) + ]))).toBe('/* inline comment */return 1;'); + }); + + it('should support JSDoc comments', () => { + expect(emitStmt(new o.ReturnStatement(o.literal(1), null, [ + o.jsDocComment([{text: 'Intro comment'}]) + ]))).toBe(`/**\n * Intro comment\n */\nreturn 1;`); + expect(emitStmt(new o.ReturnStatement(o.literal(1), null, [ + o.jsDocComment([{tagName: o.JSDocTagName.Desc, text: 'description'}]) + ]))).toBe(`/**\n * @desc description\n */\nreturn 1;`); + expect(emitStmt(new o.ReturnStatement( + o.literal(1), null, [o.jsDocComment([ + {text: 'Intro comment'}, + {tagName: o.JSDocTagName.Desc, text: 'description'}, + {tagName: o.JSDocTagName.Id, text: '{number} identifier 123'}, + ])]))) + .toBe( + `/**\n * Intro comment\n * @desc description\n * @id {number} identifier 123\n */\nreturn 1;`); + }); }); it('should support if stmt', () => { diff --git a/packages/compiler/test/output/output_ast_spec.ts b/packages/compiler/test/output/output_ast_spec.ts index c330ad8aae..d0130d1719 100644 --- a/packages/compiler/test/output/output_ast_spec.ts +++ b/packages/compiler/test/output/output_ast_spec.ts @@ -21,22 +21,5 @@ import * as o from '../../src/output/output_ast'; expect(o.collectExternalReferences([stmt])).toEqual([ref1, ref2]); }); }); - - describe('comments', () => { - it('different JSDocCommentStmt should not be equivalent', () => { - const comment1 = new o.JSDocCommentStmt([{text: 'text'}]); - const comment2 = new o.JSDocCommentStmt([{text: 'text2'}]); - const comment3 = new o.JSDocCommentStmt([{tagName: o.JSDocTagName.Desc, text: 'text2'}]); - const comment4 = new o.JSDocCommentStmt([{text: 'text2'}, {text: 'text3'}]); - - expect(comment1.isEquivalent(comment2)).toBeFalsy(); - expect(comment1.isEquivalent(comment3)).toBeFalsy(); - expect(comment1.isEquivalent(comment4)).toBeFalsy(); - expect(comment2.isEquivalent(comment3)).toBeFalsy(); - expect(comment2.isEquivalent(comment4)).toBeFalsy(); - expect(comment3.isEquivalent(comment4)).toBeFalsy(); - expect(comment1.isEquivalent(comment1)).toBeTruthy(); - }); - }); }); } diff --git a/packages/compiler/test/output/ts_emitter_spec.ts b/packages/compiler/test/output/ts_emitter_spec.ts index e20b136f38..33f659dbec 100644 --- a/packages/compiler/test/output/ts_emitter_spec.ts +++ b/packages/compiler/test/output/ts_emitter_spec.ts @@ -239,7 +239,8 @@ const externalModuleIdentifier = new o.ExternalReference(anotherModuleUrl, 'some }); it('should support comments', () => { - expect(emitStmt(new o.CommentStmt('a\nb'))).toEqual(['// a', '// b'].join('\n')); + expect(emitStmt(new o.ReturnStatement(o.literal(1), null, [o.leadingComment('a\nb')]))) + .toEqual('// a\n// b\nreturn 1;'); }); it('should support if stmt', () => { @@ -441,29 +442,40 @@ const externalModuleIdentifier = new o.ExternalReference(anotherModuleUrl, 'some }); it('should support singleline comments', () => { - expect(emitStmt(new o.CommentStmt('Simple comment'))).toBe('// Simple comment'); + expect(emitStmt(new o.ReturnStatement(o.literal(1), null, [o.leadingComment('a\nb')]))) + .toBe('// a\n// b\nreturn 1;'); }); it('should support multiline comments', () => { - expect(emitStmt(new o.CommentStmt('Multiline comment', true))) - .toBe('/* Multiline comment */'); - expect(emitStmt(new o.CommentStmt(`Multiline\ncomment`, true))) - .toBe(`/* Multiline\ncomment */`); + expect(emitStmt(new o.ReturnStatement(o.literal(1), null, [ + o.leadingComment('Multiline comment', true) + ]))).toBe('/* Multiline comment */\nreturn 1;'); + expect(emitStmt(new o.ReturnStatement(o.literal(1), null, [ + o.leadingComment(`Multiline\ncomment`, true) + ]))).toBe(`/* Multiline\ncomment */\nreturn 1;`); + }); + + it('should support inline multiline comments', () => { + expect(emitStmt(new o.ReturnStatement(o.literal(1), null, [ + o.leadingComment('inline comment', true, false) + ]))).toBe('/* inline comment */return 1;'); }); it('should support JSDoc comments', () => { - expect(emitStmt(new o.JSDocCommentStmt([{text: 'Intro comment'}]))) - .toBe(`/**\n * Intro comment\n */`); - expect(emitStmt(new o.JSDocCommentStmt([ - {tagName: o.JSDocTagName.Desc, text: 'description'} - ]))).toBe(`/**\n * @desc description\n */`); - expect(emitStmt(new o.JSDocCommentStmt([ - {text: 'Intro comment'}, - {tagName: o.JSDocTagName.Desc, text: 'description'}, - {tagName: o.JSDocTagName.Id, text: '{number} identifier 123'}, - ]))) + expect(emitStmt(new o.ReturnStatement(o.literal(1), null, [ + o.jsDocComment([{text: 'Intro comment'}]) + ]))).toBe(`/**\n * Intro comment\n */\nreturn 1;`); + expect(emitStmt(new o.ReturnStatement(o.literal(1), null, [ + o.jsDocComment([{tagName: o.JSDocTagName.Desc, text: 'description'}]) + ]))).toBe(`/**\n * @desc description\n */\nreturn 1;`); + expect(emitStmt(new o.ReturnStatement( + o.literal(1), null, [o.jsDocComment([ + {text: 'Intro comment'}, + {tagName: o.JSDocTagName.Desc, text: 'description'}, + {tagName: o.JSDocTagName.Id, text: '{number} identifier 123'}, + ])]))) .toBe( - `/**\n * Intro comment\n * @desc description\n * @id {number} identifier 123\n */`); + `/**\n * Intro comment\n * @desc description\n * @id {number} identifier 123\n */\nreturn 1;`); }); });