Add more docs (review feedback from @evmar)

Author: tbosch

src/decorator-annotator.ts

@@ -133,6 +133,18 @@ export class DecoratorClassVisitor {
     this.propDecorators.set(name, decorators);
   }
 
+  /**
+   * For lowering decorators, we need to refer to constructor types.
+   * So we start with the identifiers that represent these types.
+   * However, TypeScript does not allow use to emit them in a value position
+   * as it associated different symbol information with it.
+   *
+   * This method looks for the place where the value that is associated to
+   * the type is defined and returns that identifier instead.
+   *
+   * @param typeSymbol
+   * @return The identifier
+   */
   private getValueIdentifierForType(typeSymbol: ts.Symbol): ts.Identifier|null {
     if (!typeSymbol.valueDeclaration) {
       return null;
@@ -348,6 +360,7 @@ export function visitClassContentIncludingDecorators(
     classDecl: ts.ClassDeclaration, rewriter: Rewriter, decoratorVisitor?: DecoratorClassVisitor) {
   if (rewriter.file.text[classDecl.getEnd() - 1] !== '}') {
     rewriter.error(classDecl, 'unexpected class terminator');
+    return;
   }
   rewriter.writeNodeFrom(classDecl, classDecl.getStart(), classDecl.getEnd() - 1);
   // At this point, we've emitted up through the final child of the class, so all that

src/rewriter.ts

@@ -84,8 +84,8 @@ export abstract class Rewriter {
     this.writeNodeFrom(node, pos);
   }
 
-  writeNodeFrom(node: ts.Node, pos: number, end = Number.MAX_SAFE_INTEGER) {
-    if (node.getEnd() <= this.skipUpToOffset) {
+  writeNodeFrom(node: ts.Node, pos: number, end = node.getEnd()) {
+    if (end <= this.skipUpToOffset) {
       return;
     }
     const oldSkipUpToOffset = this.skipUpToOffset;
@@ -95,7 +95,7 @@ export abstract class Rewriter {
       this.visit(child);
       pos = child.getEnd();
     });
-    this.writeRange(node, pos, Math.min(end, node.getEnd()));
+    this.writeRange(node, pos, end);
     this.skipUpToOffset = oldSkipUpToOffset;
   }
 

src/transformer.ts

@@ -161,7 +161,7 @@ export function emitWithTsickle(
     });
   }
   // All diagnostics (including warnings) are treated as errors.
-  // If we've decided to ignore them, just discard them.
+  // If the host decides to ignore warnings, just discard them.
   // Warnings include stuff like "don't use @type in your jsdoc"; tsickle
   // warns and then fixes up the code to be Closure-compatible anyway.
   tsickleDiagnostics = tsickleDiagnostics.filter(

src/transformer_sourcemap.ts

@@ -11,6 +11,11 @@ import * as ts from 'typescript';
 import {SourceMapper, SourcePosition} from './source_map_utils';
 import {isTypeNodeKind, updateSourceFileNode, visitEachChild, visitNodeWithSynthesizedComments} from './transformer_util';
 
+/**
+ * @fileoverview Creates a TypeScript transformer that parses code into a new `ts.SourceFile`,
+ * marks the nodes as synthetic and where possible maps the new nodes back to the original nodes
+ * via sourcemap information.
+ */
 export function createTransformerFromSourceMap(
     operator: (sourceFile: ts.SourceFile, sourceMapper: SourceMapper) =>
         string): ts.TransformerFactory<ts.SourceFile> {
@@ -77,6 +82,10 @@ export function createTransformerFromSourceMap(
   };
 }
 
+/**
+ * Implementation of the `SourceMapper` that stores and retrieves mappings
+ * to original nodes.
+ */
 class NodeSourceMapper implements SourceMapper {
   private originalNodeByGeneratedRange = new Map<string, ts.Node>();
   private genStartPositions = new Map<ts.Node, number>();

src/transformer_util.ts

@@ -12,7 +12,6 @@ import * as tsickle from './tsickle';
 /**
  * Adjusts the given CustomTransformers with additional transformers
  * to fix bugs in TypeScript.
- * @param given A
  */
 export function createCustomTransformers(given: ts.CustomTransformers): ts.CustomTransformers {
   if (!given.after && !given.before) {
@@ -51,7 +50,7 @@ function assertFileContext(context: TransformationContext, sourceFile: ts.Source
 }
 
 /**
- * And extended version of the TransformationContext that stores the FileContext as well.
+ * An extended version of the TransformationContext that stores the FileContext as well.
  */
 interface TransformationContext extends ts.TransformationContext {
   fileContext?: FileContext;
@@ -74,6 +73,8 @@ class FileContext {
  * Transform that needs to be executed right before TypeScript's transform.
  *
  * This prepares the node tree to workaround some bugs in the TypeScript emitter.
+ * TODO(tbosch): file issues with TypeScript, tracked in
+ * https://github.com/angular/tsickle/issues/528.
  */
 function preTypeScriptTransform(context: ts.TransformationContext) {
   return (sourceFile: ts.SourceFile) => {
@@ -178,7 +179,7 @@ function postTypescriptTransform(context: ts.TransformationContext) {
         } else if (
             parent3 && parent3.kind === ts.SyntaxKind.VariableStatement &&
             tsickle.hasModifierFlag(parent3, ts.ModifierFlags.Export)) {
-          // TypeScript ignore synthetic comments on exported variables.
+          // TypeScript ignores synthetic comments on exported variables.
           // find the parent ExpressionStatement like exports.foo = ...
           const expressionStmt =
               lastNodeWith(nodePath, (node) => node.kind === ts.SyntaxKind.ExpressionStatement);
@@ -188,7 +189,7 @@ function postTypescriptTransform(context: ts.TransformationContext) {
           }
         }
       }
-      // TypeScript ignore synthetic comments on reexport / import statements.
+      // TypeScript ignores synthetic comments on reexport / import statements.
       const moduleName = extractModuleNameFromRequireVariableStatement(node);
       if (moduleName && fileContext.importOrReexportDeclarations) {
         const index = importOrReexportsCount++;
@@ -240,8 +241,13 @@ function lastNodeWith(nodes: ts.Node[], predicate: (node: ts.Node) => boolean):
 }
 
 /**
- * Synthesizes the comments before and after a node
- * befor calling a callback.
+ * Convert comment text ranges before and after a node
+ * into ts.SynthesizedComments for the node and prevent the
+ * comment text ranges to be emitted, to allow
+ * changing these comments.
+ *
+ * This function takes a visitor to be able to do some
+ * state management after the caller is done changing a node.
  */
 export function visitNodeWithSynthesizedComments<T extends ts.Node>(
     context: ts.TransformationContext, sourceFile: ts.SourceFile, node: T,
@@ -288,7 +294,8 @@ function resetNodeTextRangeToPreventDuplicateComments<T extends ts.Node>(node: T
   if (node.kind === ts.SyntaxKind.PropertyDeclaration) {
     allowTextRange = false;
     const pd = node as ts.Node as ts.PropertyDeclaration;
-    // TODO(tbosch): Using pd.initializer! as the typescript typings for intializer are incorrect.
+    // TODO(tbosch): Using pd.initializer! as the typescript typings for intializer are incorrect,
+    // tracked in https://github.com/angular/tsickle/issues/528.
     node = ts.updateProperty(
                pd, pd.decorators, pd.modifiers, resetTextRange(pd.name) as ts.PropertyName, pd.type,
                pd.initializer!) as ts.Node as T;
@@ -311,6 +318,13 @@ function resetNodeTextRangeToPreventDuplicateComments<T extends ts.Node>(node: T
   }
 }
 
+/**
+ * Reads in the leading comment text ranges of the given node,
+ * converts them into `ts.SyntheticComment`s and stores them on the node.
+ *
+ * @param lastCommentEnd The end of the last comment
+ * @return The end of the last found comment, -1 if no comment was found.
+ */
 function synthesizeLeadingComments(
     sourceFile: ts.SourceFile, node: ts.Node, lastCommentEnd: number): number {
   const parent = node.parent;
@@ -329,6 +343,12 @@ function synthesizeLeadingComments(
   return -1;
 }
 
+/**
+ * Reads in the trailing comment text ranges of the given node,
+ * converts them into `ts.SyntheticComment`s and stores them on the node.
+ *
+ * @return The end of the last found comment, -1 if no comment was found.
+ */
 function synthesizeTrailingComments(sourceFile: ts.SourceFile, node: ts.Node): number {
   const parent = node.parent;
   const sharesEndWithParent = parent && parent.kind !== ts.SyntaxKind.Block &&
@@ -344,6 +364,16 @@ function synthesizeTrailingComments(sourceFile: ts.SourceFile, node: ts.Node): n
   return -1;
 }
 
+/**
+ * Convert leading/trailing detached comment ranges of statement arrays
+ * (e.g. the statements of a ts.SourceFile or ts.Block) into
+ * `ts.NonEmittedStatement`s with `ts.SynthesizedComment`s and
+ * prepends / appends them to the given statement array.
+ * This is needed to allow changing these comments.
+ *
+ * This function takes a visitor to be able to do some
+ * state management after the caller is done changing a node.
+ */
 function visitNodeStatementsWithSynthesizedComments<T extends ts.Node>(
     context: ts.TransformationContext, sourceFile: ts.SourceFile, node: T,
     statements: ts.NodeArray<ts.Statement>,
@@ -371,14 +401,22 @@ function visitNodeStatementsWithSynthesizedComments<T extends ts.Node>(
   return visitor(node, statements);
 }
 
+/**
+ * Convert leading detached comment ranges of statement arrays
+ * (e.g. the statements of a ts.SourceFile or ts.Block) into a
+ * `ts.NonEmittedStatement` with `ts.SynthesizedComment`s.
+ *
+ * A Detached leading comment is the first comment in a SourceFile / Block
+ * that is separated with a newline from the first statement.
+ */
 function synthesizeDetachedLeadingComments(
     sourceFile: ts.SourceFile, node: ts.Node, statements: ts.NodeArray<ts.Statement>):
     {commentStmt: ts.Statement | null, lastCommentEnd: number} {
   let triviaEnd = statements.end;
   if (statements.length) {
     triviaEnd = statements[statements.length - 1].getStart();
   }
-  const detachedComments = getDetachedStartingComments(sourceFile, statements.pos, triviaEnd);
+  const detachedComments = getDetachedLeadingCommentRanges(sourceFile, statements.pos, triviaEnd);
   if (!detachedComments.length) {
     return {commentStmt: null, lastCommentEnd: -1};
   }
@@ -390,6 +428,14 @@ function synthesizeDetachedLeadingComments(
   return {commentStmt, lastCommentEnd};
 }
 
+/**
+ * Convert trailing detached comment ranges of statement arrays
+ * (e.g. the statements of a ts.SourceFile or ts.Block) into a
+ * `ts.NonEmittedStatement` with `ts.SynthesizedComment`s.
+ *
+ * A Detached trailing comment are all comments after the first newline
+ * the follows the last statement in a SourceFile / Block.
+ */
 function synthesizeDetachedTrailingComments(
     sourceFile: ts.SourceFile, node: ts.Node, statements: ts.NodeArray<ts.Statement>):
     {commentStmt: ts.Statement | null, lastCommentEnd: number} {
@@ -413,8 +459,14 @@ function synthesizeDetachedTrailingComments(
   return {commentStmt, lastCommentEnd};
 }
 
-// Adapted from compiler/comments.ts in TypeScript
-function getDetachedStartingComments(
+/**
+ * Calculates the the detached leading comment ranges in an area of a SourceFile.
+ * @param sourceFile The source file
+ * @param start Where to start scanning
+ * @param end Where to end scanning
+ */
+// Note: This code is based on compiler/comments.ts in TypeScript
+function getDetachedLeadingCommentRanges(
     sourceFile: ts.SourceFile, start: number, end: number): ts.CommentRange[] {
   const leadingComments = getAllLeadingCommentRanges(sourceFile, start, end);
   if (!leadingComments || !leadingComments.length) {
@@ -459,8 +511,13 @@ function getLineOfPos(sourceFile: ts.SourceFile, pos: number): number {
   return ts.getLineAndCharacterOfPosition(sourceFile, pos).line;
 }
 
-
-function synthesizeCommentRanges(sourceFile: ts.SourceFile, parsedComments: ts.CommentRange[]) {
+/**
+ * Converts `ts.CommentRange`s into `ts.SynthesizedComment`s
+ * @param sourceFile
+ * @param parsedComments
+ */
+function synthesizeCommentRanges(
+    sourceFile: ts.SourceFile, parsedComments: ts.CommentRange[]): ts.SynthesizedComment[] {
   const synthesizedComments: ts.SynthesizedComment[] = [];
   parsedComments.forEach(({kind, pos, end, hasTrailingNewLine}, commentIdx) => {
     let commentText = sourceFile.text.substring(pos, end).trim();
@@ -478,13 +535,25 @@ function synthesizeCommentRanges(sourceFile: ts.SourceFile, parsedComments: ts.C
   return synthesizedComments;
 }
 
+/**
+ * Creates a non emitted statement that can be used to store synthesized comments.
+ */
 function createNotEmittedStatement(sourceFile: ts.SourceFile) {
   const stmt = ts.createNotEmittedStatement(sourceFile);
   ts.setOriginalNode(stmt, undefined);
   ts.setTextRange(stmt, {pos: 0, end: 0});
   return stmt;
 }
 
+/**
+ * Returns the leading comment ranges in the source file that start at the given position.
+ * This is the same as `ts.getLeadingCommentRanges`, except that it does not skip
+ * comments before the first newline in the range.
+ *
+ * @param sourceFile
+ * @param start Where to start scanning
+ * @param end Where to end scanning
+ */
 function getAllLeadingCommentRanges(
     sourceFile: ts.SourceFile, start: number, end: number): ts.CommentRange[] {
   // exeute ts.getLeadingCommentRanges with pos = 0 so that it does not skip
@@ -498,6 +567,15 @@ function getAllLeadingCommentRanges(
                            }));
 }
 
+/**
+ * This is a version of `ts.updateSourceFileNode` that prevents some
+ * bugs in TypeScript.
+ * TODO(tbosch): file an issue with TypeScript, tracked in
+ * https://github.com/angular/tsickle/issues/528.
+ *
+ * @param sf
+ * @param statements
+ */
 export function updateSourceFileNode(
     sf: ts.SourceFile, statements: ts.NodeArray<ts.Statement>): ts.SourceFile {
   if (statements === sf.statements) {
@@ -522,6 +600,12 @@ export function getMutableClone<T extends ts.Node>(node: T): T {
   return clone;
 }
 
+/**
+ * This is a version of `ts.visitEachChild` that does not visit children of types
+ * to prevent errors from TypeScript.
+ * TODO(tbosch): file an issue with TypeScript, tracked in
+ * https://github.com/angular/tsickle/issues/528.
+ */
 export function visitEachChild<T extends ts.Node>(
     node: T, visitor: ts.Visitor, context: ts.TransformationContext): T {
   // Don't visit children of types,