Skip to content

Commit 0f25051

Browse files
timtebeektimtebeek
authored
Move nullable annotations to array types (#778)
- openrewrite/rewrite-migrate-java#934
1 parent 995c2d3 commit 0f25051

File tree

4 files changed

+180
-11
lines changed

4 files changed

+180
-11
lines changed

‎src/main/java/org/openrewrite/staticanalysis/AnnotateNullableMethods.java‎

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import org.openrewrite.java.tree.Expression;
2525
import org.openrewrite.java.tree.J;
2626
import org.openrewrite.java.tree.JavaType;
27+
import org.openrewrite.java.tree.TypeTree;
2728
import org.openrewrite.staticanalysis.java.MoveFieldAnnotationToType;
2829

2930
import java.util.Arrays;
@@ -80,8 +81,7 @@ public J.MethodDeclaration visitMethodDeclaration(J.MethodDeclaration methodDecl
8081
methodDeclaration.getMethodType() == null ||
8182
methodDeclaration.getMethodType().getReturnType() instanceof JavaType.Primitive ||
8283
service(AnnotationService.class).matches(getCursor(), new AnnotationMatcher("@" + fullyQualifiedName)) ||
83-
(methodDeclaration.getReturnTypeExpression() != null &&
84-
service(AnnotationService.class).matches(new Cursor(null, methodDeclaration.getReturnTypeExpression()), new AnnotationMatcher("@" + fullyQualifiedName)))) {
84+
hasNullableAnnotation(methodDeclaration.getReturnTypeExpression(), fullyQualifiedName)) {
8585
return methodDeclaration;
8686
}
8787

@@ -100,6 +100,27 @@ public J.MethodDeclaration visitMethodDeclaration(J.MethodDeclaration methodDecl
100100
}
101101
return md;
102102
}
103+
104+
private boolean hasNullableAnnotation(@Nullable TypeTree returnType, String annotationFqn) {
105+
if (returnType == null) {
106+
return false;
107+
}
108+
109+
// Check if the return type itself is annotated
110+
if (service(AnnotationService.class).matches(new Cursor(null, returnType), new AnnotationMatcher("@" + annotationFqn))) {
111+
return true;
112+
}
113+
114+
// For array types, check if the element type is annotated
115+
if (returnType instanceof J.ArrayType) {
116+
J.ArrayType arrayType = (J.ArrayType) returnType;
117+
if (arrayType.getElementType() instanceof J.AnnotatedType) {
118+
return service(AnnotationService.class).matches(new Cursor(null, arrayType.getElementType()), new AnnotationMatcher("@" + annotationFqn));
119+
}
120+
}
121+
122+
return false;
123+
}
103124
};
104125
return Repeat.repeatUntilStable(javaIsoVisitor, 5);
105126
}

‎src/main/java/org/openrewrite/staticanalysis/NullableOnMethodReturnType.java‎

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -59,14 +59,23 @@ public J.MethodDeclaration visitMethodDeclaration(J.MethodDeclaration method, Ex
5959
J.MethodDeclaration m2 = m;
6060
m2 = m2.withLeadingAnnotations(ListUtils.map(m2.getLeadingAnnotations(),
6161
a -> a == nullable.getTree() ? null : a));
62-
if (m2 != m) {
63-
m2 = m2.withReturnTypeExpression(new J.AnnotatedType(
64-
Tree.randomId(),
65-
Space.SINGLE_SPACE,
66-
Markers.EMPTY,
67-
singletonList(nullable.getTree().withPrefix(Space.EMPTY)),
68-
m2.getReturnTypeExpression()
69-
));
62+
if (m2 != m && m2.getReturnTypeExpression() != null) {
63+
// For array types, annotate the array brackets, not the element type
64+
if (m2.getReturnTypeExpression() instanceof J.ArrayType) {
65+
// For type-use annotations on arrays (JSpecify style), the annotation should be on the outermost array dimension
66+
// This creates: elementType @Nullable []
67+
// We do this by wrapping the ENTIRE array type in an AnnotatedType, but with proper spacing
68+
m2 = m2.withReturnTypeExpression(((J.ArrayType) m2.getReturnTypeExpression())
69+
.withAnnotations(singletonList(nullable.getTree().withPrefix(Space.SINGLE_SPACE))));
70+
} else {
71+
m2 = m2.withReturnTypeExpression(new J.AnnotatedType(
72+
Tree.randomId(),
73+
Space.SINGLE_SPACE,
74+
Markers.EMPTY,
75+
singletonList(nullable.getTree().withPrefix(Space.EMPTY)),
76+
m2.getReturnTypeExpression()
77+
));
78+
}
7079
m2 = autoFormat(m2, m2.getReturnTypeExpression(), ctx, getCursor().getParentOrThrow());
7180
m2 = m2.withPrefix(m2.getPrefix().withWhitespace(m2.getPrefix().getWhitespace().replace("\n\n\n", "\n\n")));
7281
}
@@ -75,6 +84,6 @@ public J.MethodDeclaration visitMethodDeclaration(J.MethodDeclaration method, Ex
7584
.orElse(m));
7685
}
7786
};
78-
return Preconditions.check(new UsesType<>("*..Nullable", false),visitor);
87+
return Preconditions.check(new UsesType<>("*..Nullable", false), visitor);
7988
}
8089
}

‎src/test/java/org/openrewrite/staticanalysis/AnnotateNullableMethodsTest.java‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -518,6 +518,47 @@ public class Test {
518518
);
519519
}
520520

521+
@Test
522+
void methodReturnsNullableArray() {
523+
rewriteRun(
524+
//language=java
525+
java(
526+
"""
527+
public class Test {
528+
529+
public String[] getArray() {
530+
return null;
531+
}
532+
533+
public int[] getIntArray() {
534+
if (System.currentTimeMillis() % 2 == 0) {
535+
return new int[]{1, 2, 3};
536+
}
537+
return null;
538+
}
539+
}
540+
""",
541+
"""
542+
import org.jspecify.annotations.Nullable;
543+
544+
public class Test {
545+
546+
public String @Nullable[] getArray() {
547+
return null;
548+
}
549+
550+
public int @Nullable[] getIntArray() {
551+
if (System.currentTimeMillis() % 2 == 0) {
552+
return new int[]{1, 2, 3};
553+
}
554+
return null;
555+
}
556+
}
557+
"""
558+
)
559+
);
560+
}
561+
521562
@Test
522563
void typescriptCode() {
523564
rewriteRun(

‎src/test/java/org/openrewrite/staticanalysis/NullableOnMethodReturnTypeTest.java‎

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,4 +131,102 @@ String test() {
131131
)
132132
);
133133
}
134+
135+
@Test
136+
void moveNullableToArrayType() {
137+
rewriteRun(
138+
//language=java
139+
java(
140+
"""
141+
import org.openrewrite.internal.lang.Nullable;
142+
class Test {
143+
@Nullable
144+
public String[] test() {
145+
return null;
146+
}
147+
}
148+
""",
149+
"""
150+
import org.openrewrite.internal.lang.Nullable;
151+
class Test {
152+
153+
public String @Nullable[] test() {
154+
return null;
155+
}
156+
}
157+
"""
158+
)
159+
);
160+
}
161+
162+
@Test
163+
void moveNullableToPrimitiveArrayType() {
164+
rewriteRun(
165+
//language=java
166+
java(
167+
"""
168+
import org.openrewrite.internal.lang.Nullable;
169+
class Test {
170+
@Nullable
171+
public int[] test() {
172+
return null;
173+
}
174+
}
175+
""",
176+
"""
177+
import org.openrewrite.internal.lang.Nullable;
178+
class Test {
179+
180+
public int @Nullable[] test() {
181+
return null;
182+
}
183+
}
184+
"""
185+
)
186+
);
187+
}
188+
189+
@Test
190+
void moveNullableToMultiDimensionalArray() {
191+
rewriteRun(
192+
//language=java
193+
java(
194+
"""
195+
import org.openrewrite.internal.lang.Nullable;
196+
class Test {
197+
@Nullable
198+
public String[][] test() {
199+
return null;
200+
}
201+
}
202+
""",
203+
"""
204+
import org.openrewrite.internal.lang.Nullable;
205+
class Test {
206+
207+
public String @Nullable[][] test() {
208+
return null;
209+
}
210+
}
211+
"""
212+
)
213+
);
214+
}
215+
216+
@Test
217+
void noChangeForNullableElements() {
218+
rewriteRun(
219+
//language=java
220+
java(
221+
"""
222+
import org.openrewrite.internal.lang.Nullable;
223+
class Test {
224+
public @Nullable String[] test() {
225+
return null;
226+
}
227+
}
228+
"""
229+
)
230+
);
231+
}
134232
}

0 commit comments

Comments
 (0)