Skip to content

Cast when needed on lambda expressions #207

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
joanvr merged 9 commits into from
Nov 8, 2023
Merged

Cast when needed on lambda expressions #207

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
971eb18
WIP
Oct 24, 2023
7284896
Merge branch 'main' into fix/cast-lambda-generics
Oct 31, 2023
05e1a3d
Simplify test a bit
knutwannheden Nov 2, 2023
727aeed
Merge branch 'main' into fix/cast-lambda-generics
Nov 6, 2023
fa6ddfa
remove redundant type cast
Nov 6, 2023
0431e9d
Fix one more edge case
knutwannheden Nov 7, 2023
19a9159
Don't remove cast when expression is lambda being cast to generic type
knutwannheden Nov 7, 2023
2b0cbc9
Merge branch 'main' into fix/cast-lambda-generics
knutwannheden Nov 7, 2023
2dc4f9c
remove redundant type cast
Nov 8, 2023
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
4 changes: 3 additions & 1 deletion src/main/java/org/openrewrite/staticanalysis/RemoveRedundantTypeCast.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,9 @@ public J visitTypeCast(J.TypeCast typeCast, ExecutionContext ctx) {
JavaType expressionType = visitedTypeCast.getExpression().getType();
JavaType castType = visitedTypeCast.getType();

if (targetType == null || targetType instanceof JavaType.Primitive && castType != expressionType) {
if (targetType == null ||
targetType instanceof JavaType.Primitive && castType != expressionType ||
typeCast.getExpression() instanceof J.Lambda && castType instanceof JavaType.Parameterized) {
// Not currently supported, this will be more accurate with dataflow analysis.
return visitedTypeCast;
} else if (!(targetType instanceof JavaType.Array) && TypeUtils.isOfClassType(targetType, "java.lang.Object") ||
Expand Down
70 changes: 47 additions & 23 deletions src/main/java/org/openrewrite/staticanalysis/UseLambdaForFunctionalInterface.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,35 +132,17 @@ public J visitNewClass(J.NewClass newClass, ExecutionContext ctx) {
return n;
}

@Nullable
private JavaType.Method getSamCompatible(JavaType type) {
JavaType.Method sam = null;
JavaType.FullyQualified fullyQualified = TypeUtils.asFullyQualified(type);
if (fullyQualified == null) {
return null;
}
for (JavaType.Method method : fullyQualified.getMethods()) {
if (method.hasFlags(Flag.Default) || method.hasFlags(Flag.Static)) {
continue;
}
if (sam != null) {
return null;
}
sam = method;
}
return sam;
}

private J maybeAddCast(J.Lambda lambda, J.NewClass original) {
J parent = getCursor().getParentTreeCursor().getValue();

if (parent instanceof J.MethodInvocation) {
J.MethodInvocation method = (J.MethodInvocation) parent;
if (parent instanceof MethodCall) {
MethodCall method = (MethodCall) parent;
List<Expression> arguments = method.getArguments();
for (int i = 0; i < arguments.size(); i++) {
Expression argument = arguments.get(i);
if (argument == original && methodArgumentRequiresCast(lambda, method, i) &&
original.getClazz() != null) {
doAfterVisit(new RemoveRedundantTypeCast().getVisitor());
return new J.TypeCast(
Tree.randomId(),
lambda.getPrefix(),
Expand All @@ -180,7 +162,7 @@ private J maybeAddCast(J.Lambda lambda, J.NewClass original) {
return lambda;
}

private boolean methodArgumentRequiresCast(J.Lambda lambda, J.MethodInvocation method, int argumentIndex) {
private boolean methodArgumentRequiresCast(J.Lambda lambda, MethodCall method, int argumentIndex) {
JavaType.FullyQualified lambdaType = TypeUtils.asFullyQualified(lambda.getType());
if (lambdaType == null) {
return false;
Expand All @@ -207,7 +189,11 @@ private boolean methodArgumentRequiresCast(J.Lambda lambda, J.MethodInvocation m
}
}
}
return count >= 2;
if (count >= 2) {
return true;
}

return hasGenerics(lambda);
}

private boolean areMethodsAmbiguous(@Nullable JavaType.Method m1, @Nullable JavaType.Method m2) {
Expand Down Expand Up @@ -419,4 +405,42 @@ public J visitVariable(J.VariableDeclarations.NamedVariable variable, Integer in

return hasShadow.get();
}

private static boolean hasGenerics(J.Lambda lambda) {
AtomicBoolean atomicBoolean = new AtomicBoolean();
new JavaVisitor<AtomicBoolean>() {
@Override
public J visitMethodInvocation(J.MethodInvocation method, AtomicBoolean atomicBoolean) {
if (method.getMethodType() != null &&
method.getMethodType().getParameterTypes().stream()
.anyMatch(p -> p instanceof JavaType.Parameterized &&
((JavaType.Parameterized) p).getTypeParameters().stream().anyMatch(t -> t instanceof JavaType.GenericTypeVariable))
) {
atomicBoolean.set(true);
}
return super.visitMethodInvocation(method, atomicBoolean);
}
}.visit(lambda.getBody(), atomicBoolean);
return atomicBoolean.get();
}

// TODO consider moving to TypeUtils
@Nullable
private static JavaType.Method getSamCompatible(@Nullable JavaType type) {
JavaType.Method sam = null;
JavaType.FullyQualified fullyQualified = TypeUtils.asFullyQualified(type);
if (fullyQualified == null) {
return null;
}
for (JavaType.Method method : fullyQualified.getMethods()) {
if (method.hasFlags(Flag.Default) || method.hasFlags(Flag.Static)) {
continue;
}
if (sam != null) {
return null;
}
sam = method;
}
return sam;
}
}
46 changes: 21 additions & 25 deletions src/test/java/org/openrewrite/staticanalysis/RemoveRedundantTypeCastTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,24 @@ public List<String> get() {
);
}

@Test
void nonSamParameter() {
rewriteRun(
//language=java
java(
"""
import java.util.*;

class Test {
public boolean foo() {
return Objects.equals("x", (Comparable<String>) (s) -> 1);
}
}
"""
)
);
}

@Issue("https://github.com/openrewrite/rewrite/issues/1647")
@Test
void redundantTypeCast() {
Expand Down Expand Up @@ -374,8 +392,7 @@ class ExtendTest extends Test {


@Test
@Issue("https://github.com/moderneinc/support-app/issues/17")
void test() {
void lambdaWithComplexTypeInference() {
rewriteRun(
java(
"""
Expand All @@ -390,29 +407,8 @@ void method() {
(Supplier<Map<String, Integer>>) () -> {
Map<String, Integer> choices = Map.of("id1", 2);
return choices.entrySet().stream()
.collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
});
}
}

class MapDropdownChoice<K, V> {
public MapDropdownChoice(Supplier<? extends Map<K, ? extends V>> choiceMap) {
}
}
""",
"""
import java.util.LinkedHashMap;
import java.util.Map;
import java.util.function.Supplier;
import java.util.stream.Collectors;

class Test {
void method() {
Object o2 = new MapDropdownChoice<String, Integer>(
() -> {
Map<String, Integer> choices = Map.of("id1", 2);
return choices.entrySet().stream()
.collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
.collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue,
(e1, e2) -> e1, LinkedHashMap::new));
});
}
}
Expand Down
71 changes: 71 additions & 0 deletions src/test/java/org/openrewrite/staticanalysis/UseLambdaForFunctionalInterfaceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -664,4 +664,75 @@ public void run() {
)
);
}

@Test
@Issue("https://github.com/moderneinc/support-app/issues/17")
void lambdaWithComplexTypeInference() {
rewriteRun(
java(
"""
import java.util.LinkedHashMap;
import java.util.Map;
import java.util.function.Supplier;
import java.util.stream.Collectors;

class Test {
void method() {
Object o = new MapDropdownChoice<String, Integer>(
new Supplier<Map<String, Integer>>() {
@Override
public Map<String, Integer> getObject() {
Map<String, Integer> choices = Map.of("id1", 1);
return choices.entrySet().stream()
.collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue, (e1, e2) -> e1, LinkedHashMap::new));
}
});
Object o2 = new MapDropdownChoice<String, Integer>(
new Supplier<Map<String, Integer>>() {
@Override
public Map<String, Integer> getObject() {
Map<String, Integer> choices = Map.of("id1", 2);
return choices.entrySet().stream()
.collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
}
});
}
}

class MapDropdownChoice<K, V> {
public MapDropdownChoice(Supplier<? extends Map<K, ? extends V>> choiceMap) {
}
}
""",
"""
import java.util.LinkedHashMap;
import java.util.Map;
import java.util.function.Supplier;
import java.util.stream.Collectors;

class Test {
void method() {
Object o = new MapDropdownChoice<String, Integer>(
(Supplier<Map<String, Integer>>) () -> {
Map<String, Integer> choices = Map.of("id1", 1);
return choices.entrySet().stream()
.collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue, (e1, e2) -> e1, LinkedHashMap::new));
});
Object o2 = new MapDropdownChoice<String, Integer>(
(Supplier<Map<String, Integer>>) () -> {
Map<String, Integer> choices = Map.of("id1", 2);
return choices.entrySet().stream()
.collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
});
}
}

class MapDropdownChoice<K, V> {
public MapDropdownChoice(Supplier<? extends Map<K, ? extends V>> choiceMap) {
}
}
"""
)
);
}
}