From 36b99511c61143ccfc7daef902c2992694e1f248 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 17 Mar 2015 18:34:42 -0700 Subject: [PATCH 1/3] Simplify code for emitting comments. Also, always emit pinned comments, even when the 'removeComments' compiler option is provided. --- src/compiler/emitter.ts | 146 ++++++++++++++++++-------------------- src/compiler/scanner.ts | 21 ++++-- src/compiler/utilities.ts | 14 ++-- 3 files changed, 91 insertions(+), 90 deletions(-) diff --git a/src/compiler/emitter.ts b/src/compiler/emitter.ts index 205f2fe2945..9f71e8322b5 100644 --- a/src/compiler/emitter.ts +++ b/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 (!(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) && !(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 && !(member).body) { - emitPinnedOrTripleSlashComments(member); + emitOnlyPinnedOrTripleSlashComments(member); } // Check if there is any non-static property assignment if (member.kind === SyntaxKind.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,17 @@ module ts { return shouldEmitEnumDeclaration(node); } + // If this is the expression body of an arrow function, then we don't want to emit + // comments when we emit the body. It will have been already 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 && + (node.parent).body === node) { + + return false; + } + // Emit comments for everything else. return true; } @@ -5928,7 +5899,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 +5924,58 @@ module ts { // get the leading comments from the node leadingComments = getLeadingCommentRangesOfNode(node, currentSourceFile); } + return leadingComments; } } } - function emitLeadingDeclarationComments(node: Node) { + function filterCommentsBasedOnOptions(ranges: CommentRange[]): CommentRange[]{ + // If we're removing comments, then we want to strip out all but the pinned or + // triple slash comments. + if (ranges && compilerOptions.removeComments) { + 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:*/ false); + } + + function emitLeadingCommentsWorker(node: Node, onlyPinnedOrTripleSlashComments: boolean) { let leadingComments = getLeadingCommentsToEmit(node); + leadingComments = onlyPinnedOrTripleSlashComments + ? filter(leadingComments, isPinnedOrTripleSlashComment) + : filterCommentsBasedOnOptions(leadingComments); + 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 = filterCommentsBasedOnOptions(getTrailingCommentRanges(currentSourceFile.text, node.end)); + // 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 +5985,15 @@ module ts { // get the leading comments from the node leadingComments = getLeadingCommentRanges(currentSourceFile.text, pos); } + + leadingComments = filterCommentsBasedOnOptions(leadingComments); 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 +6038,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); } } diff --git a/src/compiler/scanner.ts b/src/compiler/scanner.ts index fbcadab8c1d..130fcc07a22 100644 --- a/src/compiler/scanner.ts +++ b/src/compiler/scanner.ts @@ -455,11 +455,13 @@ 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; @@ -467,7 +469,9 @@ module ts { 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; diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index 4fce9c928f7..fd7e1304c85 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -361,16 +361,16 @@ module ts { return node.kind === SyntaxKind.ExpressionStatement && (node).expression.kind === SyntaxKind.StringLiteral; } - export function getLeadingCommentRangesOfNode(node: Node, sourceFileOfNode?: SourceFile) { - sourceFileOfNode = sourceFileOfNode || getSourceFileOfNode(node); - + export function getLeadingCommentRangesOfNode(node: Node, sourceFileOfNode: SourceFile) { // If parameter/type parameter, the prev token trailing comments are part of this node too if (node.kind === SyntaxKind.Parameter || node.kind === SyntaxKind.TypeParameter) { // e.g. (/** blah */ a, /** blah */ b); - return concatenate(getTrailingCommentRanges(sourceFileOfNode.text, node.pos), - // e.g.: ( - // /** blah */ a, - // /** blah */ b); + + // e.g.: ( + // /** blah */ a, + // /** blah */ b); + return concatenate( + getTrailingCommentRanges(sourceFileOfNode.text, node.pos), getLeadingCommentRanges(sourceFileOfNode.text, node.pos)); } else { From 9582d7cf283e7c7856a4741a859bd8ac6a3fcb82 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 17 Mar 2015 18:43:39 -0700 Subject: [PATCH 2/3] Add test for pinned comments. --- src/compiler/emitter.ts | 3 +++ tests/baselines/reference/pinnedComments1.js | 14 ++++++++++++++ tests/baselines/reference/pinnedComments1.types | 7 +++++++ tests/cases/compiler/pinnedComments1.ts | 6 ++++++ 4 files changed, 30 insertions(+) create mode 100644 tests/baselines/reference/pinnedComments1.js create mode 100644 tests/baselines/reference/pinnedComments1.types create mode 100644 tests/cases/compiler/pinnedComments1.ts diff --git a/src/compiler/emitter.ts b/src/compiler/emitter.ts index 9f71e8322b5..2732d79f25d 100644 --- a/src/compiler/emitter.ts +++ b/src/compiler/emitter.ts @@ -5953,6 +5953,9 @@ module ts { function emitLeadingCommentsWorker(node: Node, onlyPinnedOrTripleSlashComments: boolean) { let leadingComments = getLeadingCommentsToEmit(node); + + // 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. leadingComments = onlyPinnedOrTripleSlashComments ? filter(leadingComments, isPinnedOrTripleSlashComment) : filterCommentsBasedOnOptions(leadingComments); diff --git a/tests/baselines/reference/pinnedComments1.js b/tests/baselines/reference/pinnedComments1.js new file mode 100644 index 00000000000..c4b8b41fbd7 --- /dev/null +++ b/tests/baselines/reference/pinnedComments1.js @@ -0,0 +1,14 @@ +//// [pinnedComments1.ts] + +/* unpinned comment */ +/*! pinned comment */ +class C { +} + +//// [pinnedComments1.js] +/*! pinned comment */ +var C = (function () { + function C() { + } + return C; +})(); diff --git a/tests/baselines/reference/pinnedComments1.types b/tests/baselines/reference/pinnedComments1.types new file mode 100644 index 00000000000..d55feb67659 --- /dev/null +++ b/tests/baselines/reference/pinnedComments1.types @@ -0,0 +1,7 @@ +=== tests/cases/compiler/pinnedComments1.ts === + +/* unpinned comment */ +/*! pinned comment */ +class C { +>C : C +} diff --git a/tests/cases/compiler/pinnedComments1.ts b/tests/cases/compiler/pinnedComments1.ts new file mode 100644 index 00000000000..474769e9b20 --- /dev/null +++ b/tests/cases/compiler/pinnedComments1.ts @@ -0,0 +1,6 @@ +// @comments: false + +/* unpinned comment */ +/*! pinned comment */ +class C { +} \ No newline at end of file From 63e4420887ca5a674030dfadff5692ba2e72b976 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 17 Mar 2015 19:13:00 -0700 Subject: [PATCH 3/3] Simplify flow control. --- src/compiler/emitter.ts | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/src/compiler/emitter.ts b/src/compiler/emitter.ts index 2732d79f25d..6022b1445bd 100644 --- a/src/compiler/emitter.ts +++ b/src/compiler/emitter.ts @@ -5719,13 +5719,15 @@ module ts { return shouldEmitEnumDeclaration(node); } - // If this is the expression body of an arrow function, then we don't want to emit - // comments when we emit the body. It will have been already taken care of when - // we emitted the 'return' statement for the function expression body. + // 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 && - (node.parent).body === node) { + (node.parent).body === node && + compilerOptions.target <= ScriptTarget.ES5) { return false; } @@ -5930,10 +5932,10 @@ module ts { } } - function filterCommentsBasedOnOptions(ranges: CommentRange[]): CommentRange[]{ + 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 && compilerOptions.removeComments) { + if (ranges && onlyPinnedOrTripleSlashComments) { ranges = filter(ranges, isPinnedOrTripleSlashComment); if (ranges.length === 0) { return undefined; @@ -5948,17 +5950,13 @@ module ts { } function emitLeadingComments(node: Node) { - return emitLeadingCommentsWorker(node, /*onlyPinnedOrTripleSlashComments:*/ false); + return emitLeadingCommentsWorker(node, /*onlyPinnedOrTripleSlashComments:*/ compilerOptions.removeComments); } function emitLeadingCommentsWorker(node: Node, onlyPinnedOrTripleSlashComments: boolean) { - let leadingComments = getLeadingCommentsToEmit(node); - // 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. - leadingComments = onlyPinnedOrTripleSlashComments - ? filter(leadingComments, isPinnedOrTripleSlashComment) - : filterCommentsBasedOnOptions(leadingComments); + let leadingComments = filterComments(getLeadingCommentsToEmit(node), onlyPinnedOrTripleSlashComments); emitNewLineBeforeLeadingComments(currentSourceFile, writer, node, leadingComments); @@ -5970,7 +5968,9 @@ module ts { // 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 = filterCommentsBasedOnOptions(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); @@ -5989,7 +5989,7 @@ module ts { leadingComments = getLeadingCommentRanges(currentSourceFile.text, pos); } - leadingComments = filterCommentsBasedOnOptions(leadingComments); + leadingComments = filterComments(leadingComments, compilerOptions.removeComments); emitNewLineBeforeLeadingComments(currentSourceFile, writer, { pos: pos, end: pos }, leadingComments); // Leading comments are emitted at /*leading comment1 */space/*leading comment*/space