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.

Sign up for GitHub

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

Map trailing commas as TrailingComma Marker, to fix J.Erroneous issues seen #4869

Merged
merged 14 commits into from
Jan 9, 2025
Merged

Map trailing commas as TrailingComma Marker, to fix J.Erroneous issues seen #4869

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
f0052c3
Do not allow J.Erroneous LST nodes by default in tests
timtebeek Jan 8, 2025
1b96fe3
Allow erroneous nodes in FindCompileErrorsTest
timtebeek Jan 8, 2025
cb7a750
Use `TrailingComma` marker for enum values and array initializers
knutwannheden Jan 9, 2025
06cdd0c
Update whitespace handling for last enum constant
knutwannheden Jan 9, 2025
1938930
Allow errorneous nodes in specific tests
timtebeek Jan 9, 2025
1a9b6c8
Update JavaDoc for typeValidation.erroneous after feedback
timtebeek Jan 9, 2025
a9e6607
Use overloaded method
Laurens-W Jan 9, 2025
a46b4fa
Apply to Java 21 parser as well
Laurens-W Jan 9, 2025
4819697
Format Java 21 parser
Laurens-W Jan 9, 2025
8a398c3
Apply to Java 11 parser as well
Laurens-W Jan 9, 2025
34ff5c1
Apply to Java 8 parser as well
Laurens-W Jan 9, 2025
f688683
Fix failing tests
Laurens-W Jan 9, 2025
17bb8f0
Merge branch 'main' into do-not-allow-erroneous-lst-nodes-by-default
Laurens-W Jan 9, 2025
3d0763b
Minimize diff between versions to make it easier to compare
timtebeek Jan 9, 2025
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
98 changes: 68 additions & 30 deletions ...te-java-11/src/main/java/org/openrewrite/java/isolated/ReloadableJava11ParserVisitor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
import org.openrewrite.java.JavaPrinter;
import org.openrewrite.java.internal.JavaTypeCache;
import org.openrewrite.java.marker.OmitParentheses;
import org.openrewrite.java.marker.TrailingComma;
import org.openrewrite.java.tree.*;
import org.openrewrite.marker.Markers;
import org.openrewrite.style.NamedStyles;
Expand Down Expand Up @@ -327,7 +328,7 @@ public J visitBreak(BreakTree node, Space fmt) {

J.Identifier label = node.getLabel() == null ? null : new J.Identifier(randomId(),
sourceBefore(node.getLabel().toString()), Markers.EMPTY,
emptyList(), skip(node.getLabel().toString()), null, null);
emptyList(), node.getLabel().toString(), null, null);

return new J.Break(randomId(), fmt, Markers.EMPTY, label);
}
Expand Down Expand Up @@ -420,13 +421,31 @@ public J visitClass(ClassTree node, Space fmt) {

JRightPadded<Statement> enumSet = null;
if (!jcEnums.isEmpty()) {
AtomicBoolean semicolonPresent = new AtomicBoolean(false);

Tree lastConstant = jcEnums.get(jcEnums.size() - 1);
List<JRightPadded<J.EnumValue>> enumValues = convertAll(jcEnums, commaDelim, t -> {
// this semicolon is required when there are non-value members, but can still
// be present when there are not
semicolonPresent.set(positionOfNext(";", '}') > 0);
return semicolonPresent.get() ? sourceBefore(";", '}') : EMPTY;
if (t != lastConstant) {
return whitespace();
}
int savedCursor = cursor;
Space suffix = whitespace();
if (source.charAt(cursor) == ',' || source.charAt(cursor) == ';') {
return suffix;
}
// Whitespace should be assigned to prefix of next statement or `J.Block#end`
cursor = savedCursor;
return EMPTY;
}, t -> {
if (t == lastConstant && skip(",") != null) {
int savedCursor = cursor;
Space suffix = whitespace();
if (source.charAt(cursor) == ';') {
return Markers.build(singletonList(new TrailingComma(randomId(), suffix)));
}
// Whitespace should be assigned to prefix of next statement or `J.Block#end`
cursor = savedCursor;
return Markers.build(singletonList(new TrailingComma(randomId(), EMPTY)));
}
return Markers.EMPTY;
});

enumSet = padRight(
Expand All @@ -435,7 +454,7 @@ public J visitClass(ClassTree node, Space fmt) {
EMPTY,
Markers.EMPTY,
enumValues,
semicolonPresent.get()
skip(";") != null
),
EMPTY
);
Expand Down Expand Up @@ -1010,7 +1029,14 @@ public J visitNewArray(NewArrayTree node, Space fmt) {
JContainer<Expression> initializer = node.getInitializers() == null ? null :
JContainer.build(sourceBefore("{"), node.getInitializers().isEmpty() ?
singletonList(padRight(new J.Empty(randomId(), sourceBefore("}"), Markers.EMPTY), EMPTY)) :
convertAll(node.getInitializers(), commaDelim, t -> sourceBefore("}")), Markers.EMPTY);
convertAll(node.getInitializers(), commaDelim, t -> whitespace(), t -> {
if (t == node.getInitializers().get(node.getInitializers().size() - 1) && source.charAt(cursor) == ',') {
cursor++;
return Markers.build(singletonList(new TrailingComma(randomId(), whitespace())));
}
return Markers.EMPTY;
}), Markers.EMPTY);
skip("}");

return new J.NewArray(randomId(), fmt, Markers.EMPTY, typeExpr, dimensions,
initializer, typeMapping.type(node));
Expand Down Expand Up @@ -1622,7 +1648,6 @@ public J visitWildcard(WildcardTree node, Space fmt) {
* Conversion utilities
* --------------
*/

private <J2 extends J> @Nullable J2 convert(@Nullable Tree t) {
if (t == null) {
return null;
Expand Down Expand Up @@ -1667,20 +1692,24 @@ private static int getActualStartPosition(JCTree t) {
}

private <J2 extends @Nullable J> @Nullable JRightPadded<J2> convert(@Nullable Tree t, Function<Tree, Space> suffix) {
return convert(t, suffix, j -> Markers.EMPTY);
}

private <J2 extends @Nullable J> @Nullable JRightPadded<J2> convert(@Nullable Tree t, Function<Tree, Space> suffix, Function<Tree, Markers> markers) {
if (t == null) {
return null;
}
J2 j = convert(t);
@SuppressWarnings("ConstantConditions") JRightPadded<J2> rightPadded = j == null ? null :
new JRightPadded<>(j, suffix.apply(t), Markers.EMPTY);
new JRightPadded<>(j, suffix.apply(t), markers.apply(t));
int idx = findFirstNonWhitespaceChar(rightPadded.getAfter().getWhitespace());
if (idx >= 0) {
rightPadded = (JRightPadded<J2>) JRightPadded.build(getErroneous(List.of(rightPadded)));
}
// Cursor hasn't been updated but points at the end of erroneous node already
// This means that error node start position == end position
// Therefore ensure that cursor has moved to the end of erroneous node bu adding its length to the cursor
// Example `/pet` results in 2 erroeneous nodes: `/` and `pet`. The `/` node would have start and end position the
// Therefore ensure that cursor has moved to the end of erroneous node but adding its length to the cursor
// Example `/pet` results in 2 erroneous nodes: `/` and `pet`. The `/` node would have start and end position the
// same from the JC compiler.
if (endPos(t) == cursor && rightPadded.getElement() instanceof J.Erroneous) {
cursor += ((J.Erroneous) rightPadded.getElement()).getText().length();
Expand All @@ -1691,8 +1720,8 @@ private static int getActualStartPosition(JCTree t) {
}

private <J2 extends J> J.Erroneous getErroneous(List<JRightPadded<J2>> converted) {
PrintOutputCapture p = new PrintOutputCapture<>(0);
new JavaPrinter<>().visitContainer(JContainer.build(EMPTY, converted, Markers.EMPTY), JContainer.Location.METHOD_INVOCATION_ARGUMENTS, p);
PrintOutputCapture<Integer> p = new PrintOutputCapture<>(0);
new JavaPrinter<Integer>().visitContainer(JContainer.build(EMPTY, converted, Markers.EMPTY), JContainer.Location.METHOD_INVOCATION_ARGUMENTS, p);
return new J.Erroneous(
org.openrewrite.Tree.randomId(),
EMPTY,
Expand Down Expand Up @@ -1725,13 +1754,20 @@ private <J2 extends J> List<J2> convertAll(List<? extends Tree> trees) {
private <J2 extends J> List<JRightPadded<J2>> convertAll(List<? extends Tree> trees,
Function<Tree, Space> innerSuffix,
Function<Tree, Space> suffix) {
return convertAll(trees, innerSuffix, suffix, t -> Markers.EMPTY);
}

private <J2 extends J> List<JRightPadded<J2>> convertAll(List<? extends Tree> trees,
Function<Tree, Space> innerSuffix,
Function<Tree, Space> suffix,
Function<Tree, Markers> markers) {
int size = trees.size();
if (size == 0) {
return emptyList();
}
List<JRightPadded<J2>> converted = new ArrayList<>(size);
for (int i = 0; i < size; i++) {
converted.add(convert(trees.get(i), i == size - 1 ? suffix : innerSuffix));
converted.add(convert(trees.get(i), i == size - 1 ? suffix : innerSuffix, markers));
}
return converted;
}
Expand All @@ -1756,17 +1792,17 @@ private <J2 extends J> List<JRightPadded<J2>> convertAll(List<? extends Tree> tr

private Space statementDelim(@Nullable Tree t) {
if (t instanceof JCAssert ||
t instanceof JCAssign ||
t instanceof JCAssignOp ||
t instanceof JCBreak ||
t instanceof JCContinue ||
t instanceof JCDoWhileLoop ||
t instanceof JCImport ||
t instanceof JCMethodInvocation ||
t instanceof JCNewClass ||
t instanceof JCReturn ||
t instanceof JCThrow ||
t instanceof JCUnary) {
t instanceof JCAssign ||
t instanceof JCAssignOp ||
t instanceof JCBreak ||
t instanceof JCContinue ||
t instanceof JCDoWhileLoop ||
t instanceof JCImport ||
t instanceof JCMethodInvocation ||
t instanceof JCNewClass ||
t instanceof JCReturn ||
t instanceof JCThrow ||
t instanceof JCUnary) {
return sourceBefore(";");
}

Expand Down Expand Up @@ -1933,6 +1969,7 @@ private Space sourceBefore(String untilDelim, @Nullable Character stop) {
cursor += untilDelim.length();
return EMPTY;
}

Space space = format(source, cursor, delimIndex);
cursor = delimIndex + untilDelim.length(); // advance past the delimiter
return space;
Expand Down Expand Up @@ -2011,15 +2048,16 @@ private Space whitespace() {
return space;
}

private String skip(@Nullable String token) {
private @Nullable String skip(@Nullable String token) {
if (token == null) {
//noinspection ConstantConditions
return null;
}
if (source.startsWith(token, cursor)) {
cursor += token.length();
return token;
}
return token;
return null;
}

// Only exists as a function to make it easier to debug unexpected cursor shifts
Expand All @@ -2040,7 +2078,7 @@ private List<String> listFlags(long flags) {
try {
// FIXME instanceof probably not right here...
return field.get(null) instanceof Long &&
field.getName().matches("[A-Z_]+");
field.getName().matches("[A-Z_]+");
} catch (IllegalAccessException e) {
throw new RuntimeException(e);
}
Expand Down
Loading
Loading