Merge pull request microsoft#2406 from Microsoft/pinnedComments

Simplify code for emitting comments.

Author: CyrusNajmabadi

src/compiler/emitter.ts

@@ -1594,25 +1594,12 @@ module ts {
  /** write emitted output to disk*/
  let writeEmittedFiles = writeJavaScriptFile;
 
- /** Emit leading comments of the node */
- let emitLeadingComments = compilerOptions.removeComments ? (node: Node) => { } : emitLeadingDeclarationComments;
-
- /** Emit Trailing comments of the node */
- let emitTrailingComments = compilerOptions.removeComments ? (node: Node) => { } : emitTrailingDeclarationComments;
-
- let emitLeadingCommentsOfPosition = compilerOptions.removeComments ? (pos: number) => { } : emitLeadingCommentsOfLocalPosition;
-
  let detachedCommentsInfo: { nodePos: number; detachedCommentEndPos: number }[];
 
- /** Emit detached comments of the node */
- let emitDetachedComments = compilerOptions.removeComments ? (node: TextRange) => { } : emitDetachedCommentsAtPosition;
-
  let writeComment = writeCommentRange;
 
  /** Emit a node */
- let emitNodeWithoutSourceMap = compilerOptions.removeComments ? emitNodeWithoutSourceMapWithoutComments : emitNodeWithoutSourceMapWithComments;
  let emit = emitNodeWithoutSourceMap;
- let emitWithoutComments = emitNodeWithoutSourceMapWithoutComments;
 
  /** Called just before starting emit of a node */
  let emitStart = function (node: Node) { };
@@ -2091,17 +2078,8 @@ module ts {
  }
  }
 
- function emitNodeWithSourceMapWithoutComments(node: Node) {
- if (node) {
- recordEmitNodeStartSpan(node);
- emitNodeWithoutSourceMapWithoutComments(node);
- recordEmitNodeEndSpan(node);
- }
- }
-
  writeEmittedFiles = writeJavaScriptAndSourceMapFile;
  emit = emitNodeWithSourceMap;
- emitWithoutComments = emitNodeWithSourceMapWithoutComments;
  emitStart = recordEmitNodeStartSpan;
  emitEnd = recordEmitNodeEndSpan;
  emitToken = writeTextWithSpanRecord;
@@ -4365,7 +4343,7 @@ module ts {
 
  function emitFunctionDeclaration(node: FunctionLikeDeclaration) {
  if (nodeIsMissing(node.body)) {
- return emitPinnedOrTripleSlashComments(node);
+ return emitOnlyPinnedOrTripleSlashComments(node);
  }
 
  if (node.kind !== SyntaxKind.MethodDeclaration && node.kind !== SyntaxKind.MethodSignature) {
@@ -4521,10 +4499,7 @@ module ts {
  write(" ");
  emitStart(body);
  write("return ");
-
- // Don't emit comments on this body. We'll have already taken care of it above 
- // when we called emitDetachedComments.
- emitWithoutComments(body);
+ emit(body);
  emitEnd(body);
  write(";");
  emitTempDeclarations(/*newLine*/ false);
@@ -4535,10 +4510,7 @@ module ts {
  writeLine();
  emitLeadingComments(node.body);
  write("return ");
-
- // Don't emit comments on this body. We'll have already taken care of it above 
- // when we called emitDetachedComments.
- emitWithoutComments(node.body);
+ emit(body);
  write(";");
  emitTrailingComments(node.body);
 
@@ -4670,7 +4642,7 @@ module ts {
  forEach(node.members, member => {
  if (member.kind === SyntaxKind.MethodDeclaration || node.kind === SyntaxKind.MethodSignature) {
  if (!(<MethodDeclaration>member).body) {
- return emitPinnedOrTripleSlashComments(member);
+ return emitOnlyPinnedOrTripleSlashComments(member);
  }
 
  writeLine();
@@ -4745,7 +4717,7 @@ module ts {
  function emitMemberFunctionsForES6AndHigher(node: ClassDeclaration) {
  for (let member of node.members) {
  if ((member.kind === SyntaxKind.MethodDeclaration || node.kind === SyntaxKind.MethodSignature) && !(<MethodDeclaration>member).body) {
- emitPinnedOrTripleSlashComments(member);
+ emitOnlyPinnedOrTripleSlashComments(member);
  }
  else if (member.kind === SyntaxKind.MethodDeclaration || node.kind === SyntaxKind.MethodSignature || member.kind === SyntaxKind.GetAccessor || member.kind === SyntaxKind.SetAccessor) {
  writeLine();
@@ -4786,7 +4758,7 @@ module ts {
  // Emit the constructor overload pinned comments
  forEach(node.members, member => {
  if (member.kind === SyntaxKind.Constructor && !(<ConstructorDeclaration>member).body) {
- emitPinnedOrTripleSlashComments(member);
+ emitOnlyPinnedOrTripleSlashComments(member);
  }
  // Check if there is any non-static property assignment
  if (member.kind === SyntaxKind.PropertyDeclaration && (<PropertyDeclaration>member).initializer && (member.flags & NodeFlags.Static) === 0) {
@@ -5001,7 +4973,7 @@ module ts {
  }
 
  function emitInterfaceDeclaration(node: InterfaceDeclaration) {
- emitPinnedOrTripleSlashComments(node);
+ emitOnlyPinnedOrTripleSlashComments(node);
  }
 
  function shouldEmitEnumDeclaration(node: EnumDeclaration) {
@@ -5109,7 +5081,7 @@ module ts {
  let shouldEmit = shouldEmitModuleDeclaration(node);
 
  if (!shouldEmit) {
- return emitPinnedOrTripleSlashComments(node);
+ return emitOnlyPinnedOrTripleSlashComments(node);
  }
 
  emitStart(node);
@@ -5703,13 +5675,13 @@ module ts {
  emitLeadingComments(node.endOfFileToken);
  }
 
- function emitNodeWithoutSourceMapWithComments(node: Node, allowGeneratedIdentifiers?: boolean): void {
+ function emitNodeWithoutSourceMap(node: Node, allowGeneratedIdentifiers?: boolean): void {
  if (!node) {
  return;
  }
 
  if (node.flags & NodeFlags.Ambient) {
- return emitPinnedOrTripleSlashComments(node);
+ return emitOnlyPinnedOrTripleSlashComments(node);
  }
 
  let emitComments = shouldEmitLeadingAndTrailingComments(node);
@@ -5724,18 +5696,6 @@ module ts {
  }
  }
 
- function emitNodeWithoutSourceMapWithoutComments(node: Node, allowGeneratedIdentifiers?: boolean): void {
- if (!node) {
- return;
- }
-
- if (node.flags & NodeFlags.Ambient) {
- return emitPinnedOrTripleSlashComments(node);
- }
-
- emitJavaScriptWorker(node, allowGeneratedIdentifiers);
- }
-
  function shouldEmitLeadingAndTrailingComments(node: Node) {
  switch (node.kind) {
  // All of these entities are emitted in a specialized fashion. As such, we allow
@@ -5759,6 +5719,19 @@ module ts {
  return shouldEmitEnumDeclaration(<EnumDeclaration>node);
  }
 
+ // If this is the expression body of an arrow function that we're downleveling, 
+ // then we don't want to emit comments when we emit the body. It will have already
+ // been taken care of when we emitted the 'return' statement for the function
+ // expression body.
+ if (node.kind !== SyntaxKind.Block &&
+ node.parent &&
+ node.parent.kind === SyntaxKind.ArrowFunction &&
+ (<ArrowFunction>node.parent).body === node && 
+ compilerOptions.target <= ScriptTarget.ES5) {
+
+ return false;
+ }
+
  // Emit comments for everything else.
  return true;
  }
@@ -5928,7 +5901,8 @@ module ts {
 
  function getLeadingCommentsWithoutDetachedComments() {
  // get the leading comments from detachedPos
- let leadingComments = getLeadingCommentRanges(currentSourceFile.text, detachedCommentsInfo[detachedCommentsInfo.length - 1].detachedCommentEndPos);
+ let leadingComments = getLeadingCommentRanges(currentSourceFile.text,
+ detachedCommentsInfo[detachedCommentsInfo.length - 1].detachedCommentEndPos);
  if (detachedCommentsInfo.length - 1) {
  detachedCommentsInfo.pop();
  }
@@ -5952,30 +5926,59 @@ module ts {
  // get the leading comments from the node
  leadingComments = getLeadingCommentRangesOfNode(node, currentSourceFile);
  }
+
  return leadingComments;
  }
  }
  }
 
- function emitLeadingDeclarationComments(node: Node) {
- let leadingComments = getLeadingCommentsToEmit(node);
+ function filterComments(ranges: CommentRange[], onlyPinnedOrTripleSlashComments: boolean): CommentRange[]{
+ // If we're removing comments, then we want to strip out all but the pinned or
+ // triple slash comments.
+ if (ranges && onlyPinnedOrTripleSlashComments) {
+ ranges = filter(ranges, isPinnedOrTripleSlashComment);
+ if (ranges.length === 0) {
+ return undefined;
+ }
+ }
+
+ return ranges;
+ }
+
+ function emitOnlyPinnedOrTripleSlashComments(node: Node) {
+ emitLeadingCommentsWorker(node, /*onlyPinnedOrTripleSlashComments:*/ true);
+ }
+
+ function emitLeadingComments(node: Node) {
+ return emitLeadingCommentsWorker(node, /*onlyPinnedOrTripleSlashComments:*/ compilerOptions.removeComments);
+ }
+
+ function emitLeadingCommentsWorker(node: Node, onlyPinnedOrTripleSlashComments: boolean) {
+ // If the caller only wants pinned or triple slash comments, then always filter
+ // down to that set. Otherwise, filter based on the current compiler options.
+ let leadingComments = filterComments(getLeadingCommentsToEmit(node), onlyPinnedOrTripleSlashComments);
+
  emitNewLineBeforeLeadingComments(currentSourceFile, writer, node, leadingComments);
+
  // Leading comments are emitted at /*leading comment1 */space/*leading comment*/space
  emitComments(currentSourceFile, writer, leadingComments, /*trailingSeparator*/ true, newLine, writeComment);
  }
 
- function emitTrailingDeclarationComments(node: Node) {
+ function emitTrailingComments(node: Node) {
  // Emit the trailing comments only if the parent's end doesn't match
  if (node.parent) {
  if (node.parent.kind === SyntaxKind.SourceFile || node.end !== node.parent.end) {
- let trailingComments = getTrailingCommentRanges(currentSourceFile.text, node.end);
+ let trailingComments = filterComments(
+ getTrailingCommentRanges(currentSourceFile.text, node.end),
+ /*emitOnlyPinnedOrTripleSlashComments:*/ compilerOptions.removeComments);
+
  // trailing comments are emitted at space/*trailing comment1 */space/*trailing comment*/
  emitComments(currentSourceFile, writer, trailingComments, /*trailingSeparator*/ false, newLine, writeComment);
  }
  }
  }
 
- function emitLeadingCommentsOfLocalPosition(pos: number) {
+ function emitLeadingCommentsOfPosition(pos: number) {
  let leadingComments: CommentRange[];
  if (hasDetachedComments(pos)) {
  // get comments without detached comments
@@ -5985,12 +5988,15 @@ module ts {
  // get the leading comments from the node
  leadingComments = getLeadingCommentRanges(currentSourceFile.text, pos);
  }
+
+ leadingComments = filterComments(leadingComments, compilerOptions.removeComments);
  emitNewLineBeforeLeadingComments(currentSourceFile, writer, { pos: pos, end: pos }, leadingComments);
+
  // Leading comments are emitted at /*leading comment1 */space/*leading comment*/space
  emitComments(currentSourceFile, writer, leadingComments, /*trailingSeparator*/ true, newLine, writeComment);
  }
 
- function emitDetachedCommentsAtPosition(node: TextRange) {
+ function emitDetachedComments(node: TextRange) {
  let leadingComments = getLeadingCommentRanges(currentSourceFile.text, node.pos);
  if (leadingComments) {
  let detachedComments: CommentRange[] = [];
@@ -6035,27 +6041,18 @@ module ts {
  }
  }
 
- /** Emits /// or pinned which is comment starting with /*! comments */
- function emitPinnedOrTripleSlashComments(node: Node) {
- let pinnedComments = ts.filter(getLeadingCommentsToEmit(node), isPinnedOrTripleSlashComment);
-
- function isPinnedOrTripleSlashComment(comment: CommentRange) {
- if (currentSourceFile.text.charCodeAt(comment.pos + 1) === CharacterCodes.asterisk) {
- return currentSourceFile.text.charCodeAt(comment.pos + 2) === CharacterCodes.exclamation;
- }
- // Verify this is /// comment, but do the regexp match only when we first can find /// in the comment text
- // so that we don't end up computing comment string and doing match for all // comments
- else if (currentSourceFile.text.charCodeAt(comment.pos + 1) === CharacterCodes.slash &&
- comment.pos + 2 < comment.end &&
- currentSourceFile.text.charCodeAt(comment.pos + 2) === CharacterCodes.slash &&
- currentSourceFile.text.substring(comment.pos, comment.end).match(fullTripleSlashReferencePathRegEx)) {
- return true;
- }
+ function isPinnedOrTripleSlashComment(comment: CommentRange) {
+ if (currentSourceFile.text.charCodeAt(comment.pos + 1) === CharacterCodes.asterisk) {
+ return currentSourceFile.text.charCodeAt(comment.pos + 2) === CharacterCodes.exclamation;
+ }
+ // Verify this is /// comment, but do the regexp match only when we first can find /// in the comment text
+ // so that we don't end up computing comment string and doing match for all // comments
+ else if (currentSourceFile.text.charCodeAt(comment.pos + 1) === CharacterCodes.slash &&
+ comment.pos + 2 < comment.end &&
+ currentSourceFile.text.charCodeAt(comment.pos + 2) === CharacterCodes.slash &&
+ currentSourceFile.text.substring(comment.pos, comment.end).match(fullTripleSlashReferencePathRegEx)) {
+ return true;
  }
-
- emitNewLineBeforeLeadingComments(currentSourceFile, writer, node, pinnedComments);
- // Leading comments are emitted at /*leading comment1 */space/*leading comment*/space
- emitComments(currentSourceFile, writer, pinnedComments, /*trailingSeparator*/ true, newLine, writeComment);
  }
  }
 

src/compiler/scanner.ts

@@ -455,19 +455,23 @@ module ts {
  return pos;
  }
 
- // Extract comments from the given source text starting at the given position. If trailing is false, whitespace is skipped until
- // the first line break and comments between that location and the next token are returned. If trailing is true, comments occurring
- // between the given position and the next line break are returned. The return value is an array containing a TextRange for each
- // comment. Single-line comment ranges include the beginning '//' characters but not the ending line break. Multi-line comment
- // ranges include the beginning '/* and ending '*/' characters. The return value is undefined if no comments were found.
+ // Extract comments from the given source text starting at the given position. If trailing is 
+ // false, whitespace is skipped until the first line break and comments between that location 
+ // and the next token are returned.If trailing is true, comments occurring between the given 
+ // position and the next line break are returned.The return value is an array containing a 
+ // TextRange for each comment. Single-line comment ranges include the beginning '//' characters 
+ // but not the ending line break. Multi - line comment ranges include the beginning '/* and 
+ // ending '*/' characters.The return value is undefined if no comments were found.
  function getCommentRanges(text: string, pos: number, trailing: boolean): CommentRange[] {
  let result: CommentRange[];
  let collecting = trailing || pos === 0;
  while (true) {
  let ch = text.charCodeAt(pos);
  switch (ch) {
  case CharacterCodes.carriageReturn:
- if (text.charCodeAt(pos + 1) === CharacterCodes.lineFeed) pos++;
+ if (text.charCodeAt(pos + 1) === CharacterCodes.lineFeed) {
+ pos++;
+ }
  case CharacterCodes.lineFeed:
  pos++;
  if (trailing) {
@@ -509,7 +513,10 @@ module ts {
  }
  }
  if (collecting) {
- if (!result) result = [];
+ if (!result) {
+ result = [];
+ }
+
  result.push({ pos: startPos, end: pos, hasTrailingNewLine: hasTrailingNewLine });
  }
  continue;