Skip to content

Fix NPE in UseCollectionInterfaces with annotated raw collection types #717

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

Merged
merged 6 commits into from
Aug 19, 2025
Merged

Fix NPE in UseCollectionInterfaces with annotated raw collection types #717

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
48e3f9f
Fix NPE/ClassCastException in UseCollectionInterfaces with annotated …
timtebeek Aug 19, 2025
872087c
Deduplicate
timtebeek Aug 19, 2025
00d93a3
Merge branch 'main' into fix-713-use-collection-interfaces-npe
timtebeek Aug 19, 2025
9ba33bc
Apply suggestions from code review
timtebeek Aug 19, 2025
2d6263f
Revert "Apply suggestions from code review"
timtebeek Aug 19, 2025
0e96f62
Remove redundant else after return
timtebeek Aug 19, 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
155 changes: 82 additions & 73 deletions src/main/java/org/openrewrite/staticanalysis/UseCollectionInterfaces.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/
package org.openrewrite.staticanalysis;

import org.jetbrains.annotations.Contract;
import org.jspecify.annotations.Nullable;
import org.openrewrite.*;
import org.openrewrite.groovy.tree.G;
Expand Down Expand Up @@ -114,46 +115,6 @@ public J visit(@Nullable Tree tree, ExecutionContext ctx) {
return super.visit(tree, ctx);
}

@Override
public J.MethodDeclaration visitMethodDeclaration(J.MethodDeclaration method, ExecutionContext ctx) {
J.MethodDeclaration m = super.visitMethodDeclaration(method, ctx);
if ((m.hasModifier(J.Modifier.Type.Public) || m.hasModifier(J.Modifier.Type.Private) || m.getModifiers().isEmpty()) &&
m.getReturnTypeExpression() != null) {
JavaType.FullyQualified originalType = TypeUtils.asFullyQualified(m.getReturnTypeExpression().getType());
if (originalType != null && rspecRulesReplaceTypeMap.containsKey(originalType.getFullyQualifiedName())) {

JavaType.FullyQualified newType = TypeUtils.asFullyQualified(
JavaType.buildType(rspecRulesReplaceTypeMap.get(originalType.getFullyQualifiedName())));
if (newType != null) {
maybeRemoveImport(originalType);
maybeAddImport(newType);

TypeTree typeExpression;
if (m.getReturnTypeExpression() instanceof J.Identifier) {
typeExpression = new J.Identifier(
randomId(),
m.getReturnTypeExpression().getPrefix(),
Markers.EMPTY,
emptyList(),
newType.getClassName(),
newType,
null
);
} else if (m.getReturnTypeExpression() instanceof J.AnnotatedType) {
J.AnnotatedType annotatedType = (J.AnnotatedType) m.getReturnTypeExpression();
J.ParameterizedType parameterizedType = (J.ParameterizedType) annotatedType.getTypeExpression();
typeExpression = annotatedType.withTypeExpression(removeFromParameterizedType(newType, parameterizedType));
} else {
J.ParameterizedType parameterizedType = (J.ParameterizedType) m.getReturnTypeExpression();
typeExpression = removeFromParameterizedType(newType, parameterizedType);
}
m = m.withReturnTypeExpression(typeExpression);
}
}
}
return m;
}

@Override
public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) {
J.MethodInvocation mi = super.visitMethodInvocation(method, ctx);
Expand All @@ -176,6 +137,41 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Execu
return mi;
}

private J.MethodInvocation updateMethodInvocation(J.MethodInvocation mi, JavaType.FullyQualified newType) {
if (mi.getSelect() != null) {
mi = mi.withSelect(mi.getSelect().withType(newType));
if (mi.getSelect() instanceof J.FieldAccess) {
J.FieldAccess fieldAccess = (J.FieldAccess) mi.getSelect();
mi = mi.withSelect(fieldAccess.withName(fieldAccess.getName().withType(newType)));
}
}
if (mi.getMethodType() != null) {
mi = mi.withMethodType(mi.getMethodType().withDeclaringType(newType));
}
return mi;
}

@Override
public J.MethodDeclaration visitMethodDeclaration(J.MethodDeclaration method, ExecutionContext ctx) {
J.MethodDeclaration m = super.visitMethodDeclaration(method, ctx);
if ((m.hasModifier(J.Modifier.Type.Public) || m.hasModifier(J.Modifier.Type.Private) || m.getModifiers().isEmpty()) &&
m.getReturnTypeExpression() != null) {
JavaType.FullyQualified originalType = TypeUtils.asFullyQualified(m.getReturnTypeExpression().getType());
if (originalType != null && rspecRulesReplaceTypeMap.containsKey(originalType.getFullyQualifiedName())) {

JavaType.FullyQualified newType = TypeUtils.asFullyQualified(
JavaType.buildType(rspecRulesReplaceTypeMap.get(originalType.getFullyQualifiedName())));
if (newType != null) {
maybeRemoveImport(originalType);
maybeAddImport(newType);

m = m.withReturnTypeExpression(getTypeTree(m.getReturnTypeExpression(), newType));
}
}
}
return m;
}

@Override
public J.VariableDeclarations visitVariableDeclarations(J.VariableDeclarations multiVariable, ExecutionContext ctx) {
J.VariableDeclarations mv = super.visitVariableDeclarations(multiVariable, ctx);
Expand All @@ -192,29 +188,7 @@ public J.VariableDeclarations visitVariableDeclarations(J.VariableDeclarations m
maybeRemoveImport(originalType);
maybeAddImport(newType);

TypeTree typeExpression;
if (mv.getTypeExpression() == null) {
typeExpression = null;
} else if (mv.getTypeExpression() instanceof J.Identifier) {
typeExpression = new J.Identifier(
randomId(),
mv.getTypeExpression().getPrefix(),
Markers.EMPTY,
emptyList(),
newType.getClassName(),
newType,
null
);
} else if (mv.getTypeExpression() instanceof J.AnnotatedType) {
J.AnnotatedType annotatedType = (J.AnnotatedType) mv.getTypeExpression();
J.ParameterizedType parameterizedType = (J.ParameterizedType) annotatedType.getTypeExpression();
typeExpression = annotatedType.withTypeExpression(removeFromParameterizedType(newType, parameterizedType));
} else {
J.ParameterizedType parameterizedType = (J.ParameterizedType) mv.getTypeExpression();
typeExpression = removeFromParameterizedType(newType, parameterizedType);
}

mv = mv.withTypeExpression(typeExpression);
mv = mv.withTypeExpression(getTypeTree(mv.getTypeExpression(), newType));
mv = mv.withVariables(ListUtils.map(mv.getVariables(), var -> {
JavaType.FullyQualified varType = TypeUtils.asFullyQualified(var.getType());
if (varType != null && !varType.equals(newType)) {
Expand All @@ -227,18 +201,53 @@ public J.VariableDeclarations visitVariableDeclarations(J.VariableDeclarations m
return mv;
}

private J.MethodInvocation updateMethodInvocation(J.MethodInvocation mi, JavaType.FullyQualified newType) {
if (mi.getSelect() != null) {
mi = mi.withSelect(mi.getSelect().withType(newType));
if (mi.getSelect() instanceof J.FieldAccess) {
J.FieldAccess fieldAccess = (J.FieldAccess) mi.getSelect();
mi = mi.withSelect(fieldAccess.withName(fieldAccess.getName().withType(newType)));
}
@Contract("null, _ -> null")
private @Nullable TypeTree getTypeTree(@Nullable TypeTree inputTypeExpression, JavaType.FullyQualified newType) {
if (inputTypeExpression == null) {
return null;
}
if (mi.getMethodType() != null) {
mi = mi.withMethodType(mi.getMethodType().withDeclaringType(newType));
if (inputTypeExpression instanceof J.Identifier) {
return new J.Identifier(
randomId(),
inputTypeExpression.getPrefix(),
Markers.EMPTY,
emptyList(),
newType.getClassName(),
newType,
null
);
}
return mi;
if (inputTypeExpression instanceof J.FieldAccess) {
// Fully-qualified type name like java.util.HashSet
return new J.Identifier(
randomId(),
inputTypeExpression.getPrefix(),
Markers.EMPTY,
emptyList(),
newType.getClassName(),
newType,
null
);
}
if (inputTypeExpression instanceof J.AnnotatedType) {
J.AnnotatedType annotatedType = (J.AnnotatedType) inputTypeExpression;
TypeTree annotatedTypeExpression = annotatedType.getTypeExpression();
if (annotatedTypeExpression instanceof J.Identifier || annotatedTypeExpression instanceof J.FieldAccess) {
return annotatedType.withTypeExpression(new J.Identifier(
randomId(),
annotatedTypeExpression.getPrefix(),
Markers.EMPTY,
emptyList(),
newType.getClassName(),
newType,
null
));
}
J.ParameterizedType parameterizedType = (J.ParameterizedType) annotatedTypeExpression;
return annotatedType.withTypeExpression(removeFromParameterizedType(newType, parameterizedType));
}
J.ParameterizedType parameterizedType = (J.ParameterizedType) inputTypeExpression;
return removeFromParameterizedType(newType, parameterizedType);
}

private TypeTree removeFromParameterizedType(JavaType.FullyQualified newType,
Expand Down
138 changes: 138 additions & 0 deletions src/test/java/org/openrewrite/staticanalysis/UseCollectionInterfacesTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1211,4 +1211,142 @@ Enumeration<String> usesVectorElements() {
)
);
}

@Issue("https://github.com/openrewrite/rewrite-static-analysis/issues/713")
@Test
void annotatedReturnTypeRawArrayList() {
rewriteRun(
spec -> spec.parser(JavaParser.fromJavaVersion().classpath("jspecify")),
//language=java
java(
"""
import java.util.ArrayList;
import org.jspecify.annotations.Nullable;

class Test {
public @Nullable ArrayList transform() {
ArrayList res = new ArrayList();
return res;
}
}
""",
"""
import java.util.ArrayList;
import java.util.List;

import org.jspecify.annotations.Nullable;

class Test {
public @Nullable List transform() {
List res = new ArrayList();
return res;
}
}
"""
)
);
}

@Issue("https://github.com/openrewrite/rewrite-static-analysis/issues/713")
@Test
void annotatedFieldTypeRawArrayList() {
rewriteRun(
spec -> spec.parser(JavaParser.fromJavaVersion().classpath("jspecify")),
//language=java
java(
"""
import java.util.ArrayList;
import org.jspecify.annotations.Nullable;

class Test {
public @Nullable ArrayList values = new ArrayList();
}
""",
"""
import java.util.ArrayList;
import java.util.List;

import org.jspecify.annotations.Nullable;

class Test {
public @Nullable List values = new ArrayList();
}
"""
)
);
}

@Issue("https://github.com/openrewrite/rewrite-static-analysis/issues/716")
@Test
void fullyQualifiedRawType() {
rewriteRun(
//language=java
java(
"""
class Test {
public java.util.HashSet method() {
return new java.util.HashSet<>();
}
}
""",
"""
import java.util.Set;

class Test {
public Set method() {
return new java.util.HashSet<>();
}
}
"""
)
);
}

@Issue("https://github.com/openrewrite/rewrite-static-analysis/issues/716")
@Test
void fullyQualifiedParameterizedType() {
rewriteRun(
//language=java
java(
"""
class Test {
public java.util.HashSet<String> method() {
return new java.util.HashSet<>();
}
}
""",
"""
import java.util.Set;

class Test {
public Set<String> method() {
return new java.util.HashSet<>();
}
}
"""
)
);
}

@Issue("https://github.com/openrewrite/rewrite-static-analysis/issues/716")
@Test
void fullyQualifiedFieldType() {
rewriteRun(
//language=java
java(
"""
class Test {
public java.util.HashSet values = new java.util.HashSet();
}
""",
"""
import java.util.Set;

class Test {
public Set values = new java.util.HashSet();
}
"""
)
);
}
}