Skip to content

Commit d3fccfb

Browse files
12wrigjacopybara-github
authored andcommitted
Treat JSDoc comments within closure-unaware ranges as if they were just block comments, unless they are important comments (/*!) or have the @license or @preserve JSDoc tags.
PiperOrigin-RevId: 717960772
1 parent f0a310d commit d3fccfb

File tree

7 files changed

+367
-77
lines changed

7 files changed

+367
-77
lines changed

src/com/google/javascript/jscomp/parsing/Config.java

+7-2
Original file line numberDiff line numberDiff line change
@@ -101,14 +101,17 @@ public enum JsDocParsing {
101101
TYPES_ONLY,
102102
INCLUDE_DESCRIPTIONS_NO_WHITESPACE,
103103
INCLUDE_DESCRIPTIONS_WITH_WHITESPACE,
104-
INCLUDE_ALL_COMMENTS;
104+
INCLUDE_ALL_COMMENTS,
105+
LICENSE_COMMENTS_ONLY;
105106

106107
boolean shouldParseDescriptions() {
107108
return this != TYPES_ONLY;
108109
}
109110

110111
boolean shouldPreserveWhitespace() {
111-
return this == INCLUDE_DESCRIPTIONS_WITH_WHITESPACE || this == INCLUDE_ALL_COMMENTS;
112+
return this == INCLUDE_DESCRIPTIONS_WITH_WHITESPACE
113+
|| this == INCLUDE_ALL_COMMENTS
114+
|| this == LICENSE_COMMENTS_ONLY;
112115
}
113116
}
114117

@@ -148,6 +151,8 @@ final ImmutableSet<String> annotationNames() {
148151
return annotations().keySet();
149152
}
150153

154+
public abstract Builder toBuilder();
155+
151156
public static Builder builder() {
152157
return new AutoValue_Config.Builder()
153158
.setLanguageMode(LanguageMode.UNSUPPORTED)

src/com/google/javascript/jscomp/parsing/IRFactory.java

+110-58
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,7 @@
140140
import com.google.javascript.rhino.dtoa.DToA;
141141
import java.math.BigInteger;
142142
import java.util.ArrayDeque;
143+
import java.util.Comparator;
143144
import java.util.Deque;
144145
import java.util.LinkedHashSet;
145146
import java.util.List;
@@ -293,14 +294,7 @@ private IRFactory(
293294
this.sourceName = sourceFile == null ? null : sourceFile.getName();
294295

295296
this.config = config;
296-
// We can't just use the existing error reporter, because we might be parsing code that is
297-
// closure-unaware, and there is a conceptually-circular dependency that makes suppressing
298-
// spurious errors difficult: we need to be able to parse a file to determine if the JSDoc was
299-
// annotated as closure-unaware, but that parsing hasn't finished when the existing error
300-
// reporter is called with the parse errors from that code. Instead, we have to locally track
301-
/// whether the parser has seen the relevant closure-unaware annotation, and locally drop the
302-
// errors, handing any other errors off to the provided error reporter.
303-
this.errorReporter = new ClosureUnawareCodeSkippingJsDocInfoErroReporter(this, errorReporter);
297+
this.errorReporter = errorReporter;
304298
this.transformDispatcher = new TransformDispatcher();
305299

306300
if (config.strictMode().isStrict()) {
@@ -317,6 +311,7 @@ private static final class CommentTracker {
317311
private final ImmutableList<Comment> source;
318312
private final Predicate<Comment> filter;
319313
private int index = -1;
314+
private int previousIndex = -1;
320315

321316
CommentTracker(ImmutableList<Comment> source, Predicate<Comment> filter) {
322317
this.source = source;
@@ -330,6 +325,7 @@ private static final class CommentTracker {
330325
}
331326

332327
void advance() {
328+
this.previousIndex = this.index;
333329
while (true) {
334330
this.index++; // Always advance at least one element.
335331

@@ -340,6 +336,10 @@ void advance() {
340336
}
341337
}
342338

339+
void backtrack() {
340+
this.index = this.previousIndex;
341+
}
342+
343343
boolean hasPendingCommentBefore(SourcePosition pos) {
344344
Comment c = this.current();
345345
// The line number matters for trailing comments
@@ -379,7 +379,10 @@ public static IRFactory transformTree(
379379
for (Comment comment : tree.sourceComments) {
380380
if ((comment.type == Comment.Type.JSDOC || comment.type == Comment.Type.IMPORTANT)
381381
&& !irFactory.parsedComments.contains(comment)) {
382-
irFactory.handlePossibleFileOverviewJsDoc(comment);
382+
boolean useLicensesOnlyConfig =
383+
irFactory.withinClosureUnawareCodeRange(
384+
comment.location.start.line, comment.location.start.column);
385+
irFactory.handlePossibleFileOverviewJsDoc(comment, useLicensesOnlyConfig);
383386
}
384387
}
385388

@@ -701,8 +704,8 @@ private boolean handlePossibleFileOverviewJsDoc(JsDocInfoParser jsDocParser) {
701704
return true;
702705
}
703706

704-
private void handlePossibleFileOverviewJsDoc(Comment comment) {
705-
JsDocInfoParser jsDocParser = createJsDocInfoParser(comment);
707+
private void handlePossibleFileOverviewJsDoc(Comment comment, boolean useLicensesOnlyConfig) {
708+
JsDocInfoParser jsDocParser = createJsDocInfoParser(comment, useLicensesOnlyConfig);
706709
parsedComments.add(comment);
707710
handlePossibleFileOverviewJsDoc(jsDocParser);
708711
}
@@ -721,6 +724,17 @@ private Comment getJSDocCommentAt(SourcePosition pos) {
721724
if (comment == null) {
722725
return null;
723726
}
727+
728+
if (withinClosureUnawareCodeRange(comment.location.start.line, comment.location.start.column)) {
729+
// Within closure-unaware code, we don't parse jsdoc as if it is load-bearing.
730+
// This early-return prevents the comments here from being recorded as being parsed,
731+
// and this later allows all these jsdoc comments to be treated as "top-level" comments that
732+
// might contain license info / "important" comments (the only form of JSDoc that should
733+
// happen for closure-unaware code).
734+
this.jsdocTracker.backtrack();
735+
return null;
736+
}
737+
724738
JsDocInfoParser jsDocParser = createJsDocInfoParser(comment);
725739
parsedComments.add(comment);
726740
if (handlePossibleFileOverviewJsDoc(jsDocParser)) {
@@ -760,38 +774,78 @@ private Comment getJSDocCommentAt(SourcePosition pos) {
760774
return parseJSDocInfoFrom(getJSDocCommentAt(tree.getStart()));
761775
}
762776

763-
JSDocInfo parseJSDocInfoOnToken(com.google.javascript.jscomp.parsing.parser.Token token) {
777+
@Nullable JSDocInfo parseJSDocInfoOnToken(
778+
com.google.javascript.jscomp.parsing.parser.Token token) {
764779
return parseJSDocInfoFrom(getJSDocCommentAt(token.getStart()));
765780
}
766781

767-
JSDocInfo parseInlineJSDocAt(SourcePosition pos) {
782+
@Nullable JSDocInfo parseInlineJSDocAt(SourcePosition pos) {
783+
if (withinClosureUnawareCodeRange(pos.line, pos.column)) {
784+
// Within closure-unaware code, we don't parse jsdoc as if it is load-bearing.
785+
// This early-return prevents the comments here from being recorded as being parsed,
786+
// and this later allows all these jsdoc comments to be treated as "top-level" comments that
787+
// might contain license info / "important" comments (the only form of JSDoc that should
788+
// happen for closure-unaware code).
789+
// Unlike other points in IRFactory where we explicitly backtrack the jsdocTracker, we don't
790+
// do that here because we've actually never advanced it yet - that happens in
791+
// getJSDocCommentAt.
792+
return null;
793+
}
768794
Comment comment = getJSDocCommentAt(pos);
769795
return (comment != null && !comment.value.contains("@"))
770796
? parseInlineTypeDoc(comment)
771797
: parseJSDocInfoFrom(comment);
772798
}
773799

800+
private static final Comparator<Comment> COMMENT_START_POSITION_COMPARATOR =
801+
comparingInt((Comment x) -> x.location.start.line)
802+
.thenComparingInt(x -> x.location.start.column);
803+
804+
private boolean hasAnyPendingCommentBefore(
805+
SourcePosition pos, boolean withinClosureUnawareCodeRange) {
806+
return this.nonJsdocTracker.hasPendingCommentBefore(pos)
807+
|| (withinClosureUnawareCodeRange && this.jsdocTracker.hasPendingCommentBefore(pos));
808+
}
809+
774810
/**
775811
* Creates a single NonJSDocComment from every comment associated with this node; or null if there
776812
* are no such comments.
777813
*
778814
* <p>It would be legal to replace all comments associated with this node with that one string.
779815
*/
780816
private @Nullable NonJSDocComment parseNonJSDocCommentAt(SourcePosition pos, boolean isInline) {
781-
if (config.jsDocParsingMode() != JsDocParsing.INCLUDE_ALL_COMMENTS) {
782-
return null;
783-
}
784-
785-
if (!this.nonJsdocTracker.hasPendingCommentBefore(pos)) {
786-
return null;
787-
}
817+
boolean withinClosureUnawareCodeRange = withinClosureUnawareCodeRange(pos.line, pos.column);
788818

789819
StringBuilder result = new StringBuilder();
790-
Comment firstComment = this.nonJsdocTracker.current();
820+
Comment firstComment = null;
791821
Comment lastComment = null;
792822

793-
while (this.nonJsdocTracker.hasPendingCommentBefore(pos)) {
823+
while (hasAnyPendingCommentBefore(pos, withinClosureUnawareCodeRange)) {
794824
Comment currentComment = this.nonJsdocTracker.current();
825+
boolean commentFromJsdocTracker = false;
826+
827+
if (withinClosureUnawareCodeRange) {
828+
Comment currentJsDocComment = this.jsdocTracker.current();
829+
if (currentComment == null
830+
|| (currentJsDocComment != null
831+
&& COMMENT_START_POSITION_COMPARATOR.compare(currentComment, currentJsDocComment)
832+
> 0)) {
833+
834+
if (currentJsDocComment.value.contains("@license")) {
835+
// Skip this so it can be parsed for the license contents (which happens when a comment
836+
// isn't inserted into the parsedComments set).
837+
this.jsdocTracker.advance();
838+
continue;
839+
}
840+
841+
commentFromJsdocTracker = true;
842+
currentComment = currentJsDocComment;
843+
}
844+
}
845+
846+
if (firstComment == null) {
847+
firstComment = currentComment;
848+
}
795849

796850
if (lastComment != null) {
797851
for (int blankCount = currentComment.location.start.line - lastComment.location.end.line;
@@ -802,8 +856,22 @@ JSDocInfo parseInlineJSDocAt(SourcePosition pos) {
802856
}
803857
result.append(currentComment.value);
804858

859+
805860
lastComment = currentComment;
806-
this.nonJsdocTracker.advance();
861+
if (commentFromJsdocTracker) {
862+
this.parsedComments.add(currentComment);
863+
this.jsdocTracker.advance();
864+
} else {
865+
this.nonJsdocTracker.advance();
866+
}
867+
}
868+
869+
if (firstComment == null) {
870+
return null; // We didn't find any relevant comments.
871+
}
872+
873+
if (config.jsDocParsingMode() != JsDocParsing.INCLUDE_ALL_COMMENTS) {
874+
return null;
807875
}
808876

809877
NonJSDocComment nonJSDocComment =
@@ -929,10 +997,6 @@ private boolean withinClosureUnawareCodeRange(int line, int lineColumnNo) {
929997
}
930998

931999
private Node maybeInjectCastNode(ParseTree node, JSDocInfo info, Node irNode) {
932-
if (withinClosureUnawareCodeRange(node.location.start.line, node.location.start.column)) {
933-
// closure-unaware code should never have CAST nodes in the AST.
934-
return irNode;
935-
}
9361000
if (node.type == ParseTreeType.PAREN_EXPRESSION && info.hasType()) {
9371001
irNode = newNode(Token.CAST, irNode);
9381002
}
@@ -1006,6 +1070,9 @@ static String languageFeatureWarningMessage(Feature feature) {
10061070

10071071
void maybeWarnForFeature(ParseTree node, Feature feature) {
10081072
features = features.with(feature);
1073+
if (withinClosureUnawareCodeRange(node.location.start.line, node.location.start.column)) {
1074+
return;
1075+
}
10091076
if (!isSupportedForInputLanguageMode(feature)) {
10101077
errorReporter.warning(
10111078
languageFeatureWarningMessage(feature), sourceName, lineno(node), charno(node));
@@ -1015,6 +1082,9 @@ void maybeWarnForFeature(ParseTree node, Feature feature) {
10151082
void maybeWarnForFeature(
10161083
com.google.javascript.jscomp.parsing.parser.Token token, Feature feature) {
10171084
features = features.with(feature);
1085+
if (withinClosureUnawareCodeRange(token.location.start.line, token.location.start.column)) {
1086+
return;
1087+
}
10181088
if (!isSupportedForInputLanguageMode(feature)) {
10191089
errorReporter.warning(
10201090
languageFeatureWarningMessage(feature), sourceName, lineno(token), charno(token));
@@ -1023,6 +1093,9 @@ void maybeWarnForFeature(
10231093

10241094
void maybeWarnForFeature(Node node, Feature feature) {
10251095
features = features.with(feature);
1096+
if (withinClosureUnawareCodeRange(node.getLineno(), node.getCharno())) {
1097+
return;
1098+
}
10261099
if (!isSupportedForInputLanguageMode(feature)) {
10271100
errorReporter.warning(
10281101
languageFeatureWarningMessage(feature), sourceName, node.getLineno(), node.getCharno());
@@ -1054,6 +1127,10 @@ void setSourceInfo(Node node, SourcePosition start, SourcePosition end) {
10541127
}
10551128
}
10561129

1130+
private JsDocInfoParser createJsDocInfoParser(Comment node) {
1131+
return createJsDocInfoParser(node, false);
1132+
}
1133+
10571134
/**
10581135
* Creates a JsDocInfoParser and parses the JsDoc string.
10591136
*
@@ -1064,12 +1141,17 @@ void setSourceInfo(Node node, SourcePosition start, SourcePosition end) {
10641141
* @return A JsDocInfoParser. Will contain either fileoverview JsDoc, or normal JsDoc, or no JsDoc
10651142
* (if the method parses to the wrong level).
10661143
*/
1067-
private JsDocInfoParser createJsDocInfoParser(Comment node) {
1144+
private JsDocInfoParser createJsDocInfoParser(Comment node, boolean useLicensesOnlyConfig) {
10681145
String comment = node.value;
10691146
int lineno = lineno(node.location.start);
10701147
int charno = charno(node.location.start);
10711148
int position = node.location.start.offset;
10721149

1150+
Config config = this.config;
1151+
if (useLicensesOnlyConfig) {
1152+
config = config.toBuilder().setJsDocParsingMode(JsDocParsing.LICENSE_COMMENTS_ONLY).build();
1153+
}
1154+
10731155
// The JsDocInfoParser expects the comment without the initial '/**'.
10741156
int numOpeningChars = 3;
10751157
JsDocInfoParser jsdocParser =
@@ -1122,36 +1204,6 @@ void setLengthFrom(Node node, Node ref) {
11221204
node.setLength(ref.getLength());
11231205
}
11241206

1125-
private static final class ClosureUnawareCodeSkippingJsDocInfoErroReporter
1126-
implements ErrorReporter {
1127-
private final ErrorReporter delegate;
1128-
private final IRFactory host;
1129-
1130-
private ClosureUnawareCodeSkippingJsDocInfoErroReporter(
1131-
IRFactory host, ErrorReporter delegate) {
1132-
this.delegate = delegate;
1133-
this.host = host;
1134-
}
1135-
1136-
@Override
1137-
public void error(String message, String sourceName, int line, int lineOffset) {
1138-
// Line numbers are 1-indexed, but the SourcePosition uses 0-indexed.
1139-
if (host.withinClosureUnawareCodeRange(line - 1, lineOffset)) {
1140-
return;
1141-
}
1142-
delegate.error(message, sourceName, line, lineOffset);
1143-
}
1144-
1145-
@Override
1146-
public void warning(String message, String sourceName, int line, int lineOffset) {
1147-
// Line numbers are 1-indexed, but the SourcePosition uses 0-indexed.
1148-
if (host.withinClosureUnawareCodeRange(line - 1, lineOffset)) {
1149-
return;
1150-
}
1151-
delegate.warning(message, sourceName, line, lineOffset);
1152-
}
1153-
}
1154-
11551207
private static final QualifiedName GOOG_MODULE = QualifiedName.of("goog.module");
11561208

11571209
private class TransformDispatcher {

src/com/google/javascript/jscomp/parsing/JsDocInfoParser.java

+9
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,7 @@ private void addMissingTypeWarning(int lineno, int charno) {
131131
ImmutableSet.of("unique", "consistent", "stable", "mapped", "xid");
132132
private static final ImmutableSet<String> primitiveTypes =
133133
ImmutableSet.of("number", "string", "boolean", "symbol");
134+
private final boolean onlyParseLicenseJsDoc;
134135

135136
private @Nullable String licenseText;
136137

@@ -192,6 +193,8 @@ public JsDocInfoParser(
192193
this.closurePrimitiveNames = config.closurePrimitiveNames();
193194
this.preserveWhitespace = config.jsDocParsingMode().shouldPreserveWhitespace();
194195
this.jsDocSourceKind = jsDocSourceKind;
196+
this.onlyParseLicenseJsDoc =
197+
config.jsDocParsingMode() == Config.JsDocParsing.LICENSE_COMMENTS_ONLY;
195198

196199
this.errorReporter = errorReporter;
197200
this.templateNode = templateNode == null ? IR.script() : templateNode;
@@ -397,6 +400,12 @@ private JsDocToken parseAnnotation(JsDocToken token, List<ExtendedTypeInfo> exte
397400

398401
String annotationName = stream.getString();
399402
Annotation annotation = annotations.get(annotationName);
403+
404+
if (this.onlyParseLicenseJsDoc
405+
&& !(annotation == Annotation.LICENSE || annotation == Annotation.PRESERVE)) {
406+
return next();
407+
}
408+
400409
if (annotation == null || annotationName.isEmpty()) {
401410
addParserWarning(Msg.BAD_JSDOC_TAG, annotationName);
402411
} else {

src/com/google/javascript/rhino/testing/NodeSubject.java

+6
Original file line numberDiff line numberDiff line change
@@ -271,6 +271,12 @@ public NodeSubject isFunction() {
271271
return this;
272272
}
273273

274+
@CanIgnoreReturnValue
275+
public NodeSubject isCast() {
276+
hasToken(Token.CAST);
277+
return this;
278+
}
279+
274280
@CanIgnoreReturnValue
275281
public NodeSubject isArrowFunction() {
276282
check("isArrowFunction()").that(actual.isArrowFunction()).isTrue();

0 commit comments

Comments
 (0)