diff --git a/src/com/google/javascript/jscomp/parsing/IRFactory.java b/src/com/google/javascript/jscomp/parsing/IRFactory.java index 579c268239c..71be4248d00 100644 --- a/src/com/google/javascript/jscomp/parsing/IRFactory.java +++ b/src/com/google/javascript/jscomp/parsing/IRFactory.java @@ -751,7 +751,7 @@ private boolean hasPendingNonJSDocCommentBefore(SourceRange location) { && currentNonJSDocComment.location.end.offset <= location.start.offset; } - private ArrayList getNonJsDocComments(SourceRange location) { + private ArrayList getNonJSDocComments(SourceRange location) { ArrayList previousComments = new ArrayList<>(); while (hasPendingNonJSDocCommentBefore(location)) { previousComments.add(currentNonJSDocComment); @@ -760,12 +760,13 @@ private ArrayList getNonJsDocComments(SourceRange location) { return previousComments; } - private ArrayList getNonJsDocComment(ParseTree tree) { - return getNonJsDocComments(tree.location); + private ArrayList getNonJSDocComments( + com.google.javascript.jscomp.parsing.parser.Token token) { + return getNonJSDocComments(token.location); } - private ArrayList handleNonJSDocComment(ParseTree node) { - return getNonJsDocComment(node); + private ArrayList getNonJSDocComments(ParseTree tree) { + return getNonJSDocComments(tree.location); } private static ParseTree findNearestNode(ParseTree tree) { @@ -800,7 +801,7 @@ private static ParseTree findNearestNode(ParseTree tree) { Node transform(ParseTree tree) { JSDocInfo info = handleJsDoc(tree); - ArrayList associatedComments = handleNonJSDocComment(tree); + ArrayList associatedComments = getNonJSDocComments(tree); Node node = transformDispatcher.process(tree); if (info != null) { node = maybeInjectCastNode(tree, info, node); @@ -835,13 +836,21 @@ private Node maybeInjectCastNode(ParseTree node, JSDocInfo info, Node irNode) { * @see Using Inline Doc * Comments */ - Node transformNodeWithInlineJsDoc(ParseTree tree) { + Node transformNodeWithInlineComments(ParseTree tree) { JSDocInfo info = handleInlineJsDoc(tree); + ArrayList nonJSDocComments = getNonJSDocComments(tree); + NonJSDocComment associatedNonJSDocComment = null; + if (!nonJSDocComments.isEmpty()) { + associatedNonJSDocComment = combineCommentsIntoSingleComment(nonJSDocComments); + } Node node = transformDispatcher.process(tree); if (info != null) { node.setJSDocInfo(info); } + if (associatedNonJSDocComment != null) { + node.setNonJSDocComment(associatedNonJSDocComment); + } setSourceInfo(node, tree); return node; @@ -1111,10 +1120,10 @@ Node processArrayPattern(ArrayPatternTree tree) { break; case ITER_REST: maybeWarnForFeature(child, Feature.ARRAY_PATTERN_REST); - elementNode = transformNodeWithInlineJsDoc(child); + elementNode = transformNodeWithInlineComments(child); break; default: - elementNode = transformNodeWithInlineJsDoc(child); + elementNode = transformNodeWithInlineComments(child); break; } node.addChildToBack(elementNode); @@ -1149,7 +1158,7 @@ private Node processObjectPatternElement(ParseTree child) { case OBJECT_REST: // let {...restObject} = someObject; maybeWarnForFeature(child, Feature.OBJECT_PATTERN_REST); - Node target = transformNodeWithInlineJsDoc(child.asObjectRest().assignmentTarget); + Node target = transformNodeWithInlineComments(child.asObjectRest().assignmentTarget); Node rest = newNode(Token.OBJECT_REST, target); setSourceInfo(rest, child); return rest; @@ -1199,7 +1208,7 @@ private Node processObjectPatternPropertyNameAssignment( if (targetTree == null) { // `let { /** inlineType */ key } = something;` // The key is also the target name. - valueNode = processNameWithInlineJSDoc(propertyNameAssignment.name.asIdentifier()); + valueNode = processNameWithInlineComments(propertyNameAssignment.name.asIdentifier()); key.setShorthandProperty(true); } else { valueNode = processDestructuringElementTarget(targetTree); @@ -1219,12 +1228,12 @@ private Node processDestructuringElementTarget(ParseTree targetTree) { // let {key: /** inlineType */ name} = something // let [/** inlineType */ name] = something // Allow inline JSDoc on the name, since we may well be declaring it here. - valueNode = processNameWithInlineJSDoc(targetTree.asIdentifierExpression()); + valueNode = processNameWithInlineComments(targetTree.asIdentifierExpression()); } else { // ({prop: /** string */ ns.a.b} = someObject); // NOTE: CheckJSDoc will report an error for this case, since we want qualified names to be // declared with individual statements, like `/** @type {string} */ ns.a.b;` - valueNode = transformNodeWithInlineJsDoc(targetTree); + valueNode = transformNodeWithInlineComments(targetTree); } return valueNode; } @@ -1491,7 +1500,7 @@ Node processFunction(FunctionDeclarationTree functionTree) { IdentifierToken name = functionTree.name; Node newName; if (name != null) { - newName = processNameWithInlineJSDoc(name); + newName = processNameWithInlineComments(name); } else { if (isDeclaration || isMember) { errorReporter.error( @@ -1569,10 +1578,10 @@ Node processFormalParameterList(FormalParameterListTree tree) { break; case ITER_REST: maybeWarnForFeature(param, Feature.REST_PARAMETERS); - paramNode = transformNodeWithInlineJsDoc(param); + paramNode = transformNodeWithInlineComments(param); break; default: - paramNode = transformNodeWithInlineJsDoc(param); + paramNode = transformNodeWithInlineComments(param); break; } @@ -1598,12 +1607,12 @@ Node processDefaultParameter(DefaultParameterTree tree) { // allow inline JSDoc on an identifier // let { /** inlineType */ x = defaultValue } = someObject; // TODO(bradfordcsmith): Do we need to allow inline JSDoc for qualified names, too? - targetNode = processNameWithInlineJSDoc(targetTree.asIdentifierExpression()); + targetNode = processNameWithInlineComments(targetTree.asIdentifierExpression()); } else { // ({prop: /** string */ ns.a.b = 'foo'} = someObject); // NOTE: CheckJSDoc will report an error for this case, since we want qualified names to be // declared with individual statements, like `/** @type {string} */ ns.a.b;` - targetNode = transformNodeWithInlineJsDoc(targetTree); + targetNode = transformNodeWithInlineComments(targetTree); } Node defaultValueNode = newNode(Token.DEFAULT_VALUE, targetNode, transform(tree.defaultValue)); @@ -1612,7 +1621,7 @@ Node processDefaultParameter(DefaultParameterTree tree) { } Node processIterRest(IterRestTree tree) { - Node target = transformNodeWithInlineJsDoc(tree.assignmentTarget); + Node target = transformNodeWithInlineComments(tree.assignmentTarget); return newNode(Token.ITER_REST, target); } @@ -1783,17 +1792,25 @@ Node processTemplateLiteralToken(TemplateLiteralToken token) { return node; } - private Node processNameWithInlineJSDoc(IdentifierExpressionTree identifierExpression) { - return processNameWithInlineJSDoc(identifierExpression.identifierToken); + private Node processNameWithInlineComments(IdentifierExpressionTree identifierExpression) { + return processNameWithInlineComments(identifierExpression.identifierToken); } - Node processNameWithInlineJSDoc(IdentifierToken identifierToken) { + Node processNameWithInlineComments(IdentifierToken identifierToken) { JSDocInfo info = handleInlineJsDoc(identifierToken); + ArrayList nonJSDocComments = getNonJSDocComments(identifierToken); + NonJSDocComment associatedNonJSDocComment = null; + 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); + } setSourceInfo(node, identifierToken); return node; } @@ -2289,13 +2306,13 @@ Node processVariableDeclarationList(VariableDeclarationListTree decl) { Node node = newNode(declType); for (VariableDeclarationTree child : decl.declarations) { - node.addChildToBack(transformNodeWithInlineJsDoc(child)); + node.addChildToBack(transformNodeWithInlineComments(child)); } return node; } Node processVariableDeclaration(VariableDeclarationTree decl) { - Node node = transformNodeWithInlineJsDoc(decl.lvalue); + Node node = transformNodeWithInlineComments(decl.lvalue); Node lhs = node.isDestructuringPattern() ? newNode(Token.DESTRUCTURING_LHS, node) : node; if (decl.initializer != null) { Node initializer = transform(decl.initializer); diff --git a/src/com/google/javascript/rhino/NonJSDocComment.java b/src/com/google/javascript/rhino/NonJSDocComment.java index 1bb4cfa6dda..aa6110fa92f 100644 --- a/src/com/google/javascript/rhino/NonJSDocComment.java +++ b/src/com/google/javascript/rhino/NonJSDocComment.java @@ -38,9 +38,10 @@ * ***** END LICENSE BLOCK ***** */ package com.google.javascript.rhino; +import java.io.Serializable; /** Minimal class holding information about a nonJSDoc comment's source location and contents */ -public class NonJSDocComment { +public class NonJSDocComment implements Serializable { private final int beginOffset; private final int endOffset; private final String contents; diff --git a/test/com/google/javascript/jscomp/parsing/ParserTest.java b/test/com/google/javascript/jscomp/parsing/ParserTest.java index 94b2975acfa..6c685ed49ba 100644 --- a/test/com/google/javascript/jscomp/parsing/ParserTest.java +++ b/test/com/google/javascript/jscomp/parsing/ParserTest.java @@ -36,6 +36,7 @@ import com.google.javascript.rhino.JSDocInfo; import com.google.javascript.rhino.JSTypeExpression; import com.google.javascript.rhino.Node; +import com.google.javascript.rhino.NonJSDocComment; import com.google.javascript.rhino.SimpleSourceFile; import com.google.javascript.rhino.StaticSourceFile; import com.google.javascript.rhino.StaticSourceFile.SourceKind; @@ -1086,6 +1087,16 @@ public void testInlineJSDocAttachmentToVar() { assertTypeEquals(STRING_TYPE, info.getType()); } + @Test + public void testInlineNonJSDocCommentAttachmentToVar() { + Node letNode = parse("let /* blah */ x = 'a';").getFirstChild(); + assertNode(letNode).hasType(Token.LET); + + NonJSDocComment nonJSDocComment = letNode.getFirstChild().getNonJSDocComment(); + assertThat(nonJSDocComment).isNotNull(); + assertThat(nonJSDocComment.getCommentString()).contains("/* blah */"); + } + @Test public void testInlineJSDocAttachmentToObjPatNormalProp() { Node letNode = @@ -1101,6 +1112,20 @@ public void testInlineJSDocAttachmentToObjPatNormalProp() { assertNodeHasJSDocInfoWithJSType(normalPropTarget, STRING_TYPE); } + @Test + public void testInlineNonJSDocCommentAttachmentToObjPatNormalProp() { + 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.getNonJSDocCommentString()).contains("/* blah */"); + } + @Test public void testInlineJSDocAttachmentToObjPatNormalPropKey() { Node letNode = parse("let { /** string */ normalProp: normalProp } = {};").getFirstChild(); @@ -1116,6 +1141,22 @@ public void testInlineJSDocAttachmentToObjPatNormalPropKey() { assertNodeHasNoJSDocInfo(normalProp); } + @Test + public void testInlineNonJSDocCommentAttachmentToObjPatNormalPropKey() { + Node letNode = parse("let { /* blah */ normalProp: normalProp } = {};").getFirstChild(); + assertNode(letNode).hasType(Token.LET); + + Node destructuringLhs = letNode.getFirstChild(); + Node objectPattern = destructuringLhs.getFirstChild(); + + Node normalProp = objectPattern.getFirstChild(); + assertNode(normalProp).hasType(Token.STRING_KEY); + // TODO(b/138749905): Unlike inline JSDoc in the corresponding test above, inline nonJSDoc + // comment on key should be allowed, i.e. it should be attached to the string key and not the + // name value + assertThat(normalProp.getOnlyChild().getNonJSDocCommentString()).contains("/* blah */"); + } + @Test public void testInlineJSDocAttachmentToObjPatShorthandProp() { Node letNode = parse("let { /** string */ shorthandProp } = {};").getFirstChild(); @@ -1130,6 +1171,20 @@ public void testInlineJSDocAttachmentToObjPatShorthandProp() { assertNodeHasJSDocInfoWithJSType(shorthandPropTarget, STRING_TYPE); } + @Test + public void testInlineNonJSDocAttachmentToObjPatShorthandProp() { + Node letNode = parse("let { /* blah */ shorthandProp } = {};").getFirstChild(); + assertNode(letNode).hasType(Token.LET); + + Node destructuringLhs = letNode.getFirstChild(); + Node objectPattern = destructuringLhs.getFirstChild(); + + Node shorthandProp = objectPattern.getFirstChild(); + assertNode(shorthandProp).hasType(Token.STRING_KEY); + Node shorthandPropTarget = shorthandProp.getOnlyChild(); + assertThat(shorthandPropTarget.getNonJSDocCommentString()).contains("/* blah */"); + } + @Test public void testInlineJSDocAttachmentToObjPatNormalPropWithDefault() { Node letNode = @@ -1148,6 +1203,24 @@ public void testInlineJSDocAttachmentToObjPatNormalPropWithDefault() { assertNodeHasJSDocInfoWithJSType(normalPropWithDefaultTarget, STRING_TYPE); } + @Test + public void testInlineNonJSDocCommentAttachmentToObjPatNormalPropWithDefault() { + Node letNode = + parse("let { normalPropWithDefault: /* blah */ normalPropWithDefault = 'hi' } = {};") + .getFirstChild(); + assertNode(letNode).hasType(Token.LET); + + Node destructuringLhs = letNode.getFirstChild(); + Node objectPattern = destructuringLhs.getFirstChild(); + + Node normalPropWithDefault = objectPattern.getFirstChild(); + assertNode(normalPropWithDefault).hasType(Token.STRING_KEY); + Node normalPropDefaultValue = normalPropWithDefault.getOnlyChild(); + assertNode(normalPropDefaultValue).hasType(Token.DEFAULT_VALUE); + Node normalPropWithDefaultTarget = normalPropDefaultValue.getFirstChild(); + assertThat(normalPropWithDefaultTarget.getNonJSDocCommentString()).contains("/* blah */"); + } + @Test public void testInlineJSDocAttachmentToObjPatShorthandWithDefault() { Node letNode = @@ -1165,6 +1238,23 @@ public void testInlineJSDocAttachmentToObjPatShorthandWithDefault() { assertNodeHasJSDocInfoWithJSType(shorthandPropWithDefaultTarget, STRING_TYPE); } + @Test + public void testInlineNonJSDocCommentAttachmentToObjPatShorthandWithDefault() { + Node letNode = + parse("let { /* blah */ shorthandPropWithDefault = 'lo' } = {};").getFirstChild(); + assertNode(letNode).hasType(Token.LET); + + Node destructuringLhs = letNode.getFirstChild(); + Node objectPattern = destructuringLhs.getFirstChild(); + + Node shorthandPropWithDefault = objectPattern.getFirstChild(); + assertNode(shorthandPropWithDefault).hasType(Token.STRING_KEY); + Node shorthandPropDefaultValue = shorthandPropWithDefault.getOnlyChild(); + assertNode(shorthandPropDefaultValue).hasType(Token.DEFAULT_VALUE); + Node shorthandPropWithDefaultTarget = shorthandPropDefaultValue.getFirstChild(); + assertThat(shorthandPropWithDefaultTarget.getNonJSDocCommentString()).contains("/* blah */"); + } + @Test public void testInlineJSDocAttachmentToObjPatComputedPropKey() { Node letNode = @@ -1405,6 +1495,79 @@ public void testInlineJSDocAttachment1() { assertTypeEquals(STRING_TYPE, info.getType()); } + @Test + public void testInline_BlockCommentAttachment() { + Node fn = parse("function f(/* blah */ x) {}").getFirstChild(); + assertNode(fn).hasType(Token.FUNCTION); + Node xNode = fn.getSecondChild().getFirstChild(); + assertThat(xNode.getNonJSDocCommentString()).contains("/* blah */"); + } + + @Test + public void testInline_LineCommentAttachment() { + Node fn = parse("function f( // blah\n x) {}").getFirstChild(); + assertNode(fn).hasType(Token.FUNCTION); + Node xNode = fn.getSecondChild().getFirstChild(); + assertThat(xNode.getNonJSDocCommentString()).contains("// blah"); + } + + @Test + public void testMultipleInline_LineCommentsAttachment() { + Node fn = parse("function f( // blah1\n x, // blah2\n y) {}").getFirstChild(); + assertNode(fn).hasType(Token.FUNCTION); + + Node xNode = fn.getSecondChild().getFirstChild(); + assertThat(xNode.getNonJSDocCommentString()).contains("// blah1"); + + Node yNode = fn.getSecondChild().getSecondChild(); + assertThat(yNode.getNonJSDocCommentString()).contains("// blah2"); + } + + @Test + public void testMultipleInline_MixedCommentsAttachment() { + Node fn = parse("function f( /* blah1 */ x, // blah2\n y) {}").getFirstChild(); + assertNode(fn).hasType(Token.FUNCTION); + + Node xNode = fn.getSecondChild().getFirstChild(); + assertThat(xNode.getNonJSDocCommentString()).contains("/* blah1 */"); + + Node yNode = fn.getSecondChild().getSecondChild(); + assertThat(yNode.getNonJSDocCommentString()).contains("// blah2"); + } + + @Test + public void testMultipleInline_NonJSDocCommentsGetAttachedToSameNode() { + Node fn = parse("function f(/* blah1 */\n// blah\n x) {}").getFirstChild(); + assertNode(fn).hasType(Token.FUNCTION); + + Node xNode = fn.getSecondChild().getFirstChild(); + assertThat(xNode.getNonJSDocCommentString()).contains("/* blah1 */\n// blah"); + } + + @Test + public void testBothJSDocAndNonJSDocCommentsGetAttached() { + Node fn = parse("function f(/** string */ // nonJSDoc\n x) {}").getFirstChild(); + assertNode(fn).hasType(Token.FUNCTION); + + Node xNode = fn.getSecondChild().getFirstChild(); + JSDocInfo info = xNode.getJSDocInfo(); + assertThat(info).isNotNull(); + assertNodeHasJSDocInfoWithJSType(xNode, STRING_TYPE); + assertThat(xNode.getNonJSDocCommentString()).contains("// nonJSDoc"); + } + + @Test + public void testBothNonJSDocAndJSDocCommentsGetAttached() { + Node fn = parse("function f(// nonJSDoc\n /** string */ x) {}").getFirstChild(); + assertNode(fn).hasType(Token.FUNCTION); + + Node xNode = fn.getSecondChild().getFirstChild(); + JSDocInfo info = xNode.getJSDocInfo(); + assertThat(info).isNotNull(); + assertNodeHasJSDocInfoWithJSType(xNode, STRING_TYPE); + assertThat(xNode.getNonJSDocCommentString()).contains("// nonJSDoc"); + } + @Test public void testInlineJSDocAttachment2() { Node fn = parse( @@ -4050,6 +4213,19 @@ public void testDefaultParameterInlineJSDoc() { assertNodeHasJSDocInfoWithJSType(aName, NUMBER_TYPE); } + @Test + public void testDefaultParameterInlineNonJSDocComment() { + expectFeatures(Feature.DEFAULT_PARAMETERS); + Node functionNode = parse("function f(/* number */ a = 0) {}").getFirstChild(); + Node parameterList = functionNode.getSecondChild(); + Node defaultValue = parameterList.getFirstChild(); + assertNode(defaultValue).hasType(Token.DEFAULT_VALUE); + + Node aName = defaultValue.getFirstChild(); + assertNode(aName).hasType(Token.NAME); + assertThat(aName.getNonJSDocCommentString()).contains("/* number */"); + } + @Test public void testRestParameters() { mode = LanguageMode.ECMASCRIPT6;