Skip to content

Commit c2df1b6

Browse files
cushonError Prone Team
cushon
authored and
Error Prone Team
committed
Refactor comment handling in tokenization to use a new ErrorProneComment class that doesn't extend from javac's internal Comment
This is pre-work for handling upcoming JDK changes to the Comment API, which break the current subclass approach by changing the return type of `Comment#getPos`. This change encapsulates more of the internal javac APIs. PiperOrigin-RevId: 638438532
1 parent 3fff610 commit c2df1b6

14 files changed

Lines changed: 187 additions & 230 deletions

File tree

‎check_api/src/main/java/com/google/errorprone/fixes/SuggestedFixes.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@
5555
import com.google.errorprone.apply.SourceFile;
5656
import com.google.errorprone.fixes.SuggestedFixes.FixCompiler.Result;
5757
import com.google.errorprone.util.ASTHelpers;
58+
import com.google.errorprone.util.ErrorProneComment;
5859
import com.google.errorprone.util.ErrorProneToken;
5960
import com.google.errorprone.util.FindIdentifiers;
6061
import com.sun.source.doctree.DocTree;
@@ -96,7 +97,6 @@
9697
import com.sun.tools.javac.code.Types.DefaultTypeVisitor;
9798
import com.sun.tools.javac.main.Arguments;
9899
import com.sun.tools.javac.parser.Tokens;
99-
import com.sun.tools.javac.parser.Tokens.Comment;
100100
import com.sun.tools.javac.parser.Tokens.TokenKind;
101101
import com.sun.tools.javac.tree.DCTree;
102102
import com.sun.tools.javac.tree.DCTree.DCDocComment;
@@ -1689,9 +1689,9 @@ public static SuggestedFix replaceIncludingComments(
16891689
if (tokens.get(0).comments().isEmpty()) {
16901690
return SuggestedFix.replace(tokens.get(0).pos(), state.getEndPosition(tree), replacement);
16911691
}
1692-
ImmutableList<Comment> comments =
1692+
ImmutableList<ErrorProneComment> comments =
16931693
ImmutableList.sortedCopyOf(
1694-
Comparator.<Comment>comparingInt(c -> c.getSourcePos(0)).reversed(),
1694+
Comparator.<ErrorProneComment>comparingInt(c -> c.getSourcePos(0)).reversed(),
16951695
tokens.get(0).comments());
16961696
int startPos = getStartPosition(tree);
16971697
// This can happen for desugared expressions like `int a, b;`.
@@ -1700,7 +1700,7 @@ public static SuggestedFix replaceIncludingComments(
17001700
}
17011701
// Delete backwards for comments which are not separated from our target by a blank line.
17021702
CharSequence sourceCode = state.getSourceCode();
1703-
for (Comment comment : comments) {
1703+
for (ErrorProneComment comment : comments) {
17041704
int endOfCommentPos = comment.getSourcePos(comment.getText().length() - 1);
17051705
CharSequence stringBetweenComments = sourceCode.subSequence(endOfCommentPos, startPos);
17061706
if (stringBetweenComments.chars().filter(c -> c == '\n').count() > 1) {

‎check_api/src/main/java/com/google/errorprone/util/Commented.java‎

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020
import com.google.common.collect.ImmutableList;
2121
import com.google.errorprone.annotations.CanIgnoreReturnValue;
2222
import com.sun.source.tree.Tree;
23-
import com.sun.tools.javac.parser.Tokens.Comment;
2423

2524
/** Class to hold AST nodes annotated with the comments that are associated with them */
2625
@AutoValue
@@ -35,9 +34,9 @@ public enum Position {
3534

3635
public abstract T tree();
3736

38-
public abstract ImmutableList<Comment> beforeComments();
37+
public abstract ImmutableList<ErrorProneComment> beforeComments();
3938

40-
public abstract ImmutableList<Comment> afterComments();
39+
public abstract ImmutableList<ErrorProneComment> afterComments();
4140

4241
static <T extends Tree> Builder<T> builder() {
4342
return new AutoValue_Commented.Builder<T>();
@@ -48,14 +47,14 @@ abstract static class Builder<T extends Tree> {
4847

4948
abstract Builder<T> setTree(T tree);
5049

51-
protected abstract ImmutableList.Builder<Comment> beforeCommentsBuilder();
50+
protected abstract ImmutableList.Builder<ErrorProneComment> beforeCommentsBuilder();
5251

53-
protected abstract ImmutableList.Builder<Comment> afterCommentsBuilder();
52+
protected abstract ImmutableList.Builder<ErrorProneComment> afterCommentsBuilder();
5453

5554
@CanIgnoreReturnValue
5655
Builder<T> addComment(
57-
Comment comment, int nodePosition, int tokenizingOffset, Position position) {
58-
OffsetComment offsetComment = new OffsetComment(comment, tokenizingOffset);
56+
ErrorProneComment comment, int nodePosition, int tokenizingOffset, Position position) {
57+
ErrorProneComment offsetComment = comment.withOffset(tokenizingOffset);
5958

6059
if (comment.getSourcePos(0) < nodePosition) {
6160
if (position.equals(Position.BEFORE) || position.equals(Position.ANY)) {
@@ -71,11 +70,11 @@ Builder<T> addComment(
7170

7271
@CanIgnoreReturnValue
7372
Builder<T> addAllComment(
74-
Iterable<? extends Comment> comments,
73+
Iterable<ErrorProneComment> comments,
7574
int nodePosition,
7675
int tokenizingOffset,
7776
Position position) {
78-
for (Comment comment : comments) {
77+
for (ErrorProneComment comment : comments) {
7978
addComment(comment, nodePosition, tokenizingOffset, position);
8079
}
8180
return this;

‎check_api/src/main/java/com/google/errorprone/util/Comments.java‎

Lines changed: 9 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@
2525
import com.google.common.collect.TreeRangeSet;
2626
import com.google.errorprone.VisitorState;
2727
import com.google.errorprone.util.Commented.Position;
28-
import com.google.errorprone.util.ErrorProneTokens.CommentWithTextAndPosition;
2928
import com.sun.source.tree.BlockTree;
3029
import com.sun.source.tree.ClassTree;
3130
import com.sun.source.tree.ExpressionTree;
@@ -34,7 +33,6 @@
3433
import com.sun.source.tree.Tree;
3534
import com.sun.source.util.TreePath;
3635
import com.sun.source.util.TreeScanner;
37-
import com.sun.tools.javac.parser.Tokens.Comment;
3836
import com.sun.tools.javac.parser.Tokens.TokenKind;
3937
import com.sun.tools.javac.util.Position.LineMap;
4038
import java.util.Iterator;
@@ -92,7 +90,7 @@ public static ImmutableList<Commented<ExpressionTree>> findCommentsForArguments(
9290
*
9391
* <p>TODO(andrewrice) Update this method to handle block comments properly if we find the need
9492
*/
95-
public static String getTextFromComment(Comment comment) {
93+
public static String getTextFromComment(ErrorProneComment comment) {
9694
switch (comment.getStyle()) {
9795
case BLOCK:
9896
return comment.getText().replaceAll("^\\s*/\\*\\s*(.*?)\\s*\\*/\\s*", "$1");
@@ -152,29 +150,25 @@ private static ImmutableList<Commented<ExpressionTree>> findCommentsForArguments
152150
if (tokenTracker.atStartOfLine() && !tokenTracker.wasPreviousLineEmpty()) {
153151
// if the token is at the start of a line it could still have a comment attached which was
154152
// on the previous line
155-
for (Comment c : token.comments()) {
156-
if (!(c instanceof CommentWithTextAndPosition)) {
157-
continue;
158-
}
159-
CommentWithTextAndPosition comment = (CommentWithTextAndPosition) c;
153+
for (ErrorProneComment comment : token.comments()) {
160154
int commentStart = comment.getPos();
161155
int commentEnd = comment.getEndPos();
162156
if (exclude.intersects(Range.closedOpen(commentStart, commentEnd))) {
163157
continue;
164158
}
165-
if (tokenTracker.isCommentOnPreviousLine(c)
159+
if (tokenTracker.isCommentOnPreviousLine(comment)
166160
&& token.pos() <= argumentTracker.currentArgumentStartPosition
167161
&& argumentTracker.isPreviousArgumentOnPreviousLine()) {
168162
// token was on the previous line so therefore we should add it to the previous comment
169163
// unless the previous argument was not on the previous line with it
170-
argumentTracker.addCommentToPreviousArgument(c, Position.ANY);
164+
argumentTracker.addCommentToPreviousArgument(comment, Position.ANY);
171165
} else {
172166
// if the comment comes after the end of the invocation and it's not on the same line
173167
// as the final argument then we need to ignore it
174168
if (commentStart <= invocationEnd
175169
|| lineMap.getLineNumber(commentStart)
176170
<= lineMap.getLineNumber(argumentTracker.currentArgumentEndPosition)) {
177-
argumentTracker.addCommentToCurrentArgument(c, Position.ANY);
171+
argumentTracker.addCommentToCurrentArgument(comment, Position.ANY);
178172
}
179173
}
180174
}
@@ -366,7 +360,7 @@ void advance(ErrorProneToken token) {
366360
}
367361
}
368362

369-
boolean isCommentOnPreviousLine(Comment c) {
363+
boolean isCommentOnPreviousLine(ErrorProneComment c) {
370364
int tokenLine = lineMap.getLineNumber(c.getSourcePos(0));
371365
return tokenLine == currentLineNumber - 1;
372366
}
@@ -447,15 +441,15 @@ boolean isPreviousArgumentOnPreviousLine() {
447441
== lineMap.getLineNumber(currentArgumentStartPosition) - 1;
448442
}
449443

450-
void addCommentToPreviousArgument(Comment c, Position position) {
444+
void addCommentToPreviousArgument(ErrorProneComment c, Position position) {
451445
previousCommentedResultBuilder.addComment(c, previousArgumentEndPosition, offset, position);
452446
}
453447

454-
void addCommentToCurrentArgument(Comment c, Position position) {
448+
void addCommentToCurrentArgument(ErrorProneComment c, Position position) {
455449
currentCommentedResultBuilder.addComment(c, currentArgumentStartPosition, offset, position);
456450
}
457451

458-
void addAllCommentsToCurrentArgument(Iterable<Comment> comments, Position position) {
452+
void addAllCommentsToCurrentArgument(Iterable<ErrorProneComment> comments, Position position) {
459453
currentCommentedResultBuilder.addAllComment(
460454
comments, currentArgumentStartPosition, offset, position);
461455
}

‎check_api/src/main/java/com/google/errorprone/util/ErrorProneComment.java‎

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
/*
2+
* Copyright 2019 The Error Prone Authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package com.google.errorprone.util;
18+
19+
import static com.google.common.base.Preconditions.checkArgument;
20+
21+
import com.google.common.base.Supplier;
22+
import com.google.common.base.Suppliers;
23+
import com.sun.tools.javac.parser.Tokens.Comment;
24+
import com.sun.tools.javac.parser.Tokens.Comment.CommentStyle;
25+
26+
/** Wraps a {@link Comment} to allow an offset source position to be reported. */
27+
public final class ErrorProneComment {
28+
29+
private final int pos;
30+
private final int endPos;
31+
private final int offset;
32+
private final Supplier<String> text;
33+
private final CommentStyle style;
34+
35+
ErrorProneComment(int pos, int endPos, int offset, Supplier<String> text, CommentStyle style) {
36+
this.pos = pos;
37+
this.endPos = endPos;
38+
this.offset = offset;
39+
this.text = Suppliers.memoize(text);
40+
this.style = style;
41+
}
42+
43+
public ErrorProneComment withOffset(int offset) {
44+
return new ErrorProneComment(pos, endPos, offset, text, style);
45+
}
46+
47+
public int getPos() {
48+
return pos + offset;
49+
}
50+
51+
public int getEndPos() {
52+
return endPos + offset;
53+
}
54+
55+
/**
56+
* Returns the source position of the character at index {@code index} in the comment text.
57+
*
58+
* <p>The handling of javadoc comments in javac has more logic to skip over leading whitespace and
59+
* '*' characters when indexing into doc comments, but we don't need any of that.
60+
*/
61+
public int getSourcePos(int index) {
62+
checkArgument(
63+
0 <= index && index < (endPos - pos),
64+
"Expected %s in the range [0, %s)",
65+
index,
66+
endPos - pos);
67+
return pos + index + offset;
68+
}
69+
70+
public CommentStyle getStyle() {
71+
return style;
72+
}
73+
74+
public String getText() {
75+
return text.get();
76+
}
77+
78+
/**
79+
* We don't care about {@code @deprecated} javadoc tags (see the DepAnn check).
80+
*
81+
* @return false
82+
*/
83+
public boolean isDeprecated() {
84+
return false;
85+
}
86+
87+
@Override
88+
public String toString() {
89+
return String.format("Comment: '%s'", getText());
90+
}
91+
}

‎check_api/src/main/java/com/google/errorprone/util/ErrorProneToken.java‎

Lines changed: 8 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -16,24 +16,23 @@
1616

1717
package com.google.errorprone.util;
1818

19-
import static java.util.stream.Collectors.toList;
19+
import static com.google.common.collect.ImmutableList.toImmutableList;
2020

21-
import com.google.common.collect.Lists;
22-
import com.sun.tools.javac.parser.Tokens.Comment;
21+
import com.google.common.collect.ImmutableList;
2322
import com.sun.tools.javac.parser.Tokens.Token;
2423
import com.sun.tools.javac.parser.Tokens.TokenKind;
2524
import com.sun.tools.javac.util.Name;
26-
import java.util.Collections;
27-
import java.util.List;
2825

2926
/** Wraps a javac {@link Token} to return comments in declaration order. */
3027
public class ErrorProneToken {
31-
private final int offset;
3228
private final Token token;
29+
private final int offset;
30+
private final ImmutableList<ErrorProneComment> comments;
3331

34-
ErrorProneToken(Token token, int offset) {
32+
ErrorProneToken(Token token, int offset, ImmutableList<ErrorProneComment> comments) {
3533
this.token = token;
3634
this.offset = offset;
35+
this.comments = comments;
3736
}
3837

3938
public TokenKind kind() {
@@ -48,17 +47,8 @@ public int endPos() {
4847
return offset + token.endPos;
4948
}
5049

51-
public List<Comment> comments() {
52-
// javac stores the comments in reverse declaration order because appending to linked
53-
// lists is expensive
54-
if (token.comments == null) {
55-
return Collections.emptyList();
56-
}
57-
if (offset == 0) {
58-
return Lists.reverse(token.comments);
59-
}
60-
return Lists.reverse(
61-
token.comments.stream().map(c -> new OffsetComment(c, offset)).collect(toList()));
50+
public ImmutableList<ErrorProneComment> comments() {
51+
return comments.stream().map(c -> c.withOffset(offset)).collect(toImmutableList());
6252
}
6353

6454
public boolean hasName() {

0 commit comments

Comments
 (0)