Skip to content

Commit

Permalink
Parse and save inline comments in JS source
Browse files Browse the repository at this point in the history 
Inline source comments (nonJSDoc comments) do not get preserved in the node by the JSCompiler. A few examples:
let /* blah */ x = 'a';
function f(/* blah */ x) {}
let { normalProp: /* blah */ normalPropTarget } = {};
let { /* blah */ shorthandProp } = {};

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=261053300
  • Loading branch information
@rishipal @tjgq
rishipal authored and tjgq committed Aug 1, 2019
1 parent a48a0cb commit 42eed3f
Show file tree
Hide file tree
Showing 3 changed files with 219 additions and 25 deletions.
65 changes: 41 additions & 24 deletions src/com/google/javascript/jscomp/parsing/IRFactory.java
View file
Expand Up @@ -751,7 +751,7 @@ private boolean hasPendingNonJSDocCommentBefore(SourceRange location) {
&& currentNonJSDocComment.location.end.offset <= location.start.offset; && currentNonJSDocComment.location.end.offset <= location.start.offset;
} }


private ArrayList<Comment> getNonJsDocComments(SourceRange location) { private ArrayList<Comment> getNonJSDocComments(SourceRange location) {
ArrayList<Comment> previousComments = new ArrayList<>(); ArrayList<Comment> previousComments = new ArrayList<>();
while (hasPendingNonJSDocCommentBefore(location)) { while (hasPendingNonJSDocCommentBefore(location)) {
previousComments.add(currentNonJSDocComment); previousComments.add(currentNonJSDocComment);
Expand All @@ -760,12 +760,13 @@ private ArrayList<Comment> getNonJsDocComments(SourceRange location) {
return previousComments; return previousComments;
} }


private ArrayList<Comment> getNonJsDocComment(ParseTree tree) { private ArrayList<Comment> getNonJSDocComments(
return getNonJsDocComments(tree.location); com.google.javascript.jscomp.parsing.parser.Token token) {
return getNonJSDocComments(token.location);
} }


private ArrayList<Comment> handleNonJSDocComment(ParseTree node) { private ArrayList<Comment> getNonJSDocComments(ParseTree tree) {
return getNonJsDocComment(node); return getNonJSDocComments(tree.location);
} }


private static ParseTree findNearestNode(ParseTree tree) { private static ParseTree findNearestNode(ParseTree tree) {
Expand Down Expand Up @@ -800,7 +801,7 @@ private static ParseTree findNearestNode(ParseTree tree) {


Node transform(ParseTree tree) { Node transform(ParseTree tree) {
JSDocInfo info = handleJsDoc(tree); JSDocInfo info = handleJsDoc(tree);
ArrayList<Comment> associatedComments = handleNonJSDocComment(tree); ArrayList<Comment> associatedComments = getNonJSDocComments(tree);
Node node = transformDispatcher.process(tree); Node node = transformDispatcher.process(tree);
if (info != null) { if (info != null) {
node = maybeInjectCastNode(tree, info, node); node = maybeInjectCastNode(tree, info, node);
Expand Down Expand Up @@ -835,13 +836,21 @@ private Node maybeInjectCastNode(ParseTree node, JSDocInfo info, Node irNode) {
* @see <a href="http://code.google.com/p/jsdoc-toolkit/wiki/InlineDocs">Using Inline Doc * @see <a href="http://code.google.com/p/jsdoc-toolkit/wiki/InlineDocs">Using Inline Doc
* Comments</a> * Comments</a>
*/ */
Node transformNodeWithInlineJsDoc(ParseTree tree) { Node transformNodeWithInlineComments(ParseTree tree) {
JSDocInfo info = handleInlineJsDoc(tree); JSDocInfo info = handleInlineJsDoc(tree);
ArrayList<Comment> nonJSDocComments = getNonJSDocComments(tree);
NonJSDocComment associatedNonJSDocComment = null;
if (!nonJSDocComments.isEmpty()) {
associatedNonJSDocComment = combineCommentsIntoSingleComment(nonJSDocComments);
}
Node node = transformDispatcher.process(tree); Node node = transformDispatcher.process(tree);


if (info != null) { if (info != null) {
node.setJSDocInfo(info); node.setJSDocInfo(info);
} }
if (associatedNonJSDocComment != null) {
node.setNonJSDocComment(associatedNonJSDocComment);
}
setSourceInfo(node, tree); setSourceInfo(node, tree);


return node; return node;
Expand Down Expand Up @@ -1111,10 +1120,10 @@ Node processArrayPattern(ArrayPatternTree tree) {
break; break;
case ITER_REST: case ITER_REST:
maybeWarnForFeature(child, Feature.ARRAY_PATTERN_REST); maybeWarnForFeature(child, Feature.ARRAY_PATTERN_REST);
elementNode = transformNodeWithInlineJsDoc(child); elementNode = transformNodeWithInlineComments(child);
break; break;
default: default:
elementNode = transformNodeWithInlineJsDoc(child); elementNode = transformNodeWithInlineComments(child);
break; break;
} }
node.addChildToBack(elementNode); node.addChildToBack(elementNode);
Expand Down Expand Up @@ -1149,7 +1158,7 @@ private Node processObjectPatternElement(ParseTree child) {
case OBJECT_REST: case OBJECT_REST:
// let {...restObject} = someObject; // let {...restObject} = someObject;
maybeWarnForFeature(child, Feature.OBJECT_PATTERN_REST); 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); Node rest = newNode(Token.OBJECT_REST, target);
setSourceInfo(rest, child); setSourceInfo(rest, child);
return rest; return rest;
Expand Down Expand Up @@ -1199,7 +1208,7 @@ private Node processObjectPatternPropertyNameAssignment(
if (targetTree == null) { if (targetTree == null) {
// `let { /** inlineType */ key } = something;` // `let { /** inlineType */ key } = something;`
// The key is also the target name. // The key is also the target name.
valueNode = processNameWithInlineJSDoc(propertyNameAssignment.name.asIdentifier()); valueNode = processNameWithInlineComments(propertyNameAssignment.name.asIdentifier());
key.setShorthandProperty(true); key.setShorthandProperty(true);
} else { } else {
valueNode = processDestructuringElementTarget(targetTree); valueNode = processDestructuringElementTarget(targetTree);
Expand All @@ -1219,12 +1228,12 @@ private Node processDestructuringElementTarget(ParseTree targetTree) {
// let {key: /** inlineType */ name} = something // let {key: /** inlineType */ name} = something
// let [/** inlineType */ name] = something // let [/** inlineType */ name] = something
// Allow inline JSDoc on the name, since we may well be declaring it here. // Allow inline JSDoc on the name, since we may well be declaring it here.
valueNode = processNameWithInlineJSDoc(targetTree.asIdentifierExpression()); valueNode = processNameWithInlineComments(targetTree.asIdentifierExpression());
} else { } else {
// ({prop: /** string */ ns.a.b} = someObject); // ({prop: /** string */ ns.a.b} = someObject);
// NOTE: CheckJSDoc will report an error for this case, since we want qualified names to be // 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;` // declared with individual statements, like `/** @type {string} */ ns.a.b;`
valueNode = transformNodeWithInlineJsDoc(targetTree); valueNode = transformNodeWithInlineComments(targetTree);
} }
return valueNode; return valueNode;
} }
Expand Down Expand Up @@ -1491,7 +1500,7 @@ Node processFunction(FunctionDeclarationTree functionTree) {
IdentifierToken name = functionTree.name; IdentifierToken name = functionTree.name;
Node newName; Node newName;
if (name != null) { if (name != null) {
newName = processNameWithInlineJSDoc(name); newName = processNameWithInlineComments(name);
} else { } else {
if (isDeclaration || isMember) { if (isDeclaration || isMember) {
errorReporter.error( errorReporter.error(
Expand Down Expand Up @@ -1569,10 +1578,10 @@ Node processFormalParameterList(FormalParameterListTree tree) {
break; break;
case ITER_REST: case ITER_REST:
maybeWarnForFeature(param, Feature.REST_PARAMETERS); maybeWarnForFeature(param, Feature.REST_PARAMETERS);
paramNode = transformNodeWithInlineJsDoc(param); paramNode = transformNodeWithInlineComments(param);
break; break;
default: default:
paramNode = transformNodeWithInlineJsDoc(param); paramNode = transformNodeWithInlineComments(param);
break; break;
} }


Expand All @@ -1598,12 +1607,12 @@ Node processDefaultParameter(DefaultParameterTree tree) {
// allow inline JSDoc on an identifier // allow inline JSDoc on an identifier
// let { /** inlineType */ x = defaultValue } = someObject; // let { /** inlineType */ x = defaultValue } = someObject;
// TODO(bradfordcsmith): Do we need to allow inline JSDoc for qualified names, too? // TODO(bradfordcsmith): Do we need to allow inline JSDoc for qualified names, too?
targetNode = processNameWithInlineJSDoc(targetTree.asIdentifierExpression()); targetNode = processNameWithInlineComments(targetTree.asIdentifierExpression());
} else { } else {
// ({prop: /** string */ ns.a.b = 'foo'} = someObject); // ({prop: /** string */ ns.a.b = 'foo'} = someObject);
// NOTE: CheckJSDoc will report an error for this case, since we want qualified names to be // 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;` // declared with individual statements, like `/** @type {string} */ ns.a.b;`
targetNode = transformNodeWithInlineJsDoc(targetTree); targetNode = transformNodeWithInlineComments(targetTree);
} }
Node defaultValueNode = Node defaultValueNode =
newNode(Token.DEFAULT_VALUE, targetNode, transform(tree.defaultValue)); newNode(Token.DEFAULT_VALUE, targetNode, transform(tree.defaultValue));
Expand All @@ -1612,7 +1621,7 @@ Node processDefaultParameter(DefaultParameterTree tree) {
} }


Node processIterRest(IterRestTree tree) { Node processIterRest(IterRestTree tree) {
Node target = transformNodeWithInlineJsDoc(tree.assignmentTarget); Node target = transformNodeWithInlineComments(tree.assignmentTarget);
return newNode(Token.ITER_REST, target); return newNode(Token.ITER_REST, target);
} }


Expand Down Expand Up @@ -1783,17 +1792,25 @@ Node processTemplateLiteralToken(TemplateLiteralToken token) {
return node; return node;
} }


private Node processNameWithInlineJSDoc(IdentifierExpressionTree identifierExpression) { private Node processNameWithInlineComments(IdentifierExpressionTree identifierExpression) {
return processNameWithInlineJSDoc(identifierExpression.identifierToken); return processNameWithInlineComments(identifierExpression.identifierToken);
} }


Node processNameWithInlineJSDoc(IdentifierToken identifierToken) { Node processNameWithInlineComments(IdentifierToken identifierToken) {
JSDocInfo info = handleInlineJsDoc(identifierToken); JSDocInfo info = handleInlineJsDoc(identifierToken);
ArrayList<Comment> nonJSDocComments = getNonJSDocComments(identifierToken);
NonJSDocComment associatedNonJSDocComment = null;
if (!nonJSDocComments.isEmpty()) {
associatedNonJSDocComment = combineCommentsIntoSingleComment(nonJSDocComments);
}
maybeWarnReservedKeyword(identifierToken); maybeWarnReservedKeyword(identifierToken);
Node node = newStringNode(Token.NAME, identifierToken.value); Node node = newStringNode(Token.NAME, identifierToken.value);
if (info != null) { if (info != null) {
node.setJSDocInfo(info); node.setJSDocInfo(info);
} }
if (associatedNonJSDocComment != null) {
node.setNonJSDocComment(associatedNonJSDocComment);
}
setSourceInfo(node, identifierToken); setSourceInfo(node, identifierToken);
return node; return node;
} }
Expand Down Expand Up @@ -2289,13 +2306,13 @@ Node processVariableDeclarationList(VariableDeclarationListTree decl) {


Node node = newNode(declType); Node node = newNode(declType);
for (VariableDeclarationTree child : decl.declarations) { for (VariableDeclarationTree child : decl.declarations) {
node.addChildToBack(transformNodeWithInlineJsDoc(child)); node.addChildToBack(transformNodeWithInlineComments(child));
} }
return node; return node;
} }


Node processVariableDeclaration(VariableDeclarationTree decl) { Node processVariableDeclaration(VariableDeclarationTree decl) {
Node node = transformNodeWithInlineJsDoc(decl.lvalue); Node node = transformNodeWithInlineComments(decl.lvalue);
Node lhs = node.isDestructuringPattern() ? newNode(Token.DESTRUCTURING_LHS, node) : node; Node lhs = node.isDestructuringPattern() ? newNode(Token.DESTRUCTURING_LHS, node) : node;
if (decl.initializer != null) { if (decl.initializer != null) {
Node initializer = transform(decl.initializer); Node initializer = transform(decl.initializer);
Expand Down
3 changes: 2 additions & 1 deletion src/com/google/javascript/rhino/NonJSDocComment.java
View file
Expand Up @@ -38,9 +38,10 @@
* ***** END LICENSE BLOCK ***** */ * ***** END LICENSE BLOCK ***** */


package com.google.javascript.rhino; package com.google.javascript.rhino;
import java.io.Serializable;


/** Minimal class holding information about a nonJSDoc comment's source location and contents */ /** 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 beginOffset;
private final int endOffset; private final int endOffset;
private final String contents; private final String contents;
Expand Down

0 comments on commit 42eed3f

Please sign in to comment.