From 3b167be069b8d4a3458b94b4696d70865a4cf75c Mon Sep 17 00:00:00 2001 From: Olivier Combe Date: Mon, 12 Mar 2018 15:34:03 +0100 Subject: [PATCH] feat(compiler): support for singleline, multiline & jsdoc comments (#22715) PR Close #22715 --- .../src/transformers/node_emitter.ts | 28 +++++-- .../test/transformers/node_emitter_spec.ts | 76 +++++++++++++++---- packages/compiler/src/compiler.ts | 2 +- .../compiler/src/output/abstract_emitter.ts | 12 ++- packages/compiler/src/output/output_ast.ts | 72 +++++++++++++++++- .../compiler/src/output/output_interpreter.ts | 1 + .../compiler/test/output/output_ast_spec.ts | 17 +++++ .../compiler/test/output/ts_emitter_spec.ts | 38 ++++++++-- 8 files changed, 215 insertions(+), 31 deletions(-) diff --git a/packages/compiler-cli/src/transformers/node_emitter.ts b/packages/compiler-cli/src/transformers/node_emitter.ts index eafdb60ddf..62959fad61 100644 --- a/packages/compiler-cli/src/transformers/node_emitter.ts +++ b/packages/compiler-cli/src/transformers/node_emitter.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {AssertNotNull, BinaryOperator, BinaryOperatorExpr, BuiltinMethod, BuiltinVar, CastExpr, ClassMethod, ClassStmt, CommaExpr, CommentStmt, CompileIdentifierMetadata, ConditionalExpr, DeclareFunctionStmt, DeclareVarStmt, ExpressionStatement, ExpressionVisitor, ExternalExpr, ExternalReference, FunctionExpr, IfStmt, InstantiateExpr, InvokeFunctionExpr, InvokeMethodExpr, LiteralArrayExpr, LiteralExpr, LiteralMapExpr, NotExpr, ParseSourceFile, ParseSourceSpan, PartialModule, ReadKeyExpr, ReadPropExpr, ReadVarExpr, ReturnStatement, Statement, StatementVisitor, StaticSymbol, StmtModifier, ThrowStmt, TryCatchStmt, WriteKeyExpr, WritePropExpr, WriteVarExpr} from '@angular/compiler'; +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, WriteKeyExpr, WritePropExpr, WriteVarExpr} from '@angular/compiler'; import * as ts from 'typescript'; import {error} from './util'; @@ -72,8 +72,8 @@ export function updateSourceFile( classes.map<[string, ClassStmt]>(classStatement => [classStatement.name, classStatement])); const classNames = new Set(classes.map(classStatement => classStatement.name)); - const prefix = - prefixStatements.map(statement => statement.visitStatement(converter, null)); + const prefix: ts.Statement[] = + prefixStatements.map(statement => statement.visitStatement(converter, sourceFile)); // Add static methods to all the classes referenced in module. let newStatements = sourceFile.statements.map(node => { @@ -342,7 +342,7 @@ class _NodeEmitterVisitor implements StatementVisitor, ExpressionVisitor { return this.record(stmt, ts.createVariableStatement(this.getModifiers(stmt), varDeclList)); } - visitDeclareFunctionStmt(stmt: DeclareFunctionStmt, context: any) { + visitDeclareFunctionStmt(stmt: DeclareFunctionStmt) { return this.record( stmt, ts.createFunctionDeclaration( /* decorators */ undefined, this.getModifiers(stmt), @@ -443,7 +443,23 @@ class _NodeEmitterVisitor implements StatementVisitor, ExpressionVisitor { return this.record(stmt, ts.createThrow(stmt.error.visitExpression(this, null))); } - visitCommentStmt(stmt: CommentStmt) { return 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; + } // ExpressionVisitor visitReadVarExpr(expr: ReadVarExpr) { @@ -712,4 +728,4 @@ function modifierFromModifier(modifier: StmtModifier): ts.Modifier { function translateModifiers(modifiers: StmtModifier[] | null): ts.Modifier[]|undefined { return modifiers == null ? undefined : modifiers !.map(modifierFromModifier); -} \ No newline at end of file +} diff --git a/packages/compiler-cli/test/transformers/node_emitter_spec.ts b/packages/compiler-cli/test/transformers/node_emitter_spec.ts index b728c793f0..6eddfd22d1 100644 --- a/packages/compiler-cli/test/transformers/node_emitter_spec.ts +++ b/packages/compiler-cli/test/transformers/node_emitter_spec.ts @@ -36,7 +36,8 @@ describe('TypeScriptNodeEmitter', () => { someVar = o.variable('someVar', null, null); }); - function emitStmt(stmt: o.Statement | o.Statement[], preamble?: string): string { + function emitStmt( + stmt: o.Statement | o.Statement[], format: Format = Format.Flat, preamble?: string): string { const stmts = Array.isArray(stmt) ? stmt : [stmt]; const program = ts.createProgram( @@ -57,7 +58,7 @@ describe('TypeScriptNodeEmitter', () => { result = data; } }, undefined, undefined, transformers); - return normalizeResult(result); + return normalizeResult(result, format); } it('should declare variables', () => { @@ -248,7 +249,52 @@ describe('TypeScriptNodeEmitter', () => { ]))).toEqual(`function someFn(param1) { }`); }); - it('should support comments', () => { expect(emitStmt(new o.CommentStmt('a\nb'))).toEqual(''); }); + describe('comments', () => { + it('should support a preamble', () => { + expect(emitStmt(o.variable('a').toStmt(), Format.Flat, '/* SomePreamble */')) + .toBe('/* SomePreamble */ a;'); + }); + + it('should support singleline comments', + () => { expect(emitStmt(new o.CommentStmt('Simple comment'))).toBe('// Simple comment'); }); + + 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 */`); + }); + + 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'}]), + Format.Raw)) + .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'}, + ]), + Format.Raw)) + .toBe( + `/**\n * Intro comment\n * @desc description\n * @id {number} identifier 123\n */`); + }); + + it('should escape @ in the text', () => { + expect(emitStmt(new o.JSDocCommentStmt([{text: 'email@google.com'}]), Format.Raw)) + .toBe(`/**\n * email\\@google.com\n */`); + }); + + it('should not allow /* and */ in the text', () => { + expect(() => emitStmt(new o.JSDocCommentStmt([{text: 'some text /* */'}]), Format.Raw)) + .toThrowError(`JSDoc text cannot contain "/*" and "*/"`); + }); + }); + }); it('should support if stmt', () => { const trueCase = o.variable('trueCase').callFn([]).toStmt(); @@ -391,10 +437,6 @@ describe('TypeScriptNodeEmitter', () => { expect(emitStmt(writeVarExpr.toDeclStmt(new o.MapType(o.INT_TYPE)))).toEqual('var a = null;'); }); - it('should support a preamble', () => { - expect(emitStmt(o.variable('a').toStmt(), '/* SomePreamble */')).toBe('/* SomePreamble */ a;'); - }); - describe('source maps', () => { function emitStmt(stmt: o.Statement | o.Statement[], preamble?: string): string { const stmts = Array.isArray(stmt) ? stmt : [stmt]; @@ -515,16 +557,20 @@ const FILES: Directory = { somePackage: {'someGenFile.ts': `export var a: number;`} }; -function normalizeResult(result: string): string { +const enum Format { Raw, Flat } + +function normalizeResult(result: string, format: Format): string { // Remove TypeScript prefixes + let res = result.replace('"use strict";', ' ') + .replace('exports.__esModule = true;', ' ') + .replace('Object.defineProperty(exports, "__esModule", { value: true });', ' '); + // Remove new lines // Squish adjacent spaces + if (format === Format.Flat) { + return res.replace(/\n/g, ' ').replace(/ +/g, ' ').replace(/^ /g, '').replace(/ $/g, ''); + } + // Remove prefix and postfix spaces - return result.replace('"use strict";', ' ') - .replace('exports.__esModule = true;', ' ') - .replace('Object.defineProperty(exports, "__esModule", { value: true });', ' ') - .replace(/\n/g, ' ') - .replace(/ +/g, ' ') - .replace(/^ /g, '') - .replace(/ $/g, ''); + return res.trim(); } diff --git a/packages/compiler/src/compiler.ts b/packages/compiler/src/compiler.ts index 5c6b5a4482..e0bcadf329 100644 --- a/packages/compiler/src/compiler.ts +++ b/packages/compiler/src/compiler.ts @@ -67,7 +67,7 @@ export * from './ml_parser/html_tags'; export * from './ml_parser/interpolation_config'; export * from './ml_parser/tags'; export {NgModuleCompiler} from './ng_module_compiler'; -export {AssertNotNull, BinaryOperator, BinaryOperatorExpr, BuiltinMethod, BuiltinVar, CastExpr, ClassField, ClassMethod, ClassStmt, CommaExpr, CommentStmt, ConditionalExpr, DeclareFunctionStmt, DeclareVarStmt, ExpressionStatement, ExpressionVisitor, ExternalExpr, ExternalReference, FunctionExpr, IfStmt, InstantiateExpr, InvokeFunctionExpr, InvokeMethodExpr, LiteralArrayExpr, LiteralExpr, LiteralMapExpr, NotExpr, ReadKeyExpr, ReadPropExpr, ReadVarExpr, ReturnStatement, StatementVisitor, ThrowStmt, TryCatchStmt, WriteKeyExpr, WritePropExpr, WriteVarExpr, StmtModifier, Statement, collectExternalReferences} from './output/output_ast'; +export {AssertNotNull, BinaryOperator, BinaryOperatorExpr, BuiltinMethod, BuiltinVar, CastExpr, ClassField, ClassMethod, ClassStmt, CommaExpr, CommentStmt, ConditionalExpr, DeclareFunctionStmt, DeclareVarStmt, ExpressionStatement, ExpressionVisitor, ExternalExpr, ExternalReference, FunctionExpr, IfStmt, InstantiateExpr, InvokeFunctionExpr, InvokeMethodExpr, JSDocCommentStmt, LiteralArrayExpr, LiteralExpr, LiteralMapExpr, NotExpr, ReadKeyExpr, ReadPropExpr, ReadVarExpr, ReturnStatement, StatementVisitor, ThrowStmt, TryCatchStmt, WriteKeyExpr, WritePropExpr, WriteVarExpr, StmtModifier, Statement, collectExternalReferences} from './output/output_ast'; export {EmitterVisitorContext} from './output/abstract_emitter'; export * from './output/ts_emitter'; export * from './parse_util'; diff --git a/packages/compiler/src/output/abstract_emitter.ts b/packages/compiler/src/output/abstract_emitter.ts index ae43117306..3909393c8a 100644 --- a/packages/compiler/src/output/abstract_emitter.ts +++ b/packages/compiler/src/output/abstract_emitter.ts @@ -233,10 +233,18 @@ export abstract class AbstractEmitterVisitor implements o.StatementVisitor, o.Ex return null; } visitCommentStmt(stmt: o.CommentStmt, ctx: EmitterVisitorContext): any { - const lines = stmt.comment.split('\n'); - lines.forEach((line) => { ctx.println(stmt, `// ${line}`); }); + 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; visitWriteVarExpr(expr: o.WriteVarExpr, ctx: EmitterVisitorContext): any { diff --git a/packages/compiler/src/output/output_ast.ts b/packages/compiler/src/output/output_ast.ts index e7cf3644c3..d10ec23630 100644 --- a/packages/compiler/src/output/output_ast.ts +++ b/packages/compiler/src/output/output_ast.ts @@ -897,9 +897,8 @@ export class IfStmt extends Statement { } } - export class CommentStmt extends Statement { - constructor(public comment: string, sourceSpan?: ParseSourceSpan|null) { + constructor(public comment: string, public multiline = false, sourceSpan?: ParseSourceSpan|null) { super(null, sourceSpan); } isEquivalent(stmt: Statement): boolean { return stmt instanceof CommentStmt; } @@ -908,6 +907,18 @@ export class CommentStmt extends Statement { } } +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( @@ -947,6 +958,7 @@ export interface StatementVisitor { 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 { @@ -1157,6 +1169,10 @@ export class AstTransformer implements StatementVisitor, ExpressionVisitor { return this.transformStmt(stmt, context); } + visitJSDocCommentStmt(stmt: JSDocCommentStmt, context: any): any { + return this.transformStmt(stmt, context); + } + visitAllStatements(stmts: Statement[], context: any): Statement[] { return stmts.map(stmt => stmt.visitStatement(this, context)); } @@ -1321,6 +1337,7 @@ export class RecursiveAstVisitor implements StatementVisitor, ExpressionVisitor 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)); } @@ -1467,3 +1484,54 @@ export function literal( value: any, type?: Type | null, sourceSpan?: ParseSourceSpan | null): LiteralExpr { return new LiteralExpr(value, type, sourceSpan); } + +// The list of JSDoc tags that we currently support. Extend it if needed. +export const enum JSDocTagName {Desc = 'desc', Id = 'id', Meaning = 'meaning'} + +/* + * TypeScript has an API for JSDoc already, but it's not exposed. + * https://github.com/Microsoft/TypeScript/issues/7393 + * For now we create types that are similar to theirs so that migrating + * to their API will be easier. See e.g. `ts.JSDocTag` and `ts.JSDocComment`. + */ +export type JSDocTag = { + // `tagName` is e.g. "param" in an `@param` declaration + tagName: JSDocTagName | string; + // Any remaining text on the tag, e.g. the description + text?: string; +} | {// no `tagName` for plain text documentation that occurs before any `@param` lines + tagName?: undefined + text: string, +}; + + /* + * Serializes a `Tag` into a string. + * Returns a string like " @foo {bar} baz" (note the leading whitespace before `@foo`). + */ + function tagToString(tag: JSDocTag): string { + let out = ''; + if (tag.tagName) { + out += ` @${tag.tagName}`; + } + if (tag.text) { + if (tag.text.match(/\/\*|\*\//)) { + throw new Error('JSDoc text cannot contain "/*" and "*/"'); + } + out += ' ' + tag.text.replace(/@/g, '\\@'); + } + return out; + } + + function serializeTags(tags: JSDocTag[]): string { + if (tags.length === 0) return ''; + + let out = '*\n'; + for (const tag of tags) { + out += ' *'; + // If the tagToString is multi-line, insert " * " prefixes on subsequent lines. + out += tagToString(tag).replace(/\n/g, '\n * '); + out += '\n'; + } + out += ' '; + return out; + } diff --git a/packages/compiler/src/output/output_interpreter.ts b/packages/compiler/src/output/output_interpreter.ts index 24747422e2..f7fbec6412 100644 --- a/packages/compiler/src/output/output_interpreter.ts +++ b/packages/compiler/src/output/output_interpreter.ts @@ -226,6 +226,7 @@ class StatementInterpreter implements o.StatementVisitor, o.ExpressionVisitor { 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/test/output/output_ast_spec.ts b/packages/compiler/test/output/output_ast_spec.ts index 5e03c617f5..2af2adaa99 100644 --- a/packages/compiler/test/output/output_ast_spec.ts +++ b/packages/compiler/test/output/output_ast_spec.ts @@ -21,5 +21,22 @@ 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 cc64344e9a..e9f3c41a49 100644 --- a/packages/compiler/test/output/ts_emitter_spec.ts +++ b/packages/compiler/test/output/ts_emitter_spec.ts @@ -414,10 +414,38 @@ const externalModuleIdentifier = new o.ExternalReference(anotherModuleUrl, 'some .toEqual('var a:{[key: string]:number} = (null as any);'); }); - it('should support a preamble', () => { - expect(emitStmt(o.variable('a').toStmt(), '/* SomePreamble */')).toBe([ - '/* SomePreamble */', 'a;' - ].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.CommentStmt('Simple comment'))).toBe('// Simple comment'); + }); + + 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 */`); + }); + + 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'}, + ]))) + .toBe( + `/**\n * Intro comment\n * @desc description\n * @id {number} identifier 123\n */`); + }); }); describe('emitter context', () => { @@ -461,4 +489,4 @@ function calculateLineCol(text: string, offset: number): {line: number, col: num cur = next; } return {line, col: 0}; -} \ No newline at end of file +}