Skip to content

Commit

Permalink
Wrap inline nonJSDoc comments' parsing under the same option as regul…
Browse files Browse the repository at this point in the history 
…ar nonJSDoc parsing

Preserve inlne nonJSDoc comments only when other nonJSDoc comments are also getting
preserved. This is controlled by JsDocParsing.INCLUDE_ALL_COMMENTS option.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=261206789
  • Loading branch information
@rishipal @tjgq
rishipal authored and tjgq committed Aug 3, 2019
1 parent 33d7274 commit 79a0944
Show file tree
Hide file tree
Showing 2 changed files with 85 additions and 16 deletions.
43 changes: 27 additions & 16 deletions src/com/google/javascript/jscomp/parsing/IRFactory.java
View file
Expand Up @@ -801,16 +801,21 @@ private static ParseTree findNearestNode(ParseTree tree) {

Node transform(ParseTree tree) {
JSDocInfo info = handleJsDoc(tree);
ArrayList<Comment> associatedComments = getNonJSDocComments(tree);
NonJSDocComment associatedNonJSDocComment = null;
if (config.jsDocParsingMode() == JsDocParsing.INCLUDE_ALL_COMMENTS) {
ArrayList<Comment> nonJSDocComments = getNonJSDocComments(tree);
if (!nonJSDocComments.isEmpty()) {
associatedNonJSDocComment = combineCommentsIntoSingleComment(nonJSDocComments);
}
}
Node node = transformDispatcher.process(tree);
if (info != null) {
node = maybeInjectCastNode(tree, info, node);
node.setJSDocInfo(info);
}
if (this.config.jsDocParsingMode() == JsDocParsing.INCLUDE_ALL_COMMENTS) {
if (!associatedComments.isEmpty()) {
NonJSDocComment completeComment = combineCommentsIntoSingleComment(associatedComments);
node.setNonJSDocComment(completeComment);
if (associatedNonJSDocComment != null) {
node.setNonJSDocComment(associatedNonJSDocComment);
}
}

Expand Down Expand Up @@ -838,21 +843,23 @@ private Node maybeInjectCastNode(ParseTree node, JSDocInfo info, Node irNode) {
*/
Node transformNodeWithInlineComments(ParseTree tree) {
JSDocInfo info = handleInlineJsDoc(tree);
ArrayList<Comment> nonJSDocComments = getNonJSDocComments(tree);
NonJSDocComment associatedNonJSDocComment = null;
if (!nonJSDocComments.isEmpty()) {
associatedNonJSDocComment = combineCommentsIntoSingleComment(nonJSDocComments);
if (config.jsDocParsingMode() == JsDocParsing.INCLUDE_ALL_COMMENTS) {
ArrayList<Comment> nonJSDocComments = getNonJSDocComments(tree);
if (!nonJSDocComments.isEmpty()) {
associatedNonJSDocComment = combineCommentsIntoSingleComment(nonJSDocComments);
}
}
Node node = transformDispatcher.process(tree);

if (info != null) {
node.setJSDocInfo(info);
}
if (associatedNonJSDocComment != null) {
node.setNonJSDocComment(associatedNonJSDocComment);
if (this.config.jsDocParsingMode() == JsDocParsing.INCLUDE_ALL_COMMENTS) {
if (associatedNonJSDocComment != null) {
node.setNonJSDocComment(associatedNonJSDocComment);
}
}
setSourceInfo(node, tree);

return node;
}

Expand Down Expand Up @@ -1798,18 +1805,22 @@ private Node processNameWithInlineComments(IdentifierExpressionTree identifierEx

Node processNameWithInlineComments(IdentifierToken identifierToken) {
JSDocInfo info = handleInlineJsDoc(identifierToken);
ArrayList<Comment> nonJSDocComments = getNonJSDocComments(identifierToken);
NonJSDocComment associatedNonJSDocComment = null;
if (!nonJSDocComments.isEmpty()) {
associatedNonJSDocComment = combineCommentsIntoSingleComment(nonJSDocComments);
if (config.jsDocParsingMode() == JsDocParsing.INCLUDE_ALL_COMMENTS) {
ArrayList<Comment> nonJSDocComments = getNonJSDocComments(identifierToken);
if (!nonJSDocComments.isEmpty()) {
associatedNonJSDocComment = combineCommentsIntoSingleComment(nonJSDocComments);
}
}
maybeWarnReservedKeyword(identifierToken);
Node node = newStringNode(Token.NAME, identifierToken.value);
if (info != null) {
node.setJSDocInfo(info);
}
if (associatedNonJSDocComment != null) {
node.setNonJSDocComment(associatedNonJSDocComment);
if (config.jsDocParsingMode() == JsDocParsing.INCLUDE_ALL_COMMENTS) {
if (associatedNonJSDocComment != null) {
node.setNonJSDocComment(associatedNonJSDocComment);
}
}
setSourceInfo(node, identifierToken);
return node;
Expand Down
58 changes: 58 additions & 0 deletions test/com/google/javascript/jscomp/parsing/ParserTest.java
View file
Expand Up @@ -1089,6 +1089,8 @@ public void testInlineJSDocAttachmentToVar() {

@Test
public void testInlineNonJSDocCommentAttachmentToVar() {
isIdeMode = true;
parsingMode = JsDocParsing.INCLUDE_ALL_COMMENTS;
Node letNode = parse("let /* blah */ x = 'a';").getFirstChild();
assertNode(letNode).hasType(Token.LET);

Expand All @@ -1097,6 +1099,15 @@ public void testInlineNonJSDocCommentAttachmentToVar() {
assertThat(nonJSDocComment.getCommentString()).contains("/* blah */");
}

@Test
public void testNoInlineNonJSDocCommentAttachmentWithoutParsingMode() {
Node letNode = parse("let /* blah */ x = 'a';").getFirstChild();
assertNode(letNode).hasType(Token.LET);

NonJSDocComment nonJSDocComment = letNode.getFirstChild().getNonJSDocComment();
assertThat(nonJSDocComment).isNull();
}

@Test
public void testInlineJSDocAttachmentToObjPatNormalProp() {
Node letNode =
Expand All @@ -1114,6 +1125,8 @@ public void testInlineJSDocAttachmentToObjPatNormalProp() {

@Test
public void testInlineNonJSDocCommentAttachmentToObjPatNormalProp() {
isIdeMode = true;
parsingMode = JsDocParsing.INCLUDE_ALL_COMMENTS;
Node letNode = parse("let { normalProp: /* blah */ normalPropTarget } = {};").getFirstChild();
assertNode(letNode).hasType(Token.LET);

Expand All @@ -1126,6 +1139,20 @@ public void testInlineNonJSDocCommentAttachmentToObjPatNormalProp() {
assertThat(normalPropTarget.getNonJSDocCommentString()).contains("/* blah */");
}

@Test
public void testNoInlineNonJSDocCommentAttachmentToObjWithoutParsingMode() {
Node letNode = parse("let { normalProp: /* blah */ normalPropTarget } = {};").getFirstChild();
assertNode(letNode).hasType(Token.LET);

Node destructuringLhs = letNode.getFirstChild();
Node objectPattern = destructuringLhs.getFirstChild();

Node normalProp = objectPattern.getFirstChild();
assertNode(normalProp).hasType(Token.STRING_KEY);
Node normalPropTarget = normalProp.getOnlyChild();
assertThat(normalPropTarget.getNonJSDocComment()).isNull();
}

@Test
public void testInlineJSDocAttachmentToObjPatNormalPropKey() {
Node letNode = parse("let { /** string */ normalProp: normalProp } = {};").getFirstChild();
Expand All @@ -1143,6 +1170,8 @@ public void testInlineJSDocAttachmentToObjPatNormalPropKey() {

@Test
public void testInlineNonJSDocCommentAttachmentToObjPatNormalPropKey() {
isIdeMode = true;
parsingMode = JsDocParsing.INCLUDE_ALL_COMMENTS;
Node letNode = parse("let { /* blah */ normalProp: normalProp } = {};").getFirstChild();
assertNode(letNode).hasType(Token.LET);

Expand Down Expand Up @@ -1173,6 +1202,8 @@ public void testInlineJSDocAttachmentToObjPatShorthandProp() {

@Test
public void testInlineNonJSDocAttachmentToObjPatShorthandProp() {
isIdeMode = true;
parsingMode = JsDocParsing.INCLUDE_ALL_COMMENTS;
Node letNode = parse("let { /* blah */ shorthandProp } = {};").getFirstChild();
assertNode(letNode).hasType(Token.LET);

Expand Down Expand Up @@ -1205,6 +1236,8 @@ public void testInlineJSDocAttachmentToObjPatNormalPropWithDefault() {

@Test
public void testInlineNonJSDocCommentAttachmentToObjPatNormalPropWithDefault() {
isIdeMode = true;
parsingMode = JsDocParsing.INCLUDE_ALL_COMMENTS;
Node letNode =
parse("let { normalPropWithDefault: /* blah */ normalPropWithDefault = 'hi' } = {};")
.getFirstChild();
Expand Down Expand Up @@ -1240,6 +1273,8 @@ public void testInlineJSDocAttachmentToObjPatShorthandWithDefault() {

@Test
public void testInlineNonJSDocCommentAttachmentToObjPatShorthandWithDefault() {
isIdeMode = true;
parsingMode = JsDocParsing.INCLUDE_ALL_COMMENTS;
Node letNode =
parse("let { /* blah */ shorthandPropWithDefault = 'lo' } = {};").getFirstChild();
assertNode(letNode).hasType(Token.LET);
Expand Down Expand Up @@ -1497,6 +1532,8 @@ public void testInlineJSDocAttachment1() {

@Test
public void testInline_BlockCommentAttachment() {
isIdeMode = true;
parsingMode = JsDocParsing.INCLUDE_ALL_COMMENTS;
Node fn = parse("function f(/* blah */ x) {}").getFirstChild();
assertNode(fn).hasType(Token.FUNCTION);
Node xNode = fn.getSecondChild().getFirstChild();
Expand All @@ -1505,6 +1542,8 @@ public void testInline_BlockCommentAttachment() {

@Test
public void testInline_LineCommentAttachment() {
isIdeMode = true;
parsingMode = JsDocParsing.INCLUDE_ALL_COMMENTS;
Node fn = parse("function f( // blah\n x) {}").getFirstChild();
assertNode(fn).hasType(Token.FUNCTION);
Node xNode = fn.getSecondChild().getFirstChild();
Expand All @@ -1513,6 +1552,8 @@ public void testInline_LineCommentAttachment() {

@Test
public void testMultipleInline_LineCommentsAttachment() {
isIdeMode = true;
parsingMode = JsDocParsing.INCLUDE_ALL_COMMENTS;
Node fn = parse("function f( // blah1\n x, // blah2\n y) {}").getFirstChild();
assertNode(fn).hasType(Token.FUNCTION);

Expand All @@ -1525,6 +1566,8 @@ public void testMultipleInline_LineCommentsAttachment() {

@Test
public void testMultipleInline_MixedCommentsAttachment() {
isIdeMode = true;
parsingMode = JsDocParsing.INCLUDE_ALL_COMMENTS;
Node fn = parse("function f( /* blah1 */ x, // blah2\n y) {}").getFirstChild();
assertNode(fn).hasType(Token.FUNCTION);

Expand All @@ -1537,6 +1580,8 @@ public void testMultipleInline_MixedCommentsAttachment() {

@Test
public void testMultipleInline_NonJSDocCommentsGetAttachedToSameNode() {
isIdeMode = true;
parsingMode = JsDocParsing.INCLUDE_ALL_COMMENTS;
Node fn = parse("function f(/* blah1 */\n// blah\n x) {}").getFirstChild();
assertNode(fn).hasType(Token.FUNCTION);

Expand All @@ -1546,6 +1591,8 @@ public void testMultipleInline_NonJSDocCommentsGetAttachedToSameNode() {

@Test
public void testBothJSDocAndNonJSDocCommentsGetAttached() {
isIdeMode = true;
parsingMode = JsDocParsing.INCLUDE_ALL_COMMENTS;
Node fn = parse("function f(/** string */ // nonJSDoc\n x) {}").getFirstChild();
assertNode(fn).hasType(Token.FUNCTION);

Expand All @@ -1558,6 +1605,8 @@ public void testBothJSDocAndNonJSDocCommentsGetAttached() {

@Test
public void testBothNonJSDocAndJSDocCommentsGetAttached() {
isIdeMode = true;
parsingMode = JsDocParsing.INCLUDE_ALL_COMMENTS;
Node fn = parse("function f(// nonJSDoc\n /** string */ x) {}").getFirstChild();
assertNode(fn).hasType(Token.FUNCTION);

Expand Down Expand Up @@ -2844,6 +2893,13 @@ public void testBlockCommentParsed() {
assertThat(n.getFirstChild().getNonJSDocCommentString()).isEqualTo("/* Hi mom! */");
}

@Test
public void testBlockCommentNotParsedWithoutParsingMode() {
Node n = parse("/* Hi mom! */ \n function Foo() {}");
assertNode(n.getFirstChild()).hasType(Token.FUNCTION);
assertThat(n.getFirstChild().getNonJSDocComment()).isNull();
}

@Test
public void testLineCommentParsed() {
isIdeMode = true;
Expand Down Expand Up @@ -4215,6 +4271,8 @@ public void testDefaultParameterInlineJSDoc() {

@Test
public void testDefaultParameterInlineNonJSDocComment() {
isIdeMode = true;
parsingMode = JsDocParsing.INCLUDE_ALL_COMMENTS;
expectFeatures(Feature.DEFAULT_PARAMETERS);
Node functionNode = parse("function f(/* number */ a = 0) {}").getFirstChild();
Node parameterList = functionNode.getSecondChild();
Expand Down

0 comments on commit 79a0944

Please sign in to comment.