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

Parser does not handle annotated varargs correctly #4628

Draft
wants to merge 17 commits into
base: main
Choose a base branch
from
+86 −16
Draft

Parser does not handle annotated varargs correctly #4628

Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
f385553
Add reproducer testcase
Laurens-W Oct 30, 2024
648307a
Fixed
Laurens-W Oct 31, 2024
c04a5b4
Test locally succeeded??
Laurens-W Oct 31, 2024
d1914b8
Cover vararg of arrays
Laurens-W Oct 31, 2024
f433c85
Merge branch 'main' into annotated-varargs-issue
Laurens-W Oct 31, 2024
accd095
Add suggested unit test which indeed fails
timtebeek Nov 1, 2024
fd9df3d
Drop unnecessary final keywords
timtebeek Nov 1, 2024
c31ebfc
WIP
Laurens-W Nov 4, 2024
3bdaadc
WIP
Laurens-W Nov 4, 2024
25c5f8d
WIP
Laurens-W Nov 4, 2024
1c2d200
Getting closer
Laurens-W Nov 12, 2024
78d567e
Remove throwing of exception
Laurens-W Nov 13, 2024
7898d66
Fixed it?
Laurens-W Nov 19, 2024
48851ce
Apply bot comment
Laurens-W Nov 20, 2024
2fe8c45
Remove invalid check
Laurens-W Nov 20, 2024
11a2511
Simplify
Laurens-W Nov 20, 2024
1983024
Merge branch 'main' into annotated-varargs-issue
timtebeek Dec 11, 2024
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
20 changes: 18 additions & 2 deletions ...te-java-17/src/main/java/org/openrewrite/java/isolated/ReloadableJava17ParserVisitor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1298,7 +1298,11 @@ public J visitAnnotatedType(AnnotatedTypeTree node, Space fmt) {
if (node.getUnderlyingType() instanceof JCFieldAccess) {
return new J.AnnotatedType(randomId(), fmt, Markers.EMPTY, leadingAnnotations, annotatedTypeTree(node.getUnderlyingType(), annotationPosTable));
} else if (node.getUnderlyingType() instanceof JCArrayTypeTree) {
return new J.AnnotatedType(randomId(), fmt, Markers.EMPTY, leadingAnnotations, arrayTypeTree(node, annotationPosTable));
J.AnnotatedType annotatedType = new J.AnnotatedType(randomId(), fmt, Markers.EMPTY, leadingAnnotations, arrayTypeTree(node, annotationPosTable));
// if (((JCArrayTypeTree) node.getUnderlyingType()).getType() instanceof JCAnnotatedType) {
// visitAnnotatedType((AnnotatedTypeTree) ((JCArrayTypeTree) node.getUnderlyingType()).getType(), fmt);
Laurens-W marked this conversation as resolved.
Show resolved Hide resolved
// }
return annotatedType;
}
}
return new J.AnnotatedType(randomId(), fmt, Markers.EMPTY, leadingAnnotations, convert(node.getUnderlyingType()));
Expand Down Expand Up @@ -1387,7 +1391,7 @@ private TypeTree mapDimensions(TypeTree baseType, Tree tree, Map<Integer, JCAnno
if (typeIdent instanceof JCArrayTypeTree) {
List<J.Annotation> annotations = leadingAnnotations(annotationPosTable);
int saveCursor = cursor;
whitespace();
Space space = whitespace();
if (source.startsWith("[", cursor)) {
cursor = saveCursor;
JLeftPadded<Space> dimension = padLeft(sourceBefore("["), sourceBefore("]"));
Expand All @@ -1400,6 +1404,18 @@ private TypeTree mapDimensions(TypeTree baseType, Tree tree, Map<Integer, JCAnno
dimension,
typeMapping.type(tree)
);
} else if (source.startsWith("...", cursor)) {
J.ArrayType type = new J.ArrayType(
randomId(),
EMPTY,
Markers.EMPTY,
mapDimensions(baseType, ((JCArrayTypeTree) typeIdent).elemtype, annotationPosTable),
annotations,
JLeftPadded.build(space),
typeMapping.type(tree)
);
cursor = saveCursor;
return type;
}
cursor = saveCursor;
}
Expand Down
67 changes: 67 additions & 0 deletions rewrite-java-test/src/test/java/org/openrewrite/java/JavaParserTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,4 +214,71 @@ class A {
)
);
}

@Test
@Issue("https://github.com/openrewrite/rewrite/issues/3881")
void annotatedVargArgs() {
rewriteRun(
java(
"""
import org.jspecify.annotations.NonNull;

class C1 {
void m(@NonNull String @NonNull[] arrayArgs) {
}
}
"""
),
java(
"""
import org.jspecify.annotations.NonNull;

class C2 {
void m(@NonNull String @NonNull ... varArgs) {
}
}
"""
),
java(
"""
import org.jspecify.annotations.NonNull;

class C3 {
void m(String @NonNull[] @NonNull[] arrayArgs) {
}
}
"""
),
java(
"""
import org.jspecify.annotations.NonNull;

class C4 {
void m(String @NonNull [ ] @NonNull ... s) {
}
}
"""
),
java(
"""
import org.jspecify.annotations.NonNull;

class C5 {
void m(@NonNull String @NonNull[] @NonNull[] arrayArgs) {
}
}
"""
),
java(
"""
import org.jspecify.annotations.NonNull;

class C6 {
void m(@NonNull String @NonNull [ ] @NonNull ... s) {
}
}
"""
)
);
}
}
13 changes: 11 additions & 2 deletions rewrite-java/src/main/java/org/openrewrite/java/JavaPrinter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -428,8 +428,8 @@ protected void printStatementTerminator(Statement s, PrintOutputCapture<P> p) {
getCursor()
.dropParentUntil(
c -> c instanceof Switch ||
c instanceof SwitchExpression ||
c == Cursor.ROOT_VALUE
c instanceof SwitchExpression ||
c == Cursor.ROOT_VALUE
)
.getValue();
if (aSwitch instanceof SwitchExpression) {
Expand Down Expand Up @@ -848,6 +848,15 @@ public J visitVariableDeclarations(VariableDeclarations multiVariable, PrintOutp
p.append(']');
}
if (multiVariable.getVarargs() != null) {
if (!(multiVariable.getTypeExpression() instanceof ArrayType)) {
if (p.out.charAt(p.out.length() - 1) == ']') {
int posToCheck = p.out.length() - 2;
while (p.out.charAt(posToCheck) != '[') {
posToCheck--;
}
p.out.delete(posToCheck, p.out.length());
}
}
visitSpace(multiVariable.getVarargs(), Space.Location.VARARGS, p);
p.append("...");
}
Expand Down
Loading