feat(compiler): support for singleline, multiline & jsdoc comments (#22715)

PR Close #22715

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 =
-      <ts.Statement[]>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) {

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();
 }

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';

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 {

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;
+  }

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);

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();
+      });
+    });
   });
 }

packages/compiler/test/output/ts_emitter_spec.ts

@@ -414,12 +414,40 @@ const externalModuleIdentifier = new o.ExternalReference(anotherModuleUrl, 'some
           .toEqual('var a:{[key: string]:number} = (null as any);');
     });
 
+    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', () => {
       it('should be able to back to the generating span', () => {
         const file = new ParseSourceFile('some content', 'a.ts');