Skip to content

Commit

Permalink
Work around a crash on comments inside string template arguments
Browse files Browse the repository at this point in the history
PiperOrigin-RevId: 609732753
  • Loading branch information
cushon authored and google-java-format Team committed Feb 23, 2024
1 parent ce3cb59 commit e946e82
Show file tree
Hide file tree
Showing 4 changed files with 83 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package com.google.googlejavaformat.java;

import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkElementIndex;
import static java.util.Arrays.stream;

import com.google.common.collect.ImmutableList;
Expand All @@ -28,16 +29,18 @@
import com.sun.tools.javac.parser.Tokens.TokenKind;
import com.sun.tools.javac.parser.UnicodeReader;
import com.sun.tools.javac.util.Context;
import java.lang.reflect.Method;
import java.util.ArrayList;
import java.util.Collections;
import java.util.Comparator;
import java.util.HashSet;
import java.util.List;
import java.util.Objects;
import java.util.Set;
import org.checkerframework.checker.nullness.qual.Nullable;

/** A wrapper around javac's lexer. */
class JavacTokens {
final class JavacTokens {

/** The lexer eats terminal comments, so feed it one we don't care about. */
// TODO(b/33103797): fix javac and remove the work-around
Expand All @@ -51,6 +54,8 @@ static class RawTok {
private final int endPos;

RawTok(String stringVal, TokenKind kind, int pos, int endPos) {
checkElementIndex(pos, endPos, "pos");
checkArgument(pos < endPos, "expected pos (%s) < endPos (%s)", pos, endPos);
this.stringVal = stringVal;
this.kind = kind;
this.pos = pos;
Expand Down Expand Up @@ -136,13 +141,30 @@ public static ImmutableList<RawTok> getTokens(
int last = 0;
for (Token t : javacTokens) {
if (t.comments != null) {
// javac accumulates comments in reverse order
for (Comment c : Lists.reverse(t.comments)) {
if (last < c.getSourcePos(0)) {
tokens.add(new RawTok(null, null, last, c.getSourcePos(0)));
int pos = c.getSourcePos(0);
int length;
if (pos == -1) {
// We've found a comment whose position hasn't been recorded. Deduce its position as the
// first `/` character after the end of the previous token.
//
// javac creates a new JavaTokenizer to process string template arguments, so
// CommentSavingTokenizer doesn't get a chance to preprocess those comments and save
// their text and positions.
//
// TODO: consider always using this approach once the minimum supported JDK is 16 and
// we can assume BasicComment#getRawCharacters is always available.
pos = source.indexOf('/', last);
length = CommentSavingTokenizer.commentLength(c);
} else {
length = c.getText().length();
}
tokens.add(
new RawTok(null, null, c.getSourcePos(0), c.getSourcePos(0) + c.getText().length()));
last = c.getSourcePos(0) + c.getText().length();
if (last < pos) {
tokens.add(new RawTok(null, null, last, pos));
}
tokens.add(new RawTok(null, null, pos, pos + length));
last = pos + length;
}
}
if (stopTokens.contains(t.kind)) {
Expand Down Expand Up @@ -181,6 +203,32 @@ public static ImmutableList<RawTok> getTokens(

/** A {@link JavaTokenizer} that saves comments. */
static class CommentSavingTokenizer extends JavaTokenizer {

private static final Method GET_RAW_CHARACTERS_METHOD = getRawCharactersMethod();

private static @Nullable Method getRawCharactersMethod() {
try {
// This is a method in PositionTrackingReader, but that class is not public.
return BasicComment.class.getMethod("getRawCharacters");
} catch (NoSuchMethodException e) {
return null;
}
}

static int commentLength(Comment comment) {
if (comment instanceof BasicComment && GET_RAW_CHARACTERS_METHOD != null) {
// If we've seen a BasicComment instead of a CommentWithTextAndPosition, getText() will
// be null, so we deduce the length using getRawCharacters. See also the comment at the
// usage of this method in getTokens.
try {
return ((char[]) GET_RAW_CHARACTERS_METHOD.invoke(((BasicComment) comment))).length;
} catch (ReflectiveOperationException e) {
throw new LinkageError(e.getMessage(), e);
}
}
return comment.getText().length();
}

CommentSavingTokenizer(ScannerFactory fac, char[] buffer, int length) {
super(fac, buffer, length);
}
Expand Down Expand Up @@ -288,4 +336,6 @@ protected AccessibleReader(ScannerFactory fac, char[] buffer, int length) {
super(fac, buffer, length);
}
}

private JavacTokens() {}
}
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,8 @@ public class FormatterIntegrationTest {
"I981",
"StringTemplate",
"I1020",
"I1037")
"I1037",
"TextBlockTemplates")
.build();

@Parameters(name = "{index}: {0}")
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
abstract class RSLs {
abstract String f();

String a = STR."""
lorem
foo\{ /** a method */ f( // line comment
/* TODO */
)}bar
ipsum
""";
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
abstract class RSLs {
abstract String f();

String a =
STR."""
lorem
foo\{
/** a method */
f( // line comment
/* TODO */
)}bar
ipsum
""";
}

0 comments on commit e946e82

Please sign in to comment.