Refactor comment handling in tokenization to use a new ErrorProneComment class that doesn't extend from javac's internal Comment

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

@@ -55,6 +55,7 @@
 import com.google.errorprone.apply.SourceFile;
 import com.google.errorprone.fixes.SuggestedFixes.FixCompiler.Result;
 import com.google.errorprone.util.ASTHelpers;
+import com.google.errorprone.util.ErrorProneComment;
 import com.google.errorprone.util.ErrorProneToken;
 import com.google.errorprone.util.FindIdentifiers;
 import com.sun.source.doctree.DocTree;
@@ -96,7 +97,6 @@
 import com.sun.tools.javac.code.Types.DefaultTypeVisitor;
 import com.sun.tools.javac.main.Arguments;
 import com.sun.tools.javac.parser.Tokens;
-import com.sun.tools.javac.parser.Tokens.Comment;
 import com.sun.tools.javac.parser.Tokens.TokenKind;
 import com.sun.tools.javac.tree.DCTree;
 import com.sun.tools.javac.tree.DCTree.DCDocComment;
@@ -1689,9 +1689,9 @@ public static SuggestedFix replaceIncludingComments(
     if (tokens.get(0).comments().isEmpty()) {
       return SuggestedFix.replace(tokens.get(0).pos(), state.getEndPosition(tree), replacement);
     }
-    ImmutableList<Comment> comments =
+    ImmutableList<ErrorProneComment> comments =
         ImmutableList.sortedCopyOf(
-            Comparator.<Comment>comparingInt(c -> c.getSourcePos(0)).reversed(),
+            Comparator.<ErrorProneComment>comparingInt(c -> c.getSourcePos(0)).reversed(),
             tokens.get(0).comments());
     int startPos = getStartPosition(tree);
     // This can happen for desugared expressions like `int a, b;`.
@@ -1700,7 +1700,7 @@ public static SuggestedFix replaceIncludingComments(
     }
     // Delete backwards for comments which are not separated from our target by a blank line.
     CharSequence sourceCode = state.getSourceCode();
-    for (Comment comment : comments) {
+    for (ErrorProneComment comment : comments) {
       int endOfCommentPos = comment.getSourcePos(comment.getText().length() - 1);
       CharSequence stringBetweenComments = sourceCode.subSequence(endOfCommentPos, startPos);
       if (stringBetweenComments.chars().filter(c -> c == '\n').count() > 1) {

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

@@ -20,7 +20,6 @@
 import com.google.common.collect.ImmutableList;
 import com.google.errorprone.annotations.CanIgnoreReturnValue;
 import com.sun.source.tree.Tree;
-import com.sun.tools.javac.parser.Tokens.Comment;
 
 /** Class to hold AST nodes annotated with the comments that are associated with them */
 @AutoValue
@@ -35,9 +34,9 @@ public enum Position {
 
   public abstract T tree();
 
-  public abstract ImmutableList<Comment> beforeComments();
+  public abstract ImmutableList<ErrorProneComment> beforeComments();
 
-  public abstract ImmutableList<Comment> afterComments();
+  public abstract ImmutableList<ErrorProneComment> afterComments();
 
   static <T extends Tree> Builder<T> builder() {
     return new AutoValue_Commented.Builder<T>();
@@ -48,14 +47,14 @@ abstract static class Builder<T extends Tree> {
 
     abstract Builder<T> setTree(T tree);
 
-    protected abstract ImmutableList.Builder<Comment> beforeCommentsBuilder();
+    protected abstract ImmutableList.Builder<ErrorProneComment> beforeCommentsBuilder();
 
-    protected abstract ImmutableList.Builder<Comment> afterCommentsBuilder();
+    protected abstract ImmutableList.Builder<ErrorProneComment> afterCommentsBuilder();
 
     @CanIgnoreReturnValue
     Builder<T> addComment(
-        Comment comment, int nodePosition, int tokenizingOffset, Position position) {
-      OffsetComment offsetComment = new OffsetComment(comment, tokenizingOffset);
+        ErrorProneComment comment, int nodePosition, int tokenizingOffset, Position position) {
+      ErrorProneComment offsetComment = comment.withOffset(tokenizingOffset);
 
       if (comment.getSourcePos(0) < nodePosition) {
         if (position.equals(Position.BEFORE) || position.equals(Position.ANY)) {
@@ -71,11 +70,11 @@ Builder<T> addComment(
 
     @CanIgnoreReturnValue
     Builder<T> addAllComment(
-        Iterable<? extends Comment> comments,
+        Iterable<ErrorProneComment> comments,
         int nodePosition,
         int tokenizingOffset,
         Position position) {
-      for (Comment comment : comments) {
+      for (ErrorProneComment comment : comments) {
         addComment(comment, nodePosition, tokenizingOffset, position);
       }
       return this;

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

@@ -25,7 +25,6 @@
 import com.google.common.collect.TreeRangeSet;
 import com.google.errorprone.VisitorState;
 import com.google.errorprone.util.Commented.Position;
-import com.google.errorprone.util.ErrorProneTokens.CommentWithTextAndPosition;
 import com.sun.source.tree.BlockTree;
 import com.sun.source.tree.ClassTree;
 import com.sun.source.tree.ExpressionTree;
@@ -34,7 +33,6 @@
 import com.sun.source.tree.Tree;
 import com.sun.source.util.TreePath;
 import com.sun.source.util.TreeScanner;
-import com.sun.tools.javac.parser.Tokens.Comment;
 import com.sun.tools.javac.parser.Tokens.TokenKind;
 import com.sun.tools.javac.util.Position.LineMap;
 import java.util.Iterator;
@@ -92,7 +90,7 @@ public static ImmutableList<Commented<ExpressionTree>> findCommentsForArguments(
    *
    * <p>TODO(andrewrice) Update this method to handle block comments properly if we find the need
    */
-  public static String getTextFromComment(Comment comment) {
+  public static String getTextFromComment(ErrorProneComment comment) {
     switch (comment.getStyle()) {
       case BLOCK:
         return comment.getText().replaceAll("^\\s*/\\*\\s*(.*?)\\s*\\*/\\s*", "$1");
@@ -152,29 +150,25 @@ private static ImmutableList<Commented<ExpressionTree>> findCommentsForArguments
       if (tokenTracker.atStartOfLine() && !tokenTracker.wasPreviousLineEmpty()) {
         // if the token is at the start of a line it could still have a comment attached which was
         // on the previous line
-        for (Comment c : token.comments()) {
-          if (!(c instanceof CommentWithTextAndPosition)) {
-            continue;
-          }
-          CommentWithTextAndPosition comment = (CommentWithTextAndPosition) c;
+        for (ErrorProneComment comment : token.comments()) {
           int commentStart = comment.getPos();
           int commentEnd = comment.getEndPos();
           if (exclude.intersects(Range.closedOpen(commentStart, commentEnd))) {
             continue;
           }
-          if (tokenTracker.isCommentOnPreviousLine(c)
+          if (tokenTracker.isCommentOnPreviousLine(comment)
               && token.pos() <= argumentTracker.currentArgumentStartPosition
               && argumentTracker.isPreviousArgumentOnPreviousLine()) {
             // token was on the previous line so therefore we should add it to the previous comment
             // unless the previous argument was not on the previous line with it
-            argumentTracker.addCommentToPreviousArgument(c, Position.ANY);
+            argumentTracker.addCommentToPreviousArgument(comment, Position.ANY);
           } else {
             // if the comment comes after the end of the invocation and it's not on the same line
             // as the final argument then we need to ignore it
             if (commentStart <= invocationEnd
                 || lineMap.getLineNumber(commentStart)
                     <= lineMap.getLineNumber(argumentTracker.currentArgumentEndPosition)) {
-              argumentTracker.addCommentToCurrentArgument(c, Position.ANY);
+              argumentTracker.addCommentToCurrentArgument(comment, Position.ANY);
             }
           }
         }
@@ -366,7 +360,7 @@ void advance(ErrorProneToken token) {
       }
     }
 
-    boolean isCommentOnPreviousLine(Comment c) {
+    boolean isCommentOnPreviousLine(ErrorProneComment c) {
       int tokenLine = lineMap.getLineNumber(c.getSourcePos(0));
       return tokenLine == currentLineNumber - 1;
     }
@@ -447,15 +441,15 @@ boolean isPreviousArgumentOnPreviousLine() {
           == lineMap.getLineNumber(currentArgumentStartPosition) - 1;
     }
 
-    void addCommentToPreviousArgument(Comment c, Position position) {
+    void addCommentToPreviousArgument(ErrorProneComment c, Position position) {
       previousCommentedResultBuilder.addComment(c, previousArgumentEndPosition, offset, position);
     }
 
-    void addCommentToCurrentArgument(Comment c, Position position) {
+    void addCommentToCurrentArgument(ErrorProneComment c, Position position) {
       currentCommentedResultBuilder.addComment(c, currentArgumentStartPosition, offset, position);
     }
 
-    void addAllCommentsToCurrentArgument(Iterable<Comment> comments, Position position) {
+    void addAllCommentsToCurrentArgument(Iterable<ErrorProneComment> comments, Position position) {
       currentCommentedResultBuilder.addAllComment(
           comments, currentArgumentStartPosition, offset, position);
     }

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

@@ -0,0 +1,91 @@
+/*
+ * Copyright 2019 The Error Prone Authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.google.errorprone.util;
+
+import static com.google.common.base.Preconditions.checkArgument;
+
+import com.google.common.base.Supplier;
+import com.google.common.base.Suppliers;
+import com.sun.tools.javac.parser.Tokens.Comment;
+import com.sun.tools.javac.parser.Tokens.Comment.CommentStyle;
+
+/** Wraps a {@link Comment} to allow an offset source position to be reported. */
+public final class ErrorProneComment {
+
+  private final int pos;
+  private final int endPos;
+  private final int offset;
+  private final Supplier<String> text;
+  private final CommentStyle style;
+
+  ErrorProneComment(int pos, int endPos, int offset, Supplier<String> text, CommentStyle style) {
+    this.pos = pos;
+    this.endPos = endPos;
+    this.offset = offset;
+    this.text = Suppliers.memoize(text);
+    this.style = style;
+  }
+
+  public ErrorProneComment withOffset(int offset) {
+    return new ErrorProneComment(pos, endPos, offset, text, style);
+  }
+
+  public int getPos() {
+    return pos + offset;
+  }
+
+  public int getEndPos() {
+    return endPos + offset;
+  }
+
+  /**
+   * Returns the source position of the character at index {@code index} in the comment text.
+   *
+   * <p>The handling of javadoc comments in javac has more logic to skip over leading whitespace and
+   * '*' characters when indexing into doc comments, but we don't need any of that.
+   */
+  public int getSourcePos(int index) {
+    checkArgument(
+        0 <= index && index < (endPos - pos),
+        "Expected %s in the range [0, %s)",
+        index,
+        endPos - pos);
+    return pos + index + offset;
+  }
+
+  public CommentStyle getStyle() {
+    return style;
+  }
+
+  public String getText() {
+    return text.get();
+  }
+
+  /**
+   * We don't care about {@code @deprecated} javadoc tags (see the DepAnn check).
+   *
+   * @return false
+   */
+  public boolean isDeprecated() {
+    return false;
+  }
+
+  @Override
+  public String toString() {
+    return String.format("Comment: '%s'", getText());
+  }
+}

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

@@ -16,24 +16,23 @@
 
 package com.google.errorprone.util;
 
-import static java.util.stream.Collectors.toList;
+import static com.google.common.collect.ImmutableList.toImmutableList;
 
-import com.google.common.collect.Lists;
-import com.sun.tools.javac.parser.Tokens.Comment;
+import com.google.common.collect.ImmutableList;
 import com.sun.tools.javac.parser.Tokens.Token;
 import com.sun.tools.javac.parser.Tokens.TokenKind;
 import com.sun.tools.javac.util.Name;
-import java.util.Collections;
-import java.util.List;
 
 /** Wraps a javac {@link Token} to return comments in declaration order. */
 public class ErrorProneToken {
-  private final int offset;
   private final Token token;
+  private final int offset;
+  private final ImmutableList<ErrorProneComment> comments;
 
-  ErrorProneToken(Token token, int offset) {
+  ErrorProneToken(Token token, int offset, ImmutableList<ErrorProneComment> comments) {
     this.token = token;
     this.offset = offset;
+    this.comments = comments;
   }
 
   public TokenKind kind() {
@@ -48,17 +47,8 @@ public int endPos() {
     return offset + token.endPos;
   }
 
-  public List<Comment> comments() {
-    // javac stores the comments in reverse declaration order because appending to linked
-    // lists is expensive
-    if (token.comments == null) {
-      return Collections.emptyList();
-    }
-    if (offset == 0) {
-      return Lists.reverse(token.comments);
-    }
-    return Lists.reverse(
-        token.comments.stream().map(c -> new OffsetComment(c, offset)).collect(toList()));
+  public ImmutableList<ErrorProneComment> comments() {
+    return comments.stream().map(c -> c.withOffset(offset)).collect(toImmutableList());
   }
 
   public boolean hasName() {