Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Work around a crash on comments inside string template arguments #1071

Merged
merged 1 commit into from
Feb 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
""";
}
Loading