Skip to content

Commit

Permalink
Do not convert return types in NoGuavaJava21 by default (#601)
Browse files Browse the repository at this point in the history
* [HotFix] NoGuavaJava21 causing invalid code since v2.28.0

* Apply formatter to minimize diff

* add Ability To Convert Return Type as an option

* Apply suggestions from code review

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* fix conflicts

* Apply formatter

* Minimize diff by restoring original method order

* Minimize diff by restoring original method order

* Move tests into nested sub class

* Correctly evaluate nullable Boolean

---------

Co-authored-by: Tim te Beek <tim@moderne.io>
Co-authored-by: Tim te Beek <timtebeek@gmail.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
  • Loading branch information
4 people authored Nov 25, 2024
1 parent d6ecd54 commit 59d56a6
Show file tree
Hide file tree
Showing 8 changed files with 461 additions and 283 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,25 @@ abstract class AbstractNoGuavaImmutableOf extends Recipe {
private final String guavaType;
private final String javaType;

@Option(displayName = "Whether to convert return type (the default value is false).",
description = "converting the return type from Guava Type to Java Type " +
"The default value is false.",
example = "true",
required = false)
@Nullable
Boolean convertReturnType;

AbstractNoGuavaImmutableOf(String guavaType, String javaType) {
this.guavaType = guavaType;
this.javaType = javaType;
}

AbstractNoGuavaImmutableOf(String guavaType, String javaType, @Nullable Boolean convertReturnType) {
this.guavaType = guavaType;
this.javaType = javaType;
this.convertReturnType = convertReturnType;
}

private String getShortType(String fullyQualifiedType) {
return fullyQualifiedType.substring(javaType.lastIndexOf(".") + 1);
}
Expand Down Expand Up @@ -103,8 +117,9 @@ public J visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx)
@Override
public J.VariableDeclarations visitVariableDeclarations(J.VariableDeclarations multiVariable, ExecutionContext ctx) {
J.VariableDeclarations mv = (J.VariableDeclarations) super.visitVariableDeclarations(multiVariable, ctx);

if (multiVariable != mv && TypeUtils.isOfClassType(mv.getType(), guavaType)) {
if (Boolean.TRUE.equals(convertReturnType) &&
multiVariable != mv &&
TypeUtils.isOfClassType(mv.getType(), guavaType)) {
JavaType newType = JavaType.buildType(javaType);
mv = mv.withTypeExpression(mv.getTypeExpression() == null ?
null : createNewTypeExpression(mv.getTypeExpression(), newType));
Expand All @@ -117,7 +132,6 @@ public J.VariableDeclarations visitVariableDeclarations(J.VariableDeclarations m
return variable;
}));
}

return mv;
}

Expand Down Expand Up @@ -148,7 +162,6 @@ private TypeTree createNewTypeExpression(TypeTree typeTree, JavaType newType) {
);
}


private boolean isParentTypeDownCast(MethodCall immutableMethod) {
J parent = getCursor().dropParentUntil(J.class::isInstance).getValue();
boolean isParentTypeDownCast = false;
Expand All @@ -173,10 +186,12 @@ private boolean isParentTypeDownCast(MethodCall immutableMethod) {
} else if (parent instanceof J.MethodInvocation) {
J.MethodInvocation m = (J.MethodInvocation) parent;
int index = m.getArguments().indexOf(immutableMethod);
if (m.getMethodType() != null && index != -1 && !m.getMethodType().getParameterTypes().isEmpty()) {
isParentTypeDownCast = isParentTypeMatched(m.getMethodType().getParameterTypes().get(index));
} else {
isParentTypeDownCast = true;
if (m.getMethodType() != null) {
if (index != -1 && !m.getMethodType().getParameterTypes().isEmpty()) {
isParentTypeDownCast = isParentTypeMatched(m.getMethodType().getParameterTypes().get(index));
} else {
isParentTypeDownCast = !TypeUtils.isOfClassType(m.getMethodType().getReturnType(), guavaType);
}
}
} else if (parent instanceof J.NewClass) {
J.NewClass c = (J.NewClass) parent;
Expand Down Expand Up @@ -207,8 +222,8 @@ private boolean isParentTypeDownCast(MethodCall immutableMethod) {
private boolean isParentTypeMatched(@Nullable JavaType type) {
JavaType.FullyQualified fq = TypeUtils.asFullyQualified(type);
return TypeUtils.isOfClassType(fq, javaType) ||
TypeUtils.isOfClassType(fq, "java.lang.Object") ||
TypeUtils.isOfClassType(fq, guavaType);
(Boolean.TRUE.equals(convertReturnType) && TypeUtils.isOfClassType(fq, guavaType)) ||
TypeUtils.isOfClassType(fq, "java.lang.Object");
}
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,14 @@
*/
package org.openrewrite.java.migrate.guava;

import org.jspecify.annotations.Nullable;

public class NoGuavaImmutableListOf extends AbstractNoGuavaImmutableOf {
public NoGuavaImmutableListOf() {
super("com.google.common.collect.ImmutableList", "java.util.List");
}

public NoGuavaImmutableListOf(@Nullable Boolean convertReturnType) {
super("com.google.common.collect.ImmutableList", "java.util.List", convertReturnType);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,14 @@
*/
package org.openrewrite.java.migrate.guava;

import org.jspecify.annotations.Nullable;

public class NoGuavaImmutableMapOf extends AbstractNoGuavaImmutableOf {
public NoGuavaImmutableMapOf() {
super("com.google.common.collect.ImmutableMap", "java.util.Map");
}

public NoGuavaImmutableMapOf(@Nullable Boolean convertReturnType) {
super("com.google.common.collect.ImmutableMap", "java.util.Map", convertReturnType);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,14 @@
*/
package org.openrewrite.java.migrate.guava;

import org.jspecify.annotations.Nullable;

public class NoGuavaImmutableSetOf extends AbstractNoGuavaImmutableOf {
public NoGuavaImmutableSetOf() {
super("com.google.common.collect.ImmutableSet", "java.util.Set");
}

public NoGuavaImmutableSetOf(@Nullable Boolean convertReturnType) {
super("com.google.common.collect.ImmutableSet", "java.util.Set", convertReturnType);
}
}
Loading

0 comments on commit 59d56a6

Please sign in to comment.