Preserve comments for static fields, exported variables, import and re-export declarations.

Author: tbosch

src/transformer.ts

@@ -97,6 +97,10 @@ export function emitWithTsickle(
       afterTsTransformers.push(...customTransformers.afterTs);
     }
   }
+  afterTsTransformers.push((ctx) => (sf) => {
+    addSyntheticCommentsAfterTsTransformer(ctx, sf);
+    return sf;
+  });
   const writeFileDelegate = writeFile || tsHost.writeFile.bind(tsHost);
   const modulesManifest = new ModulesManifest();
   const writeFileImpl =
@@ -155,10 +159,14 @@ function createTransformer(
     host: TransformerHost,
     operator: (sourceFile: ts.SourceFile, sourceMapper: SourceMapper) =>
         string): ts.TransformerFactory<ts.SourceFile> {
-  return (context: ts.TransformationContext) => (sourceFile: ts.SourceFile): ts.SourceFile => {
+  return (context: TsickleTransformationContext) => (sourceFile: ts.SourceFile): ts.SourceFile => {
     if (host.shouldSkipTsickleProcessing(sourceFile.fileName)) {
       return sourceFile;
     }
+    // Reset the fields on the context for each file.
+    // This assumes that TypeScript transforms file by file.
+    context.importOrReexportDeclarations = [];
+    context.syntheticNodeParents = new Map();
     const sourceMapper = new NodeSourceMapper();
     const newFile = ts.createSourceFile(
         sourceFile.fileName, operator(sourceFile, sourceMapper), ts.ScriptTarget.Latest, true);
@@ -182,7 +190,7 @@ function createTransformer(
     transformedFile.statements = synthStmts;
     // Need to set parents as some of TypeScript's emit logic relies on it (e.g. emitting
     // decorators)
-    synthStmts.forEach(stmt => setParentsForSyntheticNodes(stmt, transformedFile));
+    synthStmts.forEach(stmt => setParentsForSyntheticNodes(context, stmt, transformedFile));
     return transformedFile;
   };
 }
@@ -252,48 +260,18 @@ function convertToSyntheticNode(
     }
   }
   if (originalNode) {
-    ts.setOriginalNode(node, originalNode);
-    ts.setSourceMapRange(node, originalNode);
-    // Needed so that e.g. `module { ... }` prints the variable statement
-    // before the closure.
-    // tslint:disable-next-line:no-any
-    (node as any).symbol = (originalNode as any).symbol;
-  }
-  if (node.kind === ts.SyntaxKind.VariableStatement) {
-    if (node.modifiers && node.modifiers.some(m => m.kind === ts.SyntaxKind.ExportKeyword)) {
-      // Note: TypeScript does not emit synthetic comments on exported variable statements,
-      // so we have to create a fake statement to hold the comments.
-      // Note: We cannot use the trick via `ts.setTextRange` that we use for export / import
-      // declarations, as somehow TypeScript would still skip some comments.
-      const synthComments = ts.getSyntheticLeadingComments(node);
-      if (synthComments && synthComments.length) {
-        ts.setSyntheticLeadingComments(node, []);
-        const commentStmt = createNotEmittedStatement(originalSourceFile);
-        ts.setSyntheticLeadingComments(commentStmt, synthComments);
-        return [commentStmt, node];
-      }
-    }
-  }
-
-  if (node.kind === ts.SyntaxKind.ExportDeclaration ||
-      node.kind === ts.SyntaxKind.ImportDeclaration) {
-    if (originalNode) {
-      // Note: TypeScript does not emit synthetic comments on import / export declarations.
-      // As tsickle never modifies comments of export / import declarations,
-      // we can just set the original text range and let typescript reuse the original comments.
-      // This is simpler than emitting a fake statement with the synthesized comments,
-      // as in that case we would need to calculate whether the export / import statement
-      // will be elided, which is far from trivial (especially for import statements).
-      ts.setTextRange(node, {pos: originalNode.getFullStart(), end: originalNode.getEnd()});
-
+    if (node.kind !== ts.SyntaxKind.ExportDeclaration) {
       // Note: Somewhow, TypeScript does not write exports correctly if we
       // have an original node for an export declaration. So we reset it to undefined.
       // This still allows TypeScript to elided exports as it will actually check
       // the values in the exportClause instead.
-      if (node.kind === ts.SyntaxKind.ExportDeclaration) {
-        ts.setOriginalNode(node, undefined);
-      }
+      ts.setOriginalNode(node, originalNode);
     }
+    ts.setSourceMapRange(node, originalNode);
+    // Needed so that e.g. `module { ... }` prints the variable statement
+    // before the closure.
+    // tslint:disable-next-line:no-any
+    (node as any).symbol = (originalNode as any).symbol;
   }
   return node;
 }
@@ -480,27 +458,157 @@ class CommentSynthesizer {
 
 
   private convertCommentRanges(parsedComments: ts.CommentRange[], text: string) {
-    return parsedComments.map(({kind, pos, end, hasTrailingNewLine}, commentIdx) => {
+    const synthesizedComments: ts.SynthesizedComment[] = [];
+    parsedComments.forEach(({kind, pos, end, hasTrailingNewLine}, commentIdx) => {
       let commentText = text.substring(pos, end).trim();
       if (kind === ts.SyntaxKind.MultiLineCommentTrivia) {
         commentText = commentText.replace(/(^\/\*)|(\*\/$)/g, '');
       } else if (kind === ts.SyntaxKind.SingleLineCommentTrivia) {
+        if (commentText.startsWith('///')) {
+          // tripple-slash comments are typescript specific, ignore them in the output.
+          return;
+        }
         commentText = commentText.replace(/(^\/\/)/g, '');
       }
-      const comment:
-          ts.SynthesizedComment = {kind, text: commentText, hasTrailingNewLine, pos: -1, end: -1};
-      return comment;
+      synthesizedComments.push({kind, text: commentText, hasTrailingNewLine, pos: -1, end: -1});
     });
+    return synthesizedComments;
   }
 }
 
-function setParentsForSyntheticNodes(node: ts.Node, parent: ts.Node|undefined) {
-  if (!(node.flags & ts.NodeFlags.Synthesized)) {
-    return;
+/**
+ * This fixes places where the TypeScript transformer does not
+ * emit synthetic comments.
+ */
+function addSyntheticCommentsAfterTsTransformer(
+    ctx: TsickleTransformationContext, node: ts.Node, path: ts.Node[] = [],
+    importOrReexports: {count: number} = {
+      count: 0
+    }) {
+  if (node.kind === ts.SyntaxKind.Identifier) {
+    const parent1 = getSyntheticParent(ctx, node);
+    const parent2 = parent1 && getSyntheticParent(ctx, parent1);
+    const parent3 = parent2 && getSyntheticParent(ctx, parent2);
+
+    if (parent1 && parent1.kind === ts.SyntaxKind.PropertyDeclaration &&
+        tsickle.hasModifierFlag(parent1, ts.ModifierFlags.Static)) {
+      // TypeScript ignore synthetic comments on static property declarations.
+      // find the parent ExpressionStatement like MyClass.foo = ...
+      const expressionStmt = lastNodeWith(
+          path, (node) => node.kind === ts.SyntaxKind.ExpressionStatement ? true : undefined);
+      if (expressionStmt) {
+        ts.setSyntheticLeadingComments(
+            expressionStmt, ts.getSyntheticLeadingComments(parent1) || []);
+      }
+    } else if (
+        parent3 && parent3.kind === ts.SyntaxKind.VariableStatement &&
+        tsickle.hasModifierFlag(parent3, ts.ModifierFlags.Export)) {
+      // TypeScript ignore synthetic comments on exported variables.
+      // find the parent ExpressionStatement like exports.foo = ...
+      const expressionStmt = lastNodeWith(
+          path, (node) => node.kind === ts.SyntaxKind.ExpressionStatement ? true : undefined);
+      if (expressionStmt) {
+        ts.setSyntheticLeadingComments(
+            expressionStmt, ts.getSyntheticLeadingComments(parent3) || []);
+      }
+    }
+  }
+  // TypeScript ignore synthetic comments on reexport / import statements.
+  const moduleName = extractModuleNameFromRequireVariableStatement(node);
+  if (moduleName && ctx.importOrReexportDeclarations) {
+    const index = importOrReexports.count++;
+    const originalImportOrReexportNode = ctx.importOrReexportDeclarations[index];
+    if (originalImportOrReexportNode) {
+      ts.setSyntheticLeadingComments(
+          node, ts.getSyntheticLeadingComments(originalImportOrReexportNode) || []);
+    }
+  }
+  node.forEachChild((child) => {
+    path.push(node);
+    addSyntheticCommentsAfterTsTransformer(ctx, child, path, importOrReexports);
+    path.pop();
+  });
+}
+
+function extractModuleNameFromRequireVariableStatement(node: ts.Node): string|null {
+  if (node.kind !== ts.SyntaxKind.VariableStatement) {
+    return null;
+  }
+  const varStmt = node as ts.VariableStatement;
+  const decls = varStmt.declarationList.declarations;
+  let init: ts.Expression|undefined;
+  if (decls.length !== 1 || !(init = decls[0].initializer) ||
+      init.kind !== ts.SyntaxKind.CallExpression) {
+    return null;
+  }
+  const callExpr = init as ts.CallExpression;
+  if (callExpr.expression.kind !== ts.SyntaxKind.Identifier ||
+      (callExpr.expression as ts.Identifier).text !== 'require' ||
+      callExpr.arguments.length !== 1) {
+    return null;
+  }
+  const moduleExpr = callExpr.arguments[0];
+  if (moduleExpr.kind !== ts.SyntaxKind.StringLiteral) {
+    return null;
+  }
+  return (moduleExpr as ts.StringLiteral).text;
+}
+
+function lastNodeWith(nodes: ts.Node[], predicate: (node: ts.Node) => boolean | undefined): ts.Node|
+    null {
+  for (let i = nodes.length - 1; i >= 0; i--) {
+    const node = nodes[i];
+    if (predicate(node) !== undefined) {
+      return node;
+    }
+  }
+  return null;
+}
+
+interface TsickleTransformationContext extends ts.TransformationContext {
+  syntheticNodeParents?: Map<ts.Node, ts.Node|undefined>;
+  importOrReexportDeclarations?: Array<ts.Node&{moduleSpecifier: ts.StringLiteral}>;
+}
+
+function getSyntheticParent(ctx: TsickleTransformationContext, node: ts.Node): ts.Node|null {
+  if (!ctx.syntheticNodeParents) {
+    return null;
+  }
+  return ctx.syntheticNodeParents.get(node) || null;
+}
+
+function setParentsForSyntheticNodes(
+    ctx: TsickleTransformationContext, node: ts.Node, parent: ts.Node|undefined) {
+  if (node.flags & ts.NodeFlags.Synthesized) {
+    // Note: don't update the `parent` of original nodes, as:
+    // 1) we don't want to change them at all
+    // 2) TS emit becomes errorneous in some cases if we add a synthetic parent.
+    node.parent = parent;
+  }
+  // This field has been filled in our transformer as first step.
+  ctx.syntheticNodeParents!.set(node, parent);
+  let importOrReexportNode: ts.Node&{moduleSpecifier: ts.StringLiteral}|undefined;
+  if (node.kind === ts.SyntaxKind.ImportDeclaration) {
+    const id = node as ts.ImportDeclaration;
+    // this should always be the case...
+    if (id.moduleSpecifier.kind === ts.SyntaxKind.StringLiteral) {
+      importOrReexportNode = node as ts.Node & {moduleSpecifier: ts.StringLiteral};
+    }
+  } else if (node.kind === ts.SyntaxKind.ExportDeclaration) {
+    const ed = node as ts.ExportDeclaration;
+    // this should always be the case...
+    if (ed.moduleSpecifier && ed.moduleSpecifier.kind === ts.SyntaxKind.StringLiteral) {
+      // Use any as TypeScript gives a cast error...
+      // tslint:disable-next-line:no-any
+      importOrReexportNode = ed as any /* ts.Node & {moduleSpecifier: ts.StringLiteral} */;
+    }
+  }
+  if (importOrReexportNode) {
+    // This field has been filled in our transformer as first step.
+    ctx.importOrReexportDeclarations!.push(importOrReexportNode);
   }
-  node.parent = parent;
   node.forEachChild(child => {
-    setParentsForSyntheticNodes(child, node);
+    setParentsForSyntheticNodes(ctx, child, node);
   });
 }
 

src/tsickle.ts

@@ -138,7 +138,7 @@ export function formatDiagnostics(diags: ts.Diagnostic[]): string {
 }
 
 /** @return true if node has the specified modifier flag set. */
-function hasModifierFlag(node: ts.Node, flag: ts.ModifierFlags): boolean {
+export function hasModifierFlag(node: ts.Node, flag: ts.ModifierFlags): boolean {
   return (ts.getCombinedModifierFlags(node) & flag) !== 0;
 }