Skip to content

Commit 5f64939

Browse files
timtebeekclaudegithub-actions[bot]
authored
Fix NPE in UseCollectionInterfaces with annotated raw collection types (#717)
* Fix NPE/ClassCastException in UseCollectionInterfaces with annotated and fully-qualified types This commit fixes two related issues: 1. Issue #713: NPE with annotated raw collection types When a method or field has an annotated raw collection type (e.g., `@Nullable ArrayList`), the recipe would throw a ClassCastException because it incorrectly assumed that annotated types always wrap parameterized types. Raw types are represented as `J.Identifier`, not `J.ParameterizedType`. 2. Issue #716: ClassCastException with fully-qualified type names When using fully-qualified type names (e.g., `java.util.HashSet`), the recipe would throw a ClassCastException because these are represented as `J.FieldAccess`, not `J.Identifier` or `J.ParameterizedType`. The fix adds proper handling for all these cases: - Raw types (`J.Identifier`) - Fully-qualified types (`J.FieldAccess`) - Parameterized types (`J.ParameterizedType`) - All of the above when wrapped in annotations (`J.AnnotatedType`) Fixes #713 Fixes #716 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]> * Deduplicate * Apply suggestions from code review Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * Revert "Apply suggestions from code review" This reverts commit 9ba33bc. * Remove redundant else after return --------- Co-authored-by: Claude <[email protected]> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
1 parent feee568 commit 5f64939

File tree

2 files changed

+220
-73
lines changed

2 files changed

+220
-73
lines changed

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

Lines changed: 82 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
*/
1616
package org.openrewrite.staticanalysis;
1717

18+
import org.jetbrains.annotations.Contract;
1819
import org.jspecify.annotations.Nullable;
1920
import org.openrewrite.*;
2021
import org.openrewrite.groovy.tree.G;
@@ -114,46 +115,6 @@ public J visit(@Nullable Tree tree, ExecutionContext ctx) {
114115
return super.visit(tree, ctx);
115116
}
116117

117-
@Override
118-
public J.MethodDeclaration visitMethodDeclaration(J.MethodDeclaration method, ExecutionContext ctx) {
119-
J.MethodDeclaration m = super.visitMethodDeclaration(method, ctx);
120-
if ((m.hasModifier(J.Modifier.Type.Public) || m.hasModifier(J.Modifier.Type.Private) || m.getModifiers().isEmpty()) &&
121-
m.getReturnTypeExpression() != null) {
122-
JavaType.FullyQualified originalType = TypeUtils.asFullyQualified(m.getReturnTypeExpression().getType());
123-
if (originalType != null && rspecRulesReplaceTypeMap.containsKey(originalType.getFullyQualifiedName())) {
124-
125-
JavaType.FullyQualified newType = TypeUtils.asFullyQualified(
126-
JavaType.buildType(rspecRulesReplaceTypeMap.get(originalType.getFullyQualifiedName())));
127-
if (newType != null) {
128-
maybeRemoveImport(originalType);
129-
maybeAddImport(newType);
130-
131-
TypeTree typeExpression;
132-
if (m.getReturnTypeExpression() instanceof J.Identifier) {
133-
typeExpression = new J.Identifier(
134-
randomId(),
135-
m.getReturnTypeExpression().getPrefix(),
136-
Markers.EMPTY,
137-
emptyList(),
138-
newType.getClassName(),
139-
newType,
140-
null
141-
);
142-
} else if (m.getReturnTypeExpression() instanceof J.AnnotatedType) {
143-
J.AnnotatedType annotatedType = (J.AnnotatedType) m.getReturnTypeExpression();
144-
J.ParameterizedType parameterizedType = (J.ParameterizedType) annotatedType.getTypeExpression();
145-
typeExpression = annotatedType.withTypeExpression(removeFromParameterizedType(newType, parameterizedType));
146-
} else {
147-
J.ParameterizedType parameterizedType = (J.ParameterizedType) m.getReturnTypeExpression();
148-
typeExpression = removeFromParameterizedType(newType, parameterizedType);
149-
}
150-
m = m.withReturnTypeExpression(typeExpression);
151-
}
152-
}
153-
}
154-
return m;
155-
}
156-
157118
@Override
158119
public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) {
159120
J.MethodInvocation mi = super.visitMethodInvocation(method, ctx);
@@ -176,6 +137,41 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Execu
176137
return mi;
177138
}
178139

140+
private J.MethodInvocation updateMethodInvocation(J.MethodInvocation mi, JavaType.FullyQualified newType) {
141+
if (mi.getSelect() != null) {
142+
mi = mi.withSelect(mi.getSelect().withType(newType));
143+
if (mi.getSelect() instanceof J.FieldAccess) {
144+
J.FieldAccess fieldAccess = (J.FieldAccess) mi.getSelect();
145+
mi = mi.withSelect(fieldAccess.withName(fieldAccess.getName().withType(newType)));
146+
}
147+
}
148+
if (mi.getMethodType() != null) {
149+
mi = mi.withMethodType(mi.getMethodType().withDeclaringType(newType));
150+
}
151+
return mi;
152+
}
153+
154+
@Override
155+
public J.MethodDeclaration visitMethodDeclaration(J.MethodDeclaration method, ExecutionContext ctx) {
156+
J.MethodDeclaration m = super.visitMethodDeclaration(method, ctx);
157+
if ((m.hasModifier(J.Modifier.Type.Public) || m.hasModifier(J.Modifier.Type.Private) || m.getModifiers().isEmpty()) &&
158+
m.getReturnTypeExpression() != null) {
159+
JavaType.FullyQualified originalType = TypeUtils.asFullyQualified(m.getReturnTypeExpression().getType());
160+
if (originalType != null && rspecRulesReplaceTypeMap.containsKey(originalType.getFullyQualifiedName())) {
161+
162+
JavaType.FullyQualified newType = TypeUtils.asFullyQualified(
163+
JavaType.buildType(rspecRulesReplaceTypeMap.get(originalType.getFullyQualifiedName())));
164+
if (newType != null) {
165+
maybeRemoveImport(originalType);
166+
maybeAddImport(newType);
167+
168+
m = m.withReturnTypeExpression(getTypeTree(m.getReturnTypeExpression(), newType));
169+
}
170+
}
171+
}
172+
return m;
173+
}
174+
179175
@Override
180176
public J.VariableDeclarations visitVariableDeclarations(J.VariableDeclarations multiVariable, ExecutionContext ctx) {
181177
J.VariableDeclarations mv = super.visitVariableDeclarations(multiVariable, ctx);
@@ -192,29 +188,7 @@ public J.VariableDeclarations visitVariableDeclarations(J.VariableDeclarations m
192188
maybeRemoveImport(originalType);
193189
maybeAddImport(newType);
194190

195-
TypeTree typeExpression;
196-
if (mv.getTypeExpression() == null) {
197-
typeExpression = null;
198-
} else if (mv.getTypeExpression() instanceof J.Identifier) {
199-
typeExpression = new J.Identifier(
200-
randomId(),
201-
mv.getTypeExpression().getPrefix(),
202-
Markers.EMPTY,
203-
emptyList(),
204-
newType.getClassName(),
205-
newType,
206-
null
207-
);
208-
} else if (mv.getTypeExpression() instanceof J.AnnotatedType) {
209-
J.AnnotatedType annotatedType = (J.AnnotatedType) mv.getTypeExpression();
210-
J.ParameterizedType parameterizedType = (J.ParameterizedType) annotatedType.getTypeExpression();
211-
typeExpression = annotatedType.withTypeExpression(removeFromParameterizedType(newType, parameterizedType));
212-
} else {
213-
J.ParameterizedType parameterizedType = (J.ParameterizedType) mv.getTypeExpression();
214-
typeExpression = removeFromParameterizedType(newType, parameterizedType);
215-
}
216-
217-
mv = mv.withTypeExpression(typeExpression);
191+
mv = mv.withTypeExpression(getTypeTree(mv.getTypeExpression(), newType));
218192
mv = mv.withVariables(ListUtils.map(mv.getVariables(), var -> {
219193
JavaType.FullyQualified varType = TypeUtils.asFullyQualified(var.getType());
220194
if (varType != null && !varType.equals(newType)) {
@@ -227,18 +201,53 @@ public J.VariableDeclarations visitVariableDeclarations(J.VariableDeclarations m
227201
return mv;
228202
}
229203

230-
private J.MethodInvocation updateMethodInvocation(J.MethodInvocation mi, JavaType.FullyQualified newType) {
231-
if (mi.getSelect() != null) {
232-
mi = mi.withSelect(mi.getSelect().withType(newType));
233-
if (mi.getSelect() instanceof J.FieldAccess) {
234-
J.FieldAccess fieldAccess = (J.FieldAccess) mi.getSelect();
235-
mi = mi.withSelect(fieldAccess.withName(fieldAccess.getName().withType(newType)));
236-
}
204+
@Contract("null, _ -> null")
205+
private @Nullable TypeTree getTypeTree(@Nullable TypeTree inputTypeExpression, JavaType.FullyQualified newType) {
206+
if (inputTypeExpression == null) {
207+
return null;
237208
}
238-
if (mi.getMethodType() != null) {
239-
mi = mi.withMethodType(mi.getMethodType().withDeclaringType(newType));
209+
if (inputTypeExpression instanceof J.Identifier) {
210+
return new J.Identifier(
211+
randomId(),
212+
inputTypeExpression.getPrefix(),
213+
Markers.EMPTY,
214+
emptyList(),
215+
newType.getClassName(),
216+
newType,
217+
null
218+
);
240219
}
241-
return mi;
220+
if (inputTypeExpression instanceof J.FieldAccess) {
221+
// Fully-qualified type name like java.util.HashSet
222+
return new J.Identifier(
223+
randomId(),
224+
inputTypeExpression.getPrefix(),
225+
Markers.EMPTY,
226+
emptyList(),
227+
newType.getClassName(),
228+
newType,
229+
null
230+
);
231+
}
232+
if (inputTypeExpression instanceof J.AnnotatedType) {
233+
J.AnnotatedType annotatedType = (J.AnnotatedType) inputTypeExpression;
234+
TypeTree annotatedTypeExpression = annotatedType.getTypeExpression();
235+
if (annotatedTypeExpression instanceof J.Identifier || annotatedTypeExpression instanceof J.FieldAccess) {
236+
return annotatedType.withTypeExpression(new J.Identifier(
237+
randomId(),
238+
annotatedTypeExpression.getPrefix(),
239+
Markers.EMPTY,
240+
emptyList(),
241+
newType.getClassName(),
242+
newType,
243+
null
244+
));
245+
}
246+
J.ParameterizedType parameterizedType = (J.ParameterizedType) annotatedTypeExpression;
247+
return annotatedType.withTypeExpression(removeFromParameterizedType(newType, parameterizedType));
248+
}
249+
J.ParameterizedType parameterizedType = (J.ParameterizedType) inputTypeExpression;
250+
return removeFromParameterizedType(newType, parameterizedType);
242251
}
243252

244253
private TypeTree removeFromParameterizedType(JavaType.FullyQualified newType,

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

Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1211,4 +1211,142 @@ Enumeration<String> usesVectorElements() {
12111211
)
12121212
);
12131213
}
1214+
1215+
@Issue("https://github.com/openrewrite/rewrite-static-analysis/issues/713")
1216+
@Test
1217+
void annotatedReturnTypeRawArrayList() {
1218+
rewriteRun(
1219+
spec -> spec.parser(JavaParser.fromJavaVersion().classpath("jspecify")),
1220+
//language=java
1221+
java(
1222+
"""
1223+
import java.util.ArrayList;
1224+
import org.jspecify.annotations.Nullable;
1225+
1226+
class Test {
1227+
public @Nullable ArrayList transform() {
1228+
ArrayList res = new ArrayList();
1229+
return res;
1230+
}
1231+
}
1232+
""",
1233+
"""
1234+
import java.util.ArrayList;
1235+
import java.util.List;
1236+
1237+
import org.jspecify.annotations.Nullable;
1238+
1239+
class Test {
1240+
public @Nullable List transform() {
1241+
List res = new ArrayList();
1242+
return res;
1243+
}
1244+
}
1245+
"""
1246+
)
1247+
);
1248+
}
1249+
1250+
@Issue("https://github.com/openrewrite/rewrite-static-analysis/issues/713")
1251+
@Test
1252+
void annotatedFieldTypeRawArrayList() {
1253+
rewriteRun(
1254+
spec -> spec.parser(JavaParser.fromJavaVersion().classpath("jspecify")),
1255+
//language=java
1256+
java(
1257+
"""
1258+
import java.util.ArrayList;
1259+
import org.jspecify.annotations.Nullable;
1260+
1261+
class Test {
1262+
public @Nullable ArrayList values = new ArrayList();
1263+
}
1264+
""",
1265+
"""
1266+
import java.util.ArrayList;
1267+
import java.util.List;
1268+
1269+
import org.jspecify.annotations.Nullable;
1270+
1271+
class Test {
1272+
public @Nullable List values = new ArrayList();
1273+
}
1274+
"""
1275+
)
1276+
);
1277+
}
1278+
1279+
@Issue("https://github.com/openrewrite/rewrite-static-analysis/issues/716")
1280+
@Test
1281+
void fullyQualifiedRawType() {
1282+
rewriteRun(
1283+
//language=java
1284+
java(
1285+
"""
1286+
class Test {
1287+
public java.util.HashSet method() {
1288+
return new java.util.HashSet<>();
1289+
}
1290+
}
1291+
""",
1292+
"""
1293+
import java.util.Set;
1294+
1295+
class Test {
1296+
public Set method() {
1297+
return new java.util.HashSet<>();
1298+
}
1299+
}
1300+
"""
1301+
)
1302+
);
1303+
}
1304+
1305+
@Issue("https://github.com/openrewrite/rewrite-static-analysis/issues/716")
1306+
@Test
1307+
void fullyQualifiedParameterizedType() {
1308+
rewriteRun(
1309+
//language=java
1310+
java(
1311+
"""
1312+
class Test {
1313+
public java.util.HashSet<String> method() {
1314+
return new java.util.HashSet<>();
1315+
}
1316+
}
1317+
""",
1318+
"""
1319+
import java.util.Set;
1320+
1321+
class Test {
1322+
public Set<String> method() {
1323+
return new java.util.HashSet<>();
1324+
}
1325+
}
1326+
"""
1327+
)
1328+
);
1329+
}
1330+
1331+
@Issue("https://github.com/openrewrite/rewrite-static-analysis/issues/716")
1332+
@Test
1333+
void fullyQualifiedFieldType() {
1334+
rewriteRun(
1335+
//language=java
1336+
java(
1337+
"""
1338+
class Test {
1339+
public java.util.HashSet values = new java.util.HashSet();
1340+
}
1341+
""",
1342+
"""
1343+
import java.util.Set;
1344+
1345+
class Test {
1346+
public Set values = new java.util.HashSet();
1347+
}
1348+
"""
1349+
)
1350+
);
1351+
}
12141352
}

0 commit comments

Comments
 (0)